* [PATCH net v3 1/2] net: phy: marvell10g: disable temperature sensor on 2110
@ 2020-04-23 5:08 Baruch Siach
2020-04-23 5:08 ` [PATCH net v3 2/2] net: phy: marvell10g: hwmon support for 2110 Baruch Siach
2020-04-23 13:37 ` [PATCH net v3 1/2] net: phy: marvell10g: disable temperature sensor on 2110 Russell King - ARM Linux admin
0 siblings, 2 replies; 6+ messages in thread
From: Baruch Siach @ 2020-04-23 5:08 UTC (permalink / raw)
To: Russell King
Cc: netdev, Andrew Lunn, Florian Fainelli, Heiner Kallweit,
Baruch Siach, Maxime Chevallier
The 88E2110 temperature sensor is in a different location than 88X3310,
and it has no enable/disable option.
Fixes: 62d01535474b61 ("net: phy: marvell10g: add support for the 88x2110 PHY")
Cc: Maxime Chevallier <maxime.chevallier@bootlin.com>
Signed-off-by: Baruch Siach <baruch@tkos.co.il>
---
v3: No change
v2: No change
---
drivers/net/phy/marvell10g.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/net/phy/marvell10g.c b/drivers/net/phy/marvell10g.c
index 95e3f4644aeb..69530a84450f 100644
--- a/drivers/net/phy/marvell10g.c
+++ b/drivers/net/phy/marvell10g.c
@@ -169,6 +169,9 @@ static int mv3310_hwmon_config(struct phy_device *phydev, bool enable)
u16 val;
int ret;
+ if (phydev->drv->phy_id != MARVELL_PHY_ID_88X3310)
+ return 0;
+
ret = phy_write_mmd(phydev, MDIO_MMD_VEND2, MV_V2_TEMP,
MV_V2_TEMP_UNKNOWN);
if (ret < 0)
@@ -193,6 +196,9 @@ static int mv3310_hwmon_probe(struct phy_device *phydev)
struct mv3310_priv *priv = dev_get_drvdata(&phydev->mdio.dev);
int i, j, ret;
+ if (phydev->drv->phy_id != MARVELL_PHY_ID_88X3310)
+ return 0;
+
priv->hwmon_name = devm_kstrdup(dev, dev_name(dev), GFP_KERNEL);
if (!priv->hwmon_name)
return -ENODEV;
--
2.26.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH net v3 2/2] net: phy: marvell10g: hwmon support for 2110
2020-04-23 5:08 [PATCH net v3 1/2] net: phy: marvell10g: disable temperature sensor on 2110 Baruch Siach
@ 2020-04-23 5:08 ` Baruch Siach
2020-04-23 13:09 ` Andrew Lunn
2020-04-23 13:39 ` Russell King - ARM Linux admin
2020-04-23 13:37 ` [PATCH net v3 1/2] net: phy: marvell10g: disable temperature sensor on 2110 Russell King - ARM Linux admin
1 sibling, 2 replies; 6+ messages in thread
From: Baruch Siach @ 2020-04-23 5:08 UTC (permalink / raw)
To: Russell King
Cc: netdev, Andrew Lunn, Florian Fainelli, Heiner Kallweit, Baruch Siach
Read the temperature sensor register from the correct location for the
88E2110 PHY. There is no enable/disable bit, so leave
mv3310_hwmon_config() for 88X3310 only.
Signed-off-by: Baruch Siach <baruch@tkos.co.il>
---
v3: Split temperature register read routine per variant (Andrew Lunn)
v2: Fix indentation (Andrew Lunn)
---
drivers/net/phy/marvell10g.c | 25 +++++++++++++++++++++++--
1 file changed, 23 insertions(+), 2 deletions(-)
diff --git a/drivers/net/phy/marvell10g.c b/drivers/net/phy/marvell10g.c
index 69530a84450f..e14b9c2e5efe 100644
--- a/drivers/net/phy/marvell10g.c
+++ b/drivers/net/phy/marvell10g.c
@@ -66,6 +66,8 @@ enum {
MV_PCS_CSSR1_SPD2_2500 = 0x0004,
MV_PCS_CSSR1_SPD2_10000 = 0x0000,
+ MV_PCS_TEMP = 0x8042,
+
/* These registers appear at 0x800X and 0xa00X - the 0xa00X control
* registers appear to set themselves to the 0x800X when AN is
* restarted, but status registers appear readable from either.
@@ -104,6 +106,24 @@ static umode_t mv3310_hwmon_is_visible(const void *data,
return 0;
}
+static int mv3310_hwmon_read_temp_reg(struct phy_device *phydev)
+{
+ return phy_read_mmd(phydev, MDIO_MMD_VEND2, MV_V2_TEMP);
+}
+
+static int mv2110_hwmon_read_temp_reg(struct phy_device *phydev)
+{
+ return phy_read_mmd(phydev, MDIO_MMD_PCS, MV_PCS_TEMP);
+}
+
+static int mv10g_hwmon_read_temp_reg(struct phy_device *phydev)
+{
+ if (phydev->drv->phy_id == MARVELL_PHY_ID_88X3310)
+ return mv3310_hwmon_read_temp_reg(phydev);
+ else /* MARVELL_PHY_ID_88E2110 */
+ return mv2110_hwmon_read_temp_reg(phydev);
+}
+
static int mv3310_hwmon_read(struct device *dev, enum hwmon_sensor_types type,
u32 attr, int channel, long *value)
{
@@ -116,7 +136,7 @@ static int mv3310_hwmon_read(struct device *dev, enum hwmon_sensor_types type,
}
if (type == hwmon_temp && attr == hwmon_temp_input) {
- temp = phy_read_mmd(phydev, MDIO_MMD_VEND2, MV_V2_TEMP);
+ temp = mv10g_hwmon_read_temp_reg(phydev);
if (temp < 0)
return temp;
@@ -196,7 +216,8 @@ static int mv3310_hwmon_probe(struct phy_device *phydev)
struct mv3310_priv *priv = dev_get_drvdata(&phydev->mdio.dev);
int i, j, ret;
- if (phydev->drv->phy_id != MARVELL_PHY_ID_88X3310)
+ if (phydev->drv->phy_id != MARVELL_PHY_ID_88X3310 &&
+ phydev->drv->phy_id != MARVELL_PHY_ID_88E2110)
return 0;
priv->hwmon_name = devm_kstrdup(dev, dev_name(dev), GFP_KERNEL);
--
2.26.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH net v3 2/2] net: phy: marvell10g: hwmon support for 2110
2020-04-23 5:08 ` [PATCH net v3 2/2] net: phy: marvell10g: hwmon support for 2110 Baruch Siach
@ 2020-04-23 13:09 ` Andrew Lunn
2020-04-23 13:39 ` Russell King - ARM Linux admin
1 sibling, 0 replies; 6+ messages in thread
From: Andrew Lunn @ 2020-04-23 13:09 UTC (permalink / raw)
To: Baruch Siach; +Cc: Russell King, netdev, Florian Fainelli, Heiner Kallweit
On Thu, Apr 23, 2020 at 08:08:02AM +0300, Baruch Siach wrote:
> Read the temperature sensor register from the correct location for the
> 88E2110 PHY. There is no enable/disable bit, so leave
> mv3310_hwmon_config() for 88X3310 only.
>
> Signed-off-by: Baruch Siach <baruch@tkos.co.il>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Andrew
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net v3 1/2] net: phy: marvell10g: disable temperature sensor on 2110
2020-04-23 5:08 [PATCH net v3 1/2] net: phy: marvell10g: disable temperature sensor on 2110 Baruch Siach
2020-04-23 5:08 ` [PATCH net v3 2/2] net: phy: marvell10g: hwmon support for 2110 Baruch Siach
@ 2020-04-23 13:37 ` Russell King - ARM Linux admin
1 sibling, 0 replies; 6+ messages in thread
From: Russell King - ARM Linux admin @ 2020-04-23 13:37 UTC (permalink / raw)
To: Baruch Siach
Cc: netdev, Andrew Lunn, Florian Fainelli, Heiner Kallweit,
Maxime Chevallier
On Thu, Apr 23, 2020 at 08:08:01AM +0300, Baruch Siach wrote:
> The 88E2110 temperature sensor is in a different location than 88X3310,
> and it has no enable/disable option.
>
> Fixes: 62d01535474b61 ("net: phy: marvell10g: add support for the 88x2110 PHY")
> Cc: Maxime Chevallier <maxime.chevallier@bootlin.com>
> Signed-off-by: Baruch Siach <baruch@tkos.co.il>
> ---
Shouldn't this series have a covering message?
In any case:
Reviewed-by: Russell King <rmk+kernel@armlinux.org.uk>
> v3: No change
>
> v2: No change
> ---
> drivers/net/phy/marvell10g.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/drivers/net/phy/marvell10g.c b/drivers/net/phy/marvell10g.c
> index 95e3f4644aeb..69530a84450f 100644
> --- a/drivers/net/phy/marvell10g.c
> +++ b/drivers/net/phy/marvell10g.c
> @@ -169,6 +169,9 @@ static int mv3310_hwmon_config(struct phy_device *phydev, bool enable)
> u16 val;
> int ret;
>
> + if (phydev->drv->phy_id != MARVELL_PHY_ID_88X3310)
> + return 0;
> +
> ret = phy_write_mmd(phydev, MDIO_MMD_VEND2, MV_V2_TEMP,
> MV_V2_TEMP_UNKNOWN);
> if (ret < 0)
> @@ -193,6 +196,9 @@ static int mv3310_hwmon_probe(struct phy_device *phydev)
> struct mv3310_priv *priv = dev_get_drvdata(&phydev->mdio.dev);
> int i, j, ret;
>
> + if (phydev->drv->phy_id != MARVELL_PHY_ID_88X3310)
> + return 0;
> +
> priv->hwmon_name = devm_kstrdup(dev, dev_name(dev), GFP_KERNEL);
> if (!priv->hwmon_name)
> return -ENODEV;
> --
> 2.26.1
>
>
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 10.2Mbps down 587kbps up
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net v3 2/2] net: phy: marvell10g: hwmon support for 2110
2020-04-23 5:08 ` [PATCH net v3 2/2] net: phy: marvell10g: hwmon support for 2110 Baruch Siach
2020-04-23 13:09 ` Andrew Lunn
@ 2020-04-23 13:39 ` Russell King - ARM Linux admin
2020-04-23 13:41 ` Russell King - ARM Linux admin
1 sibling, 1 reply; 6+ messages in thread
From: Russell King - ARM Linux admin @ 2020-04-23 13:39 UTC (permalink / raw)
To: Baruch Siach; +Cc: netdev, Andrew Lunn, Florian Fainelli, Heiner Kallweit
On Thu, Apr 23, 2020 at 08:08:02AM +0300, Baruch Siach wrote:
> Read the temperature sensor register from the correct location for the
> 88E2110 PHY. There is no enable/disable bit, so leave
> mv3310_hwmon_config() for 88X3310 only.
>
> Signed-off-by: Baruch Siach <baruch@tkos.co.il>
> ---
> v3: Split temperature register read routine per variant (Andrew Lunn)
>
> v2: Fix indentation (Andrew Lunn)
> ---
> drivers/net/phy/marvell10g.c | 25 +++++++++++++++++++++++--
> 1 file changed, 23 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/phy/marvell10g.c b/drivers/net/phy/marvell10g.c
> index 69530a84450f..e14b9c2e5efe 100644
> --- a/drivers/net/phy/marvell10g.c
> +++ b/drivers/net/phy/marvell10g.c
> @@ -66,6 +66,8 @@ enum {
> MV_PCS_CSSR1_SPD2_2500 = 0x0004,
> MV_PCS_CSSR1_SPD2_10000 = 0x0000,
>
> + MV_PCS_TEMP = 0x8042,
Please add a comment mentioning that this is for the 88E2110, and
it would probably be a good idea to document the MV_V2_TEMP definition
as 88X3310 specific as well.
> +
> /* These registers appear at 0x800X and 0xa00X - the 0xa00X control
> * registers appear to set themselves to the 0x800X when AN is
> * restarted, but status registers appear readable from either.
> @@ -104,6 +106,24 @@ static umode_t mv3310_hwmon_is_visible(const void *data,
> return 0;
> }
>
> +static int mv3310_hwmon_read_temp_reg(struct phy_device *phydev)
> +{
> + return phy_read_mmd(phydev, MDIO_MMD_VEND2, MV_V2_TEMP);
> +}
> +
> +static int mv2110_hwmon_read_temp_reg(struct phy_device *phydev)
> +{
> + return phy_read_mmd(phydev, MDIO_MMD_PCS, MV_PCS_TEMP);
> +}
> +
> +static int mv10g_hwmon_read_temp_reg(struct phy_device *phydev)
> +{
> + if (phydev->drv->phy_id == MARVELL_PHY_ID_88X3310)
> + return mv3310_hwmon_read_temp_reg(phydev);
> + else /* MARVELL_PHY_ID_88E2110 */
> + return mv2110_hwmon_read_temp_reg(phydev);
> +}
> +
> static int mv3310_hwmon_read(struct device *dev, enum hwmon_sensor_types type,
> u32 attr, int channel, long *value)
> {
> @@ -116,7 +136,7 @@ static int mv3310_hwmon_read(struct device *dev, enum hwmon_sensor_types type,
> }
>
> if (type == hwmon_temp && attr == hwmon_temp_input) {
> - temp = phy_read_mmd(phydev, MDIO_MMD_VEND2, MV_V2_TEMP);
> + temp = mv10g_hwmon_read_temp_reg(phydev);
> if (temp < 0)
> return temp;
>
> @@ -196,7 +216,8 @@ static int mv3310_hwmon_probe(struct phy_device *phydev)
> struct mv3310_priv *priv = dev_get_drvdata(&phydev->mdio.dev);
> int i, j, ret;
>
> - if (phydev->drv->phy_id != MARVELL_PHY_ID_88X3310)
> + if (phydev->drv->phy_id != MARVELL_PHY_ID_88X3310 &&
> + phydev->drv->phy_id != MARVELL_PHY_ID_88E2110)
> return 0;
Doesn't that mean this condition can be removed, as this can only be
reached when one of those conditions is true?
>
> priv->hwmon_name = devm_kstrdup(dev, dev_name(dev), GFP_KERNEL);
> --
> 2.26.1
>
>
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 10.2Mbps down 587kbps up
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net v3 2/2] net: phy: marvell10g: hwmon support for 2110
2020-04-23 13:39 ` Russell King - ARM Linux admin
@ 2020-04-23 13:41 ` Russell King - ARM Linux admin
0 siblings, 0 replies; 6+ messages in thread
From: Russell King - ARM Linux admin @ 2020-04-23 13:41 UTC (permalink / raw)
To: Baruch Siach; +Cc: netdev, Andrew Lunn, Florian Fainelli, Heiner Kallweit
On Thu, Apr 23, 2020 at 02:39:36PM +0100, Russell King - ARM Linux admin wrote:
> On Thu, Apr 23, 2020 at 08:08:02AM +0300, Baruch Siach wrote:
> > Read the temperature sensor register from the correct location for the
> > 88E2110 PHY. There is no enable/disable bit, so leave
> > mv3310_hwmon_config() for 88X3310 only.
> >
> > Signed-off-by: Baruch Siach <baruch@tkos.co.il>
> > ---
> > v3: Split temperature register read routine per variant (Andrew Lunn)
> >
> > v2: Fix indentation (Andrew Lunn)
> > ---
> > drivers/net/phy/marvell10g.c | 25 +++++++++++++++++++++++--
> > 1 file changed, 23 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/net/phy/marvell10g.c b/drivers/net/phy/marvell10g.c
> > index 69530a84450f..e14b9c2e5efe 100644
> > --- a/drivers/net/phy/marvell10g.c
> > +++ b/drivers/net/phy/marvell10g.c
> > @@ -66,6 +66,8 @@ enum {
> > MV_PCS_CSSR1_SPD2_2500 = 0x0004,
> > MV_PCS_CSSR1_SPD2_10000 = 0x0000,
> >
> > + MV_PCS_TEMP = 0x8042,
>
> Please add a comment mentioning that this is for the 88E2110, and
> it would probably be a good idea to document the MV_V2_TEMP definition
> as 88X3310 specific as well.
>
> > +
> > /* These registers appear at 0x800X and 0xa00X - the 0xa00X control
> > * registers appear to set themselves to the 0x800X when AN is
> > * restarted, but status registers appear readable from either.
> > @@ -104,6 +106,24 @@ static umode_t mv3310_hwmon_is_visible(const void *data,
> > return 0;
> > }
> >
> > +static int mv3310_hwmon_read_temp_reg(struct phy_device *phydev)
> > +{
> > + return phy_read_mmd(phydev, MDIO_MMD_VEND2, MV_V2_TEMP);
> > +}
> > +
> > +static int mv2110_hwmon_read_temp_reg(struct phy_device *phydev)
> > +{
> > + return phy_read_mmd(phydev, MDIO_MMD_PCS, MV_PCS_TEMP);
> > +}
> > +
> > +static int mv10g_hwmon_read_temp_reg(struct phy_device *phydev)
> > +{
> > + if (phydev->drv->phy_id == MARVELL_PHY_ID_88X3310)
> > + return mv3310_hwmon_read_temp_reg(phydev);
> > + else /* MARVELL_PHY_ID_88E2110 */
> > + return mv2110_hwmon_read_temp_reg(phydev);
> > +}
> > +
> > static int mv3310_hwmon_read(struct device *dev, enum hwmon_sensor_types type,
> > u32 attr, int channel, long *value)
> > {
> > @@ -116,7 +136,7 @@ static int mv3310_hwmon_read(struct device *dev, enum hwmon_sensor_types type,
> > }
> >
> > if (type == hwmon_temp && attr == hwmon_temp_input) {
> > - temp = phy_read_mmd(phydev, MDIO_MMD_VEND2, MV_V2_TEMP);
> > + temp = mv10g_hwmon_read_temp_reg(phydev);
> > if (temp < 0)
> > return temp;
> >
> > @@ -196,7 +216,8 @@ static int mv3310_hwmon_probe(struct phy_device *phydev)
> > struct mv3310_priv *priv = dev_get_drvdata(&phydev->mdio.dev);
> > int i, j, ret;
> >
> > - if (phydev->drv->phy_id != MARVELL_PHY_ID_88X3310)
> > + if (phydev->drv->phy_id != MARVELL_PHY_ID_88X3310 &&
> > + phydev->drv->phy_id != MARVELL_PHY_ID_88E2110)
> > return 0;
>
> Doesn't that mean this condition can be removed, as this can only be
> reached when one of those conditions is true?
Thinking about this more, I think it may make sense to either reverse
the order of this patch series, or even better combine the two patches
into a single patch.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 10.2Mbps down 587kbps up
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-04-23 13:42 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-23 5:08 [PATCH net v3 1/2] net: phy: marvell10g: disable temperature sensor on 2110 Baruch Siach
2020-04-23 5:08 ` [PATCH net v3 2/2] net: phy: marvell10g: hwmon support for 2110 Baruch Siach
2020-04-23 13:09 ` Andrew Lunn
2020-04-23 13:39 ` Russell King - ARM Linux admin
2020-04-23 13:41 ` Russell King - ARM Linux admin
2020-04-23 13:37 ` [PATCH net v3 1/2] net: phy: marvell10g: disable temperature sensor on 2110 Russell King - ARM Linux admin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).