[PATCH 0/4] hwmon: (adt7470) Add thermal framework support

[PATCH 0/4] hwmon: (adt7470) Add thermal framework support

[Luiz Angelo Daros de Luca]

The ADT7470 is a temperature monitor and PWM fan controller capable of
monitoring up to 10 external temperature sensors and controlling up to 4
PWM outputs.

Currently, the driver exposes these sensors and PWM controls solely via
the standard hwmon sysfs interface. This patch series integrates the
ADT7470 driver with the kernel's thermal subsystem, allowing its
temperature sensors to be referenced by Device Tree thermal zones.

To avoid saturating the I2C bus during frequent polling by the thermal
core, the thermal callbacks are designed to read from the driver's
internal cache.

Patch 1 adds the YAML device tree binding documentation, defining
the necessary #cooling-cells and #thermal-sensor-cells properties.
Patch 2 performs a minor refactoring to introduce the ADT7470_PWM_MAX
macro, replacing hardcoded values.
Patch 3 registers the 4 PWM channels as optional thermal cooling
devices.
Patch 4 registers the external temperature sensors as optional thermal
zones.

Signed-off-by: Luiz Angelo Daros de Luca <luizluca@gmail.com>

---
Luiz Angelo Daros de Luca (4):
      dt-bindings: hwmon: add binding for adi,adt7470
      hwmon: (adt7470) Add ADT7470_PWM_MAX macro
      hwmon: (adt7470) Add thermal cooling device support
      hwmon: (adt7470) Add thermal zone sensor support

 .../devicetree/bindings/hwmon/adi,adt7470.yaml     |  53 ++++++
 drivers/hwmon/adt7470.c                            | 180 ++++++++++++++++++++-
 2 files changed, 230 insertions(+), 3 deletions(-)
---
base-commit: ca078d004cf58137bcf8cb24a8b271397431ba58
change-id: 20260716-adt7470_thermalzone-59a102278d72

Best regards,
--  
Luiz Angelo Daros de Luca <luizluca@gmail.com>

[PATCH 1/4] dt-bindings: hwmon: add binding for adi,adt7470

[Luiz Angelo Daros de Luca]

The Analog Devices ADT7470 is a temperature monitor and PWM fan
controller. It supports up to 10 external temperature sensors and
up to 4 PWM fan outputs.

Add the YAML device tree binding documentation for it. This includes
support for the thermal framework by defining the "#cooling-cells"
and "#thermal-sensor-cells" properties.

Signed-off-by: Luiz Angelo Daros de Luca <luizluca@gmail.com>
---
 .../devicetree/bindings/hwmon/adi,adt7470.yaml     | 53 ++++++++++++++++++++++
 1 file changed, 53 insertions(+)

diff --git a/Documentation/devicetree/bindings/hwmon/adi,adt7470.yaml b/Documentation/devicetree/bindings/hwmon/adi,adt7470.yaml
new file mode 100644
index 000000000000..e0c0fdadd1cf
--- /dev/null
+++ b/Documentation/devicetree/bindings/hwmon/adi,adt7470.yaml
@@ -0,0 +1,53 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/hwmon/adi,adt7470.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Analog Devices ADT7470 hwmon sensor
+
+maintainers:
+  - Luiz Angelo Daros de Luca <luizluca@gmail.com>
+
+description: |
+  The ADT7470 is a temperature monitor and multiple PWM outputs.
+  It supports monitoring up to 10 external temperature sensors and controlling
+  up to four fans.
+
+properties:
+  compatible:
+    const: adi,adt7470
+
+  reg:
+    maxItems: 1
+
+  "#cooling-cells":
+    const: 2
+    description: |
+      Cooling device exposing the 4 PWM channels.
+
+  "#thermal-sensor-cells":
+    const: 1
+    description: |
+      Number of cells required to uniquely identify the temperature sensors.
+      Valid index values are 0 to 9, corresponding to temp1 through temp10.
+
+required:
+  - compatible
+  - reg
+
+additionalProperties: false
+
+examples:
+  - |
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        hwmon@2f {
+            compatible = "adi,adt7470";
+            reg = <0x2f>;
+            #cooling-cells = <2>;
+            #thermal-sensor-cells = <1>;
+        };
+    };

-- 
2.55.0

Re: [PATCH 1/4] dt-bindings: hwmon: add binding for adi,adt7470

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The dt-binding for the multi-channel ADT7470 PWM fan controller omits the mandatory `#pwm-cells` property for a PWM output provider and incorrectly models the channels as a single cooling device.
--

--- Patch [1]: [PATCH 1/4] dt-bindings: hwmon: add binding for adi,adt7470 ---
commit 5b60ee676010a73eb2ebad187c402e61ddc230ad
Author: Luiz Angelo Daros de Luca <luizluca@gmail.com>

dt-bindings: hwmon: add binding for adi,adt7470

This commit adds the YAML device tree binding documentation for the Analog
Devices ADT7470 temperature monitor and PWM fan controller. It includes
support for the thermal framework by defining cooling and thermal sensor
properties.

> diff --git a/Documentation/devicetree/bindings/hwmon/adi,adt7470.yaml b/Documentation/devicetree/bindings/hwmon/adi,adt7470.yaml
> new file mode 100644
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/hwmon/adi,adt7470.yaml
[ ... ]
> +properties:
> +  compatible:
> +    const: adi,adt7470
> +
> +  reg:
> +    maxItems: 1
> +
> +  "#cooling-cells":
> +    const: 2
> +    description: |
> +      Cooling device exposing the 4 PWM channels.

[Severity: Medium]
Does defining #cooling-cells directly on the parent node force all 4 PWM
channels to be ganged together into a single cooling device? A 2-cell cooling
specifier lacks a cell for the channel index, which might permanently prevent
the thermal framework from addressing the individual fans independently. Would
it be more appropriate to model this using explicit fan sub-nodes or PWM
consumers?

> +
> +  "#thermal-sensor-cells":
> +    const: 1
> +    description: |
> +      Number of cells required to uniquely identify the temperature sensors.
> +      Valid index values are 0 to 9, corresponding to temp1 through temp10.
> +
> +required:
> +  - compatible
> +  - reg

[Severity: Medium]
Is the #pwm-cells property missing from this required list? Since the ADT7470
acts as a multi-channel PWM provider, subsystem guidelines suggest that PWM
output providers need to include #pwm-cells in both the properties and the
required list. Omitting this allows incomplete device trees to pass schema
validation.

> +
> +additionalProperties: false
[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260716-adt7470_thermalzone-v1-0-cc55ef35edde@gmail.com?part=1

Re: [PATCH 1/4] dt-bindings: hwmon: add binding for adi,adt7470

[Luiz Angelo Daros de Luca]

> +  "#cooling-cells":
> +    const: 2
> +    description: |
> +      Cooling device exposing the 4 PWM channels.

Since "thermal/of: Support cooling device ID in cooling-spec", I could
be 2 or 3. I don't know if I should use enum [2, 3] or just go
directly to the new const: 3.
My test device has only the 0 channel in use and 2 is fine for now.

Sashiko also asked to require #pwm-cells, but it would mean to
implement a parser for that too. I don't know if it is worth it but I
can do it if needed.
Regards,

Luiz

[PATCH 2/4] hwmon: (adt7470) Add ADT7470_PWM_MAX macro

[Luiz Angelo Daros de Luca]

Instead of a magic 255, use a macro to refer to it.

Signed-off-by: Luiz Angelo Daros de Luca <luizluca@gmail.com>
---
 drivers/hwmon/adt7470.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/hwmon/adt7470.c b/drivers/hwmon/adt7470.c
index 664349756dc2..4ac292cd7f47 100644
--- a/drivers/hwmon/adt7470.c
+++ b/drivers/hwmon/adt7470.c
@@ -100,6 +100,7 @@ static const unsigned short normal_i2c[] = { 0x2C, 0x2E, 0x2F, I2C_CLIENT_END };
 #define ADT7470_REG_FAN_MAX(x)	(ADT7470_REG_FAN_MAX_BASE_ADDR + ((x) * 2))
 
 #define ADT7470_PWM_COUNT	4
+#define ADT7470_PWM_MAX		255
 #define ADT7470_REG_PWM(x)	(ADT7470_REG_PWM_BASE_ADDR + (x))
 #define ADT7470_REG_PWM_MAX(x)	(ADT7470_REG_PWM_MAX_BASE_ADDR + (x))
 #define ADT7470_REG_PWM_MIN(x)	(ADT7470_REG_PWM_MIN_BASE_ADDR + (x))
@@ -811,7 +812,7 @@ static int adt7470_pwm_write(struct device *dev, u32 attr, int channel, long val
 
 	switch (attr) {
 	case hwmon_pwm_input:
-		val = clamp_val(val, 0, 255);
+		val = clamp_val(val, 0, ADT7470_PWM_MAX);
 		mutex_lock(&data->lock);
 		data->pwm[channel] = val;
 		err = regmap_write(data->regmap, ADT7470_REG_PWM(channel),
@@ -869,7 +870,7 @@ static ssize_t pwm_max_store(struct device *dev,
 	if (kstrtol(buf, 10, &temp))
 		return -EINVAL;
 
-	temp = clamp_val(temp, 0, 255);
+	temp = clamp_val(temp, 0, ADT7470_PWM_MAX);
 
 	mutex_lock(&data->lock);
 	data->pwm_max[attr->index] = temp;
@@ -904,7 +905,7 @@ static ssize_t pwm_min_store(struct device *dev,
 	if (kstrtol(buf, 10, &temp))
 		return -EINVAL;
 
-	temp = clamp_val(temp, 0, 255);
+	temp = clamp_val(temp, 0, ADT7470_PWM_MAX);
 
 	mutex_lock(&data->lock);
 	data->pwm_min[attr->index] = temp;

-- 
2.55.0

Re: [PATCH 2/4] hwmon: (adt7470) Add ADT7470_PWM_MAX macro

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] I2C error during adt7470_read_temperatures() leaves fans permanently stuck in manual mode
- [High] Unbounded sysfs values for auto_update_interval and num_temp_sensors cause a tight loop and I2C bus flooding
--

--- Patch [2]: [PATCH 2/4] hwmon: (adt7470) Add ADT7470_PWM_MAX macro ---
commit 176c7576aebc83ec3b52518b957109a401b37ada
Author: Luiz Angelo Daros de Luca <luizluca@gmail.com>

hwmon: (adt7470) Add ADT7470_PWM_MAX macro

Instead of a magic 255, use a macro to refer to it.

> diff --git a/drivers/hwmon/adt7470.c b/drivers/hwmon/adt7470.c
> index 664349756dc2b..4ac292cd7f47e 100644
> --- a/drivers/hwmon/adt7470.c
> +++ b/drivers/hwmon/adt7470.c
> @@ -100,6 +100,7 @@ static const unsigned short normal_i2c[] = { 0x2C, 0x2E, 0x2F, I2C_CLIENT_END };
>  #define ADT7470_REG_FAN_MAX(x)	(ADT7470_REG_FAN_MAX_BASE_ADDR + ((x) * 2))
>  
>  #define ADT7470_PWM_COUNT	4
> +#define ADT7470_PWM_MAX		255
>  #define ADT7470_REG_PWM(x)	(ADT7470_REG_PWM_BASE_ADDR + (x))
>  #define ADT7470_REG_PWM_MAX(x)	(ADT7470_REG_PWM_MAX_BASE_ADDR + (x))
>  #define ADT7470_REG_PWM_MIN(x)	(ADT7470_REG_PWM_MIN_BASE_ADDR + (x))

[ ... ]

[Severity: High]
This is a pre-existing issue, but could an I2C error during
adt7470_read_temperatures() leave the fans permanently stuck in manual mode?

If there is a transient I2C bus error (e.g., NACK) during
adt7470_read_temperatures() when updating ADT7470_REG_PWM_CFG(2) or
resetting ADT7470_T05_STB_MASK, it looks like it returns immediately:

drivers/hwmon/adt7470.c:adt7470_read_temperatures() {
	...
	err = regmap_update_bits(data->regmap, ADT7470_REG_PWM_CFG(2),
				 ADT7470_PWM_AUTO_MASK, 0);
	if (err < 0)
		return err;
	...
}

If the function returns here, does it fail to restore PWM_CFG(0) back from
manual mode?

On the next iteration of the periodic kthread, might the driver read this
corrupted manual mode as the original state, permanently locking the fans
in manual mode?

[Severity: High]
This is a pre-existing issue, but does an unbounded sysfs value for
auto_update_interval and num_temp_sensors cause a tight loop and I2C bus
flooding?

When setting the auto_update_interval, the value is allowed to be 0:

drivers/hwmon/adt7470.c:auto_update_interval_store() {
	...
	temp = clamp_val(temp, 0, 60000);

	mutex_lock(&data->lock);
	data->auto_update_interval = temp;
	...
}

If auto_update_interval is set to 0, does the background thread spin without
delay?

drivers/hwmon/adt7470.c:adt7470_update_thread() {
	...
		schedule_timeout_interruptible(msecs_to_jiffies(data->auto_update_interval));
	...
}

If both auto_update_interval and num_temp_sensors are 0, does this create a
busy-loop of I2C bus transactions?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260716-adt7470_thermalzone-v1-0-cc55ef35edde@gmail.com?part=2

[PATCH 3/4] hwmon: (adt7470) Add thermal cooling device support

[Luiz Angelo Daros de Luca]

The ADT7470 has four PWM outputs that can be used to control fans.
Register each PWM output as an optional thermal cooling device via
Device Tree so it can be referenced by thermal zones.

When the thermal subsystem sets a cooling state, the driver switches
the corresponding PWM channel to manual mode so that the requested duty
cycle takes effect.

Registration is optional: when the "#cooling-cells" property is absent,
devm_thermal_of_child_cooling_device_register() returns -ENODEV and the
driver silently skips that PWM channel.

The PWM cache is populated with an explicit adt7470_update_sensors()
call before the cooling devices are registered, so that an early
get_cur_state() call from the thermal core does not observe stale
(zeroed) values.

Signed-off-by: Luiz Angelo Daros de Luca <luizluca@gmail.com>
---
 drivers/hwmon/adt7470.c | 122 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 122 insertions(+)

diff --git a/drivers/hwmon/adt7470.c b/drivers/hwmon/adt7470.c
index 4ac292cd7f47..b865f4bd50f8 100644
--- a/drivers/hwmon/adt7470.c
+++ b/drivers/hwmon/adt7470.c
@@ -22,6 +22,7 @@
 #include <linux/sched.h>
 #include <linux/slab.h>
 #include <linux/util_macros.h>
+#include <linux/thermal.h>
 
 /* Addresses to scan */
 static const unsigned short normal_i2c[] = { 0x2C, 0x2E, 0x2F, I2C_CLIENT_END };
@@ -142,6 +143,13 @@ static const unsigned short normal_i2c[] = { 0x2C, 0x2E, 0x2F, I2C_CLIENT_END };
 #define ADT7470_FREQ_MASK	0x70
 #define ADT7470_FREQ_SHIFT	4
 
+struct adt7470_data;
+
+struct adt7470_cooling_device {
+	struct adt7470_data *data;
+	int pwm_index;
+};
+
 struct adt7470_data {
 	struct regmap		*regmap;
 	struct mutex		lock;
@@ -171,6 +179,8 @@ struct adt7470_data {
 
 	struct task_struct	*auto_update;
 	unsigned int		auto_update_interval;
+
+	struct adt7470_cooling_device cooling_devices[ADT7470_PWM_COUNT];
 };
 
 /*
@@ -846,6 +856,107 @@ static int adt7470_pwm_write(struct device *dev, u32 attr, int channel, long val
 	return err;
 }
 
+static int adt7470_get_max_state(struct thermal_cooling_device *cdev,
+				 unsigned long *state)
+{
+	*state = ADT7470_PWM_MAX;
+	return 0;
+}
+
+static int adt7470_get_cur_state(struct thermal_cooling_device *cdev,
+				 unsigned long *state)
+{
+	struct adt7470_cooling_device *cooling = cdev->devdata;
+	struct adt7470_data *data = cooling->data;
+
+	mutex_lock(&data->lock);
+	*state = data->pwm[cooling->pwm_index];
+	mutex_unlock(&data->lock);
+
+	return 0;
+}
+
+static int adt7470_set_cur_state(struct thermal_cooling_device *cdev,
+				 unsigned long state)
+{
+	struct adt7470_cooling_device *cooling = cdev->devdata;
+	struct adt7470_data *data = cooling->data;
+	unsigned int pwm_auto_reg_mask;
+	int err;
+
+	if (cooling->pwm_index & 1)
+		pwm_auto_reg_mask = ADT7470_PWM2_AUTO_MASK;
+	else
+		pwm_auto_reg_mask = ADT7470_PWM1_AUTO_MASK;
+
+	state = clamp_val(state, 0, ADT7470_PWM_MAX);
+
+	mutex_lock(&data->lock);
+
+	if (data->pwm[cooling->pwm_index] == state &&
+	    data->pwm_automatic[cooling->pwm_index] == 0) {
+		mutex_unlock(&data->lock);
+		return 0;
+	}
+
+	/* Put the PWM channel in manual mode before updating it. */
+	err = regmap_update_bits(data->regmap,
+				 ADT7470_REG_PWM_CFG(cooling->pwm_index),
+				 pwm_auto_reg_mask, 0);
+	if (err < 0)
+		goto out;
+
+	data->pwm_automatic[cooling->pwm_index] = 0;
+
+	err = regmap_write(data->regmap,
+			   ADT7470_REG_PWM(cooling->pwm_index), state);
+
+	if (err < 0)
+		goto out;
+
+	data->pwm[cooling->pwm_index] = state;
+out:
+	mutex_unlock(&data->lock);
+
+	return err;
+}
+
+static const struct thermal_cooling_device_ops adt7470_cooling_ops = {
+	.get_max_state = adt7470_get_max_state,
+	.get_cur_state = adt7470_get_cur_state,
+	.set_cur_state = adt7470_set_cur_state,
+};
+
+static int adt7470_register_cooling_devices(struct device *dev,
+					    struct adt7470_data *data)
+{
+	int i;
+
+	for (i = 0; i < ADT7470_PWM_COUNT; i++) {
+		struct thermal_cooling_device *cdev;
+		char cdev_name[THERMAL_NAME_LENGTH];
+
+		data->cooling_devices[i].data = data;
+		data->cooling_devices[i].pwm_index = i;
+
+		snprintf(cdev_name, sizeof(cdev_name), "adt7470-pwm%d", i);
+		cdev = devm_thermal_of_child_cooling_device_register(dev,
+								     dev->of_node, cdev_name,
+								     &data->cooling_devices[i],
+								     &adt7470_cooling_ops);
+
+		if (IS_ERR(cdev)) {
+			if (PTR_ERR(cdev) == -ENODEV)
+				continue;
+			return dev_warn_probe(dev, PTR_ERR(cdev),
+					      "failed to register cooling device %s\n",
+					      cdev_name);
+		}
+	}
+
+	return 0;
+}
+
 static ssize_t pwm_max_show(struct device *dev,
 			    struct device_attribute *devattr, char *buf)
 {
@@ -1281,6 +1392,17 @@ static int adt7470_probe(struct i2c_client *client)
 	if (IS_ERR(hwmon_dev))
 		return PTR_ERR(hwmon_dev);
 
+	if (IS_ENABLED(CONFIG_THERMAL)) {
+		/* fill the cache before registering the cooling devices */
+		err = adt7470_update_sensors(data);
+		if (err)
+			return err;
+
+		err = adt7470_register_cooling_devices(dev, data);
+		if (err)
+			return err;
+	}
+
 	data->auto_update = kthread_run(adt7470_update_thread, client, "%s",
 					dev_name(hwmon_dev));
 	if (IS_ERR(data->auto_update))

-- 
2.55.0

Re: [PATCH 3/4] hwmon: (adt7470) Add thermal cooling device support

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] All 4 PWM cooling devices are registered with identical device tree nodes and cdev_id, preventing independent thermal zone binding.
- [High] `adt7470_update_sensors()` is called locklessly in `adt7470_probe()` after the hwmon device is registered, leading to a severe data race.
- [Medium] The driver unconditionally registers all 4 cooling devices, failing to make them optional based on `#cooling-cells`, contrary to the commit message.
--

--- Patch [3]: [PATCH 3/4] hwmon: (adt7470) Add thermal cooling device support ---
commit f3b7f3fcc935f749dd7f53e14d3533656b277e5b
Author: Luiz Angelo Daros de Luca <luizluca@gmail.com>

hwmon: (adt7470) Add thermal cooling device support

The ADT7470 has four PWM outputs that can be used to control fans.
Register each PWM output as an optional thermal cooling device via
Device Tree so it can be referenced by thermal zones.

When the thermal subsystem sets a cooling state, the driver switches
the corresponding PWM channel to manual mode so that the requested duty
cycle takes effect.

> Registration is optional: when the "#cooling-cells" property is absent,
> devm_thermal_of_child_cooling_device_register() returns -ENODEV and the
> driver silently skips that PWM channel.

[Severity: Medium]
Is this description accurate? Looking at thermal_of_cooling_device_register(),
it does not appear to parse the device tree for #cooling-cells during
registration. Since it doesn't return -ENODEV for a missing property,
doesn't this result in unconditionally registering all 4 cooling devices
even if the device tree does not specify them?

[ ... ]

> diff --git a/drivers/hwmon/adt7470.c b/drivers/hwmon/adt7470.c
> index 4ac292cd7f47e..b865f4bd50f87 100644
> --- a/drivers/hwmon/adt7470.c
> +++ b/drivers/hwmon/adt7470.c

[ ... ]

> @@ -846,6 +856,107 @@ static int adt7470_pwm_write(struct device *dev, u32 attr, int channel, long val
[ ... ]
> +static int adt7470_register_cooling_devices(struct device *dev,
> +					    struct adt7470_data *data)
> +{
> +	int i;
> +
> +	for (i = 0; i < ADT7470_PWM_COUNT; i++) {
> +		struct thermal_cooling_device *cdev;
> +		char cdev_name[THERMAL_NAME_LENGTH];
> +
> +		data->cooling_devices[i].data = data;
> +		data->cooling_devices[i].pwm_index = i;
> +
> +		snprintf(cdev_name, sizeof(cdev_name), "adt7470-pwm%d", i);
> +		cdev = devm_thermal_of_child_cooling_device_register(dev,
> +								     dev->of_node, cdev_name,
> +								     &data->cooling_devices[i],
> +								     &adt7470_cooling_ops);

[Severity: High]
When registering the cooling devices in a loop, does passing the same parent
dev->of_node for every PWM channel prevent independent thermal zone binding?

Since __devm_thermal_of_cooling_device_register() binds them all with the
same node, they will all get assigned cdev_id = 0. When the thermal core
parses a DT cooling map requiring a specific cdev_id, will it fail to match
the individual channels since they are indistinguishable?

> +
> +		if (IS_ERR(cdev)) {
> +			if (PTR_ERR(cdev) == -ENODEV)
> +				continue;

[Severity: Medium]
Will this check ever be hit? As noted in the commit message review above,
the thermal registration API does not return -ENODEV for missing device
tree properties, which causes all PWM channels to be registered
unconditionally.

> +			return dev_warn_probe(dev, PTR_ERR(cdev),
> +					      "failed to register cooling device %s\n",
> +					      cdev_name);
> +		}
> +	}
> +
> +	return 0;
> +}

[ ... ]

> @@ -1281,6 +1392,17 @@ static int adt7470_probe(struct i2c_client *client)
>  	if (IS_ERR(hwmon_dev))
>  		return PTR_ERR(hwmon_dev);
>  
> +	if (IS_ENABLED(CONFIG_THERMAL)) {
> +		/* fill the cache before registering the cooling devices */
> +		err = adt7470_update_sensors(data);

[Severity: High]
Can this call to adt7470_update_sensors() cause hardware configuration
corruption due to a missing lock?

This is executed in adt7470_probe() immediately after the hwmon device is
registered, meaning sysfs attributes are already exposed to userspace.

Since adt7470_update_sensors() calls adt7470_read_temperatures(), which
explicitly assumes data->lock is held while performing multi-step register
manipulations and a 2-second sleep, what prevents a concurrent userspace sysfs
access from racing during this window and corrupting the PWM configuration?

> +		if (err)
> +			return err;
> +
> +		err = adt7470_register_cooling_devices(dev, data);
> +		if (err)
> +			return err;
> +	}
> +
>  	data->auto_update = kthread_run(adt7470_update_thread, client, "%s",
>  					dev_name(hwmon_dev));

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260716-adt7470_thermalzone-v1-0-cc55ef35edde@gmail.com?part=3

[PATCH 4/4] hwmon: (adt7470) Add thermal zone sensor support

[Luiz Angelo Daros de Luca]

Expose the ADT7470 external temperature sensors to the thermal framework
via Device Tree. The thermal callbacks use the driver's cached sensor data,
avoiding any I2C bus traffic during frequent polling by the thermal core.

Signed-off-by: Luiz Angelo Daros de Luca <luizluca@gmail.com>
---
 drivers/hwmon/adt7470.c | 51 +++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 51 insertions(+)

diff --git a/drivers/hwmon/adt7470.c b/drivers/hwmon/adt7470.c
index b865f4bd50f8..44fe310f0650 100644
--- a/drivers/hwmon/adt7470.c
+++ b/drivers/hwmon/adt7470.c
@@ -150,6 +150,11 @@ struct adt7470_cooling_device {
 	int pwm_index;
 };
 
+struct adt7470_thermal_sensor {
+	struct adt7470_data *data;
+	int id;
+};
+
 struct adt7470_data {
 	struct regmap		*regmap;
 	struct mutex		lock;
@@ -181,6 +186,7 @@ struct adt7470_data {
 	unsigned int		auto_update_interval;
 
 	struct adt7470_cooling_device cooling_devices[ADT7470_PWM_COUNT];
+	struct adt7470_thermal_sensor thermal_sensors[ADT7470_TEMP_COUNT];
 };
 
 /*
@@ -957,6 +963,47 @@ static int adt7470_register_cooling_devices(struct device *dev,
 	return 0;
 }
 
+static int adt7470_get_temp(struct thermal_zone_device *tz, int *temp)
+{
+	struct adt7470_thermal_sensor *sensor = thermal_zone_device_priv(tz);
+	struct adt7470_data *data = sensor->data;
+
+	mutex_lock(&data->lock);
+	*temp = 1000 * data->temp[sensor->id];
+	mutex_unlock(&data->lock);
+
+	return 0;
+}
+
+static const struct thermal_zone_device_ops adt7470_thermal_ops = {
+	.get_temp = adt7470_get_temp,
+};
+
+static int adt7470_register_thermal_sensors(struct device *dev,
+					    struct adt7470_data *data)
+{
+	int i;
+
+	for (i = 0; i < ADT7470_TEMP_COUNT; i++) {
+		struct thermal_zone_device *tz;
+
+		data->thermal_sensors[i].data = data;
+		data->thermal_sensors[i].id = i;
+
+		tz = devm_thermal_of_zone_register(dev, i, &data->thermal_sensors[i],
+						   &adt7470_thermal_ops);
+		if (IS_ERR(tz)) {
+			if (PTR_ERR(tz) == -ENODEV)
+				continue;
+			return dev_warn_probe(dev, PTR_ERR(tz),
+					      "failed to register thermal zone %d\n",
+					      i);
+		}
+	}
+
+	return 0;
+}
+
 static ssize_t pwm_max_show(struct device *dev,
 			    struct device_attribute *devattr, char *buf)
 {
@@ -1401,6 +1448,10 @@ static int adt7470_probe(struct i2c_client *client)
 		err = adt7470_register_cooling_devices(dev, data);
 		if (err)
 			return err;
+
+		err = adt7470_register_thermal_sensors(dev, data);
+		if (err)
+			return err;
 	}
 
 	data->auto_update = kthread_run(adt7470_update_thread, client, "%s",

-- 
2.55.0

Re: [PATCH 4/4] hwmon: (adt7470) Add thermal zone sensor support

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
- [High] The thermal zone callback `adt7470_get_temp()` relies on a cached temperature array (`data->temp`) that is never updated by the background thread, resulting in permanently stale temperature readings.
- [High] Severe lock contention on `data->lock` stalling the thermal polling worker for up to 2 seconds.
- [High] Unprotected call to `adt7470_update_sensors()` during probe races with sysfs reads, allowing concurrent execution and hardware state corruption.
- [High] Cooling device ID collision prevents independent PWM channel control because `devm_thermal_of_child_cooling_device_register()` hardcodes `cdev_id = 0`.
- [Medium] The newly added thermal callback violates the hwmon subsystem locking guidelines by using a private mutex instead of `hwmon_lock()`.
--

--- Patch [4]: [PATCH 4/4] hwmon: (adt7470) Add thermal zone sensor support ---
commit 76a97c2b3fe41c135377aaeddcae6f2428b2ba1f
Author: Luiz Angelo Daros de Luca <luizluca@gmail.com>

hwmon: (adt7470) Add thermal zone sensor support
    
Expose the ADT7470 external temperature sensors to the thermal framework
via Device Tree. The thermal callbacks use the driver's cached sensor data,
avoiding any I2C bus traffic during frequent polling by the thermal core.

> diff --git a/drivers/hwmon/adt7470.c b/drivers/hwmon/adt7470.c
> index b865f4bd50f87..44fe310f06506 100644
> --- a/drivers/hwmon/adt7470.c
> +++ b/drivers/hwmon/adt7470.c

[ ... ]

> @@ -957,6 +963,47 @@ static int adt7470_register_cooling_devices(struct device *dev,
>  	return 0;
>  }
>  
> +static int adt7470_get_temp(struct thermal_zone_device *tz, int *temp)
> +{
> +	struct adt7470_thermal_sensor *sensor = thermal_zone_device_priv(tz);
> +	struct adt7470_data *data = sensor->data;
> +
> +	mutex_lock(&data->lock);

[Severity: Medium]
Does using this private mutex violate the hwmon subsystem locking guidelines?

The documentation specifies that drivers should use hwmon_lock() and
hwmon_unlock() for attributes registered outside the hwmon core, rather than
their internal lock.

[Severity: High]
Can taking this lock cause the thermal polling worker to stall?

The background thread adt7470_update_thread() acquires data->lock and then
sleeps for up to 2 seconds while waiting for hardware measurements:

adt7470_read_temperatures()
    res = msleep_interruptible((data->num_temp_sensors >= 0 ?
                                data->num_temp_sensors * 200 :
                                TEMP_COLLECTION_TIME));

Any concurrent attempt by the thermal core to call adt7470_get_temp() will
block on this exact same mutex for the duration of that sleep, potentially
starving the system thermal workqueue.

> +	*temp = 1000 * data->temp[sensor->id];

[Severity: High]
Will this temperature reading remain permanently stale?

The background thread adt7470_update_thread() calls
adt7470_read_temperatures(), which initiates conversions but returns early
without reading the values:

adt7470_read_temperatures()
    if (data->num_temp_sensors >= 0)
        return 0;

Since it bypasses reading the results into the data->temp cache, and the cache
is exclusively updated when userspace reads sysfs attributes via
adt7470_update_sensors(), relying entirely on the thermal core for polling
will prevent the reported temperature from ever changing.

> +	mutex_unlock(&data->lock);
> +
> +	return 0;
> +}

[ ... ]

> @@ -1401,6 +1448,10 @@ static int adt7470_probe(struct i2c_client *client)
>  		err = adt7470_register_cooling_devices(dev, data);

[Severity: High]
Does this cause a cooling device ID collision?

In adt7470_register_cooling_devices(), the driver loops to register 4 separate
cooling devices using devm_thermal_of_child_cooling_device_register(), passing
the exact same device node:

adt7470_register_cooling_devices()
    cdev = devm_thermal_of_child_cooling_device_register(dev,
                                                         dev->of_node,
                                                         ...);

Because this child API internally hardcodes cdev_id = 0, all 4 channels end up
sharing identical matching criteria. This prevents the thermal framework from
distinguishing between the PWM cooling devices, destroying independent channel
control.

[Severity: High]
Is there a race condition with sysfs reads during probe?

Earlier in adt7470_probe(), the hwmon device is registered with
devm_hwmon_device_register_with_info(), exposing sysfs attributes to userspace.
Immediately after, adt7470_update_sensors(data) is called without holding
data->lock:

adt7470_probe()
    hwmon_dev = devm_hwmon_device_register_with_info(...)
    ...
    err = adt7470_update_sensors(data);

A concurrent sysfs read can trigger adt7470_update_device(), which takes the
lock and calls adt7470_update_sensors() simultaneously. This interleaved
execution inside adt7470_read_temperatures() could corrupt hardware sequences.

>  		if (err)
>  			return err;
> +
> +		err = adt7470_register_thermal_sensors(dev, data);
> +		if (err)
> +			return err;
>  	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260716-adt7470_thermalzone-v1-0-cc55ef35edde@gmail.com?part=4

Re: [PATCH 4/4] hwmon: (adt7470) Add thermal zone sensor support

[Guenter Roeck]

On 7/16/26 14:21, Luiz Angelo Daros de Luca wrote:
> Expose the ADT7470 external temperature sensors to the thermal framework
> via Device Tree. The thermal callbacks use the driver's cached sensor data,
> avoiding any I2C bus traffic during frequent polling by the thermal core.
> 

Detailed explanation why HWMON_C_REGISTER_TZ does not work or is
insufficient goes here and into the driver code as comment.

Thanks,
Guenter

> Signed-off-by: Luiz Angelo Daros de Luca <luizluca@gmail.com>
> ---
>   drivers/hwmon/adt7470.c | 51 +++++++++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 51 insertions(+)
> 
> diff --git a/drivers/hwmon/adt7470.c b/drivers/hwmon/adt7470.c
> index b865f4bd50f8..44fe310f0650 100644
> --- a/drivers/hwmon/adt7470.c
> +++ b/drivers/hwmon/adt7470.c
> @@ -150,6 +150,11 @@ struct adt7470_cooling_device {
>   	int pwm_index;
>   };
>   
> +struct adt7470_thermal_sensor {
> +	struct adt7470_data *data;
> +	int id;
> +};
> +
>   struct adt7470_data {
>   	struct regmap		*regmap;
>   	struct mutex		lock;
> @@ -181,6 +186,7 @@ struct adt7470_data {
>   	unsigned int		auto_update_interval;
>   
>   	struct adt7470_cooling_device cooling_devices[ADT7470_PWM_COUNT];
> +	struct adt7470_thermal_sensor thermal_sensors[ADT7470_TEMP_COUNT];
>   };
>   
>   /*
> @@ -957,6 +963,47 @@ static int adt7470_register_cooling_devices(struct device *dev,
>   	return 0;
>   }
>   
> +static int adt7470_get_temp(struct thermal_zone_device *tz, int *temp)
> +{
> +	struct adt7470_thermal_sensor *sensor = thermal_zone_device_priv(tz);
> +	struct adt7470_data *data = sensor->data;
> +
> +	mutex_lock(&data->lock);
> +	*temp = 1000 * data->temp[sensor->id];
> +	mutex_unlock(&data->lock);
> +
> +	return 0;
> +}
> +
> +static const struct thermal_zone_device_ops adt7470_thermal_ops = {
> +	.get_temp = adt7470_get_temp,
> +};
> +
> +static int adt7470_register_thermal_sensors(struct device *dev,
> +					    struct adt7470_data *data)
> +{
> +	int i;
> +
> +	for (i = 0; i < ADT7470_TEMP_COUNT; i++) {
> +		struct thermal_zone_device *tz;
> +
> +		data->thermal_sensors[i].data = data;
> +		data->thermal_sensors[i].id = i;
> +
> +		tz = devm_thermal_of_zone_register(dev, i, &data->thermal_sensors[i],
> +						   &adt7470_thermal_ops);
> +		if (IS_ERR(tz)) {
> +			if (PTR_ERR(tz) == -ENODEV)
> +				continue;
> +			return dev_warn_probe(dev, PTR_ERR(tz),
> +					      "failed to register thermal zone %d\n",
> +					      i);
> +		}
> +	}
> +
> +	return 0;
> +}
> +
>   static ssize_t pwm_max_show(struct device *dev,
>   			    struct device_attribute *devattr, char *buf)
>   {
> @@ -1401,6 +1448,10 @@ static int adt7470_probe(struct i2c_client *client)
>   		err = adt7470_register_cooling_devices(dev, data);
>   		if (err)
>   			return err;
> +
> +		err = adt7470_register_thermal_sensors(dev, data);
> +		if (err)
> +			return err;
>   	}
>   
>   	data->auto_update = kthread_run(adt7470_update_thread, client, "%s",
> 

Re: [PATCH 4/4] hwmon: (adt7470) Add thermal zone sensor support

[Luiz Angelo Daros de Luca]

> Detailed explanation why HWMON_C_REGISTER_TZ does not work or is
> insufficient goes here and into the driver code as comment.

No need to explain anything. I was just unaware of the existence of
HWMON_C_REGISTER_TZ.

You are absolutely right. I'll refactor the driver to use it and make
the code much cleaner. Thanks!

Regards,

Luiz

Re: [PATCH 0/4] hwmon: (adt7470) Add thermal framework support

[Guenter Roeck]

On 7/16/26 14:21, Luiz Angelo Daros de Luca wrote:
> The ADT7470 is a temperature monitor and PWM fan controller capable of
> monitoring up to 10 external temperature sensors and controlling up to 4
> PWM outputs.
> 
> Currently, the driver exposes these sensors and PWM controls solely via
> the standard hwmon sysfs interface. This patch series integrates the
> ADT7470 driver with the kernel's thermal subsystem, allowing its
> temperature sensors to be referenced by Device Tree thermal zones.
> 
> To avoid saturating the I2C bus during frequent polling by the thermal
> core, the thermal callbacks are designed to read from the driver's
> internal cache.
> 
> Patch 1 adds the YAML device tree binding documentation, defining
> the necessary #cooling-cells and #thermal-sensor-cells properties.
> Patch 2 performs a minor refactoring to introduce the ADT7470_PWM_MAX
> macro, replacing hardcoded values.
> Patch 3 registers the 4 PWM channels as optional thermal cooling
> devices.
> Patch 4 registers the external temperature sensors as optional thermal
> zones.
> 
> Signed-off-by: Luiz Angelo Daros de Luca <luizluca@gmail.com>

This chip is 20+ years old, and it is very unlikely that it is used
in current designs. Please provide evidence that this code is actually
needed and not just a coding exercise.

Thanks,
Guenter

Re: [PATCH 0/4] hwmon: (adt7470) Add thermal framework support

[Luiz Angelo Daros de Luca]

> This chip is 20+ years old, and it is very unlikely that it is used
> in current designs. Please provide evidence that this code is actually
> needed and not just a coding exercise.

I understand. It is used in multiple Edgecore switches like ECS2100.
I'm preparing a port to OpenWrt.

https://github.com/luizluca/openwrt/commits/ecs2110/
https://forum.openwrt.org/t/add-support-for-edgecore-ecs2100-ecs4100-switches/251818/10

Regards,

Luiz