netdev - Re: [PATCH net-next] net: phy: marvell: mv88e6390 temperature sensor reading

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: <20180110135224.GA32117@lunn.ch>
Date:   Wed, 10 Jan 2018 14:52:24 +0100
From:   Andrew Lunn <andrew@...n.ch>
To:     Florian Fainelli <f.fainelli@...il.com>
Cc:     David Miller <davem@...emloft.net>, netdev <netdev@...r.kernel.org>
Subject: Re: [PATCH net-next] net: phy: marvell: mv88e6390 temperature sensor
 reading

On Tue, Jan 09, 2018 at 04:14:44PM -0800, Florian Fainelli wrote:
> On 01/09/2018 01:42 PM, Andrew Lunn wrote:
> > The internal PHYs in the mv88e6390 switch have a temperature sensor.
> > It uses a different register layout to other PHY currently supported.
> > It also has an errata, in that some reads of the sensor result in bad
> > values. So a number of reads need to be made, and the average taken.
> > 
> > Signed-off-by: Andrew Lunn <andrew@...n.ch>
> 
> Just a few nits below:
> 
> [snip]
> 
> > +static int m88e6390_hwmon_read(struct device *dev,
> > +			       enum hwmon_sensor_types type,
> > +			       u32 attr, int channel, long *temp)
> > +{
> > +	struct phy_device *phydev = dev_get_drvdata(dev);
> 
> Why not to_phy_device()?

Hi Florian

The marvell driver already supports two other temperature
sensors. While implementing this third, i just copy/pasted the
m88e1121 code, and then applied changes as needed for the 6390.
dev_get_drvdata() is what the other two use.

> [snip]
> 
> > +
> > +static int m88e6390_hwmon_probe(struct phy_device *phydev)
> > +{
> > +	return marvell_hwmon_probe(phydev, &m88e6390_hwmon_chip_info);
> > +}
> >  #else
> >  static int m88e1121_hwmon_probe(struct phy_device *phydev)
> >  {
> > @@ -1794,6 +1927,11 @@ static int m88e1510_hwmon_probe(struct phy_device *phydev)
> >  {
> >  	return 0;
> >  }
> > +
> > +static int m88e6390_hwmon_probe(struct phy_device *phydev)
> > +{
> > +	return 0;
> > +}
> 
> Instead of having to define m88e6390_hwmon_probe() twice, I would just
> make marvell_hwmon_probe() a stub when CONFIG_HWMON=n?

Yes, i could do that. But again, i'm just following the pattern from
the other two sensors.

So in order to make these change requests, i will add another two
patches to the series, which first changes the existing two temperature
sensors. Then add the 6390.

	 Andrew