On 09.03.25 16:04, Guenter Roeck wrote:
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.
Here a more detailed description, which I would add to
Documentation/hwmon/ in the proper form:
The PLC devices use a coin cell battery for the RTC to keep the current
time. The goal is to provide information about the coin cell state to
user space. Actually the user space shall be informed that the coin cell
battery is nearly empty and needs to be replaced.
The coin cell must be tested actively to get to know if its nearly empty
or not. Therefore, a load is put on the coin cell and the resulting
voltage is evaluated. This evaluation is done by some hard wired analog
logic, which compares the voltage to a defined limit. If the voltage is
above the limit, then the coin cell is assumed to be ok. If the voltage
is below the limit, then the coin cell is nearly empty (or broken,
or removed, ...) and shall be replaced by a new one. The battery
controller allows to start the test of the coin cell and to get the
result if the voltage is above or below the limit. The actual voltage is
not available. Only the information if the voltage is below a limit is
available.
That's why I thought a voltage alarm is a good fit. But I'm not an
expert and I'm curious about your opinion.
(...)
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.
I'm sorry, I only wanted to mention where the idea comes from. I will
start with channel 0.
Gerhard
