Re: [hwmon PATCH v2 6/8] hwmon: (adt7470) Use cached PWM frequency value

On 7/27/26 17:36, [email protected] wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] A partial register write failure in pwm1_freq_set leaves the cached pwm_freq permanently out of sync with the actual hardware state.
> --
> 

I don't know a good way of fixing this. If this error condition really
happens, the chip is in bad shape either case. Trying to write the old
value into the first register might fail as well, and then what ?

Ok to leave it alone.

Guenter

> --- Patch [6]: [hwmon PATCH v2 6/8] hwmon: (adt7470) Use cached PWM frequency value ---
> commit 3b9b6b91fb202f2983f66278540c1017bb53f4e6
> Author: Luiz Angelo Daros de Luca <[email protected]>
> 
> hwmon: (adt7470) Use cached PWM frequency value
> 
> This commit updates the ADT7470 driver to use a cached PWM frequency
> value instead of querying the hardware on each read. The cache is
> initialized during probe and updated after a successful write operation
> to prevent exposing negative error codes through sysfs if a read fails.
> 
>> diff --git a/drivers/hwmon/adt7470.c b/drivers/hwmon/adt7470.c
>> index c6fc7d38d698c..1fbca4869b7b6 100644
>> --- a/drivers/hwmon/adt7470.c
>> +++ b/drivers/hwmon/adt7470.c
> 
> [ ... ]
> 
>> @@ -832,6 +835,10 @@ static int pwm1_freq_set(struct device *dev, long freq)
>>   	err = regmap_update_bits(data->regmap, ADT7470_REG_CFG_2,
>>   				 ADT7470_FREQ_MASK,
>>   				 index << ADT7470_FREQ_SHIFT);
>> +	if (err < 0)
>> +		goto out;
>> +
>> +	data->pwm_freq = closest_freq;
>>   out:
>>   	mutex_unlock(&data->lock);
> 
> [Severity: Medium]
> Does this leave the cached pwm_freq out of sync with the hardware if the
> second register write fails?
> 
> If regmap_update_bits() fails for ADT7470_REG_CFG_2, the function aborts
> and jumps to out without updating data->pwm_freq. However, the first
> register (ADT7470_REG_CFG) may have already been updated successfully right
> above this snippet.
> 
> Because adt7470_pwm_read() now relies solely on this cache instead of
> reading the hardware, sysfs reads will permanently return the old frequency
> value even though the hardware might be operating in a mixed frequency
> state.
>