diff mbox series

[RFC,v3,1/9] dt-bindings: battery: Add temperature-capacity degradation table

Message ID 740503b6b6439e01959016223f1ae464e82824c3.1637061794.git.matti.vaittinen@fi.rohmeurope.com
State Changes Requested, archived
Headers show
Series power: supply: Add some fuel-gauge logic | expand

Checks

Context Check Description
robh/checkpatch success
robh/dt-meta-schema fail build log

Commit Message

Matti Vaittinen Nov. 16, 2021, 12:24 p.m. UTC
Some charger/battery vendors describe the temperature impact to
battery capacity by providing tables with capacity change at
given temperature. Support providing this temperature - capacity
dependency using the simple-battery DT nodes.

Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
---
 .../bindings/power/supply/battery.yaml        | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

Comments

Rob Herring Nov. 16, 2021, 2:02 p.m. UTC | #1
On Tue, 16 Nov 2021 14:24:48 +0200, Matti Vaittinen wrote:
> Some charger/battery vendors describe the temperature impact to
> battery capacity by providing tables with capacity change at
> given temperature. Support providing this temperature - capacity
> dependency using the simple-battery DT nodes.
> 
> Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
> ---
>  .../bindings/power/supply/battery.yaml        | 19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:
./Documentation/devicetree/bindings/power/supply/battery.yaml:134:36: [error] syntax error: expected <block end>, but found '<scalar>' (syntax)

dtschema/dtc warnings/errors:
./Documentation/devicetree/bindings/power/supply/battery.yaml:  while parsing a block mapping
  in "<unicode string>", line 134, column 11
did not find expected key
  in "<unicode string>", line 134, column 36
make[1]: *** Deleting file 'Documentation/devicetree/bindings/power/supply/battery.example.dts'
Traceback (most recent call last):
  File "/usr/local/bin/dt-extract-example", line 45, in <module>
    binding = yaml.load(open(args.yamlfile, encoding='utf-8').read())
  File "/usr/local/lib/python3.8/dist-packages/ruamel/yaml/main.py", line 434, in load
    return constructor.get_single_data()
  File "/usr/local/lib/python3.8/dist-packages/ruamel/yaml/constructor.py", line 119, in get_single_data
    node = self.composer.get_single_node()
  File "_ruamel_yaml.pyx", line 706, in _ruamel_yaml.CParser.get_single_node
  File "_ruamel_yaml.pyx", line 724, in _ruamel_yaml.CParser._compose_document
  File "_ruamel_yaml.pyx", line 775, in _ruamel_yaml.CParser._compose_node
  File "_ruamel_yaml.pyx", line 889, in _ruamel_yaml.CParser._compose_mapping_node
  File "_ruamel_yaml.pyx", line 775, in _ruamel_yaml.CParser._compose_node
  File "_ruamel_yaml.pyx", line 889, in _ruamel_yaml.CParser._compose_mapping_node
  File "_ruamel_yaml.pyx", line 775, in _ruamel_yaml.CParser._compose_node
  File "_ruamel_yaml.pyx", line 889, in _ruamel_yaml.CParser._compose_mapping_node
  File "_ruamel_yaml.pyx", line 775, in _ruamel_yaml.CParser._compose_node
  File "_ruamel_yaml.pyx", line 889, in _ruamel_yaml.CParser._compose_mapping_node
  File "_ruamel_yaml.pyx", line 773, in _ruamel_yaml.CParser._compose_node
  File "_ruamel_yaml.pyx", line 850, in _ruamel_yaml.CParser._compose_sequence_node
  File "_ruamel_yaml.pyx", line 775, in _ruamel_yaml.CParser._compose_node
  File "_ruamel_yaml.pyx", line 891, in _ruamel_yaml.CParser._compose_mapping_node
  File "_ruamel_yaml.pyx", line 904, in _ruamel_yaml.CParser._parse_next_event
ruamel.yaml.parser.ParserError: while parsing a block mapping
  in "<unicode string>", line 134, column 11
did not find expected key
  in "<unicode string>", line 134, column 36
make[1]: *** [Documentation/devicetree/bindings/Makefile:25: Documentation/devicetree/bindings/power/supply/battery.example.dts] Error 1
make[1]: *** Waiting for unfinished jobs....
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/power/supply/battery.yaml: ignoring, error parsing file
warning: no schema found in file: ./Documentation/devicetree/bindings/power/supply/battery.yaml
make: *** [Makefile:1413: dt_binding_check] Error 2

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/patch/1555776

This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit.
Linus Walleij Nov. 18, 2021, 1:57 a.m. UTC | #2
On Tue, Nov 16, 2021 at 1:24 PM Matti Vaittinen
<matti.vaittinen@fi.rohmeurope.com> wrote:

> Some charger/battery vendors describe the temperature impact to
> battery capacity by providing tables with capacity change at
> given temperature. Support providing this temperature - capacity
> dependency using the simple-battery DT nodes.
>
> Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>

Since we already support providing the capacity at different
temperatures using ocv-capacity-celsius and the array of
arrays ocv-capacity-table-0, 1, 2... you are introducing a
second parallel method of describing how capacity changes
in accordance with temperature, right?

What do you expect to happen if someone specifies both?

If this is an either/or situation then the schema has to
guarantee the exclusiveness for each.

(I would probably just use the formula you have to calculate
a few tables using the existing method but that's just me.)

Yours,
Linus Walleij
Matti Vaittinen Nov. 18, 2021, 5:27 a.m. UTC | #3
Hi deee Ho peeps!

On 11/18/21 03:57, Linus Walleij wrote:
> On Tue, Nov 16, 2021 at 1:24 PM Matti Vaittinen
> <matti.vaittinen@fi.rohmeurope.com> wrote:
> 
>> Some charger/battery vendors describe the temperature impact to
>> battery capacity by providing tables with capacity change at
>> given temperature. Support providing this temperature - capacity
>> dependency using the simple-battery DT nodes.
>>
>> Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
> 
> Since we already support providing the capacity at different
> temperatures using ocv-capacity-celsius and the array of
> arrays ocv-capacity-table-0, 1, 2... you are introducing a
> second parallel method of describing how capacity changes
> in accordance with temperature, right?

Oh, right. This is why sending out RFCs at early stage can be beneficial :)

The way I have seen OCV-CAP and TEMP-CAP 'dependencies' modelled has 
been that the OCV-CAP is defined only in one temperature (say, 25 C).

The impact of the temperature has then been estimated by storing values 
which reflect the delta CAP when temperature changes from this 'nominal 
temperature'. Hence it never even crossed my mind that the temperature 
impact to CAP should actually be modelled in OCV tables.

> What do you expect to happen if someone specifies both?

Right. I see this now. The current implementation would indeed apply the 
temperature impact twice. I didn't even think of this as we have only 
provided the OCV-CAP for one temperature.

> If this is an either/or situation then the schema has to
> guarantee the exclusiveness for each.
> 
> (I would probably just use the formula you have to calculate
> a few tables using the existing method but that's just me.)

I need to try to find out how the temperature-degradation is really used 
in setups which use our chargers (sigh. this is always the hard part for 
me) and see if we can replace the temp-degradation table by several 
OCV-CAP tables for different temperatures. I am afraid we may lack the 
OCV information for different temperatures - but I'll see. I'd rather 
not add overlapping properties.

Anyways - Thanks a lot Linus for giving me another view on this :) 
You're really helpful.

Best Regards
     --Matti
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/power/supply/battery.yaml b/Documentation/devicetree/bindings/power/supply/battery.yaml
index d56ac484fec5..98cc85b92a71 100644
--- a/Documentation/devicetree/bindings/power/supply/battery.yaml
+++ b/Documentation/devicetree/bindings/power/supply/battery.yaml
@@ -114,6 +114,25 @@  properties:
       - description: alert when battery temperature is lower than this value
       - description: alert when battery temperature is higher than this value
 
+  temp-degrade-table:
+    $ref: /schemas/types.yaml#/definitions/uint32-matrix
+    description: |
+      An array of value triplets. First value is "capacity change per degree C"
+      when temperature differs from 'set point'. Second value is "capacity
+      degradation at given 'set point' temperature" and third value is "the
+      'set-point' temperature" where given degradation is correct.
+      Up to 100 value triplets can be provided to specify different degradation
+      for different temperature ranges. When capacity change caused by
+      temperatures is computed the range which 'set point' is closest to the
+      current temperature is used. Capacity change should be in units of
+      micro Ah. Temperature is in units of 0.1 C degree.
+    maxItems: 100
+    items:
+      items:
+        - description: capacity drop per degree C in micro Ah
+        - description: capacity drop at 'set point' temperature in micro Ah
+        - description: 'set point' temperature for this range in 0.1 degree C
+
 required:
   - compatible