2024-02-01 13:36:20

by Guenter Roeck

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

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 <[email protected]>
> ---
> 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

2024-02-01 16:23:31

by Dimitri Fedrau

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

Am Thu, Feb 01, 2024 at 05:18:23AM -0800 schrieb Guenter Roeck:
[...]
> > +
> > +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.
>

Will fix it.

Dimitri