On Tue, Dec 17, 2024 at 09:50:12PM +0100, Heiner Kallweit wrote: > On 17.12.2024 11:43, Andrew Lunn wrote: > >> @@ -2071,6 +2071,7 @@ void phy_advertise_eee_all(struct phy_device *phydev); > >> void phy_support_sym_pause(struct phy_device *phydev); > >> void phy_support_asym_pause(struct phy_device *phydev); > >> void phy_support_eee(struct phy_device *phydev); > >> +void phy_disable_eee(struct phy_device *phydev); > > > > So we have three states: > > > > MAC tells PHYLIB it does support EEE > > MAC tells PHYLIB it does not support EEE > > MAC says nothing. > > > > Do we really want this? > > > > For phylink, i think we have a nice new clean design and can say, if > > the MAC does not indicate it supports EEE, we turn it off in the > > PHY. For phylib, we have more of a mess, and there could be MACs > > actually doing EEE by default using default setting but with no user > > space control. Do we want to keep this, or should we say any MAC which > > does not call phy_support_eee() before phy_start() would have EEE > > disabled in the PHY? > > > The case "MAC says nothing" isn't desirable. However, if we did what > you mention, we'd silently change the behavior of several drivers, > resulting in disabled EEE and higher power consumption. > I briefly grepped the kernel source for phy_start() and found about > 70 drivers. Some of them have the phylib EEE call, and in some cases > like cpsw the MAC doesn't support EEE. But what remains is IMO too > many drivers where we'd change the behavior. So for phylib, we keep with the three states. But phylink? Can we disable EEE when the MAC says nothing? Andrew
