Re: [PATCH 5/7] net: phy: at803x: add support for configuring SmartEEE

[Ahmad Fatoum <a.fatoum@pengutronix.de>]

On 10.07.23 08:36, Marco Felsch wrote:
> This commit port Linux commit:
> 
> | commit 390b4cad81484124db2b676ed20a265adc032bae
> | Author: Russell King <rmk+kernel@armlinux.org.uk>
> | Date:   Thu Jan 14 10:45:49 2021 +0000
> |
> |     net: phy: at803x: add support for configuring SmartEEE
> |
> |     SmartEEE for the atheros phy was deemed buggy by Freescale and commits
> |     were added to disable it for their boards.
> |
> |     In initial testing, SolidRun found that the default settings were
> |     causing disconnects but by increasing the Tw buffer time we could allow
> |     enough time for all parts of the link to come out of a low power state
> |     and function properly without causing a disconnect. This allows us to
> |     have functional power savings of between 300 and 400mW, rather than
> |     disabling the feature altogether.
> |
> |     This commit adds support for disabling SmartEEE and configuring the Tw
> |     parameters for 1G and 100M speeds.
> |
> |     Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> |     Signed-off-by: Jakub Kicinski <kuba@kernel.org>
> 
> I slightly adapted the at803x_config_init() as well to be more in line
> with the Linux code base.
> 
> Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>

Same question as in follow-up commit: Can't we just disable it unconditionally?

> ---
>  drivers/net/phy/at803x.c | 75 ++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 73 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/phy/at803x.c b/drivers/net/phy/at803x.c
> index 41010c58ad..cc425c318f 100644
> --- a/drivers/net/phy/at803x.c
> +++ b/drivers/net/phy/at803x.c
> @@ -59,6 +59,15 @@
>   */
>  #define AT8035_CLK_OUT_MASK			GENMASK(4, 3)
>  
> +#define AT803X_MMD3_SMARTEEE_CTL1		0x805b
> +#define AT803X_MMD3_SMARTEEE_CTL3		0x805d
> +#define AT803X_MMD3_SMARTEEE_CTL3_LPI_EN	BIT(8)
> +
> +#define AT803X_DISABLE_SMARTEEE			BIT(1)
> +
> +/* disable hibernation mode */
> +#define AT803X_DISABLE_HIBERNATION_MODE		BIT(2)
> +
>  #define AT803X_CLK_OUT_STRENGTH_MASK		GENMASK(8, 7)
>  #define AT803X_CLK_OUT_STRENGTH_FULL		0
>  #define AT803X_CLK_OUT_STRENGTH_HALF		1
> @@ -72,8 +81,11 @@
>  #define AT8030_PHY_ID_MASK			0xffffffef
>  
>  struct at803x_priv {
> +	int flags;
>  	u16 clk_25m_reg;
>  	u16 clk_25m_mask;
> +	u8 smarteee_lpi_tw_1g;
> +	u8 smarteee_lpi_tw_100m;
>  };
>  
>  static int at803x_debug_reg_read(struct phy_device *phydev, u16 reg)
> @@ -141,10 +153,29 @@ static int at803x_parse_dt(struct phy_device *phydev)
>  	const struct device *dev = &phydev->dev;
>  	const struct device_node *node = dev->of_node;
>  	struct at803x_priv *priv = phydev->priv;
> +	u32 freq, strength, tw;
>  	unsigned int sel;
> -	u32 freq, strength;
>  	int ret;
>  
> +	if (of_property_read_bool(node, "qca,disable-smarteee"))
> +		priv->flags |= AT803X_DISABLE_SMARTEEE;
> +
> +	if (!of_property_read_u32(node, "qca,smarteee-tw-us-1g", &tw)) {
> +		if (!tw || tw > 255) {
> +			phydev_err(phydev, "invalid qca,smarteee-tw-us-1g\n");
> +			return -EINVAL;
> +		}
> +		priv->smarteee_lpi_tw_1g = tw;
> +	}
> +
> +	if (!of_property_read_u32(node, "qca,smarteee-tw-us-100m", &tw)) {
> +		if (!tw || tw > 255) {
> +			phydev_err(phydev, "invalid qca,smarteee-tw-us-100m\n");
> +			return -EINVAL;
> +		}
> +		priv->smarteee_lpi_tw_100m = tw;
> +	}
> +
>  	ret = of_property_read_u32(node, "qca,clk-out-frequency", &freq);
>  	if (!ret) {
>  		switch (freq) {
> @@ -221,6 +252,38 @@ static int at803x_probe(struct phy_device *phydev)
>  	return at803x_parse_dt(phydev);
>  }
>  
> +static int at803x_smarteee_config(struct phy_device *phydev)
> +{
> +	struct at803x_priv *priv = phydev->priv;
> +	u16 mask = 0, val = 0;
> +	int ret;
> +
> +	if (priv->flags & AT803X_DISABLE_SMARTEEE)
> +		return phy_modify_mmd_indirect(phydev, MDIO_MMD_PCS,
> +					       AT803X_MMD3_SMARTEEE_CTL3,
> +					       AT803X_MMD3_SMARTEEE_CTL3_LPI_EN, 0);
> +
> +	if (priv->smarteee_lpi_tw_1g) {
> +		mask |= 0xff00;
> +		val |= priv->smarteee_lpi_tw_1g << 8;
> +	}
> +	if (priv->smarteee_lpi_tw_100m) {
> +		mask |= 0x00ff;
> +		val |= priv->smarteee_lpi_tw_100m;
> +	}
> +	if (!mask)
> +		return 0;
> +
> +	ret = phy_modify_mmd_indirect(phydev, MDIO_MMD_PCS, AT803X_MMD3_SMARTEEE_CTL1,
> +				      mask, val);
> +	if (ret)
> +		return ret;
> +
> +	return phy_modify_mmd_indirect(phydev, MDIO_MMD_PCS, AT803X_MMD3_SMARTEEE_CTL3,
> +				       AT803X_MMD3_SMARTEEE_CTL3_LPI_EN,
> +				       AT803X_MMD3_SMARTEEE_CTL3_LPI_EN);
> +}
> +
>  static int at803x_clk_out_config(struct phy_device *phydev)
>  {
>  	struct at803x_priv *priv = phydev->priv;
> @@ -270,7 +333,15 @@ static int at803x_config_init(struct phy_device *phydev)
>  	if (ret < 0)
>  		return ret;
>  
> -	return at803x_clk_out_config(phydev);
> +	ret = at803x_smarteee_config(phydev);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = at803x_clk_out_config(phydev);
> +	if (ret < 0)
> +		return ret;
> +
> +	return 0;
>  }
>  
>  static struct phy_driver at803x_driver[] = {

-- 
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 |