Re: [PATCH v2 1/1] hwmon: Add Minisforum UM780 XTX EC monitoring and fan control

["Sebastián Peyrott" <speyrott@gmail.com>]

Thanks for the review.

> Are you sure this is what you want ? It will end up in a tight loop
> if the problem is persistent.

The in-driver retry loop is bounded, but I agree that returning -EAGAIN after
the retries are exhausted can encourage immediate retries by the caller. I
will return -EIO for a persistently incoherent or out-of-range RPM value.

> This is an inappropriate return value. Either define mode = 4 or similar
> for "other" or "unknown" or return -ENODATA.

Agreed. An unknown register value does not necessarily identify another valid
firmware mode, so I will return -ENODATA.

> This will require explanation. The values are stored in probe,
> stored here again, stored in suspend, and (only) restored in resume.
>
> Why store here and in probe if they are updated during suspend anyway ?
> Alternatively, why update in suspend if they are already updated from
> the probe function and here ?

The intention was to retain the last coherent configuration for restoration,
not to maintain a general hardware cache, but reading it again at suspend made
that ownership unclear and is redundant.

I will seed the saved state once at probe, update it only after successful
hwmon writes, and remove the suspend callback. Resume will restore that saved
state. I will also rename the fields to make their purpose explicit. The live
reads in the system-threshold store path will remain only to validate and
clamp against the current peer threshold before an individual, non-atomic EC
write.

Removing the suspend callback also addresses the automated review observation
that a transient EC read failure there could abort system suspend.

I will give this some time for any further feedback before sending v3.

Thanks,
Sebastian