On 10.03.25 00:22, Guenter Roeck wrote:
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.
I will take a look at in0_min_alarm. Thank you for your advice.
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.
You are right, this test would drain the battery too faster if done too
often. This is why the test is limited to every 5 seconds and this is
agreed with the electronics engineers. I will document that fact too.
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's possible for some designs yes. I will discuss that with the
electronics engineer for future designs with separate RTC chip.
Thank you for your feedback!
Gerhard
