Re: [PATCH] hwmon: (aquacomputer_d5next) Add support for temperature sensor offsets

From: Aleksa Savic
Date: Sun Oct 23 2022 - 09:16:21 EST


On 2022-10-22 15:57:20 GMT+02:00, Guenter Roeck wrote:
> Please go up to 100 columns to avoid excessive line splits.

Will fix this and other comments in v2.

> Is it really necessary to re-read the control buffer repeatedly
> to report this value ? I don't know how costly that is, but unlike
> the pwm value I would not expect the number to change.

Yes, aside from the driver userspace can also change settings on the
device using hidraw and we'd end up with stale data. Reading it is
very fast, it takes about 4ms in my testing.

> Also, is this number indeed not included in the regular reports
> sent from the controller ?

Unfortunately, it's not. The sensor report only includes final (calculated)
sensor readings.

> The driver doesn't distinguish between offsets in the control buffer
> (pwm, and now temperature sensor offset) and offsets in the report buffer,
> making it a bit difficult to determine if those are the same or not.
> Some explanation in the driver would be nice if someone finds the time
> to provide one. If the control buffer offsets are in a different number
> space, they should really be marked accordingly (for example with a
> _CTRL in the define).

I can see how it can be confusing. After this, I can send a patch to
reorder the macros & initializations and add more comments regarding
what is what.

Thanks,
Aleksa