[v11,01/10] devicetree: power: Add battery.txt

Submitted by Liam Breck on March 20, 2017, 9:43 a.m.

Details

Message ID 20170320094335.19224-2-liam@networkimprov.net
State Not Applicable
Headers show

Commit Message

Liam Breck March 20, 2017, 9:43 a.m.
From: Liam Breck <kernel@networkimprov.net>

Documentation of static battery characteristics that can be defined
for batteries which cannot self-identify. This information is required
by fuel-gauge and charger chips for proper handling of the battery.

Cc: Rob Herring <robh@kernel.org>
Cc: devicetree@vger.kernel.org
Signed-off-by: Matt Ranostay <matt@ranostay.consulting>
Signed-off-by: Liam Breck <kernel@networkimprov.net>
---
 .../devicetree/bindings/power/supply/battery.txt   | 43 ++++++++++++++++++++++
 1 file changed, 43 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/power/supply/battery.txt

Comments

Sebastian Reichel March 23, 2017, 10:20 a.m.
Hi,

On Mon, Mar 20, 2017 at 02:43:26AM -0700, Liam Breck wrote:
> From: Liam Breck <kernel@networkimprov.net>
> 
> Documentation of static battery characteristics that can be defined
> for batteries which cannot self-identify. This information is required
> by fuel-gauge and charger chips for proper handling of the battery.
> 
> Cc: Rob Herring <robh@kernel.org>
> Cc: devicetree@vger.kernel.org
> Signed-off-by: Matt Ranostay <matt@ranostay.consulting>
> Signed-off-by: Liam Breck <kernel@networkimprov.net>
> ---
>  .../devicetree/bindings/power/supply/battery.txt   | 43 ++++++++++++++++++++++
>  1 file changed, 43 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/power/supply/battery.txt
> 
> diff --git a/Documentation/devicetree/bindings/power/supply/battery.txt b/Documentation/devicetree/bindings/power/supply/battery.txt
> new file mode 100644
> index 0000000..53a68c0
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/power/supply/battery.txt
> @@ -0,0 +1,43 @@
> +Battery Characteristics
> +
> +The devicetree battery node provides static battery characteristics. 
> +In smart batteries, these are typically stored in non-volatile memory 
> +on a fuel gauge chip. The battery node should be used where there is 
> +no appropriate non-volatile memory, or it is unprogrammed/incorrect.
> +
> +Required Properties:
> + - compatible: Must be "simple-battery"
> +
> +Optional Properties:
> + - voltage-min-design-microvolt: drained battery voltage
> + - energy-full-design-microwatt-hours: battery design energy
> + - charge-full-design-microamp-hours: battery design capacity
> +
> +Battery properties are named, where possible, for the corresponding 
> +elements in enum power_supply_property, defined in
> +https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/linux/power_supply.h#n86

The above paragraph does not belong into DT bindings. Apart
from that

Acked-by: Sebastian Reichel <sre@kernel.org>

FYI: I will wait for Acked-by from Rob Herring on this patch.

-- Sebastian

> +Batteries must be referenced by chargers and/or fuel-gauges
> +using a phandle. The phandle's property should be named
> +"monitored-battery".
> +
> +Example:
> +
> +	bat: battery {
> +		compatible = "simple-battery";
> +		voltage-min-design-microvolt = <3200000>;
> +		energy-full-design-microwatt-hours = <5290000>;
> +		charge-full-design-microamp-hours = <1430000>;
> +	};
> +
> +	charger: charger@11 {
> +		....
> +		monitored-battery = <&bat>;
> +		...
> +	};
> +
> +	fuel_gauge: fuel-gauge@22 {
> +		....
> +		monitored-battery = <&bat>;
> +		...
> +	};
> -- 
> 2.9.3
>
Liam Breck March 23, 2017, 10:30 a.m.
On Thu, Mar 23, 2017 at 3:20 AM, Sebastian Reichel <sre@kernel.org> wrote:
> Hi,
>
> On Mon, Mar 20, 2017 at 02:43:26AM -0700, Liam Breck wrote:
>> From: Liam Breck <kernel@networkimprov.net>
>>
>> Documentation of static battery characteristics that can be defined
>> for batteries which cannot self-identify. This information is required
>> by fuel-gauge and charger chips for proper handling of the battery.
>>
>> Cc: Rob Herring <robh@kernel.org>
>> Cc: devicetree@vger.kernel.org
>> Signed-off-by: Matt Ranostay <matt@ranostay.consulting>
>> Signed-off-by: Liam Breck <kernel@networkimprov.net>
>> ---
>>  .../devicetree/bindings/power/supply/battery.txt   | 43 ++++++++++++++++++++++
>>  1 file changed, 43 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/power/supply/battery.txt
>>
>> diff --git a/Documentation/devicetree/bindings/power/supply/battery.txt b/Documentation/devicetree/bindings/power/supply/battery.txt
>> new file mode 100644
>> index 0000000..53a68c0
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/power/supply/battery.txt
>> @@ -0,0 +1,43 @@
>> +Battery Characteristics
>> +
>> +The devicetree battery node provides static battery characteristics.
>> +In smart batteries, these are typically stored in non-volatile memory
>> +on a fuel gauge chip. The battery node should be used where there is
>> +no appropriate non-volatile memory, or it is unprogrammed/incorrect.
>> +
>> +Required Properties:
>> + - compatible: Must be "simple-battery"
>> +
>> +Optional Properties:
>> + - voltage-min-design-microvolt: drained battery voltage
>> + - energy-full-design-microwatt-hours: battery design energy
>> + - charge-full-design-microamp-hours: battery design capacity
>> +
>> +Battery properties are named, where possible, for the corresponding
>> +elements in enum power_supply_property, defined in
>> +https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/linux/power_supply.h#n86
>
> The above paragraph does not belong into DT bindings. Apart
> from that

How then should I indicate that there is a method to the madness of
the above names? More information in the docs is helpful than less!

> Acked-by: Sebastian Reichel <sre@kernel.org>
>
> FYI: I will wait for Acked-by from Rob Herring on this patch.
>
> -- Sebastian
>
>> +Batteries must be referenced by chargers and/or fuel-gauges
>> +using a phandle. The phandle's property should be named
>> +"monitored-battery".
>> +
>> +Example:
>> +
>> +     bat: battery {
>> +             compatible = "simple-battery";
>> +             voltage-min-design-microvolt = <3200000>;
>> +             energy-full-design-microwatt-hours = <5290000>;
>> +             charge-full-design-microamp-hours = <1430000>;
>> +     };
>> +
>> +     charger: charger@11 {
>> +             ....
>> +             monitored-battery = <&bat>;
>> +             ...
>> +     };
>> +
>> +     fuel_gauge: fuel-gauge@22 {
>> +             ....
>> +             monitored-battery = <&bat>;
>> +             ...
>> +     };
>> --
>> 2.9.3
>>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Sebastian Reichel March 23, 2017, 12:18 p.m.
Hi,

On Thu, Mar 23, 2017 at 03:30:42AM -0700, Liam Breck wrote:
> On Thu, Mar 23, 2017 at 3:20 AM, Sebastian Reichel <sre@kernel.org> wrote:
> > Hi,
> >
> > On Mon, Mar 20, 2017 at 02:43:26AM -0700, Liam Breck wrote:
> >> From: Liam Breck <kernel@networkimprov.net>
> >>
> >> Documentation of static battery characteristics that can be defined
> >> for batteries which cannot self-identify. This information is required
> >> by fuel-gauge and charger chips for proper handling of the battery.
> >>
> >> Cc: Rob Herring <robh@kernel.org>
> >> Cc: devicetree@vger.kernel.org
> >> Signed-off-by: Matt Ranostay <matt@ranostay.consulting>
> >> Signed-off-by: Liam Breck <kernel@networkimprov.net>
> >> ---
> >>  .../devicetree/bindings/power/supply/battery.txt   | 43 ++++++++++++++++++++++
> >>  1 file changed, 43 insertions(+)
> >>  create mode 100644 Documentation/devicetree/bindings/power/supply/battery.txt
> >>
> >> diff --git a/Documentation/devicetree/bindings/power/supply/battery.txt b/Documentation/devicetree/bindings/power/supply/battery.txt
> >> new file mode 100644
> >> index 0000000..53a68c0
> >> --- /dev/null
> >> +++ b/Documentation/devicetree/bindings/power/supply/battery.txt
> >> @@ -0,0 +1,43 @@
> >> +Battery Characteristics
> >> +
> >> +The devicetree battery node provides static battery characteristics.
> >> +In smart batteries, these are typically stored in non-volatile memory
> >> +on a fuel gauge chip. The battery node should be used where there is
> >> +no appropriate non-volatile memory, or it is unprogrammed/incorrect.
> >> +
> >> +Required Properties:
> >> + - compatible: Must be "simple-battery"
> >> +
> >> +Optional Properties:
> >> + - voltage-min-design-microvolt: drained battery voltage
> >> + - energy-full-design-microwatt-hours: battery design energy
> >> + - charge-full-design-microamp-hours: battery design capacity
> >> +
> >> +Battery properties are named, where possible, for the corresponding
> >> +elements in enum power_supply_property, defined in
> >> +https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/linux/power_supply.h#n86
> >
> > The above paragraph does not belong into DT bindings. Apart
> > from that
> >
> > Acked-by: Sebastian Reichel <sre@kernel.org>
> 
> How then should I indicate that there is a method to the madness of
> the above names? More information in the docs is helpful than less!

You don't. This does not belong into the DT binding document. DT
binding documents are _not_ Linux FW API. I thought we were through
this already. FWIW if anything is not named like the power-supply
subsystem names it, then that's not nice, but ok. Also if anybody
adds stuff he will 

1. send patches to the power-supply subsystem maintainer (me at
   the moment), who can reject patches.
2. implement the parsing in the driver, which asks for power-supply
   naming to be used if possible.

-- Sebastian

> > FYI: I will wait for Acked-by from Rob Herring on this patch.
> >
> > -- Sebastian
> >
> >> +Batteries must be referenced by chargers and/or fuel-gauges
> >> +using a phandle. The phandle's property should be named
> >> +"monitored-battery".
> >> +
> >> +Example:
> >> +
> >> +     bat: battery {
> >> +             compatible = "simple-battery";
> >> +             voltage-min-design-microvolt = <3200000>;
> >> +             energy-full-design-microwatt-hours = <5290000>;
> >> +             charge-full-design-microamp-hours = <1430000>;
> >> +     };
> >> +
> >> +     charger: charger@11 {
> >> +             ....
> >> +             monitored-battery = <&bat>;
> >> +             ...
> >> +     };
> >> +
> >> +     fuel_gauge: fuel-gauge@22 {
> >> +             ....
> >> +             monitored-battery = <&bat>;
> >> +             ...
> >> +     };
> >> --
> >> 2.9.3
> >>
Rob Herring March 24, 2017, 3:55 p.m.
On Mon, Mar 20, 2017 at 02:43:26AM -0700, Liam Breck wrote:
> From: Liam Breck <kernel@networkimprov.net>
> 
> Documentation of static battery characteristics that can be defined
> for batteries which cannot self-identify. This information is required
> by fuel-gauge and charger chips for proper handling of the battery.
> 
> Cc: Rob Herring <robh@kernel.org>
> Cc: devicetree@vger.kernel.org
> Signed-off-by: Matt Ranostay <matt@ranostay.consulting>
> Signed-off-by: Liam Breck <kernel@networkimprov.net>
> ---
>  .../devicetree/bindings/power/supply/battery.txt   | 43 ++++++++++++++++++++++
>  1 file changed, 43 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/power/supply/battery.txt

While "simple" is not generally something I like to see in a compatible 
string, we've beat this one to death.

Acked-by: Rob Herring <robh@kernel.org>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Liam Breck March 24, 2017, 9:11 p.m.
Hi Rob,

On Fri, Mar 24, 2017 at 8:55 AM, Rob Herring <robh@kernel.org> wrote:
> On Mon, Mar 20, 2017 at 02:43:26AM -0700, Liam Breck wrote:
>> From: Liam Breck <kernel@networkimprov.net>
>>
>> Documentation of static battery characteristics that can be defined
>> for batteries which cannot self-identify. This information is required
>> by fuel-gauge and charger chips for proper handling of the battery.
>>
>> Cc: Rob Herring <robh@kernel.org>
>> Cc: devicetree@vger.kernel.org
>> Signed-off-by: Matt Ranostay <matt@ranostay.consulting>
>> Signed-off-by: Liam Breck <kernel@networkimprov.net>
>> ---
>>  .../devicetree/bindings/power/supply/battery.txt   | 43 ++++++++++++++++++++++
>>  1 file changed, 43 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/power/supply/battery.txt
>
> While "simple" is not generally something I like to see in a compatible
> string, we've beat this one to death.
>
> Acked-by: Rob Herring <robh@kernel.org>

Thanks. Are you in agreement with Sebastian on the following, or is
this OK to include?

On Thu, Mar 23, 2017 at 3:20 AM, Sebastian Reichel <sre@kernel.org> wrote:
>> +Battery properties are named, where possible, for the corresponding
>> +elements in enum power_supply_property, defined in
>> +https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/linux/power_supply.h#n86
>
>The above paragraph does not belong into DT bindings.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Liam Breck April 7, 2017, 7:23 p.m.
[+CC Tony Lindgren]

Rob & Sebastian & Tony & Andrew,

With this binding we allow a DT to hard-code battery characteristics.
But mainline dts files for boards & boxes really should NOT include
immutable battery properties unless the battery is inseparable from
the electronics. We want to prevent unpredictable behavior due to
incorrect DT properties after a user changes a battery to a different
type.

It's OK for device vendors to hard-code battery characteristics in DT,
but mainline dts is another matter.

If this is agreed, how should we document it?


On Mon, Mar 20, 2017 at 2:43 AM, Liam Breck <liam@networkimprov.net> wrote:
> From: Liam Breck <kernel@networkimprov.net>
>
> Documentation of static battery characteristics that can be defined
> for batteries which cannot self-identify. This information is required
> by fuel-gauge and charger chips for proper handling of the battery.
>
> Cc: Rob Herring <robh@kernel.org>
> Cc: devicetree@vger.kernel.org
> Signed-off-by: Matt Ranostay <matt@ranostay.consulting>
> Signed-off-by: Liam Breck <kernel@networkimprov.net>
> ---
>  .../devicetree/bindings/power/supply/battery.txt   | 43 ++++++++++++++++++++++
>  1 file changed, 43 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/power/supply/battery.txt
>
> diff --git a/Documentation/devicetree/bindings/power/supply/battery.txt b/Documentation/devicetree/bindings/power/supply/battery.txt
> new file mode 100644
> index 0000000..53a68c0
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/power/supply/battery.txt
> @@ -0,0 +1,43 @@
> +Battery Characteristics
> +
> +The devicetree battery node provides static battery characteristics.
> +In smart batteries, these are typically stored in non-volatile memory
> +on a fuel gauge chip. The battery node should be used where there is
> +no appropriate non-volatile memory, or it is unprogrammed/incorrect.
> +
> +Required Properties:
> + - compatible: Must be "simple-battery"
> +
> +Optional Properties:
> + - voltage-min-design-microvolt: drained battery voltage
> + - energy-full-design-microwatt-hours: battery design energy
> + - charge-full-design-microamp-hours: battery design capacity
> +
> +Battery properties are named, where possible, for the corresponding
> +elements in enum power_supply_property, defined in
> +https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/linux/power_supply.h#n86
> +
> +Batteries must be referenced by chargers and/or fuel-gauges
> +using a phandle. The phandle's property should be named
> +"monitored-battery".
> +
> +Example:
> +
> +       bat: battery {
> +               compatible = "simple-battery";
> +               voltage-min-design-microvolt = <3200000>;
> +               energy-full-design-microwatt-hours = <5290000>;
> +               charge-full-design-microamp-hours = <1430000>;
> +       };
> +
> +       charger: charger@11 {
> +               ....
> +               monitored-battery = <&bat>;
> +               ...
> +       };
> +
> +       fuel_gauge: fuel-gauge@22 {
> +               ....
> +               monitored-battery = <&bat>;
> +               ...
> +       };
> --
> 2.9.3
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Sebastian Reichel April 14, 2017, 12:33 a.m.
Hi,

On Fri, Apr 07, 2017 at 12:23:29PM -0700, Liam Breck wrote:
> [+CC Tony Lindgren]
> 
> Rob & Sebastian & Tony & Andrew,
> 
> With this binding we allow a DT to hard-code battery characteristics.
> But mainline dts files for boards & boxes really should NOT include
> immutable battery properties unless the battery is inseparable from
> the electronics. We want to prevent unpredictable behavior due to
> incorrect DT properties after a user changes a battery to a different
> type.
> 
> It's OK for device vendors to hard-code battery characteristics in DT,
> but mainline dts is another matter.
> 
> If this is agreed, how should we document it?

That's not a problem of mainline vs vendor. The binding may only be
used for batteries, that cannot be exchanged with a different type.
That's why I originally suggested the "fixed-battery" name. IMHO
it would be ok to use the binding also for DTS files distributed
with the mainline kernel as long as the described device provides
some kind of protection against using batteries of a different type.

For example I would feel ok adding a simple-battery node for the
Droid 4 (which probably does not need one), since the battery has
a custom form-factor, is screwed to the device and has a custom
connector. So while it can be replaced, the new battery would have
the same technical specs.

I suggest to add something similar as the following to the binding:

Using incorrect battery parameters is a SAFETY HAZARD - If used
incorrectly, Lithium based batteries can explode. You should be
really sure, that all provided values are correct. Also if the
battery in your device can be replaced with a different battery
(without major hacks), you may not use this binding for safety
reasons.

-- Sebastian

> On Mon, Mar 20, 2017 at 2:43 AM, Liam Breck <liam@networkimprov.net> wrote:
> > From: Liam Breck <kernel@networkimprov.net>
> >
> > Documentation of static battery characteristics that can be defined
> > for batteries which cannot self-identify. This information is required
> > by fuel-gauge and charger chips for proper handling of the battery.
> >
> > Cc: Rob Herring <robh@kernel.org>
> > Cc: devicetree@vger.kernel.org
> > Signed-off-by: Matt Ranostay <matt@ranostay.consulting>
> > Signed-off-by: Liam Breck <kernel@networkimprov.net>
> > ---
> >  .../devicetree/bindings/power/supply/battery.txt   | 43 ++++++++++++++++++++++
> >  1 file changed, 43 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/power/supply/battery.txt
> >
> > diff --git a/Documentation/devicetree/bindings/power/supply/battery.txt b/Documentation/devicetree/bindings/power/supply/battery.txt
> > new file mode 100644
> > index 0000000..53a68c0
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/power/supply/battery.txt
> > @@ -0,0 +1,43 @@
> > +Battery Characteristics
> > +
> > +The devicetree battery node provides static battery characteristics.
> > +In smart batteries, these are typically stored in non-volatile memory
> > +on a fuel gauge chip. The battery node should be used where there is
> > +no appropriate non-volatile memory, or it is unprogrammed/incorrect.
> > +
> > +Required Properties:
> > + - compatible: Must be "simple-battery"
> > +
> > +Optional Properties:
> > + - voltage-min-design-microvolt: drained battery voltage
> > + - energy-full-design-microwatt-hours: battery design energy
> > + - charge-full-design-microamp-hours: battery design capacity
> > +
> > +Battery properties are named, where possible, for the corresponding
> > +elements in enum power_supply_property, defined in
> > +https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/linux/power_supply.h#n86
> > +
> > +Batteries must be referenced by chargers and/or fuel-gauges
> > +using a phandle. The phandle's property should be named
> > +"monitored-battery".
> > +
> > +Example:
> > +
> > +       bat: battery {
> > +               compatible = "simple-battery";
> > +               voltage-min-design-microvolt = <3200000>;
> > +               energy-full-design-microwatt-hours = <5290000>;
> > +               charge-full-design-microamp-hours = <1430000>;
> > +       };
> > +
> > +       charger: charger@11 {
> > +               ....
> > +               monitored-battery = <&bat>;
> > +               ...
> > +       };
> > +
> > +       fuel_gauge: fuel-gauge@22 {
> > +               ....
> > +               monitored-battery = <&bat>;
> > +               ...
> > +       };
> > --
> > 2.9.3
> >

Patch hide | download patch | download mbox

diff --git a/Documentation/devicetree/bindings/power/supply/battery.txt b/Documentation/devicetree/bindings/power/supply/battery.txt
new file mode 100644
index 0000000..53a68c0
--- /dev/null
+++ b/Documentation/devicetree/bindings/power/supply/battery.txt
@@ -0,0 +1,43 @@ 
+Battery Characteristics
+
+The devicetree battery node provides static battery characteristics. 
+In smart batteries, these are typically stored in non-volatile memory 
+on a fuel gauge chip. The battery node should be used where there is 
+no appropriate non-volatile memory, or it is unprogrammed/incorrect.
+
+Required Properties:
+ - compatible: Must be "simple-battery"
+
+Optional Properties:
+ - voltage-min-design-microvolt: drained battery voltage
+ - energy-full-design-microwatt-hours: battery design energy
+ - charge-full-design-microamp-hours: battery design capacity
+
+Battery properties are named, where possible, for the corresponding 
+elements in enum power_supply_property, defined in
+https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/linux/power_supply.h#n86
+
+Batteries must be referenced by chargers and/or fuel-gauges
+using a phandle. The phandle's property should be named
+"monitored-battery".
+
+Example:
+
+	bat: battery {
+		compatible = "simple-battery";
+		voltage-min-design-microvolt = <3200000>;
+		energy-full-design-microwatt-hours = <5290000>;
+		charge-full-design-microamp-hours = <1430000>;
+	};
+
+	charger: charger@11 {
+		....
+		monitored-battery = <&bat>;
+		...
+	};
+
+	fuel_gauge: fuel-gauge@22 {
+		....
+		monitored-battery = <&bat>;
+		...
+	};