Re: [PATCH v2] hwmon: add driver for ARCTIC Fan Controller

[Guenter Roeck]

On 3/16/26 21:21, Aureo Serrano wrote:
> On 2026-03-13 18:41:38+0100, Thomas Weißschuh wrote:
> > > +#define ARCTIC_ACK_TIMEOUT_MS	1000
> >
> > One second is a very long time. How long does the device take in
> > practice?
>
> Measured over 500 iterations: min ~32 ms, average ~100 ms, max ~563 ms.
> The latency is a characteristic of the device firmware and cannot be
> changed at this stage. I can change ARCTIC_ACK_TIMEOUT_MS from 1000 to 700.
> Does it work for you?

Since you measured more than 500 ms, I'd suggest to keep the one-second timeout
and add a note saying that 563ms was observed. Twice the observed value doesn't
seem extreme to me.

Thanks,
Guenter

> > > +	memset(buf + 11, 0, ARCTIC_REPORT_LEN - 11);
> >
> > The magic '11' should be '1 + ARCTIC_NUM_FANS'.
> > Or even easier, use kzalloc() above.
>
> Since the buffer is now struct-embedded and devm_kzalloc zeroes the struct
> at allocation, bytes 11-31 are always zero and the memset is not needed at
> all. This also addresses the suggestion from another reviewer to use a
> single once-allocated buffer rather than allocating per write. Hope that is
> acceptable.