Re: [PATCH 1/2] platform/x86: dell-ddv: Add hwmon support

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 2/6/23 13:19, Armin Wolf wrote:
Am 06.02.23 um 17:23 schrieb Guenter Roeck:

On Mon, Feb 06, 2023 at 03:13:25PM +0100, Hans de Goede wrote:
Hi Armin,

On 2/5/23 21:54, Armin Wolf wrote:
Thanks to bugreport 216655 on bugzilla triggered by the
dell-smm-hwmon driver, the contents of the sensor buffers
could be almost completely decoded.
Add an hwmon interface for exposing the fan and thermal
sensor values. Since the WMI interface can be quite slow
on some machines, the sensor buffers are cached for 1 second
to lessen the performance impact.
The debugfs interface remains in place to aid in reverse-engineering
of unknown sensor types and the thermal buffer.

Tested-by: Antonín Skala <skala.antonin@xxxxxxxxx>
Tested-by: Gustavo Walbon <gustavowalbon@xxxxxxxxx>
Signed-off-by: Armin Wolf <W_Armin@xxxxxx>
This looks nice and clean to me, thank you:

Reviewed-by: Hans de Goede <hdegoede@xxxxxxxxxx>

I'm going to wait a bit with merging this to see if Guenter
has any remarks. If there are no remarks by next Monday then
I'll merge this for the 6.3 merge window.

I don't have any further remarks. I won't send an Ack, though.
I noticed that the empty lines before return statements distract
me sufficiently enough that I am not sure about the actual code.

Guenter

I will send a 3rd revision of the patch series to remove those empty lines then.
Speaking of a 3rd revision, i noticed that jiffies are not updated during suspend,
so the sensor values might be wrong for a short time when resuming after a long time.

This can be solved by either usingktime_get_boottime_ns(), which does not stop during suspend, or by
invalidating the sensor cache upon resume. Which method should be used
in this case? Armin Wolf


I would suggest to invalidate the sensor cache; that seems less complex.

Guenter




[Index of Archives]     [LM Sensors]     [Linux Sound]     [ALSA Users]     [ALSA Devel]     [Linux Audio Users]     [Linux Media]     [Kernel]     [Gimp]     [Yosemite News]     [Linux Media]

  Powered by Linux