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

[Matti Vaittinen <mazziesaccount@gmail.com>]

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?

Oh, and sorry for asking to review something which is obviously not 
working approach. I should've understood this from the beginning.

Yours,
	-- Matti

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

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