Re: [PATCH v4 04/16] dt-bindings: power: supply: BD72720 managed battery

From: Matti Vaittinen
Date: Mon Nov 17 2025 - 03:15:24 EST

On 14/11/2025 18:39, Rob Herring wrote:
On Fri, Nov 14, 2025 at 11:04:27AM +0200, Matti Vaittinen wrote:
On 13/11/2025 12:53, Rob Herring (Arm) wrote:

On Thu, 13 Nov 2025 10:52:19 +0200, Matti Vaittinen wrote:
From: Matti Vaittinen <mazziesaccount@xxxxxxxxx>

//snip


So, as far as I understand, the only viable options are expanding the
existing battery.yaml with these properties (which I hoped to avoid, see
below)

The right place for them is the battery node, which is described by the
generic "battery.yaml". I was not comfortable with adding these
properties to the generic battery.yaml because they are:
- Meaningful only for those charger drivers which have the VDR
algorithm implemented. (And even though the algorithm is not charger
specific, AFAICS, it is currently only used by some ROHM PMIC
drivers).
- Technique of measuring the VDR tables for a battery is not widely
known. AFAICS, only folks at ROHM are measuring those for some
customer products. We do have those tables available for some of the
products though (Kobo?).

or, to add new compatible for the "vdr-battery".
AFAICS, adding new compatible would require us to wither duplicate the used
properties from battery.yaml here (as battery.yaml mandates the
"simple-battery" - compatible) - or to split the battery.yaml in two files,
one containing the generic properties, other containing the "simple-battery"
-compatible and referencing the generic one. Then the "vdr-battery" could
also reference the generic one.

Any suggestions for the next path to follow?

Probably the latter option. You could do the former and make the new
properties conditional on the "vdr-battery" compatible. That's fine with
small differences, but gets messy as there are more properties and
variations.

But is "VDR" a type of battery though? Is there a certain type/chemistry
of battery we should be describing where VDR is applicable?

No. Not that I know. My understanding is that the "VDR (voltage drop rate)" refers to measured voltage drop-rates under certain conditions - which can be used to (more accurately) estimate the remaining capacity when battery is nearly depleted. As far as I know, this is only used with Lithium-ion batteries (I am not at all sure of this) - but I _assume_ the technique could be applied to other type of batteries as well.

I don't
think it scales well if we define battery compatibles for every
variation of charger algorithm. Honestly I don't mind just adding 1
property. I care more if we allow undocumented properties than
allowing documented but invalid for the platform properties.

I see. The "VDR" stuff is really tightly bound to the fuel-gauging algorithm. It is measured characteristics of the battery - but those values are only usable by the "VDR" algorithm. I don't really have a good insight in the amount of fuel-gauging algorithm related properties suggested to be added during the years - but don't think there have been that many of them. So, I am not that worried about adding the compatible. On the other hand, there is no technical reason (other than adding properties which are unused on many platforms) why not to add the vdr tables in the static-battey node without adding own compatible. And, reading reply from Andreas (I'll copy it here to answer it in same mail)

/// Below text is form Andreas:
> just keep in mind, that several kobo devices have one pmic in one board
> revision and another one in the other (e.g. Kobo Nia rev A vs rev C).
> But probably the same battery. So if the "vdr-battery" is a compatible
> just to allow a more properties,
> then "simple-battery" should be allowed as fallback.

I didn't know Kobos use multiple chargers. Thanks Andreas! So, in that sense, adding the "vdr" tables in static-battery node, without new compatible, would maybe be simplest solution. Then the charger(s) (fuel-gauge(s)) which implement VDR algorithm, can pick the tables while those chargers which don't implement the VDR will just ignore these tables.

When it
becomes 10, 20, 30 properties, then I might start to care.

For VDR there are only:

rohm,voltage-vdr-thresh-microvolt,
rohm,volt-drop-soc-bp,
rohm,volt-drop-temperatures-millicelsius

and

patternProperties:
'^rohm,volt-drop-[0-9]-microvolt':

So, from the binding point of view (.yaml), it's not _that_ lot. In the .dts there will be quite some noise as the tables have several values.


If that
happens, either we are doing a poor job of generically describing
battery parameters or chargers and batteries are tightly coupled and
can't be described independently.

I am under impression that chargers tend to be pretty flexible, and they can be configured to work with many different batteries by altering the charging profiles. Most of the battery properties (like and charging phases [like pre, CC, CV], their limits, currents and voltages etc) are very generally usable. So, large subset of charging functionality can be handled with standard properties. I believe it is only the fuel-gauging where things get more hairy.

I did prepare a series which does the split and adds new compatible for the 'rohm,vdr-battery'. (The power-supply class is not yet modified in the series, but we would probably want to modify the battery-info getters to also accept the 'rohm,vdr-battery' -compatible.)

I wonder if I should actually prepare also a series where these properties are just placed in the existing static battery node without adding new compatible. That way it would be easier to see which way is better.

If I do that, should I only spin these bindings as RFC to avoid the unnecessary noise?

Oh, and a big thanks to both of you Rob and Andreas! I feel this gained more clarity after your feedback :)

Yours,
-- Matti

---
Matti Vaittinen
Linux kernel developer at ROHM Semiconductors
Oulu Finland

~~ When things go utterly wrong vim users can always type :help! ~~