netdev - Re: [net-next] net: phy: marvell-88q2xxx: Enable temperature sensor for mv88q211x

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250317003249.GB660648@ragnatech.se>
Date: Mon, 17 Mar 2025 01:32:49 +0100
From: Niklas Söderlund <niklas.soderlund+renesas@...natech.se>
To: Dimitri Fedrau <dima.fedrau@...il.com>
Cc: Andrew Lunn <andrew@...n.ch>, 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>,
	netdev@...r.kernel.org, linux-renesas-soc@...r.kernel.org
Subject: Re: [net-next] net: phy: marvell-88q2xxx: Enable temperature sensor
 for mv88q211x

Hi Dimitri,

On 2025-03-16 16:23:24 +0100, Dimitri Fedrau wrote:
> Hi Niklas,
> 
> Am Sun, Mar 16, 2025 at 12:20:33PM +0100 schrieb Niklas Söderlund:
> > The temperature sensor enabled for mv88q222x devices also functions for
> > mv88q211x based devices. Unify the two devices probe functions to enable
> > the sensors for all devices supported by this driver.
> > 
> > The same oddity as for mv88q222x devices exists, the PHY must be up for
> > a correct temperature reading to be reported.
> > 
> >     # cat /sys/class/hwmon/hwmon9/temp1_input
> >     -75000
> > 
> >     # ifconfig end5 up
> > 
> >     # cat /sys/class/hwmon/hwmon9/temp1_input
> >     59000
> > 
> > Worth noting is that while the temperature register offsets and layout
> > are the same between mv88q211x and mv88q222x devices their names in the
> > datasheets are different. This change keeps the mv88q222x names for the
> > mv88q211x support.
> > 
> > Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@...natech.se>
> > ---
> >  drivers/net/phy/marvell-88q2xxx.c | 14 ++------------
> >  1 file changed, 2 insertions(+), 12 deletions(-)
> > 
> > diff --git a/drivers/net/phy/marvell-88q2xxx.c b/drivers/net/phy/marvell-88q2xxx.c
> > index 15c0f8adc2f5..cdd40b613ce8 100644
> > --- a/drivers/net/phy/marvell-88q2xxx.c
> > +++ b/drivers/net/phy/marvell-88q2xxx.c
> > @@ -834,6 +834,7 @@ static int mv88q2xxx_leds_probe(struct phy_device *phydev)
> >  static int mv88q2xxx_probe(struct phy_device *phydev)
> >  {
> >  	struct mv88q2xxx_priv *priv;
> > +	int ret;
> >  
> >  	priv = devm_kzalloc(&phydev->mdio.dev, sizeof(*priv), GFP_KERNEL);
> >  	if (!priv)
> > @@ -841,17 +842,6 @@ static int mv88q2xxx_probe(struct phy_device *phydev)
> >  
> >  	phydev->priv = priv;
> >  
> > -	return 0;
> > -}
> > -
> > -static int mv88q222x_probe(struct phy_device *phydev)
> > -{
> > -	int ret;
> > -
> > -	ret = mv88q2xxx_probe(phydev);
> > -	if (ret)
> > -		return ret;
> > -
> >  	ret = mv88q2xxx_leds_probe(phydev);
> >  	if (ret)
> >  		return ret;
> > @@ -1124,7 +1114,7 @@ static struct phy_driver mv88q2xxx_driver[] = {
> >  		.phy_id_mask		= MARVELL_PHY_ID_MASK,
> >  		.name			= "mv88q2220",
> >  		.flags			= PHY_POLL_CABLE_TEST,
> > -		.probe			= mv88q222x_probe,
> > +		.probe			= mv88q2xxx_probe,
> >  		.get_features		= mv88q2xxx_get_features,
> >  		.config_aneg		= mv88q2xxx_config_aneg,
> >  		.aneg_done		= genphy_c45_aneg_done,
> > -- 
> > 2.48.1
> > 
> 
> thanks for your patch. Looks good to me.
> Did you have the chance to test the LED support as well ? I'm asking
> because mv88q2xxx_leds_probe gets called in mv88q2xxx_probe. LED
> handling should be equal for all 88Q2XXX devices, adding the support
> should be easy. Anyway:

The board I have to test on is remote, so I have no way to inspect the 
LED. I did check the datasheet and schematics, and as you say it looks 
the same for both devices.

> 
> Reviewed-by: Dimitri Fedrau <dima.fedrau@...il.com>
> 
> Best regards,
> Dimitri Fedrau

-- 
Kind Regards,
Niklas Söderlund