Re: [RFC/RFT 03/23] net: phy: Add helper to set EEE Clock stop enable bit

[Oleksij Rempel <o.rempel@pengutronix.de>]

On Mon, Mar 27, 2023 at 07:01:41PM +0200, Andrew Lunn wrote:
> The MAC driver can request that the PHY stops the clock during EEE
> LPI. This has normally been does as part of phy_init_eee(), however
> that function is overly complex and often wrongly used. Add a
> standalone helper, to aid removing phy_init_eee().
> 
> Signed-off-by: Andrew Lunn <andrew@lunn.ch>
> ---
> v2: Add missing EXPORT_SYMBOL_GPL
> ---
>  drivers/net/phy/phy.c | 20 ++++++++++++++++++++
>  include/linux/phy.h   |  1 +
>  2 files changed, 21 insertions(+)
> 
> diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
> index 68e1ce942dd6..d3d6ff4ed488 100644
> --- a/drivers/net/phy/phy.c
> +++ b/drivers/net/phy/phy.c
> @@ -1503,6 +1503,26 @@ void phy_mac_interrupt(struct phy_device *phydev)
>  }
>  EXPORT_SYMBOL(phy_mac_interrupt);
>  
> +/**
> + * phy_eee_clk_stop_enable - Clock should stop during LIP
> + * @phydev: target phy_device struct
> + *
> + * Description: Program the MMD register 3.0 setting the "Clock stop enable"
> + * bit.


> + */
> +int phy_eee_clk_stop_enable(struct phy_device *phydev)

this function should go to drivers/net/phy/phy-c45.c
and renamed to genphy_c45_eee_clk_stop_enable()
> +{
> +	int ret;
> +
> +	mutex_lock(&phydev->lock);

	/* IEEE 802.3-2018 45.2.3.1.4 Clock stop enable (3.0.10) */

> +	ret = phy_set_bits_mmd(phydev, MDIO_MMD_PCS, MDIO_CTRL1,
> +			       MDIO_PCS_CTRL1_CLKSTOP_EN);

It would be better to write it conditionally. Only if EEE is supported
and only if this bit is supported as well. Support is indicated by the
IEEE 802.3:2018 - 45.2.3.2.6 Clock stop capable (3.1.6)

It looks like there are other registers for same functionality too but
other types of PHYs:
45.2.4.1.4 Clock stop enable (4.0.10)
45.2.4.2.6 Clock stop capable (4.1.6)
45.2.5.1.4 Clock stop enable (5.0.10)
45.2.5.2.6 Clock stop capable (5.1.6)

If I see it correctly, Clock-stop is possible only for GMII/RGMII.
Integrated PHYs or EEE capable PHYs with RMII do not support it.
For example KSZ8091RNA with RMII:
https://ww1.microchip.com/downloads/en/DeviceDoc/KSZ8091RNA-RND-10BASE-T-100BASE-TX-PHY-with-RMII-and-EEE-Support-DS00002298B.pdf
KSZ9477 switch with integrated PHYs:
https://ww1.microchip.com/downloads/aemDocuments/documents/OTH/ProductDocuments/DataSheets/KSZ9477S-Data-Sheet-DS00002392C.pdf

> +	mutex_unlock(&phydev->lock);
> +
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(phy_eee_clk_stop_enable);
> +
>  /**
>   * phy_init_eee - init and check the EEE feature
>   * @phydev: target phy_device struct
> diff --git a/include/linux/phy.h b/include/linux/phy.h
> index 2508f1d99777..12addd1c29f2 100644
> --- a/include/linux/phy.h
> +++ b/include/linux/phy.h
> @@ -1846,6 +1846,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_init_eee(struct phy_device *phydev, bool clk_stop_enable);
> +int phy_eee_clk_stop_enable(struct phy_device *phydev);
>  int phy_get_eee_err(struct phy_device *phydev);
>  int phy_ethtool_set_eee(struct phy_device *phydev, struct ethtool_eee *data);
>  int phy_ethtool_get_eee(struct phy_device *phydev, struct ethtool_eee *data);
> -- 
> 2.39.2
> 
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |