Subject: Re: [PATCH 1/2] platform/x86: dell-ddv: Add hwmon support
To:     Guenter Roeck <linux@roeck-us.net>,
        Hans de Goede <hdegoede@redhat.com>
Cc:     markgross@kernel.org, jdelvare@suse.com,
        platform-driver-x86@vger.kernel.org, linux-hwmon@vger.kernel.org,
        linux-kernel@vger.kernel.org
References: <20230205205456.2364-1-W_Armin@gmx.de>
 <20230205205456.2364-2-W_Armin@gmx.de>
 <a7d2e011-142d-88b9-2591-cf4508f1b8c5@redhat.com>
 <20230206162303.GA195090@roeck-us.net>
From:   Armin Wolf <W_Armin@gmx.de>
Message-ID: <08af724c-8c65-db41-ece8-f330f67a3dc2@gmx.de>
Date:   Mon, 6 Feb 2023 22:19:47 +0100
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101
 Thunderbird/78.13.0
MIME-Version: 1.0
In-Reply-To: <20230206162303.GA195090@roeck-us.net>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Transfer-Encoding: quoted-printable
Content-Language: en-US
UI-OutboundReport: notjunk:1;M01:P0:rKKXN3+pvtM=;owy0w449dm+D2EMy/Rzxcf04ygy
 kz5sz8iPjgRTpenrSmA4CRxfgCHL5oEFAD86Pm3/WCwQ2/34ZGbUXe/sd6PhIGInG0nSw1ojc
 srO1dOFBRx5DfkRpprDantaaco6QL5QdktGLU6P0Qlesb2CbD5urmM/3SchaSVVf/x/5Yog30
 gEWF1td/ka3hMxwcTztj2Qsfo7oGdQwO/WkGEgt0AnQMHtrbG0w4Ky3mV6pfarm5FATbMaTNi
 n91sxGFkLs3+MqvqZ8uMqjW1az3KvFWQVAhOmP+NbdgZjxUjIGZ1uC/TXJV00yrseMq2yOpML
 P6Sx7qaJuDvfAgCcgz0kN7+xSgWTnbh6YrEGVNbDfn/HRU2Zw42FghNUHatL9LR2W810yhn2K
 CVZC3KwpeMa4C9Nx/XB+wyFg4vQI4FyK46rBWP2+Ulex7swc/pBzu3B7jmQgHpXNB1vnznCdq
 aznvMMhazOKjg5x6qj2MXDCPPpXsaCimFytLMCDnADPvzCFEc5oMx4bTgyj9YN/f9F176a7mD
 Y/IYKjjsFI4fhknfXBdLZ/kzzsR3x16S5+EAliYJWhomIZE7xoAzxQ9pgd0HdlKGH1GBWKtVf
 m7AkQXKIg/OxGGSz1IzOrw7w5KA/yYQNToTzoH8i0kEeRD4x27hjZlR40x0FF4QNcslsy8G69
 02yFaBPMoF+w7GIi+R0W7/XYebmSmvrZtwAPSm+nPKW1/acpM7LkcpxDntntLtlG8/Eq3n1H4
 aCv+W0qI9ATCuvbg+q66XDc9f+W9U22FdLJ9LYzHAI1eOHl/cFuCLv6Pa/VTsfMCXU5/DOsoE
 NUfn8TTgneCieLufgFW3LXKG/i0ZCIMyBUTUrRb5VYEID/XV7118UVU8EqH49IMG0Su/UXxr4
 zEK/9E0oWoPvfzRTwRyDkQZYFIx51UFCm+Gmd1OfvKPL6TbDfDCoR89mWm891ddiNZiRRPyUo
 3B2/hxxABwLzOsZpal7GwpxxZ3o=
Precedence: bulk

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=C3=ADn Skala <skala.antonin@gmail.com>
>>> Tested-by: Gustavo Walbon <gustavowalbon@gmail.com>
>>> Signed-off-by: Armin Wolf <W_Armin@gmx.de>
>> This looks nice and clean to me, thank you:
>>
>> Reviewed-by: Hans de Goede <hdegoede@redhat.com>
>>
>> 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