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

[Andreas Kemnade <andreas@kemnade.info>]

On Fri, 14 Nov 2025 10:39:54 -0600
Rob Herring <robh@kernel.org> 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@gmail.com>
> > > > 
> > > > The BD72720 PMIC has a battery charger + coulomb counter block. These
> > > > can be used to manage charging of a lithium-ion battery and to do fuel
> > > > gauging.
> > > > 
> > > > ROHM has developed a so called "zero-correction" -algorithm to improve
> > > > the fuel-gauging accuracy close to the point where battery is depleted.
> > > > This relies on battery specific "VDR" tables, which are measured from
> > > > the battery, and which describe the voltage drop rate. More thorough
> > > > explanation about the "zero correction" and "VDR" parameters is here:
> > > > https://lore.kernel.org/all/676253b9-ff69-7891-1f26-a8b5bb5a421b@fi.rohmeurope.com/
> > > > 
> > > > Document the VDR zero-correction specific battery properties used by the
> > > > BD72720 and some other ROHM chargers.
> > > > 
> > > > Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com>
> > > > Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
> > > > 
> > > > ---
> > > > NOTE:
> > > > Linus' rb-tag holds only if there's no further comments from Rob.
> > > > 
> > > > Revision history:
> > > >   v3 =>:
> > > >   - No changes
> > > > 
> > > >   v2 => v3:
> > > >   - Constrain VDR threshold voltage to 48V
> > > >   - Use standard '-bp' -suffix for the rohm,volt-drop-soc
> > > > 
> > > >   RFCv1 => v2:
> > > >   - Add units to rohm,volt-drop-soc (tenths of %)
> > > >   - Give real temperatures matching the VDR tables, instead of vague
> > > >     'high', 'normal', 'low', 'very low'. (Add table of temperatures and
> > > >     use number matching the right temperature index in the VDR table name).
> > > >   - Fix typoed 'algorithm' in commit message.
> > > > 
> > > > The parameters are describing the battery voltage drop rates - so they
> > > > are properties of the battery, not the charger. Thus they do not belong
> > > > in the charger node.
> > > >   
> > 
> > // snip
> >   
> > > My bot found errors running 'make dt_binding_check' on your patch:
> > > 
> > > yamllint warnings/errors:
> > > 
> > > dtschema/dtc warnings/errors:
> > > /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/power/supply/rohm,vdr-battery.example.dtb: battery (simple-battery): 'degrade-cycle-microamp-hours', 'rohm,volt-drop-0-microvolt', 'rohm,volt-drop-1-microvolt', 'rohm,volt-drop-2-microvolt', 'rohm,volt-drop-3-temp-microvolt', 'rohm,volt-drop-soc-bp', 'rohm,volt-drop-temperatures-millicelsius', 'rohm,voltage-vdr-thresh-microvolt' do not match any of the regexes: '^ocv-capacity-table-[0-9]+$', '^pinctrl-[0-9]+$'
> > > 	from schema $id: http://devicetree.org/schemas/power/supply/battery.yaml
> > >   
> > 
> > Odd. I am pretty sure I didn't see this when I ran the make
> > dt_binding_check. Not 100% sure what happened there. I get this error now
> > though when including all the bindings to the check.
> > 
> > Do I get this right - these errors result from the properties used in
> > example not being included in the battery.yaml? So, this means that the
> > check is done based on the binding (battery.yaml) where the compatible
> > (simple-battery) is defined - not based on the properties which are present
> > in this file where the example resides, (and which references the
> > battery.yaml)?
> > 
> > ...
> > 
> > Oh... Now that I wrote it I feel like an idiot.
> > 
> > This approach couldn't work for the validation, right? Let's assume I had a
> > VDR battery, and I added a static-battery -node for it. Running the
> > validation would pick the battery.yaml based on the compatible (just as it
> > does here), and be completely unaware of this vdr-battery.yaml. I have no
> > idea why I thought this would work. Probably because I only thought this
> > from the documentation POV.
> > 
> > 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.
> 
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. 

Regards,
Andreas