| Message ID | 20170122222212.27086-1-liam@networkimprov.net |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
On Sun, Jan 22, 2017 at 2:22 PM, Liam Breck <liam@networkimprov.net> wrote: > I think Matt meant the following :-) > > Note: nominal-microvolt is not the correct term for termination voltage. > Changed to termination-microvolt > --- > > Documentation on battery properties that can be defined for > fine tuning fuel gauge state machines. > > From: Matt Ranostay <matt@ranostay.consulting> > Cc: Rob Herring <robh@kernel.org> > Cc: devicetree@vger.kernel.org > Signed-off-by: Matt Ranostay <matt@ranostay.consulting> > --- > .../devicetree/bindings/power/supply/battery.txt | 34 ++++++++++++++++++++++ > 1 file changed, 34 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 000000000000..398b4d622883 > --- /dev/null > +++ b/Documentation/devicetree/bindings/power/supply/battery.txt > @@ -0,0 +1,34 @@ > +Battery Characteristics > + > +Required Properties: > + - compatible: Must be "fixed-battery" > + > +Optional Properties: > + - termination-microvolt: dead battery voltage > + - design-microwatt-hours: battery design energy > + - design-microamp-hours: battery design capacity Two more battery characteristics should be included. Battery chargers need them (for instance, BQ24190). - precharge-microamps: maximum charge current during precharge phase (typically 20% of battery capacity) - termination-microamps: a charge cycle terminates when the battery voltage is above recharge threshold, and the current is below this setting (typically 10% of battery capacity) Perhaps termination-microvolt should be called terminal-microvolt or similar to avoid confusion. Sebastian, did you want to include the other fields from your example? /* Nokia BL-5J */ nominal-microvolt = <3700000>; chemistry-type = <POWER_SUPPLY_LI_ION>; -- 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
On Sun, Jan 22, 2017 at 2:22 PM, Liam Breck <liam@networkimprov.net> wrote: > diff --git a/Documentation/devicetree/bindings/power/supply/battery.txt b/Documentation/devicetree/bindings/power/supply/battery.txt > new file mode 100644 > index 000000000000..398b4d622883 > --- /dev/null > +++ b/Documentation/devicetree/bindings/power/supply/battery.txt > @@ -0,0 +1,34 @@ > +Battery Characteristics > + > +Required Properties: > + - compatible: Must be "fixed-battery" > + > +Optional Properties: > + - termination-microvolt: dead battery voltage > + - design-microwatt-hours: battery design energy > + - design-microamp-hours: battery design capacity Also I suspect the members of struct power_supply_battery_info should use the same names as appear in dts: +struct power_supply_battery_info { + int design_microwatt_hours; + int design_microamp_hours; + int termination_microvolt; etc -- 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
On Wed, Jan 25, 2017 at 10:19 PM, Liam Breck <liam@networkimprov.net> wrote: > On Sun, Jan 22, 2017 at 2:22 PM, Liam Breck <liam@networkimprov.net> wrote: > >> diff --git a/Documentation/devicetree/bindings/power/supply/battery.txt b/Documentation/devicetree/bindings/power/supply/battery.txt >> new file mode 100644 >> index 000000000000..398b4d622883 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/power/supply/battery.txt >> @@ -0,0 +1,34 @@ >> +Battery Characteristics >> + >> +Required Properties: >> + - compatible: Must be "fixed-battery" >> + >> +Optional Properties: >> + - termination-microvolt: dead battery voltage >> + - design-microwatt-hours: battery design energy >> + - design-microamp-hours: battery design capacity > > Also I suspect the members of struct power_supply_battery_info should > use the same names as appear in dts: These are internal and we can't be thinking just device tree.. ACPI and platform data is also an option. > > +struct power_supply_battery_info { > + int design_microwatt_hours; > + int design_microamp_hours; > + int termination_microvolt; > etc -- 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
Hi, On Wed, Jan 25, 2017 at 11:02:03PM -0800, Matt Ranostay wrote: > On Wed, Jan 25, 2017 at 10:19 PM, Liam Breck <liam@networkimprov.net> wrote: > > On Sun, Jan 22, 2017 at 2:22 PM, Liam Breck <liam@networkimprov.net> wrote: > > > >> diff --git a/Documentation/devicetree/bindings/power/supply/battery.txt b/Documentation/devicetree/bindings/power/supply/battery.txt > >> new file mode 100644 > >> index 000000000000..398b4d622883 > >> --- /dev/null > >> +++ b/Documentation/devicetree/bindings/power/supply/battery.txt > >> @@ -0,0 +1,34 @@ > >> +Battery Characteristics > >> + > >> +Required Properties: > >> + - compatible: Must be "fixed-battery" > >> + > >> +Optional Properties: > >> + - termination-microvolt: dead battery voltage > >> + - design-microwatt-hours: battery design energy > >> + - design-microamp-hours: battery design capacity > > > > Also I suspect the members of struct power_supply_battery_info should > > use the same names as appear in dts: > > These are internal and we can't be thinking just device tree.. ACPI > and platform data is also an option. well platform data just uses the struct. ACPI is probably not relevant, since in ACPI world one usually has smart batteries. But yes, the names can be different. The important part is, that the API is used correctly, so it should be clear what each property is used for. For example termination_microvolt is not clear: Is this charge termination voltage or system shutdown voltage? Also we do not need to add "micro", since the power-supply subsystem always uses micro based units. -- Sebastian
Hi, On Tue, Jan 24, 2017 at 11:56:33AM -0800, Liam Breck wrote: > On Sun, Jan 22, 2017 at 2:22 PM, Liam Breck <liam@networkimprov.net> wrote: > > I think Matt meant the following :-) > > > > Note: nominal-microvolt is not the correct term for termination voltage. > > Changed to termination-microvolt > > --- > > > > Documentation on battery properties that can be defined for > > fine tuning fuel gauge state machines. > > > > From: Matt Ranostay <matt@ranostay.consulting> > > Cc: Rob Herring <robh@kernel.org> > > Cc: devicetree@vger.kernel.org > > Signed-off-by: Matt Ranostay <matt@ranostay.consulting> > > --- > > .../devicetree/bindings/power/supply/battery.txt | 34 ++++++++++++++++++++++ > > 1 file changed, 34 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 000000000000..398b4d622883 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/power/supply/battery.txt > > @@ -0,0 +1,34 @@ > > +Battery Characteristics > > + > > +Required Properties: > > + - compatible: Must be "fixed-battery" > > + > > +Optional Properties: > > + - termination-microvolt: dead battery voltage > > + - design-microwatt-hours: battery design energy > > + - design-microamp-hours: battery design capacity > > > Two more battery characteristics should be included. Battery chargers > need them (for instance, BQ24190). > > - precharge-microamps: maximum charge current during precharge > phase (typically 20% of battery capacity) > > - termination-microamps: a charge cycle terminates when the > battery voltage is above recharge threshold, and the current is below > this setting (typically 10% of battery capacity) I guess quite a few battery characteristics could be added. Let's do this once we have a user for them. > Perhaps termination-microvolt should be called terminal-microvolt or > similar to avoid confusion. > > Sebastian, did you want to include the other fields from your example? Let's add properties once we have users for them. I just used the stuff printed on my Nokia N900's battery as an example :) I guess chemistry-type may actually be useful already. I think at least some bq27xxx chips would also work with LiPo based batteries. -- Sebastian
On Sun, Jan 22, 2017 at 02:22:12PM -0800, Liam Breck wrote: > I think Matt meant the following :-) > > Note: nominal-microvolt is not the correct term for termination voltage. > Changed to termination-microvolt That's right. > Documentation on battery properties that can be defined for > fine tuning fuel gauge state machines. Maybe: Documentation for batteries, that cannot identify themself. The information is required by fuel-gauge and charger chips for proper handling of the battery. > From: Matt Ranostay <matt@ranostay.consulting> > Cc: Rob Herring <robh@kernel.org> > Cc: devicetree@vger.kernel.org > Signed-off-by: Matt Ranostay <matt@ranostay.consulting> > --- > .../devicetree/bindings/power/supply/battery.txt | 34 ++++++++++++++++++++++ > 1 file changed, 34 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 000000000000..398b4d622883 > --- /dev/null > +++ b/Documentation/devicetree/bindings/power/supply/battery.txt > @@ -0,0 +1,34 @@ > +Battery Characteristics > + > +Required Properties: > + - compatible: Must be "fixed-battery" > + > +Optional Properties: > + - termination-microvolt: dead battery voltage I think this is not named optimally, since it's not clear if its related to charging. > + - design-microwatt-hours: battery design energy > + - design-microamp-hours: battery design capacity > + > +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 = "fixed-battery"; > + terminate-microvolt = <3700000>; > + design-microwatt-hours = <5290000>; > + design-microamp-hours = <1430000>; > + }; > + > + charger: charger@0 { > + .... > + monitored-battery = <&bat>; > + ... > + }; > + > + fuel_gauge: fuel_gauge@0 { > + .... > + monitored-battery = <&bat>; > + ... > + }; The charger stuff does does not integrate well with pre-existing support for power-supplies property described in Documentation/devicetree/bindings/power/supply/power_supply.txt I think the proper chain would be: bat: battery { compatible = "fixed-battery"; terminate-microvolt = <3700000>; design-microwatt-hours = <5290000>; design-microamp-hours = <1430000>; }; fuel_gauge: fuel_gauge@0 { ... monitored-battery = <&bat>; power-supplies = <&charger>; ... }; charger: charger@0 { ... }; I added the power-supplies node to the fuel-gauge instead of the battery, since fuel-gauge + fixed-battery is basically a smart battery. -- Sebastian
On Sun, Jan 29, 2017 at 9:20 AM, Sebastian Reichel <sre@kernel.org> wrote: > Hi, > > On Wed, Jan 25, 2017 at 11:02:03PM -0800, Matt Ranostay wrote: >> On Wed, Jan 25, 2017 at 10:19 PM, Liam Breck <liam@networkimprov.net> wrote: >> > On Sun, Jan 22, 2017 at 2:22 PM, Liam Breck <liam@networkimprov.net> wrote: >> > >> >> diff --git a/Documentation/devicetree/bindings/power/supply/battery.txt b/Documentation/devicetree/bindings/power/supply/battery.txt >> >> new file mode 100644 >> >> index 000000000000..398b4d622883 >> >> --- /dev/null >> >> +++ b/Documentation/devicetree/bindings/power/supply/battery.txt >> >> @@ -0,0 +1,34 @@ >> >> +Battery Characteristics >> >> + >> >> +Required Properties: >> >> + - compatible: Must be "fixed-battery" >> >> + >> >> +Optional Properties: >> >> + - termination-microvolt: dead battery voltage >> >> + - design-microwatt-hours: battery design energy >> >> + - design-microamp-hours: battery design capacity >> > >> > Also I suspect the members of struct power_supply_battery_info should >> > use the same names as appear in dts: >> >> These are internal and we can't be thinking just device tree.. ACPI >> and platform data is also an option. > > well platform data just uses the struct. ACPI is probably not > relevant, since in ACPI world one usually has smart batteries. > But yes, the names can be different. > > The important part is, that the API is used correctly, so it > should be clear what each property is used for. For example > termination_microvolt is not clear: Is this charge termination > voltage or system shutdown voltage? Also we do not need to add > "micro", since the power-supply subsystem always uses micro > based units. It's the min battery voltage. So... drained_voltage, depleted_voltage, sapped_voltage, spent_voltage... Help me out here :-) -- 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
On Sun, Jan 29, 2017 at 10:06 AM, Sebastian Reichel <sre@kernel.org> wrote: > On Sun, Jan 22, 2017 at 02:22:12PM -0800, Liam Breck wrote: ... >> +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 = "fixed-battery"; >> + terminate-microvolt = <3700000>; >> + design-microwatt-hours = <5290000>; >> + design-microamp-hours = <1430000>; >> + }; >> + >> + charger: charger@0 { >> + .... >> + monitored-battery = <&bat>; >> + ... >> + }; >> + >> + fuel_gauge: fuel_gauge@0 { >> + .... >> + monitored-battery = <&bat>; >> + ... >> + }; > > The charger stuff does does not integrate well with > pre-existing support for power-supplies property > described in > Documentation/devicetree/bindings/power/supply/power_supply.txt > > I think the proper chain would be: > > bat: battery { > compatible = "fixed-battery"; > terminate-microvolt = <3700000>; > design-microwatt-hours = <5290000>; > design-microamp-hours = <1430000>; > }; > > fuel_gauge: fuel_gauge@0 { > ... > monitored-battery = <&bat>; > power-supplies = <&charger>; > ... > }; > > charger: charger@0 { > ... > }; > > I added the power-supplies node to the fuel-gauge instead of the battery, > since fuel-gauge + fixed-battery is basically a smart battery. BQ24190 charger will need other params from a battery node. How does it get them in this scenario? -- 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
Hi, On Sun, Jan 29, 2017 at 03:22:31PM -0800, Liam Breck wrote: > On Sun, Jan 29, 2017 at 9:20 AM, Sebastian Reichel <sre@kernel.org> wrote: > > On Wed, Jan 25, 2017 at 11:02:03PM -0800, Matt Ranostay wrote: > >> On Wed, Jan 25, 2017 at 10:19 PM, Liam Breck <liam@networkimprov.net> wrote: > >> > On Sun, Jan 22, 2017 at 2:22 PM, Liam Breck <liam@networkimprov.net> wrote: > >> > > >> >> diff --git a/Documentation/devicetree/bindings/power/supply/battery.txt b/Documentation/devicetree/bindings/power/supply/battery.txt > >> >> new file mode 100644 > >> >> index 000000000000..398b4d622883 > >> >> --- /dev/null > >> >> +++ b/Documentation/devicetree/bindings/power/supply/battery.txt > >> >> @@ -0,0 +1,34 @@ > >> >> +Battery Characteristics > >> >> + > >> >> +Required Properties: > >> >> + - compatible: Must be "fixed-battery" > >> >> + > >> >> +Optional Properties: > >> >> + - termination-microvolt: dead battery voltage > >> >> + - design-microwatt-hours: battery design energy > >> >> + - design-microamp-hours: battery design capacity > >> > > >> > Also I suspect the members of struct power_supply_battery_info should > >> > use the same names as appear in dts: > >> > >> These are internal and we can't be thinking just device tree.. ACPI > >> and platform data is also an option. > > > > well platform data just uses the struct. ACPI is probably not > > relevant, since in ACPI world one usually has smart batteries. > > But yes, the names can be different. > > > > The important part is, that the API is used correctly, so it > > should be clear what each property is used for. For example > > termination_microvolt is not clear: Is this charge termination > > voltage or system shutdown voltage? Also we do not need to add > > "micro", since the power-supply subsystem always uses micro > > based units. > > It's the min battery voltage. So... > drained_voltage, depleted_voltage, sapped_voltage, spent_voltage... This is often called EOD (end of discharge) voltage, so let's use end_of_discharge_voltage. -- Sebastian
Hi, On Sun, Jan 29, 2017 at 03:32:10PM -0800, Liam Breck wrote: > On Sun, Jan 29, 2017 at 10:06 AM, Sebastian Reichel <sre@kernel.org> wrote: > > On Sun, Jan 22, 2017 at 02:22:12PM -0800, Liam Breck wrote: > ... > >> +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 = "fixed-battery"; > >> + terminate-microvolt = <3700000>; > >> + design-microwatt-hours = <5290000>; > >> + design-microamp-hours = <1430000>; > >> + }; > >> + > >> + charger: charger@0 { > >> + .... > >> + monitored-battery = <&bat>; > >> + ... > >> + }; > >> + > >> + fuel_gauge: fuel_gauge@0 { > >> + .... > >> + monitored-battery = <&bat>; > >> + ... > >> + }; > > > > The charger stuff does does not integrate well with > > pre-existing support for power-supplies property > > described in > > Documentation/devicetree/bindings/power/supply/power_supply.txt > > > > I think the proper chain would be: > > > > bat: battery { > > compatible = "fixed-battery"; > > terminate-microvolt = <3700000>; > > design-microwatt-hours = <5290000>; > > design-microamp-hours = <1430000>; > > }; > > > > fuel_gauge: fuel_gauge@0 { > > ... > > monitored-battery = <&bat>; > > power-supplies = <&charger>; > > ... > > }; > > > > charger: charger@0 { > > ... > > }; > > > > I added the power-supplies node to the fuel-gauge instead of the battery, > > since fuel-gauge + fixed-battery is basically a smart battery. > > BQ24190 charger will need other params from a battery node. How does > it get them in this scenario? It should be able to get all required properties from the bq27xxx. Otherwise supporting smart batteries is impossible. What properties do you need? -- Sebastian
On Sun, Jan 29, 2017 at 6:39 PM, Sebastian Reichel <sre@kernel.org> wrote: > Hi, > > On Sun, Jan 29, 2017 at 03:32:10PM -0800, Liam Breck wrote: >> On Sun, Jan 29, 2017 at 10:06 AM, Sebastian Reichel <sre@kernel.org> wrote: >> > On Sun, Jan 22, 2017 at 02:22:12PM -0800, Liam Breck wrote: >> ... >> >> +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 = "fixed-battery"; >> >> + terminate-microvolt = <3700000>; >> >> + design-microwatt-hours = <5290000>; >> >> + design-microamp-hours = <1430000>; >> >> + }; >> >> + >> >> + charger: charger@0 { >> >> + .... >> >> + monitored-battery = <&bat>; >> >> + ... >> >> + }; >> >> + >> >> + fuel_gauge: fuel_gauge@0 { >> >> + .... >> >> + monitored-battery = <&bat>; >> >> + ... >> >> + }; >> > >> > The charger stuff does does not integrate well with >> > pre-existing support for power-supplies property >> > described in >> > Documentation/devicetree/bindings/power/supply/power_supply.txt >> > >> > I think the proper chain would be: >> > >> > bat: battery { >> > compatible = "fixed-battery"; >> > terminate-microvolt = <3700000>; >> > design-microwatt-hours = <5290000>; >> > design-microamp-hours = <1430000>; >> > }; >> > >> > fuel_gauge: fuel_gauge@0 { >> > ... >> > monitored-battery = <&bat>; >> > power-supplies = <&charger>; >> > ... >> > }; >> > >> > charger: charger@0 { >> > ... >> > }; >> > >> > I added the power-supplies node to the fuel-gauge instead of the battery, >> > since fuel-gauge + fixed-battery is basically a smart battery. >> >> BQ24190 charger will need other params from a battery node. How does >> it get them in this scenario? > > It should be able to get all required properties from the bq27xxx. > Otherwise supporting smart batteries is impossible. What properties > do you need? These two, at a minimum: - precharge-microamps: maximum charge current during precharge phase (typically 20% of battery capacity) - termination-microamps: a charge cycle terminates when the battery voltage is above recharge threshold, and the current is below this setting (typically 10% of battery capacity) Sorry, I'm not seeing how the charger will obtain these from the fuel gauge. Could you clarify with a code snippet? -- 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
On Sun, Jan 29, 2017 at 6:30 PM, Sebastian Reichel <sre@kernel.org> wrote: > Hi, > > On Sun, Jan 29, 2017 at 03:22:31PM -0800, Liam Breck wrote: >> On Sun, Jan 29, 2017 at 9:20 AM, Sebastian Reichel <sre@kernel.org> wrote: >> > On Wed, Jan 25, 2017 at 11:02:03PM -0800, Matt Ranostay wrote: >> >> On Wed, Jan 25, 2017 at 10:19 PM, Liam Breck <liam@networkimprov.net> wrote: >> >> > On Sun, Jan 22, 2017 at 2:22 PM, Liam Breck <liam@networkimprov.net> wrote: >> >> > >> >> >> diff --git a/Documentation/devicetree/bindings/power/supply/battery.txt b/Documentation/devicetree/bindings/power/supply/battery.txt >> >> >> new file mode 100644 >> >> >> index 000000000000..398b4d622883 >> >> >> --- /dev/null >> >> >> +++ b/Documentation/devicetree/bindings/power/supply/battery.txt >> >> >> @@ -0,0 +1,34 @@ >> >> >> +Battery Characteristics >> >> >> + >> >> >> +Required Properties: >> >> >> + - compatible: Must be "fixed-battery" >> >> >> + >> >> >> +Optional Properties: >> >> >> + - termination-microvolt: dead battery voltage >> >> >> + - design-microwatt-hours: battery design energy >> >> >> + - design-microamp-hours: battery design capacity >> >> > >> >> > Also I suspect the members of struct power_supply_battery_info should >> >> > use the same names as appear in dts: >> >> >> >> These are internal and we can't be thinking just device tree.. ACPI >> >> and platform data is also an option. >> > >> > well platform data just uses the struct. ACPI is probably not >> > relevant, since in ACPI world one usually has smart batteries. >> > But yes, the names can be different. >> > >> > The important part is, that the API is used correctly, so it >> > should be clear what each property is used for. For example >> > termination_microvolt is not clear: Is this charge termination >> > voltage or system shutdown voltage? Also we do not need to add >> > "micro", since the power-supply subsystem always uses micro >> > based units. >> >> It's the min battery voltage. So... >> drained_voltage, depleted_voltage, sapped_voltage, spent_voltage... > > This is often called EOD (end of discharge) voltage, so let's use > end_of_discharge_voltage. How's this? Optional Properties: - end-of-discharge-microvolt: dead battery voltage - design-microwatt-hours: battery design energy - design-microamp-hours: battery design capacity struct power_supply_battery_info { int design_energy_uwh; /* microWatt-hours */ int design_current_uah; /* microAmp-hours */ int end_of_discharge_uv; /* microVolts */ }; Default value for unset DT properties will be -EINVAL, since 0 is valid for design_* on the chips. -- 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
On Mon, Jan 30, 2017 at 2:54 AM, Liam Breck <liam@networkimprov.net> wrote: > On Sun, Jan 29, 2017 at 6:30 PM, Sebastian Reichel <sre@kernel.org> wrote: >> Hi, >> >> On Sun, Jan 29, 2017 at 03:22:31PM -0800, Liam Breck wrote: >>> On Sun, Jan 29, 2017 at 9:20 AM, Sebastian Reichel <sre@kernel.org> wrote: >>> > On Wed, Jan 25, 2017 at 11:02:03PM -0800, Matt Ranostay wrote: >>> >> On Wed, Jan 25, 2017 at 10:19 PM, Liam Breck <liam@networkimprov.net> wrote: >>> >> > On Sun, Jan 22, 2017 at 2:22 PM, Liam Breck <liam@networkimprov.net> wrote: >>> >> > >>> >> >> diff --git a/Documentation/devicetree/bindings/power/supply/battery.txt b/Documentation/devicetree/bindings/power/supply/battery.txt >>> >> >> new file mode 100644 >>> >> >> index 000000000000..398b4d622883 >>> >> >> --- /dev/null >>> >> >> +++ b/Documentation/devicetree/bindings/power/supply/battery.txt >>> >> >> @@ -0,0 +1,34 @@ >>> >> >> +Battery Characteristics >>> >> >> + >>> >> >> +Required Properties: >>> >> >> + - compatible: Must be "fixed-battery" >>> >> >> + >>> >> >> +Optional Properties: >>> >> >> + - termination-microvolt: dead battery voltage >>> >> >> + - design-microwatt-hours: battery design energy >>> >> >> + - design-microamp-hours: battery design capacity >>> >> > >>> >> > Also I suspect the members of struct power_supply_battery_info should >>> >> > use the same names as appear in dts: >>> >> >>> >> These are internal and we can't be thinking just device tree.. ACPI >>> >> and platform data is also an option. >>> > >>> > well platform data just uses the struct. ACPI is probably not >>> > relevant, since in ACPI world one usually has smart batteries. >>> > But yes, the names can be different. >>> > >>> > The important part is, that the API is used correctly, so it >>> > should be clear what each property is used for. For example >>> > termination_microvolt is not clear: Is this charge termination >>> > voltage or system shutdown voltage? Also we do not need to add >>> > "micro", since the power-supply subsystem always uses micro >>> > based units. >>> >>> It's the min battery voltage. So... >>> drained_voltage, depleted_voltage, sapped_voltage, spent_voltage... >> >> This is often called EOD (end of discharge) voltage, so let's use >> end_of_discharge_voltage. > > ... Actually, let's use names from POWER_SUPPLY_PROP_*, as DT inputs may be visible in sysfs, e.g. *_CHARGE_FULL_DESIGN. This would require new enum elements in future for the BQ24190 properties I mentioned up-thread. That looks like: Optional Properties: - voltage-min-design-microvolt: dead battery voltage - energy-full-design-microwatt-hours: battery design energy - charge-full-design-microamp-hours: battery design capacity struct power_supply_battery_info { int voltage_min_design_uv; /* microVolts */ int energy_full_design_uwh; /* microWatt-hours */ int charge_full_design_uah; /* microAmp-hours */ }; Default value for unset DT properties will be -EINVAL, since 0 is valid for *_full_design_* on the chips. ~.~ -- 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
On Sun, Jan 29, 2017 at 6:46 PM, Liam Breck <liam@networkimprov.net> wrote: > On Sun, Jan 29, 2017 at 6:39 PM, Sebastian Reichel <sre@kernel.org> wrote: >> Hi, >> >> On Sun, Jan 29, 2017 at 03:32:10PM -0800, Liam Breck wrote: >>> On Sun, Jan 29, 2017 at 10:06 AM, Sebastian Reichel <sre@kernel.org> wrote: >>> > On Sun, Jan 22, 2017 at 02:22:12PM -0800, Liam Breck wrote: >>> ... >>> >> +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 = "fixed-battery"; >>> >> + terminate-microvolt = <3700000>; >>> >> + design-microwatt-hours = <5290000>; >>> >> + design-microamp-hours = <1430000>; >>> >> + }; >>> >> + >>> >> + charger: charger@0 { >>> >> + .... >>> >> + monitored-battery = <&bat>; >>> >> + ... >>> >> + }; >>> >> + >>> >> + fuel_gauge: fuel_gauge@0 { >>> >> + .... >>> >> + monitored-battery = <&bat>; >>> >> + ... >>> >> + }; >>> > >>> > The charger stuff does does not integrate well with >>> > pre-existing support for power-supplies property >>> > described in >>> > Documentation/devicetree/bindings/power/supply/power_supply.txt >>> > >>> > I think the proper chain would be: >>> > >>> > bat: battery { >>> > compatible = "fixed-battery"; >>> > terminate-microvolt = <3700000>; >>> > design-microwatt-hours = <5290000>; >>> > design-microamp-hours = <1430000>; >>> > }; >>> > >>> > fuel_gauge: fuel_gauge@0 { >>> > ... >>> > monitored-battery = <&bat>; >>> > power-supplies = <&charger>; >>> > ... >>> > }; >>> > >>> > charger: charger@0 { >>> > ... >>> > }; >>> > >>> > I added the power-supplies node to the fuel-gauge instead of the battery, >>> > since fuel-gauge + fixed-battery is basically a smart battery. >>> >>> BQ24190 charger will need other params from a battery node. How does >>> it get them in this scenario? >> >> It should be able to get all required properties from the bq27xxx. >> Otherwise supporting smart batteries is impossible. What properties >> do you need? > > These two, at a minimum: > > - precharge-microamps: maximum charge current during precharge > phase (typically 20% of battery capacity) > > - termination-microamps: a charge cycle terminates when the > battery voltage is above recharge threshold, and the current is below > this setting (typically 10% of battery capacity) The relevant docs, see section 9.5.1.4 http://www.ti.com/lit/ds/symlink/bq24190.pdf#31 BQ27xxx doesn't know about these parameters, but they are characteristics of a fixed-battery, since the values are based on its capacity. Smart batteries may have a circuit to limit current during these phases. The BQ24190 charger is intended for dumb batteries :-) So I believe letting charger and fuel-gauge both reference monitored-battery is the right scheme. ~.~ -- 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
diff --git a/Documentation/devicetree/bindings/power/supply/battery.txt b/Documentation/devicetree/bindings/power/supply/battery.txt new file mode 100644 index 000000000000..398b4d622883 --- /dev/null +++ b/Documentation/devicetree/bindings/power/supply/battery.txt @@ -0,0 +1,34 @@ +Battery Characteristics + +Required Properties: + - compatible: Must be "fixed-battery" + +Optional Properties: + - termination-microvolt: dead battery voltage + - design-microwatt-hours: battery design energy + - design-microamp-hours: battery design capacity + +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 = "fixed-battery"; + terminate-microvolt = <3700000>; + design-microwatt-hours = <5290000>; + design-microamp-hours = <1430000>; + }; + + charger: charger@0 { + .... + monitored-battery = <&bat>; + ... + }; + + fuel_gauge: fuel_gauge@0 { + .... + monitored-battery = <&bat>; + ... + };