[PATCH v2 0/6] hwmon: (adt7470) Add thermal zone and PWM provider support

[PATCH v2 0/6] hwmon: (adt7470) Add thermal zone and PWM provider 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 and PWM subsystems. This allows
its temperature sensors to be referenced by Device Tree thermal zones,
and its PWM channels to be consumed by generic Device Tree nodes (such
as "pwm-fan").

Patch 1 adds the YAML device tree binding documentation.
Patch 2 and 3 fix two pre-existing bugs found while testing this series
(fans stuck in manual mode after an I2C error, and a
busy-loop/I2C-flooding condition when auto_update_interval or
num_temp_sensors is 0).
Patch 4 performs a minor refactoring to introduce the ADT7470_PWM_MAX
macro.
Patch 5 registers the 4 PWM channels as a generic PWM provider.
Patch 6 registers the external temperature sensors as optional thermal
zones.

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

---
Changes in v2:
- Fixed 2 previous bugs uncovered by Sashiko-bot during review.
- Dropped the thermal cooling device registration for the PWM channels.
- Implemented generic PWM provider support using the PWM subsystem.
- Updated Kconfig to safely handle the PWM subsystem dependency (depends on PWM || PWM=n).
- Updated Device Tree bindings to require #pwm-cells instead of #cooling-cells.
- Link to v1: https://patch.msgid.link/20260716-adt7470_thermalzone-v1-0-cc55ef35edde@gmail.com

To: Guenter Roeck <linux@roeck-us.net>
To: Rob Herring <robh@kernel.org>
To: Krzysztof Kozlowski <krzk+dt@kernel.org>
To: Conor Dooley <conor+dt@kernel.org>
To: de Luca <luizluca@gmail.com>
To: Chris Packham <chris.packham@alliedtelesis.co.nz>
To: Andrew Morton <akpm@linux-foundation.org>
To: "Darrick J. Wong" <djwong@us.ibm.com>
To: Uwe Kleine-König <ukleinek@kernel.org>
Cc: linux-hwmon@vger.kernel.org
Cc: devicetree@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: linux-pwm@vger.kernel.org

---
Luiz Angelo Daros de Luca (6):
      dt-bindings: hwmon: add binding for adi,adt7470
      hwmon: (adt7470) Fix fans stuck in manual mode on I2C errors
      hwmon: (adt7470) Fix busy-loop and I2C flooding in update thread
      hwmon: (adt7470) Add ADT7470_PWM_MAX macro
      hwmon: (adt7470) Register as a PWM provider
      hwmon: (adt7470) Add thermal zone sensor support

 .../devicetree/bindings/hwmon/adi,adt7470.yaml     |  62 +++++++++++
 drivers/hwmon/Kconfig                              |   1 +
 drivers/hwmon/adt7470.c                            | 118 ++++++++++++++++++---
 3 files changed, 166 insertions(+), 15 deletions(-)
---
base-commit: ca078d004cf58137bcf8cb24a8b271397431ba58
change-id: 20260716-adt7470_thermalzone-59a102278d72

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

[PATCH v2 1/6] 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 "#thermal-sensor-cells"
property, and support for the PWM subsystem by defining the "#pwm-cells"
property.

Signed-off-by: Luiz Angelo Daros de Luca <luizluca@gmail.com>
---
 .../devicetree/bindings/hwmon/adi,adt7470.yaml     | 62 ++++++++++++++++++++++
 1 file changed, 62 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..14bb6359c91f
--- /dev/null
+++ b/Documentation/devicetree/bindings/hwmon/adi,adt7470.yaml
@@ -0,0 +1,62 @@
+# 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
+
+  "#pwm-cells":
+    const: 2
+    description: |
+      Number of cells in a PWM specifier.
+      - The first cell is the PWM channel (0 to 3).
+      - The second cell is the PWM period in nanoseconds.
+
+  "#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>;
+
+        adt7470: hwmon@2f {
+            compatible = "adi,adt7470";
+            reg = <0x2f>;
+            #pwm-cells = <2>;
+            #thermal-sensor-cells = <1>;
+        };
+    };
+
+    fan0 {
+        compatible = "pwm-fan";
+        /* Connects to PWM channel 0 with a 50000 ns (20 kHz) period */
+        pwms = <&adt7470 0 50000>;
+        cooling-levels = <0 100 150 255>;
+    };

-- 
2.55.0

[PATCH v2 2/6] hwmon: (adt7470) Fix fans stuck in manual mode on I2C errors

[Luiz Angelo Daros de Luca]

During adt7470_read_temperatures(), the driver temporarily switches
the PWM channels to manual mode, performs the temperature collection,
and then restores the original configuration registers.

However, if an I2C transaction fails at any point after entering manual
mode, the function aborts and returns immediately. This leaves the
configuration registers un-restored, permanently trapping the fans in
manual mode.

Introduce a recovery path to ensure that the original PWM configuration
registers are always restored, even when intermediate I2C operations
fail.

Reported-by: sashiko-bot@kernel.org
Closes: https://lore.kernel.org/r/20260716213252.EACA71F000E9@smtp.kernel.org
Fixes: ef67959c4253 ("hwmon: (adt7470) Convert to use regmap")
Signed-off-by: Luiz Angelo Daros de Luca <luizluca@gmail.com>
---
 drivers/hwmon/adt7470.c | 40 +++++++++++++++++++++++++++++-----------
 1 file changed, 29 insertions(+), 11 deletions(-)

diff --git a/drivers/hwmon/adt7470.c b/drivers/hwmon/adt7470.c
index 664349756dc2..481d51617f4b 100644
--- a/drivers/hwmon/adt7470.c
+++ b/drivers/hwmon/adt7470.c
@@ -205,11 +205,12 @@ static inline int adt7470_write_word_data(struct adt7470_data *data, unsigned in
 /* Probe for temperature sensors.  Assumes lock is held */
 static int adt7470_read_temperatures(struct adt7470_data *data)
 {
-	unsigned long res;
+	struct device *dev = regmap_get_device(data->regmap);
+	u8 pwm[ADT7470_FAN_COUNT];
 	unsigned int pwm_cfg[2];
-	int err;
+	unsigned long res;
+	int err, err2;
 	int i;
-	u8 pwm[ADT7470_FAN_COUNT];
 
 	/* save pwm[1-4] config register */
 	err = regmap_read(data->regmap, ADT7470_REG_PWM_CFG(0), &pwm_cfg[0]);
@@ -233,19 +234,19 @@ static int adt7470_read_temperatures(struct adt7470_data *data)
 	err = regmap_update_bits(data->regmap, ADT7470_REG_PWM_CFG(2),
 				 ADT7470_PWM_AUTO_MASK, 0);
 	if (err < 0)
-		return err;
+		goto out_restore;
 
 	/* write pwm control to whatever it was */
 	err = regmap_bulk_write(data->regmap, ADT7470_REG_PWM(0), &pwm[0],
 				ADT7470_PWM_COUNT);
 	if (err < 0)
-		return err;
+		goto out_restore;
 
 	/* start reading temperature sensors */
 	err = regmap_update_bits(data->regmap, ADT7470_REG_CFG,
 				 ADT7470_T05_STB_MASK, ADT7470_T05_STB_MASK);
 	if (err < 0)
-		return err;
+		goto out_restore;
 
 	/* Delay is 200ms * number of temp sensors. */
 	res = msleep_interruptible((data->num_temp_sensors >= 0 ?
@@ -256,13 +257,30 @@ static int adt7470_read_temperatures(struct adt7470_data *data)
 	err = regmap_update_bits(data->regmap, ADT7470_REG_CFG,
 				 ADT7470_T05_STB_MASK, 0);
 	if (err < 0)
-		return err;
+		goto out_restore;
 
+out_restore:
 	/* restore pwm[1-4] config registers */
-	err = regmap_write(data->regmap, ADT7470_REG_PWM_CFG(0), pwm_cfg[0]);
-	if (err < 0)
-		return err;
-	err = regmap_write(data->regmap, ADT7470_REG_PWM_CFG(2), pwm_cfg[1]);
+	err2 = regmap_write(data->regmap, ADT7470_REG_PWM_CFG(0), pwm_cfg[0]);
+	if (err2 < 0) {
+		dev_warn_ratelimited(dev,
+				     "failed to restore PWM{1,2} config (%d)\n",
+				     err2);
+
+		if (!err)
+			err = err2;
+	}
+
+	err2 = regmap_write(data->regmap, ADT7470_REG_PWM_CFG(2), pwm_cfg[1]);
+	if (err2 < 0) {
+		dev_warn_ratelimited(dev,
+				     "failed to restore PWM{3,4} config (%d)\n",
+				     err2);
+
+		if (!err)
+			err = err2;
+	}
+
 	if (err < 0)
 		return err;
 

-- 
2.55.0

[PATCH v2 3/6] hwmon: (adt7470) Fix busy-loop and I2C flooding in update thread

[Luiz Angelo Daros de Luca]

When userspace configures 'auto_update_interval' to 0 via sysfs, the
background kthread executes schedule_timeout_interruptible(0), which
returns immediately.

If 'num_temp_sensors' is concurrently or previously set to 0, the
msleep_interruptible() delay inside adt7470_read_temperatures() also
becomes 0. This combination forces the background thread into a tight,
unbounded busy-loop, hogging the CPU and flooding the I2C bus with a
continuous stream of transactions.

Fix this vulnerability by raising the lower limit of the clamp_val in
auto_update_interval_store() from 0 to 500 milliseconds. This guarantees
a reasonable minimum sleep window between sensor updates, protecting the
system from intentional or accidental I2C bus denial of service.

Reported-by: sashiko-bot@kernel.org
Closes: https://lore.kernel.org/r/20260716213252.EACA71F000E9@smtp.kernel.org
Fixes: 89fac11cb3e7 ("adt7470: make automatic fan control really work")
Signed-off-by: Luiz Angelo Daros de Luca <luizluca@gmail.com>
---
 drivers/hwmon/adt7470.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/hwmon/adt7470.c b/drivers/hwmon/adt7470.c
index 481d51617f4b..77e7be692790 100644
--- a/drivers/hwmon/adt7470.c
+++ b/drivers/hwmon/adt7470.c
@@ -509,7 +509,7 @@ static ssize_t auto_update_interval_store(struct device *dev,
 	if (kstrtol(buf, 10, &temp))
 		return -EINVAL;
 
-	temp = clamp_val(temp, 0, 60000);
+	temp = clamp_val(temp, 500, 60000);
 
 	mutex_lock(&data->lock);
 	data->auto_update_interval = temp;

-- 
2.55.0

[PATCH v2 4/6] 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 77e7be692790..d881609c99aa 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))
@@ -829,7 +830,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),
@@ -887,7 +888,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;
@@ -922,7 +923,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

[PATCH v2 5/6] hwmon: (adt7470) Register as a PWM provider

[Luiz Angelo Daros de Luca]

hwmon: (adt7470) Register as a PWM provider

The ADT7470 features four PWM outputs that can be used to control fans.
Previously, these were only accessible through the legacy hwmon sysfs
interface.

Register the ADT7470 as a generic PWM provider. This enables standard
Device Tree PWM consumers, such as "pwm-fan", to use the device through
the "#pwm-cells" property.

When a PWM consumer applies a new PWM state, the driver automatically
switches the corresponding PWM channel to manual mode so that the
requested duty cycle takes effect. The duty cycle specified by the PWM
framework is internally converted to the 0-255 scale expected by the
hardware registers.

The legacy sysfs interface remains unaffected and operates in parallel.

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

diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index 08c29685126a..8838b6714e60 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -224,6 +224,7 @@ config SENSORS_ADT7462
 config SENSORS_ADT7470
 	tristate "Analog Devices ADT7470"
 	depends on I2C
+	depends on PWM || PWM=n
 	select REGMAP_I2C
 	help
 	  If you say yes here you get support for the Analog Devices
diff --git a/drivers/hwmon/adt7470.c b/drivers/hwmon/adt7470.c
index d881609c99aa..aaae14a421cd 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/pwm.h>
 
 /* Addresses to scan */
 static const unsigned short normal_i2c[] = { 0x2C, 0x2E, 0x2F, I2C_CLIENT_END };
@@ -864,6 +865,57 @@ static int adt7470_pwm_write(struct device *dev, u32 attr, int channel, long val
 	return err;
 }
 
+static int adt7470_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
+			     const struct pwm_state *state)
+{
+	struct adt7470_data *data = pwmchip_get_drvdata(chip);
+	unsigned int pwm_auto_reg_mask;
+	int err;
+	u8 val;
+
+	if (pwm->hwpwm % 2)
+		pwm_auto_reg_mask = ADT7470_PWM2_AUTO_MASK;
+	else
+		pwm_auto_reg_mask = ADT7470_PWM1_AUTO_MASK;
+
+	if (state->enabled && state->period > 0)
+		val = DIV_ROUND_CLOSEST_ULL(state->duty_cycle * 255, state->period);
+	else
+		val = 0;
+
+	mutex_lock(&data->lock);
+
+	if (data->pwm[pwm->hwpwm] == val &&
+	    data->pwm_automatic[pwm->hwpwm] == 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(pwm->hwpwm),
+				 pwm_auto_reg_mask, 0);
+	if (err < 0)
+		goto out;
+
+	data->pwm_automatic[pwm->hwpwm] = 0;
+
+	err = regmap_write(data->regmap,
+			   ADT7470_REG_PWM(pwm->hwpwm), val);
+	if (err < 0)
+		goto out;
+
+	data->pwm[pwm->hwpwm] = val;
+out:
+	mutex_unlock(&data->lock);
+
+	return err;
+}
+
+static const struct pwm_ops adt7470_pwm_ops = {
+	.apply = adt7470_pwm_apply,
+};
+
 static ssize_t pwm_max_show(struct device *dev,
 			    struct device_attribute *devattr, char *buf)
 {
@@ -1299,6 +1351,21 @@ static int adt7470_probe(struct i2c_client *client)
 	if (IS_ERR(hwmon_dev))
 		return PTR_ERR(hwmon_dev);
 
+	if (IS_REACHABLE(CONFIG_PWM)) {
+		struct pwm_chip *chip;
+
+		chip = devm_pwmchip_alloc(dev, ADT7470_PWM_COUNT, 0);
+		if (IS_ERR(chip))
+			return PTR_ERR(chip);
+
+		chip->ops = &adt7470_pwm_ops;
+		pwmchip_set_drvdata(chip, data);
+
+		err = devm_pwmchip_add(dev, chip);
+		if (err)
+			return dev_warn_probe(dev, err, "failed to register PWM chip\n");
+	}
+
 	data->auto_update = kthread_run(adt7470_update_thread, client, "%s",
 					dev_name(hwmon_dev));
 	if (IS_ERR(data->auto_update))

-- 
2.55.0

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

[Luiz Angelo Daros de Luca]

Expose the ADT7470 external temperature sensors to the thermal
framework using HWMON_C_REGISTER_TZ.

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

diff --git a/drivers/hwmon/adt7470.c b/drivers/hwmon/adt7470.c
index aaae14a421cd..595a8f934d60 100644
--- a/drivers/hwmon/adt7470.c
+++ b/drivers/hwmon/adt7470.c
@@ -1251,6 +1251,8 @@ static const struct hwmon_ops adt7470_hwmon_ops = {
 };
 
 static const struct hwmon_channel_info * const adt7470_info[] = {
+	HWMON_CHANNEL_INFO(chip,
+			   HWMON_C_REGISTER_TZ),
 	HWMON_CHANNEL_INFO(temp,
 			   HWMON_T_INPUT | HWMON_T_MIN | HWMON_T_MAX | HWMON_T_ALARM,
 			   HWMON_T_INPUT | HWMON_T_MIN | HWMON_T_MAX | HWMON_T_ALARM,

-- 
2.55.0

Re: [PATCH v2 1/6] 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 binding defines PWM output and thermal sensor provider capabilities but fails to include `#pwm-cells` and `#thermal-sensor-cells` in the `required` list.
--

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

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

The ADT7470 hardware monitoring chip supports PWM fan control and temperature
monitoring. This commit introduces the YAML device tree binding schema for it,
defining PWM and thermal sensor provider capabilities.

> diff --git a/Documentation/devicetree/bindings/hwmon/adi,adt7470.yaml b/Documentation/devicetree/bindings/hwmon/adi,adt7470.yaml
> new file mode 100644
> index 0000000000000..14bb6359c91f2
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/hwmon/adi,adt7470.yaml
[ ... ]
> +properties:
> +  compatible:
> +    const: adi,adt7470
> +
> +  reg:
> +    maxItems: 1
> +
> +  "#pwm-cells":
> +    const: 2
> +    description: |
> +      Number of cells in a PWM specifier.
> +      - The first cell is the PWM channel (0 to 3).
> +      - The second cell is the PWM period in nanoseconds.
> +
> +  "#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]
Should #pwm-cells and #thermal-sensor-cells be included in this required list?

Because this binding defines the device as a PWM output and thermal sensor
provider, omitting these cell properties from the required list allows
incomplete device tree nodes to silently pass schema validation. 

If an adt7470 node is created without them, dependent drivers attempting to
reference it as a provider may fail at runtime when parsing the provider
specifier.

Could both #pwm-cells and #thermal-sensor-cells be added to the required
array to enforce complete node validation?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260717-adt7470_thermalzone-v2-0-a55147958fad@gmail.com?part=1

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

[sashiko-bot]

> Instead of a magic 255, use a macro to refer to it.
> 
> Signed-off-by: Luiz Angelo Daros de Luca <luizluca@gmail.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260717-adt7470_thermalzone-v2-0-a55147958fad@gmail.com?part=4

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The background update thread holds a private mutex during a multi-second sleep, which will severely stall the thermal subsystem polling.
--

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

hwmon: (adt7470) Add thermal zone sensor support

This commit exposes the ADT7470 external temperature sensors to the thermal
framework using the HWMON_C_REGISTER_TZ flag.

> diff --git a/drivers/hwmon/adt7470.c b/drivers/hwmon/adt7470.c
> index aaae14a421cd8..595a8f934d60a 100644
> --- a/drivers/hwmon/adt7470.c
> +++ b/drivers/hwmon/adt7470.c
> @@ -1251,6 +1251,8 @@ static const struct hwmon_ops adt7470_hwmon_ops = {
>  };
>  
>  static const struct hwmon_channel_info * const adt7470_info[] = {
> +	HWMON_CHANNEL_INFO(chip,
> +			   HWMON_C_REGISTER_TZ),

[Severity: High]
Could exposing this sensor to the thermal framework cause a regression by
stalling the thermal workqueue?

The thermal subsystem will automatically poll this sensor via adt7470_read(),
which acquires data->lock.

Concurrently, the driver's background kthread adt7470_update_thread() holds
this exact lock while executing msleep_interruptible() for up to 2 seconds
in adt7470_read_temperatures():

drivers/hwmon/adt7470.c:adt7470_update_thread() {
    ...
    mutex_lock(&data->lock);
    adt7470_read_temperatures(data);
    mutex_unlock(&data->lock);
    ...
}

drivers/hwmon/adt7470.c:adt7470_read_temperatures() {
    ...
    res = msleep_interruptible((data->num_temp_sensors >= 0 ?
                                data->num_temp_sensors * 200 :
                                TEMP_COLLECTION_TIME));
    ...
}

Will the thermal workqueue block for up to 2 seconds when trying to read the
temperature, risking a stall in thermal management for other zones sharing
the same workqueue?

>  	HWMON_CHANNEL_INFO(temp,
>  			   HWMON_T_INPUT | HWMON_T_MIN | HWMON_T_MAX | HWMON_T_ALARM,

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260717-adt7470_thermalzone-v2-0-a55147958fad@gmail.com?part=6

Re: [PATCH v2 2/6] hwmon: (adt7470) Fix fans stuck in manual mode on I2C errors

[sashiko-bot]

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

Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but the driver violates hwmon subsystem guidelines by using a private mutex `data->lock` for sysfs attributes instead of the standard `hwmon_lock()` API.
--

--- Patch [2]: [PATCH v2 2/6] hwmon: (adt7470) Fix fans stuck in manual mode on I2C errors ---
commit f5bf8b0ee6f2d048580f6d00a71e61df362fa4c6
Author: Luiz Angelo Daros de Luca <luizluca@gmail.com>

hwmon: (adt7470) Fix fans stuck in manual mode on I2C errors

This commit introduces a recovery path in adt7470_read_temperatures() to ensure
that original PWM configuration registers are restored if an I2C operation
fails after temporarily switching the fans to manual mode.

Closes: https://lore.kernel.org/r/20260716213252.EACA71F000E9@smtp.kernel.org

> diff --git a/drivers/hwmon/adt7470.c b/drivers/hwmon/adt7470.c
> index 664349756dc2b..481d51617f4be 100644
> --- a/drivers/hwmon/adt7470.c
> +++ b/drivers/hwmon/adt7470.c
> @@ -205,11 +205,12 @@ static inline int adt7470_write_word_data(struct adt7470_data *data, unsigned in
>  /* Probe for temperature sensors.  Assumes lock is held */
>  static int adt7470_read_temperatures(struct adt7470_data *data)
>  {

[Severity: High]
This isn't a bug introduced by this patch, but this driver appears to use a
private mutex (data->lock) for internal synchronization instead of the
standard hwmon_lock() API.

For example, it uses mutex_init(&data->lock) in adt7470_probe() and locks it
with mutex_lock(&data->lock) in sysfs attribute operations like
pwm_auto_temp_store().

Does this bypass the centralized hwmon core serialization mechanism and risk
logical race conditions with the thermal subsystem? Should this be updated
to follow the hwmon subsystem guidelines which mandate using hwmon_lock()
and hwmon_unlock() for attributes not registered via the info parameter?

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260717-adt7470_thermalzone-v2-0-a55147958fad@gmail.com?part=2

Re: [PATCH v2 3/6] hwmon: (adt7470) Fix busy-loop and I2C flooding in update thread

[sashiko-bot]

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

Pre-existing issues:
- [High] Module unload hang due to missed wakeup in kthread loop
- [High] Temperature alarm bitmask logic error causes false negatives and incorrect states
- [High] Unfreezable polling kthread can crash or hang the I2C bus during system suspend
- [High] Subsystem guideline violation: Driver uses private lock instead of hwmon_lock()
--

--- Patch [3]: [PATCH v2 3/6] hwmon: (adt7470) Fix busy-loop and I2C flooding in update thread ---
commit e6d87eed20a630165f01d0513e17b82247a7f775
Author: Luiz Angelo Daros de Luca <luizluca@gmail.com>

hwmon: (adt7470) Fix busy-loop and I2C flooding in update thread

This commit prevents a busy-loop and I2C flooding in the background kthread
when userspace configures auto_update_interval and num_temp_sensors to 0.
It resolves the issue by raising the lower limit of the clamp_val in
auto_update_interval_store() from 0 to 500 milliseconds.

Closes: https://lore.kernel.org/r/20260716213252.EACA71F000E9@smtp.kernel.org

> diff --git a/drivers/hwmon/adt7470.c b/drivers/hwmon/adt7470.c
> index 481d51617f4be..77e7be6927906 100644
> --- a/drivers/hwmon/adt7470.c
> +++ b/drivers/hwmon/adt7470.c
> @@ -509,7 +509,7 @@ static ssize_t auto_update_interval_store(struct device *dev,
>  	if (kstrtol(buf, 10, &temp))
>  		return -EINVAL;
>  
> -	temp = clamp_val(temp, 0, 60000);
> +	temp = clamp_val(temp, 500, 60000);
>  
>  	mutex_lock(&data->lock);

[Severity: High]
This isn't a bug introduced by this patch, but does the use of a private
mutex for an extra sysfs attribute violate the HWMON subsystem directive
to use the central hwmon_lock() and hwmon_unlock() for this purpose?

>  	data->auto_update_interval = temp;

I would also like to ask about some other pre-existing issues in this file:

drivers/hwmon/adt7470.c:adt7470_update_thread() {
    [ ... ]
	while (!kthread_should_stop()) {
		mutex_lock(&data->lock);
		adt7470_read_temperatures(data);
		mutex_unlock(&data->lock);

		if (kthread_should_stop())
			break;

		schedule_timeout_interruptible(msecs_to_jiffies(data->auto_update_interval));
	}
    [ ... ]
}

[Severity: High]
This is a pre-existing issue, but does checking the stop condition while
the thread is in TASK_RUNNING without preparing to sleep cause missed
wakeups?

If the unbind process calls kthread_stop(), it sets the stop flag and
wakes the thread. Because the thread is still in TASK_RUNNING, the wake
is a no-op, and the thread will then call schedule_timeout_interruptible()
and go to sleep, hanging module unload for up to 60 seconds.

[Severity: High]
This is also a pre-existing issue, but does this kthread lack
set_freezable() and try_to_freeze()? It appears to unconditionally attempt
I2C access while the bus might be suspended. Can this crash or hang the
I2C bus during system suspend and resume transitions?

drivers/hwmon/adt7470.c:adt7470_temp_read() {
    [ ... ]
	case hwmon_temp_alarm:
		*val = !!(data->alarm & channel);
		break;
    [ ... ]
}

[Severity: High]
This is a pre-existing issue, but does this logic incorrectly use the
integer index channel as a bitwise AND mask instead of a bit shift?
Could this yield completely incorrect hardware alarm states, preventing
userspace from properly monitoring temperature emergencies?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260717-adt7470_thermalzone-v2-0-a55147958fad@gmail.com?part=3

Re: [PATCH v2 5/6] hwmon: (adt7470) Register as a PWM provider

[sashiko-bot]

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

New issues:
- [High] Calculating the PWM duty cycle using `DIV_ROUND_CLOSEST_ULL` with a 64-bit divisor causes 32-bit truncation and a divide-by-zero kernel panic on 32-bit architectures.
- [High] Early return in PWM apply relies on unsynchronized, zero-initialized cache values, potentially ignoring the first 0% duty cycle request.
- [Medium] Registering a generic PWM provider directly from a hardware monitoring driver violates subsystem layering guidelines.
- [Medium] The driver silently ignores the requested PWM signal polarity, applying normal polarity even when inverted is requested.

Pre-existing issues:
- [High] The PWM automatic mode mask calculation swaps the masks for PWM channels 3 and 4, causing writes to one channel to affect the configuration of the other.
--

--- Patch [5]: [PATCH v2 5/6] hwmon: (adt7470) Register as a PWM provider ---
commit e11bee28efc81066c4940f5c07591cd6181c6453
Author: Luiz Angelo Daros de Luca <luizluca@gmail.com>

hwmon: (adt7470) Register as a PWM provider

The patch registers the ADT7470 hardware monitoring driver as a generic PWM
provider, allowing standard Device Tree consumers like "pwm-fan" to use it.
It converts the PWM framework's requested duty cycle to the 0-255 scale
used by the hardware registers. The legacy sysfs interface operates in
parallel and remains unaffected.

> diff --git a/drivers/hwmon/adt7470.c b/drivers/hwmon/adt7470.c
> index d881609c99aa3..aaae14a421cd8 100644
> --- a/drivers/hwmon/adt7470.c
> +++ b/drivers/hwmon/adt7470.c

[ ... ]

> @@ -864,6 +865,57 @@ static int adt7470_pwm_write(struct device *dev, u32 attr, int channel, long val
>  	return err;
>  }
>  
> +static int adt7470_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
> +			     const struct pwm_state *state)
> +{
> +	struct adt7470_data *data = pwmchip_get_drvdata(chip);
> +	unsigned int pwm_auto_reg_mask;
> +	int err;
> +	u8 val;
> +
> +	if (pwm->hwpwm % 2)
> +		pwm_auto_reg_mask = ADT7470_PWM2_AUTO_MASK;
> +	else
> +		pwm_auto_reg_mask = ADT7470_PWM1_AUTO_MASK;

[Severity: High]
This is a pre-existing issue, but does this logic incorrectly swap the masks
for PWM channels 3 and 4?

The logic uses hwpwm % 2 to select between ADT7470_PWM1_AUTO_MASK and
ADT7470_PWM2_AUTO_MASK. For channel 3 (hwpwm = 2) and channel 4 (hwpwm = 3),
does this inadvertently select 0x80 for PWM3 and 0x40 for PWM4?

If the datasheet and macro definitions dictate 0x40 for PWM3 and 0x80 for
PWM4, could writing to one of these channels corrupt the configuration
of the other?

This issue seems to be copied from the preexisting adt7470_pwm_write() sysfs
handler.

> +
> +	if (state->enabled && state->period > 0)

[Severity: Medium]
Does this apply callback silently ignore requested PWM polarities?

If a consumer specifies PWM_POLARITY_INVERSED in the PWM state, the hardware
only supports normal polarity.

Since there is no check to see if state->polarity != PWM_POLARITY_NORMAL,
will this cause the driver to quietly apply normal polarity instead of
rejecting the unsupported request?

> +		val = DIV_ROUND_CLOSEST_ULL(state->duty_cycle * 255, state->period);

[Severity: High]
Does this calculation cause a divide-by-zero on 32-bit architectures?

When a consumer requests a PWM period that is a multiple of 2^32 ns,
do_div() implicitly casts the divisor to a 32-bit integer on 32-bit systems,
meaning a value like 1ULL << 32 becomes 0.

Since the truncation happens during the division after the state->period > 0
check, can this lead to a kernel panic?

> +	else
> +		val = 0;
> +
> +	mutex_lock(&data->lock);
> +
> +	if (data->pwm[pwm->hwpwm] == val &&
> +	    data->pwm_automatic[pwm->hwpwm] == 0) {
> +		mutex_unlock(&data->lock);
> +		return 0;
> +	}

[Severity: High]
Can this early return falsely ignore the first 0% duty cycle request?

Because the driver data is allocated with devm_kzalloc(), data->pwm is
zero-initialized.

If a consumer requests a 0% duty cycle (val = 0) before the
adt7470_update_thread has completed its first hardware state synchronization,
will this condition pass and return early without sending the 0% duty cycle
to the hardware?

[ ... ]

> @@ -1299,6 +1351,21 @@ static int adt7470_probe(struct i2c_client *client)
>  	if (IS_ERR(hwmon_dev))
>  		return PTR_ERR(hwmon_dev);
>  
> +	if (IS_REACHABLE(CONFIG_PWM)) {
> +		struct pwm_chip *chip;
> +
> +		chip = devm_pwmchip_alloc(dev, ADT7470_PWM_COUNT, 0);
> +		if (IS_ERR(chip))
> +			return PTR_ERR(chip);
> +
> +		chip->ops = &adt7470_pwm_ops;
> +		pwmchip_set_drvdata(chip, data);
> +
> +		err = devm_pwmchip_add(dev, chip);
> +		if (err)
> +			return dev_warn_probe(dev, err, "failed to register PWM chip\n");
> +	}

[Severity: Medium]
Does this violate the hardware monitoring subsystem guidelines?

The guidelines indicate that secondary functionalities, such as PWM or GPIO,
should be implemented as auxiliary devices rather than registering directly
from drivers/hwmon/.

By using devm_pwmchip_alloc() and devm_pwmchip_add() directly in the hwmon
probe function, does this bypass the separation into an auxiliary device and
cross-subsystem review?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260717-adt7470_thermalzone-v2-0-a55147958fad@gmail.com?part=5