Re: [PATCH] hwmon: Add KEBA battery monitoring controller support

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

 



On 09.03.25 15:50, Guenter Roeck wrote:
On 3/9/25 00:23, Thomas Weißschuh wrote:

+static const struct hwmon_channel_info *kbatt_info[] = {
+    HWMON_CHANNEL_INFO(in,
+               /* 0: dummy, skipped in is_visible */

Why?

For compatibility reasons, as the out of tree version of the driver did
start with index 1 and there is software which rely on that fact. But
I'm unsure if this is a valid argument for mainline code. Guenter Roeck
also commented that, so will discuss this in the other thread.

Ack, lets' discuss with Guenter.
However I don't think it's going to fly.

This kind of argument is often used by those who want to implement non- standard
code. Implement it out-of-tree first and then say "sorry, we have to do it,
the out-of-tree code does it and our userspace depends on it". That is completely unacceptable. If that is what you want, and you are not willing to adjust your
userspace code, keep your code out of tree.

I'm sorry that I created the impression that I don't want to change
driver code and user space code. This is not the case, I'm will remove
that dummy and start with 0.

On top of that, I don't even know what the attribute means. An alarm attribute is supposed to indicate that a value is out of range. The implementation suggests
that this is is not the case. What is "battery ok" ? Voltage out of range ?
Battery failed ? The term itself suggests that it may reflect a failure.
It might be a "fault" attribute, and even that would not be a good match.
I'll need to see the actual description to determine what if anything is
acceptable. It will most definitely not be in1_alarm.

I will try to provide a clear picture in the other thread.

Gerhard




[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