Re: [PATCH] net: phy: phy_ethtool_ksettings_set: Allow any supported speed

["Russell King (Oracle)" <linux@armlinux.org.uk>]

On Tue, Dec 03, 2024 at 03:45:09PM +0100, Andrew Lunn wrote:
> On Tue, Dec 03, 2024 at 02:05:07PM +0000, Dennis Ostermann wrote:
> > Hi,
> > 
> > according to IEE 802.3-2022, ch. 125.2.4.3, Auto-Negotiation is optional for 2.5GBASE-T1
> > 
> > > 125.2.4.3 Auto-Negotiation, type single differential-pair media
> > > Auto-Negotiation (Clause 98) may be used by 2.5GBASE-T1 and 5GBASE-T1 devices to detect the
> > > abilities (modes of operation) supported by the device at the other end of a link segment, determine common
> > > abilities, and configure for joint operation. Auto-Negotiation is performed upon link startup through the use
> > > of half-duplex differential Manchester encoding.
> > > The use of Clause 98 Auto-Negotiation is optional for 2.5GBASE-T1 and 5GBASE-T1 PHYs
> > 
> > So, purposed change could make sense for T1 PHYs.
> 
> The proposed change it too liberal. We need the PHY to say it supports
> 2.5GBASE-T1, not 2.5GBASE-T. We can then allow 2.5GBASE-T1 to not use
> autoneg, but 2.5GBASE-T has to use autoneg.

I'm wondering whether we should add:

	__ETHTOOL_DECLARE_LINK_MODE_MASK(requires_an);

to struct phy_device, and have phylib populate that by default with all
base-T link modes > 1G (which would be the default case as it is now.)
Then, PHY drivers can change this bitmap as they need for their device.
After the PHY features have been discovered, this should then get
AND-ed with the supported bitmap.

We can then check this in ksettings_set() to determine whether the fixed
speed requires AN.

This would be more flexible, and allow future cases to be handled.

Good idea, or over-engineering at this point?

Another idea would be to have a boolean in struct phy_device that
identifies the PHY as base-T1, and makes an exception that way.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!