[PATCH net v2 1/2] net: phy: marvell10g: disable temperature sensor on 2110

[PATCH net v2 1/2] net: phy: marvell10g: disable temperature sensor on 2110

[Baruch Siach]

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>
---
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

[PATCH net v2 2/2] net: phy: marvell10g: hwmon support for 2110

[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>
---
v2: Fix indentation (Andrew Lunn)
---
 drivers/net/phy/marvell10g.c | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/drivers/net/phy/marvell10g.c b/drivers/net/phy/marvell10g.c
index 69530a84450f..b6115537eb66 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,14 @@ static umode_t mv3310_hwmon_is_visible(const void *data,
 	return 0;
 }
 
+static int mv3310_hwmon_read_temp_reg(struct phy_device *phydev)
+{
+	if (phydev->drv->phy_id == MARVELL_PHY_ID_88X3310)
+		return phy_read_mmd(phydev, MDIO_MMD_VEND2, MV_V2_TEMP);
+	else /* MARVELL_PHY_ID_88E2110 */
+		return phy_read_mmd(phydev, MDIO_MMD_PCS, MV_PCS_TEMP);
+}
+
 static int mv3310_hwmon_read(struct device *dev, enum hwmon_sensor_types type,
 			     u32 attr, int channel, long *value)
 {
@@ -116,7 +126,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 = mv3310_hwmon_read_temp_reg(phydev);
 		if (temp < 0)
 			return temp;
 
@@ -196,7 +206,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

Re: [PATCH net v2 2/2] net: phy: marvell10g: hwmon support for 2110

[Andrew Lunn]

On Tue, Apr 21, 2020 at 01:56:15PM +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>
> ---
> v2: Fix indentation (Andrew Lunn)
> ---
>  drivers/net/phy/marvell10g.c | 15 +++++++++++++--
>  1 file changed, 13 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/phy/marvell10g.c b/drivers/net/phy/marvell10g.c
> index 69530a84450f..b6115537eb66 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,14 @@ static umode_t mv3310_hwmon_is_visible(const void *data,
>  	return 0;
>  }
>  
> +static int mv3310_hwmon_read_temp_reg(struct phy_device *phydev)
> +{
> +	if (phydev->drv->phy_id == MARVELL_PHY_ID_88X3310)
> +		return phy_read_mmd(phydev, MDIO_MMD_VEND2, MV_V2_TEMP);
> +	else /* MARVELL_PHY_ID_88E2110 */
> +		return phy_read_mmd(phydev, MDIO_MMD_PCS, MV_PCS_TEMP);
> +}
> +

Hi Baruch

Given the discussion about splitting the driver up a bit, could you
add

mv2110_hwmon_read_temp_reg()

And maybe a wrapper

mvxx10_hwmon_read_temp_reg() which does the call into the correct
version? Not that i particularly like that prefix.

Do we have enough differences yet it is worth adding a structure of
function pointers for family member? But i would save that for
net-next, where as you are aiming for net with these patches.

	 Andrew