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

["Uwe Kleine-König" <ukleinek@kernel.org>]

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