Re: [PATCH v5 net-next 08/13] net: phy: marvell-88q2xxx: add support for temperature sensor

[Guenter Roeck]

On 1/22/24 13:28, Dimitri Fedrau wrote:
> Marvell 88q2xxx devices have an inbuilt temperature sensor. Add hwmon
> support for this sensor.
>
> Signed-off-by: Dimitri Fedrau <dima.fedrau@xxxxxxxxx>
> ---
>   drivers/net/phy/marvell-88q2xxx.c | 152 ++++++++++++++++++++++++++++++
>   1 file changed, 152 insertions(+)
>
> diff --git a/drivers/net/phy/marvell-88q2xxx.c b/drivers/net/phy/marvell-88q2xxx.c
> index 4cb8fe524795..6900bad275d0 100644
> --- a/drivers/net/phy/marvell-88q2xxx.c
> +++ b/drivers/net/phy/marvell-88q2xxx.c
> @@ -5,6 +5,7 @@
>   #include <linux/ethtool_netlink.h>
>   #include <linux/marvell_phy.h>
>   #include <linux/phy.h>
> +#include <linux/hwmon.h>
>   
>   #define PHY_ID_88Q2220_REVB0	(MARVELL_PHY_ID_88Q2220 | 0x1)
>   
> @@ -33,6 +34,19 @@
>   #define MDIO_MMD_PCS_MV_GPIO_INT_CTRL			32787
>   #define MDIO_MMD_PCS_MV_GPIO_INT_CTRL_TRI_DIS		0x0800
>   
> +#define MDIO_MMD_PCS_MV_TEMP_SENSOR1			32833
> +#define MDIO_MMD_PCS_MV_TEMP_SENSOR1_RAW_INT		0x0001
> +#define MDIO_MMD_PCS_MV_TEMP_SENSOR1_INT		0x0040
> +#define MDIO_MMD_PCS_MV_TEMP_SENSOR1_INT_EN		0x0080
> +
> +#define MDIO_MMD_PCS_MV_TEMP_SENSOR2			32834
> +#define MDIO_MMD_PCS_MV_TEMP_SENSOR2_DIS_MASK		0xc000
> +
> +#define MDIO_MMD_PCS_MV_TEMP_SENSOR3			32835
> +#define MDIO_MMD_PCS_MV_TEMP_SENSOR3_INT_THRESH_MASK	0xff00
> +#define MDIO_MMD_PCS_MV_TEMP_SENSOR3_INT_THRESH_SHIFT	8
> +#define MDIO_MMD_PCS_MV_TEMP_SENSOR3_MASK		0x00ff
> +
>   #define MDIO_MMD_PCS_MV_100BT1_STAT1			33032
>   #define MDIO_MMD_PCS_MV_100BT1_STAT1_IDLE_ERROR		0x00ff
>   #define MDIO_MMD_PCS_MV_100BT1_STAT1_JABBER		0x0100
> @@ -488,6 +502,143 @@ static int mv88q2xxx_resume(struct phy_device *phydev)
>   	return phy_clear_bits_mmd(phydev, MDIO_MMD_PMAPMD, MDIO_CTRL1,
>   				  MDIO_CTRL1_LPOWER);
>   }
> +#ifdef CONFIG_HWMON

HWMON is tristate, so this may be problematic if the driver is built
into the kernel and hwmon is built as module.

[ ... ]
> +
> +static int mv88q2xxx_hwmon_write(struct device *dev,
> +				 enum hwmon_sensor_types type, u32 attr,
> +				 int channel, long val)
> +{
> +	struct phy_device *phydev = dev_get_drvdata(dev);
> +
> +	switch (attr) {
> +	case hwmon_temp_max:
> +		if (val < -75000 || val > 180000)
> +			return -EINVAL;
> +

Not that it matters much, but we typically use clamp_val() to limit
the range of temperature limits because the valid range differs for
each chip and is otherwise difficult to determine for the user.

Guenter