On 17.12.2024 23:34, Andrew Lunn wrote: > 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? > Looking at patch 5 of Russell's series behavior doesn't change if pl->mac_supports_eee is false. So I think he also goes with the three states, at least initially, until all drivers using phylink have implemented the new phylink ops. AFAICS this affects about 25 drivers. > Andrew Heiner
