* [PATCH net-next] net: phy: marvell: mv88e6390 temperature sensor reading
@ 2018-01-09 21:42 Andrew Lunn
2018-01-10 0:14 ` Florian Fainelli
2018-01-10 20:35 ` David Miller
0 siblings, 2 replies; 6+ messages in thread
From: Andrew Lunn @ 2018-01-09 21:42 UTC (permalink / raw)
To: David Miller; +Cc: netdev, Florian Fainelli, Andrew Lunn
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@lunn.ch>
---
drivers/net/phy/marvell.c | 151 +++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 150 insertions(+), 1 deletion(-)
diff --git a/drivers/net/phy/marvell.c b/drivers/net/phy/marvell.c
index fd66304f18b7..22d9bc9c33a4 100644
--- a/drivers/net/phy/marvell.c
+++ b/drivers/net/phy/marvell.c
@@ -96,6 +96,17 @@
#define MII_88E1510_TEMP_SENSOR 0x1b
#define MII_88E1510_TEMP_SENSOR_MASK 0xff
+#define MII_88E6390_MISC_TEST 0x1b
+#define MII_88E6390_MISC_TEST_SAMPLE_1S 0
+#define MII_88E6390_MISC_TEST_SAMPLE_10MS BIT(14)
+#define MII_88E6390_MISC_TEST_SAMPLE_DISABLE BIT(15)
+#define MII_88E6390_MISC_TEST_SAMPLE_ENABLE 0
+#define MII_88E6390_MISC_TEST_SAMPLE_MASK (0x3 << 14)
+
+#define MII_88E6390_TEMP_SENSOR 0x1c
+#define MII_88E6390_TEMP_SENSOR_MASK 0xff
+#define MII_88E6390_TEMP_SENSOR_SAMPLES 10
+
#define MII_88E1318S_PHY_MSCR1_REG 16
#define MII_88E1318S_PHY_MSCR1_PAD_ODD BIT(6)
@@ -1738,6 +1749,123 @@ static const struct hwmon_chip_info m88e1510_hwmon_chip_info = {
.info = m88e1510_hwmon_info,
};
+static int m88e6390_get_temp(struct phy_device *phydev, long *temp)
+{
+ int sum = 0;
+ int oldpage;
+ int ret = 0;
+ int i;
+
+ *temp = 0;
+
+ oldpage = phy_select_page(phydev, MII_MARVELL_MISC_TEST_PAGE);
+ if (oldpage < 0)
+ goto error;
+
+ /* Enable temperature sensor */
+ ret = __phy_read(phydev, MII_88E6390_MISC_TEST);
+ if (ret < 0)
+ goto error;
+
+ ret = ret & ~MII_88E6390_MISC_TEST_SAMPLE_MASK;
+ ret |= MII_88E6390_MISC_TEST_SAMPLE_ENABLE |
+ MII_88E6390_MISC_TEST_SAMPLE_1S;
+
+ ret = __phy_write(phydev, MII_88E6390_MISC_TEST, ret);
+ if (ret < 0)
+ goto error;
+
+ /* Wait for temperature to stabilize */
+ usleep_range(10000, 12000);
+
+ /* Reading the temperature sense has an errata. You need to read
+ * a number of times and take an average.
+ */
+ for (i = 0; i < MII_88E6390_TEMP_SENSOR_SAMPLES; i++) {
+ ret = __phy_read(phydev, MII_88E6390_TEMP_SENSOR);
+ if (ret < 0)
+ goto error;
+ sum += ret & MII_88E6390_TEMP_SENSOR_MASK;
+ }
+
+ sum /= MII_88E6390_TEMP_SENSOR_SAMPLES;
+ *temp = (sum - 75) * 1000;
+
+ /* Disable temperature sensor */
+ ret = __phy_read(phydev, MII_88E6390_MISC_TEST);
+ if (ret < 0)
+ goto error;
+
+ ret = ret & ~MII_88E6390_MISC_TEST_SAMPLE_MASK;
+ ret |= MII_88E6390_MISC_TEST_SAMPLE_DISABLE;
+
+ ret = __phy_write(phydev, MII_88E6390_MISC_TEST, ret);
+
+error:
+ phy_restore_page(phydev, oldpage, ret);
+
+ return ret;
+}
+
+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);
+ int err;
+
+ switch (attr) {
+ case hwmon_temp_input:
+ err = m88e6390_get_temp(phydev, temp);
+ break;
+ default:
+ return -EOPNOTSUPP;
+ }
+
+ return err;
+}
+
+static umode_t m88e6390_hwmon_is_visible(const void *data,
+ enum hwmon_sensor_types type,
+ u32 attr, int channel)
+{
+ if (type != hwmon_temp)
+ return 0;
+
+ switch (attr) {
+ case hwmon_temp_input:
+ return 0444;
+ default:
+ return 0;
+ }
+}
+
+static u32 m88e6390_hwmon_temp_config[] = {
+ HWMON_T_INPUT,
+ 0
+};
+
+static const struct hwmon_channel_info m88e6390_hwmon_temp = {
+ .type = hwmon_temp,
+ .config = m88e6390_hwmon_temp_config,
+};
+
+static const struct hwmon_channel_info *m88e6390_hwmon_info[] = {
+ &m88e1121_hwmon_chip,
+ &m88e6390_hwmon_temp,
+ NULL
+};
+
+static const struct hwmon_ops m88e6390_hwmon_hwmon_ops = {
+ .is_visible = m88e6390_hwmon_is_visible,
+ .read = m88e6390_hwmon_read,
+};
+
+static const struct hwmon_chip_info m88e6390_hwmon_chip_info = {
+ .ops = &m88e6390_hwmon_hwmon_ops,
+ .info = m88e6390_hwmon_info,
+};
+
static int marvell_hwmon_name(struct phy_device *phydev)
{
struct marvell_priv *priv = phydev->priv;
@@ -1784,6 +1912,11 @@ static int m88e1510_hwmon_probe(struct phy_device *phydev)
{
return marvell_hwmon_probe(phydev, &m88e1510_hwmon_chip_info);
}
+
+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;
+}
#endif
static int marvell_probe(struct phy_device *phydev)
@@ -1831,6 +1969,17 @@ static int m88e1510_probe(struct phy_device *phydev)
return m88e1510_hwmon_probe(phydev);
}
+static int m88e6390_probe(struct phy_device *phydev)
+{
+ int err;
+
+ err = marvell_probe(phydev);
+ if (err)
+ return err;
+
+ return m88e6390_hwmon_probe(phydev);
+}
+
static struct phy_driver marvell_drivers[] = {
{
.phy_id = MARVELL_PHY_ID_88E1101,
@@ -2122,7 +2271,7 @@ static struct phy_driver marvell_drivers[] = {
.name = "Marvell 88E6390",
.features = PHY_GBIT_FEATURES,
.flags = PHY_HAS_INTERRUPT,
- .probe = m88e1510_probe,
+ .probe = m88e6390_probe,
.config_init = &marvell_config_init,
.config_aneg = &m88e1510_config_aneg,
.read_status = &marvell_read_status,
--
2.15.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH net-next] net: phy: marvell: mv88e6390 temperature sensor reading
2018-01-09 21:42 [PATCH net-next] net: phy: marvell: mv88e6390 temperature sensor reading Andrew Lunn
@ 2018-01-10 0:14 ` Florian Fainelli
2018-01-10 13:52 ` Andrew Lunn
2018-01-10 20:35 ` David Miller
1 sibling, 1 reply; 6+ messages in thread
From: Florian Fainelli @ 2018-01-10 0:14 UTC (permalink / raw)
To: Andrew Lunn, David Miller; +Cc: netdev
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@lunn.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()?
[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?
--
Florian
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net-next] net: phy: marvell: mv88e6390 temperature sensor reading
2018-01-10 0:14 ` Florian Fainelli
@ 2018-01-10 13:52 ` Andrew Lunn
2018-01-10 14:05 ` Andrew Lunn
0 siblings, 1 reply; 6+ messages in thread
From: Andrew Lunn @ 2018-01-10 13:52 UTC (permalink / raw)
To: Florian Fainelli; +Cc: David Miller, netdev
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@lunn.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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net-next] net: phy: marvell: mv88e6390 temperature sensor reading
2018-01-10 13:52 ` Andrew Lunn
@ 2018-01-10 14:05 ` Andrew Lunn
2018-01-10 17:49 ` Florian Fainelli
0 siblings, 1 reply; 6+ messages in thread
From: Andrew Lunn @ 2018-01-10 14:05 UTC (permalink / raw)
To: Florian Fainelli; +Cc: David Miller, netdev
> > > +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.
Humm, actually, no. It makes it more complex. If marvell_hwmon_probe()
is a stub, and we keep m88e6390_hwmon_probe() as the real
implementation, it means we need m88e6390_hwmon_chip_info, so we can
pass it. Either i need a stub version of m88e6390_hwmon_chip_info, or
i just build all the hwmon code even when CONFIG_HWMON is
disabled. The compiler might be able to figure out it is all unused
and throw it away, but i doubt it.
Having m88e6390_hwmon_probe() a stub is much simpler.
Andrew
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net-next] net: phy: marvell: mv88e6390 temperature sensor reading
2018-01-10 14:05 ` Andrew Lunn
@ 2018-01-10 17:49 ` Florian Fainelli
0 siblings, 0 replies; 6+ messages in thread
From: Florian Fainelli @ 2018-01-10 17:49 UTC (permalink / raw)
To: Andrew Lunn; +Cc: David Miller, netdev
On 01/10/2018 06:05 AM, Andrew Lunn wrote:
>>>> +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.
>
> Humm, actually, no. It makes it more complex. If marvell_hwmon_probe()
> is a stub, and we keep m88e6390_hwmon_probe() as the real
> implementation, it means we need m88e6390_hwmon_chip_info, so we can
> pass it. Either i need a stub version of m88e6390_hwmon_chip_info, or
> i just build all the hwmon code even when CONFIG_HWMON is
> disabled. The compiler might be able to figure out it is all unused
> and throw it away, but i doubt it.
>
> Having m88e6390_hwmon_probe() a stub is much simpler.
Fair enough:
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
--
Florian
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net-next] net: phy: marvell: mv88e6390 temperature sensor reading
2018-01-09 21:42 [PATCH net-next] net: phy: marvell: mv88e6390 temperature sensor reading Andrew Lunn
2018-01-10 0:14 ` Florian Fainelli
@ 2018-01-10 20:35 ` David Miller
1 sibling, 0 replies; 6+ messages in thread
From: David Miller @ 2018-01-10 20:35 UTC (permalink / raw)
To: andrew; +Cc: netdev, f.fainelli
From: Andrew Lunn <andrew@lunn.ch>
Date: Tue, 9 Jan 2018 22:42:09 +0100
> 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@lunn.ch>
Applied, thanks Andrew.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2018-01-10 20:35 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-01-09 21:42 [PATCH net-next] net: phy: marvell: mv88e6390 temperature sensor reading Andrew Lunn
2018-01-10 0:14 ` Florian Fainelli
2018-01-10 13:52 ` Andrew Lunn
2018-01-10 14:05 ` Andrew Lunn
2018-01-10 17:49 ` Florian Fainelli
2018-01-10 20:35 ` David Miller
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.