[hwmon-next PATCH v4 0/4] hwmon: (adt7470) Add PWM provider and thermal sensor support

[hwmon-next PATCH v4 0/4] hwmon: (adt7470) Add PWM provider and thermal sensor support

[Luiz Angelo Daros de Luca]

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

Currently, the driver exposes these sensors and PWM controls only
through the hwmon sysfs interface. This patch series integrates the
ADT7470 driver with the kernel's thermal and PWM subsystems. This allows
temperature sensors to participate in Device Tree thermal zones while
exposing the PWM outputs as generic PWM providers for consumers such as
pwm-fan.

Patch 1 adds the YAML device tree binding documentation.
Patch 2 introduces the ADT7470_PWM_MAX macro.
Patch 3 registers the four PWM channels as a generic PWM provider.
Patch 4 registers the external temperature sensors with the thermal
framework.

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

---
Changes in v4:
- dt-bindings: Dropped redundant descriptions for standard properties
  (#pwm-cells and #thermal-sensor-cells).
- dt-bindings: Conditionally referenced thermal-sensor.yaml to preserve
  ABI compatibility.
- dt-bindings: Switched to unevaluatedProperties due to the imported $ref.
- Link to v3: https://patch.msgid.link/20260728-adt7470_thermalzone-v3-0-d4586478cdba@gmail.com

Changes in v3:
- Migrated from the `.apply` callback to the PWM Waveform API.
- Moved fixes to another patch series:
  20260727-adt7470_thermalzone-v3-0-21424d550c3d@gmail.com
- Use guard(mutex) for simpler locking.
- Link to v2: https://patch.msgid.link/20260717-adt7470_thermalzone-v2-0-a55147958fad@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: Luiz Angelo Daros 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 (4):
      dt-bindings: hwmon: add adi,adt7470
      hwmon: (adt7470) Add ADT7470_PWM_MAX macro
      hwmon: (adt7470) Expose fan control via PWM framework
      hwmon: (adt7470) Add thermal zone sensor support

 .../devicetree/bindings/hwmon/adi,adt7470.yaml     |  58 ++++++++
 drivers/hwmon/Kconfig                              |   1 +
 drivers/hwmon/adt7470.c                            | 146 ++++++++++++++++++++-
 3 files changed, 202 insertions(+), 3 deletions(-)
---
base-commit: 4c2e8c377915496fbe9cc1c9d91a7297aaa01783
change-id: 20260716-adt7470_thermalzone-59a102278d72

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

[hwmon-next PATCH v4 1/4] dt-bindings: hwmon: add adi,adt7470

[Luiz Angelo Daros de Luca]

The Analog Devices ADT7470 is a multichannel temperature sensor and
PWM fan controller. It supports monitoring up to 10 external
temperature sensors and controlling up to 4 fans.

Add the device tree binding documentation for it. This includes
support for the thermal framework by defining the "#thermal-sensor-cells"
property, and models the fan control lines as PWM channels by
defining the "#pwm-cells" property.

Signed-off-by: Luiz Angelo Daros de Luca <luizluca@gmail.com>
---
 .../devicetree/bindings/hwmon/adi,adt7470.yaml     | 58 ++++++++++++++++++++++
 1 file changed, 58 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..06881e521111
--- /dev/null
+++ b/Documentation/devicetree/bindings/hwmon/adi,adt7470.yaml
@@ -0,0 +1,58 @@
+# 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:
+  Multi-channel temperature monitor and PWM fan controller.
+  It supports monitoring up to 10 external temperature sensors and
+  controlling up to four fans. The fan control lines are modeled
+  as standard PWM channels.
+
+allOf:
+  - if:
+      required:
+        - "#thermal-sensor-cells"
+    then:
+      $ref: /schemas/thermal/thermal-sensor.yaml#
+
+properties:
+  compatible:
+    const: adi,adt7470
+
+  reg:
+    maxItems: 1
+
+  "#pwm-cells":
+    const: 3
+
+  "#thermal-sensor-cells":
+    const: 1
+    description:
+      Valid index values are 0 to 9, corresponding to temp1 through temp10.
+
+required:
+  - compatible
+  - reg
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        hwmon@2f {
+            compatible = "adi,adt7470";
+            reg = <0x2f>;
+            #pwm-cells = <3>;
+            #thermal-sensor-cells = <1>;
+        };
+    };

-- 
2.55.0

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The binding defines PWM capabilities for the fan control lines but fails to include `#pwm-cells` in the `required` array.
--

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

dt-bindings: hwmon: add adi,adt7470

This commit adds the device tree binding documentation for the Analog Devices
ADT7470 multichannel temperature sensor and PWM fan controller. It includes
support for the thermal framework and models fan control lines as PWM channels.

> 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
[ ... ]
> +  "#pwm-cells":
> +    const: 3
> +
> +  "#thermal-sensor-cells":
> +    const: 1
> +    description:
> +      Valid index values are 0 to 9, corresponding to temp1 through temp10.
> +
> +required:
> +  - compatible
> +  - reg

[Severity: Medium]
Since the binding defines PWM capabilities for the fan control lines, should
#pwm-cells be included in the required properties list? Without it, a device
tree node omitting the property will still pass schema validation, which
could lead to runtime failures in drivers or dependent subsystems attempting
to use the PWM functionality.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260729-adt7470_thermalzone-v4-0-c14039dfa0e6@gmail.com?part=1

[hwmon-next PATCH v4 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 c45b984c02e6..87ddd7b8ddae 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))
@@ -850,7 +851,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);
 		err = regmap_write(data->regmap, ADT7470_REG_PWM(channel),
 				   val);
@@ -910,7 +911,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;
@@ -945,7 +946,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: [hwmon-next PATCH v4 2/4] 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/20260729-adt7470_thermalzone-v4-0-c14039dfa0e6@gmail.com?part=2

[hwmon-next PATCH v4 3/4] hwmon: (adt7470) Expose fan control via PWM framework

[Luiz Angelo Daros de Luca]

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

Expose the ADT7470 fan control lines through the generic PWM framework.
This allows generic PWM consumers described in Device Tree, such as
"pwm-fan", to use the device through the "#pwm-cells" property.  This is
particularly necessary for boards where the ADT7470 external temperature
sensors are not populated and the chip is used strictly as a fan
controller (e.g., Edgecore ECS-2100-52T switches). In such setups,
delegating control to the generic PWM subsystem allows the kernel's
thermal framework to manage the fans based on unrelated temperature
zones, such as internal SoC sensors.

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 | 137 ++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 138 insertions(+)

diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index 6b71e3304b4f..0d6bd117d14d 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 87ddd7b8ddae..883838fdcd7d 100644
--- a/drivers/hwmon/adt7470.c
+++ b/drivers/hwmon/adt7470.c
@@ -22,6 +22,9 @@
 #include <linux/sched.h>
 #include <linux/slab.h>
 #include <linux/util_macros.h>
+#include <linux/pwm.h>
+#include <linux/cleanup.h>
+#include <linux/math64.h>
 
 /* Addresses to scan */
 static const unsigned short normal_i2c[] = { 0x2C, 0x2E, 0x2F, I2C_CLIENT_END };
@@ -887,6 +890,125 @@ static int adt7470_pwm_write(struct device *dev, u32 attr, int channel, long val
 	return err;
 }
 
+struct adt7470_pwm_wfhw {
+	u8 val;
+};
+
+static int adt7470_pwm_round_waveform_tohw(struct pwm_chip *chip,
+					   struct pwm_device *pwm,
+					   const struct pwm_waveform *wf,
+					   void *_wfhw)
+{
+	struct adt7470_data *data = pwmchip_get_drvdata(chip);
+	struct adt7470_pwm_wfhw *wfhw = _wfhw;
+	u64 period_ns;
+
+	if (wf->duty_length_ns == 0) {
+		wfhw->val = 0;
+		return 0;
+	}
+
+	/*
+	 * The PWM frequency (period) is a single chip-wide setting shared by
+	 * all 4 channels, so it cannot be changed on a per-pwm_device basis
+	 * through this API. The duty cycle is rounded against the currently
+	 * configured hardware period rather than the period requested in
+	 * @wf; round_waveform_fromhw() reports the actual resulting
+	 * waveform back so the core/consumer can detect a mismatch.
+	 */
+	period_ns = DIV_ROUND_CLOSEST(NSEC_PER_SEC, data->pwm_freq);
+	wfhw->val = min_t(u64,
+			  mul_u64_u64_div_u64(wf->duty_length_ns, ADT7470_PWM_MAX, period_ns),
+			  ADT7470_PWM_MAX);
+	return 0;
+}
+
+static int adt7470_pwm_round_waveform_fromhw(struct pwm_chip *chip,
+					     struct pwm_device *pwm,
+					     const void *_wfhw,
+					     struct pwm_waveform *wf)
+{
+	struct adt7470_data *data = pwmchip_get_drvdata(chip);
+	const struct adt7470_pwm_wfhw *wfhw = _wfhw;
+
+	wf->period_length_ns = DIV_ROUND_CLOSEST(NSEC_PER_SEC, data->pwm_freq);
+	wf->duty_offset_ns = 0;
+	wf->duty_length_ns = mul_u64_u64_div_u64(wfhw->val,
+						 wf->period_length_ns,
+						 ADT7470_PWM_MAX);
+
+	return 0;
+}
+
+static int adt7470_pwm_read_waveform(struct pwm_chip *chip,
+				     struct pwm_device *pwm,
+				     void *_wfhw)
+{
+	struct adt7470_data *data = pwmchip_get_drvdata(chip);
+	struct device *dev = regmap_get_device(data->regmap);
+	struct adt7470_pwm_wfhw *wfhw = _wfhw;
+
+	data = adt7470_update_device(dev);
+	if (IS_ERR(data))
+		return PTR_ERR(data);
+
+	/*
+	 * No lock needed: like the other hwmon_ops read callbacks in this
+	 * driver (e.g. adt7470_pwm_read()), this only does a single byte
+	 * read from the cache populated by adt7470_update_device().
+	 */
+	wfhw->val = data->pwm[pwm->hwpwm];
+
+	return 0;
+}
+
+static int adt7470_pwm_write_waveform(struct pwm_chip *chip,
+				      struct pwm_device *pwm,
+				      const void *_wfhw)
+{
+	struct adt7470_data *data = pwmchip_get_drvdata(chip);
+	const struct adt7470_pwm_wfhw *wfhw = _wfhw;
+	unsigned int pwm_auto_reg_mask;
+	int err;
+
+	if (pwm->hwpwm % 2)
+		pwm_auto_reg_mask = ADT7470_PWM2_AUTO_MASK;
+	else
+		pwm_auto_reg_mask = ADT7470_PWM1_AUTO_MASK;
+
+	guard(mutex)(&data->lock);
+
+	if (data->pwm[pwm->hwpwm] == wfhw->val &&
+	    data->pwm_automatic[pwm->hwpwm] == 0)
+		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)
+		return err;
+
+	data->pwm_automatic[pwm->hwpwm] = 0;
+
+	err = regmap_write(data->regmap,
+			   ADT7470_REG_PWM(pwm->hwpwm), wfhw->val);
+	if (err < 0)
+		return err;
+
+	data->pwm[pwm->hwpwm] = wfhw->val;
+
+	return 0;
+}
+
+static const struct pwm_ops adt7470_pwm_ops = {
+	.sizeof_wfhw = sizeof(struct adt7470_pwm_wfhw),
+	.round_waveform_tohw = adt7470_pwm_round_waveform_tohw,
+	.round_waveform_fromhw = adt7470_pwm_round_waveform_fromhw,
+	.read_waveform = adt7470_pwm_read_waveform,
+	.write_waveform = adt7470_pwm_write_waveform,
+};
+
 static ssize_t pwm_max_show(struct device *dev,
 			    struct device_attribute *devattr, char *buf)
 {
@@ -1336,6 +1458,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_err_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

Re: [hwmon-next PATCH v4 3/4] hwmon: (adt7470) Expose fan control via PWM framework

[sashiko-bot]

> The ADT7470 features four PWM outputs designed to control fans.
> Previously, these were only accessible through the legacy hwmon
> sysfs interface.
> 
> Expose the ADT7470 fan control lines through the generic PWM framework.
> This allows generic PWM consumers described in Device Tree, such as
> "pwm-fan", to use the device through the "#pwm-cells" property.  This is
> particularly necessary for boards where the ADT7470 external temperature
> sensors are not populated and the chip is used strictly as a fan
> controller (e.g., Edgecore ECS-2100-52T switches). In such setups,
> delegating control to the generic PWM subsystem allows the kernel's
> thermal framework to manage the fans based on unrelated temperature
> zones, such as internal SoC sensors.
> 
> 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>

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

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260729-adt7470_thermalzone-v4-0-c14039dfa0e6@gmail.com?part=3

Re: [hwmon-next PATCH v4 3/4] hwmon: (adt7470) Expose fan control via PWM framework

[Guenter Roeck]

On 7/28/26 21:46, Luiz Angelo Daros de Luca wrote:
> The ADT7470 features four PWM outputs designed to control fans.
> Previously, these were only accessible through the legacy hwmon
> sysfs interface.
> 
> Expose the ADT7470 fan control lines through the generic PWM framework.
> This allows generic PWM consumers described in Device Tree, such as
> "pwm-fan", to use the device through the "#pwm-cells" property.  This is
> particularly necessary for boards where the ADT7470 external temperature
> sensors are not populated and the chip is used strictly as a fan
> controller (e.g., Edgecore ECS-2100-52T switches). In such setups,
> delegating control to the generic PWM subsystem allows the kernel's
> thermal framework to manage the fans based on unrelated temperature
> zones, such as internal SoC sensors.
> 
> 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.
> 

No, I won't accept this. It has to be either or. A pwm channel modeled as pwm
subsystem channel must not be visible via hwmon attributes.

Guenter

Re: [hwmon-next PATCH v4 3/4] hwmon: (adt7470) Expose fan control via PWM framework

[Uwe Kleine-König]

[-- Attachment #1: Type: text/plain, Size: 1748 bytes --]

On Tue, Jul 28, 2026 at 10:56:04PM -0700, Guenter Roeck wrote:
> On 7/28/26 21:46, Luiz Angelo Daros de Luca wrote:
> > The ADT7470 features four PWM outputs designed to control fans.
> > Previously, these were only accessible through the legacy hwmon
> > sysfs interface.
> > 
> > Expose the ADT7470 fan control lines through the generic PWM framework.
> > This allows generic PWM consumers described in Device Tree, such as
> > "pwm-fan", to use the device through the "#pwm-cells" property.  This is
> > particularly necessary for boards where the ADT7470 external temperature
> > sensors are not populated and the chip is used strictly as a fan
> > controller (e.g., Edgecore ECS-2100-52T switches). In such setups,
> > delegating control to the generic PWM subsystem allows the kernel's
> > thermal framework to manage the fans based on unrelated temperature
> > zones, such as internal SoC sensors.
> > 
> > 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.
> 
> No, I won't accept this. It has to be either or. A pwm channel modeled as pwm
> subsystem channel must not be visible via hwmon attributes.

I didn't look in detail, but maybe my red line is a bit more lax. IMHO
it's not OK to be able to modify a setting that results from pushing
hwmon knobs using PWM operations or vice-versa. Switching between
exclusive use at runtime is IMHO ok.

Best regards
Uwe

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [hwmon-next PATCH v4 3/4] hwmon: (adt7470) Expose fan control via PWM framework

[Guenter Roeck]

On 7/29/26 04:49, Uwe Kleine-König wrote:
> On Tue, Jul 28, 2026 at 10:56:04PM -0700, Guenter Roeck wrote:
>> On 7/28/26 21:46, Luiz Angelo Daros de Luca wrote:
>>> The ADT7470 features four PWM outputs designed to control fans.
>>> Previously, these were only accessible through the legacy hwmon
>>> sysfs interface.
>>>
>>> Expose the ADT7470 fan control lines through the generic PWM framework.
>>> This allows generic PWM consumers described in Device Tree, such as
>>> "pwm-fan", to use the device through the "#pwm-cells" property.  This is
>>> particularly necessary for boards where the ADT7470 external temperature
>>> sensors are not populated and the chip is used strictly as a fan
>>> controller (e.g., Edgecore ECS-2100-52T switches). In such setups,
>>> delegating control to the generic PWM subsystem allows the kernel's
>>> thermal framework to manage the fans based on unrelated temperature
>>> zones, such as internal SoC sensors.
>>>
>>> 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.
>>
>> No, I won't accept this. It has to be either or. A pwm channel modeled as pwm
>> subsystem channel must not be visible via hwmon attributes.
> 
> I didn't look in detail, but maybe my red line is a bit more lax. IMHO
> it's not OK to be able to modify a setting that results from pushing
> hwmon knobs using PWM operations or vice-versa. Switching between
> exclusive use at runtime is IMHO ok.
> 
It is my call to make, and my call is either-or to avoid confusion.

Thanks,
Guenter

Re: [hwmon-next PATCH v4 3/4] hwmon: (adt7470) Expose fan control via PWM framework

[Luiz Angelo Daros de Luca]

Hi Guenter, Uwe,

Thank you both for the detailed reviews. I hope I'll address all the
points discussed in the next version.

> >> No, I won't accept this. It has to be either or. A pwm channel modeled as pwm
> >> subsystem channel must not be visible via hwmon attributes.
> >
> > I didn't look in detail, but maybe my red line is a bit more lax. IMHO
> > it's not OK to be able to modify a setting that results from pushing
> > hwmon knobs using PWM operations or vice-versa. Switching between
> > exclusive use at runtime is IMHO ok.
> >
> It is my call to make, and my call is either-or to avoid confusion.

Guenter, I agree and followed your strict "either-or" directive. If
the device is bound via the PWM framework (i.e., #pwm-cells is present
in the DT), the driver completely hides all legacy hwmon PWM
attributes.

One implementation detail: currently, the driver hides the hwmon
attributes only if #pwm-cells is present and IS_REACHABLE(CONFIG_PWM)
is true. If the DTS has #pwm-cells but the PWM framework is disabled
in the kernel, it emits a dev_warn and falls back to exposing the
hwmon attributes. Please let me know if you would prefer to
unconditionally hide the hwmon attributes merely based on the presence
of the DT property, regardless of the CONFIG_PWM state.

> > +static int adt7470_pwm_round_waveform_tohw(struct pwm_chip *chip,
> > +                                        struct pwm_device *pwm,
> > +                                        const struct pwm_waveform *wf,
> > +                                        void *_wfhw)
> > +{
> > +     struct adt7470_data *data = pwmchip_get_drvdata(chip);
> > +     struct adt7470_pwm_wfhw *wfhw = _wfhw;
> > +     u64 period_ns;
> > +
> > +     if (wf->duty_length_ns == 0) {
> > +             wfhw->val = 0;
> > +             return 0;
> > +     }
> > +
> > +     /*
> > +      * The PWM frequency (period) is a single chip-wide setting shared by
> > +      * all 4 channels, so it cannot be changed on a per-pwm_device basis
> > +      * through this API. The duty cycle is rounded against the currently
> > +      * configured hardware period rather than the period requested in
> > +      * @wf; round_waveform_fromhw() reports the actual resulting
> > +      * waveform back so the core/consumer can detect a mismatch.
> > +      */
> > +     period_ns = DIV_ROUND_CLOSEST(NSEC_PER_SEC, data->pwm_freq);
> > +     wfhw->val = min_t(u64,
> > +                       mul_u64_u64_div_u64(wf->duty_length_ns, ADT7470_PWM_MAX, period_ns),
> > +                       ADT7470_PWM_MAX);
>
> This is wrong. Please test your driver with PWM_DEBUG enabled, this
> should emit warnings.
>
> Here the right thing is (untested):
>
>         if (!wf->period_length_ns) {
>                 wfhw->val = 0;
>                 return 0;
>         }
>
>         actual_period = NSEC_PER_SEC / data->pwm_freq;
>
>         if (wf->duty_length_ns >= actual_period)
>                 val = ADT7470_PWM_MAX;
>         else
>                 val = mul_u64_u64_div_u64(wf->duty_length_ns, ADT7470_PWM_MAX, actual_period);
>
>         return (actual_period < wf->period_length_ns) ? 0 : 1;

Uwe, thank you for the waveform callbacks snippet. It was much
cleaner. I only had to make one small mathematical adjustment to the
actual_period calculation in tohw() to satisfy CONFIG_PWM_DEBUG.

When using a truncating division (NSEC_PER_SEC / data->pwm_freq), it
triggered a "Wrong rounding" error during testing:

pwm pwmchip0: Wrong rounding: requested 714285/714286 [+0], result
714286/714286 [+0]

For example, at 1400 Hz, the truncating division gives an
actual_period of 714285 ns. If the framework requests a duty cycle of
714285 ns and a period of 714286 ns, the evaluation (714285 * 255) /
714285 yields a register value of 255. When read back in fromhw()
(which correctly rounds the period up to 714286 ns), the resulting
duty cycle becomes 714286 ns. Since 714286 > 714285, CONFIG_PWM_DEBUG
rightfully complains about the duty cycle inflation.

To ensure the calculated register value is strictly rounded down, I
changed the actual_period denominator in tohw() to use
DIV_ROUND_UP_ULL(NSEC_PER_SEC, data->pwm_freq). Maximizing the
denominator guarantees the resulting duty cycle fraction never
inflates, effectively passing all strict debug checks.

Thanks again for the time and guidance.

Best regards,

Luiz

Re: [hwmon-next PATCH v4 3/4] hwmon: (adt7470) Expose fan control via PWM framework

[Uwe Kleine-König]

[-- Attachment #1: Type: text/plain, Size: 4809 bytes --]

Hello,

On Wed, Jul 29, 2026 at 01:46:11AM -0300, Luiz Angelo Daros de Luca wrote:
> The ADT7470 features four PWM outputs designed to control fans.
> Previously, these were only accessible through the legacy hwmon
> sysfs interface.
> 
> Expose the ADT7470 fan control lines through the generic PWM framework.
> This allows generic PWM consumers described in Device Tree, such as
> "pwm-fan", to use the device through the "#pwm-cells" property.  This is
> particularly necessary for boards where the ADT7470 external temperature
> sensors are not populated and the chip is used strictly as a fan
> controller (e.g., Edgecore ECS-2100-52T switches). In such setups,
> delegating control to the generic PWM subsystem allows the kernel's
> thermal framework to manage the fans based on unrelated temperature
> zones, such as internal SoC sensors.
> 
> 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 | 137 ++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 138 insertions(+)
> 
> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> index 6b71e3304b4f..0d6bd117d14d 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 87ddd7b8ddae..883838fdcd7d 100644
> --- a/drivers/hwmon/adt7470.c
> +++ b/drivers/hwmon/adt7470.c
> @@ -22,6 +22,9 @@
>  #include <linux/sched.h>
>  #include <linux/slab.h>
>  #include <linux/util_macros.h>
> +#include <linux/pwm.h>
> +#include <linux/cleanup.h>
> +#include <linux/math64.h>
>  
>  /* Addresses to scan */
>  static const unsigned short normal_i2c[] = { 0x2C, 0x2E, 0x2F, I2C_CLIENT_END };
> @@ -887,6 +890,125 @@ static int adt7470_pwm_write(struct device *dev, u32 attr, int channel, long val
>  	return err;
>  }
>  
> +struct adt7470_pwm_wfhw {
> +	u8 val;
> +};
> +
> +static int adt7470_pwm_round_waveform_tohw(struct pwm_chip *chip,
> +					   struct pwm_device *pwm,
> +					   const struct pwm_waveform *wf,
> +					   void *_wfhw)
> +{
> +	struct adt7470_data *data = pwmchip_get_drvdata(chip);
> +	struct adt7470_pwm_wfhw *wfhw = _wfhw;
> +	u64 period_ns;
> +
> +	if (wf->duty_length_ns == 0) {
> +		wfhw->val = 0;
> +		return 0;
> +	}
> +
> +	/*
> +	 * The PWM frequency (period) is a single chip-wide setting shared by
> +	 * all 4 channels, so it cannot be changed on a per-pwm_device basis
> +	 * through this API. The duty cycle is rounded against the currently
> +	 * configured hardware period rather than the period requested in
> +	 * @wf; round_waveform_fromhw() reports the actual resulting
> +	 * waveform back so the core/consumer can detect a mismatch.
> +	 */
> +	period_ns = DIV_ROUND_CLOSEST(NSEC_PER_SEC, data->pwm_freq);
> +	wfhw->val = min_t(u64,
> +			  mul_u64_u64_div_u64(wf->duty_length_ns, ADT7470_PWM_MAX, period_ns),
> +			  ADT7470_PWM_MAX);

This is wrong. Please test your driver with PWM_DEBUG enabled, this
should emit warnings.

Here the right thing is (untested):

	if (!wf->period_length_ns) {
		wfhw->val = 0;
		return 0;
	}

	actual_period = NSEC_PER_SEC / data->pwm_freq;

	if (wf->duty_length_ns >= actual_period)
		val = ADT7470_PWM_MAX;
	else
		val = mul_u64_u64_div_u64(wf->duty_length_ns, ADT7470_PWM_MAX, actual_period);

	return (actual_period < wf->period_length_ns) ? 0 : 1;


> +	return 0;
> +}
> +
> +static int adt7470_pwm_round_waveform_fromhw(struct pwm_chip *chip,
> +					     struct pwm_device *pwm,
> +					     const void *_wfhw,
> +					     struct pwm_waveform *wf)
> +{
> +	struct adt7470_data *data = pwmchip_get_drvdata(chip);
> +	const struct adt7470_pwm_wfhw *wfhw = _wfhw;
> +
> +	wf->period_length_ns = DIV_ROUND_CLOSEST(NSEC_PER_SEC, data->pwm_freq);

Round up here please to match the rounding down in the tohw callback.


> +	wf->duty_offset_ns = 0;
> +	wf->duty_length_ns = mul_u64_u64_div_u64(wfhw->val,
> +						 wf->period_length_ns,
> +						 ADT7470_PWM_MAX);

Also here.

> +
> +	return 0;
> +}

Best regards
Uwe

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

[Luiz Angelo Daros de Luca]

Register the ADT7470 temperature channels as thermal zone sensors.

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 883838fdcd7d..6cc246aa1285 100644
--- a/drivers/hwmon/adt7470.c
+++ b/drivers/hwmon/adt7470.c
@@ -1349,6 +1349,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: [hwmon-next PATCH v4 4/4] hwmon: (adt7470) Add thermal zone sensor support

[sashiko-bot]

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

New issues:
- [Medium] The driver implements PWM functionality directly rather than using an auxiliary device.

Pre-existing issues:
- [High] The driver holds a mutex for up to 2 seconds while sleeping during temperature reads, which will severely block thermal core polling.
- [Medium] The driver implements its own lock (`data->lock`) for extra sysfs attributes instead of using the required `hwmon_lock()` API.
--

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

hwmon: (adt7470) Add thermal zone sensor support

This patch registers the ADT7470 temperature channels as thermal zone sensors.

> diff --git a/drivers/hwmon/adt7470.c b/drivers/hwmon/adt7470.c
> index 883838fdcd7db..6cc246aa12851 100644
> --- a/drivers/hwmon/adt7470.c
> +++ b/drivers/hwmon/adt7470.c
> @@ -1349,6 +1349,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]
This is a pre-existing issue, but does registering the thermal zone cause the
thermal core to severely block on reading temperatures?

The adt7470_update_thread repeatedly acquires data->lock and calls
adt7470_read_temperatures(), which sleeps for up to 2000 ms:

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

Concurrently, the kernel thermal subsystem automatically polls the temperature
because this patch registers the sensors as a thermal zone.

The thermal core calls adt7470_update_device(), which blocks on data->lock.
Blocking the thermal core's system-wide freezable workqueue for up to 2
seconds per read causes significant delays for other thermal zones and
background tasks.

[ ... ]

[Severity: Medium]
This is a pre-existing issue, but does the driver violate the subsystem
locking rules by implementing its own lock for sysfs attributes?

drivers/hwmon/adt7470.c:pwm_min_store() {
    ...
    mutex_lock(&data->lock);
    data->pwm_min[attr->index] = temp;
    ...
}

The hardware monitoring subsystem guidelines strictly state that drivers must
use hwmon_lock() and hwmon_unlock() for attributes registered by other means.

[Severity: Medium]
Is pwm functionality implemented directly within the hardware monitoring
driver rather than using an auxiliary device?

drivers/hwmon/adt7470.c:adt7470_probe() {
    ...
    chip = devm_pwmchip_alloc(dev, ADT7470_PWM_COUNT, 0);
    ...
    err = devm_pwmchip_add(dev, chip);
    ...
}

The hwmon subsystem guide explicitly mandates that secondary functionalities
must be implemented as an auxiliary device, with the driver residing in the
appropriate subsystem directory.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260729-adt7470_thermalzone-v4-0-c14039dfa0e6@gmail.com?part=4