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

[Guenter Roeck]

On 7/27/26 11:44, Luiz Angelo Daros de Luca wrote:
> Em seg., 27 de jul. de 2026 às 11:33, Guenter Roeck
> <linux@xxxxxxxxxxxx> 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