Subject: Re: [PATCH net-next 03/10] net: phy: add configuration of rx clock stop mode

From: "Russell King (Oracle)" <[email-masked]>
Date: Tue, 10 Dec 2024 09:51:54 +0000

On Tue, Dec 10, 2024 at 04:11:09AM +0100, Andrew Lunn wrote:
> > @@ -2073,6 +2073,7 @@ int phy_unregister_fixup_for_id(const char *bus_id);
> >  int phy_unregister_fixup_for_uid(u32 phy_uid, u32 phy_uid_mask);
> >  
> >  int phy_eee_tx_clock_stop_capable(struct phy_device *phydev);
> > +int phy_eee_rx_clock_stop(struct phy_device *phydev, bool clk_stop_enable);
> 
> Hi Russell
> 
> Do you have patches to MAC drivers using phylib, not phylink, using
> these two new calls?
> 
> We see phylib users get EEE wrong. I'm worried phylib users are going
> to try to use these new API methods and make an even bigger mess. If
> we think these should only be used by phylink, maybe they should be
> put into a header in drivers/net/phy to stop MAC drivers using them?

If we want to fix the current phylib-using drivers, then we do need
at minimum phy_eee_rx_clock_stop() to do that - we have drivers that
call phy_init_eee(..., true) which would need to call this.

It's quite rare that drivers allow the PHY to stop the clock. There
may be several reasons for this:

1) the MAC doesn't support EEE on the MII link type(s) that have a
   clock. (e.g. supporting EEE on SGMII but not RGMII.)

2) the documentation for the MAC doesn't comment on this aspect
   (so the safest thing is to always keep the clock running.)

3) the driver developer hasn't understood the feature and the safest
   approach is to pass phy_init_eee() with a value of zero/false
   which leaves the setting unchanged.

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