Re: [U-Boot] [PATCH] net: phy: Set SUPPORTED_1000baseX_Half flag in ESTATUS_1000_XHALF case
On Fri, Jul 19, 2013 at 9:01 AM, Fabio Estevam feste...@gmail.com wrote: From: Fabio Estevam fabio.este...@freescale.com Commit de1d786e (add support for Xilinx 1000BASE-X phy (GTX)) introduced the checking for ESTATUS_1000_XHALF, but it incorrectly sets the SUPPORTED_1000baseX_Full flag in this case. Signed-off-by: Charles Coldwell coldw...@gmail.com -- Charles M. Coldwell, W1CMC Belmont, Massachusetts, New England Turn on, log in, tune out ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] net: phy: Set SUPPORTED_1000baseX_Half flag in ESTATUS_1000_XHALF case
On Fri, Jul 19, 2013 at 10:01 AM, Fabio Estevam feste...@gmail.com wrote: Hi Charles, On Fri, Jul 19, 2013 at 10:59 AM, Charles Coldwell coldw...@gmail.com wrote: On Fri, Jul 19, 2013 at 9:01 AM, Fabio Estevam feste...@gmail.com wrote: From: Fabio Estevam fabio.este...@freescale.com Commit de1d786e (add support for Xilinx 1000BASE-X phy (GTX)) introduced the checking for ESTATUS_1000_XHALF, but it incorrectly sets the SUPPORTED_1000baseX_Full flag in this case. Signed-off-by: Charles Coldwell coldw...@gmail.com I guess you meant Acked-by instead? Sigh. Indeed, acked. My LKML skills are rusty. -- Charles M. Coldwell, W1CMC Belmont, Massachusetts, New England Turn on, log in, tune out ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] Ethernet support broken for Wandboard Quad on master
It's difficult. The board has been taken away from me. It was a Virtex 6 FPGA running Microblaze Linux; I might be able to test on a Virtex 7 but not for several days. Sascha Silbe's fix looks good to me, however. On Fri, Jul 19, 2013 at 8:42 AM, Tom Rini tr...@ti.com wrote: On Fri, Jul 19, 2013 at 09:38:42AM -0300, Fabio Estevam wrote: On Fri, Jul 19, 2013 at 7:25 AM, Sascha Silbe t-ub...@infra-silbe.de wrote: Hello Charles, [CC += a few people that were CC'ed on the revert of Charles' patch] Charles Coldwell coldw...@gmail.com writes: I've never heard of the Wandboard Quad, so I suppose the short answer is no. However, the philosophy of the patch I submitted was: [...] Thanks for the description and the pointer to the Xilinx register description. I think I got to the bottom of it. The Xilinx PHY supports the GMII basic register set (registers 0, 1 and 15), but not the full extended register set (registers 2-14). Especially the MASTER-SLAVE Control and Status registers (IEEE 802.3 terminology) are missing. Bit 0 (Extended Capability) of the (non-Extended) Status register is correctly set to 0 to indicate this lack of support. Without the MASTER-SLAVE Status register, we can't tell whether the _peer_ also supports 1Gbps operation. Your patch ends up enabling it anyway, even for 10/100Mbps peers. Can you try the patch below, please? It restricts Extended Status processing to the PHYs that don't support the MASTER-SLAVE Control and Status registers, like the Xilinx one you use. The other PHYs should continue to work as before your patch. Tested successfully on Wandboard Quad. Thanks for your detailed analysis, Sascha. On a mx6qsabresd: Tested-by: Fabio Estevam fabio.este...@freescale.com Hopefully we can get it applied into the upcoming 2013.07 in order to avoid the regression on some mx6 boards. Can we also get this tested on the board Charles has? Thanks! -- Tom -- Charles M. Coldwell, W1CMC Belmont, Massachusetts, New England Turn on, log in, tune out ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] net: phy: Set SUPPORTED_1000baseX_Half flag in ESTATUS_1000_XHALF case
On Fri, Jul 19, 2013 at 12:55 PM, Tom Rini tr...@ti.com wrote: On Fri, Jul 19, 2013 at 10:01:34AM -0300, Fabio Estevam wrote: From: Fabio Estevam fabio.este...@freescale.com Commit de1d786e (add support for Xilinx 1000BASE-X phy (GTX)) introduced the checking for ESTATUS_1000_XHALF, but it incorrectly sets the SUPPORTED_1000baseX_Full flag in this case. Set the SUPPORTED_1000baseX_Half flag instead. Signed-off-by: Fabio Estevam fabio.este...@freescale.com So, do we need both patches to fix the problem? One patch fixes the problem; the other fixes another problem we haven't seen yet (but will). -- Charles M. Coldwell, W1CMC Belmont, Massachusetts, New England Turn on, log in, tune out ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] Ethernet support broken for Wandboard Quad on master
On Thu, Jul 18, 2013 at 5:07 AM, Sascha Silbe t-ub...@infra-silbe.de wrote: Charles, do you have any idea why your commit breaks ethernet support on Wandboard Quad? I've never heard of the Wandboard Quad, so I suppose the short answer is no. However, the philosophy of the patch I submitted was: 1. Check the ESTATEN (Extended Status Enabled) bit in the BMSR (Basic Mode Status Register at address 0x01) MII register. 2. If it is set, read the ESTATUS (Extended Status) register 3. If the ESTATUS register indicates that it supports 1000BASE-X or 1000BASE-T, set the corresponding speed and duplex fields of the phy_device structure. The MDIO registers in the Xilinx 1000BASE-X phy are documented here http://www.xilinx.com/support/documentation/user_guides/ug194.pdf starting on page 122. ESTATEN is bit 8 of the BMSR at address 0x01; ESTATUS is at register address 15 and should always hold the value 0x8000 on the Xilinx 1000BASE-X phy since the only mode it supports is 1000BASE-X full duplex. So, AFAICS, the patch should function correctly if both phys are using the same register maps. However, I don't think the Xilinx register map is blessed by the MDIO standards bodies, and therein might lie the trouble. Can somebody look up the phy register maps for the i.MX6 phy (if it's built in) or whatever the Wandboard is using? -- Charles M. Coldwell, W1CMC Belmont, Massachusetts, New England Turn on, log in, tune out ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] Ethernet support broken for Wandboard Quad on master
On Thu, Jul 18, 2013 at 11:44 AM, Fabio Estevam feste...@gmail.com wrote: The following patch fixes it: --- a/drivers/net/phy/phy.c +++ b/drivers/net/phy/phy.c @@ -337,12 +337,8 @@ int genphy_parse_link(struct phy_device *phydev) estatus = phy_read(phydev, MDIO_DEVAD_NONE, MII_ESTATUS); - if (estatus (ESTATUS_1000_XFULL | ESTATUS_1000_XHALF | - ESTATUS_1000_TFULL | ESTATUS_1000_THALF)) { - phydev-speed = SPEED_1000; - if (estatus (ESTATUS_1000_XFULL | ESTATUS_1000_TFULL)) - phydev-duplex = DUPLEX_FULL; - } + if (estatus (ESTATUS_1000_XFULL | ESTATUS_1000_TFULL)) + phydev-duplex = DUPLEX_FULL; } else { u32 bmcr = phy_read(phydev, MDIO_DEVAD_NONE, MII_BMCR); Is this the correct fix? No, I don't think so. In the previous version, it would have set both phydev-speed and phydev-duplex; after your patch it only sets phydev-duplex. What happens if you just remove the line that assigns phydev-speed in the previous version? -- Charles M. Coldwell, W1CMC Belmont, Massachusetts, New England Turn on, log in, tune out ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH] add support for Xilinx 1000BASE-X phy (GTX)
commit 39695029bc15041c809df3db4ba19bd729c447fa Author: Charles Coldwell coldw...@ll.mit.edu Date: Tue Feb 19 08:27:33 2013 -0500 Changes to support the Xilinx 1000BASE-X phy (GTX/MGT) diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c index d0ed766..8a38ccb 100644 --- a/drivers/net/phy/phy.c +++ b/drivers/net/phy/phy.c @@ -75,6 +75,10 @@ static int genphy_config_advert(struct phy_device *phydev) adv |= ADVERTISE_PAUSE_CAP; if (advertise ADVERTISED_Asym_Pause) adv |= ADVERTISE_PAUSE_ASYM; + if (advertise ADVERTISED_1000baseX_Half) + adv |= ADVERTISE_1000XHALF; + if (advertise ADVERTISED_1000baseX_Full) + adv |= ADVERTISE_1000XFULL; if (adv != oldadv) { err = phy_write(phydev, MDIO_DEVAD_NONE, MII_ADVERTISE, adv); @@ -288,6 +292,7 @@ static int genphy_parse_link(struct phy_device *phydev) if (mii_reg BMSR_ANEGCAPABLE) { u32 lpa = 0; u32 gblpa = 0; + u32 estatus = 0; /* Check for gigabit capability */ if (mii_reg BMSR_ERCAP) { @@ -327,6 +332,17 @@ static int genphy_parse_link(struct phy_device *phydev) } else if (lpa LPA_10FULL) phydev-duplex = DUPLEX_FULL; + + if (mii_reg BMSR_ESTATEN) + estatus = phy_read(phydev, MDIO_DEVAD_NONE, MII_ESTATUS); + + if (estatus (ESTATUS_1000_XFULL | ESTATUS_1000_XHALF | + ESTATUS_1000_TFULL | ESTATUS_1000_THALF)) { + phydev-speed = SPEED_1000; + if (estatus (ESTATUS_1000_XFULL | ESTATUS_1000_TFULL)) + phydev-duplex = DUPLEX_FULL; + } + } else { u32 bmcr = phy_read(phydev, MDIO_DEVAD_NONE, MII_BMCR); @@ -384,6 +400,10 @@ int genphy_config(struct phy_device *phydev) features |= SUPPORTED_1000baseT_Full; if (val ESTATUS_1000_THALF) features |= SUPPORTED_1000baseT_Half; + if (val ESTATUS_1000_XFULL) + features |= SUPPORTED_1000baseX_Full; + if (val ESTATUS_1000_XHALF) + features |= SUPPORTED_1000baseX_Full; } phydev-supported = features; diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h index fcb20fe..f6dbdb0 100644 --- a/include/linux/ethtool.h +++ b/include/linux/ethtool.h @@ -580,6 +580,8 @@ enum ethtool_sfeatures_retval_bits { #define SUPPORTED_1baseKX4_Full(1 18) #define SUPPORTED_1baseKR_Full (1 19) #define SUPPORTED_1baseR_FEC (1 20) +#define SUPPORTED_1000baseX_Half (1 21) +#define SUPPORTED_1000baseX_Full (1 22) /* Indicates what features are advertised by the interface. */ #define ADVERTISED_10baseT_Half(1 0) @@ -603,6 +605,8 @@ enum ethtool_sfeatures_retval_bits { #define ADVERTISED_1baseKX4_Full (1 18) #define ADVERTISED_1baseKR_Full(1 19) #define ADVERTISED_1baseR_FEC (1 20) +#define ADVERTISED_1000baseX_Half (1 21) +#define ADVERTISED_1000baseX_Full (1 22) /* The following are all involved in forcing a particular link * mode for the device for setting things. When getting the diff --git a/include/linux/mii.h b/include/linux/mii.h index 8b92692..66b83d8 100644 --- a/include/linux/mii.h +++ b/include/linux/mii.h @@ -115,6 +115,8 @@ #define EXPANSION_MFAULTS 0x0010 /* Multiple faults detected*/ #define EXPANSION_RESV 0xffe0 /* Unused... */ +#define ESTATUS_1000_XFULL 0x8000 /* Can do 1000BX Full */ +#define ESTATUS_1000_XHALF 0x4000 /* Can do 1000BX Half */ #define ESTATUS_1000_TFULL 0x2000 /* Can do 1000BT Full */ #define ESTATUS_1000_THALF 0x1000 /* Can do 1000BT Half */ -- Charles M. Coldwell, W1CMC Turn on, log in, tune out Somerville, Massachusetts, New England (FN42kj) GPG ID: 852E052F GPG FPR: 77E5 2B51 4907 F08A 7E92 DE80 AFA9 9A8F 852E 052F ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] patch to support Xilinx 1000BASE-X phy (GTX)
and here it is again, in-line commit 39695029bc15041c809df3db4ba19bd729c447fa Author: Charles Coldwell coldw...@ll.mit.edu Date: Tue Feb 19 08:27:33 2013 -0500 Changes to support the Xilinx 1000BASE-X phy (GTX/MGT) diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c index d0ed766..8a38ccb 100644 --- a/drivers/net/phy/phy.c +++ b/drivers/net/phy/phy.c @@ -75,6 +75,10 @@ static int genphy_config_advert(struct phy_device *phydev) adv |= ADVERTISE_PAUSE_CAP; if (advertise ADVERTISED_Asym_Pause) adv |= ADVERTISE_PAUSE_ASYM; + if (advertise ADVERTISED_1000baseX_Half) + adv |= ADVERTISE_1000XHALF; + if (advertise ADVERTISED_1000baseX_Full) + adv |= ADVERTISE_1000XFULL; if (adv != oldadv) { err = phy_write(phydev, MDIO_DEVAD_NONE, MII_ADVERTISE, adv); @@ -288,6 +292,7 @@ static int genphy_parse_link(struct phy_device *phydev) if (mii_reg BMSR_ANEGCAPABLE) { u32 lpa = 0; u32 gblpa = 0; + u32 estatus = 0; /* Check for gigabit capability */ if (mii_reg BMSR_ERCAP) { @@ -327,6 +332,17 @@ static int genphy_parse_link(struct phy_device *phydev) } else if (lpa LPA_10FULL) phydev-duplex = DUPLEX_FULL; + + if (mii_reg BMSR_ESTATEN) + estatus = phy_read(phydev, MDIO_DEVAD_NONE, MII_ESTATUS); + + if (estatus (ESTATUS_1000_XFULL | ESTATUS_1000_XHALF | + ESTATUS_1000_TFULL | ESTATUS_1000_THALF)) { + phydev-speed = SPEED_1000; + if (estatus (ESTATUS_1000_XFULL | ESTATUS_1000_TFULL)) + phydev-duplex = DUPLEX_FULL; + } + } else { u32 bmcr = phy_read(phydev, MDIO_DEVAD_NONE, MII_BMCR); @@ -384,6 +400,10 @@ int genphy_config(struct phy_device *phydev) features |= SUPPORTED_1000baseT_Full; if (val ESTATUS_1000_THALF) features |= SUPPORTED_1000baseT_Half; + if (val ESTATUS_1000_XFULL) + features |= SUPPORTED_1000baseX_Full; + if (val ESTATUS_1000_XHALF) + features |= SUPPORTED_1000baseX_Full; } phydev-supported = features; diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h index fcb20fe..f6dbdb0 100644 --- a/include/linux/ethtool.h +++ b/include/linux/ethtool.h @@ -580,6 +580,8 @@ enum ethtool_sfeatures_retval_bits { #define SUPPORTED_1baseKX4_Full (1 18) #define SUPPORTED_1baseKR_Full (1 19) #define SUPPORTED_1baseR_FEC (1 20) +#define SUPPORTED_1000baseX_Half (1 21) +#define SUPPORTED_1000baseX_Full (1 22) /* Indicates what features are advertised by the interface. */ #define ADVERTISED_10baseT_Half (1 0) @@ -603,6 +605,8 @@ enum ethtool_sfeatures_retval_bits { #define ADVERTISED_1baseKX4_Full (1 18) #define ADVERTISED_1baseKR_Full (1 19) #define ADVERTISED_1baseR_FEC (1 20) +#define ADVERTISED_1000baseX_Half (1 21) +#define ADVERTISED_1000baseX_Full (1 22) /* The following are all involved in forcing a particular link * mode for the device for setting things. When getting the diff --git a/include/linux/mii.h b/include/linux/mii.h index 8b92692..66b83d8 100644 --- a/include/linux/mii.h +++ b/include/linux/mii.h @@ -115,6 +115,8 @@ #define EXPANSION_MFAULTS 0x0010 /* Multiple faults detected*/ #define EXPANSION_RESV 0xffe0 /* Unused... */ +#define ESTATUS_1000_XFULL 0x8000 /* Can do 1000BX Full */ +#define ESTATUS_1000_XHALF 0x4000 /* Can do 1000BX Half */ #define ESTATUS_1000_TFULL 0x2000 /* Can do 1000BT Full */ #define ESTATUS_1000_THALF 0x1000 /* Can do 1000BT Half */ On Tue, Feb 19, 2013 at 8:35 AM, Charles Coldwell coldw...@gmail.com wrote: Attached is a (small) patch that enabled me to use the 1000BASE-X phy on the Xilinx Virtex-6 hard TEMAC (i.e. the one that uses the GTX transceivers). -- Charles M. Coldwell, W1CMC Belmont, Massachusetts, New England Turn on, log in, tune out -- Charles M. Coldwell, W1CMC Belmont, Massachusetts, New England Turn on, log in, tune out ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] to support Xilinx 1000BASE-X phy (GTX)
On Tue, 19 Feb 2013, Wolfgang Denk wrote: Your patch is white space corrupted. Please fix your mailer. commit 39695029bc15041c809df3db4ba19bd729c447fa Author: Charles Coldwell coldw...@ll.mit.edu Date: Tue Feb 19 08:27:33 2013 -0500 Changes to support the Xilinx 1000BASE-X phy (GTX/MGT) diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c index d0ed766..8a38ccb 100644 --- a/drivers/net/phy/phy.c +++ b/drivers/net/phy/phy.c @@ -75,6 +75,10 @@ static int genphy_config_advert(struct phy_device *phydev) adv |= ADVERTISE_PAUSE_CAP; if (advertise ADVERTISED_Asym_Pause) adv |= ADVERTISE_PAUSE_ASYM; + if (advertise ADVERTISED_1000baseX_Half) + adv |= ADVERTISE_1000XHALF; + if (advertise ADVERTISED_1000baseX_Full) + adv |= ADVERTISE_1000XFULL; if (adv != oldadv) { err = phy_write(phydev, MDIO_DEVAD_NONE, MII_ADVERTISE, adv); @@ -288,6 +292,7 @@ static int genphy_parse_link(struct phy_device *phydev) if (mii_reg BMSR_ANEGCAPABLE) { u32 lpa = 0; u32 gblpa = 0; + u32 estatus = 0; /* Check for gigabit capability */ if (mii_reg BMSR_ERCAP) { @@ -327,6 +332,17 @@ static int genphy_parse_link(struct phy_device *phydev) } else if (lpa LPA_10FULL) phydev-duplex = DUPLEX_FULL; + + if (mii_reg BMSR_ESTATEN) + estatus = phy_read(phydev, MDIO_DEVAD_NONE, MII_ESTATUS); + + if (estatus (ESTATUS_1000_XFULL | ESTATUS_1000_XHALF | + ESTATUS_1000_TFULL | ESTATUS_1000_THALF)) { + phydev-speed = SPEED_1000; + if (estatus (ESTATUS_1000_XFULL | ESTATUS_1000_TFULL)) + phydev-duplex = DUPLEX_FULL; + } + } else { u32 bmcr = phy_read(phydev, MDIO_DEVAD_NONE, MII_BMCR); @@ -384,6 +400,10 @@ int genphy_config(struct phy_device *phydev) features |= SUPPORTED_1000baseT_Full; if (val ESTATUS_1000_THALF) features |= SUPPORTED_1000baseT_Half; + if (val ESTATUS_1000_XFULL) + features |= SUPPORTED_1000baseX_Full; + if (val ESTATUS_1000_XHALF) + features |= SUPPORTED_1000baseX_Full; } phydev-supported = features; diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h index fcb20fe..f6dbdb0 100644 --- a/include/linux/ethtool.h +++ b/include/linux/ethtool.h @@ -580,6 +580,8 @@ enum ethtool_sfeatures_retval_bits { #define SUPPORTED_1baseKX4_Full(1 18) #define SUPPORTED_1baseKR_Full (1 19) #define SUPPORTED_1baseR_FEC (1 20) +#define SUPPORTED_1000baseX_Half (1 21) +#define SUPPORTED_1000baseX_Full (1 22) /* Indicates what features are advertised by the interface. */ #define ADVERTISED_10baseT_Half(1 0) @@ -603,6 +605,8 @@ enum ethtool_sfeatures_retval_bits { #define ADVERTISED_1baseKX4_Full (1 18) #define ADVERTISED_1baseKR_Full(1 19) #define ADVERTISED_1baseR_FEC (1 20) +#define ADVERTISED_1000baseX_Half (1 21) +#define ADVERTISED_1000baseX_Full (1 22) /* The following are all involved in forcing a particular link * mode for the device for setting things. When getting the diff --git a/include/linux/mii.h b/include/linux/mii.h index 8b92692..66b83d8 100644 --- a/include/linux/mii.h +++ b/include/linux/mii.h @@ -115,6 +115,8 @@ #define EXPANSION_MFAULTS 0x0010 /* Multiple faults detected*/ #define EXPANSION_RESV 0xffe0 /* Unused... */ +#define ESTATUS_1000_XFULL 0x8000 /* Can do 1000BX Full */ +#define ESTATUS_1000_XHALF 0x4000 /* Can do 1000BX Half */ #define ESTATUS_1000_TFULL 0x2000 /* Can do 1000BT Full */ #define ESTATUS_1000_THALF 0x1000 /* Can do 1000BT Half */ -- Charles M. Coldwell, W1CMC Turn on, log in, tune out Somerville, Massachusetts, New England (FN42kj) GPG ID: 852E052F GPG FPR: 77E5 2B51 4907 F08A 7E92 DE80 AFA9 9A8F 852E 052F ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot