Re: [PATCH v10 1/8] devicetree: power: Add battery.txt

[Sebastian Reichel <sre@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 3066 bytes --]

Hi Liam & Matt,

Sorry for my absence in the discussion. I skipped the previous
iterations for now and start with this version.

On Wed, Mar 15, 2017 at 12:26:46PM -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   | 45 ++++++++++++++++++++++
>  1 file changed, 45 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..0278617
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/power/supply/battery.txt
> @@ -0,0 +1,45 @@
> +Battery Characteristics

Maybe add something like

This device provides static battery information, that is usually
available in the EEPROM of a smart battery. It's supposed to be
used for batteries, which do not have their own EEPROM (or if its
unusable).

> +Required Properties:
> + - compatible: Must be "fixed-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

Looks fine to me.

> +Because drivers surface properties in sysfs using names derived
> +from enum power_supply_property, e.g.
> +/sys/class/power_supply/<device>/charge_full_design, our
> +battery properties must be named for the corresponding elements in
> +enum power_supply_property, defined in include/linux/power_supply.h.

This is Linux/implementation specific and does not belong
into a DT binding document.

> +Batteries must be referenced by chargers and/or fuel-gauges
> +using a phandle. The phandle's property should be named
> +"monitored-battery".

This looks fine.

> +Driver code should call power_supply_get_battery_info() to obtain
> +battery properties via monitored-battery. For details see:
> +  drivers/power/supply/power_supply_core.c
> +  drivers/power/supply/bq27xxx_battery.c

This is also Linux/implementation specific and should be dropped.

> +Example:
> +
> +	bat: battery {
> +		compatible = "fixed-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
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]