Re: [PATCH net] net: bcmgenet: fix broken EEE by converting to phylib-managed state

[Florian Fainelli <florian.fainelli@broadcom.com>]

On 3/6/26 00:31, Nicolai Buchwitz wrote:
> The bcmgenet EEE implementation is broken in several ways.
> phy_support_eee() is never called, so the PHY never advertises EEE
> and phylib never sets phydev->enable_tx_lpi.  bcmgenet_mac_config()
> checks priv->eee.eee_enabled to decide whether to enable the MAC
> LPI logic, but that field is never initialised to true, so the MAC
> never enters Low Power Idle even when EEE is negotiated - wasting
> the power savings EEE is designed to provide.  The only way to get
> EEE working at all is a manual 'ethtool --set-eee eth0 eee on' after
> every link-up, and even then bcmgenet_get_eee() immediately clobbers
> the reported state because phy_ethtool_get_eee() overwrites
> eee_enabled and tx_lpi_enabled with the uninitialised PHY eee_cfg
> values.  Finally, bcmgenet_mac_config() is only called on link-up,
> so EEE is never disabled in hardware on link-down.
> 
> Fix all of this by removing the MAC-side EEE state tracking
> (priv->eee) and aligning with the pattern used by other non-phylink
> MAC drivers such as FEC.
> 
> Call phy_support_eee() in bcmgenet_mii_probe() so the PHY advertises
> EEE link modes and phylib tracks negotiation state.  Move the EEE
> hardware control to bcmgenet_mii_setup(), which is called on every
> link event, and drive it directly from phydev->enable_tx_lpi - the
> flag phylib sets when EEE is negotiated and the user has not disabled
> it.  This enables EEE automatically once the link partner agrees and
> disables it cleanly on link-down.
> 
> Make bcmgenet_get_eee() and bcmgenet_set_eee() pure passthroughs to
> phy_ethtool_get_eee() and phy_ethtool_set_eee(), with the MAC
> hardware register read/written for tx_lpi_timer.  Drop struct
> ethtool_keee eee from struct bcmgenet_priv.
> 
> Fixes: fe0d4fd9285e ("net: phy: Keep track of EEE configuration")
> Link: https://lore.kernel.org/netdev/d352039f-4cbb-41e6-9aeb-0b4f3941b54c@lunn.ch/
> Suggested-by: Andrew Lunn <andrew@lunn.ch>
> Signed-off-by: Nicolai Buchwitz <nb@tipi-net.de>

Reviewed-by: Florian Fainelli <florian.fainelli@broadcom.com>
Tested-by: Florian Fainelli <florian.fainelli@broadcom.com>
-- 
Florian