Re: [PATCH v2] hwmon: corsair-psu: add reporting of rail mode via debugfs
From: Wilken Gottwalt
Date: Wed Aug 10 2022 - 13:49:41 EST
On Wed, 10 Aug 2022 10:29:08 -0700
Guenter Roeck <linux@xxxxxxxxxxxx> wrote:
> On 8/10/22 09:56, Wilken Gottwalt wrote:
> > On Wed, 10 Aug 2022 09:31:21 -0700
> > Guenter Roeck <linux@xxxxxxxxxxxx> 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@xxxxxxxxxx>
> >>> ---
> >>> 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.
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