linux-kernel - Re: [PATCH v2] hwmon: corsair-psu: add reporting of rail mode via debugfs

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <8beb8ae2-04cd-fa37-5dbe-8d18e6a53f4d@roeck-us.net>
Date:   Wed, 10 Aug 2022 11:21:36 -0700
From:   Guenter Roeck <linux@...ck-us.net>
To:     Wilken Gottwalt <wilken.gottwalt@...teo.net>
Cc:     linux-kernel@...r.kernel.org, Jean Delvare <jdelvare@...e.com>,
        Jonathan Corbet <corbet@....net>, linux-hwmon@...r.kernel.org
Subject: Re: [PATCH v2] hwmon: corsair-psu: add reporting of rail mode via
 debugfs

On 8/10/22 10:48, Wilken Gottwalt wrote:
> On Wed, 10 Aug 2022 10:29:08 -0700
> Guenter Roeck <linux@...ck-us.net> wrote:
> 
>> On 8/10/22 09:56, Wilken Gottwalt wrote:
>>> On Wed, 10 Aug 2022 09:31:21 -0700
>>> Guenter Roeck <linux@...ck-us.net> wrote:
>>>
>>>> On 8/10/22 06:53, Wilken Gottwalt wrote:
>>>>> Add reporting if the PSU is running in single or multi rail mode via
>>>>> ocpmode debugfs entry. Also update the documentation accordingly.
>>>>>
>>>>> Signed-off-by: Wilken Gottwalt <wilken.gottwalt@...teo.net>
>>>>> ---
>>>>> Changes in v2:
>>>>>      - fixed spelling issues in commit message
>>>>
>>>> You did not address or even provide feedback on my second comment.
>>>
>>> Oh darn ... sorry, I was quite busy and didn't really pay attention. I will
>>> answer the earlier mail and think about it.
>>>
>>> Though, maybe you can help me with that what keeps me so busy. Would it be okay
>>> to use a kthread in a hwmon driver to do sampling (500ms - 10s) in conjunction
>>> with HWMON_C_UPDATE_INTERVAL, or is this a strict no-no? I know it is actually
>>> used to set a sample/update rate in a sensor (-register), but this USB-HID
>>> approach is a pure polling thing. It seems to work quite and enables the driver
>>> to collect data quite early in the boot process.
>>>
>>
>> It really depends. Is it _necessary_ ? The pwm-fan driver uses a timer for
>> periodic polling, but that is because it has to. We should not do it purely
>> for convenience, and from the code I don't immediately see why it would
>> be necessary.
> 
> Together with the polling I would add encountered lowest and highest values and
> the average of basically all available sensors (kind of session statistics). I
> know it is a bit odd, but currently these power supplies are sold again in a
> newer version and people really like to use them in their servers/workstations
> because of the "realtime" data and this driver. No joke, but I really got
> several requests to add this and I must admit I have quite some fun implementing
> it.
> 

That is out of scope for a kernel driver. If desired, a user space application
should do the polling and calculate statistics such as lowest/highest or averages.

Guenter

> Maybe I can provide a patch after I'm done and you can decide if this is okay
> or not. After all I provide an external more enhanced driver via github where
> some features are added, which are a clear no-go. It would be nice to have that
> in mainline, but I'm absolutely fine with a "no".
> 
> greetings,
> Wilken