Re: [PATCH] net: phy: marvell-88q2xxx: Fix temperature measurement with reset-gpios

[Dimitri Fedrau]

Hi Andrew,

Am Fri, Jan 17, 2025 at 02:22:55PM +0100 schrieb Andrew Lunn:
> >  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;
> > +	}
> 
> Does enabling the sensor when it is already enabled cause issues? I'm
> not sure if it is worth having priv->enable_temp just to save one
> write which is going to be performed very infrequently.

Tested it, there haven't been any issues with enabling it again.

You are right, but I would need struct mv88q2xxx_priv anyway for patch:
https://lore.kernel.org/netdev/20250110-marvell-88q2xxx-leds-v1-1-22e7734941c2@xxxxxxxxx/

There I'm running into the same issue as here and have to fix it there
too. Then it makes sense to keep the priv->enable_temp. I just wanted to
fix this issue before I continue with the patch for LED driver.

By the way, I forgot to add the fixes tag:
Fixes: a557a92e6881 ("net: phy: marvell-88q2xxx: add support for temperature sensor")
Will add it in V2.

Best regards,
Dimitri