[U-Boot] Ethernet support broken for Wandboard Quad on master

[Sascha Silbe]

Hello Charles,

[CC += a few people that were CC'ed on the revert of Charles' patch]

Charles Coldwell <coldwell at 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.

Sascha

-- >8 --

From: Sascha Silbe <t-uboot at infra-silbe.de>
Date: Fri, 19 Jul 2013 11:37:54 +0200
Subject: [PATCH] phy: fix 10/100Mbps operation on 1Gbps-capable links

de1d786 [add support for Xilinx 1000BASE-X phy (GTX)] introduced a
check for the extended status register in order to support
1Gbps-capable PHYs that don't have the 1000BASE-T registers. Since
Extended Status only indicates what the PHY (i.e. the local side) is
capable of, this broke communication with non-1Gbps peers.

Only check the extended status if the 1000BASE-T registers are
actually missing so we don't end up setting speed to 1Gbps even though
the previous test (for the combination of local and peer support for
1Gbps) already indicated we can't do 1Gbps with the current peer.

Signed-off-by: Sascha Silbe <t-uboot at infra-silbe.de>
---
 drivers/net/phy/phy.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
index 7c0eaec..f803834 100644
--- a/drivers/net/phy/phy.c
+++ b/drivers/net/phy/phy.c
@@ -333,7 +333,14 @@ int genphy_parse_link(struct phy_device *phydev)
 		} else if (lpa & LPA_10FULL)
 			phydev->duplex = DUPLEX_FULL;
 
-		if (mii_reg & BMSR_ESTATEN)
+		/* Extended status may indicate that the PHY supports
+		 * 1000BASE-T/X even though the 1000BASE-T registers
+		 * are missing. In this case we can't tell whether the
+		 * peer also supports it, so we only check extended
+		 * status if the 1000BASE-T registers are actually
+		 * missing.
+		*/
+		if ((mii_reg & BMSR_ESTATEN) && !(mii_reg & BMSR_ERCAP))
 			estatus = phy_read(phydev, MDIO_DEVAD_NONE,
 					   MII_ESTATUS);
 
-- 
1.8.3.2

-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 489 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20130719/369ef7ab/attachment.pgp>