Re: [PATCH net-next] net: phy: improve link partner capability detection

[Andrew Lunn <andrew@lunn.ch>]

On Fri, Apr 05, 2019 at 10:04:22PM +0200, Heiner Kallweit wrote:
> On 05.04.2019 21:48, Andrew Lunn wrote:
> > On Fri, Apr 05, 2019 at 09:23:13PM +0200, Heiner Kallweit wrote:
> >> genphy_read_status() so far checks phydev->supported, not the actual
> >> PHY capabilities. This can make a difference if the supported speeds
> >> have been limited by of_set_phy_supported() or phy_set_max_speed().
> >>
> >> It seems that this issue only affects the link partner advertisements
> >> as displayed by ethtool. Also this patch wouldn't apply to older
> >> kernels because linkmode bitmaps have been introduced recently.
> >> Therefore net-next.
> > 
> > Hi Heiner
> > 
> Hi Andrew,
> 
> > Are you saying, if the local PHY/MAC does not support 1000Base-T, we
> > should not check if the peer is advertising 1000Base-T? That seems
> > wrong. We should report everything it offers. The fact we cannot make
> > use of 1000Base-T should not matter, and resolving of the auto-get
> > should do the right thing when presented with modes it cannot use.
> > 
> Link partner gigabit advertisement is reported in register MII_STAT1000.
> If we have a 100MBit PHY w/o this register, then we will never know
> whether the link partner supports gigabit.

Hi Heiner

There seems to be two uses cases:

A Fast MAC connected to a Fast PHY. In that case, MII_STAT1000
probably does not exist, so we should not read it. Hopefully .features
says BASIC, or if we ask the PHY what is it, it reports it is a Fast
PHY.

A Fast MAC connected to a Giga PHY. The MAC driver will of used
phy_set_max_speed() to indicate its limits. In that case, MII_STAT1000
does exist and we should report what the peer is advertising.

      Andrew