devicetree battery support and client bq27xxx_battery
Subsystems (4)
v6 cover devicetree battery support and client bq27xxx_battery
Overview:
* new devicetree battery node specifies static battery data
* fuel gauge and charger nodes shall use monitored-battery=<&battery_node>
* new power_supply_get_battery_info() reads battery data from devicetree
* new struct power_supply_battery_info provides battery data to drivers
* drivers surface battery data in sysfs via related power_supply_prop_* fields
* bq27xxx driver calls the above and writes battery data to NVM for params essential to correct operation: energy-full-design-microwatt-hours, charge-full-design-microamp-hours, voltage-min-design-microvolt
Changes in v6:
* Documentation/devictree/... fixes
* bq27xxx_battery: clarify names
* bq27xxx_battery: verify that selected registers are supported
* bq27xxx_battery: allocate NVM buffer on stack
* bq27xxx_battery_i2c: fix return code of bulk_read
Changes in v5:
* incorporate feedback into Documentation/devicetree/.../battery.txt
* use power_supply_prop_* names in devicetree and power_supply_battery_info
* default fields to -EINVAL in power_supply_battery_info
* power_supply_get_battery_info() always looks for "monitored-battery"
* power_supply_get_battery_info() emits a warning if !psy->of_node
* squash patches for power_supply_battery_info
* bq27xxx_battery: check power_supply_battery_info values
* bq27xxx_battery: note missing power_supply_prop_* features
* bq27xxx_battery: new patch for access methods
Changes in v4:
* add "fixed-battery" compatible field to be be more consistant with devicetree
Changes in v3:
* split i2c changes into respective patches
* add documentation for battery information for fuel gauge
* rebased documentation patches on change on the list
* abstracted the battery configuration for the state machine to an generic struct and platform data access function
Changes in v2:
* add documentation for uWh and uAh property units
* change devicetree entries to match new property units
Matt Ranostay (8):
devicetree: power: Add battery.txt
devicetree: property-units: Add uWh and uAh units
devicetree: power: bq27xxx: Add monitored-battery documentation
power: power_supply: Add power_supply_battery_info and API
power: bq27xxx_battery: Define access methods to write chip registers
power: bq27xxx_battery: Add BQ27425 chip id
power: bq27xxx_battery: Add power_supply_battery_info support
power: bq27xxx_battery_i2c: Add I2C bulk read/write functions
.../devicetree/bindings/power/supply/battery.txt | 37 +++ .../devicetree/bindings/power/supply/bq27xxx.txt | 11 +- .../devicetree/bindings/property-units.txt | 2 + drivers/power/supply/bq27xxx_battery.c | 325 ++++++++++++++++++++- drivers/power/supply/bq27xxx_battery_i2c.c | 67 ++++- drivers/power/supply/power_supply_core.c | 40 +++ include/linux/power/bq27xxx_battery.h | 6 +- include/linux/power_supply.h | 18 ++ 8 files changed, 501 insertions(+), 5 deletions(-) create mode 100644 Documentation/devicetree/bindings/power/supply/battery.txt
Signature · 1 line
2.9.3
PATCH v6 1/8 devicetree: power: Add battery.txt
From: Matt Ranostay <[email protected]>
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.
--- .../devicetree/bindings/power/supply/battery.txt | 37 ++++++++++++++++++++++ 1 file changed, 37 insertions(+) create mode 100644 Documentation/devicetree/bindings/power/supply/battery.txt
Documentation/devicetree/bindings/power/supply/battery.txt new file +37 -0
@@ -0,0 +1,37 @@ +Battery Characteristics + +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 + +Future Properties must be named for the corresponding elements in +enum power_supply_property, defined in include/linux/power_supply.h. + +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"; + 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>; + ... + };
Signature · 1 line
2.9.3
A message that is not in the archive (sent off-list or not yet received): 20170211024340.19491-2-liam-RYWXG+zxWwBdeoIcmNTgJF6hYfS7NtTn@public.gmane.org
Re: [PATCH v6 1/8] devicetree: power: Add battery.txt
Quoted 5 lines · Liam Breck
From: Matt Ranostay <[email protected]>
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.
I think some of the confusion about what a "smart-battery" binding would look like is due to a miss-understanding what it means to "self-identify". Originally the bq27xxx *was* the chip that lived its whole life inside a battery (the iphone battery is one such example and a good way to test the bq27000). This way it could be factory programmed with the battery's basics and then learn about the individual pack over time to provide better info on time to empty, discharge rates, etc..
Eventually, people wanted to have the battery become cheap and replaceable, so fuel-gauges were made that are "system side", and ride along with the device, not the battery. Now this has one big problem, how can we learn about a battery when it may be a new battery every time we boot up? So we added programmable memory to the devices, and programming this memory looks to be what this series is about.
Only the battery design info is programmed in this series, but much more battery info can be uploaded, including stuff that is not fixed and therefor cannot be added to the DT. Even the design parameters may not be fix if one changes the battery as different manufactures of the same battery type may have different design capacities.
The ideal use-case in a system with a system side gauge is the OS should periodically, or just before shutdown, read the battery's learned info and when the device is powered back up, if the battery serial number matches from before, re-upload this learned info to the gauge so it doesn't have to re-learn everything. All of this will need to happen in user-space, so what we really need is a user-space API for manipulating these properties.
The properties added in this series are okay, and it's good to have some sane defaults in DT, but looking forward this may not be enough to get full use out of these devices.
Andrew
Quoted 44 lines
Cc Rob Herring <[email protected]>Signed-off-by Matt Ranostay <[email protected]>Signed-off-by Liam Breck <[email protected]>--- .../devicetree/bindings/power/supply/battery.txt | 37 ++++++++++++++++++++++ 1 file changed, 37 insertions(+) create mode 100644 Documentation/devicetree/bindings/power/supply/battery.txtDocumentation/devicetree/bindings/power/supply/battery.txt new file +27
-0@@ -0,0 +1,37 @@ +Battery Characteristics + +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 + +Future Properties must be named for the corresponding elements in +enum power_supply_property, defined in include/linux/power_supply.h. + +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"; + voltage-min-design-microvolt = <3200000>; + energy-full-design-microwatt-hours = <5290000>; + charge-full-design-microamp-hours = <1430000>; + }; + + charger: charger@11 {
Documentation/devicetree/bindings/power/supply/battery.txt line 37
Quoted 10 lines
Documentation/devicetree/bindings/power/supply/battery.txt new file +10
-0+ .... + monitored-battery = <&bat>; + ... + }; + + fuel_gauge: fuel-gauge@22 { + .... + monitored-battery = <&bat>; + ... + };
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
A message that is not in the archive (sent off-list or not yet received): ce69f358-ac3b-993d-0639-844856362c9d-l0cyMroinI0@public.gmane.org
Re: [PATCH v6 1/8] devicetree: power: Add battery.txt
Hi Andrew, thanks for the drill-down on BQ27* chips.
Quoted 29 lines · Andrew F. Davis
Quoted 5 lines · Liam Breck
From: Matt Ranostay <[email protected]>
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.
I think some of the confusion about what a "smart-battery" binding would look like is due to a miss-understanding what it means to "self-identify". Originally the bq27xxx *was* the chip that lived its whole life inside a battery (the iphone battery is one such example and a good way to test the bq27000). This way it could be factory programmed with the battery's basics and then learn about the individual pack over time to provide better info on time to empty, discharge rates, etc..
Eventually, people wanted to have the battery become cheap and replaceable, so fuel-gauges were made that are "system side", and ride along with the device, not the battery. Now this has one big problem, how can we learn about a battery when it may be a new battery every time we boot up? So we added programmable memory to the devices, and programming this memory looks to be what this series is about.
Only the battery design info is programmed in this series, but much more battery info can be uploaded, including stuff that is not fixed and therefor cannot be added to the DT. Even the design parameters may not be fix if one changes the battery as different manufactures of the same battery type may have different design capacities.
Quoted 7 lines
The ideal use-case in a system with a system side gauge is the OS should periodically, or just before shutdown, read the battery's learned info and when the device is powered back up, if the battery serial number matches from before, re-upload this learned info to the gauge so it doesn't have to re-learn everything. All of this will need to happen in user-space, so what we really need is a user-space API for manipulating these properties.
We would need a feature in the driver to get/set the blob of pack- or cell-unique data. The userspace api is one sysfs file.
However the DT fixed-battery node could indicate when/how-often the object is stored, and where, e.g. filename. If the latter is given, the driver should write/read the file itself.
Is there some documentation about what registers should go in this blob?
That concept is beyond the scope of this patchset (and doesn't supersede it), but is something I'd like to implement for our chip, the *425.
Quoted 3 lines
The properties added in this series are okay, and it's good to have some sane defaults in DT, but looking forward this may not be enough to get full use out of these devices.
I'm relieved you're not opposed to our patch :-) Now I just need that guidance I mentioned re the chip family...
Quoted 45 lines
Quoted 45 lines
Cc Rob Herring <[email protected]>Signed-off-by Matt Ranostay <[email protected]>Signed-off-by Liam Breck <[email protected]>--- .../devicetree/bindings/power/supply/battery.txt | 37 ++++++++++++++++++++++ 1 file changed, 37 insertions(+) create mode 100644 Documentation/devicetree/bindings/power/supply/battery.txtDocumentation/devicetree/bindings/power/supply/battery.txt new file +30
-0@@ -0,0 +1,37 @@ +Battery Characteristics + +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 + +Future Properties must be named for the corresponding elements in +enum power_supply_property, defined in include/linux/power_supply.h. + +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"; + voltage-min-design-microvolt = <3200000>; + energy-full-design-microwatt-hours = <5290000>; + charge-full-design-microamp-hours = <1430000>; + }; + + charger: charger@11 { + .... + monitored-battery = <&bat>; + ...
Quoted 8 lines
Documentation/devicetree/bindings/power/supply/battery.txt line 37
Quoted 8 lines
Documentation/devicetree/bindings/power/supply/battery.txt new file +7
-0+ }; + + fuel_gauge: fuel-gauge@22 { + .... + monitored-battery = <&bat>; + ... + };
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
A message that is not in the archive (sent off-list or not yet received): CAKvHMgSSpbkX1NMwaDmuJwBcgu6Avy65izAaBGTYongqL51kWQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org
Re: [PATCH v6 1/8] devicetree: power: Add battery.txt
Quoted 40 lines · Liam Breck
Hi Andrew, thanks for the drill-down on BQ27* chips.
Quoted 29 lines · Andrew F. Davis
Quoted 5 lines · Liam Breck
From: Matt Ranostay <[email protected]>
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.
I think some of the confusion about what a "smart-battery" binding would look like is due to a miss-understanding what it means to "self-identify". Originally the bq27xxx *was* the chip that lived its whole life inside a battery (the iphone battery is one such example and a good way to test the bq27000). This way it could be factory programmed with the battery's basics and then learn about the individual pack over time to provide better info on time to empty, discharge rates, etc..
Eventually, people wanted to have the battery become cheap and replaceable, so fuel-gauges were made that are "system side", and ride along with the device, not the battery. Now this has one big problem, how can we learn about a battery when it may be a new battery every time we boot up? So we added programmable memory to the devices, and programming this memory looks to be what this series is about.
Only the battery design info is programmed in this series, but much more battery info can be uploaded, including stuff that is not fixed and therefor cannot be added to the DT. Even the design parameters may not be fix if one changes the battery as different manufactures of the same battery type may have different design capacities.
Quoted 7 lines
The ideal use-case in a system with a system side gauge is the OS should periodically, or just before shutdown, read the battery's learned info and when the device is powered back up, if the battery serial number matches from before, re-upload this learned info to the gauge so it doesn't have to re-learn everything. All of this will need to happen in user-space, so what we really need is a user-space API for manipulating these properties.
Quoted 9 lines
We would need a feature in the driver to get/set the blob of pack- or cell-unique data. The userspace api is one sysfs file.
However the DT fixed-battery node could indicate when/how-often the object is stored, and where, e.g. filename. If the latter is given, the driver should write/read the file itself.
Is there some documentation about what registers should go in this blob?
This should all be available in the parts' technical reference (looks like page 24 for your device), but the blob isn't meant to be modified, the information/format is mostly internal to the part, all we should do is save blob / restore blob as needed.
Quoted 5 lines
That concept is beyond the scope of this patchset (and doesn't supersede it), but is something I'd like to implement for our chip, the *425.
Quoted 7 lines
Quoted 3 lines
The properties added in this series are okay, and it's good to have some sane defaults in DT, but looking forward this may not be enough to get full use out of these devices.
I'm relieved you're not opposed to our patch :-) Now I just need that guidance I mentioned re the chip family...
Still looking into it, I think for now would be easiest to add the register info to each chip family struct. The un-seal sequence should be generic enough already.
Andrew
Quoted 50 lines
Quoted 50 lines
Quoted 50 lines
Cc Rob Herring <[email protected]>Signed-off-by Matt Ranostay <[email protected]>Signed-off-by Liam Breck <[email protected]>--- .../devicetree/bindings/power/supply/battery.txt | 37 ++++++++++++++++++++++ 1 file changed, 37 insertions(+) create mode 100644 Documentation/devicetree/bindings/power/supply/battery.txtDocumentation/devicetree/bindings/power/supply/battery.txt new file +34
-0@@ -0,0 +1,37 @@ +Battery Characteristics + +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 + +Future Properties must be named for the corresponding elements in +enum power_supply_property, defined in include/linux/power_supply.h. + +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"; + 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 { + ....
Quoted 9 lines
Quoted 4 lines
Documentation/devicetree/bindings/power/supply/battery.txt line 31
Quoted 4 lines
Documentation/devicetree/bindings/power/supply/battery.txt new file +3
-0+ monitored-battery = <&bat>; + ... + };--
To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 1/8] devicetree: power: Add battery.txt
[Dropped devicetree from CC]
Quoted 48 lines · Andrew F. Davis
Quoted 47 lines · Liam Breck
Hi Andrew, thanks for the drill-down on BQ27* chips.
Quoted 29 lines · Andrew F. Davis
Quoted 5 lines · Liam Breck
From: Matt Ranostay <[email protected]>
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.
I think some of the confusion about what a "smart-battery" binding would look like is due to a miss-understanding what it means to "self-identify". Originally the bq27xxx *was* the chip that lived its whole life inside a battery (the iphone battery is one such example and a good way to test the bq27000). This way it could be factory programmed with the battery's basics and then learn about the individual pack over time to provide better info on time to empty, discharge rates, etc..
Eventually, people wanted to have the battery become cheap and replaceable, so fuel-gauges were made that are "system side", and ride along with the device, not the battery. Now this has one big problem, how can we learn about a battery when it may be a new battery every time we boot up? So we added programmable memory to the devices, and programming this memory looks to be what this series is about.
Only the battery design info is programmed in this series, but much more battery info can be uploaded, including stuff that is not fixed and therefor cannot be added to the DT. Even the design parameters may not be fix if one changes the battery as different manufactures of the same battery type may have different design capacities.
Quoted 7 lines
The ideal use-case in a system with a system side gauge is the OS should periodically, or just before shutdown, read the battery's learned info and when the device is powered back up, if the battery serial number matches from before, re-upload this learned info to the gauge so it doesn't have to re-learn everything. All of this will need to happen in user-space, so what we really need is a user-space API for manipulating these properties.
We would need a feature in the driver to get/set the blob of pack- or cell-unique data. The userspace api is one sysfs file.
However the DT fixed-battery node could indicate when/how-often the object is stored, and where, e.g. filename. If the latter is given, the driver should write/read the file itself.
Quoted 7 lines
Quoted 2 lines
Is there some documentation about what registers should go in this blob?
This should all be available in the parts' technical reference (looks like page 24 for your device), but the blob isn't meant to be modified, the information/format is mostly internal to the part, all we should do is save blob / restore blob as needed.
Page 24 has "8.5.5 Data Block Summary" That?
Subclass IDs for NVM are 82, 88,104, 105. Do all those go in the blob?
Quoted 9 lines
Quoted 9 lines
That concept is beyond the scope of this patchset (and doesn't supersede it), but is something I'd like to implement for our chip, the *425.
Quoted 3 lines
The properties added in this series are okay, and it's good to have some sane defaults in DT, but looking forward this may not be enough to get full use out of these devices.
Quoted 7 lines
Quoted 3 lines
I'm relieved you're not opposed to our patch :-) Now I just need that guidance I mentioned re the chip family...
Still looking into it, I think for now would be easiest to add the register info to each chip family struct. The un-seal sequence should be generic enough already.
I'll wait for your full reply on that thread.
Quoted 58 lines
Quoted 58 lines
Quoted 45 lines
Quoted 45 lines
Cc Rob Herring <[email protected]>Signed-off-by Matt Ranostay <[email protected]>Signed-off-by Liam Breck <[email protected]>--- .../devicetree/bindings/power/supply/battery.txt | 37 ++++++++++++++++++++++ 1 file changed, 37 insertions(+) create mode 100644 Documentation/devicetree/bindings/power/supply/battery.txtDocumentation/devicetree/bindings/power/supply/battery.txt new file +30
-0@@ -0,0 +1,37 @@ +Battery Characteristics + +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 + +Future Properties must be named for the corresponding elements in +enum power_supply_property, defined in include/linux/power_supply.h. + +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"; + voltage-min-design-microvolt = <3200000>; + energy-full-design-microwatt-hours = <5290000>; + charge-full-design-microamp-hours = <1430000>; + }; + + charger: charger@11 { + .... + monitored-battery = <&bat>; + ...Quoted 8 lines
Documentation/devicetree/bindings/power/supply/battery.txt line 37
Quoted 8 lines
Documentation/devicetree/bindings/power/supply/battery.txt new file +7
-0+ }; + + fuel_gauge: fuel-gauge@22 { + .... + monitored-battery = <&bat>; + ... + };--
To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 1/8] devicetree: power: Add battery.txt
Quoted 54 lines · Liam Breck
[Dropped devicetree from CC]
Quoted 51 lines · Andrew F. Davis
Quoted 49 lines · Liam Breck
Hi Andrew, thanks for the drill-down on BQ27* chips.
Quoted 29 lines · Andrew F. Davis
Quoted 5 lines · Liam Breck
From: Matt Ranostay <[email protected]>
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.
I think some of the confusion about what a "smart-battery" binding would look like is due to a miss-understanding what it means to "self-identify". Originally the bq27xxx *was* the chip that lived its whole life inside a battery (the iphone battery is one such example and a good way to test the bq27000). This way it could be factory programmed with the battery's basics and then learn about the individual pack over time to provide better info on time to empty, discharge rates, etc..
Eventually, people wanted to have the battery become cheap and replaceable, so fuel-gauges were made that are "system side", and ride along with the device, not the battery. Now this has one big problem, how can we learn about a battery when it may be a new battery every time we boot up? So we added programmable memory to the devices, and programming this memory looks to be what this series is about.
Only the battery design info is programmed in this series, but much more battery info can be uploaded, including stuff that is not fixed and therefor cannot be added to the DT. Even the design parameters may not be fix if one changes the battery as different manufactures of the same battery type may have different design capacities.
Quoted 7 lines
The ideal use-case in a system with a system side gauge is the OS should periodically, or just before shutdown, read the battery's learned info and when the device is powered back up, if the battery serial number matches from before, re-upload this learned info to the gauge so it doesn't have to re-learn everything. All of this will need to happen in user-space, so what we really need is a user-space API for manipulating these properties.
We would need a feature in the driver to get/set the blob of pack- or cell-unique data. The userspace api is one sysfs file.
However the DT fixed-battery node could indicate when/how-often the object is stored, and where, e.g. filename. If the latter is given, the driver should write/read the file itself.
Is there some documentation about what registers should go in this blob?
Quoted 9 lines
Quoted 4 lines
This should all be available in the parts' technical reference (looks like page 24 for your device), but the blob isn't meant to be modified, the information/format is mostly internal to the part, all we should do is save blob / restore blob as needed.
Page 24 has "8.5.5 Data Block Summary" That?
Subclass IDs for NVM are 82, 88,104, 105. Do all those go in the blob?
Some devices (bq27421) do not contain an NVM, and so rely on the host to restore its "learned data" across power cycles with the same battery.
When using devices with NVM this "learned data" can be saved to the NVM and it will automatically be copied over to the run-time RAM sections on powerup.
So we should only ever need to save the RAM section.
Another thing to consider is that we (TI) have GUI tools to allow manufacturers to input battery info and have it generate a blob that can be deployed to these devices. The blob format is meant to be handled and uploaded by custom user-space tools as it only really needs to be done once (power-down events should only happen when changing batteries, in which case you want to let the device forget what it has learned about the old battery).
All of the details of this programming are probably not the job of the kernel, in end-user hands the kernel only needs to know how to read from the device, not program it.
Take everything I say with a grain of salt though, I'm not on the team that makes these. I don't have much additional info not in the public data-sheets.
Andrew
Quoted 79 lines
Quoted 10 lines
Quoted 10 lines
That concept is beyond the scope of this patchset (and doesn't supersede it), but is something I'd like to implement for our chip, the *425.
Quoted 3 lines
The properties added in this series are okay, and it's good to have some sane defaults in DT, but looking forward this may not be enough to get full use out of these devices.
Quoted 7 lines
Quoted 3 lines
I'm relieved you're not opposed to our patch :-) Now I just need that guidance I mentioned re the chip family...
Still looking into it, I think for now would be easiest to add the register info to each chip family struct. The un-seal sequence should be generic enough already.
I'll wait for your full reply on that thread.
Quoted 58 lines
Quoted 58 lines
Quoted 45 lines
Quoted 45 lines
Cc Rob Herring <[email protected]>Signed-off-by Matt Ranostay <[email protected]>Signed-off-by Liam Breck <[email protected]>--- .../devicetree/bindings/power/supply/battery.txt | 37 ++++++++++++++++++++++ 1 file changed, 37 insertions(+) create mode 100644 Documentation/devicetree/bindings/power/supply/battery.txtDocumentation/devicetree/bindings/power/supply/battery.txt new file +30
-0@@ -0,0 +1,37 @@ +Battery Characteristics + +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 + +Future Properties must be named for the corresponding elements in +enum power_supply_property, defined in include/linux/power_supply.h. + +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"; + voltage-min-design-microvolt = <3200000>; + energy-full-design-microwatt-hours = <5290000>; + charge-full-design-microamp-hours = <1430000>; + }; + + charger: charger@11 { + .... + monitored-battery = <&bat>; + ...Quoted 8 lines
Documentation/devicetree/bindings/power/supply/battery.txt line 37
Quoted 8 lines
Documentation/devicetree/bindings/power/supply/battery.txt new file +7
-0+ }; + + fuel_gauge: fuel-gauge@22 { + .... + monitored-battery = <&bat>; + ... + };--
To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 1/8] devicetree: power: Add battery.txt
Quoted 60 lines · Andrew F. Davis
Quoted 59 lines · Liam Breck
[Dropped devicetree from CC]
Quoted 55 lines · Andrew F. Davis
Quoted 40 lines · Liam Breck
Hi Andrew, thanks for the drill-down on BQ27* chips.
Quoted 29 lines · Andrew F. Davis
Quoted 5 lines · Liam Breck
From: Matt Ranostay <[email protected]>
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.
I think some of the confusion about what a "smart-battery" binding would look like is due to a miss-understanding what it means to "self-identify". Originally the bq27xxx *was* the chip that lived its whole life inside a battery (the iphone battery is one such example and a good way to test the bq27000). This way it could be factory programmed with the battery's basics and then learn about the individual pack over time to provide better info on time to empty, discharge rates, etc..
Eventually, people wanted to have the battery become cheap and replaceable, so fuel-gauges were made that are "system side", and ride along with the device, not the battery. Now this has one big problem, how can we learn about a battery when it may be a new battery every time we boot up? So we added programmable memory to the devices, and programming this memory looks to be what this series is about.
Only the battery design info is programmed in this series, but much more battery info can be uploaded, including stuff that is not fixed and therefor cannot be added to the DT. Even the design parameters may not be fix if one changes the battery as different manufactures of the same battery type may have different design capacities.
Quoted 7 lines
The ideal use-case in a system with a system side gauge is the OS should periodically, or just before shutdown, read the battery's learned info and when the device is powered back up, if the battery serial number matches from before, re-upload this learned info to the gauge so it doesn't have to re-learn everything. All of this will need to happen in user-space, so what we really need is a user-space API for manipulating these properties.
Quoted 9 lines
We would need a feature in the driver to get/set the blob of pack- or cell-unique data. The userspace api is one sysfs file.
However the DT fixed-battery node could indicate when/how-often the object is stored, and where, e.g. filename. If the latter is given, the driver should write/read the file itself.
Is there some documentation about what registers should go in this blob?
This should all be available in the parts' technical reference (looks like page 24 for your device), but the blob isn't meant to be modified, the information/format is mostly internal to the part, all we should do is save blob / restore blob as needed.
Quoted 7 lines
Quoted 4 lines
Page 24 has "8.5.5 Data Block Summary" That?
Subclass IDs for NVM are 82, 88,104, 105. Do all those go in the blob?
Some devices (bq27421) do not contain an NVM, and so rely on the host to restore its "learned data" across power cycles with the same battery.
Ah, so my concept for DT battery:nvm-storage: "filename" would apply to these RAM-only chips.
Quoted 3 lines
When using devices with NVM this "learned data" can be saved to the NVM and it will automatically be copied over to the run-time RAM sections on powerup. So we should only ever need to save the RAM section.
For the NVM chips, we'd want a userspace-accessible reset_nvm function in driver which returns the pack-unique data to factory defaults. That can be provided via a sysfs file to which you write a key (safer than "echo 1 > /sys/class.../reset_nvm") to trigger the reset.
Quoted 11 lines
Another thing to consider is that we (TI) have GUI tools to allow manufacturers to input battery info and have it generate a blob that can be deployed to these devices. The blob format is meant to be handled and uploaded by custom user-space tools as it only really needs to be done once (power-down events should only happen when changing batteries, in which case you want to let the device forget what it has learned about the old battery).
All of the details of this programming are probably not the job of the kernel, in end-user hands the kernel only needs to know how to read from the device, not program it.
Were there an independently maintained battery manager app with wide support for battery ICs, it would be a natural venue for BQ27* reset/save/restore_nvm functions. But there is none that I know of, and these features can be implemented in the driver with little new code beyond the read/write_dm_block functions added by this patchset.
It's fine to ask hardware makers to rely on IC vendor tools in the factory, but I don't think such tools belong in the end-user's stack.
Quoted 85 lines
Take everything I say with a grain of salt though, I'm not on the team that makes these. I don't have much additional info not in the public data-sheets.
Andrew
Quoted 79 lines
Quoted 10 lines
Quoted 10 lines
That concept is beyond the scope of this patchset (and doesn't supersede it), but is something I'd like to implement for our chip, the *425.
Quoted 3 lines
The properties added in this series are okay, and it's good to have some sane defaults in DT, but looking forward this may not be enough to get full use out of these devices.
Quoted 7 lines
Quoted 3 lines
I'm relieved you're not opposed to our patch :-) Now I just need that guidance I mentioned re the chip family...
Still looking into it, I think for now would be easiest to add the register info to each chip family struct. The un-seal sequence should be generic enough already.
I'll wait for your full reply on that thread.
Quoted 58 lines
Quoted 58 lines
Quoted 45 lines
Quoted 45 lines
Cc Rob Herring <[email protected]>Signed-off-by Matt Ranostay <[email protected]>Signed-off-by Liam Breck <[email protected]>--- .../devicetree/bindings/power/supply/battery.txt | 37 ++++++++++++++++++++++ 1 file changed, 37 insertions(+) create mode 100644 Documentation/devicetree/bindings/power/supply/battery.txtDocumentation/devicetree/bindings/power/supply/battery.txt new file +30
-0@@ -0,0 +1,37 @@ +Battery Characteristics + +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 + +Future Properties must be named for the corresponding elements in +enum power_supply_property, defined in include/linux/power_supply.h. + +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"; + voltage-min-design-microvolt = <3200000>; + energy-full-design-microwatt-hours = <5290000>; + charge-full-design-microamp-hours = <1430000>; + }; + + charger: charger@11 { + .... + monitored-battery = <&bat>; + ...Quoted 8 lines
Documentation/devicetree/bindings/power/supply/battery.txt line 37
Quoted 8 lines
Documentation/devicetree/bindings/power/supply/battery.txt new file +7
-0+ }; + + fuel_gauge: fuel-gauge@22 { + .... + monitored-battery = <&bat>; + ... + };--
To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 1/8] devicetree: power: Add battery.txt
Quoted 63 lines · Liam Breck
Quoted 62 lines · Andrew F. Davis
Quoted 61 lines · Liam Breck
[Dropped devicetree from CC]
Quoted 48 lines · Andrew F. Davis
Quoted 47 lines · Liam Breck
Hi Andrew, thanks for the drill-down on BQ27* chips.
Quoted 29 lines · Andrew F. Davis
Quoted 5 lines · Liam Breck
From: Matt Ranostay <[email protected]>
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.
I think some of the confusion about what a "smart-battery" binding would look like is due to a miss-understanding what it means to "self-identify". Originally the bq27xxx *was* the chip that lived its whole life inside a battery (the iphone battery is one such example and a good way to test the bq27000). This way it could be factory programmed with the battery's basics and then learn about the individual pack over time to provide better info on time to empty, discharge rates, etc..
Eventually, people wanted to have the battery become cheap and replaceable, so fuel-gauges were made that are "system side", and ride along with the device, not the battery. Now this has one big problem, how can we learn about a battery when it may be a new battery every time we boot up? So we added programmable memory to the devices, and programming this memory looks to be what this series is about.
Only the battery design info is programmed in this series, but much more battery info can be uploaded, including stuff that is not fixed and therefor cannot be added to the DT. Even the design parameters may not be fix if one changes the battery as different manufactures of the same battery type may have different design capacities.
Quoted 7 lines
The ideal use-case in a system with a system side gauge is the OS should periodically, or just before shutdown, read the battery's learned info and when the device is powered back up, if the battery serial number matches from before, re-upload this learned info to the gauge so it doesn't have to re-learn everything. All of this will need to happen in user-space, so what we really need is a user-space API for manipulating these properties.
We would need a feature in the driver to get/set the blob of pack- or cell-unique data. The userspace api is one sysfs file.
However the DT fixed-battery node could indicate when/how-often the object is stored, and where, e.g. filename. If the latter is given, the driver should write/read the file itself.
Quoted 7 lines
Quoted 2 lines
Is there some documentation about what registers should go in this blob?
This should all be available in the parts' technical reference (looks like page 24 for your device), but the blob isn't meant to be modified, the information/format is mostly internal to the part, all we should do is save blob / restore blob as needed.
Page 24 has "8.5.5 Data Block Summary" That?
Quoted 9 lines
Quoted 5 lines
Quoted 2 lines
Subclass IDs for NVM are 82, 88,104, 105. Do all those go in the blob?
Some devices (bq27421) do not contain an NVM, and so rely on the host to restore its "learned data" across power cycles with the same battery.
Ah, so my concept for DT battery:nvm-storage: "filename" would apply to these RAM-only chips.
I think so, I was going to do something similar, but let userspace take care of the filename, or have it a fixed/module param like firmware. DT is not really the right spot for filenames.
If you can find a kernel-friendly way to do this then I'm all for it!
Quoted 9 lines
Quoted 3 lines
When using devices with NVM this "learned data" can be saved to the NVM and it will automatically be copied over to the run-time RAM sections on powerup. So we should only ever need to save the RAM section.
For the NVM chips, we'd want a userspace-accessible
reset_nvmfunction in driver which returns the pack-unique data to factory defaults. That can be provided via a sysfs file to which you write a key (safer than "echo 1 >/sys/class.../reset_nvm") to trigger the reset.
Still not sure why one would need this, the only time you would want to reset the gauge is when changing the battery, and that will already cause it to power cycle (unless you have some kind of back-up battery, but that doesn't make sense to me).
Quoted 12 lines
Quoted 11 lines
Another thing to consider is that we (TI) have GUI tools to allow manufacturers to input battery info and have it generate a blob that can be deployed to these devices. The blob format is meant to be handled and uploaded by custom user-space tools as it only really needs to be done once (power-down events should only happen when changing batteries, in which case you want to let the device forget what it has learned about the old battery).
All of the details of this programming are probably not the job of the kernel, in end-user hands the kernel only needs to know how to read from the device, not program it.
Quoted 9 lines
Were there an independently maintained battery manager app with wide support for battery ICs, it would be a natural venue for BQ27* reset/save/restore_nvm functions. But there is none that I know of, and these features can be implemented in the driver with little new code beyond the read/write_dm_block functions added by this patchset.
It's fine to ask hardware makers to rely on IC vendor tools in the factory, but I don't think such tools belong in the end-user's stack.
If they don't belong in the end-user's user-space tool-set then they definitely don't belong in kernel-space. The other option is to define a whole new API for interfacing with all these device specific parameters and internal info tables. It's that or we accept that these are factory settings and we don't need all too much kernel infrastructure for handling them.
Andrew
Quoted 85 lines
Quoted 85 lines
Take everything I say with a grain of salt though, I'm not on the team that makes these. I don't have much additional info not in the public data-sheets.
Andrew
Quoted 79 lines
Quoted 10 lines
Quoted 10 lines
That concept is beyond the scope of this patchset (and doesn't supersede it), but is something I'd like to implement for our chip, the *425.
Quoted 3 lines
The properties added in this series are okay, and it's good to have some sane defaults in DT, but looking forward this may not be enough to get full use out of these devices.
Quoted 7 lines
Quoted 3 lines
I'm relieved you're not opposed to our patch :-) Now I just need that guidance I mentioned re the chip family...
Still looking into it, I think for now would be easiest to add the register info to each chip family struct. The un-seal sequence should be generic enough already.
I'll wait for your full reply on that thread.
Quoted 58 lines
Quoted 58 lines
Quoted 45 lines
Quoted 45 lines
Cc Rob Herring <[email protected]>Signed-off-by Matt Ranostay <[email protected]>Signed-off-by Liam Breck <[email protected]>--- .../devicetree/bindings/power/supply/battery.txt | 37 ++++++++++++++++++++++ 1 file changed, 37 insertions(+) create mode 100644 Documentation/devicetree/bindings/power/supply/battery.txtDocumentation/devicetree/bindings/power/supply/battery.txt new file +30
-0@@ -0,0 +1,37 @@ +Battery Characteristics + +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 + +Future Properties must be named for the corresponding elements in +enum power_supply_property, defined in include/linux/power_supply.h. + +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"; + voltage-min-design-microvolt = <3200000>; + energy-full-design-microwatt-hours = <5290000>; + charge-full-design-microamp-hours = <1430000>; + }; + + charger: charger@11 { + .... + monitored-battery = <&bat>; + ...Quoted 8 lines
Documentation/devicetree/bindings/power/supply/battery.txt line 37
Quoted 8 lines
Documentation/devicetree/bindings/power/supply/battery.txt new file +7
-0+ }; + + fuel_gauge: fuel-gauge@22 { + .... + monitored-battery = <&bat>; + ... + };--
To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 1/8] devicetree: power: Add battery.txt
Quoted 74 lines · Andrew F. Davis
Quoted 63 lines · Liam Breck
Quoted 62 lines · Andrew F. Davis
Quoted 61 lines · Liam Breck
[Dropped devicetree from CC]
Quoted 48 lines · Andrew F. Davis
Quoted 47 lines · Liam Breck
Hi Andrew, thanks for the drill-down on BQ27* chips.
Quoted 29 lines · Andrew F. Davis
Quoted 5 lines · Liam Breck
From: Matt Ranostay <[email protected]>
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.
I think some of the confusion about what a "smart-battery" binding would look like is due to a miss-understanding what it means to "self-identify". Originally the bq27xxx *was* the chip that lived its whole life inside a battery (the iphone battery is one such example and a good way to test the bq27000). This way it could be factory programmed with the battery's basics and then learn about the individual pack over time to provide better info on time to empty, discharge rates, etc..
Eventually, people wanted to have the battery become cheap and replaceable, so fuel-gauges were made that are "system side", and ride along with the device, not the battery. Now this has one big problem, how can we learn about a battery when it may be a new battery every time we boot up? So we added programmable memory to the devices, and programming this memory looks to be what this series is about.
Only the battery design info is programmed in this series, but much more battery info can be uploaded, including stuff that is not fixed and therefor cannot be added to the DT. Even the design parameters may not be fix if one changes the battery as different manufactures of the same battery type may have different design capacities.
Quoted 7 lines
The ideal use-case in a system with a system side gauge is the OS should periodically, or just before shutdown, read the battery's learned info and when the device is powered back up, if the battery serial number matches from before, re-upload this learned info to the gauge so it doesn't have to re-learn everything. All of this will need to happen in user-space, so what we really need is a user-space API for manipulating these properties.
We would need a feature in the driver to get/set the blob of pack- or cell-unique data. The userspace api is one sysfs file.
However the DT fixed-battery node could indicate when/how-often the object is stored, and where, e.g. filename. If the latter is given, the driver should write/read the file itself.
Quoted 7 lines
Quoted 2 lines
Is there some documentation about what registers should go in this blob?
This should all be available in the parts' technical reference (looks like page 24 for your device), but the blob isn't meant to be modified, the information/format is mostly internal to the part, all we should do is save blob / restore blob as needed.
Page 24 has "8.5.5 Data Block Summary" That?
Quoted 9 lines
Quoted 5 lines
Quoted 2 lines
Subclass IDs for NVM are 82, 88,104, 105. Do all those go in the blob?
Some devices (bq27421) do not contain an NVM, and so rely on the host to restore its "learned data" across power cycles with the same battery.
Ah, so my concept for DT battery:nvm-storage: "filename" would apply to these RAM-only chips.
Quoted 5 lines
I think so, I was going to do something similar, but let userspace take care of the filename, or have it a fixed/module param like firmware. DT is not really the right spot for filenames.
If you can find a kernel-friendly way to do this then I'm all for it!
Hm, you have a point about filenames in DT. Maybe an inode number? (kidding :-)
We could hand the driver a filename via sysfs file or module param. Perhaps use a filename in DT as a fallback.
Quoted 10 lines
Quoted 9 lines
Quoted 3 lines
When using devices with NVM this "learned data" can be saved to the NVM and it will automatically be copied over to the run-time RAM sections on powerup. So we should only ever need to save the RAM section.
For the NVM chips, we'd want a userspace-accessible
reset_nvmfunction in driver which returns the pack-unique data to factory defaults. That can be provided via a sysfs file to which you write a key (safer than "echo 1 >/sys/class.../reset_nvm") to trigger the reset.
Quoted 4 lines
Still not sure why one would need this, the only time you would want to reset the gauge is when changing the battery, and that will already cause it to power cycle (unless you have some kind of back-up battery, but that doesn't make sense to me).
This is for gauges with NVM, where power-cycle does not clear the pack-specific stats. Or do those gauges recognize that the pack has changed?
Quoted 22 lines
Quoted 12 lines
Quoted 11 lines
Another thing to consider is that we (TI) have GUI tools to allow manufacturers to input battery info and have it generate a blob that can be deployed to these devices. The blob format is meant to be handled and uploaded by custom user-space tools as it only really needs to be done once (power-down events should only happen when changing batteries, in which case you want to let the device forget what it has learned about the old battery).
All of the details of this programming are probably not the job of the kernel, in end-user hands the kernel only needs to know how to read from the device, not program it.
Quoted 9 lines
Were there an independently maintained battery manager app with wide support for battery ICs, it would be a natural venue for BQ27* reset/save/restore_nvm functions. But there is none that I know of, and these features can be implemented in the driver with little new code beyond the read/write_dm_block functions added by this patchset.
It's fine to ask hardware makers to rely on IC vendor tools in the factory, but I don't think such tools belong in the end-user's stack.
Quoted 6 lines
If they don't belong in the end-user's user-space tool-set then they definitely don't belong in kernel-space. The other option is to define a whole new API for interfacing with all these device specific parameters and internal info tables. It's that or we accept that these are factory settings and we don't need all too much kernel infrastructure for handling them.
My point is that an IC vendor *codebase* does not belong in the end-user's stack; it's a production tool. The functionality we're discussing clearly belongs in both the device driver and the production tool.
Quoted 85 lines
Quoted 85 lines
Quoted 85 lines
Take everything I say with a grain of salt though, I'm not on the team that makes these. I don't have much additional info not in the public data-sheets.
Andrew
Quoted 79 lines
Quoted 10 lines
Quoted 10 lines
That concept is beyond the scope of this patchset (and doesn't supersede it), but is something I'd like to implement for our chip, the *425.
Quoted 3 lines
The properties added in this series are okay, and it's good to have some sane defaults in DT, but looking forward this may not be enough to get full use out of these devices.
Quoted 7 lines
Quoted 3 lines
I'm relieved you're not opposed to our patch :-) Now I just need that guidance I mentioned re the chip family...
Still looking into it, I think for now would be easiest to add the register info to each chip family struct. The un-seal sequence should be generic enough already.
I'll wait for your full reply on that thread.
Quoted 58 lines
Quoted 58 lines
Quoted 45 lines
Quoted 45 lines
Cc Rob Herring <[email protected]>Signed-off-by Matt Ranostay <[email protected]>Signed-off-by Liam Breck <[email protected]>--- .../devicetree/bindings/power/supply/battery.txt | 37 ++++++++++++++++++++++ 1 file changed, 37 insertions(+) create mode 100644 Documentation/devicetree/bindings/power/supply/battery.txtDocumentation/devicetree/bindings/power/supply/battery.txt new file +30
-0@@ -0,0 +1,37 @@ +Battery Characteristics + +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 + +Future Properties must be named for the corresponding elements in +enum power_supply_property, defined in include/linux/power_supply.h. + +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"; + voltage-min-design-microvolt = <3200000>; + energy-full-design-microwatt-hours = <5290000>; + charge-full-design-microamp-hours = <1430000>; + }; + + charger: charger@11 { + .... + monitored-battery = <&bat>; + ...Quoted 8 lines
Documentation/devicetree/bindings/power/supply/battery.txt line 37
Quoted 8 lines
Documentation/devicetree/bindings/power/supply/battery.txt new file +7
-0+ }; + + fuel_gauge: fuel-gauge@22 { + .... + monitored-battery = <&bat>; + ... + };--
To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 1/8] devicetree: power: Add battery.txt
Quoted 75 lines · Liam Breck
Quoted 74 lines · Andrew F. Davis
Quoted 72 lines · Liam Breck
Quoted 60 lines · Andrew F. Davis
Quoted 59 lines · Liam Breck
[Dropped devicetree from CC]
Quoted 55 lines · Andrew F. Davis
Quoted 40 lines · Liam Breck
Hi Andrew, thanks for the drill-down on BQ27* chips.
Quoted 29 lines · Andrew F. Davis
Quoted 5 lines · Liam Breck
From: Matt Ranostay <[email protected]>
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.
I think some of the confusion about what a "smart-battery" binding would look like is due to a miss-understanding what it means to "self-identify". Originally the bq27xxx *was* the chip that lived its whole life inside a battery (the iphone battery is one such example and a good way to test the bq27000). This way it could be factory programmed with the battery's basics and then learn about the individual pack over time to provide better info on time to empty, discharge rates, etc..
Eventually, people wanted to have the battery become cheap and replaceable, so fuel-gauges were made that are "system side", and ride along with the device, not the battery. Now this has one big problem, how can we learn about a battery when it may be a new battery every time we boot up? So we added programmable memory to the devices, and programming this memory looks to be what this series is about.
Only the battery design info is programmed in this series, but much more battery info can be uploaded, including stuff that is not fixed and therefor cannot be added to the DT. Even the design parameters may not be fix if one changes the battery as different manufactures of the same battery type may have different design capacities.
Quoted 7 lines
The ideal use-case in a system with a system side gauge is the OS should periodically, or just before shutdown, read the battery's learned info and when the device is powered back up, if the battery serial number matches from before, re-upload this learned info to the gauge so it doesn't have to re-learn everything. All of this will need to happen in user-space, so what we really need is a user-space API for manipulating these properties.
Quoted 9 lines
We would need a feature in the driver to get/set the blob of pack- or cell-unique data. The userspace api is one sysfs file.
However the DT fixed-battery node could indicate when/how-often the object is stored, and where, e.g. filename. If the latter is given, the driver should write/read the file itself.
Is there some documentation about what registers should go in this blob?
This should all be available in the parts' technical reference (looks like page 24 for your device), but the blob isn't meant to be modified, the information/format is mostly internal to the part, all we should do is save blob / restore blob as needed.
Quoted 7 lines
Quoted 4 lines
Page 24 has "8.5.5 Data Block Summary" That?
Subclass IDs for NVM are 82, 88,104, 105. Do all those go in the blob?
Some devices (bq27421) do not contain an NVM, and so rely on the host to restore its "learned data" across power cycles with the same battery.
Ah, so my concept for DT battery:nvm-storage: "filename" would apply to these RAM-only chips.
Quoted 8 lines
Quoted 5 lines
I think so, I was going to do something similar, but let userspace take care of the filename, or have it a fixed/module param like firmware. DT is not really the right spot for filenames.
If you can find a kernel-friendly way to do this then I'm all for it!
Hm, you have a point about filenames in DT. Maybe an inode number? (kidding :-)
:)
Quoted 3 lines
We could hand the driver a filename via sysfs file or module param. Perhaps use a filename in DT as a fallback.
If you can convince Rob, then I'd say go for it.
Quoted 11 lines
Quoted 11 lines
Quoted 10 lines
Quoted 4 lines
When using devices with NVM this "learned data" can be saved to the NVM and it will automatically be copied over to the run-time RAM sections on powerup. So we should only ever need to save the RAM section.
For the NVM chips, we'd want a userspace-accessible
reset_nvmfunction in driver which returns the pack-unique data to factory defaults. That can be provided via a sysfs file to which you write a key (safer than "echo 1 >/sys/class.../reset_nvm") to trigger the reset.
Quoted 9 lines
Quoted 4 lines
Still not sure why one would need this, the only time you would want to reset the gauge is when changing the battery, and that will already cause it to power cycle (unless you have some kind of back-up battery, but that doesn't make sense to me).
This is for gauges with NVM, where power-cycle does not clear the pack-specific stats. Or do those gauges recognize that the pack has changed?
The NVM *is* the factory default, RAM tracks the pack-specific stats, when the device power-cycles NVM values are automatically moved over into working RAM, so that is always cleared across all devices. There is really no good way to recognize that a pack has changed, so simply forgetting the pack-specific stats and re-loading the defaults is the safest option after a power-cycle. If we want to keep this info in RAM and re-use it, then it is up to the manufacturer to implement a solution for this detection (as only they can know what kind of device they are making and whether that detection makes sense or not for it).
Quoted 30 lines
Quoted 23 lines
Quoted 13 lines
Quoted 9 lines
Another thing to consider is that we (TI) have GUI tools to allow manufacturers to input battery info and have it generate a blob that can be deployed to these devices. The blob format is meant to be handled and uploaded by custom user-space tools as it only really needs to be done once (power-down events should only happen when changing batteries, in which case you want to let the device forget what it has learned about the old battery).
Quoted 3 lines
All of the details of this programming are probably not the job of the kernel, in end-user hands the kernel only needs to know how to read from the device, not program it.
Quoted 9 lines
Were there an independently maintained battery manager app with wide support for battery ICs, it would be a natural venue for BQ27* reset/save/restore_nvm functions. But there is none that I know of, and these features can be implemented in the driver with little new code beyond the read/write_dm_block functions added by this patchset.
It's fine to ask hardware makers to rely on IC vendor tools in the factory, but I don't think such tools belong in the end-user's stack.
Quoted 6 lines
If they don't belong in the end-user's user-space tool-set then they definitely don't belong in kernel-space. The other option is to define a whole new API for interfacing with all these device specific parameters and internal info tables. It's that or we accept that these are factory settings and we don't need all too much kernel infrastructure for handling them.
Quoted 5 lines
My point is that an IC vendor *codebase* does not belong in the end-user's stack; it's a production tool. The functionality we're discussing clearly belongs in both the device driver and the production tool.
What functionality?, specifying your battery's design capacity, sure, reprogramming the gauge's internal "Impedance Track™" algorithm data, not so much. It's not up to me to draw the line in what goes into the kernel driver vs user-space tools, at this point I think we can wait for Sebastian to weigh in on this.
Andrew
Quoted 86 lines
Quoted 86 lines
Quoted 86 lines
Quoted 86 lines
Take everything I say with a grain of salt though, I'm not on the team that makes these. I don't have much additional info not in the public data-sheets.
Andrew
Quoted 79 lines
Quoted 10 lines
Quoted 10 lines
That concept is beyond the scope of this patchset (and doesn't supersede it), but is something I'd like to implement for our chip, the *425.
Quoted 3 lines
The properties added in this series are okay, and it's good to have some sane defaults in DT, but looking forward this may not be enough to get full use out of these devices.
Quoted 7 lines
Quoted 3 lines
I'm relieved you're not opposed to our patch :-) Now I just need that guidance I mentioned re the chip family...
Still looking into it, I think for now would be easiest to add the register info to each chip family struct. The un-seal sequence should be generic enough already.
I'll wait for your full reply on that thread.
Quoted 58 lines
Quoted 58 lines
Quoted 45 lines
Quoted 45 lines
Cc Rob Herring <[email protected]>Signed-off-by Matt Ranostay <[email protected]>Signed-off-by Liam Breck <[email protected]>--- .../devicetree/bindings/power/supply/battery.txt | 37 ++++++++++++++++++++++ 1 file changed, 37 insertions(+) create mode 100644 Documentation/devicetree/bindings/power/supply/battery.txtDocumentation/devicetree/bindings/power/supply/battery.txt new file +30
-0@@ -0,0 +1,37 @@ +Battery Characteristics + +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 + +Future Properties must be named for the corresponding elements in +enum power_supply_property, defined in include/linux/power_supply.h. + +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"; + voltage-min-design-microvolt = <3200000>; + energy-full-design-microwatt-hours = <5290000>; + charge-full-design-microamp-hours = <1430000>; + }; + + charger: charger@11 { + .... + monitored-battery = <&bat>; + ...Quoted 8 lines
Documentation/devicetree/bindings/power/supply/battery.txt line 37
Quoted 8 lines
Documentation/devicetree/bindings/power/supply/battery.txt new file +7
-0+ }; + + fuel_gauge: fuel-gauge@22 { + .... + monitored-battery = <&bat>; + ... + };--
To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 1/8] devicetree: power: Add battery.txt
Quoted 66 lines · Andrew F. Davis
Quoted 65 lines · Liam Breck
Quoted 64 lines · Andrew F. Davis
Quoted 63 lines · Liam Breck
Quoted 62 lines · Andrew F. Davis
Quoted 61 lines · Liam Breck
[Dropped devicetree from CC]
Quoted 48 lines · Andrew F. Davis
Quoted 47 lines · Liam Breck
Hi Andrew, thanks for the drill-down on BQ27* chips.
Quoted 29 lines · Andrew F. Davis
Quoted 5 lines · Liam Breck
From: Matt Ranostay <[email protected]>
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.
I think some of the confusion about what a "smart-battery" binding would look like is due to a miss-understanding what it means to "self-identify". Originally the bq27xxx *was* the chip that lived its whole life inside a battery (the iphone battery is one such example and a good way to test the bq27000). This way it could be factory programmed with the battery's basics and then learn about the individual pack over time to provide better info on time to empty, discharge rates, etc..
Eventually, people wanted to have the battery become cheap and replaceable, so fuel-gauges were made that are "system side", and ride along with the device, not the battery. Now this has one big problem, how can we learn about a battery when it may be a new battery every time we boot up? So we added programmable memory to the devices, and programming this memory looks to be what this series is about.
Only the battery design info is programmed in this series, but much more battery info can be uploaded, including stuff that is not fixed and therefor cannot be added to the DT. Even the design parameters may not be fix if one changes the battery as different manufactures of the same battery type may have different design capacities.
Quoted 7 lines
The ideal use-case in a system with a system side gauge is the OS should periodically, or just before shutdown, read the battery's learned info and when the device is powered back up, if the battery serial number matches from before, re-upload this learned info to the gauge so it doesn't have to re-learn everything. All of this will need to happen in user-space, so what we really need is a user-space API for manipulating these properties.
We would need a feature in the driver to get/set the blob of pack- or cell-unique data. The userspace api is one sysfs file.
However the DT fixed-battery node could indicate when/how-often the object is stored, and where, e.g. filename. If the latter is given, the driver should write/read the file itself.
Quoted 7 lines
Quoted 2 lines
Is there some documentation about what registers should go in this blob?
This should all be available in the parts' technical reference (looks like page 24 for your device), but the blob isn't meant to be modified, the information/format is mostly internal to the part, all we should do is save blob / restore blob as needed.
Page 24 has "8.5.5 Data Block Summary" That?
Quoted 7 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 2 lines
Subclass IDs for NVM are 82, 88,104, 105. Do all those go in the blob?
Some devices (bq27421) do not contain an NVM, and so rely on the host to restore its "learned data" across power cycles with the same battery.
From your above statement, I inferred that NVM retains pack-unique
stats on poweroff. Below you state that NVM only retains factory defaults. Could you clarify?
Quoted 13 lines
Quoted 4 lines
Quoted 4 lines
Quoted 3 lines
Ah, so my concept for DT battery:nvm-storage: "filename" would apply to these RAM-only chips.
Quoted 8 lines
Quoted 5 lines
I think so, I was going to do something similar, but let userspace take care of the filename, or have it a fixed/module param like firmware. DT is not really the right spot for filenames.
If you can find a kernel-friendly way to do this then I'm all for it!
Hm, you have a point about filenames in DT. Maybe an inode number? (kidding :-)
Quoted 7 lines
:)
Quoted 3 lines
We could hand the driver a filename via sysfs file or module param. Perhaps use a filename in DT as a fallback.
If you can convince Rob, then I'd say go for it.
OK, I restate the proposal below.
Quoted 66 lines
Quoted 10 lines
Quoted 10 lines
Quoted 9 lines
Quoted 3 lines
When using devices with NVM this "learned data" can be saved to the NVM and it will automatically be copied over to the run-time RAM sections on powerup. So we should only ever need to save the RAM section.
For the NVM chips, we'd want a userspace-accessible
reset_nvmfunction in driver which returns the pack-unique data to factory defaults. That can be provided via a sysfs file to which you write a key (safer than "echo 1 >/sys/class.../reset_nvm") to trigger the reset.Quoted 9 lines
Quoted 4 lines
Still not sure why one would need this, the only time you would want to reset the gauge is when changing the battery, and that will already cause it to power cycle (unless you have some kind of back-up battery, but that doesn't make sense to me).
This is for gauges with NVM, where power-cycle does not clear the pack-specific stats. Or do those gauges recognize that the pack has changed?
The NVM *is* the factory default, RAM tracks the pack-specific stats, when the device power-cycles NVM values are automatically moved over into working RAM, so that is always cleared across all devices. There is really no good way to recognize that a pack has changed, so simply forgetting the pack-specific stats and re-loading the defaults is the safest option after a power-cycle. If we want to keep this info in RAM and re-use it, then it is up to the manufacturer to implement a solution for this detection (as only they can know what kind of device they are making and whether that detection makes sense or not for it).
Quoted 30 lines
Quoted 23 lines
Quoted 13 lines
Quoted 9 lines
Another thing to consider is that we (TI) have GUI tools to allow manufacturers to input battery info and have it generate a blob that can be deployed to these devices. The blob format is meant to be handled and uploaded by custom user-space tools as it only really needs to be done once (power-down events should only happen when changing batteries, in which case you want to let the device forget what it has learned about the old battery).
Quoted 3 lines
All of the details of this programming are probably not the job of the kernel, in end-user hands the kernel only needs to know how to read from the device, not program it.
Quoted 9 lines
Were there an independently maintained battery manager app with wide support for battery ICs, it would be a natural venue for BQ27* reset/save/restore_nvm functions. But there is none that I know of, and these features can be implemented in the driver with little new code beyond the read/write_dm_block functions added by this patchset.
It's fine to ask hardware makers to rely on IC vendor tools in the factory, but I don't think such tools belong in the end-user's stack.
Quoted 6 lines
If they don't belong in the end-user's user-space tool-set then they definitely don't belong in kernel-space. The other option is to define a whole new API for interfacing with all these device specific parameters and internal info tables. It's that or we accept that these are factory settings and we don't need all too much kernel infrastructure for handling them.
Quoted 5 lines
My point is that an IC vendor *codebase* does not belong in the end-user's stack; it's a production tool. The functionality we're discussing clearly belongs in both the device driver and the production tool.
Quoted 5 lines
What functionality?, specifying your battery's design capacity, sure, reprogramming the gauge's internal "Impedance Track™" algorithm data, not so much. It's not up to me to draw the line in what goes into the kernel driver vs user-space tools, at this point I think we can wait for Sebastian to weigh in on this.
The functionality of save/restore pack-unique data!
Restating the pack-unique data (PUD) storage proposal:
- Allow driver to archive/install PUD from/to chip RAM
- Provide userspace api to above features via sysfs file
- If driver has a PUD storage policy, it writes/reads a blob/csv on disk
- PUD storage policy fields: bool poweroff, int period, char* filename
- PUD storage policy may be set via sysfs file or module param
- DT may give PUD storage policy as a fallback
Quoted 85 lines
Quoted 85 lines
Quoted 85 lines
Quoted 85 lines
Quoted 85 lines
Take everything I say with a grain of salt though, I'm not on the team that makes these. I don't have much additional info not in the public data-sheets.
Andrew
Quoted 79 lines
Quoted 10 lines
Quoted 10 lines
That concept is beyond the scope of this patchset (and doesn't supersede it), but is something I'd like to implement for our chip, the *425.
Quoted 3 lines
The properties added in this series are okay, and it's good to have some sane defaults in DT, but looking forward this may not be enough to get full use out of these devices.
Quoted 7 lines
Quoted 3 lines
I'm relieved you're not opposed to our patch :-) Now I just need that guidance I mentioned re the chip family...
Still looking into it, I think for now would be easiest to add the register info to each chip family struct. The un-seal sequence should be generic enough already.
I'll wait for your full reply on that thread.
Quoted 58 lines
Quoted 58 lines
Quoted 45 lines
Quoted 45 lines
Cc Rob Herring <[email protected]>Signed-off-by Matt Ranostay <[email protected]>Signed-off-by Liam Breck <[email protected]>--- .../devicetree/bindings/power/supply/battery.txt | 37 ++++++++++++++++++++++ 1 file changed, 37 insertions(+) create mode 100644 Documentation/devicetree/bindings/power/supply/battery.txtDocumentation/devicetree/bindings/power/supply/battery.txt new file +30
-0@@ -0,0 +1,37 @@ +Battery Characteristics + +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 + +Future Properties must be named for the corresponding elements in +enum power_supply_property, defined in include/linux/power_supply.h. + +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"; + voltage-min-design-microvolt = <3200000>; + energy-full-design-microwatt-hours = <5290000>; + charge-full-design-microamp-hours = <1430000>; + }; + + charger: charger@11 { + .... + monitored-battery = <&bat>; + ...Quoted 8 lines
Documentation/devicetree/bindings/power/supply/battery.txt line 37
Quoted 8 lines
Documentation/devicetree/bindings/power/supply/battery.txt new file +7
-0+ }; + + fuel_gauge: fuel-gauge@22 { + .... + monitored-battery = <&bat>; + ... + };--
To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 1/8] devicetree: power: Add battery.txt
Quoted 70 lines · Liam Breck
Quoted 69 lines · Andrew F. Davis
Quoted 68 lines · Liam Breck
Quoted 67 lines · Andrew F. Davis
Quoted 66 lines · Liam Breck
Quoted 65 lines · Andrew F. Davis
Quoted 63 lines · Liam Breck
[Dropped devicetree from CC]
Quoted 48 lines · Andrew F. Davis
Quoted 47 lines · Liam Breck
Hi Andrew, thanks for the drill-down on BQ27* chips.
Quoted 29 lines · Andrew F. Davis
Quoted 5 lines · Liam Breck
From: Matt Ranostay <[email protected]>
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.
I think some of the confusion about what a "smart-battery" binding would look like is due to a miss-understanding what it means to "self-identify". Originally the bq27xxx *was* the chip that lived its whole life inside a battery (the iphone battery is one such example and a good way to test the bq27000). This way it could be factory programmed with the battery's basics and then learn about the individual pack over time to provide better info on time to empty, discharge rates, etc..
Eventually, people wanted to have the battery become cheap and replaceable, so fuel-gauges were made that are "system side", and ride along with the device, not the battery. Now this has one big problem, how can we learn about a battery when it may be a new battery every time we boot up? So we added programmable memory to the devices, and programming this memory looks to be what this series is about.
Only the battery design info is programmed in this series, but much more battery info can be uploaded, including stuff that is not fixed and therefor cannot be added to the DT. Even the design parameters may not be fix if one changes the battery as different manufactures of the same battery type may have different design capacities.
Quoted 7 lines
The ideal use-case in a system with a system side gauge is the OS should periodically, or just before shutdown, read the battery's learned info and when the device is powered back up, if the battery serial number matches from before, re-upload this learned info to the gauge so it doesn't have to re-learn everything. All of this will need to happen in user-space, so what we really need is a user-space API for manipulating these properties.
We would need a feature in the driver to get/set the blob of pack- or cell-unique data. The userspace api is one sysfs file.
However the DT fixed-battery node could indicate when/how-often the object is stored, and where, e.g. filename. If the latter is given, the driver should write/read the file itself.
Quoted 7 lines
Quoted 2 lines
Is there some documentation about what registers should go in this blob?
This should all be available in the parts' technical reference (looks like page 24 for your device), but the blob isn't meant to be modified, the information/format is mostly internal to the part, all we should do is save blob / restore blob as needed.
Page 24 has "8.5.5 Data Block Summary" That?
Subclass IDs for NVM are 82, 88,104, 105. Do all those go in the blob?
Quoted 7 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Some devices (bq27421) do not contain an NVM, and so rely on the host to restore its "learned data" across power cycles with the same battery.
From your above statement, I inferred that NVM retains pack-unique stats on poweroff. Below you state that NVM only retains factory defaults. Could you clarify?
This is a simple device, all that it knows is that it should move NVM stats to RAM when it powers up, and then it uses and updates RAM stats as it learns.
You can program the NVM values to the updated stats from RAM before power-off manually if you would like, so that on the next power-on it will move those updated values over to RAM. You should only do this if you know you will have the same battery after a power-cycle.
Out of the box, the stats in NVM are the factory defaults, so if you don't program NVM, and you power-cycle, you will get those factory defaults in RAM. You are free to overwrite both the NVM and RAM with updated values if you would like.
Quoted 97 lines
Quoted 13 lines
Quoted 4 lines
Quoted 4 lines
Quoted 3 lines
Ah, so my concept for DT battery:nvm-storage: "filename" would apply to these RAM-only chips.
Quoted 8 lines
Quoted 5 lines
I think so, I was going to do something similar, but let userspace take care of the filename, or have it a fixed/module param like firmware. DT is not really the right spot for filenames.
If you can find a kernel-friendly way to do this then I'm all for it!
Hm, you have a point about filenames in DT. Maybe an inode number? (kidding :-)
Quoted 7 lines
:)
Quoted 3 lines
We could hand the driver a filename via sysfs file or module param. Perhaps use a filename in DT as a fallback.
If you can convince Rob, then I'd say go for it.
OK, I restate the proposal below.
Quoted 66 lines
Quoted 10 lines
Quoted 10 lines
Quoted 9 lines
Quoted 3 lines
When using devices with NVM this "learned data" can be saved to the NVM and it will automatically be copied over to the run-time RAM sections on powerup. So we should only ever need to save the RAM section.
For the NVM chips, we'd want a userspace-accessible
reset_nvmfunction in driver which returns the pack-unique data to factory defaults. That can be provided via a sysfs file to which you write a key (safer than "echo 1 >/sys/class.../reset_nvm") to trigger the reset.Quoted 9 lines
Quoted 4 lines
Still not sure why one would need this, the only time you would want to reset the gauge is when changing the battery, and that will already cause it to power cycle (unless you have some kind of back-up battery, but that doesn't make sense to me).
This is for gauges with NVM, where power-cycle does not clear the pack-specific stats. Or do those gauges recognize that the pack has changed?
The NVM *is* the factory default, RAM tracks the pack-specific stats, when the device power-cycles NVM values are automatically moved over into working RAM, so that is always cleared across all devices. There is really no good way to recognize that a pack has changed, so simply forgetting the pack-specific stats and re-loading the defaults is the safest option after a power-cycle. If we want to keep this info in RAM and re-use it, then it is up to the manufacturer to implement a solution for this detection (as only they can know what kind of device they are making and whether that detection makes sense or not for it).
Quoted 30 lines
Quoted 23 lines
Quoted 13 lines
Quoted 9 lines
Another thing to consider is that we (TI) have GUI tools to allow manufacturers to input battery info and have it generate a blob that can be deployed to these devices. The blob format is meant to be handled and uploaded by custom user-space tools as it only really needs to be done once (power-down events should only happen when changing batteries, in which case you want to let the device forget what it has learned about the old battery).
Quoted 3 lines
All of the details of this programming are probably not the job of the kernel, in end-user hands the kernel only needs to know how to read from the device, not program it.
Quoted 9 lines
Were there an independently maintained battery manager app with wide support for battery ICs, it would be a natural venue for BQ27* reset/save/restore_nvm functions. But there is none that I know of, and these features can be implemented in the driver with little new code beyond the read/write_dm_block functions added by this patchset.
It's fine to ask hardware makers to rely on IC vendor tools in the factory, but I don't think such tools belong in the end-user's stack.
Quoted 6 lines
If they don't belong in the end-user's user-space tool-set then they definitely don't belong in kernel-space. The other option is to define a whole new API for interfacing with all these device specific parameters and internal info tables. It's that or we accept that these are factory settings and we don't need all too much kernel infrastructure for handling them.
Quoted 5 lines
My point is that an IC vendor *codebase* does not belong in the end-user's stack; it's a production tool. The functionality we're discussing clearly belongs in both the device driver and the production tool.
Quoted 5 lines
What functionality?, specifying your battery's design capacity, sure, reprogramming the gauge's internal "Impedance Track™" algorithm data, not so much. It's not up to me to draw the line in what goes into the kernel driver vs user-space tools, at this point I think we can wait for Sebastian to weigh in on this.
The functionality of save/restore pack-unique data!
Quoted 8 lines
Restating the pack-unique data (PUD) storage proposal:
- Allow driver to archive/install PUD from/to chip RAM
- Provide userspace api to above features via sysfs file
- If driver has a PUD storage policy, it writes/reads a blob/csv on disk
- PUD storage policy fields: bool poweroff, int period, char* filename
- PUD storage policy may be set via sysfs file or module param
- DT may give PUD storage policy as a fallback
I have no issue with any of this, but these are a bit out of the scope of this patchset, right? Does anything in this set prevent the future addition of those features?
Andrew
Quoted 86 lines
Quoted 86 lines
Quoted 86 lines
Quoted 86 lines
Quoted 86 lines
Quoted 86 lines
Take everything I say with a grain of salt though, I'm not on the team that makes these. I don't have much additional info not in the public data-sheets.
Andrew
Quoted 79 lines
Quoted 10 lines
Quoted 10 lines
That concept is beyond the scope of this patchset (and doesn't supersede it), but is something I'd like to implement for our chip, the *425.
Quoted 3 lines
The properties added in this series are okay, and it's good to have some sane defaults in DT, but looking forward this may not be enough to get full use out of these devices.
Quoted 7 lines
Quoted 3 lines
I'm relieved you're not opposed to our patch :-) Now I just need that guidance I mentioned re the chip family...
Still looking into it, I think for now would be easiest to add the register info to each chip family struct. The un-seal sequence should be generic enough already.
I'll wait for your full reply on that thread.
Quoted 58 lines
Quoted 58 lines
Quoted 45 lines
Quoted 45 lines
Cc Rob Herring <[email protected]>Signed-off-by Matt Ranostay <[email protected]>Signed-off-by Liam Breck <[email protected]>--- .../devicetree/bindings/power/supply/battery.txt | 37 ++++++++++++++++++++++ 1 file changed, 37 insertions(+) create mode 100644 Documentation/devicetree/bindings/power/supply/battery.txtDocumentation/devicetree/bindings/power/supply/battery.txt new file +30
-0@@ -0,0 +1,37 @@ +Battery Characteristics + +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 + +Future Properties must be named for the corresponding elements in +enum power_supply_property, defined in include/linux/power_supply.h. + +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"; + voltage-min-design-microvolt = <3200000>; + energy-full-design-microwatt-hours = <5290000>; + charge-full-design-microamp-hours = <1430000>; + }; + + charger: charger@11 { + .... + monitored-battery = <&bat>; + ...Quoted 8 lines
Documentation/devicetree/bindings/power/supply/battery.txt line 37
Quoted 8 lines
Documentation/devicetree/bindings/power/supply/battery.txt new file +7
-0+ }; + + fuel_gauge: fuel-gauge@22 { + .... + monitored-battery = <&bat>; + ... + };--
To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 1/8] devicetree: power: Add battery.txt
Quoted 88 lines · Andrew F. Davis
Quoted 70 lines · Liam Breck
Quoted 69 lines · Andrew F. Davis
Quoted 68 lines · Liam Breck
Quoted 67 lines · Andrew F. Davis
Quoted 66 lines · Liam Breck
Quoted 65 lines · Andrew F. Davis
Quoted 63 lines · Liam Breck
[Dropped devicetree from CC]
Quoted 48 lines · Andrew F. Davis
Quoted 47 lines · Liam Breck
Hi Andrew, thanks for the drill-down on BQ27* chips.
Quoted 29 lines · Andrew F. Davis
Quoted 5 lines · Liam Breck
From: Matt Ranostay <[email protected]>
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.
I think some of the confusion about what a "smart-battery" binding would look like is due to a miss-understanding what it means to "self-identify". Originally the bq27xxx *was* the chip that lived its whole life inside a battery (the iphone battery is one such example and a good way to test the bq27000). This way it could be factory programmed with the battery's basics and then learn about the individual pack over time to provide better info on time to empty, discharge rates, etc..
Eventually, people wanted to have the battery become cheap and replaceable, so fuel-gauges were made that are "system side", and ride along with the device, not the battery. Now this has one big problem, how can we learn about a battery when it may be a new battery every time we boot up? So we added programmable memory to the devices, and programming this memory looks to be what this series is about.
Only the battery design info is programmed in this series, but much more battery info can be uploaded, including stuff that is not fixed and therefor cannot be added to the DT. Even the design parameters may not be fix if one changes the battery as different manufactures of the same battery type may have different design capacities.
Quoted 7 lines
The ideal use-case in a system with a system side gauge is the OS should periodically, or just before shutdown, read the battery's learned info and when the device is powered back up, if the battery serial number matches from before, re-upload this learned info to the gauge so it doesn't have to re-learn everything. All of this will need to happen in user-space, so what we really need is a user-space API for manipulating these properties.
We would need a feature in the driver to get/set the blob of pack- or cell-unique data. The userspace api is one sysfs file.
However the DT fixed-battery node could indicate when/how-often the object is stored, and where, e.g. filename. If the latter is given, the driver should write/read the file itself.
Quoted 7 lines
Quoted 2 lines
Is there some documentation about what registers should go in this blob?
This should all be available in the parts' technical reference (looks like page 24 for your device), but the blob isn't meant to be modified, the information/format is mostly internal to the part, all we should do is save blob / restore blob as needed.
Page 24 has "8.5.5 Data Block Summary" That?
Subclass IDs for NVM are 82, 88,104, 105. Do all those go in the blob?
Quoted 7 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Some devices (bq27421) do not contain an NVM, and so rely on the host to restore its "learned data" across power cycles with the same battery.
From your above statement, I inferred that NVM retains pack-unique stats on poweroff. Below you state that NVM only retains factory defaults. Could you clarify?
This is a simple device, all that it knows is that it should move NVM stats to RAM when it powers up, and then it uses and updates RAM stats as it learns.
You can program the NVM values to the updated stats from RAM before power-off manually if you would like, so that on the next power-on it will move those updated values over to RAM. You should only do this if you know you will have the same battery after a power-cycle.
Quoted 4 lines
Out of the box, the stats in NVM are the factory defaults, so if you don't program NVM, and you power-cycle, you will get those factory defaults in RAM. You are free to overwrite both the NVM and RAM with updated values if you would like.
Looking at the 425 manual, 8.5.5 Data Block Summary, page 24, the only RAM & NVM blocks which correspond are the Ra Tables, 88 & 89. It doesn't appear that any other RAM block could be saved to NVM.
http://www.ti.com/lit/ds/symlink/bq27425-g2a.pdf#24
Is the Ra Table the only bq27* RAM data we should store on poweroff to NVM or disk?
Quoted 97 lines
Quoted 97 lines
Quoted 13 lines
Quoted 4 lines
Quoted 4 lines
Quoted 3 lines
Ah, so my concept for DT battery:nvm-storage: "filename" would apply to these RAM-only chips.
Quoted 8 lines
Quoted 5 lines
I think so, I was going to do something similar, but let userspace take care of the filename, or have it a fixed/module param like firmware. DT is not really the right spot for filenames.
If you can find a kernel-friendly way to do this then I'm all for it!
Hm, you have a point about filenames in DT. Maybe an inode number? (kidding :-)
Quoted 7 lines
:)
Quoted 3 lines
We could hand the driver a filename via sysfs file or module param. Perhaps use a filename in DT as a fallback.
If you can convince Rob, then I'd say go for it.
OK, I restate the proposal below.
Quoted 66 lines
Quoted 10 lines
Quoted 10 lines
Quoted 9 lines
Quoted 3 lines
When using devices with NVM this "learned data" can be saved to the NVM and it will automatically be copied over to the run-time RAM sections on powerup. So we should only ever need to save the RAM section.
For the NVM chips, we'd want a userspace-accessible
reset_nvmfunction in driver which returns the pack-unique data to factory defaults. That can be provided via a sysfs file to which you write a key (safer than "echo 1 >/sys/class.../reset_nvm") to trigger the reset.Quoted 9 lines
Quoted 4 lines
Still not sure why one would need this, the only time you would want to reset the gauge is when changing the battery, and that will already cause it to power cycle (unless you have some kind of back-up battery, but that doesn't make sense to me).
This is for gauges with NVM, where power-cycle does not clear the pack-specific stats. Or do those gauges recognize that the pack has changed?
The NVM *is* the factory default, RAM tracks the pack-specific stats, when the device power-cycles NVM values are automatically moved over into working RAM, so that is always cleared across all devices. There is really no good way to recognize that a pack has changed, so simply forgetting the pack-specific stats and re-loading the defaults is the safest option after a power-cycle. If we want to keep this info in RAM and re-use it, then it is up to the manufacturer to implement a solution for this detection (as only they can know what kind of device they are making and whether that detection makes sense or not for it).
Quoted 30 lines
Quoted 23 lines
Quoted 13 lines
Quoted 9 lines
Another thing to consider is that we (TI) have GUI tools to allow manufacturers to input battery info and have it generate a blob that can be deployed to these devices. The blob format is meant to be handled and uploaded by custom user-space tools as it only really needs to be done once (power-down events should only happen when changing batteries, in which case you want to let the device forget what it has learned about the old battery).
Quoted 3 lines
All of the details of this programming are probably not the job of the kernel, in end-user hands the kernel only needs to know how to read from the device, not program it.
Quoted 9 lines
Were there an independently maintained battery manager app with wide support for battery ICs, it would be a natural venue for BQ27* reset/save/restore_nvm functions. But there is none that I know of, and these features can be implemented in the driver with little new code beyond the read/write_dm_block functions added by this patchset.
It's fine to ask hardware makers to rely on IC vendor tools in the factory, but I don't think such tools belong in the end-user's stack.
Quoted 6 lines
If they don't belong in the end-user's user-space tool-set then they definitely don't belong in kernel-space. The other option is to define a whole new API for interfacing with all these device specific parameters and internal info tables. It's that or we accept that these are factory settings and we don't need all too much kernel infrastructure for handling them.
Quoted 5 lines
My point is that an IC vendor *codebase* does not belong in the end-user's stack; it's a production tool. The functionality we're discussing clearly belongs in both the device driver and the production tool.
Quoted 5 lines
What functionality?, specifying your battery's design capacity, sure, reprogramming the gauge's internal "Impedance Track™" algorithm data, not so much. It's not up to me to draw the line in what goes into the kernel driver vs user-space tools, at this point I think we can wait for Sebastian to weigh in on this.
The functionality of save/restore pack-unique data!
Quoted 7 lines
Quoted 7 lines
Restating the pack-unique data (PUD) storage proposal:
- Allow driver to archive/install PUD from/to chip RAM
- Provide userspace api to above features via sysfs file
- If driver has a PUD storage policy, it writes/reads a blob/csv on disk
- PUD storage policy fields: bool poweroff, int period, char* filename
- PUD storage policy may be set via sysfs file or module param
- DT may give PUD storage policy as a fallback
"Pack-unique data" should perhaps be "Ra Table" unless there's more to it.
One more feature for the above list:
- DT bq27xxx:save-ra-table saves table in NVM (if present) on poweroff I'll tackle this first since our board's chip has NVM.
Quoted 3 lines
I have no issue with any of this, but these are a bit out of the scope of this patchset, right? Does anything in this set prevent the future addition of those features?
Yes the above is outside scope of this patchset, but will use four functions added in it.
Quoted 85 lines
Quoted 85 lines
Quoted 85 lines
Quoted 85 lines
Quoted 85 lines
Quoted 85 lines
Quoted 85 lines
Take everything I say with a grain of salt though, I'm not on the team that makes these. I don't have much additional info not in the public data-sheets.
Andrew
Quoted 79 lines
Quoted 10 lines
Quoted 10 lines
That concept is beyond the scope of this patchset (and doesn't supersede it), but is something I'd like to implement for our chip, the *425.
Quoted 3 lines
The properties added in this series are okay, and it's good to have some sane defaults in DT, but looking forward this may not be enough to get full use out of these devices.
Quoted 7 lines
Quoted 3 lines
I'm relieved you're not opposed to our patch :-) Now I just need that guidance I mentioned re the chip family...
Still looking into it, I think for now would be easiest to add the register info to each chip family struct. The un-seal sequence should be generic enough already.
I'll wait for your full reply on that thread.
Quoted 58 lines
Quoted 58 lines
Quoted 45 lines
Quoted 45 lines
Cc Rob Herring <[email protected]>Signed-off-by Matt Ranostay <[email protected]>Signed-off-by Liam Breck <[email protected]>--- .../devicetree/bindings/power/supply/battery.txt | 37 ++++++++++++++++++++++ 1 file changed, 37 insertions(+) create mode 100644 Documentation/devicetree/bindings/power/supply/battery.txtDocumentation/devicetree/bindings/power/supply/battery.txt new file +30
-0@@ -0,0 +1,37 @@ +Battery Characteristics + +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 + +Future Properties must be named for the corresponding elements in +enum power_supply_property, defined in include/linux/power_supply.h. + +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"; + voltage-min-design-microvolt = <3200000>; + energy-full-design-microwatt-hours = <5290000>; + charge-full-design-microamp-hours = <1430000>; + }; + + charger: charger@11 { + .... + monitored-battery = <&bat>; + ...Quoted 8 lines
Documentation/devicetree/bindings/power/supply/battery.txt line 37
Quoted 8 lines
Documentation/devicetree/bindings/power/supply/battery.txt new file +7
-0+ }; + + fuel_gauge: fuel-gauge@22 { + .... + monitored-battery = <&bat>; + ... + };--
To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 1/8] devicetree: power: Add battery.txt
Quoted 94 lines · Liam Breck
Quoted 88 lines · Andrew F. Davis
Quoted 70 lines · Liam Breck
Quoted 69 lines · Andrew F. Davis
Quoted 68 lines · Liam Breck
Quoted 67 lines · Andrew F. Davis
Quoted 66 lines · Liam Breck
Quoted 65 lines · Andrew F. Davis
Quoted 63 lines · Liam Breck
[Dropped devicetree from CC]
Quoted 48 lines · Andrew F. Davis
Quoted 47 lines · Liam Breck
Hi Andrew, thanks for the drill-down on BQ27* chips.
Quoted 29 lines · Andrew F. Davis
Quoted 5 lines · Liam Breck
From: Matt Ranostay <[email protected]>
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.I think some of the confusion about what a "smart-battery" binding would look like is due to a miss-understanding what it means to "self-identify". Originally the bq27xxx *was* the chip that lived its whole life inside a battery (the iphone battery is one such example and a good way to test the bq27000). This way it could be factory programmed with the battery's basics and then learn about the individual pack over time to provide better info on time to empty, discharge rates, etc..
Eventually, people wanted to have the battery become cheap and replaceable, so fuel-gauges were made that are "system side", and ride along with the device, not the battery. Now this has one big problem, how can we learn about a battery when it may be a new battery every time we boot up? So we added programmable memory to the devices, and programming this memory looks to be what this series is about.
Only the battery design info is programmed in this series, but much more battery info can be uploaded, including stuff that is not fixed and therefor cannot be added to the DT. Even the design parameters may not be fix if one changes the battery as different manufactures of the same battery type may have different design capacities.
Quoted 7 lines
The ideal use-case in a system with a system side gauge is the OS should periodically, or just before shutdown, read the battery's learned info and when the device is powered back up, if the battery serial number matches from before, re-upload this learned info to the gauge so it doesn't have to re-learn everything. All of this will need to happen in user-space, so what we really need is a user-space API for manipulating these properties.
We would need a feature in the driver to get/set the blob of pack- or cell-unique data. The userspace api is one sysfs file.
However the DT fixed-battery node could indicate when/how-often the object is stored, and where, e.g. filename. If the latter is given, the driver should write/read the file itself.
Quoted 7 lines
Quoted 2 lines
Is there some documentation about what registers should go in this blob?
This should all be available in the parts' technical reference (looks like page 24 for your device), but the blob isn't meant to be modified, the information/format is mostly internal to the part, all we should do is save blob / restore blob as needed.
Page 24 has "8.5.5 Data Block Summary" That?
Subclass IDs for NVM are 82, 88,104, 105. Do all those go in the blob?
Quoted 7 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Some devices (bq27421) do not contain an NVM, and so rely on the host to restore its "learned data" across power cycles with the same battery.
From your above statement, I inferred that NVM retains pack-unique stats on poweroff. Below you state that NVM only retains factory defaults. Could you clarify?
This is a simple device, all that it knows is that it should move NVM stats to RAM when it powers up, and then it uses and updates RAM stats as it learns.
You can program the NVM values to the updated stats from RAM before power-off manually if you would like, so that on the next power-on it will move those updated values over to RAM. You should only do this if you know you will have the same battery after a power-cycle.
Quoted 4 lines
Out of the box, the stats in NVM are the factory defaults, so if you don't program NVM, and you power-cycle, you will get those factory defaults in RAM. You are free to overwrite both the NVM and RAM with updated values if you would like.
Quoted 8 lines
Looking at the 425 manual, 8.5.5 Data Block Summary, page 24, the only RAM & NVM blocks which correspond are the Ra Tables, 88 & 89. It doesn't appear that any other RAM block could be saved to NVM.
http://www.ti.com/lit/ds/symlink/bq27425-g2a.pdf#24Is the Ra Table the only bq27* RAM data we should store on poweroff to NVM or disk?
See below
Quoted 105 lines
Quoted 97 lines
Quoted 97 lines
Quoted 13 lines
Quoted 4 lines
Quoted 4 lines
Quoted 3 lines
Ah, so my concept for DT battery:nvm-storage: "filename" would apply to these RAM-only chips.
Quoted 8 lines
Quoted 5 lines
I think so, I was going to do something similar, but let userspace take care of the filename, or have it a fixed/module param like firmware. DT is not really the right spot for filenames.
If you can find a kernel-friendly way to do this then I'm all for it!
Hm, you have a point about filenames in DT. Maybe an inode number? (kidding :-)
Quoted 7 lines
:)
Quoted 3 lines
We could hand the driver a filename via sysfs file or module param. Perhaps use a filename in DT as a fallback.
If you can convince Rob, then I'd say go for it.
OK, I restate the proposal below.
Quoted 66 lines
Quoted 10 lines
Quoted 10 lines
Quoted 9 lines
Quoted 3 lines
When using devices with NVM this "learned data" can be saved to the NVM and it will automatically be copied over to the run-time RAM sections on powerup. So we should only ever need to save the RAM section.
For the NVM chips, we'd want a userspace-accessible
reset_nvmfunction in driver which returns the pack-unique data to factory defaults. That can be provided via a sysfs file to which you write a key (safer than "echo 1 >/sys/class.../reset_nvm") to trigger the reset.Quoted 9 lines
Quoted 4 lines
Still not sure why one would need this, the only time you would want to reset the gauge is when changing the battery, and that will already cause it to power cycle (unless you have some kind of back-up battery, but that doesn't make sense to me).
This is for gauges with NVM, where power-cycle does not clear the pack-specific stats. Or do those gauges recognize that the pack has changed?
The NVM *is* the factory default, RAM tracks the pack-specific stats, when the device power-cycles NVM values are automatically moved over into working RAM, so that is always cleared across all devices. There is really no good way to recognize that a pack has changed, so simply forgetting the pack-specific stats and re-loading the defaults is the safest option after a power-cycle. If we want to keep this info in RAM and re-use it, then it is up to the manufacturer to implement a solution for this detection (as only they can know what kind of device they are making and whether that detection makes sense or not for it).
Quoted 30 lines
Quoted 23 lines
Quoted 13 lines
Quoted 9 lines
Another thing to consider is that we (TI) have GUI tools to allow manufacturers to input battery info and have it generate a blob that can be deployed to these devices. The blob format is meant to be handled and uploaded by custom user-space tools as it only really needs to be done once (power-down events should only happen when changing batteries, in which case you want to let the device forget what it has learned about the old battery).
Quoted 3 lines
All of the details of this programming are probably not the job of the kernel, in end-user hands the kernel only needs to know how to read from the device, not program it.
Quoted 9 lines
Were there an independently maintained battery manager app with wide support for battery ICs, it would be a natural venue for BQ27* reset/save/restore_nvm functions. But there is none that I know of, and these features can be implemented in the driver with little new code beyond the read/write_dm_block functions added by this patchset.
It's fine to ask hardware makers to rely on IC vendor tools in the factory, but I don't think such tools belong in the end-user's stack.
Quoted 6 lines
If they don't belong in the end-user's user-space tool-set then they definitely don't belong in kernel-space. The other option is to define a whole new API for interfacing with all these device specific parameters and internal info tables. It's that or we accept that these are factory settings and we don't need all too much kernel infrastructure for handling them.
Quoted 5 lines
My point is that an IC vendor *codebase* does not belong in the end-user's stack; it's a production tool. The functionality we're discussing clearly belongs in both the device driver and the production tool.
Quoted 5 lines
What functionality?, specifying your battery's design capacity, sure, reprogramming the gauge's internal "Impedance Track™" algorithm data, not so much. It's not up to me to draw the line in what goes into the kernel driver vs user-space tools, at this point I think we can wait for Sebastian to weigh in on this.
The functionality of save/restore pack-unique data!
Quoted 7 lines
Quoted 7 lines
Restating the pack-unique data (PUD) storage proposal:
- Allow driver to archive/install PUD from/to chip RAM
- Provide userspace api to above features via sysfs file
- If driver has a PUD storage policy, it writes/reads a blob/csv on disk
- PUD storage policy fields: bool poweroff, int period, char* filename
- PUD storage policy may be set via sysfs file or module param
- DT may give PUD storage policy as a fallback
Quoted 2 lines
"Pack-unique data" should perhaps be "Ra Table" unless there's more to it.
I'm not really sure on this one, (or even what the Ra Table does exactly), this may be something that could be asked over in the fuel gauge forum on e2e[0], someone over there may have more info on intended uses of this.
Andrew
[0] http://e2e.ti.com/support/power_management/battery_management/f/180
Quoted 96 lines
One more feature for the above list:
- DT bq27xxx:save-ra-table saves table in NVM (if present) on poweroff I'll tackle this first since our board's chip has NVM.Quoted 3 lines
I have no issue with any of this, but these are a bit out of the scope of this patchset, right? Does anything in this set prevent the future addition of those features?
Yes the above is outside scope of this patchset, but will use four functions added in it.
Quoted 85 lines
Quoted 85 lines
Quoted 85 lines
Quoted 85 lines
Quoted 85 lines
Quoted 85 lines
Quoted 85 lines
Take everything I say with a grain of salt though, I'm not on the team that makes these. I don't have much additional info not in the public data-sheets.
Andrew
Quoted 79 lines
Quoted 10 lines
Quoted 10 lines
That concept is beyond the scope of this patchset (and doesn't supersede it), but is something I'd like to implement for our chip, the *425.
Quoted 3 lines
The properties added in this series are okay, and it's good to have some sane defaults in DT, but looking forward this may not be enough to get full use out of these devices.
Quoted 7 lines
Quoted 3 lines
I'm relieved you're not opposed to our patch :-) Now I just need that guidance I mentioned re the chip family...
Still looking into it, I think for now would be easiest to add the register info to each chip family struct. The un-seal sequence should be generic enough already.
I'll wait for your full reply on that thread.
Quoted 58 lines
Quoted 58 lines
Quoted 45 lines
Quoted 45 lines
Cc: Rob Herring <[email protected]>
Cc: [email protected]
Signed-off-by: Matt Ranostay <[email protected]>
Signed-off-by: Liam Breck <[email protected]>
---
.../devicetree/bindings/power/supply/battery.txt | 37 ++++++++++++++++++++++
1 file changed, 37 insertions(+)
create mode 100644 Documentation/devicetree/bindings/power/supply/battery.txtdiff --git a/Documentation/devicetree/bindings/power/supply/battery.txt b/Documentation/devicetree/bindings/power/supply/battery.txt
new file mode 100644
index 0000000..d78a4aa
--- /dev/null
+++ b/Documentation/devicetree/bindings/power/supply/battery.txt
@@ -0,0 +1,37 @@
+Battery Characteristics
+
+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
+
+Future Properties must be named for the corresponding elements in
+enum power_supply_property, defined in include/linux/power_supply.h.
+
+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";
+ voltage-min-design-microvolt = <3200000>;
+ energy-full-design-microwatt-hours = <5290000>;
+ charge-full-design-microamp-hours = <1430000>;
+ };
+
+ charger: charger@11 {
+ ....
+ monitored-battery = <&bat>;
+ ...Quoted 8 lines
Quoted 8 lines
+ };
+
+ fuel_gauge: fuel-gauge@22 {
+ ....
+ monitored-battery = <&bat>;
+ ...
+ };--
To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
PATCH v6 2/8 devicetree: property-units: Add uWh and uAh units
In reply to Liam BreckFrom: Matt Ranostay <[email protected]>
Add entries for microwatt-hours and microamp-hours.
--- Documentation/devicetree/bindings/property-units.txt | 2 ++ 1 file changed, 2 insertions(+)
Documentation/devicetree/bindings/property-units.txt +2 -0
@@ -25,8 +25,10 @@ Distance Electricity ---------------------------------------- -microamp : micro amps +-microamp-hours : micro amp-hours -ohms : Ohms -micro-ohms : micro Ohms +-microwatt-hours: micro Watt-hours -microvolt : micro volts Temperature
Signature · 6 lines
2.9.3 -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
PATCH v6 3/8 devicetree: power: bq27xxx: Add monitored-battery documentation
In reply to Liam BreckFrom: Matt Ranostay <[email protected]>
Document monitored-battery = <&battery_node>
--- Documentation/devicetree/bindings/power/supply/bq27xxx.txt | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-)
Documentation/devicetree/bindings/power/supply/bq27xxx.txt +10 -1
@@ -28,9 +28,18 @@ Required properties: * "ti,bq27621" - BQ27621 - reg: integer, i2c address of the device. +Optional properties: +- monitored-battery: phandle of battery information devicetree node + + See Documentation/devicetree/bindings/power/supply/battery.txt + If either of the referenced battery's *-full-design-*-hours properties are set, + then both must be. + Example: -bq27510g3 { +bq27510g3 : fuel-gauge@55 { compatible = "ti,bq27510g3"; reg = <0x55>; + + monitored-battery = <&bat>; };
Signature · 1 line
2.9.3
PATCH v6 4/8 power: power_supply: Add power_supply_battery_info and API
In reply to Liam BreckFrom: Matt Ranostay <[email protected]>
power_supply_get_battery_info() reads battery data from devicetree. struct power_supply_battery_info provides battery data to drivers. Drivers may surface battery data in sysfs via corresponding POWER_SUPPLY_PROP_* fields.
--- drivers/power/supply/power_supply_core.c | 40 ++++++++++++++++++++++++++++++++ include/linux/power_supply.h | 18 ++++++++++++++ 2 files changed, 58 insertions(+)
drivers/power/supply/power_supply_core.c +40 -0
@@ -17,6 +17,7 @@#include <linux/device.h> #include <linux/notifier.h> #include <linux/err.h> +#include <linux/of.h> #include <linux/power_supply.h> #include <linux/thermal.h> #include "power_supply.h"
@@ -487,6 +488,45 @@ struct power_supply *devm_power_supply_get_by_phandle(struct device *dev,EXPORT_SYMBOL_GPL (devm_power_supply_get_by_phandle);#endif /* CONFIG_OF */ +int power_supply_get_battery_info (struct power_supply *psy, +struct power_supply_battery_info *info) +{ +struct device_node *battery_np; +const char *value; +int err; + + info->energy_full_design_uwh = -EINVAL ; + info->charge_full_design_uah = -EINVAL ; + info->voltage_min_design_uv = -EINVAL ; + +if (!psy->of_node) { +dev_warn (&psy->dev,"%s currently only supports devicetree\n" , + __func__); +return -ENXIO ; + } + + battery_np =of_parse_phandle (psy->of_node,"monitored-battery" ,0 ); +if (!battery_np) +return -ENODEV ; + + err =of_property_read_string (battery_np,"compatible" , &value); +if (err) +return err; + +if (strcmp ("fixed-battery" , value)) +return -ENODEV ; + +of_property_read_u32 (battery_np,"energy-full-design-microwatt-hours" , + &info->energy_full_design_uwh); +of_property_read_u32 (battery_np,"charge-full-design-microamp-hours" , + &info->charge_full_design_uah); +of_property_read_u32 (battery_np,"voltage-min-design-microvolt" , + &info->voltage_min_design_uv); + +return 0 ; +} +EXPORT_SYMBOL_GPL (power_supply_get_battery_info); +int power_supply_get_property (struct power_supply *psy,enum power_supply_property psp,union power_supply_propval *val)
include/linux/power_supply.h +18 -0
@@ -288,6 +288,21 @@ struct power_supply_info {int use_for_apm; }; +/* +* This is the recommended struct to manage static battery parameters, +* populated by power_supply_get_battery_info(). Most platform drivers should +* use these for consistency. +* Its field names must correspond to elements in enum power_supply_property. +* The default field value is -EINVAL. +* Power supply class itself doesn't use this. +*/ + +struct power_supply_battery_info { +int energy_full_design_uwh;/* microWatt-hours */ +int charge_full_design_uah;/* microAmp-hours */ +int voltage_min_design_uv;/* microVolts */ +}; +extern struct atomic_notifier_head power_supply_notifier;extern int power_supply_reg_notifier (struct notifier_block *nb);extern void power_supply_unreg_notifier (struct notifier_block *nb);
@@ -306,6 +321,9 @@ static inline struct power_supply * devm_power_supply_get_by_phandle(struct device *dev, const char *property) {return NULL ; }#endif /* CONFIG_OF */ + +extern int power_supply_get_battery_info (struct power_supply *psy, +struct power_supply_battery_info *info);extern void power_supply_changed (struct power_supply *psy);extern int power_supply_am_i_supplied (struct power_supply *psy);extern int power_supply_set_battery_charged (struct power_supply *psy);
Signature · 1 line
2.9.3
PATCH v6 5/8 power: bq27xxx_battery: Define access methods to write chip registers
In reply to Liam BreckFrom: Matt Ranostay <[email protected]>
write(), read_bulk(), write_bulk() support setting chip registers, e.g. with data obtained from power_supply_battery_info.
--- include/linux/power/bq27xxx_battery.h | 3 +++ 1 file changed, 3 insertions(+)
include/linux/power/bq27xxx_battery.h +3 -0
@@ -32,6 +32,9 @@ struct bq27xxx_platform_data {struct bq27xxx_device_info ;struct bq27xxx_access_methods {int (*read)(struct bq27xxx_device_info *di,u8 reg,bool single); +int (*write)(struct bq27xxx_device_info *di,u8 reg,int value,bool single); +int (*read_bulk)(struct bq27xxx_device_info *di,u8 reg,u8 *data,int len); +int (*write_bulk)(struct bq27xxx_device_info *di,u8 reg,u8 *data,int len); };struct bq27xxx_reg_cache {
Signature · 1 line
2.9.3
PATCH v6 6/8 power: bq27xxx_battery: Add BQ27425 chip id
In reply to Liam BreckFrom: Matt Ranostay <[email protected]>
This patch does not alter the function of the driver. This chip ID is referenced in the devicetree code.
--- drivers/power/supply/bq27xxx_battery.c | 23 ++++++++++++++++++++++- drivers/power/supply/bq27xxx_battery_i2c.c | 2 +- include/linux/power/bq27xxx_battery.h | 3 ++- 3 files changed, 25 insertions(+), 3 deletions(-)
drivers/power/supply/bq27xxx_battery.c +22 -1
@@ -259,6 +259,25 @@ static u8 bq27xxx_regs[][BQ27XXX_REG_MAX] = { [BQ27XXX_REG_DCAP] = 0x3c, [BQ27XXX_REG_AP] = 0x18, }, + [BQ27425] = { + [BQ27XXX_REG_CTRL] = 0x00, + [BQ27XXX_REG_TEMP] = 0x02, + [BQ27XXX_REG_INT_TEMP] = 0x1e, + [BQ27XXX_REG_VOLT] = 0x04, + [BQ27XXX_REG_AI] = 0x10, + [BQ27XXX_REG_FLAGS] = 0x06, + [BQ27XXX_REG_TTE] = INVALID_REG_ADDR, + [BQ27XXX_REG_TTF] = INVALID_REG_ADDR, + [BQ27XXX_REG_TTES] = INVALID_REG_ADDR, + [BQ27XXX_REG_TTECP] = INVALID_REG_ADDR, + [BQ27XXX_REG_NAC] = 0x08, + [BQ27XXX_REG_FCC] = 0x0e, + [BQ27XXX_REG_CYCT] = INVALID_REG_ADDR, + [BQ27XXX_REG_AE] = INVALID_REG_ADDR, + [BQ27XXX_REG_SOC] = 0x1c, + [BQ27XXX_REG_DCAP] = 0x3c, + [BQ27XXX_REG_AP] = 0x18, + }, };static enum power_supply_property bq27000_battery_props[] = {
@@ -427,6 +446,7 @@ static struct {BQ27XXX_PROP (BQ27541 , bq27541_battery_props),BQ27XXX_PROP (BQ27545 , bq27545_battery_props),BQ27XXX_PROP (BQ27421 , bq27421_battery_props), + BQ27XXX_PROP(BQ27425, bq27421_battery_props), };static DEFINE_MUTEX (bq27xxx_list_lock);
@@ -682,6 +702,7 @@ static bool bq27xxx_battery_overtemp(struct bq27xxx_device_info *di, u16 flags)return flags & (BQ27XXX_FLAG_OTC |BQ27XXX_FLAG_OTD );case BQ27530 :case BQ27421 : +case BQ27425 :return flags &BQ27XXX_FLAG_OT ;default :return false;
@@ -693,7 +714,7 @@ static bool bq27xxx_battery_overtemp(struct bq27xxx_device_info *di, u16 flags)*/ static bool bq27xxx_battery_undertemp (struct bq27xxx_device_info *di,u16 flags) { -if (di->chip ==BQ27530 || di->chip ==BQ27421 ) +if (di->chip ==BQ27530 || di->chip ==BQ27421 || di->chip ==BQ27425 )return flags &BQ27XXX_FLAG_UT ;return false;
drivers/power/supply/bq27xxx_battery_i2c.c +1 -1
@@ -159,7 +159,7 @@ static const struct i2c_device_id bq27xxx_i2c_id_table[] = { { "bq27742", BQ27541 }, { "bq27545", BQ27545 }, { "bq27421", BQ27421 }, - { "bq27425", BQ27421 }, + { "bq27425", BQ27425 }, { "bq27441", BQ27421 }, { "bq27621", BQ27421 }, {},
include/linux/power/bq27xxx_battery.h +2 -1
@@ -9,7 +9,8 @@ enum bq27xxx_chip {BQ27530 ,/* bq27530, bq27531 */ BQ27541 ,/* bq27541, bq27542, bq27546, bq27742 */ BQ27545 ,/* bq27545 */ - BQ27421, /* bq27421, bq27425, bq27441, bq27621 */ +BQ27421 ,/* bq27421, bq27441, bq27621 */ + BQ27425, /* bq27425 */ };/**
Signature · 1 line
2.9.3
PATCH v6 7/8 power: bq27xxx_battery: Add power_supply_battery_info support
In reply to Liam BreckFrom: Matt Ranostay <[email protected]>
Previously there was no way to set chip registers in the event that the defaults didn't match the battery in question.
BQ27xxx driver now calls power_supply_get_battery_info, checks the inputs, and writes battery data to non-volatile memory.
--- drivers/power/supply/bq27xxx_battery.c | 302 ++++++++++++++++++++++++++++++++- 1 file changed, 301 insertions(+), 1 deletion(-)
drivers/power/supply/bq27xxx_battery.c +301 -1
@@ -37,6 +37,7 @@* http://www.ti.com/product/bq27621-g1 */ +#include <linux/delay.h> #include <linux/device.h> #include <linux/module.h> #include <linux/mutex.h>
@@ -452,6 +453,59 @@ static struct {static DEFINE_MUTEX (bq27xxx_list_lock);static LIST_HEAD (bq27xxx_battery_devices); +#define BQ27XXX_TERM_V_MIN 2800 +#define BQ27XXX_TERM_V_MAX 3700 + +#define BQ27XXX_BLOCK_DATA_CLASS 0x3E +#define BQ27XXX_DATA_BLOCK 0x3F +#define BQ27XXX_BLOCK_DATA 0x40 +#define BQ27XXX_BLOCK_DATA_CHECKSUM 0x60 +#define BQ27XXX_BLOCK_DATA_CONTROL 0x61 +#define BQ27XXX_SET_CFGUPDATE 0x13 +#define BQ27XXX_SOFT_RESET 0x42 +#define BQ27XXX_SUBCLASS_STATE_NVM 82 + +struct bq27xxx_dm_buf { +u8 a[32 ]; +}; + +struct bq27xxx_dm_reg { +unsigned int subclass_id; +unsigned int offset; +unsigned int bytes; +char *name; +}; + +#define BQ27XXX_DM_SUPPORTED (r) ( \ + r->subclass_id == BQ27XXX_SUBCLASS_STATE_NVM \ + && r->offset <= 30 \ + && r->bytes == 2 \ +) + +enum bq27xxx_dm_reg_id { +BQ27XXX_DM_DESIGN_CAPACITY =0 , +BQ27XXX_DM_DESIGN_ENERGY , +BQ27XXX_DM_TERMINATE_VOLTAGE , +BQ27XXX_DM_END , +}; + +static struct bq27xxx_dm_reg bq27425_dm_regs[] = { + [BQ27XXX_DM_DESIGN_CAPACITY ] = + {BQ27XXX_SUBCLASS_STATE_NVM ,12 ,2 ,"design-capacity" }, + [BQ27XXX_DM_DESIGN_ENERGY ] = + {BQ27XXX_SUBCLASS_STATE_NVM ,14 ,2 ,"design-energy" }, + [BQ27XXX_DM_TERMINATE_VOLTAGE ] = + {BQ27XXX_SUBCLASS_STATE_NVM ,18 ,2 ,"terminate-voltage" }, +}; + +static struct bq27xxx_dm_reg *bq27xxx_dm_regs[] = { + [BQ27425 ] = bq27425_dm_regs, +}; + +static unsigned int bq27xxx_unseal_keys[] = { + [BQ27425 ] =0x04143672 , +}; +static int poll_interval_param_set (const char *val,const struct kernel_param *kp) {struct bq27xxx_device_info *di;
@@ -496,6 +550,183 @@ static inline int bq27xxx_read(struct bq27xxx_device_info *di, int reg_index,return di->bus.read (di, di->regs[reg_index], single); } +static int bq27xxx_battery_set_seal_state (struct bq27xxx_device_info *di, +bool state) +{ +unsigned int key = bq27xxx_unseal_keys[di->chip]; +int ret; + +if (state) +return di->bus.write (di,BQ27XXX_REG_CTRL ,0x20 , false); + + ret = di->bus.write (di,BQ27XXX_REG_CTRL , (key >>16 ) &0xffff , false); +if (ret <0 ) +return ret; + +return di->bus.write (di,BQ27XXX_REG_CTRL , key &0xffff , false); +} + +static int bq27xxx_battery_read_dm_block (struct bq27xxx_device_info *di, +int subclass, +struct bq27xxx_dm_buf *buf) +{ +int ret; + + ret = di->bus.write (di,BQ27XXX_REG_CTRL ,0 , false); +if (ret <0 ) +return ret; + + ret = di->bus.write (di,BQ27XXX_BLOCK_DATA_CONTROL ,0 , true); +if (ret <0 ) +return ret; + + ret = di->bus.write (di,BQ27XXX_BLOCK_DATA_CLASS , subclass, true); +if (ret <0 ) +return ret; + + ret = di->bus.write (di,BQ27XXX_DATA_BLOCK ,0 , true); +if (ret <0 ) +return ret; + +usleep_range (1000 ,1500 ); + +return di->bus.read_bulk (di,BQ27XXX_BLOCK_DATA , buf->a,sizeof buf->a); +} + +static int bq27xxx_battery_print_config (struct bq27xxx_device_info *di) +{ +struct bq27xxx_dm_buf buf; +struct bq27xxx_dm_reg *reg = bq27xxx_dm_regs[di->chip]; +int ret, i; + + ret =bq27xxx_battery_read_dm_block (di,BQ27XXX_SUBCLASS_STATE_NVM , + &buf); +if (ret <0 ) +return ret; + +for (i =0 ; i <BQ27XXX_DM_END ; i++, reg++) { +int val; + +if (!BQ27XXX_DM_SUPPORTED (reg)) { +dev_warn (di->dev,"unsupported config register %s\n" , reg->name); +continue ; + } + + val =be16_to_cpup ((u16 *) &buf.a[reg->offset]); + +dev_info (di->dev,"config register %s set at %d\n" , reg->name, val); + } + +return 0 ; +} + +static bool bq27xxx_battery_update_dm_block (struct bq27xxx_device_info *di, +struct bq27xxx_dm_buf *buf, +enum bq27xxx_dm_reg_id reg_id, +unsigned int val) +{ +struct bq27xxx_dm_reg *reg = &bq27xxx_dm_regs[di->chip][reg_id]; +u16 *prev; + +if (!BQ27XXX_DM_SUPPORTED (reg)) +return false; + + prev = (u16 *) &buf->a[reg->offset]; + +if (be16_to_cpup (prev) == val) +return false; + + *prev =cpu_to_be16 (val); + +return true; +} + +static u8 bq27xxx_battery_checksum (struct bq27xxx_dm_buf *buf) +{ +u16 sum =0 ; +int i; + +for (i =0 ; i <sizeof buf->a; i++) { + sum += buf->a[i]; + sum &=0xff ; + } + +return 0xff - sum; +} + +static int bq27xxx_battery_write_dm_block (struct bq27xxx_device_info *di, +int subclass, +struct bq27xxx_dm_buf *buf) +{ +int ret; + + ret = di->bus.write (di,BQ27XXX_REG_CTRL ,BQ27XXX_SET_CFGUPDATE , false); +if (ret <0 ) +return ret; + + ret = di->bus.write (di,BQ27XXX_BLOCK_DATA_CONTROL ,0 , true); +if (ret <0 ) +return ret; + + ret = di->bus.write (di,BQ27XXX_BLOCK_DATA_CLASS , subclass, true); +if (ret <0 ) +return ret; + + ret = di->bus.write (di,BQ27XXX_DATA_BLOCK ,0 , true); +if (ret <0 ) +return ret; + + ret = di->bus.write_bulk (di,BQ27XXX_BLOCK_DATA , buf->a,sizeof buf->a); +if (ret <0 ) +return ret; + +usleep_range (1000 ,1500 ); + + ret = di->bus.write (di,BQ27XXX_BLOCK_DATA_CHECKSUM , +bq27xxx_battery_checksum (buf), true); +if (ret <0 ) +return ret; + +usleep_range (1000 ,1500 ); + +return di->bus.write (di,BQ27XXX_REG_CTRL ,BQ27XXX_SOFT_RESET , false); +} + +static int bq27xxx_battery_set_config (struct bq27xxx_device_info *di, +struct power_supply_battery_info *info) +{ +struct bq27xxx_dm_buf buf; +int ret; + + ret =bq27xxx_battery_read_dm_block (di,BQ27XXX_SUBCLASS_STATE_NVM , + &buf); +if (ret <0 ) +return ret; + +if (info->charge_full_design_uah != -EINVAL + && info->energy_full_design_uwh != -EINVAL ) { + ret |=bq27xxx_battery_update_dm_block (di, &buf, +BQ27XXX_DM_DESIGN_CAPACITY , + info->charge_full_design_uah /1000 ); + ret |=bq27xxx_battery_update_dm_block (di, &buf, +BQ27XXX_DM_DESIGN_ENERGY , + info->energy_full_design_uwh /1000 ); + } + +if (info->voltage_min_design_uv != -EINVAL ) + ret |=bq27xxx_battery_update_dm_block (di, &buf, +BQ27XXX_DM_TERMINATE_VOLTAGE , + info->voltage_min_design_uv /1000 ); + +if (ret) { +dev_info (di->dev,"updating NVM settings\n" ); +return bq27xxx_battery_write_dm_block (di,BQ27XXX_SUBCLASS_STATE_NVM , + &buf); + } + +return 0 ; +} +/* * Return the battery State-of-Charge * Or < 0 if something fails.
@@ -757,6 +988,64 @@ static int bq27xxx_battery_read_health(struct bq27xxx_device_info *di)return POWER_SUPPLY_HEALTH_GOOD ; } +void bq27xxx_battery_settings (struct bq27xxx_device_info *di) +{ +struct power_supply_battery_info info = {}; + +/* functions don't exist for writing data so abort */ +if (!di->bus.write || !di->bus.write_bulk) +return ; + +/* no settings to be set for this chipset so abort */ +if (!bq27xxx_dm_regs[di->chip]) +return ; + +bq27xxx_battery_set_seal_state (di, false); + +if (power_supply_get_battery_info (di->bat, &info) <0 ) + goto out; + +if (info.energy_full_design_uwh != info.charge_full_design_uah) { +if (info.energy_full_design_uwh == -EINVAL ) +dev_warn (di->dev, +"missing battery:energy-full-design-microwatt-hours\n" ); +else if (info.charge_full_design_uah == -EINVAL ) +dev_warn (di->dev, +"missing battery:charge-full-design-microamp-hours\n" ); + } + +if (info.energy_full_design_uwh >0x7fff *1000 ) { +dev_err (di->dev,"invalid battery:energy-full-design-microwatt-hours %d\n" , + info.energy_full_design_uwh); + info.energy_full_design_uwh = -EINVAL ; + } + +if (info.charge_full_design_uah >0x7fff *1000 ) { +dev_err (di->dev,"invalid battery:charge-full-design-microamp-hours %d\n" , + info.charge_full_design_uah); + info.charge_full_design_uah = -EINVAL ; + } + +if ((info.voltage_min_design_uv <BQ27XXX_TERM_V_MIN *1000 + || info.voltage_min_design_uv >BQ27XXX_TERM_V_MAX *1000 ) + && info.voltage_min_design_uv != -EINVAL ) { +dev_err (di->dev,"invalid battery:voltage-min-design-microvolt %d\n" , + info.voltage_min_design_uv); + info.voltage_min_design_uv = -EINVAL ; + } + +if ((info.energy_full_design_uwh == -EINVAL + || info.charge_full_design_uah == -EINVAL ) + && info.voltage_min_design_uv == -EINVAL ) + goto out; + +bq27xxx_battery_set_config (di, &info); + +out: +bq27xxx_battery_print_config (di); +bq27xxx_battery_set_seal_state (di, true); +} +void bq27xxx_battery_update (struct bq27xxx_device_info *di) {struct bq27xxx_reg_cache cache = {0 , };
@@ -1006,6 +1295,13 @@ static int bq27xxx_battery_get_property(struct power_supply *psy,case POWER_SUPPLY_PROP_CHARGE_FULL_DESIGN : ret =bq27xxx_simple_value (di->charge_design_full, val);break ; +/* +* TODO: Implement these to make registers set from +* power_supply_battery_info visible in sysfs. +*/ +case POWER_SUPPLY_PROP_ENERGY_FULL_DESIGN : +case POWER_SUPPLY_PROP_VOLTAGE_MIN_DESIGN : +return -EINVAL ;case POWER_SUPPLY_PROP_CYCLE_COUNT : ret =bq27xxx_simple_value (di->cache.cycle_count, val);break ;
@@ -1039,7 +1335,10 @@ static void bq27xxx_external_power_changed(struct power_supply *psy)int bq27xxx_battery_setup (struct bq27xxx_device_info *di) {struct power_supply_desc *psy_desc; -struct power_supply_config psy_cfg = { .drv_data = di, }; +struct power_supply_config psy_cfg = { + .of_node = di->dev->of_node, + .drv_data = di, + };INIT_DELAYED_WORK (&di->work, bq27xxx_battery_poll);mutex_init (&di->lock);
@@ -1064,6 +1363,7 @@ int bq27xxx_battery_setup(struct bq27xxx_device_info *di)dev_info (di->dev,"support ver. %s enabled\n" ,DRIVER_VERSION ); +bq27xxx_battery_settings (di);bq27xxx_battery_update (di);mutex_lock (&bq27xxx_list_lock);
Signature · 1 line
2.9.3
Re: [PATCH v6 7/8] power: bq27xxx_battery: Add power_supply_battery_info support
Hi Andrew,
Based on Sebastian's feedback on v1-4 of this patchset, I believe it will go upstream. (He hasn't written anything on linux-pm since before I posted v5 a week ago.)
Could we get your feedback on making this adaptable to the rest of the BQ27 family? Specifically...
Is the op sequence in these functions common across the chips?
bq27xxx_battery_set_seal_state()bq27xxx_battery_read_dm_block()bq27xxx_battery_write_dm_block()
Should we move these constants to bq27xxx_regs[chip][name]?
/* writable registers */
#define BQ27XXX_DATA_CLASS 0x3E #define BQ27XXX_DATA_BLOCK 0x3F #define BQ27XXX_BLOCK_DATA 0x40 #define BQ27XXX_BLOCK_DATA_CHECKSUM 0x60 #define BQ27XXX_BLOCK_DATA_CONTROL 0x61
/* writable register inputs */
#define BQ27XXX_SET_CFGUPDATE 0x13 #define BQ27XXX_SOFT_RESET 0x42 #define BQ27XXX_SUBCLASS_STATE_NVM 82
If you are dead set against including this in bq27xxx_battery, we could site the code in a companion module loaded by the I2C module. Then the main module calls just one function.
Thanks,
Liam
Quoted 372 lines · Liam Breck
From: Matt Ranostay <[email protected]>
Previously there was no way to set chip registers in the event that the defaults didn't match the battery in question.
BQ27xxx driver now calls
power_supply_get_battery_info, checks the inputs, and writes battery data to non-volatile memory.Signed-off-by Matt Ranostay <[email protected]>Signed-off-by Liam Breck <[email protected]>--- drivers/power/supply/bq27xxx_battery.c | 302 ++++++++++++++++++++++++++++++++- 1 file changed, 301 insertions(+), 1 deletion(-)drivers/power/supply/bq27xxx_battery.c +301
-2@@ -37,6 +37,7 @@* http://www.ti.com/product/bq27621-g1 */ +#include <linux/delay.h> #include <linux/device.h> #include <linux/module.h> #include <linux/mutex.h> @@ -452,6 +453,59 @@ static struct {static DEFINE_MUTEX (bq27xxx_list_lock);static LIST_HEAD (bq27xxx_battery_devices); +#define BQ27XXX_TERM_V_MIN 2800 +#define BQ27XXX_TERM_V_MAX 3700 + +#define BQ27XXX_BLOCK_DATA_CLASS 0x3E +#define BQ27XXX_DATA_BLOCK 0x3F +#define BQ27XXX_BLOCK_DATA 0x40 +#define BQ27XXX_BLOCK_DATA_CHECKSUM 0x60 +#define BQ27XXX_BLOCK_DATA_CONTROL 0x61 +#define BQ27XXX_SET_CFGUPDATE 0x13 +#define BQ27XXX_SOFT_RESET 0x42 +#define BQ27XXX_SUBCLASS_STATE_NVM 82 + +struct bq27xxx_dm_buf { +u8 a[32 ]; +}; + +struct bq27xxx_dm_reg { +unsigned int subclass_id; +unsigned int offset; +unsigned int bytes; +char *name; +}; + +#define BQ27XXX_DM_SUPPORTED (r) ( \ + r->subclass_id == BQ27XXX_SUBCLASS_STATE_NVM \ + && r->offset <= 30 \ + && r->bytes == 2 \ +) + +enum bq27xxx_dm_reg_id { +BQ27XXX_DM_DESIGN_CAPACITY =0 , +BQ27XXX_DM_DESIGN_ENERGY , +BQ27XXX_DM_TERMINATE_VOLTAGE , +BQ27XXX_DM_END , +}; + +static struct bq27xxx_dm_reg bq27425_dm_regs[] = { + [BQ27XXX_DM_DESIGN_CAPACITY ] = + {BQ27XXX_SUBCLASS_STATE_NVM ,12 ,2 ,"design-capacity" }, + [BQ27XXX_DM_DESIGN_ENERGY ] = + {BQ27XXX_SUBCLASS_STATE_NVM ,14 ,2 ,"design-energy" }, + [BQ27XXX_DM_TERMINATE_VOLTAGE ] = + {BQ27XXX_SUBCLASS_STATE_NVM ,18 ,2 ,"terminate-voltage" }, +}; + +static struct bq27xxx_dm_reg *bq27xxx_dm_regs[] = { + [BQ27425 ] = bq27425_dm_regs, +}; + +static unsigned int bq27xxx_unseal_keys[] = { + [BQ27425 ] =0x04143672 , +}; +static int poll_interval_param_set (const char *val,const struct kernel_param *kp) {struct bq27xxx_device_info *di;@@ -496,6 +550,183 @@ static inline int bq27xxx_read(struct bq27xxx_device_info *di, int reg_index,return di->bus.read (di, di->regs[reg_index], single); } +static int bq27xxx_battery_set_seal_state (struct bq27xxx_device_info *di, +bool state) +{ +unsigned int key = bq27xxx_unseal_keys[di->chip]; +int ret; + +if (state) +return di->bus.write (di,BQ27XXX_REG_CTRL ,0x20 , false); + + ret = di->bus.write (di,BQ27XXX_REG_CTRL , (key >>16 ) &0xffff , false); +if (ret <0 ) +return ret; + +return di->bus.write (di,BQ27XXX_REG_CTRL , key &0xffff , false); +} + +static int bq27xxx_battery_read_dm_block (struct bq27xxx_device_info *di, +int subclass, +struct bq27xxx_dm_buf *buf) +{ +int ret; + + ret = di->bus.write (di,BQ27XXX_REG_CTRL ,0 , false); +if (ret <0 ) +return ret; + + ret = di->bus.write (di,BQ27XXX_BLOCK_DATA_CONTROL ,0 , true); +if (ret <0 ) +return ret; + + ret = di->bus.write (di,BQ27XXX_BLOCK_DATA_CLASS , subclass, true); +if (ret <0 ) +return ret; + + ret = di->bus.write (di,BQ27XXX_DATA_BLOCK ,0 , true); +if (ret <0 ) +return ret; + +usleep_range (1000 ,1500 ); + +return di->bus.read_bulk (di,BQ27XXX_BLOCK_DATA , buf->a,sizeof buf->a); +} + +static int bq27xxx_battery_print_config (struct bq27xxx_device_info *di) +{ +struct bq27xxx_dm_buf buf; +struct bq27xxx_dm_reg *reg = bq27xxx_dm_regs[di->chip]; +int ret, i; + + ret =bq27xxx_battery_read_dm_block (di,BQ27XXX_SUBCLASS_STATE_NVM , + &buf); +if (ret <0 ) +return ret; + +for (i =0 ; i <BQ27XXX_DM_END ; i++, reg++) { +int val; + +if (!BQ27XXX_DM_SUPPORTED (reg)) { +dev_warn (di->dev,"unsupported config register %s\n" , reg->name); +continue ; + } + + val =be16_to_cpup ((u16 *) &buf.a[reg->offset]); + +dev_info (di->dev,"config register %s set at %d\n" , reg->name, val); + } + +return 0 ; +} + +static bool bq27xxx_battery_update_dm_block (struct bq27xxx_device_info *di, +struct bq27xxx_dm_buf *buf, +enum bq27xxx_dm_reg_id reg_id, +unsigned int val) +{ +struct bq27xxx_dm_reg *reg = &bq27xxx_dm_regs[di->chip][reg_id]; +u16 *prev; + +if (!BQ27XXX_DM_SUPPORTED (reg)) +return false; + + prev = (u16 *) &buf->a[reg->offset]; + +if (be16_to_cpup (prev) == val) +return false; + + *prev =cpu_to_be16 (val); + +return true; +} + +static u8 bq27xxx_battery_checksum (struct bq27xxx_dm_buf *buf) +{ +u16 sum =0 ; +int i; + +for (i =0 ; i <sizeof buf->a; i++) { + sum += buf->a[i]; + sum &=0xff ; + } + +return 0xff - sum; +} + +static int bq27xxx_battery_write_dm_block (struct bq27xxx_device_info *di, +int subclass, +struct bq27xxx_dm_buf *buf) +{ +int ret; + + ret = di->bus.write (di,BQ27XXX_REG_CTRL ,BQ27XXX_SET_CFGUPDATE , false); +if (ret <0 ) +return ret; + + ret = di->bus.write (di,BQ27XXX_BLOCK_DATA_CONTROL ,0 , true); +if (ret <0 ) +return ret; + + ret = di->bus.write (di,BQ27XXX_BLOCK_DATA_CLASS , subclass, true); +if (ret <0 ) +return ret; + + ret = di->bus.write (di,BQ27XXX_DATA_BLOCK ,0 , true); +if (ret <0 ) +return ret; + + ret = di->bus.write_bulk (di,BQ27XXX_BLOCK_DATA , buf->a,sizeof buf->a); +if (ret <0 ) +return ret; + +usleep_range (1000 ,1500 ); + + ret = di->bus.write (di,BQ27XXX_BLOCK_DATA_CHECKSUM , +bq27xxx_battery_checksum (buf), true); +if (ret <0 ) +return ret; + +usleep_range (1000 ,1500 ); + +return di->bus.write (di,BQ27XXX_REG_CTRL ,BQ27XXX_SOFT_RESET , false); +} + +static int bq27xxx_battery_set_config (struct bq27xxx_device_info *di, +struct power_supply_battery_info *info) +{ +struct bq27xxx_dm_buf buf; +int ret; + + ret =bq27xxx_battery_read_dm_block (di,BQ27XXX_SUBCLASS_STATE_NVM , + &buf); +if (ret <0 ) +return ret; + +if (info->charge_full_design_uah != -EINVAL + && info->energy_full_design_uwh != -EINVAL ) { + ret |=bq27xxx_battery_update_dm_block (di, &buf, +BQ27XXX_DM_DESIGN_CAPACITY , + info->charge_full_design_uah /1000 ); + ret |=bq27xxx_battery_update_dm_block (di, &buf, +BQ27XXX_DM_DESIGN_ENERGY , + info->energy_full_design_uwh /1000 ); + } + +if (info->voltage_min_design_uv != -EINVAL ) + ret |=bq27xxx_battery_update_dm_block (di, &buf, +BQ27XXX_DM_TERMINATE_VOLTAGE , + info->voltage_min_design_uv /1000 ); + +if (ret) { +dev_info (di->dev,"updating NVM settings\n" ); +return bq27xxx_battery_write_dm_block (di,BQ27XXX_SUBCLASS_STATE_NVM , + &buf); + } + +return 0 ; +} +/* * Return the battery State-of-Charge * Or < 0 if something fails. @@ -757,6 +988,64 @@ static int bq27xxx_battery_read_health(struct bq27xxx_device_info *di)return POWER_SUPPLY_HEALTH_GOOD ; } +void bq27xxx_battery_settings (struct bq27xxx_device_info *di) +{ +struct power_supply_battery_info info = {}; + +/* functions don't exist for writing data so abort */ +if (!di->bus.write || !di->bus.write_bulk) +return ; + +/* no settings to be set for this chipset so abort */ +if (!bq27xxx_dm_regs[di->chip]) +return ; + +bq27xxx_battery_set_seal_state (di, false); + +if (power_supply_get_battery_info (di->bat, &info) <0 ) + goto out; + +if (info.energy_full_design_uwh != info.charge_full_design_uah) { +if (info.energy_full_design_uwh == -EINVAL ) +dev_warn (di->dev, +"missing battery:energy-full-design-microwatt-hours\n" ); +else if (info.charge_full_design_uah == -EINVAL ) +dev_warn (di->dev, +"missing battery:charge-full-design-microamp-hours\n" ); + } + +if (info.energy_full_design_uwh >0x7fff *1000 ) { +dev_err (di->dev,"invalid battery:energy-full-design-microwatt-hours %d\n" , + info.energy_full_design_uwh); + info.energy_full_design_uwh = -EINVAL ; + } + +if (info.charge_full_design_uah >0x7fff *1000 ) { +dev_err (di->dev,"invalid battery:charge-full-design-microamp-hours %d\n" , + info.charge_full_design_uah); + info.charge_full_design_uah = -EINVAL ; + } + +if ((info.voltage_min_design_uv <BQ27XXX_TERM_V_MIN *1000 + || info.voltage_min_design_uv >BQ27XXX_TERM_V_MAX *1000 ) + && info.voltage_min_design_uv != -EINVAL ) { +dev_err (di->dev,"invalid battery:voltage-min-design-microvolt %d\n" , + info.voltage_min_design_uv); + info.voltage_min_design_uv = -EINVAL ; + } + +if ((info.energy_full_design_uwh == -EINVAL + || info.charge_full_design_uah == -EINVAL ) + && info.voltage_min_design_uv == -EINVAL ) + goto out; + +bq27xxx_battery_set_config (di, &info); + +out: +bq27xxx_battery_print_config (di); +bq27xxx_battery_set_seal_state (di, true); +} +void bq27xxx_battery_update (struct bq27xxx_device_info *di) {struct bq27xxx_reg_cache cache = {0 , };@@ -1006,6 +1295,13 @@ static int bq27xxx_battery_get_property(struct power_supply *psy,case POWER_SUPPLY_PROP_CHARGE_FULL_DESIGN : ret =bq27xxx_simple_value (di->charge_design_full, val);break ; +/* +* TODO: Implement these to make registers set from +* power_supply_battery_info visible in sysfs. +*/ +case POWER_SUPPLY_PROP_ENERGY_FULL_DESIGN : +case POWER_SUPPLY_PROP_VOLTAGE_MIN_DESIGN : +return -EINVAL ;case POWER_SUPPLY_PROP_CYCLE_COUNT : ret =bq27xxx_simple_value (di->cache.cycle_count, val);break ;@@ -1039,7 +1335,10 @@ static void bq27xxx_external_power_changed(struct power_supply *psy)int bq27xxx_battery_setup (struct bq27xxx_device_info *di) {struct power_supply_desc *psy_desc; -struct power_supply_config psy_cfg = { .drv_data = di, }; +struct power_supply_config psy_cfg = { + .of_node = di->dev->of_node, + .drv_data = di, + };INIT_DELAYED_WORK (&di->work, bq27xxx_battery_poll);mutex_init (&di->lock);@@ -1064,6 +1363,7 @@ int bq27xxx_battery_setup(struct bq27xxx_device_info *di)dev_info (di->dev,"support ver. %s enabled\n" ,DRIVER_VERSION ); +bq27xxx_battery_settings (di);bq27xxx_battery_update (di);mutex_lock (&bq27xxx_list_lock); --2.9.3
Re: [PATCH v6 7/8] power: bq27xxx_battery: Add power_supply_battery_info support
Quoted 24 lines · Liam Breck
Hi Andrew,
Based on Sebastian's feedback on v1-4 of this patchset, I believe it will go upstream. (He hasn't written anything on linux-pm since before I posted v5 a week ago.)
Could we get your feedback on making this adaptable to the rest of the BQ27 family? Specifically...
Is the op sequence in these functions common across the chips?
bq27xxx_battery_set_seal_state()bq27xxx_battery_read_dm_block()bq27xxx_battery_write_dm_block()Should we move these constants to bq27xxx_regs[chip][name]?
/* writable registers */
#define BQ27XXX_DATA_CLASS 0x3E #define BQ27XXX_DATA_BLOCK 0x3F #define BQ27XXX_BLOCK_DATA 0x40 #define BQ27XXX_BLOCK_DATA_CHECKSUM 0x60 #define BQ27XXX_BLOCK_DATA_CONTROL 0x61
Quoted 4 lines
/* writable register inputs */
#define BQ27XXX_SET_CFGUPDATE 0x13 #define BQ27XXX_SOFT_RESET 0x42 #define BQ27XXX_SUBCLASS_STATE_NVM 82
I dug into bqtool:gauge.c. I learned a couple things which I've added to the patch, e.g. retrieve and compare checksum after read_dm_block.
Re command ids and params, only subclass, field offsets, and unseal key differ among chips, which I planned for. However the driver's current 9 enumerated chips won't accommodate all the diff unseal keys. But I shall leave that for others to solve.
Re op sequence, bqtool apparently uses set_cfgupdate for the 441 & 621 chips, but not others, whereas the 425 manual implies that it's nec. And bqtool uses exit_cfgupdate where 425 manual indicates soft_reset. See open/close_dm_rom (441 & 621) vs. open/close_dm_flash. We need some guidance on that issue.
Bqtool also uses loops to poll for begin & end of cfgupdate mode, and to retry read/write_dm_block with increasing delays. We could add that if nec...
http://git.ti.com/bms-linux/bqtool/blobs/master/gauge.c
Thanks!
Re: [PATCH v6 7/8] power: bq27xxx_battery: Add power_supply_battery_info support
Quoted 38 lines · Liam Breck
Quoted 24 lines · Liam Breck
Hi Andrew,
Based on Sebastian's feedback on v1-4 of this patchset, I believe it will go upstream. (He hasn't written anything on linux-pm since before I posted v5 a week ago.)
Could we get your feedback on making this adaptable to the rest of the BQ27 family? Specifically...
Is the op sequence in these functions common across the chips?
bq27xxx_battery_set_seal_state()bq27xxx_battery_read_dm_block()bq27xxx_battery_write_dm_block()Should we move these constants to bq27xxx_regs[chip][name]?
/* writable registers */
#define BQ27XXX_DATA_CLASS 0x3E #define BQ27XXX_DATA_BLOCK 0x3F #define BQ27XXX_BLOCK_DATA 0x40 #define BQ27XXX_BLOCK_DATA_CHECKSUM 0x60 #define BQ27XXX_BLOCK_DATA_CONTROL 0x61Quoted 4 lines
/* writable register inputs */
#define BQ27XXX_SET_CFGUPDATE 0x13 #define BQ27XXX_SOFT_RESET 0x42 #define BQ27XXX_SUBCLASS_STATE_NVM 82I dug into bqtool:gauge.c. I learned a couple things which I've added to the patch, e.g. retrieve and compare checksum after
read_dm_block.Re command ids and params, only subclass, field offsets, and unseal key differ among chips, which I planned for. However the driver's current 9 enumerated chips won't accommodate all the diff unseal keys. But I shall leave that for others to solve.
Quoted 5 lines
Re op sequence, bqtool apparently uses
set_cfgupdatefor the 441 & 621 chips, but not others, whereas the 425 manual implies that it's nec. And bqtool usesexit_cfgupdatewhere 425 manual indicatessoft_reset. See open/close_dm_rom (441 & 621) vs. open/close_dm_flash. We need some guidance on that issue.
I checked datasheets for one chip in each family defined in bqtool, and all chips in the 421-621 & 425 families. Only the latter 2 families use set_cfgupdate & soft_reset. I've rev'd patch accordingly; will post this evening.
http://git.ti.com/bms-linux/bqtool/blobs/master/gauge.c#line732
Bqtool may have a bug on 425 since it doesn't set_cfgupdate in that case, which the manual says is required.
PATCH v6 8/8 power: bq27xxx_battery_i2c: Add I2C bulk read/write functions
In reply to Liam BreckFrom: Matt Ranostay <[email protected]>
write(), read_bulk(), write_bulk() are required by bq27xxx_battery devicetree code.
--- drivers/power/supply/bq27xxx_battery_i2c.c | 65 ++++++++++++++++++++++++++++++ 1 file changed, 65 insertions(+)
drivers/power/supply/bq27xxx_battery_i2c.c +65 -0
@@ -68,6 +68,67 @@ static int bq27xxx_battery_i2c_read(struct bq27xxx_device_info *di, u8 reg,return ret; } +static int bq27xxx_battery_i2c_write (struct bq27xxx_device_info *di,u8 reg, +int value,bool single) +{ +struct i2c_client *client =to_i2c_client (di->dev); +struct i2c_msg msg; +unsigned char data[4 ]; + +if (!client->adapter) +return -ENODEV ; + + data[0 ] = reg; +if (single) { + data[1 ] = (unsigned char ) value; + msg.len =2 ; + }else { +put_unaligned_le16 (value, &data[1 ]); + msg.len =3 ; + } + + msg.buf = data; + msg.addr = client->addr; + msg.flags =0 ; + +return i2c_transfer (client->adapter, &msg,1 ) ==1 ? +0 : -EINVAL ; +} + +static int bq27xxx_battery_i2c_bulk_read (struct bq27xxx_device_info *di,u8 reg, +u8 *data,int len) +{ +struct i2c_client *client =to_i2c_client (di->dev); + +if (!client->adapter) +return -ENODEV ; + +return i2c_smbus_read_i2c_block_data (client, reg, len, data) == len ? +0 : -EINVAL ; +} + +static int bq27xxx_battery_i2c_bulk_write (struct bq27xxx_device_info *di, +u8 reg,u8 *data,int len) +{ +struct i2c_client *client =to_i2c_client (di->dev); +struct i2c_msg msg; +u8 buf[33 ]; + +if (!client->adapter) +return -ENODEV ; + + buf[0 ] = reg; +memcpy (&buf[1 ], data, len); + + msg.buf = buf; + msg.addr = client->addr; + msg.flags =0 ; + msg.len = len +1 ; + +return i2c_transfer (client->adapter, &msg,1 ) ==1 ? +0 : -EINVAL ; +} +static int bq27xxx_battery_i2c_probe (struct i2c_client *client,const struct i2c_device_id *id) {
@@ -95,7 +156,11 @@ static int bq27xxx_battery_i2c_probe(struct i2c_client *client, di->dev = &client->dev; di->chip = id->driver_data; di->name = name; + di->bus.read = bq27xxx_battery_i2c_read; + di->bus.write = bq27xxx_battery_i2c_write; + di->bus.read_bulk = bq27xxx_battery_i2c_bulk_read; + di->bus.write_bulk = bq27xxx_battery_i2c_bulk_write; ret =bq27xxx_battery_setup (di);if (ret)
Signature · 1 line
2.9.3