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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20250117150804.GA16023@debian>
Date: Fri, 17 Jan 2025 16:08:04 +0100
From: Dimitri Fedrau <dima.fedrau@...il.com>
To: Andrew Lunn <andrew@...n.ch>
Cc: Heiner Kallweit <hkallweit1@...il.com>,
	Russell King <linux@...linux.org.uk>,
	"David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	Niklas Söderlund <niklas.soderlund+renesas@...natech.se>,
	Gregor Herburger <gregor.herburger@...tq-group.com>,
	Stefan Eichenberger <eichest@...il.com>, netdev@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] net: phy: marvell-88q2xxx: Fix temperature measurement
 with reset-gpios

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@gmail.com/

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