devicetree battery support and client bq27xxx_battery

From: Liam Breck <[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.

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   | 43 ++++++++++++++++++++++
 1 file changed, 43 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/power/supply/battery.txt

Documentation/devicetree/bindings/power/supply/battery.txt new file +8 -0

new file mode 100644
index 0000000..53a68c0
@@ -0,0 +1,43 @@
+Battery Characteristics
+
+The devicetree battery node provides static battery characteristics. 
+In smart batteries, these are typically stored in non-volatile memory 
+on a fuel gauge chip. The battery node should be used where there is 
+no appropriate non-volatile memory, or it is unprogrammed/incorrect.
+
+Required Properties:

Documentation/devicetree/bindings/power/supply/battery.txt new file +24 -0

+Batteries must be referenced by chargers and/or fuel-gauges
+using a phandle. The phandle's property should be named
+"monitored-battery".
+
+Example:
+
+	bat: battery {
+		compatible = "simple-battery";
+		voltage-min-design-microvolt = <3200000>;
+		energy-full-design-microwatt-hours = <5290000>;
+		charge-full-design-microamp-hours = <1430000>;
+	};
+
+	charger: charger@11 {
+		....
+		monitored-battery = <&bat>;
+		...
+	};
+
+	fuel_gauge: fuel-gauge@22 {
+		....
+		monitored-battery = <&bat>;
+		...
+	};

Signature · 1 line

2.9.3

Hi,

Quoted 34 lines · Liam Breck

From: Liam Breck <[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.

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   | 43 ++++++++++++++++++++++
 1 file changed, 43 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/power/supply/battery.txt

Documentation/devicetree/bindings/power/supply/battery.txt new file +13 -0

new file mode 100644
index 0000000..53a68c0
@@ -0,0 +1,43 @@
+Battery Characteristics
+
+The devicetree battery node provides static battery characteristics.
+In smart batteries, these are typically stored in non-volatile memory
+on a fuel gauge chip. The battery node should be used where there is
+no appropriate non-volatile memory, or it is unprogrammed/incorrect.
+
+Required Properties:
+ - compatible: Must be "simple-battery"
+
+Optional Properties:
+ - voltage-min-design-microvolt: drained battery voltage
+ - energy-full-design-microwatt-hours: battery design energy

Documentation/devicetree/bindings/power/supply/battery.txt line 18

Quoted 5 lines

Documentation/devicetree/bindings/power/supply/battery.txt new file +5 -0

+ - charge-full-design-microamp-hours: battery design capacity
+
+Battery properties are named, where possible, for the corresponding
+elements in enum power_supply_property, defined in
+https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/linux/power_supply.h#n86

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

Acked-by Sebastian Reichel <[email protected]>

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

-- Sebastian

Quoted 27 lines

Documentation/devicetree/bindings/power/supply/battery.txt new file +24 -1

+Batteries must be referenced by chargers and/or fuel-gauges
+using a phandle. The phandle's property should be named
+"monitored-battery".
+
+Example:
+
+     bat: battery {
+             compatible = "simple-battery";
+             voltage-min-design-microvolt = <3200000>;
+             energy-full-design-microwatt-hours = <5290000>;
+             charge-full-design-microamp-hours = <1430000>;
+     };
+
+     charger: charger@11 {
+             ....
+             monitored-battery = <&bat>;
+             ...
+     };
+
+     fuel_gauge: fuel-gauge@22 {
+             ....
+             monitored-battery = <&bat>;
+             ...
+     };
--

2.9.3

Quoted 46 lines · Sebastian Reichel

Hi,

Quoted 29 lines · Liam Breck

From: Liam Breck <[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.

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   | 43 ++++++++++++++++++++++
 1 file changed, 43 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/power/supply/battery.txt

Documentation/devicetree/bindings/power/supply/battery.txt new file +8 -0

new file mode 100644
index 0000000..53a68c0
@@ -0,0 +1,43 @@
+Battery Characteristics
+
+The devicetree battery node provides static battery characteristics.
+In smart batteries, these are typically stored in non-volatile memory
+on a fuel gauge chip. The battery node should be used where there is
+no appropriate non-volatile memory, or it is unprogrammed/incorrect.
+
+Required Properties:

Documentation/devicetree/bindings/power/supply/battery.txt line 18

Quoted 10 lines

Documentation/devicetree/bindings/power/supply/battery.txt new file +10 -0

+ - compatible: Must be "simple-battery"
+
+Optional Properties:
+ - voltage-min-design-microvolt: drained battery voltage
+ - energy-full-design-microwatt-hours: battery design energy
+ - charge-full-design-microamp-hours: battery design capacity
+
+Battery properties are named, where possible, for the corresponding
+elements in enum power_supply_property, defined in
+https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/linux/power_supply.h#n86

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

Quoted 1 line

Acked-by Sebastian Reichel <[email protected]>

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

Quoted 31 lines

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

-- Sebastian

Quoted 27 lines

Documentation/devicetree/bindings/power/supply/battery.txt new file +24 -1

+Batteries must be referenced by chargers and/or fuel-gauges
+using a phandle. The phandle's property should be named
+"monitored-battery".
+
+Example:
+
+     bat: battery {
+             compatible = "simple-battery";
+             voltage-min-design-microvolt = <3200000>;
+             energy-full-design-microwatt-hours = <5290000>;
+             charge-full-design-microamp-hours = <1430000>;
+     };
+
+     charger: charger@11 {
+             ....
+             monitored-battery = <&bat>;
+             ...
+     };
+
+     fuel_gauge: fuel-gauge@22 {
+             ....
+             monitored-battery = <&bat>;
+             ...
+     };
--

2.9.3

From: Liam Breck <[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.

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   | 43 ++++++++++++++++++++++
 1 file changed, 43 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/power/supply/battery.txt

Documentation/devicetree/bindings/power/supply/battery.txt new file +43 -1

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

2.9.3

[+CC Tony Lindgren]

Rob & Sebastian & Tony & Andrew,

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

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

If this is agreed, how should we document it?

Quoted 67 lines

From: Liam Breck <[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.

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   | 43 ++++++++++++++++++++++
 1 file changed, 43 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/power/supply/battery.txt

Documentation/devicetree/bindings/power/supply/battery.txt new file +43 -1

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

2.9.3

From: Liam Breck <[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.

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   | 43 ++++++++++++++++++++++
 1 file changed, 43 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/power/supply/battery.txt

Documentation/devicetree/bindings/power/supply/battery.txt new file +8 -0

new file mode 100644
index 0000000..53a68c0
@@ -0,0 +1,43 @@
+Battery Characteristics
+
+The devicetree battery node provides static battery characteristics. 
+In smart batteries, these are typically stored in non-volatile memory 
+on a fuel gauge chip. The battery node should be used where there is 
+no appropriate non-volatile memory, or it is unprogrammed/incorrect.
+
+Required Properties:

From: Liam Breck <[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.

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   | 43 ++++++++++++++++++++++
 1 file changed, 43 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/power/supply/battery.txt

Quoted 6 lines · Liam Breck

From: Liam Breck <[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.

Quoted 8 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   | 43 ++++++++++++++++++++++
 1 file changed, 43 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/power/supply/battery.txt

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

Acked-by Rob Herring <[email protected]>

Documentation/devicetree/bindings/power/supply/battery.txt line 18

Quoted 3 lines

Documentation/devicetree/bindings/power/supply/battery.txt new file +3 -0

+Battery properties are named, where possible, for the corresponding
+elements in enum power_supply_property, defined in
+https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/linux/power_supply.h#n86

The above paragraph does not belong into DT bindings.

From: Liam Breck <[email protected]>

Document monitored-battery = <&battery_node>

Cc Rob Herring <[email protected]>

Cc [email protected]

Signed-off-by Matt Ranostay <[email protected]>

Signed-off-by Liam Breck <[email protected]>

Acked-by Rob Herring <[email protected]>

Acked-by Sebastian Reichel <[email protected]>

---
 Documentation/devicetree/bindings/power/supply/bq27xxx.txt | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

Documentation/devicetree/bindings/power/supply/bq27xxx.txt +6 -0

index b0c95ef..cf83371 100644
@@ -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.

From: Matt Ranostay <[email protected]>

Add entries for microwatt-hours and microamp-hours.

Cc Rob Herring <[email protected]>

Cc Mark Rutland <[email protected]>

Cc [email protected]

Cc [email protected]

Signed-off-by Matt Ranostay <[email protected]>

Signed-off-by Liam Breck <[email protected]>

Acked-by Sebastian Reichel <[email protected]>

Acked-by Rob Herring <[email protected]>

---
 Documentation/devicetree/bindings/property-units.txt | 2 ++
 1 file changed, 2 insertions(+)

Documentation/devicetree/bindings/property-units.txt +0 -0

index 12278d7..0849618 100644

From: Liam Breck <[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.

Signed-off-by Matt Ranostay <[email protected]>

Signed-off-by Liam Breck <[email protected]>

---
 Documentation/power/power_supply_class.txt | 12 ++++++++
 drivers/power/supply/power_supply_core.c   | 45 ++++++++++++++++++++++++++++++
 include/linux/power_supply.h               | 18 ++++++++++++
 3 files changed, 75 insertions(+)

Documentation/power/power_supply_class.txt +12 -0

index 0c72588..dc92c77 100644
@@ -174,6 +174,18 @@ issued by external power supply will notify supplicants via
 external_power_changed callback.
 
 
+Devicetree battery characteristics
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+Drivers should call power_supply_get_battery_info() to obtain battery
+characteristics from a devicetree battery node, defined in
+Documentation/devicetree/bindings/power/supply/battery.txt. This is
+implemented in drivers/power/supply/bq27xxx_battery.c.
+
+Properties in struct power_supply_battery_info and their counterparts in the
+battery node have names corresponding to elements in enum power_supply_property,
+for naming consistency between sysfs attributes and battery node properties.
+
+
 QA
 ~~
 Q: Where is POWER_SUPPLY_PROP_XYZ attribute?

drivers/power/supply/power_supply_core.c +30 -0

index a74d8ca..61e20b1 100644
@@ -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,50 @@ 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("simple-battery", value))
+               return -ENODEV;
+
+       /* The property and field names below must correspond to elements

drivers/power/supply/power_supply_core.c +5 -0

+
+       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 -1

index 3965503..e84f1d3 100644
@@ -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);
--

2.9.3

Quoted 78 lines · Liam Breck

From: Liam Breck <[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.

Signed-off-by Matt Ranostay <[email protected]>

Signed-off-by Liam Breck <[email protected]>

---
 Documentation/power/power_supply_class.txt | 12 ++++++++
 drivers/power/supply/power_supply_core.c   | 45 ++++++++++++++++++++++++++++++
 include/linux/power_supply.h               | 18 ++++++++++++
 3 files changed, 75 insertions(+)

Documentation/power/power_supply_class.txt +12 -0

index 0c72588..dc92c77 100644
@@ -174,6 +174,18 @@ issued by external power supply will notify supplicants via
 external_power_changed callback.
 
 
+Devicetree battery characteristics
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+Drivers should call power_supply_get_battery_info() to obtain battery
+characteristics from a devicetree battery node, defined in
+Documentation/devicetree/bindings/power/supply/battery.txt. This is
+implemented in drivers/power/supply/bq27xxx_battery.c.
+
+Properties in struct power_supply_battery_info and their counterparts in the
+battery node have names corresponding to elements in enum power_supply_property,
+for naming consistency between sysfs attributes and battery node properties.
+
+
 QA
 ~~
 Q: Where is POWER_SUPPLY_PROP_XYZ attribute?

drivers/power/supply/power_supply_core.c +25 -0

index a74d8ca..61e20b1 100644
@@ -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,50 @@ 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;

drivers/power/supply/power_supply_core.c struct power_supply *devm_power_supply_get_by_phandle(struct device *dev, · line 522

Quoted 8 lines

drivers/power/supply/power_supply_core.c +8 -0

+
+       if (strcmp("simple-battery", value))
+               return -ENODEV;
+
+       /* The property and field names below must correspond to elements
+        * in enum power_supply_property. For reasoning, see
+        * Documentation/power/power_supply_class.txt.
+        */

drivers/power/supply/power_supply_core.c struct power_supply *devm_power_supply_get_by_phandle(struct device *dev, · line 529

Quoted 6 lines

drivers/power/supply/power_supply_core.c +6 -0

+       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);

Should we also check for an acpi_handle, and use device_property_read_* here? Tho presumably the acpi property names would be different...

I wouldn't have a way to test that however.

Quoted 47 lines

drivers/power/supply/power_supply_core.c +5 -0

+
+       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 -1

index 3965503..e84f1d3 100644
@@ -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);
--

2.9.3

Hi,

Quoted 55 lines · Liam Breck

Quoted 54 lines · Liam Breck

From: Liam Breck <[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.

Signed-off-by Matt Ranostay <[email protected]>

Signed-off-by Liam Breck <[email protected]>

---
 Documentation/power/power_supply_class.txt | 12 ++++++++
 drivers/power/supply/power_supply_core.c   | 45 ++++++++++++++++++++++++++++++
 include/linux/power_supply.h               | 18 ++++++++++++
 3 files changed, 75 insertions(+)

Documentation/power/power_supply_class.txt +12 -0

index 0c72588..dc92c77 100644
@@ -174,6 +174,18 @@ issued by external power supply will notify supplicants via
 external_power_changed callback.
 
 
+Devicetree battery characteristics
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+Drivers should call power_supply_get_battery_info() to obtain battery
+characteristics from a devicetree battery node, defined in
+Documentation/devicetree/bindings/power/supply/battery.txt. This is
+implemented in drivers/power/supply/bq27xxx_battery.c.
+
+Properties in struct power_supply_battery_info and their counterparts in the
+battery node have names corresponding to elements in enum power_supply_property,
+for naming consistency between sysfs attributes and battery node properties.
+
+
 QA
 ~~
 Q: Where is POWER_SUPPLY_PROP_XYZ attribute?

drivers/power/supply/power_supply_core.c +1 -0

index a74d8ca..61e20b1 100644
@@ -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,50 @@ struct power_supply *devm_power_supply_get_by_phandle(struct device *dev,
 EXPORT_SYMBOL_GPL(devm_power_supply_get_by_phandle);
 #endif /* CONFIG_OF */
 

Quoted 4 lines

Quoted 4 lines

drivers/power/supply/power_supply_core.c +4 -0

+int power_supply_get_battery_info(struct power_supply *psy,
+                                 struct power_supply_battery_info *info)
+{
+       struct device_node *battery_np;

Quoted 8 lines

drivers/power/supply/power_supply_core.c struct power_supply *devm_power_supply_get_by_phandle(struct device *dev, · line 502

Quoted 8 lines

drivers/power/supply/power_supply_core.c +8 -0

+       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) {

Quoted 6 lines

drivers/power/supply/power_supply_core.c struct power_supply *devm_power_supply_get_by_phandle(struct device *dev, · line 508

Quoted 6 lines

drivers/power/supply/power_supply_core.c +6 -0

+               dev_warn(&psy->dev, "%s currently only supports devicetree\n",
+                        __func__);
+               return -ENXIO;
+       }
+
+       battery_np = of_parse_phandle(psy->of_node, "monitored-battery", 0);

Quoted 8 lines

Quoted 8 lines

drivers/power/supply/power_supply_core.c +8 -0

+       if (!battery_np)
+               return -ENODEV;
+
+       err = of_property_read_string(battery_np, "compatible", &value);
+       if (err)
+               return err;
+
+       if (strcmp("simple-battery", value))

Quoted 6 lines

drivers/power/supply/power_supply_core.c struct power_supply *devm_power_supply_get_by_phandle(struct device *dev, · line 522

Quoted 6 lines

drivers/power/supply/power_supply_core.c +6 -0

+               return -ENODEV;
+
+       /* The property and field names below must correspond to elements
+        * in enum power_supply_property. For reasoning, see
+        * Documentation/power/power_supply_class.txt.
+        */

Let's make this "should correspond".

Quoted 6 lines

Quoted 6 lines

drivers/power/supply/power_supply_core.c +6 -0

+       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);

Quoted 5 lines

Should we also check for an acpi_handle, and use device_property_read_* here? Tho presumably the acpi property names would be different...

I wouldn't have a way to test that however.

Yes, please use device_property_read_* instead of of_property_read_*. Apart from that you do not have to care for ACPI for now. It can be added once needed.

Quoted 47 lines

Quoted 47 lines

drivers/power/supply/power_supply_core.c +5 -0

+
+       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 -1

index 3965503..e84f1d3 100644
@@ -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);
--

2.9.3

Quoted 6 lines

Quoted 6 lines

drivers/power/supply/power_supply_core.c struct power_supply *devm_power_supply_get_by_phandle(struct device *dev, · line 508

Quoted 6 lines

drivers/power/supply/power_supply_core.c +6 -0

+               dev_warn(&psy->dev, "%s currently only supports devicetree\n",
+                        __func__);
+               return -ENXIO;
+       }
+
+       battery_np = of_parse_phandle(psy->of_node, "monitored-battery", 0);

Does that change? If not, will psy->dev work in device_property_read_* ?

From: Liam Breck <[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.

Signed-off-by Matt Ranostay <[email protected]>

Signed-off-by Liam Breck <[email protected]>

From: Liam Breck <[email protected]>

Previously errors were reported in a mix of styles via both dev_err() & dev_dbg(). Report all errors with dev_err() and include error code in message. Report register ID separately in dev_dbg() message.

Signed-off-by Liam Breck <[email protected]>

---
 drivers/power/supply/bq27xxx_battery.c | 35 ++++++++++++++++++----------------
 1 file changed, 19 insertions(+), 16 deletions(-)

drivers/power/supply/bq27xxx_battery.c +1 -1

index 398801a..2e2a3a8 100644
@@ -794,11 +794,17 @@ MODULE_PARM_DESC(poll_interval,
 static inline int bq27xxx_read(struct bq27xxx_device_info *di, int reg_index,
 			       bool single)
 {
-	/* Reports EINVAL for invalid/missing registers */
+	int ret;

drivers/power/supply/bq27xxx_battery.c +12 -14

+	return ret;
 }
 
 /*

@@ -815,7 +821,7 @@ static int bq27xxx_battery_read_soc(struct bq27xxx_device_info *di)
 		soc = bq27xxx_read(di, BQ27XXX_REG_SOC, false);
 
 	if (soc < 0)
-		dev_dbg(di->dev, "error reading State-of-Charge\n");
+		dev_err(di->dev, "error %d reading state-of-charge\n", soc);
 
 	return soc;
 }

@@ -830,8 +836,7 @@ static int bq27xxx_battery_read_charge(struct bq27xxx_device_info *di, u8 reg)
 
 	charge = bq27xxx_read(di, reg, false);
 	if (charge < 0) {
-		dev_dbg(di->dev, "error reading charge register %02x: %d\n",
-			reg, charge);
+		dev_err(di->dev, "error %d reading charge\n", charge);
 		return charge;
 	}
 

@@ -883,7 +888,7 @@ static int bq27xxx_battery_read_dcap(struct bq27xxx_device_info *di)
 		dcap = bq27xxx_read(di, BQ27XXX_REG_DCAP, false);
 
 	if (dcap < 0) {
-		dev_dbg(di->dev, "error reading initial last measured discharge\n");
+		dev_err(di->dev, "error %d reading design capacity\n", dcap);
 		return dcap;
 	}
 

@@ -905,7 +910,7 @@ static int bq27xxx_battery_read_energy(struct bq27xxx_device_info *di)
 
 	ae = bq27xxx_read(di, BQ27XXX_REG_AE, false);
 	if (ae < 0) {
-		dev_dbg(di->dev, "error reading available energy\n");
+		dev_err(di->dev, "error %d reading available energy\n", ae);
 		return ae;
 	}
 

@@ -927,7 +932,7 @@ static int bq27xxx_battery_read_temperature(struct bq27xxx_device_info *di)
 
 	temp = bq27xxx_read(di, BQ27XXX_REG_TEMP, false);
 	if (temp < 0) {
-		dev_err(di->dev, "error reading temperature\n");
+		dev_err(di->dev, "error %d reading temperature\n", temp);
 		return temp;
 	}
 

@@ -947,7 +952,7 @@ static int bq27xxx_battery_read_cyct(struct bq27xxx_device_info *di)
 
 	cyct = bq27xxx_read(di, BQ27XXX_REG_CYCT, false);
 	if (cyct < 0)
-		dev_err(di->dev, "error reading cycle count total\n");
+		dev_err(di->dev, "error %d reading cycle count total\n", cyct);
 
 	return cyct;
 }

@@ -962,8 +967,7 @@ static int bq27xxx_battery_read_time(struct bq27xxx_device_info *di, u8 reg)
 
 	tval = bq27xxx_read(di, reg, false);
 	if (tval < 0) {
-		dev_dbg(di->dev, "error reading time register %02x: %d\n",
-			reg, tval);
+		dev_err(di->dev, "error %d reading time\n", tval);
 		return tval;
 	}
 

@@ -983,8 +987,7 @@ static int bq27xxx_battery_read_pwr_avg(struct bq27xxx_device_info *di)
 
 	tval = bq27xxx_read(di, BQ27XXX_REG_AP, false);
 	if (tval < 0) {
-		dev_err(di->dev, "error reading average power register  %02x: %d\n",
-			BQ27XXX_REG_AP, tval);
+		dev_err(di->dev, "error %d reading average power\n", tval);
 		return tval;
 	}
 

@@ -1054,7 +1057,7 @@ static int bq27xxx_battery_read_health(struct bq27xxx_device_info *di)
 
 	flags = bq27xxx_read(di, BQ27XXX_REG_FLAGS, has_singe_flag);
 	if (flags < 0) {
-		dev_err(di->dev, "error reading flag register:%d\n", flags);
+		dev_err(di->dev, "error %d reading flags\n", flags);
 		return flags;
 	}
 

@@ -1147,7 +1150,7 @@ static int bq27xxx_battery_current(struct bq27xxx_device_info *di,
 
 	curr = bq27xxx_read(di, BQ27XXX_REG_AI, false);
 	if (curr < 0) {
-		dev_err(di->dev, "error reading current\n");
+		dev_err(di->dev, "error %d reading current\n", curr);
 		return curr;
 	}
 

@@ -1236,7 +1239,7 @@ static int bq27xxx_battery_voltage(struct bq27xxx_device_info *di,
 
 	volt = bq27xxx_read(di, BQ27XXX_REG_VOLT, false);
 	if (volt < 0) {
-		dev_err(di->dev, "error reading voltage\n");
+		dev_err(di->dev, "error %d reading voltage\n", volt);
 		return volt;
 	}
 

Quoted 25 lines · Liam Breck

From: Liam Breck <[email protected]>

Previously errors were reported in a mix of styles via both dev_err() & dev_dbg(). Report all errors with dev_err() and include error code in message. Report register ID separately in dev_dbg() message.

Signed-off-by Liam Breck <[email protected]>

---
 drivers/power/supply/bq27xxx_battery.c | 35 ++++++++++++++++++----------------
 1 file changed, 19 insertions(+), 16 deletions(-)

drivers/power/supply/bq27xxx_battery.c +2 -1

index 398801a..2e2a3a8 100644
@@ -794,11 +794,17 @@ MODULE_PARM_DESC(poll_interval,
 static inline int bq27xxx_read(struct bq27xxx_device_info *di, int reg_index,
                             bool single)
 {
-     /* Reports EINVAL for invalid/missing registers */
+     int ret;
+
      if (!di || di->regs[reg_index] == INVALID_REG_ADDR)
              return -EINVAL;
 

drivers/power/supply/bq27xxx_battery.c MODULE_PARM_DESC(poll_interval, · line 806

Quoted 6 lines

drivers/power/supply/bq27xxx_battery.c +5 -1

-     return di->bus.read(di, di->regs[reg_index], single);
+     ret = di->bus.read(di, di->regs[reg_index], single);
+     if (ret < 0)
+             dev_dbg(di->dev, "failed to read register 0x%02x (index %d)\n",
+                     di->regs[reg_index], reg_index);
+

If we print out the error code here we don't have to do that in each of the below printouts.

Otherwise looks good:

Acked-by Andrew F. Davis <[email protected]>

Quoted 107 lines

drivers/power/supply/bq27xxx_battery.c +12 -14

+     return ret;
 }
 
 /*

@@ -815,7 +821,7 @@ static int bq27xxx_battery_read_soc(struct bq27xxx_device_info *di)
              soc = bq27xxx_read(di, BQ27XXX_REG_SOC, false);
 
      if (soc < 0)
-             dev_dbg(di->dev, "error reading State-of-Charge\n");
+             dev_err(di->dev, "error %d reading state-of-charge\n", soc);
 
      return soc;
 }

@@ -830,8 +836,7 @@ static int bq27xxx_battery_read_charge(struct bq27xxx_device_info *di, u8 reg)
 
      charge = bq27xxx_read(di, reg, false);
      if (charge < 0) {
-             dev_dbg(di->dev, "error reading charge register %02x: %d\n",
-                     reg, charge);
+             dev_err(di->dev, "error %d reading charge\n", charge);
              return charge;
      }
 

@@ -883,7 +888,7 @@ static int bq27xxx_battery_read_dcap(struct bq27xxx_device_info *di)
              dcap = bq27xxx_read(di, BQ27XXX_REG_DCAP, false);
 
      if (dcap < 0) {
-             dev_dbg(di->dev, "error reading initial last measured discharge\n");
+             dev_err(di->dev, "error %d reading design capacity\n", dcap);
              return dcap;
      }
 

@@ -905,7 +910,7 @@ static int bq27xxx_battery_read_energy(struct bq27xxx_device_info *di)
 
      ae = bq27xxx_read(di, BQ27XXX_REG_AE, false);
      if (ae < 0) {
-             dev_dbg(di->dev, "error reading available energy\n");
+             dev_err(di->dev, "error %d reading available energy\n", ae);
              return ae;
      }
 

@@ -927,7 +932,7 @@ static int bq27xxx_battery_read_temperature(struct bq27xxx_device_info *di)
 
      temp = bq27xxx_read(di, BQ27XXX_REG_TEMP, false);
      if (temp < 0) {
-             dev_err(di->dev, "error reading temperature\n");
+             dev_err(di->dev, "error %d reading temperature\n", temp);
              return temp;
      }
 

@@ -947,7 +952,7 @@ static int bq27xxx_battery_read_cyct(struct bq27xxx_device_info *di)
 
      cyct = bq27xxx_read(di, BQ27XXX_REG_CYCT, false);
      if (cyct < 0)
-             dev_err(di->dev, "error reading cycle count total\n");
+             dev_err(di->dev, "error %d reading cycle count total\n", cyct);
 
      return cyct;
 }

@@ -962,8 +967,7 @@ static int bq27xxx_battery_read_time(struct bq27xxx_device_info *di, u8 reg)
 
      tval = bq27xxx_read(di, reg, false);
      if (tval < 0) {
-             dev_dbg(di->dev, "error reading time register %02x: %d\n",
-                     reg, tval);
+             dev_err(di->dev, "error %d reading time\n", tval);
              return tval;
      }
 

@@ -983,8 +987,7 @@ static int bq27xxx_battery_read_pwr_avg(struct bq27xxx_device_info *di)
 
      tval = bq27xxx_read(di, BQ27XXX_REG_AP, false);
      if (tval < 0) {
-             dev_err(di->dev, "error reading average power register  %02x: %d\n",
-                     BQ27XXX_REG_AP, tval);
+             dev_err(di->dev, "error %d reading average power\n", tval);
              return tval;
      }
 

@@ -1054,7 +1057,7 @@ static int bq27xxx_battery_read_health(struct bq27xxx_device_info *di)
 
      flags = bq27xxx_read(di, BQ27XXX_REG_FLAGS, has_singe_flag);
      if (flags < 0) {
-             dev_err(di->dev, "error reading flag register:%d\n", flags);
+             dev_err(di->dev, "error %d reading flags\n", flags);
              return flags;
      }
 

@@ -1147,7 +1150,7 @@ static int bq27xxx_battery_current(struct bq27xxx_device_info *di,
 
      curr = bq27xxx_read(di, BQ27XXX_REG_AI, false);
      if (curr < 0) {
-             dev_err(di->dev, "error reading current\n");
+             dev_err(di->dev, "error %d reading current\n", curr);
              return curr;
      }
 

@@ -1236,7 +1239,7 @@ static int bq27xxx_battery_voltage(struct bq27xxx_device_info *di,
 
      volt = bq27xxx_read(di, BQ27XXX_REG_VOLT, false);
      if (volt < 0) {
-             dev_err(di->dev, "error reading voltage\n");
+             dev_err(di->dev, "error %d reading voltage\n", volt);
              return volt;
      }
 

Quoted 26 lines · Andrew F. Davis

Quoted 25 lines · Liam Breck

From: Liam Breck <[email protected]>

Previously errors were reported in a mix of styles via both dev_err() & dev_dbg(). Report all errors with dev_err() and include error code in message. Report register ID separately in dev_dbg() message.

Signed-off-by Liam Breck <[email protected]>

---
 drivers/power/supply/bq27xxx_battery.c | 35 ++++++++++++++++++----------------
 1 file changed, 19 insertions(+), 16 deletions(-)

drivers/power/supply/bq27xxx_battery.c +2 -1

index 398801a..2e2a3a8 100644
@@ -794,11 +794,17 @@ MODULE_PARM_DESC(poll_interval,
 static inline int bq27xxx_read(struct bq27xxx_device_info *di, int reg_index,
                             bool single)
 {
-     /* Reports EINVAL for invalid/missing registers */
+     int ret;
+
      if (!di || di->regs[reg_index] == INVALID_REG_ADDR)
              return -EINVAL;
 

Quoted 9 lines

drivers/power/supply/bq27xxx_battery.c MODULE_PARM_DESC(poll_interval, · line 806

Quoted 6 lines

drivers/power/supply/bq27xxx_battery.c +5 -1

-     return di->bus.read(di, di->regs[reg_index], single);
+     ret = di->bus.read(di, di->regs[reg_index], single);
+     if (ret < 0)
+             dev_dbg(di->dev, "failed to read register 0x%02x (index %d)\n",
+                     di->regs[reg_index], reg_index);
+

If we print out the error code here we don't have to do that in each of the below printouts.

This is a dev_dbg, the below are dev_err, and I think you want error code in dev_err. BTW the above addresses your request to learn which register was in play in the event of a DM read/write failure.

Quoted 110 lines

Otherwise looks good:

Acked-by Andrew F. Davis <[email protected]>

Quoted 107 lines

drivers/power/supply/bq27xxx_battery.c +12 -14

+     return ret;
 }
 
 /*

@@ -815,7 +821,7 @@ static int bq27xxx_battery_read_soc(struct bq27xxx_device_info *di)
              soc = bq27xxx_read(di, BQ27XXX_REG_SOC, false);
 
      if (soc < 0)
-             dev_dbg(di->dev, "error reading State-of-Charge\n");
+             dev_err(di->dev, "error %d reading state-of-charge\n", soc);
 
      return soc;
 }

@@ -830,8 +836,7 @@ static int bq27xxx_battery_read_charge(struct bq27xxx_device_info *di, u8 reg)
 
      charge = bq27xxx_read(di, reg, false);
      if (charge < 0) {
-             dev_dbg(di->dev, "error reading charge register %02x: %d\n",
-                     reg, charge);
+             dev_err(di->dev, "error %d reading charge\n", charge);
              return charge;
      }
 

@@ -883,7 +888,7 @@ static int bq27xxx_battery_read_dcap(struct bq27xxx_device_info *di)
              dcap = bq27xxx_read(di, BQ27XXX_REG_DCAP, false);
 
      if (dcap < 0) {
-             dev_dbg(di->dev, "error reading initial last measured discharge\n");
+             dev_err(di->dev, "error %d reading design capacity\n", dcap);
              return dcap;
      }
 

@@ -905,7 +910,7 @@ static int bq27xxx_battery_read_energy(struct bq27xxx_device_info *di)
 
      ae = bq27xxx_read(di, BQ27XXX_REG_AE, false);
      if (ae < 0) {
-             dev_dbg(di->dev, "error reading available energy\n");
+             dev_err(di->dev, "error %d reading available energy\n", ae);
              return ae;
      }
 

@@ -927,7 +932,7 @@ static int bq27xxx_battery_read_temperature(struct bq27xxx_device_info *di)
 
      temp = bq27xxx_read(di, BQ27XXX_REG_TEMP, false);
      if (temp < 0) {
-             dev_err(di->dev, "error reading temperature\n");
+             dev_err(di->dev, "error %d reading temperature\n", temp);
              return temp;
      }
 

@@ -947,7 +952,7 @@ static int bq27xxx_battery_read_cyct(struct bq27xxx_device_info *di)
 
      cyct = bq27xxx_read(di, BQ27XXX_REG_CYCT, false);
      if (cyct < 0)
-             dev_err(di->dev, "error reading cycle count total\n");
+             dev_err(di->dev, "error %d reading cycle count total\n", cyct);
 
      return cyct;
 }

@@ -962,8 +967,7 @@ static int bq27xxx_battery_read_time(struct bq27xxx_device_info *di, u8 reg)
 
      tval = bq27xxx_read(di, reg, false);
      if (tval < 0) {
-             dev_dbg(di->dev, "error reading time register %02x: %d\n",
-                     reg, tval);
+             dev_err(di->dev, "error %d reading time\n", tval);
              return tval;
      }
 

@@ -983,8 +987,7 @@ static int bq27xxx_battery_read_pwr_avg(struct bq27xxx_device_info *di)
 
      tval = bq27xxx_read(di, BQ27XXX_REG_AP, false);
      if (tval < 0) {
-             dev_err(di->dev, "error reading average power register  %02x: %d\n",
-                     BQ27XXX_REG_AP, tval);
+             dev_err(di->dev, "error %d reading average power\n", tval);
              return tval;
      }
 

@@ -1054,7 +1057,7 @@ static int bq27xxx_battery_read_health(struct bq27xxx_device_info *di)
 
      flags = bq27xxx_read(di, BQ27XXX_REG_FLAGS, has_singe_flag);
      if (flags < 0) {
-             dev_err(di->dev, "error reading flag register:%d\n", flags);
+             dev_err(di->dev, "error %d reading flags\n", flags);
              return flags;
      }
 

@@ -1147,7 +1150,7 @@ static int bq27xxx_battery_current(struct bq27xxx_device_info *di,
 
      curr = bq27xxx_read(di, BQ27XXX_REG_AI, false);
      if (curr < 0) {
-             dev_err(di->dev, "error reading current\n");
+             dev_err(di->dev, "error %d reading current\n", curr);
              return curr;
      }
 

@@ -1236,7 +1239,7 @@ static int bq27xxx_battery_voltage(struct bq27xxx_device_info *di,
 
      volt = bq27xxx_read(di, BQ27XXX_REG_VOLT, false);
      if (volt < 0) {
-             dev_err(di->dev, "error reading voltage\n");
+             dev_err(di->dev, "error %d reading voltage\n", volt);
              return volt;
      }
 

Hi,

Quoted 37 lines · Liam Breck

Quoted 35 lines · Andrew F. Davis

Quoted 21 lines · Liam Breck

From: Liam Breck <[email protected]>

Previously errors were reported in a mix of styles via both dev_err() & dev_dbg(). Report all errors with dev_err() and include error code in message. Report register ID separately in dev_dbg() message.

Signed-off-by Liam Breck <[email protected]>

---
 drivers/power/supply/bq27xxx_battery.c | 35 ++++++++++++++++++----------------
 1 file changed, 19 insertions(+), 16 deletions(-)

drivers/power/supply/bq27xxx_battery.c +1 -1

index 398801a..2e2a3a8 100644
@@ -794,11 +794,17 @@ MODULE_PARM_DESC(poll_interval,
 static inline int bq27xxx_read(struct bq27xxx_device_info *di, int reg_index,
                             bool single)
 {
-     /* Reports EINVAL for invalid/missing registers */
+     int ret;

drivers/power/supply/bq27xxx_battery.c MODULE_PARM_DESC(poll_interval, · line 806

Quoted 10 lines

drivers/power/supply/bq27xxx_battery.c +6 -1

+
      if (!di || di->regs[reg_index] == INVALID_REG_ADDR)
              return -EINVAL;
 
-     return di->bus.read(di, di->regs[reg_index], single);
+     ret = di->bus.read(di, di->regs[reg_index], single);
+     if (ret < 0)
+             dev_dbg(di->dev, "failed to read register 0x%02x (index %d)\n",
+                     di->regs[reg_index], reg_index);
+

If we print out the error code here we don't have to do that in each of the below printouts.

Quoted 3 lines

This is a dev_dbg, the below are dev_err, and I think you want error code in dev_err. BTW the above addresses your request to learn which register was in play in the event of a DM read/write failure.

Just make it dev_err and also print ret. Then we can remove all other prints. That will result in some information-loss (name of the register), but

1. it's not that important anyways. If you read battery voltage file in sysfs, then obviously reading voltage fails and so on.
2. the error should be very unlikely on productive systems, since it basically mean the i2c/1-wire transaction failed.

-- Sebastian

Quoted 110 lines

Quoted 110 lines

Otherwise looks good:

Acked-by Andrew F. Davis <[email protected]>

Quoted 107 lines

drivers/power/supply/bq27xxx_battery.c +12 -14

+     return ret;
 }
 
 /*

@@ -815,7 +821,7 @@ static int bq27xxx_battery_read_soc(struct bq27xxx_device_info *di)
              soc = bq27xxx_read(di, BQ27XXX_REG_SOC, false);
 
      if (soc < 0)
-             dev_dbg(di->dev, "error reading State-of-Charge\n");
+             dev_err(di->dev, "error %d reading state-of-charge\n", soc);
 
      return soc;
 }

@@ -830,8 +836,7 @@ static int bq27xxx_battery_read_charge(struct bq27xxx_device_info *di, u8 reg)
 
      charge = bq27xxx_read(di, reg, false);
      if (charge < 0) {
-             dev_dbg(di->dev, "error reading charge register %02x: %d\n",
-                     reg, charge);
+             dev_err(di->dev, "error %d reading charge\n", charge);
              return charge;
      }
 

@@ -883,7 +888,7 @@ static int bq27xxx_battery_read_dcap(struct bq27xxx_device_info *di)
              dcap = bq27xxx_read(di, BQ27XXX_REG_DCAP, false);
 
      if (dcap < 0) {
-             dev_dbg(di->dev, "error reading initial last measured discharge\n");
+             dev_err(di->dev, "error %d reading design capacity\n", dcap);
              return dcap;
      }
 

@@ -905,7 +910,7 @@ static int bq27xxx_battery_read_energy(struct bq27xxx_device_info *di)
 
      ae = bq27xxx_read(di, BQ27XXX_REG_AE, false);
      if (ae < 0) {
-             dev_dbg(di->dev, "error reading available energy\n");
+             dev_err(di->dev, "error %d reading available energy\n", ae);
              return ae;
      }
 

@@ -927,7 +932,7 @@ static int bq27xxx_battery_read_temperature(struct bq27xxx_device_info *di)
 
      temp = bq27xxx_read(di, BQ27XXX_REG_TEMP, false);
      if (temp < 0) {
-             dev_err(di->dev, "error reading temperature\n");
+             dev_err(di->dev, "error %d reading temperature\n", temp);
              return temp;
      }
 

@@ -947,7 +952,7 @@ static int bq27xxx_battery_read_cyct(struct bq27xxx_device_info *di)
 
      cyct = bq27xxx_read(di, BQ27XXX_REG_CYCT, false);
      if (cyct < 0)
-             dev_err(di->dev, "error reading cycle count total\n");
+             dev_err(di->dev, "error %d reading cycle count total\n", cyct);
 
      return cyct;
 }

@@ -962,8 +967,7 @@ static int bq27xxx_battery_read_time(struct bq27xxx_device_info *di, u8 reg)
 
      tval = bq27xxx_read(di, reg, false);
      if (tval < 0) {
-             dev_dbg(di->dev, "error reading time register %02x: %d\n",
-                     reg, tval);
+             dev_err(di->dev, "error %d reading time\n", tval);
              return tval;
      }
 

@@ -983,8 +987,7 @@ static int bq27xxx_battery_read_pwr_avg(struct bq27xxx_device_info *di)
 
      tval = bq27xxx_read(di, BQ27XXX_REG_AP, false);
      if (tval < 0) {
-             dev_err(di->dev, "error reading average power register  %02x: %d\n",
-                     BQ27XXX_REG_AP, tval);
+             dev_err(di->dev, "error %d reading average power\n", tval);
              return tval;
      }
 

@@ -1054,7 +1057,7 @@ static int bq27xxx_battery_read_health(struct bq27xxx_device_info *di)
 
      flags = bq27xxx_read(di, BQ27XXX_REG_FLAGS, has_singe_flag);
      if (flags < 0) {
-             dev_err(di->dev, "error reading flag register:%d\n", flags);
+             dev_err(di->dev, "error %d reading flags\n", flags);
              return flags;
      }
 

@@ -1147,7 +1150,7 @@ static int bq27xxx_battery_current(struct bq27xxx_device_info *di,
 
      curr = bq27xxx_read(di, BQ27XXX_REG_AI, false);
      if (curr < 0) {
-             dev_err(di->dev, "error reading current\n");
+             dev_err(di->dev, "error %d reading current\n", curr);
              return curr;
      }
 

@@ -1236,7 +1239,7 @@ static int bq27xxx_battery_voltage(struct bq27xxx_device_info *di,
 
      volt = bq27xxx_read(di, BQ27XXX_REG_VOLT, false);
      if (volt < 0) {
-             dev_err(di->dev, "error reading voltage\n");
+             dev_err(di->dev, "error %d reading voltage\n", volt);
              return volt;
      }
 

Quoted 38 lines · Sebastian Reichel

Quoted 37 lines · Liam Breck

Quoted 35 lines · Andrew F. Davis

Quoted 21 lines · Liam Breck

From: Liam Breck <[email protected]>

Previously errors were reported in a mix of styles via both dev_err() & dev_dbg(). Report all errors with dev_err() and include error code in message. Report register ID separately in dev_dbg() message.

Signed-off-by Liam Breck <[email protected]>

---
 drivers/power/supply/bq27xxx_battery.c | 35 ++++++++++++++++++----------------
 1 file changed, 19 insertions(+), 16 deletions(-)

drivers/power/supply/bq27xxx_battery.c +1 -1

index 398801a..2e2a3a8 100644
@@ -794,11 +794,17 @@ MODULE_PARM_DESC(poll_interval,
 static inline int bq27xxx_read(struct bq27xxx_device_info *di, int reg_index,
                             bool single)
 {
-     /* Reports EINVAL for invalid/missing registers */
+     int ret;

drivers/power/supply/bq27xxx_battery.c MODULE_PARM_DESC(poll_interval, · line 806

Quoted 10 lines

drivers/power/supply/bq27xxx_battery.c +6 -1

+
      if (!di || di->regs[reg_index] == INVALID_REG_ADDR)
              return -EINVAL;
 
-     return di->bus.read(di, di->regs[reg_index], single);
+     ret = di->bus.read(di, di->regs[reg_index], single);
+     if (ret < 0)
+             dev_dbg(di->dev, "failed to read register 0x%02x (index %d)\n",
+                     di->regs[reg_index], reg_index);
+

If we print out the error code here we don't have to do that in each of the below printouts.

Quoted 7 lines

Quoted 3 lines

This is a dev_dbg, the below are dev_err, and I think you want error code in dev_err. BTW the above addresses your request to learn which register was in play in the event of a DM read/write failure.

Just make it dev_err and also print ret. Then we can remove all other prints. That will result in some information-loss (name of the register), but

Andrew specifically asked for error reporting that gives the error context elsewhere in the patchset. In complying with that request, I filed this patch.

We could pass a context string into read() ...

Hi,

Quoted 47 lines · Liam Breck

Quoted 45 lines · Sebastian Reichel

Quoted 37 lines · Liam Breck

Quoted 26 lines · Andrew F. Davis

Quoted 25 lines · Liam Breck

From: Liam Breck <[email protected]>

Previously errors were reported in a mix of styles via both dev_err() & dev_dbg(). Report all errors with dev_err() and include error code in message. Report register ID separately in dev_dbg() message.

Signed-off-by Liam Breck <[email protected]>

---
 drivers/power/supply/bq27xxx_battery.c | 35 ++++++++++++++++++----------------
 1 file changed, 19 insertions(+), 16 deletions(-)

drivers/power/supply/bq27xxx_battery.c +2 -1

index 398801a..2e2a3a8 100644
@@ -794,11 +794,17 @@ MODULE_PARM_DESC(poll_interval,
 static inline int bq27xxx_read(struct bq27xxx_device_info *di, int reg_index,
                             bool single)
 {
-     /* Reports EINVAL for invalid/missing registers */
+     int ret;
+
      if (!di || di->regs[reg_index] == INVALID_REG_ADDR)
              return -EINVAL;
 

Quoted 9 lines

drivers/power/supply/bq27xxx_battery.c MODULE_PARM_DESC(poll_interval, · line 806

Quoted 6 lines

drivers/power/supply/bq27xxx_battery.c +5 -1

-     return di->bus.read(di, di->regs[reg_index], single);
+     ret = di->bus.read(di, di->regs[reg_index], single);
+     if (ret < 0)
+             dev_dbg(di->dev, "failed to read register 0x%02x (index %d)\n",
+                     di->regs[reg_index], reg_index);
+

If we print out the error code here we don't have to do that in each of the below printouts.

Quoted 3 lines

This is a dev_dbg, the below are dev_err, and I think you want error code in dev_err. BTW the above addresses your request to learn which register was in play in the event of a DM read/write failure.

Just make it dev_err and also print ret. Then we can remove all other prints. That will result in some information-loss (name of the register), but

Quoted 5 lines

Andrew specifically asked for error reporting that gives the error context elsewhere in the patchset. In complying with that request, I filed this patch.

We could pass a context string into read() ...

That's fine with me.

From: Liam Breck <[email protected]>

Declare bus.write/read_bulk/write_bulk() to support get/set of chip registers, e.g. with inputs from power_supply_battery_info.

Add bq27xxx_write() & bq27xxx_xfer() helper functions to call the above.

Signed-off-by Matt Ranostay <[email protected]>

Signed-off-by Liam Breck <[email protected]>

---
 drivers/power/supply/bq27xxx_battery.c | 35 ++++++++++++++++++++++++++++++++++
 include/linux/power/bq27xxx_battery.h  |  3 +++
 2 files changed, 38 insertions(+)

drivers/power/supply/bq27xxx_battery.c +0 -0

index 2e2a3a8..f36a6f1 100644
@@ -807,6 +807,41 @@ static inline int bq27xxx_read(struct bq27xxx_device_info *di, int reg_index,
 	return ret;
 }

drivers/power/supply/bq27xxx_battery.c +26 -0

+	if (ret < 0)
+		dev_dbg(di->dev, "failed to write register 0x%02x (index %d)\n",
+			di->regs[reg_index], reg_index);
+
+	return ret;
+}
+
+static inline int bq27xxx_xfer(struct bq27xxx_device_info *di,
+			       struct bq27xxx_dm_buf *buf, bool read)
+{
+	int ret;
+	int (*xfer)(struct bq27xxx_device_info*, u8, u8*, int) =
+		read ? di->bus.read_bulk : di->bus.write_bulk;
+
+	if (!di || di->regs[BQ27XXX_DM_DATA] == INVALID_REG_ADDR)
+		return -EINVAL;
+
+	ret = xfer(di, di->regs[BQ27XXX_DM_DATA], buf->a, BQ27XXX_DM_SZ);
+	if (ret < 0)
+		dev_dbg(di->dev, "failed to %s_bulk register 0x%02x (index %d)\n",
+			read ? "read" : "write",
+			di->regs[BQ27XXX_DM_DATA], BQ27XXX_DM_DATA);
+
+	return ret;
+}
+
 /*
  * Return the battery State-of-Charge
  * Or < 0 if something fails.

include/linux/power/bq27xxx_battery.h +3 -0

index b312bce..c3369fa 100644
@@ -40,6 +40,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 {

From: Liam Breck <[email protected]>

Pass actual chip ID into _setup(), which translates it to a group ID, to allow support for all chips by the power_supply_battery_info code. There are no functional changes to the driver.

Signed-off-by Liam Breck <[email protected]>

 drivers/power/supply/bq27xxx_battery.c     | 27 +++++++++++++++++++++++++++
 drivers/power/supply/bq27xxx_battery_i2c.c | 16 ++++++++--------
 include/linux/power/bq27xxx_battery.h      | 11 +++++++++++
 3 files changed, 46 insertions(+), 8 deletions(-)

drivers/power/supply/bq27xxx_battery.c +27 -0

index f36a6f1..db1a388 100644
@@ -1391,6 +1391,33 @@ int bq27xxx_battery_setup(struct bq27xxx_device_info *di)
 	struct power_supply_desc *psy_desc;
 	struct power_supply_config psy_cfg = { .drv_data = di, };
 
+	switch(di->chip) {
+	case BQ27000:
+	case BQ27010:
+	case BQ2750X:
+	case BQ2751X:
+	case BQ27500:
+	case BQ27510G1:
+	case BQ27510G2:
+	case BQ27510G3:
+	case BQ27520G1:
+	case BQ27520G2:
+	case BQ27520G3:
+	case BQ27520G4:
+	case BQ27530:
+	case BQ27541:
+	case BQ27545:
+	case BQ27421: break;
+	case BQ2752X: di->chip = BQ2751X; break;
+	case BQ27531: di->chip = BQ27530; break;
+	case BQ27542: di->chip = BQ27541; break;
+	case BQ27546: di->chip = BQ27541; break;
+	case BQ27742: di->chip = BQ27541; break;
+	case BQ27425: di->chip = BQ27421; break;
+	case BQ27441: di->chip = BQ27421; break;
+	case BQ27621: di->chip = BQ27421; break;
+	}
+
 	INIT_DELAYED_WORK(&di->work, bq27xxx_battery_poll);
 	mutex_init(&di->lock);
 	di->regs = bq27xxx_regs[di->chip];

drivers/power/supply/bq27xxx_battery_i2c.c +8 -8

index c68fbc3..b3f2494 100644
@@ -150,7 +150,7 @@ static const struct i2c_device_id bq27xxx_i2c_id_table[] = {
 	{ "bq27210", BQ27010 },
 	{ "bq27500", BQ2750X },
 	{ "bq27510", BQ2751X },
-	{ "bq27520", BQ2751X },
+	{ "bq27520", BQ2752X },
 	{ "bq27500-1", BQ27500 },
 	{ "bq27510g1", BQ27510G1 },
 	{ "bq27510g2", BQ27510G2 },

@@ -160,16 +160,16 @@ static const struct i2c_device_id bq27xxx_i2c_id_table[] = {
 	{ "bq27520g3", BQ27520G3 },
 	{ "bq27520g4", BQ27520G4 },
 	{ "bq27530", BQ27530 },
-	{ "bq27531", BQ27530 },
+	{ "bq27531", BQ27531 },
 	{ "bq27541", BQ27541 },
-	{ "bq27542", BQ27541 },
-	{ "bq27546", BQ27541 },
-	{ "bq27742", BQ27541 },
+	{ "bq27542", BQ27542 },
+	{ "bq27546", BQ27546 },
+	{ "bq27742", BQ27742 },
 	{ "bq27545", BQ27545 },
 	{ "bq27421", BQ27421 },
-	{ "bq27425", BQ27421 },
-	{ "bq27441", BQ27421 },
-	{ "bq27621", BQ27421 },
+	{ "bq27425", BQ27425 },
+	{ "bq27441", BQ27441 },
+	{ "bq27621", BQ27621 },
 	{},
 };
 MODULE_DEVICE_TABLE(i2c, bq27xxx_i2c_id_table);

include/linux/power/bq27xxx_battery.h +11 -0

index c3369fa..96cec17 100644
@@ -2,6 +2,7 @@
 #define __LINUX_BQ27X00_BATTERY_H__
 
 enum bq27xxx_chip {
+	/* categories; index for bq27xxx_regs[] */
 	BQ27000 = 1, /* bq27000, bq27200 */
 	BQ27010, /* bq27010, bq27210 */
 	BQ2750X, /* bq27500 deprecated alias */

@@ -18,6 +19,16 @@ enum bq27xxx_chip {
 	BQ27541, /* bq27541, bq27542, bq27546, bq27742 */
 	BQ27545, /* bq27545 */
 	BQ27421, /* bq27421, bq27425, bq27441, bq27621 */
+
+	/* members of categories; translate these to category in _setup() */
+	BQ2752X, /* deprecated alias */
+	BQ27531,
+	BQ27542,
+	BQ27546,
+	BQ27742,
+	BQ27425,
+	BQ27441,
+	BQ27621,
 };
 
 /**

Signature · 1 line

2.9.3

Hi,

Quoted 6 lines · Liam Breck

From: Liam Breck <[email protected]>

Pass actual chip ID into _setup(), which translates it to a group ID, to allow support for all chips by the power_supply_battery_info code. There are no functional changes to the driver.

Quoted 1 line

Signed-off-by Liam Breck <[email protected]>

This is really ugly. If we need chip ID and group ID let's store them in different variables. For example put the detailed chipid into di->realchip and then do

switch(di->realchip) {
case FOO:

    di->chip = GRP_FOO;
    break;
case BAR:
    di->chip = GRP_BAR;
    break;
default:
    di->chip = di->realchip;
    break;
}

-- Sebastian

Quoted 112 lines

 drivers/power/supply/bq27xxx_battery.c     | 27 +++++++++++++++++++++++++++
 drivers/power/supply/bq27xxx_battery_i2c.c | 16 ++++++++--------
 include/linux/power/bq27xxx_battery.h      | 11 +++++++++++
 3 files changed, 46 insertions(+), 8 deletions(-)

drivers/power/supply/bq27xxx_battery.c +27 -0

index f36a6f1..db1a388 100644
@@ -1391,6 +1391,33 @@ int bq27xxx_battery_setup(struct bq27xxx_device_info *di)
      struct power_supply_desc *psy_desc;
      struct power_supply_config psy_cfg = { .drv_data = di, };
 
+     switch(di->chip) {
+     case BQ27000:
+     case BQ27010:
+     case BQ2750X:
+     case BQ2751X:
+     case BQ27500:
+     case BQ27510G1:
+     case BQ27510G2:
+     case BQ27510G3:
+     case BQ27520G1:
+     case BQ27520G2:
+     case BQ27520G3:
+     case BQ27520G4:
+     case BQ27530:
+     case BQ27541:
+     case BQ27545:
+     case BQ27421: break;
+     case BQ2752X: di->chip = BQ2751X; break;
+     case BQ27531: di->chip = BQ27530; break;
+     case BQ27542: di->chip = BQ27541; break;
+     case BQ27546: di->chip = BQ27541; break;
+     case BQ27742: di->chip = BQ27541; break;
+     case BQ27425: di->chip = BQ27421; break;
+     case BQ27441: di->chip = BQ27421; break;
+     case BQ27621: di->chip = BQ27421; break;
+     }
+
      INIT_DELAYED_WORK(&di->work, bq27xxx_battery_poll);
      mutex_init(&di->lock);
      di->regs = bq27xxx_regs[di->chip];

drivers/power/supply/bq27xxx_battery_i2c.c +8 -8

index c68fbc3..b3f2494 100644
@@ -150,7 +150,7 @@ static const struct i2c_device_id bq27xxx_i2c_id_table[] = {
      { "bq27210", BQ27010 },
      { "bq27500", BQ2750X },
      { "bq27510", BQ2751X },
-     { "bq27520", BQ2751X },
+     { "bq27520", BQ2752X },
      { "bq27500-1", BQ27500 },
      { "bq27510g1", BQ27510G1 },
      { "bq27510g2", BQ27510G2 },

@@ -160,16 +160,16 @@ static const struct i2c_device_id bq27xxx_i2c_id_table[] = {
      { "bq27520g3", BQ27520G3 },
      { "bq27520g4", BQ27520G4 },
      { "bq27530", BQ27530 },
-     { "bq27531", BQ27530 },
+     { "bq27531", BQ27531 },
      { "bq27541", BQ27541 },
-     { "bq27542", BQ27541 },
-     { "bq27546", BQ27541 },
-     { "bq27742", BQ27541 },
+     { "bq27542", BQ27542 },
+     { "bq27546", BQ27546 },
+     { "bq27742", BQ27742 },
      { "bq27545", BQ27545 },
      { "bq27421", BQ27421 },
-     { "bq27425", BQ27421 },
-     { "bq27441", BQ27421 },
-     { "bq27621", BQ27421 },
+     { "bq27425", BQ27425 },
+     { "bq27441", BQ27441 },
+     { "bq27621", BQ27621 },
      {},
 };
 MODULE_DEVICE_TABLE(i2c, bq27xxx_i2c_id_table);

include/linux/power/bq27xxx_battery.h +11 -1

index c3369fa..96cec17 100644
@@ -2,6 +2,7 @@
 #define __LINUX_BQ27X00_BATTERY_H__
 
 enum bq27xxx_chip {
+     /* categories; index for bq27xxx_regs[] */
      BQ27000 = 1, /* bq27000, bq27200 */
      BQ27010, /* bq27010, bq27210 */
      BQ2750X, /* bq27500 deprecated alias */

@@ -18,6 +19,16 @@ enum bq27xxx_chip {
      BQ27541, /* bq27541, bq27542, bq27546, bq27742 */
      BQ27545, /* bq27545 */
      BQ27421, /* bq27421, bq27425, bq27441, bq27621 */
+
+     /* members of categories; translate these to category in _setup() */
+     BQ2752X, /* deprecated alias */
+     BQ27531,
+     BQ27542,
+     BQ27546,
+     BQ27742,
+     BQ27425,
+     BQ27441,
+     BQ27621,
 };
 
 /**
--

2.9.3

Quoted 11 lines · Sebastian Reichel

Hi,

Quoted 7 lines · Liam Breck

From: Liam Breck <[email protected]>

Pass actual chip ID into _setup(), which translates it to a group ID, to allow support for all chips by the power_supply_battery_info code. There are no functional changes to the driver.

Signed-off-by Liam Breck <[email protected]>

Quoted 3 lines

This is really ugly. If we need chip ID and group ID let's store them in different variables. For example put the detailed chipid into di->realchip and then do

I tried this in a previous version, and Andrew rejected it as confusing to have two IDs. That was while you were away on business :-)

Hi,

Quoted 16 lines · Liam Breck

Quoted 14 lines · Sebastian Reichel

Hi,

Quoted 7 lines · Liam Breck

From: Liam Breck <[email protected]>

Pass actual chip ID into _setup(), which translates it to a group ID, to allow support for all chips by the power_supply_battery_info code. There are no functional changes to the driver.

Signed-off-by Liam Breck <[email protected]>

This is really ugly. If we need chip ID and group ID let's store them in different variables. For example put the detailed chipid into di->realchip and then do

Quoted 3 lines

I tried this in a previous version, and Andrew rejected it as confusing to have two IDs. That was while you were away on business :-)

now we have two IDs in the same variable.
That definitely more confusing. We can name the second variable group to reduce confusion.

Quoted 23 lines · Sebastian Reichel

Hi,

Quoted 19 lines · Liam Breck

Quoted 9 lines · Sebastian Reichel

Hi,

Quoted 6 lines · Liam Breck

From: Liam Breck <[email protected]>

Pass actual chip ID into _setup(), which translates it to a group ID, to allow support for all chips by the power_supply_battery_info code. There are no functional changes to the driver.

Quoted 5 lines

Quoted 1 line

Signed-off-by Liam Breck <[email protected]>

This is really ugly. If we need chip ID and group ID let's store them in different variables. For example put the detailed chipid into di->realchip and then do

I tried this in a previous version, and Andrew rejected it as confusing to have two IDs. That was while you were away on business :-)

Quoted 3 lines

now we have two IDs in the same variable.
That definitely more confusing. We can name the second variable group to reduce confusion.

I was trying not to rename every instance of di->chip; there's a lot.

But we can pass real_chip as an argument to this function, so that gets you what you wanted.

Quoted 11 lines · Sebastian Reichel

Hi,

Quoted 7 lines · Liam Breck

From: Liam Breck <[email protected]>

Pass actual chip ID into _setup(), which translates it to a group ID, to allow support for all chips by the power_supply_battery_info code. There are no functional changes to the driver.

Signed-off-by Liam Breck <[email protected]>

Quoted 3 lines

This is really ugly. If we need chip ID and group ID let's store them in different variables. For example put the detailed chipid into di->realchip and then do

I tried this in a previous version, and Andrew rejected it as confusing to have two IDs. That was while you were away on business :-)

Quoted 126 lines

switch(di->realchip) {
case FOO:

    di->chip = GRP_FOO;
    break;
case BAR:
    di->chip = GRP_BAR;
    break;
default:
    di->chip = di->realchip;
    break;
}

-- Sebastian

Quoted 112 lines

 drivers/power/supply/bq27xxx_battery.c     | 27 +++++++++++++++++++++++++++
 drivers/power/supply/bq27xxx_battery_i2c.c | 16 ++++++++--------
 include/linux/power/bq27xxx_battery.h      | 11 +++++++++++
 3 files changed, 46 insertions(+), 8 deletions(-)

drivers/power/supply/bq27xxx_battery.c +27 -0

index f36a6f1..db1a388 100644
@@ -1391,6 +1391,33 @@ int bq27xxx_battery_setup(struct bq27xxx_device_info *di)
      struct power_supply_desc *psy_desc;
      struct power_supply_config psy_cfg = { .drv_data = di, };
 
+     switch(di->chip) {
+     case BQ27000:
+     case BQ27010:
+     case BQ2750X:
+     case BQ2751X:
+     case BQ27500:
+     case BQ27510G1:
+     case BQ27510G2:
+     case BQ27510G3:
+     case BQ27520G1:
+     case BQ27520G2:
+     case BQ27520G3:
+     case BQ27520G4:
+     case BQ27530:
+     case BQ27541:
+     case BQ27545:
+     case BQ27421: break;
+     case BQ2752X: di->chip = BQ2751X; break;
+     case BQ27531: di->chip = BQ27530; break;
+     case BQ27542: di->chip = BQ27541; break;
+     case BQ27546: di->chip = BQ27541; break;
+     case BQ27742: di->chip = BQ27541; break;
+     case BQ27425: di->chip = BQ27421; break;
+     case BQ27441: di->chip = BQ27421; break;
+     case BQ27621: di->chip = BQ27421; break;
+     }
+
      INIT_DELAYED_WORK(&di->work, bq27xxx_battery_poll);
      mutex_init(&di->lock);
      di->regs = bq27xxx_regs[di->chip];

drivers/power/supply/bq27xxx_battery_i2c.c +8 -8

index c68fbc3..b3f2494 100644
@@ -150,7 +150,7 @@ static const struct i2c_device_id bq27xxx_i2c_id_table[] = {
      { "bq27210", BQ27010 },
      { "bq27500", BQ2750X },
      { "bq27510", BQ2751X },
-     { "bq27520", BQ2751X },
+     { "bq27520", BQ2752X },
      { "bq27500-1", BQ27500 },
      { "bq27510g1", BQ27510G1 },
      { "bq27510g2", BQ27510G2 },

@@ -160,16 +160,16 @@ static const struct i2c_device_id bq27xxx_i2c_id_table[] = {
      { "bq27520g3", BQ27520G3 },
      { "bq27520g4", BQ27520G4 },
      { "bq27530", BQ27530 },
-     { "bq27531", BQ27530 },
+     { "bq27531", BQ27531 },
      { "bq27541", BQ27541 },
-     { "bq27542", BQ27541 },
-     { "bq27546", BQ27541 },
-     { "bq27742", BQ27541 },
+     { "bq27542", BQ27542 },
+     { "bq27546", BQ27546 },
+     { "bq27742", BQ27742 },
      { "bq27545", BQ27545 },
      { "bq27421", BQ27421 },
-     { "bq27425", BQ27421 },
-     { "bq27441", BQ27421 },
-     { "bq27621", BQ27421 },
+     { "bq27425", BQ27425 },
+     { "bq27441", BQ27441 },
+     { "bq27621", BQ27621 },
      {},
 };
 MODULE_DEVICE_TABLE(i2c, bq27xxx_i2c_id_table);

include/linux/power/bq27xxx_battery.h +11 -1

index c3369fa..96cec17 100644
@@ -2,6 +2,7 @@
 #define __LINUX_BQ27X00_BATTERY_H__
 
 enum bq27xxx_chip {
+     /* categories; index for bq27xxx_regs[] */
      BQ27000 = 1, /* bq27000, bq27200 */
      BQ27010, /* bq27010, bq27210 */
      BQ2750X, /* bq27500 deprecated alias */

@@ -18,6 +19,16 @@ enum bq27xxx_chip {
      BQ27541, /* bq27541, bq27542, bq27546, bq27742 */
      BQ27545, /* bq27545 */
      BQ27421, /* bq27421, bq27425, bq27441, bq27621 */
+
+     /* members of categories; translate these to category in _setup() */
+     BQ2752X, /* deprecated alias */
+     BQ27531,
+     BQ27542,
+     BQ27546,
+     BQ27742,
+     BQ27425,
+     BQ27441,
+     BQ27621,
 };
 
 /**
--

2.9.3

Quoted 16 lines · Liam Breck

Quoted 14 lines · Sebastian Reichel

Hi,

Quoted 7 lines · Liam Breck

From: Liam Breck <[email protected]>

Pass actual chip ID into _setup(), which translates it to a group ID, to allow support for all chips by the power_supply_battery_info code. There are no functional changes to the driver.

Signed-off-by Liam Breck <[email protected]>

This is really ugly. If we need chip ID and group ID let's store them in different variables. For example put the detailed chipid into di->realchip and then do

Quoted 4 lines

I tried this in a previous version, and Andrew rejected it as confusing to have two IDs. That was while you were away on business :-)

I rejected two tables, to indexes is what I've been pushing for a while.

Quoted 126 lines

Quoted 126 lines

switch(di->realchip) {
case FOO:

    di->chip = GRP_FOO;
    break;
case BAR:
    di->chip = GRP_BAR;
    break;
default:
    di->chip = di->realchip;
    break;
}

-- Sebastian

Quoted 112 lines

 drivers/power/supply/bq27xxx_battery.c     | 27 +++++++++++++++++++++++++++
 drivers/power/supply/bq27xxx_battery_i2c.c | 16 ++++++++--------
 include/linux/power/bq27xxx_battery.h      | 11 +++++++++++
 3 files changed, 46 insertions(+), 8 deletions(-)

drivers/power/supply/bq27xxx_battery.c +27 -0

index f36a6f1..db1a388 100644
@@ -1391,6 +1391,33 @@ int bq27xxx_battery_setup(struct bq27xxx_device_info *di)
      struct power_supply_desc *psy_desc;
      struct power_supply_config psy_cfg = { .drv_data = di, };
 
+     switch(di->chip) {
+     case BQ27000:
+     case BQ27010:
+     case BQ2750X:
+     case BQ2751X:
+     case BQ27500:
+     case BQ27510G1:
+     case BQ27510G2:
+     case BQ27510G3:
+     case BQ27520G1:
+     case BQ27520G2:
+     case BQ27520G3:
+     case BQ27520G4:
+     case BQ27530:
+     case BQ27541:
+     case BQ27545:
+     case BQ27421: break;
+     case BQ2752X: di->chip = BQ2751X; break;
+     case BQ27531: di->chip = BQ27530; break;
+     case BQ27542: di->chip = BQ27541; break;
+     case BQ27546: di->chip = BQ27541; break;
+     case BQ27742: di->chip = BQ27541; break;
+     case BQ27425: di->chip = BQ27421; break;
+     case BQ27441: di->chip = BQ27421; break;
+     case BQ27621: di->chip = BQ27421; break;
+     }
+
      INIT_DELAYED_WORK(&di->work, bq27xxx_battery_poll);
      mutex_init(&di->lock);
      di->regs = bq27xxx_regs[di->chip];

drivers/power/supply/bq27xxx_battery_i2c.c +8 -8

index c68fbc3..b3f2494 100644
@@ -150,7 +150,7 @@ static const struct i2c_device_id bq27xxx_i2c_id_table[] = {
      { "bq27210", BQ27010 },
      { "bq27500", BQ2750X },
      { "bq27510", BQ2751X },
-     { "bq27520", BQ2751X },
+     { "bq27520", BQ2752X },
      { "bq27500-1", BQ27500 },
      { "bq27510g1", BQ27510G1 },
      { "bq27510g2", BQ27510G2 },

@@ -160,16 +160,16 @@ static const struct i2c_device_id bq27xxx_i2c_id_table[] = {
      { "bq27520g3", BQ27520G3 },
      { "bq27520g4", BQ27520G4 },
      { "bq27530", BQ27530 },
-     { "bq27531", BQ27530 },
+     { "bq27531", BQ27531 },
      { "bq27541", BQ27541 },
-     { "bq27542", BQ27541 },
-     { "bq27546", BQ27541 },
-     { "bq27742", BQ27541 },
+     { "bq27542", BQ27542 },
+     { "bq27546", BQ27546 },
+     { "bq27742", BQ27742 },
      { "bq27545", BQ27545 },
      { "bq27421", BQ27421 },
-     { "bq27425", BQ27421 },
-     { "bq27441", BQ27421 },
-     { "bq27621", BQ27421 },
+     { "bq27425", BQ27425 },
+     { "bq27441", BQ27441 },
+     { "bq27621", BQ27621 },
      {},
 };
 MODULE_DEVICE_TABLE(i2c, bq27xxx_i2c_id_table);

include/linux/power/bq27xxx_battery.h +11 -1

index c3369fa..96cec17 100644
@@ -2,6 +2,7 @@
 #define __LINUX_BQ27X00_BATTERY_H__
 
 enum bq27xxx_chip {
+     /* categories; index for bq27xxx_regs[] */
      BQ27000 = 1, /* bq27000, bq27200 */
      BQ27010, /* bq27010, bq27210 */
      BQ2750X, /* bq27500 deprecated alias */

@@ -18,6 +19,16 @@ enum bq27xxx_chip {
      BQ27541, /* bq27541, bq27542, bq27546, bq27742 */
      BQ27545, /* bq27545 */
      BQ27421, /* bq27421, bq27425, bq27441, bq27621 */
+
+     /* members of categories; translate these to category in _setup() */
+     BQ2752X, /* deprecated alias */
+     BQ27531,
+     BQ27542,
+     BQ27546,
+     BQ27742,
+     BQ27425,
+     BQ27441,
+     BQ27621,
 };
 
 /**
--

2.9.3

Quoted 22 lines · Andrew F. Davis

Quoted 20 lines · Liam Breck

Quoted 9 lines · Sebastian Reichel

Hi,

Quoted 6 lines · Liam Breck

From: Liam Breck <[email protected]>

Pass actual chip ID into _setup(), which translates it to a group ID, to allow support for all chips by the power_supply_battery_info code. There are no functional changes to the driver.

Quoted 5 lines

Quoted 1 line

Signed-off-by Liam Breck <[email protected]>

This is really ugly. If we need chip ID and group ID let's store them in different variables. For example put the detailed chipid into di->realchip and then do

I tried this in a previous version, and Andrew rejected it as confusing to have two IDs. That was while you were away on business :-)

Quoted 1 line

I rejected two tables, to indexes is what I've been pushing for a while.

On March 9, you wrote, "each chip should have one unique ID, I think what ever way you prefer would work for the table structure, lets just keep the chip ID enums clean"

Anyway, we have a solution - realid via argument.

Quoted 126 lines

Quoted 126 lines

Quoted 126 lines

switch(di->realchip) {
case FOO:

    di->chip = GRP_FOO;
    break;
case BAR:
    di->chip = GRP_BAR;
    break;
default:
    di->chip = di->realchip;
    break;
}

-- Sebastian

Quoted 112 lines

 drivers/power/supply/bq27xxx_battery.c     | 27 +++++++++++++++++++++++++++
 drivers/power/supply/bq27xxx_battery_i2c.c | 16 ++++++++--------
 include/linux/power/bq27xxx_battery.h      | 11 +++++++++++
 3 files changed, 46 insertions(+), 8 deletions(-)

drivers/power/supply/bq27xxx_battery.c +27 -0

index f36a6f1..db1a388 100644
@@ -1391,6 +1391,33 @@ int bq27xxx_battery_setup(struct bq27xxx_device_info *di)
      struct power_supply_desc *psy_desc;
      struct power_supply_config psy_cfg = { .drv_data = di, };
 
+     switch(di->chip) {
+     case BQ27000:
+     case BQ27010:
+     case BQ2750X:
+     case BQ2751X:
+     case BQ27500:
+     case BQ27510G1:
+     case BQ27510G2:
+     case BQ27510G3:
+     case BQ27520G1:
+     case BQ27520G2:
+     case BQ27520G3:
+     case BQ27520G4:
+     case BQ27530:
+     case BQ27541:
+     case BQ27545:
+     case BQ27421: break;
+     case BQ2752X: di->chip = BQ2751X; break;
+     case BQ27531: di->chip = BQ27530; break;
+     case BQ27542: di->chip = BQ27541; break;
+     case BQ27546: di->chip = BQ27541; break;
+     case BQ27742: di->chip = BQ27541; break;
+     case BQ27425: di->chip = BQ27421; break;
+     case BQ27441: di->chip = BQ27421; break;
+     case BQ27621: di->chip = BQ27421; break;
+     }
+
      INIT_DELAYED_WORK(&di->work, bq27xxx_battery_poll);
      mutex_init(&di->lock);
      di->regs = bq27xxx_regs[di->chip];

drivers/power/supply/bq27xxx_battery_i2c.c +8 -8

index c68fbc3..b3f2494 100644
@@ -150,7 +150,7 @@ static const struct i2c_device_id bq27xxx_i2c_id_table[] = {
      { "bq27210", BQ27010 },
      { "bq27500", BQ2750X },
      { "bq27510", BQ2751X },
-     { "bq27520", BQ2751X },
+     { "bq27520", BQ2752X },
      { "bq27500-1", BQ27500 },
      { "bq27510g1", BQ27510G1 },
      { "bq27510g2", BQ27510G2 },

@@ -160,16 +160,16 @@ static const struct i2c_device_id bq27xxx_i2c_id_table[] = {
      { "bq27520g3", BQ27520G3 },
      { "bq27520g4", BQ27520G4 },
      { "bq27530", BQ27530 },
-     { "bq27531", BQ27530 },
+     { "bq27531", BQ27531 },
      { "bq27541", BQ27541 },
-     { "bq27542", BQ27541 },
-     { "bq27546", BQ27541 },
-     { "bq27742", BQ27541 },
+     { "bq27542", BQ27542 },
+     { "bq27546", BQ27546 },
+     { "bq27742", BQ27742 },
      { "bq27545", BQ27545 },
      { "bq27421", BQ27421 },
-     { "bq27425", BQ27421 },
-     { "bq27441", BQ27421 },
-     { "bq27621", BQ27421 },
+     { "bq27425", BQ27425 },
+     { "bq27441", BQ27441 },
+     { "bq27621", BQ27621 },
      {},
 };
 MODULE_DEVICE_TABLE(i2c, bq27xxx_i2c_id_table);

include/linux/power/bq27xxx_battery.h +11 -1

index c3369fa..96cec17 100644
@@ -2,6 +2,7 @@
 #define __LINUX_BQ27X00_BATTERY_H__
 
 enum bq27xxx_chip {
+     /* categories; index for bq27xxx_regs[] */
      BQ27000 = 1, /* bq27000, bq27200 */
      BQ27010, /* bq27010, bq27210 */
      BQ2750X, /* bq27500 deprecated alias */

@@ -18,6 +19,16 @@ enum bq27xxx_chip {
      BQ27541, /* bq27541, bq27542, bq27546, bq27742 */
      BQ27545, /* bq27545 */
      BQ27421, /* bq27421, bq27425, bq27441, bq27621 */
+
+     /* members of categories; translate these to category in _setup() */
+     BQ2752X, /* deprecated alias */
+     BQ27531,
+     BQ27542,
+     BQ27546,
+     BQ27742,
+     BQ27425,
+     BQ27441,
+     BQ27621,
 };
 
 /**
--

2.9.3

Quoted 23 lines · Liam Breck

Quoted 22 lines · Andrew F. Davis

Quoted 20 lines · Liam Breck

Quoted 9 lines · Sebastian Reichel

Hi,

Quoted 6 lines · Liam Breck

From: Liam Breck <[email protected]>

Pass actual chip ID into _setup(), which translates it to a group ID, to allow support for all chips by the power_supply_battery_info code. There are no functional changes to the driver.

Quoted 5 lines

Quoted 1 line

Signed-off-by Liam Breck <[email protected]>

This is really ugly. If we need chip ID and group ID let's store them in different variables. For example put the detailed chipid into di->realchip and then do

I tried this in a previous version, and Andrew rejected it as confusing to have two IDs. That was while you were away on business :-)

Quoted 6 lines

Quoted 1 line

I rejected two tables, to indexes is what I've been pushing for a while.

On March 9, you wrote, "each chip should have one unique ID, I think what ever way you prefer would work for the table structure, lets just keep the chip ID enums clean"

And I still want this, the chip id enum is *not* clean right now, a BQ27531 for instance could be BQ27530 or BQ27531. enum bq27xxx_chip should not have "categories" and "members of categories" in one enum, it's a hack to reduce the number of table indexes, just add di->realchip or similar as Sebastian suggested and problem solved.

Quoted 128 lines

Anyway, we have a solution - realid via argument.

Quoted 126 lines

Quoted 126 lines

Quoted 126 lines

switch(di->realchip) {
case FOO:

    di->chip = GRP_FOO;
    break;
case BAR:
    di->chip = GRP_BAR;
    break;
default:
    di->chip = di->realchip;
    break;
}

-- Sebastian

Quoted 112 lines

 drivers/power/supply/bq27xxx_battery.c     | 27 +++++++++++++++++++++++++++
 drivers/power/supply/bq27xxx_battery_i2c.c | 16 ++++++++--------
 include/linux/power/bq27xxx_battery.h      | 11 +++++++++++
 3 files changed, 46 insertions(+), 8 deletions(-)

drivers/power/supply/bq27xxx_battery.c +27 -0

index f36a6f1..db1a388 100644
@@ -1391,6 +1391,33 @@ int bq27xxx_battery_setup(struct bq27xxx_device_info *di)
      struct power_supply_desc *psy_desc;
      struct power_supply_config psy_cfg = { .drv_data = di, };
 
+     switch(di->chip) {
+     case BQ27000:
+     case BQ27010:
+     case BQ2750X:
+     case BQ2751X:
+     case BQ27500:
+     case BQ27510G1:
+     case BQ27510G2:
+     case BQ27510G3:
+     case BQ27520G1:
+     case BQ27520G2:
+     case BQ27520G3:
+     case BQ27520G4:
+     case BQ27530:
+     case BQ27541:
+     case BQ27545:
+     case BQ27421: break;
+     case BQ2752X: di->chip = BQ2751X; break;
+     case BQ27531: di->chip = BQ27530; break;
+     case BQ27542: di->chip = BQ27541; break;
+     case BQ27546: di->chip = BQ27541; break;
+     case BQ27742: di->chip = BQ27541; break;
+     case BQ27425: di->chip = BQ27421; break;
+     case BQ27441: di->chip = BQ27421; break;
+     case BQ27621: di->chip = BQ27421; break;
+     }
+
      INIT_DELAYED_WORK(&di->work, bq27xxx_battery_poll);
      mutex_init(&di->lock);
      di->regs = bq27xxx_regs[di->chip];

drivers/power/supply/bq27xxx_battery_i2c.c +8 -8

index c68fbc3..b3f2494 100644
@@ -150,7 +150,7 @@ static const struct i2c_device_id bq27xxx_i2c_id_table[] = {
      { "bq27210", BQ27010 },
      { "bq27500", BQ2750X },
      { "bq27510", BQ2751X },
-     { "bq27520", BQ2751X },
+     { "bq27520", BQ2752X },
      { "bq27500-1", BQ27500 },
      { "bq27510g1", BQ27510G1 },
      { "bq27510g2", BQ27510G2 },

@@ -160,16 +160,16 @@ static const struct i2c_device_id bq27xxx_i2c_id_table[] = {
      { "bq27520g3", BQ27520G3 },
      { "bq27520g4", BQ27520G4 },
      { "bq27530", BQ27530 },
-     { "bq27531", BQ27530 },
+     { "bq27531", BQ27531 },
      { "bq27541", BQ27541 },
-     { "bq27542", BQ27541 },
-     { "bq27546", BQ27541 },
-     { "bq27742", BQ27541 },
+     { "bq27542", BQ27542 },
+     { "bq27546", BQ27546 },
+     { "bq27742", BQ27742 },
      { "bq27545", BQ27545 },
      { "bq27421", BQ27421 },
-     { "bq27425", BQ27421 },
-     { "bq27441", BQ27421 },
-     { "bq27621", BQ27421 },
+     { "bq27425", BQ27425 },
+     { "bq27441", BQ27441 },
+     { "bq27621", BQ27621 },
      {},
 };
 MODULE_DEVICE_TABLE(i2c, bq27xxx_i2c_id_table);

include/linux/power/bq27xxx_battery.h +11 -1

index c3369fa..96cec17 100644
@@ -2,6 +2,7 @@
 #define __LINUX_BQ27X00_BATTERY_H__
 
 enum bq27xxx_chip {
+     /* categories; index for bq27xxx_regs[] */
      BQ27000 = 1, /* bq27000, bq27200 */
      BQ27010, /* bq27010, bq27210 */
      BQ2750X, /* bq27500 deprecated alias */

@@ -18,6 +19,16 @@ enum bq27xxx_chip {
      BQ27541, /* bq27541, bq27542, bq27546, bq27742 */
      BQ27545, /* bq27545 */
      BQ27421, /* bq27421, bq27425, bq27441, bq27621 */
+
+     /* members of categories; translate these to category in _setup() */
+     BQ2752X, /* deprecated alias */
+     BQ27531,
+     BQ27542,
+     BQ27546,
+     BQ27742,
+     BQ27425,
+     BQ27441,
+     BQ27621,
 };
 
 /**
--

2.9.3