Re: net: ti: weirdness (was Re: [PATCH RFC net-next 00/23] net: phylink managed EEE support)

On Tue, Nov 26, 2024 at 12:51:36PM +0000, Russell King (Oracle) wrote:

In doing this, I came across the fact that the addition of phylib
managed EEE support has actually broken a huge number of drivers -
phylib will now overwrite all members of struct ethtool_keee whether
the netdev driver wants it or not. This leads to weird scenarios where
doing a get_eee() op followed by a set_eee() op results in e.g.
tx_lpi_timer being zeroed, because the MAC driver doesn't know it needs
to initialise phylib's phydev->eee_cfg.tx_lpi_timer member. This mess
really needs urgently addressing, and I believe it came about because
Andrew's patches were only partly merged via another party - I guess
highlighting the inherent danger of "thou shalt limit your patch series
to no more than 15 patches" when one has a subsystem who's in-kernel
API is changing.

Looking at the two TI offerings that call phy_ethtool_get_eee(), both
of them call the phylib functions from their ethtool ops, but it looks
like the driver does diddly squat with LPI state, which makes me wonder
why they implemented the calls to phy_ethtool_get_eee() and
phy_ethtool_set_eee(), since EEE will not be functional unless the PHY
has been configured with a SmartEEE mode outside the kernel.

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