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

[sashiko-bot@kernel.org]

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