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

On 7/27/26 11:44, Luiz Angelo Daros de Luca wrote:
> Em seg., 27 de jul. de 2026 às 11:33, Guenter Roeck
> <[email protected]> escreveu:
>>
>> On 7/26/26 20:34, Luiz Angelo Daros de Luca wrote:
>>> adt7470_pwm_read() currently ignores failures returned by
>>> pwm1_freq_get(). If the register read fails, the negative error code is
>>> returned through *val while the function itself reports success,
>>> potentially exposing a negative PWM frequency through sysfs.
>>>
>>> Fix this by using the cached PWM frequency maintained by the driver,
>>> eliminating the register access from the read path.
>>>
>>> Apart from the corrected error propagation and using the cached value,
>>> no functional change is intended.
>>>
>>
>> An alternative might be to use regmap for caching and drop all driver-internal
>> caching. I have done that for other drivers; typically it simplifies the code,
>> sometimes significantly, since the update_device() function is no longer needed.
>> Something to consider for later, maybe.
> 
> That definitely sounds like a nice upgrade to consider!
> 
> I haven't used regmap's caching feature before, so I'm not entirely
> sure how cache expiration is typically handled. In adt7470, caching is
> mainly used for volatile readings because hardware access over
> SMBus/I2C is quite slow. On my device (which might be an extreme
> case), reading all registers can take over a second.
> 

regmap caching does not time out. Registers are either cached or not.
Usually that works just fine because there are not that many volatile
registers in a chip. Most are limits which can be cached forever.

> What would be the recommended strategy here? Would it be caching all
> registers and periodically calling regcache_drop_region() (or mark
> them volatile) to force a refresh?
> 

What I usually do is to just mark the volatile registers in regmap,
and drop all driver-internal caching. I would not see the point of
re-reading limit registers.

There is still a case-by-case decision to be made for cacheable
values which would need excessive calculations (more than just mapping)
if read from a cached register value.

I'd say never mind though - you are right, the driver has a number
of issues, and Sashiko is likely to keep finding more.

Guenter