Re: [PATCH net-next 3/3] net: phy: marvell-88q2xxx: enable temperature sensor in mv88q2xxx_config_init

["Niklas Söderlund" <niklas.soderlund+renesas@ragnatech.se>]

Hi Dimitir,

Thanks for your work.

On 2025-02-14 17:32:05 +0100, Dimitri Fedrau wrote:
> Temperature sensor gets enabled for 88Q222X devices in
> mv88q222x_config_init. Move enabling to mv88q2xxx_config_init because
> all 88Q2XXX devices support the temperature sensor.

Is this true for mv88q2110 devices too? The current implementation only 
enables it for mv88q222x devices. The private structure is not even 
initialized for mv88q2110, and currently crashes. I have fixed that [1], 
but I'm not sure if that should be extended to also enable temperature 
sensor for mv88q2110?

> 
> Signed-off-by: Dimitri Fedrau <dima.fedrau@gmail.com>

In either case with [1] for an unrelated fix this is tested on 
mv88q2110.

Tested-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>

1.  https://lore.kernel.org/all/20250214174650.2056949-1-niklas.soderlund+renesas@ragnatech.se/

> ---
>  drivers/net/phy/marvell-88q2xxx.c | 21 +++++++++------------
>  1 file changed, 9 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/net/phy/marvell-88q2xxx.c b/drivers/net/phy/marvell-88q2xxx.c
> index 7b0913968bb404df1d271b040e698a4c8c391705..1859db10b3914f54486c7e6132b10b0353fa49e6 100644
> --- a/drivers/net/phy/marvell-88q2xxx.c
> +++ b/drivers/net/phy/marvell-88q2xxx.c
> @@ -513,6 +513,15 @@ static int mv88q2xxx_config_init(struct phy_device *phydev)
>  			return ret;
>  	}
>  
> +	/* Enable temperature sense */
> +	if (priv->enable_temp) {
> +		ret = phy_modify_mmd(phydev, MDIO_MMD_PCS,
> +				     MDIO_MMD_PCS_MV_TEMP_SENSOR2,
> +				     MDIO_MMD_PCS_MV_TEMP_SENSOR2_DIS_MASK, 0);
> +		if (ret < 0)
> +			return ret;
> +	}
> +
>  	return 0;
>  }
>  
> @@ -903,18 +912,6 @@ static int mv88q222x_revb1_revb2_config_init(struct phy_device *phydev)
>  
>  static int mv88q222x_config_init(struct phy_device *phydev)
>  {
> -	struct mv88q2xxx_priv *priv = phydev->priv;
> -	int ret;
> -
> -	/* Enable temperature sense */
> -	if (priv->enable_temp) {
> -		ret = phy_modify_mmd(phydev, MDIO_MMD_PCS,
> -				     MDIO_MMD_PCS_MV_TEMP_SENSOR2,
> -				     MDIO_MMD_PCS_MV_TEMP_SENSOR2_DIS_MASK, 0);
> -		if (ret < 0)
> -			return ret;
> -	}
> -
>  	if (phydev->c45_ids.device_ids[MDIO_MMD_PMAPMD] == PHY_ID_88Q2220_REVB0)
>  		return mv88q222x_revb0_config_init(phydev);
>  	else
> 
> -- 
> 2.39.5
> 

-- 
Kind Regards,
Niklas Söderlund