Message-ID: <3dd9aa2d-a318-4a94-b53f-11dac139ccb2@gmail.com>
Date: Mon, 18 Aug 2025 12:32:43 +0300
From: Matti Vaittinen <mazziesaccount@...il.com>
To: Andreas Kemnade <andreas@...nade.info>
Cc: Krzysztof Kozlowski <krzk@...nel.org>, Lee Jones <lee@...nel.org>,
 Sebastian Reichel <sre@...nel.org>, linux-kernel@...r.kernel.org,
 linux-pm@...r.kernel.org
Subject: Re: [PATCH 2/2] power: supply: Add bd718(15/28/78) charger driver

On 18/08/2025 11:36, Andreas Kemnade wrote:
> Hi Matti,
> 
> Am Mon, 18 Aug 2025 09:34:02 +0300
> schrieb Matti Vaittinen <mazziesaccount@...il.com>:
> 
>> On 17/08/2025 11:13, Krzysztof Kozlowski wrote:
>>> On 17/08/2025 10:11, Andreas Kemnade wrote:
>>>> Am Sun, 17 Aug 2025 07:58:35 +0200
>>>> schrieb Krzysztof Kozlowski <krzk@...nel.org>:
>>>>   
>>>>> On 16/08/2025 21:19, Andreas Kemnade wrote:
>>>>>> Add charger driver for ROHM BD718(15/28/78) PMIC charger block.
>>>>>> It is a stripped down version of the driver here:
>>>>>> https://lore.kernel.org/lkml/dbd97c1b0d715aa35a8b4d79741e433d97c562aa.1637061794.git.matti.vaittinen@fi.rohmeurope.com/
>>>>>
>>>>> Why are you duplicating the driver? Why original cannot be used?
>>>>>
>>>>>   
>>>> I am not duplicating the driver. That patch series never went in. I am
>>>> stripping it down to let things go in step by step. I have also talked
>>>> with Sebastian about this. And he also prefers a step by step approach
>>>> to have it more easily reviewed.
>>>> I also do not have the infrastructure to test things like capacity
>>>> degradation over time. There is non-trivial rebasing work involved, so
>>>> I even do not feel confident submitting such at all.
>>>
>>>
>>> OK, but if you refer to other work, then also please explain why this is
>>> stripped down.
>>
>> First of all, thanks a ton Andreas for continuing this work which I
>> never managed to finish!
>>
>> Battery fuel-gauging with coulomb-counter is hard. I believe we can get
>> some results with the original RFC code - but it requires quite a bit of
>> effort. AFAIR, there are (at least) 4 "pain-points".
>>
> Newest rebase I have is for 6.15. Yes, capacity calculation is hard.
> Even the ugly-patched Kobo vendor driver has some surprises. It once
> says battery is empty, then I put in charger, rebooted into debian,
> Vbat = 4.1V even with charger detached.

:/

> I think the fuel-gauging stuff itself should go in a step by step
> approach.

I agree.

> I am wondering how sophisticated other drivers and hardware
> are.

I have no deep knowledge on this (either). I remember having some 
(email) discussions with Linus W about Samsung's chargers / batteries... 
My understanding is that there are very different levels of 
"sophistication", both in HW and in SW. I really find this fascinating. 
Unfortunately, there has also been infamous exploding batteries and 
other less pleasant events. Hence this is also slightly dangerous area.

> The rn5t618/rc5t619 mainline driver just uses raw coloumb counter
> values and there is no compensation for anything. Some hardware does
> more sophisticated things itself.

Yes.

>> 1. Lack of persistent storage for charging cycles. For proper
>> fuel-gauging, we would need information about battery aging. The PMIC
>> has nothing to store the charging cycle counter when power is cut.
>> That'd require some user-space solution which could store the cycle
>> information in a persistent storage && tell it to the driver at
>> start-up. I don't know if there is open-source userspace solution for this.
>>
> I do not think so, and you will have trouble if you have dual-boot or
> from some alternative boot media involved.

I didn't even think about it. So, even with persistent PMIC areas, if 
software is doing the charging count book-keeping, it won't be great for 
a generic design. (May work Ok with an embedded device which is likely 
to not get booted with other flavours of software).

> The BQ27000 stuff has afaik
> hw calculation of battery capacity to deal with this.
> 
>> 2. Battery parameters. This is the real problem. In order to make the
>> fuel-gauging work, the driver needs proper battery information. I wrote
>> the original driver to be able to retrieve the data from a
>> static-battery DT node - but I have a feeling the device-vendor using
>> this PMIC provided battery-info via module parameters. I am not sure if
>> those parameters can be recovered - and as Andreas said, defining them
>> is not easy task. By minimum we would need the OCV-tables and some aging
>> + temperature degradation effects (or VDR-tables which ROHM uses for
>> it's zero-correction algorithm - but AFAIR, defining those VDR tables is
>> not widely known information).
>>
> Kobo kernels have these tables as part of the driver, the right one is
> selected via an index in the NTX_HWCONFIG blob provided by the
> bootloader to the kernel. So that is not necessarily a problem. It
> could be translated into dt.
> 
> static int ocv_table_28_PR284983N[23] = {
>          //4200000, 4162288, 4110762, 4066502, 4025265, 3988454, 3955695, 3926323, 3900244, 3876035, 3834038, 3809386, 3794093, 3782718, 3774483, 3768044, 3748158, 3728750, 3704388, 3675577, 3650676, 3463852, 2768530
>          4200000, 4166349, 4114949, 4072016, 4031575, 3995353, 3963956, 3935650, 3910161, 3883395, 3845310, 3817535, 3801354, 3789708, 3781393, 3774994, 3765230, 3749035, 3726707, 3699147, 3671953, 3607301, 3148394
> };
> 
> static int vdr_table_h_28_PR284983N[23] = {
>          //100, 100, 101, 101, 102, 102, 103, 103, 104, 104, 105, 106, 106, 107, 107, 108, 108, 109, 110, 112, 124, 157, 786
>          100, 100, 101, 102, 102, 105, 106, 107, 112, 108, 108, 105, 105, 108, 110, 110, 110, 111, 112, 114, 120, 131, 620
> };
> 
> static int vdr_table_m_28_PR284983N[23] = {
>          //100, 100, 101, 101, 102, 102, 103, 103, 104, 104, 105, 102, 100, 100, 102, 103, 103, 105, 108, 112, 124, 157, 586
>          100, 100, 103, 106, 110, 114, 115, 119, 122, 122, 115, 113, 112, 114, 117, 124, 126, 123, 122, 126, 140, 156, 558
> };
> 
> static int vdr_table_l_28_PR284983N[23] = {
>          //100, 100, 103, 105, 110, 110, 113, 112, 112, 112, 105, 110, 110, 111, 122, 131, 138, 143, 150, 166, 242, 354, 357
>          100, 100, 105, 110, 114, 117, 121, 125, 126, 122, 116, 114, 115, 118, 124, 132, 140, 148, 156, 170, 210, 355, 579
> };
> 
> static int vdr_table_vl_28_PR284983N[23] = {
>          //100, 100, 103, 106, 108, 111, 114, 117, 118, 115, 108, 106, 108, 113, 115, 114, 118, 125, 144, 159, 204, 361, 874
>          100, 100, 109, 115, 118, 123, 127, 130, 140, 139, 134, 130, 128, 138, 140, 150, 154, 164, 178, 204, 271, 362, 352
> };

Oh, good. If we can get the right battery parameters from the vendor 
driver, then the main problem gets solved. Although, multiple sets of 
different VDR tables probably means, that there are variants with 
different types of battery out there. I assume the bootloader can 
somehow detect the battery type to provide the correct blob?

> 
>> 3. ADC offset. The coulomb-counter operates by measuring and integrating
>> voltage-drop over known Rsense resistor. If (when) the ADC has some
>> measurement offset, it will produce a systematic error which accumulates
>> over time. Hence a calibration is required. The BD718[15/28] have an ADC
>> calibration routine, but AFAIR, there was some limitations. I don't
>> remember all the dirty details, but it probably didn't work too well if
>> current consumption was varying during the calibration(?). I think
>> running the calibration is not supported by the driver.
>>
> Yes, that is a pain.

I am pretty sure I can dig the registers which initiate the ADC 
calibration, but I don't have real devices with real battery to test it. 
I can try to find that information if if you wish to experiment with it 
though...

...The BD718xx had a magic "test register area" - where this calibration 
stuff (amongst other, very hazardous things) resides. Problem is that 
this "test register area" is implemented in a way, that it is behind 
another I2C slave address, which can be enabled by a magic write 
sequence. Enabling it from a generically usable driver can't really be 
done. It would be hazardous if there was another device in the I2C with 
the same slave address as the "test register area".

> [...]
>> TLDR; It'd be hard to do accurate fuel-gauging without proper
> battery
>> information and some extra work. We could probably get some rough
>> estimates about the capacity - but implementing it only makes sense if
>> there is someone really using it. Charger control on the other hand
>> makes some sense. [It at least allows Andreas to charge his eReader
>> using solar-power when on a biking hiking! How cool is that? ;)]
>>
> And using a hub dynamo.
> For now I have a user space script to help me, probably moving that into
> input_current_limit.

Sounds good to me.

> But it is really nice to see if things are charging or are discharging
> unusually fast.
> It is a pity that such power sources are not taken into consideration
> in standards or charges much. Around 20 year ago or something, I could
> just attach a Thinkpad to a solar panel, there was a smooth transition
> between discharging a litte (displaying battery discharging time in
> weeks) and more ore less charging. Today often the recommendation is to
> put somehow another battery in between. But that is technically
> somehow nonsense. You need a buffer for power and another one in the
> row.

Yours,
	-- Matti

Powered by blists - more mailing lists