On 3/9/25 14:36, Gerhard Engleder wrote:
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.
It is ok, though in0_min_alarm would be a better fit. And, yes, please
add the above to the documentation.
Given the above description, and the code itself, I'd be a bit concerned
that reading the value repeatedly will cause the battery to be drain faster
than necessary (otherwise it could be active all the time). If so, it might
make sense to reduce the frequency of such readings to, say, once a day.
From the MAX6916 datasheet:
"The MAX6916 does not constantly monitor an attached battery because such
monitoring would drastically reduce the life of the battery. As a result,
the MAX6916 only tests the battery for 1s every 24hr"
Personally I'd have tried to rely on the rtc itself to read the battery
status (RTC_VL_READ ioctl), but of course not every rtc supports that,
and not every rtc driver supports actually reporting it. Just in case
the rtc in your system does support it, and the driver doesn't (such
as the above mentioned MAX6916), it might make sense to submit a patch
for the rtc driver and use that mechanism, since that would be a more
generic solution than relying on proprietary fpga functionality.
That is just a note; it is your call, obviously, to decide how and how
often to check the battery status.
Thanks,
Guenter
