On Fri, Apr 05, 2019 at 07:40:51PM +0200, Heiner Kallweit wrote:
> On 05.04.2019 18:16, Simon Horman wrote:
> > On Tue, Apr 02, 2019 at 08:43:30PM +0200, Heiner Kallweit wrote:
> >> The original patch didn't consider the case that autoneg process
> >> finishes successfully but both link partners have no mode in common.
> >> In this case there's no link, nevertheless we may be interested in
> >> what the link partner advertised.
> >>
> >> Like phydev->link we set phydev->autoneg_complete in
> >> genphy_update_link() and use the stored value in genphy_read_status().
> >> This way we don't have to read register BMSR again.
> >>
> >> Fixes: b6163f194c69 ("net: phy: improve genphy_read_status")
> >> Signed-off-by: Heiner Kallweit <hkallweit1@xxxxxxxxx>
> >
> > Hi,
> >
> > I have observed a regression with this patch as present as
> > 4950c2ba49cc ("net: phy: fix autoneg mismatch case in genphy_read_status")
> > in net-next.
> >
> > The platform is the Renesas R-Car E3 (r8a77990) based Ebisu-4D board.
> >
> >
> > Without this patch (commit before 4950c2ba49cc) the link is negotiated
> > as follows:
> >
> > [ 3.257699] libphy: ravb_mii: probed
> > [ 3.266498] ravb e6800000.ethernet eth0: Base address at 0xe6800000, 2e:09:0a:03:85:38, IRQ 104.
> > [ 3.537082] Micrel KSZ9031 Gigabit PHY e6800000.ethernet-ffffffff:00: attached PHY driver [Micrel KSZ9031 Gigabit PHY] (mii_bus:phy_addr=e6800000.ethernet-ffffffff:00, irq=165)
> > [ 9.667161] ravb e6800000.ethernet eth0: Link is Up - 1Gbps/Full - flow control off
> >
> > # ethtool --version
> > ethtool version 3.16
> > # ethtool eth0
> > Settings for eth0:
> > Supported ports: [ TP MII ]
> > Supported link modes: 100baseT/Full
> > 1000baseT/Full
> > Supported pause frame use: No
> > Supports auto-negotiation: Yes
> > Advertised link modes: 100baseT/Full
> > 1000baseT/Full
> > Advertised pause frame use: No
> > Advertised auto-negotiation: Yes
> > Link partner advertised link modes: 10baseT/Half 10baseT/Full
> > 100baseT/Half 100baseT/Full
> > 1000baseT/Full
> > Link partner advertised pause frame use: No
> > Link partner advertised auto-negotiation: Yes
> > Speed: 1000Mb/s
> > Duplex: Full
> > Port: MII
> > PHYAD: 0
> > Transceiver: external
> > Auto-negotiation: on
> > Supports Wake-on: g
> > Wake-on: d
> > Current message level: 0x000000cc (204)
> > link timer rx_err tx_err
> > Link detected: yes
> >
> >
> > With this patch the link is negotiated as follows, note the "Unknown":
> >
> > [ 3.268609] libphy: ravb_mii: probed
> > [ 3.277402] ravb e6800000.ethernet eth0: Base address at 0xe6800000, 2e:09:0a:03:85:38, IRQ 104.
> > [ 3.565057] Micrel KSZ9031 Gigabit PHY e6800000.ethernet-ffffffff:00: attached PHY driver [Micrel KSZ9031 Gigabit PHY] (mii_bus:phy_addr=e6800000.ethernet-ffffffff:00, irq=165)
> > [ 9.804423] ravb e6800000.ethernet eth0: Link is Up - Unknown/Unknown - flow control off
> >
> Looks like the PHY doesn't set the "aneg complete" bit. But then, how
> can the link be up? Could you please add a debug output in
> genphy_update_link() printing the value of register BMSR?
I added the following debug patch:
diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index 72fc714c9427..88c6cf79c438 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -1710,6 +1710,7 @@ int genphy_update_link(struct phy_device *phydev)
*/
if (!phy_polling_mode(phydev)) {
status = phy_read(phydev, MII_BMSR);
+ pr_err("1: MII_BMSR=0x%08x\n", status);
if (status < 0) {
return status;
} else if (status & BMSR_LSTATUS) {
@@ -1720,6 +1721,7 @@ int genphy_update_link(struct phy_device *phydev)
/* Read link and autonegotiation status */
status = phy_read(phydev, MII_BMSR);
+ pr_err("2: MII_BMSR=0x%08x\n", status);
if (status < 0)
return status;
And the out put is as follows:
# dmesg | egrep '(ravb|libphy|Mic)'
[ 2.028963] libphy: Fixed MDIO Bus: probed
[ 3.244122] ravb e6800000.ethernet: ignoring dependency for device, assuming no driver
[ 3.256398] libphy: ravb_mii: probed
[ 3.264867] ravb e6800000.ethernet eth0: Base address at 0xe6800000, 2e:09:0a:03:85:38, IRQ 104.
[ 3.551385] Micrel KSZ9031 Gigabit PHY e6800000.ethernet-ffffffff:00: attached PHY driver [Micrel KSZ9031 Gigabit PHY] (mii_bus:phy_addr=e6800000.ethernet-ffffffff:00, irq=165)
[ 3.569904] libphy: 1: MII_BMSR=0x00007949
[ 3.574397] libphy: 2: MII_BMSR=0x00007949
[ 9.899946] libphy: 1: MII_BMSR=0x0000796d
[ 9.904786] ravb e6800000.ethernet eth0: Link is Up - Unknown/Unknown - flow control off
# ethtool eth0
Settings for eth0:
Supported ports: [ TP MII ]
Supported link modes: 100baseT/Full
1000baseT/Full
Supported pause frame use: No
Supports auto-negotiation: Yes
Advertised link modes: 100baseT/Full
1000baseT/Full
Advertised pause frame use: No
Advertised auto-negotiation: Yes
Speed: Unknown!
Duplex: Unknown! (255)
Port: MII
PHYAD: 0
Transceiver: external
Auto-negotiation: on
Supports Wake-on: g
Wake-on: d
Current message level: 0x000000cc (204)
link timer rx_err tx_err
Link detected: yes
> I wonder whether your case may be caused by a chip erratum. Item 5 in
> the errata documentation
> (http://ww1.microchip.com/downloads/en/DeviceDoc/80000692D.pdf)
> may be a candidate. Could you please check (as explained in the
> errata document) and report the exact PHY version?
>
> Could you please also test with another link partner?
With a different link partner it seems that the problem remains:
root@Debian:~# dmesg | egrep '(ravb|libphy|Mic)'
[ 2.023596] libphy: Fixed MDIO Bus: probed
[ 3.243061] ravb e6800000.ethernet: ignoring dependency for device, assuming no driver
[ 3.255157] libphy: ravb_mii: probed
[ 3.263732] ravb e6800000.ethernet eth0: Base address at 0xe6800000, 2e:09:0a:03:85:38, IRQ 104.
[ 3.543377] Micrel KSZ9031 Gigabit PHY e6800000.ethernet-ffffffff:00: attached PHY driver [Micrel KSZ9031 Gigabit PHY] (mii_bus:phy_addr=e6800000.ethernet-ffffffff:00, irq=165)
[ 3.561950] libphy: 1: MII_BMSR=0x00007949
[ 3.566436] libphy: 2: MII_BMSR=0x00007949
[ 12.798173] libphy: 1: MII_BMSR=0x0000796d
[ 12.802974] ravb e6800000.ethernet eth0: Link is Up - Unknown/Unknown - flow control off
root@Debian:~# ethtool eth0
Settings for eth0:
Supported ports: [ TP MII ]
Supported link modes: 100baseT/Full
1000baseT/Full
Supported pause frame use: No
Supports auto-negotiation: Yes
Advertised link modes: 100baseT/Full
1000baseT/Full
Advertised pause frame use: No
Advertised auto-negotiation: Yes
Speed: Unknown!
Duplex: Unknown! (255)
Port: MII
PHYAD: 0
Transceiver: external
Auto-negotiation: on
Supports Wake-on: g
Wake-on: d
Current message level: 0x000000cc (204)
link timer rx_err tx_err
Link detected: yes
> > And ethtool reports:
> >
> > # ethtool eth0
> > Settings for eth0:
> > Supported ports: [ TP MII ]
> > Supported link modes: 100baseT/Full
> > 1000baseT/Full
> > Supported pause frame use: No
> > Supports auto-negotiation: Yes
> > Advertised link modes: 100baseT/Full
> > 1000baseT/Full
> > Advertised pause frame use: No
> > Advertised auto-negotiation: Yes
> > Speed: Unknown!
> > Duplex: Unknown! (255)
> > Port: MII
> > PHYAD: 0
> > Transceiver: external
> > Auto-negotiation: on
> > Supports Wake-on: g
> > Wake-on: d
> > Current message level: 0x000000cc (204)
> > link timer rx_err tx_err
> > Link detected: yes
> >
> >
> >
> > I noticed this when preparing a patch limit the maximum speed the device to
> > 100Mbit/s as follows:
> >
> > --- a/arch/arm64/boot/dts/renesas/r8a77990-ebisu.dts
> > +++ b/arch/arm64/boot/dts/renesas/r8a77990-ebisu.dts
> > @@ -272,6 +272,7 @@
> > interrupt-parent = <&gpio2>;
> > interrupts = <21 IRQ_TYPE_LEVEL_LOW>;
> > reset-gpios = <&gpio1 20 GPIO_ACTIVE_LOW>;
> > + max-speed = <100>;
> > };
> > };
> >
> > While the 100Mbit/s limit applied on top of the commit before 4950c2ba49cc
> > things work as expected:
> >
> > [ 3.258347] libphy: ravb_mii: probed
> > [ 3.266739] ravb e6800000.ethernet eth0: Base address at 0xe6800000, 2e:09:0a:03:85:38, IRQ 104.
> > [ 3.553642] Micrel KSZ9031 Gigabit PHY e6800000.ethernet-ffffffff:00: attached PHY driver [Micrel KSZ9031 Gigabit PHY] (mii_bus:phy_addr=e6800000.ethernet-ffffffff:00, irq=165)
> > [ 5.585027] ravb e6800000.ethernet eth0: Link is Up - 100Mbps/Full - flow control off
> >
> > # ethtool eth0
> > Settings for eth0:
> > Supported ports: [ TP MII ]
> > Supported link modes: 100baseT/Full
> > Supported pause frame use: No
> > Supports auto-negotiation: Yes
> > Advertised link modes: 100baseT/Full
> > Advertised pause frame use: No
> > Advertised auto-negotiation: Yes
> > Link partner advertised link modes: 10baseT/Half 10baseT/Full
> > 100baseT/Half 100baseT/Full
> I wondered why the link partner suddenly doesn't report 1Gbps support.
> Seems to be a small flaw in phylib, if locally 1Gbps isn't supported
> (or support is disabled) then we don't check for the link partners
> 1Gbps capability.
>
> > Link partner advertised pause frame use: No
> > Link partner advertised auto-negotiation: Yes
> > Speed: 100Mb/s
> > Duplex: Full
> > Port: MII
> > PHYAD: 0
> > Transceiver: external
> > Auto-negotiation: on
> > Supports Wake-on: g
> > Wake-on: d
> > Current message level: 0x000000cc (204)
> > link timer rx_err tx_err
> > Link detected: yes
> >
> >
> > However, with the 100Mbit/s limit applied on top of 4950c2ba49cc the boot hangs
> > while (or just after?) negotiating the link:
> >
> > [ 3.541549] Micrel KSZ9031 Gigabit PHY e6800000.ethernet-ffffffff:00: attached PHY driver [Micrel KSZ9031 Gigabit PHY] (mii_bus:phy_addr=e6800000.ethernet-ffffffff:00, irq=165)
> > [ 5.536389] ravb e6800000.ethernet eth0: Link is Up - Unknown/Unknown - flow control off
> >
> >
>
