Re: [PATCH net-next v1 4/4] net: phy: Always read EEE LPA in genphy_c45_ethtool_get_eee()

[Andrew Lunn]

On Thu, Apr 24, 2025 at 02:16:01PM +0100, Russell King (Oracle) wrote:
> On Thu, Apr 24, 2025 at 03:02:22PM +0200, Oleksij Rempel wrote:
> > Previously, genphy_c45_ethtool_get_eee() used genphy_c45_eee_is_active(),
> > which skips reading the EEE LPA register if local EEE is disabled. This
> > prevents ethtool from reporting the link partner's EEE capabilities in
> > that case.
> > 
> > Replace it with genphy_c45_read_eee_lpa(), which always reads the LPA
> > register regardless of local EEE state. This allows users to see the
> > link partner's EEE advertisement even when EEE is disabled locally.
> > 
> > Example before the patch:
> > 
> >   EEE settings for lan1:
> >           EEE status: disabled
> >           Tx LPI: disabled
> >           Supported EEE link modes:  100baseT/Full
> >                                      1000baseT/Full
> >           Advertised EEE link modes:  Not reported
> >           Link partner advertised EEE link modes:  Not reported
> > 
> > After the patch:
> > 
> >   EEE settings for lan1:
> >           EEE status: disabled
> >           Tx LPI: disabled
> >           Supported EEE link modes:  100baseT/Full
> >                                      1000baseT/Full
> >           Advertised EEE link modes:  Not reported
> >           Link partner advertised EEE link modes:  100baseT/Full
> >                                                    1000baseT/Full
> 
> Seems to me this takes the opposite view to patch 3... not sure there's
> much consistency here.

+1

> However, I've no objection to reading the LPA EEE state and
> reporting it.

What happens with normal link mode LPA when autoneg is disabled? I
guess they are not reported because the PHY is not even listening for
the autoneg pulses. We could be inconsistent between normal LPA and
LPA EEE, but is that a good idea?

	Andrew