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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <88a60be9-083b-4618-845c-6983bcad3540@roeck-us.net>
Date: Thu, 1 Feb 2024 05:18:23 -0800
From: Guenter Roeck <linux@...ck-us.net>
To: Dimitri Fedrau <dima.fedrau@...il.com>
Cc: Andrew Lunn <andrew@...n.ch>, Heiner Kallweit <hkallweit1@...il.com>,
 Russell King <linux@...linux.org.uk>, "David S. Miller"
 <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>,
 Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
 Jean Delvare <jdelvare@...e.com>, Stefan Eichenberger <eichest@...il.com>,
 netdev@...r.kernel.org, linux-kernel@...r.kernel.org,
 linux-hwmon@...r.kernel.org
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 <dima.fedrau@...il.com>
> ---
>   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