From patchwork Fri Jul 19 10:25:10 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Sascha Silbe X-Patchwork-Id: 260234 X-Patchwork-Delegate: trini@ti.com Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from theia.denx.de (theia.denx.de [85.214.87.163]) by ozlabs.org (Postfix) with ESMTP id 059272C0090 for ; Fri, 19 Jul 2013 20:25:48 +1000 (EST) Received: from localhost (localhost [127.0.0.1]) by theia.denx.de (Postfix) with ESMTP id 39D154A01E; Fri, 19 Jul 2013 12:25:47 +0200 (CEST) X-Virus-Scanned: Debian amavisd-new at theia.denx.de Received: from theia.denx.de ([127.0.0.1]) by localhost (theia.denx.de [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id ekV1v7xGSyQf; Fri, 19 Jul 2013 12:25:47 +0200 (CEST) Received: from theia.denx.de (localhost [127.0.0.1]) by theia.denx.de (Postfix) with ESMTP id 9DA064A027; Fri, 19 Jul 2013 12:25:42 +0200 (CEST) Received: from localhost (localhost [127.0.0.1]) by theia.denx.de (Postfix) with ESMTP id 822374A027 for ; Fri, 19 Jul 2013 12:25:36 +0200 (CEST) X-Virus-Scanned: Debian amavisd-new at theia.denx.de Received: from theia.denx.de ([127.0.0.1]) by localhost (theia.denx.de [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id aGL-WitQBu-6 for ; Fri, 19 Jul 2013 12:25:31 +0200 (CEST) X-policyd-weight: NOT_IN_SBL_XBL_SPAMHAUS=-1.5 NOT_IN_SPAMCOP=-1.5 NOT_IN_BL_NJABL=-1.5 (only DNSBL check requested) Received: from b.mx.chost.de (s15264669.onlinehome-server.info [87.106.8.89]) by theia.denx.de (Postfix) with ESMTPS id 291264A01E for ; Fri, 19 Jul 2013 12:25:25 +0200 (CEST) Received: from sshgate.chost.de (sshgate.chost.de [::ffff:87.106.80.19]) by b.mx.chost.de with esmtp; Fri, 19 Jul 2013 12:25:24 +0200 id 0E1A12DB.51E91414.00001278 Received: by sshgate.chost.de (sSMTP sendmail emulation); Fri, 19 Jul 2013 12:25:24 +0200 Received: (nullmailer pid 29002 invoked by uid 8193); Fri, 19 Jul 2013 10:25:17 -0000 To: Charles Coldwell In-Reply-To: References: User-Agent: Notmuch/0.15.2 (http://notmuchmail.org) Emacs/23.4.1 (x86_64-pc-linux-gnu) Date: Fri, 19 Jul 2013 12:25:10 +0200 Message-ID: Mime-Version: 1.0 From: Sascha Silbe Cc: Fabio Estevam , Otavio Salvador , joe.hershberger@ni.com, "u-boot@lists.denx.de" , Tom Rini Subject: Re: [U-Boot] Ethernet support broken for Wandboard Quad on master X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.11 Precedence: list List-Id: U-Boot discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: u-boot-bounces@lists.denx.de Errors-To: u-boot-bounces@lists.denx.de Hello Charles, [CC += a few people that were CC'ed on the revert of Charles' patch] Charles Coldwell 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 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 Tested-by: Fabio Estevam Reviewed-by: Joe Hershberger --- 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);