On 3/9/25 00:03, Gerhard Engleder wrote:
On 08.03.25 23:32, Guenter Roeck wrote:
On 3/8/25 13:23, Gerhard Engleder wrote:
From: Gerhard Engleder <eg@xxxxxxxx>
The KEBA battery monitoring controller is found in the system FPGA of
KEBA PLC devices. It puts a load on the coin cell battery to check the
state of the battery.
Signed-off-by: Gerhard Engleder <eg@xxxxxxxx>
Looking into the keba code, that is kind of piece by piece approach.
I see a reference to fans in drivers/misc/keba/cp500.c, so I guess I'll see
a fan controller driver at some point in the future. I do not support
the idea of having multiple drivers for the hardware monitoring
functionality of a single device.
Yes, the fan controller will follow. The cp500 driver supports multiple
devices. All of them have the battery controller, but only some of them
have the fan controller. Fanless devices don't have a fan controller in
the FPGA. There are also devices with two fan controllers.
The battery controller and the fan controller are separate IP cores with
its own 4k address range (also I2C, SPI, ...). So I see them as separate
devices. There is also no guarantee if the address range of both
controllers is next to each other or not.
Does that help you to see the battery controller and the fan controller
as separate devices?
Barely. At this point I am not convinced that this should be a hwmon driver
in the first place.
Either case, the attribute needs to be documented.
You mean the documentation Documentation/hwmon/, which Thomas Weißschuh
also mentioned? I will add it.
Yes.
...
Not acceptable. The first voltage channel is channel 0, not 1.
I did that 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. I saw a similar dummy in ina3221.c, so I thought this is ok.
Isn't this a nice thing in the Linux kernel: You can find almost everything
in there to use as precedence.
The ina3221 driver slipped this in when it was submitted and, yes, I didn't
notice. When it was converted to the with_info API, it was too late to
change it. That doesn't make it better, and it is most definitely not an
argument to make for a new driver doing the same.
Guenter
