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 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?
> 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
>
>
