| Message ID | 20090624182734.GA14306@oksana.dev.rtsoft.ru |
|---|---|
| State | Superseded, archived |
| Delegated to: | David Miller |
| Headers | show |
On Jun 24, 2009, at 1:27 PM, Anton Vorontsov wrote: > It appears that gianfar driver has the same problem[1] that I > just fixed for ucc_geth. > > NFS boot using 10/half link takes about 10 minutes to complete: > > > The symptoms were observed on MPC8379E-RDB boards (eTSEC). Although > I didn't find where documentation forbids clearing Full Duplex bit > for non-MII/RMII modes, it's pretty distinct that the bit should be > set. > > It's no wonder though, QE Ethernet and TSEC are pretty similar. > - if (!(phydev->duplex)) > - tempval &= ~(MACCFG2_FULL_DUPLEX); > + if (!phydev->duplex && > + (phyi == PHY_INTERFACE_MODE_MII || > + phyi == PHY_INTERFACE_MODE_RMII)) Hmm....have you tested this on a GMII interface? *Technically*, full duplex is required for GMII, as GMII is used only for gigabit. However, we've been treating the GMII interface type as an indicator that the PHY *has* a GMII connection to the NIC. When gianfar detects the speed is 10/100 it switches to the compatible MII interface via this code, just below: case 100: case 10: tempval = ((tempval & ~(MACCFG2_IF)) | MACCFG2_MII); My concern is that you will be detecting the GMII interface, and disallowing half-duplex, despite the fact that the interface is actually running at 10 or 100 Mbit. Andy -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Jun 24, 2009 at 03:18:59PM -0500, Andy Fleming wrote: > > On Jun 24, 2009, at 1:27 PM, Anton Vorontsov wrote: > >> It appears that gianfar driver has the same problem[1] that I >> just fixed for ucc_geth. >> >> NFS boot using 10/half link takes about 10 minutes to complete: >> > >> >> The symptoms were observed on MPC8379E-RDB boards (eTSEC). Although >> I didn't find where documentation forbids clearing Full Duplex bit >> for non-MII/RMII modes, it's pretty distinct that the bit should be >> set. >> >> It's no wonder though, QE Ethernet and TSEC are pretty similar. > >> - if (!(phydev->duplex)) >> - tempval &= ~(MACCFG2_FULL_DUPLEX); >> + if (!phydev->duplex && >> + (phyi == PHY_INTERFACE_MODE_MII || >> + phyi == PHY_INTERFACE_MODE_RMII)) > > > Hmm....have you tested this on a GMII interface? Nope, only RGMII. > *Technically*, full > duplex is required for GMII, as GMII is used only for gigabit. However, > we've been treating the GMII interface type as an indicator that the PHY > *has* a GMII connection to the NIC. When gianfar detects the speed is > 10/100 it switches to the compatible MII interface via this code, just > below: > > case 100: > case 10: > tempval = > ((tempval & ~(MACCFG2_IF)) | > MACCFG2_MII); > > > My concern is that you will be detecting the GMII interface, and > disallowing half-duplex, despite the fact that the interface is actually > running at 10 or 100 Mbit. Very interesting, though I'm not sure I'm completely following. :-) Are you saying that I should do this instead: if (!phydev->duplex && (phyi == PHY_INTERFACE_MODE_MII || phyi == PHY_INTERFACE_MODE_RMII || (phyi == PHY_INTERFACE_MODE_GMII && phydev->speed < 1000))) tempval &= ~MACCFG2_FULL_DUPLEX; else tempval |= MACCFG2_FULL_DUPLEX; i.e. we detected GMII interface initially, but it downgraded to MII since speed is < 1000, thus we can set half-duplex in MAC? Thanks,
On Wed, Jun 24, 2009 at 04:25:06PM -0500, Andy Fleming wrote: [...] >>> My concern is that you will be detecting the GMII interface, and >>> disallowing half-duplex, despite the fact that the interface is >>> actually >>> running at 10 or 100 Mbit. >> >> Very interesting, though I'm not sure I'm completely following. :-) >> >> Are you saying that I should do this instead: >> >> if (!phydev->duplex && >> (phyi == PHY_INTERFACE_MODE_MII || >> phyi == PHY_INTERFACE_MODE_RMII || >> (phyi == PHY_INTERFACE_MODE_GMII && >> phydev->speed < 1000))) >> tempval &= ~MACCFG2_FULL_DUPLEX; >> else >> tempval |= MACCFG2_FULL_DUPLEX; >> >> i.e. we detected GMII interface initially, but it downgraded >> to MII since speed is < 1000, thus we can set half-duplex in MAC? > > Yeah, I think that works out more correctly. Cool, thanks. Do you happen to know how gianfar iface auto-detection works in HW? I mean, if we connect 100 Mbs link to the GMII PHY, then gfar_get_interface() would return MII, correct? If so, then I think I'll also have to change phy_interface_t phyi = phydev->interface; to phy_interface_t phyi = gfar_get_interface(dev); since phydev->interface may contain outdated information. Or this can't happen? > And I suspect that the same is true for UCC Yup. Much thanks for catching this!
On Thu, Jun 25, 2009 at 01:39:45AM +0400, Anton Vorontsov wrote: > On Wed, Jun 24, 2009 at 04:25:06PM -0500, Andy Fleming wrote: > [...] > >>> My concern is that you will be detecting the GMII interface, and > >>> disallowing half-duplex, despite the fact that the interface is > >>> actually > >>> running at 10 or 100 Mbit. > >> > >> Very interesting, though I'm not sure I'm completely following. :-) > >> > >> Are you saying that I should do this instead: > >> > >> if (!phydev->duplex && > >> (phyi == PHY_INTERFACE_MODE_MII || > >> phyi == PHY_INTERFACE_MODE_RMII || > >> (phyi == PHY_INTERFACE_MODE_GMII && > >> phydev->speed < 1000))) > >> tempval &= ~MACCFG2_FULL_DUPLEX; > >> else > >> tempval |= MACCFG2_FULL_DUPLEX; > >> > >> i.e. we detected GMII interface initially, but it downgraded > >> to MII since speed is < 1000, thus we can set half-duplex in MAC? > > > > Yeah, I think that works out more correctly. > > Cool, thanks. > > Do you happen to know how gianfar iface auto-detection works in HW? > I mean, if we connect 100 Mbs link to the GMII PHY, then > gfar_get_interface() would return MII, correct? Stupid me. HW has nothing to do with this. GMII, just as you said, is just a marker, comes from FSL_GIANFAR_DEV_HAS_GIGABIT flag.
diff --git a/drivers/net/gianfar.c b/drivers/net/gianfar.c index 8741bb0..15dbffa 100644 --- a/drivers/net/gianfar.c +++ b/drivers/net/gianfar.c @@ -1984,13 +1984,16 @@ static void adjust_link(struct net_device *dev) if (phydev->link) { u32 tempval = gfar_read(®s->maccfg2); u32 ecntrl = gfar_read(®s->ecntrl); + phy_interface_t phyi = phydev->interface; /* Now we make sure that we can be in full duplex mode. * If not, we operate in half-duplex mode. */ if (phydev->duplex != priv->oldduplex) { new_state = 1; - if (!(phydev->duplex)) - tempval &= ~(MACCFG2_FULL_DUPLEX); + if (!phydev->duplex && + (phyi == PHY_INTERFACE_MODE_MII || + phyi == PHY_INTERFACE_MODE_RMII)) + tempval &= ~MACCFG2_FULL_DUPLEX; else tempval |= MACCFG2_FULL_DUPLEX;
It appears that gianfar driver has the same problem[1] that I just fixed for ucc_geth. NFS boot using 10/half link takes about 10 minutes to complete: eth0: Gianfar Ethernet Controller Version 1.2, 00:11:22:33:44:55 eth0: Running with NAPI enabled eth0: 256/256 RX/TX BD ring size [...] Sending DHCP requests . PHY: mdio@e0024520:02 - Link is Up - 10/Half ., OK [...] Looking up port of RPC 100003/2 on 10.0.0.2 Looking up port of RPC 100005/1 on 10.0.0.2 VFS: Mounted root (nfs filesystem) readonly on device 0:11. Freeing unused kernel memory: 188k init nfs: server 10.0.0.2 not responding, still trying nfs: server 10.0.0.2 OK nfs: server 10.0.0.2 not responding, still trying nfs: server 10.0.0.2 not responding, still trying nfs: server 10.0.0.2 not responding, still trying [... few minutes of OK/not responding flood ...] And just as with ucc_geth, statistic shows errors: # ethtool -S eth0 | grep -v ": 0" NIC statistics: rx-dropped-by-kernel: 2 tx-rx-64-frames: 49 tx-rx-65-127-frames: 1270 tx-rx-128-255-frames: 9992 tx-rx-256-511-frames: 75 tx-rx-512-1023-frames: 142 tx-rx-1024-1518-frames: 8828 rx-bytes: 13299414 rx-packets: 13122 rx-jabber-frames: 9 tx-byte-counter: 1284847 tx-packets: 8115 tx-broadcast-packets: 3 tx-deferral-packets: 43 tx-single-collision-packets: 15 tx-multiple-collision-packets: 301 tx-late-collision-packets: 872 tx-total-collision: 999 tx-undersize-frames: 6 The symptoms were observed on MPC8379E-RDB boards (eTSEC). Although I didn't find where documentation forbids clearing Full Duplex bit for non-MII/RMII modes, it's pretty distinct that the bit should be set. It's no wonder though, QE Ethernet and TSEC are pretty similar. [1] http://lists.ozlabs.org/pipermail/linuxppc-dev/2009-June/073631.html Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com> --- drivers/net/gianfar.c | 7 +++++-- 1 files changed, 5 insertions(+), 2 deletions(-)