* [PATCH v14 00/11] devicetree simple-battery and client in bq27xxx_battery
@ 2017-06-07 18:37 Liam Breck
2017-06-07 18:37 ` [PATCH v14 01/11] devicetree: property-units: Add uWh and uAh units Liam Breck
` (11 more replies)
0 siblings, 12 replies; 25+ messages in thread
From: Liam Breck @ 2017-06-07 18:37 UTC (permalink / raw)
To: Sebastian Reichel, Pali Rohar, linux-pm
Cc: Enric Balletbo, Paul Kocialkowski, Quentin Schulz
Hi Sebastian, I think this is finally ready to go. I'll make a final
test pass while awaiting any further comments. There's a lot here, so
I'd appreciate it if you could look closely, as it's been a while since
you had any feedback. Also note comment about Rob's ack in v13 change list.
Enric, Paul, Quentin, and I all have patchsets waiting on this one.
Thanks!
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 RAM/NVM for params
essential to correct operation: energy-full-design-microwatt-hours,
charge-full-design-microamp-hours, voltage-min-design-microvolt
Changes in v14:
Doc...bindings/power/supply/ & power_supply_core
* add constant-charge-{current,voltage}-max-* for Enric's patchset
bq27xxx_battery:
* separate set_cfgupdate() and soft_reset() functions
* module param dt_monitored_battery_updates_nvm visible even when disabled
* real_chip & chip IDs combined in i2c_id_table[]
* hide untested chip params behind ifdef DEBUG
Changes in v13:
Doc...bindings/power/supply/*
* add fields in battery.txt, see below
* caution about changing battery type in battery.txt
* note: Rob acked v11 battery.txt containing URL to Linux header
* more detail in bq27xxx.txt
power_supply_core:
* add battery_info fields precharge-current-microamp & charge-term-current-microamp
* new patch for power_supply_prop_precharge_current
bq27xxx_battery:
* add copyright notice
* for flash/NVM chips, emit warning instead of doing update
* config_battery_bq27xxx_dt_updates_nvm enables update of flash/NVM
* module param dt_monitored_battery_updates_nvm lets user disallow update
* new patch to flag dupes in bq27xxx_regs[]
* drop patch to clean up error reporting
* polishing from Andrew's feedback
* resolve checkpatch errors
* refactor patchset
* fix missing static keyword flagged by Colin
Changes in v12: (several partial series posts; see above)
Changes in v11:
power_supply_core:
* switch to compatible = "simple-battery"
* add docs to power_supply_class.txt
Documentation/devicetree/.../battery.txt:
* add description, drop refs to Linux, "simple-battery"
bq27xxx_battery:
* reset flash chips after DM update
* add bq27xxx_write/xfer()
* polishing from Andrew's feedback
* new patch to clean up error reporting
* new patch to consolidate duplicate register/property arrays
Changes in v10:
bq27xxx_battery:
* pass actual chip ID into _setup()
* add di->unseal_key & di->dm_regs; drop static arrays
* support bq27425, 441, 621
Changes in v9:
bq27xxx_battery:
* fix set_cfgupdate()
* support bq27500, 545, 421; defer others
* drop print_dm_blocks() patch
* minor polishing
Documentation/devicetree/.../battery.txt:
* describe rationale for enum power_supply_property names
Changes in v8:
bq27xxx_battery:
* wait on flag after set_cfgupdate & soft_reset
* drop print_config(), report status in update_dm_block()
* clarify error messages
* cleanup from Andrew's feedback; minor polishing
Changes in v7:
bq27xxx_battery:
* support chips where terminate_voltage & design_* live in separate blocks
* draft support for 421, 441, 621 chips
* new patch to log chip memory fields
* report bus I/O errors; return error code in bq27xxx_battery_i2c
* verify checksum in read_dm_block()
* use set_cfgupdate only if chip provides it, soft_reset on I/O error
* block_data_control=0 only in write_dm_block()
* note toxic code from TI bqtool in write_dm_block()
* lots of functionally neutral polishing
Documentation/devicetree/.../battery.txt:
* mention power_supply_get_battery_info()
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
Liam Breck (9):
dt-bindings: power: supply: Add battery.txt with simple-battery binding
power: supply: core: Add power_supply_battery_info and API
power: supply: core: Add power_supply_prop_precharge
dt-bindings: power: supply: bq27xxx: Add monitored-battery documentation
power: supply: bq27xxx: Add chip data memory read/write support
power: supply: bq27xxx: Add power_supply_battery_info support
power: supply: bq27xxx: Enable data memory update for certain chips
power: supply: bq27xxx: Flag identical register maps when in debug mode
power: supply: bq27xxx: Remove duplicate register arrays
Matt Ranostay (2):
devicetree: property-units: Add uWh and uAh units
power: supply: bq27xxx: Add bulk transfer bus methods
.../devicetree/bindings/power/supply/battery.txt | 57 ++
.../devicetree/bindings/power/supply/bq27xxx.txt | 31 +-
.../devicetree/bindings/property-units.txt | 2 +
Documentation/power/power_supply_class.txt | 31 +-
drivers/power/supply/Kconfig | 11 +
drivers/power/supply/bq27xxx_battery.c | 752 +++++++++++++++++----
drivers/power/supply/bq27xxx_battery_i2c.c | 134 +++-
drivers/power/supply/power_supply_core.c | 57 ++
drivers/power/supply/power_supply_sysfs.c | 1 +
include/linux/power/bq27xxx_battery.h | 30 +-
include/linux/power_supply.h | 25 +
11 files changed, 955 insertions(+), 176 deletions(-)
create mode 100644 Documentation/devicetree/bindings/power/supply/battery.txt
--
2.13.0
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH v14 01/11] devicetree: property-units: Add uWh and uAh units
2017-06-07 18:37 [PATCH v14 00/11] devicetree simple-battery and client in bq27xxx_battery Liam Breck
@ 2017-06-07 18:37 ` Liam Breck
2017-06-07 18:37 ` [PATCH v14 02/11] dt-bindings: power: supply: Add battery.txt with simple-battery binding Liam Breck
` (10 subsequent siblings)
11 siblings, 0 replies; 25+ messages in thread
From: Liam Breck @ 2017-06-07 18:37 UTC (permalink / raw)
To: Sebastian Reichel, Pali Rohar, linux-pm
Cc: Enric Balletbo, Paul Kocialkowski, Quentin Schulz, Rob Herring,
Mark Rutland, devicetree, linux-kernel, Matt Ranostay,
Liam Breck
From: Matt Ranostay <matt@ranostay.consulting>
Add entries for microwatt-hours and microamp-hours.
Cc: Rob Herring <robh@kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: devicetree@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Matt Ranostay <matt@ranostay.consulting>
Signed-off-by: Liam Breck <kernel@networkimprov.net>
Acked-by: Sebastian Reichel <sre@kernel.org>
Acked-by: Rob Herring <robh@kernel.org>
---
Documentation/devicetree/bindings/property-units.txt | 2 ++
1 file changed, 2 insertions(+)
diff --git a/Documentation/devicetree/bindings/property-units.txt b/Documentation/devicetree/bindings/property-units.txt
index 12278d79..0849618a 100644
--- a/Documentation/devicetree/bindings/property-units.txt
+++ b/Documentation/devicetree/bindings/property-units.txt
@@ -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
--
2.13.0
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH v14 02/11] dt-bindings: power: supply: Add battery.txt with simple-battery binding
2017-06-07 18:37 [PATCH v14 00/11] devicetree simple-battery and client in bq27xxx_battery Liam Breck
2017-06-07 18:37 ` [PATCH v14 01/11] devicetree: property-units: Add uWh and uAh units Liam Breck
@ 2017-06-07 18:37 ` Liam Breck
2017-06-07 18:37 ` [PATCH v14 03/11] power: supply: core: Add power_supply_battery_info and API Liam Breck
` (9 subsequent siblings)
11 siblings, 0 replies; 25+ messages in thread
From: Liam Breck @ 2017-06-07 18:37 UTC (permalink / raw)
To: Sebastian Reichel, Pali Rohar, linux-pm
Cc: Enric Balletbo, Paul Kocialkowski, Quentin Schulz, Rob Herring,
devicetree, Matt Ranostay, Liam Breck
From: Liam Breck <kernel@networkimprov.net>
Documentation of static battery characteristics that can be defined
for batteries that do not embed this data, which are required by
fuel-gauge and charger chips for proper handling of the battery.
The following properties are defined:
voltage-min-design-microvolt
charge-full-design-microamp-hours
energy-full-design-microwatt-hours
precharge-current-microamp
charge-term-current-microamp
constant-charge-current-max-microamp
constant-charge-voltage-max-microamp
Property names are derived from corresponding elements in
enum power_supply_property from include/linux/power_supply.h
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/linux/power_supply.h
Cc: Rob Herring <robh@kernel.org>
Cc: devicetree@vger.kernel.org
Signed-off-by: Matt Ranostay <matt@ranostay.consulting>
Signed-off-by: Liam Breck <kernel@networkimprov.net>
Acked-by: Rob Herring <robh@kernel.org>
---
.../devicetree/bindings/power/supply/battery.txt | 57 ++++++++++++++++++++++
1 file changed, 57 insertions(+)
create mode 100644 Documentation/devicetree/bindings/power/supply/battery.txt
diff --git a/Documentation/devicetree/bindings/power/supply/battery.txt b/Documentation/devicetree/bindings/power/supply/battery.txt
new file mode 100644
index 00000000..f4d3b4a1
--- /dev/null
+++ b/Documentation/devicetree/bindings/power/supply/battery.txt
@@ -0,0 +1,57 @@
+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.
+
+Upstream dts files should not include battery nodes, unless the battery
+represented cannot easily be replaced in the system by one of a
+different type. This prevents unpredictable, potentially harmful,
+behavior should a replacement that changes the battery type occur
+without a corresponding update to the dtb.
+
+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
+ - precharge-current-microamp: current for pre-charge phase
+ - charge-term-current-microamp: current for charge termination phase
+ - constant-charge-current-max-microamp: maximum constant input current
+ - constant-charge-voltage-max-microvolt: maximum constant input voltage
+
+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
+
+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>;
+ precharge-current-microamp = <256000>;
+ charge-term-current-microamp = <128000>;
+ constant-charge-current-max-microamp = <900000>;
+ constant-charge-voltage-max-microvolt = <4200000>;
+ };
+
+ charger: charger@11 {
+ ....
+ monitored-battery = <&bat>;
+ ...
+ };
+
+ fuel_gauge: fuel-gauge@22 {
+ ....
+ monitored-battery = <&bat>;
+ ...
+ };
--
2.13.0
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH v14 03/11] power: supply: core: Add power_supply_battery_info and API
2017-06-07 18:37 [PATCH v14 00/11] devicetree simple-battery and client in bq27xxx_battery Liam Breck
2017-06-07 18:37 ` [PATCH v14 01/11] devicetree: property-units: Add uWh and uAh units Liam Breck
2017-06-07 18:37 ` [PATCH v14 02/11] dt-bindings: power: supply: Add battery.txt with simple-battery binding Liam Breck
@ 2017-06-07 18:37 ` Liam Breck
2017-06-07 18:37 ` [PATCH v14 04/11] power: supply: core: Add power_supply_prop_precharge Liam Breck
` (8 subsequent siblings)
11 siblings, 0 replies; 25+ messages in thread
From: Liam Breck @ 2017-06-07 18:37 UTC (permalink / raw)
To: Sebastian Reichel, Pali Rohar, linux-pm
Cc: Enric Balletbo, Paul Kocialkowski, Quentin Schulz, Matt Ranostay,
Liam Breck
From: Liam Breck <kernel@networkimprov.net>
power_supply_get_battery_info() reads battery data from devicetree.
struct power_supply_battery_info provides battery data to drivers.
Its fields correspond to elements in enum power_supply_property.
Drivers may surface battery data in sysfs via corresponding
POWER_SUPPLY_PROP_* fields.
Signed-off-by: Matt Ranostay <matt@ranostay.consulting>
Signed-off-by: Liam Breck <kernel@networkimprov.net>
---
Documentation/power/power_supply_class.txt | 12 +++++++
drivers/power/supply/power_supply_core.c | 57 ++++++++++++++++++++++++++++++
include/linux/power_supply.h | 22 ++++++++++++
3 files changed, 91 insertions(+)
diff --git a/Documentation/power/power_supply_class.txt b/Documentation/power/power_supply_class.txt
index 0c72588b..01f00759 100644
--- a/Documentation/power/power_supply_class.txt
+++ b/Documentation/power/power_supply_class.txt
@@ -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?
diff --git a/drivers/power/supply/power_supply_core.c b/drivers/power/supply/power_supply_core.c
index a74d8ca3..494b105b 100644
--- a/drivers/power/supply/power_supply_core.c
+++ b/drivers/power/supply/power_supply_core.c
@@ -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,62 @@ 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;
+ info->precharge_current_ua = -EINVAL;
+ info->charge_term_current_ua = -EINVAL;
+ info->constant_charge_current_max_ua = -EINVAL;
+ info->constant_charge_voltage_max_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
+ * in enum power_supply_property. For reasoning, see
+ * Documentation/power/power_supply_class.txt.
+ */
+
+ 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);
+ of_property_read_u32(battery_np, "precharge-current-microamp",
+ &info->precharge_current_ua);
+ of_property_read_u32(battery_np, "charge-term-current-microamp",
+ &info->charge_term_current_ua);
+ of_property_read_u32(battery_np, "constant_charge_current_max_microamp",
+ &info->constant_charge_current_max_ua);
+ of_property_read_u32(battery_np, "constant_charge_voltage_max_microvolt",
+ &info->constant_charge_voltage_max_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)
diff --git a/include/linux/power_supply.h b/include/linux/power_supply.h
index 39655033..85853834 100644
--- a/include/linux/power_supply.h
+++ b/include/linux/power_supply.h
@@ -288,6 +288,25 @@ 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 */
+ int precharge_current_ua; /* microAmps */
+ int charge_term_current_ua; /* microAmps */
+ int constant_charge_current_max_ua; /* microAmps */
+ int constant_charge_voltage_max_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 +325,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.13.0
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH v14 04/11] power: supply: core: Add power_supply_prop_precharge
2017-06-07 18:37 [PATCH v14 00/11] devicetree simple-battery and client in bq27xxx_battery Liam Breck
` (2 preceding siblings ...)
2017-06-07 18:37 ` [PATCH v14 03/11] power: supply: core: Add power_supply_battery_info and API Liam Breck
@ 2017-06-07 18:37 ` Liam Breck
2017-06-07 18:37 ` [PATCH v14 05/11] dt-bindings: power: supply: bq27xxx: Add monitored-battery documentation Liam Breck
` (7 subsequent siblings)
11 siblings, 0 replies; 25+ messages in thread
From: Liam Breck @ 2017-06-07 18:37 UTC (permalink / raw)
To: Sebastian Reichel, Pali Rohar, linux-pm
Cc: Enric Balletbo, Paul Kocialkowski, Quentin Schulz, Liam Breck
From: Liam Breck <kernel@networkimprov.net>
Battery chargers use POWER_SUPPLY_PROP_PRECHARGE_CURRENT
Clarify related item POWER_SUPPLY_PROP_CHARGE_TERM_CURRENT
Signed-off-by: Liam Breck <kernel@networkimprov.net>
---
Documentation/power/power_supply_class.txt | 19 ++++++++++++-------
drivers/power/supply/power_supply_sysfs.c | 1 +
include/linux/power_supply.h | 3 +++
3 files changed, 16 insertions(+), 7 deletions(-)
diff --git a/Documentation/power/power_supply_class.txt b/Documentation/power/power_supply_class.txt
index 01f00759..300d3789 100644
--- a/Documentation/power/power_supply_class.txt
+++ b/Documentation/power/power_supply_class.txt
@@ -115,28 +115,33 @@ of charge when battery became full/empty". It also could mean "value of
charge when battery considered full/empty at given conditions (temperature,
age)". I.e. these attributes represents real thresholds, not design values.
+ENERGY_FULL, ENERGY_EMPTY - same as above but for energy.
+
CHARGE_COUNTER - the current charge counter (in µAh). This could easily
be negative; there is no empty or full value. It is only useful for
relative, time-based measurements.
+PRECHARGE_CURRENT - the maximum charge current during precharge phase
+of charge cycle (typically 20% of battery capacity).
+CHARGE_TERM_CURRENT - Charge termination current. The charge cycle
+terminates when battery voltage is above recharge threshold, and charge
+current is below this setting (typically 10% of battery capacity).
+
CONSTANT_CHARGE_CURRENT - constant charge current programmed by charger.
CONSTANT_CHARGE_CURRENT_MAX - maximum charge current supported by the
power supply object.
-INPUT_CURRENT_LIMIT - input current limit programmed by charger. Indicates
-the current drawn from a charging source.
-CHARGE_TERM_CURRENT - Charge termination current used to detect the end of charge
-condition.
-
-CALIBRATE - battery or coulomb counter calibration status
CONSTANT_CHARGE_VOLTAGE - constant charge voltage programmed by charger.
CONSTANT_CHARGE_VOLTAGE_MAX - maximum charge voltage supported by the
power supply object.
+INPUT_CURRENT_LIMIT - input current limit programmed by charger. Indicates
+the current drawn from a charging source.
+
CHARGE_CONTROL_LIMIT - current charge control limit setting
CHARGE_CONTROL_LIMIT_MAX - maximum charge control limit setting
-ENERGY_FULL, ENERGY_EMPTY - same as above but for energy.
+CALIBRATE - battery or coulomb counter calibration status
CAPACITY - capacity in percents.
CAPACITY_ALERT_MIN - minimum capacity alert value in percents.
diff --git a/drivers/power/supply/power_supply_sysfs.c b/drivers/power/supply/power_supply_sysfs.c
index bcde8d13..937b03c7 100644
--- a/drivers/power/supply/power_supply_sysfs.c
+++ b/drivers/power/supply/power_supply_sysfs.c
@@ -196,6 +196,7 @@ static struct device_attribute power_supply_attrs[] = {
POWER_SUPPLY_ATTR(time_to_full_avg),
POWER_SUPPLY_ATTR(type),
POWER_SUPPLY_ATTR(scope),
+ POWER_SUPPLY_ATTR(precharge_current),
POWER_SUPPLY_ATTR(charge_term_current),
POWER_SUPPLY_ATTR(calibrate),
/* Properties of type `const char *' */
diff --git a/include/linux/power_supply.h b/include/linux/power_supply.h
index 85853834..31c4961b 100644
--- a/include/linux/power_supply.h
+++ b/include/linux/power_supply.h
@@ -146,6 +146,7 @@ enum power_supply_property {
POWER_SUPPLY_PROP_TIME_TO_FULL_AVG,
POWER_SUPPLY_PROP_TYPE, /* use power_supply.type instead */
POWER_SUPPLY_PROP_SCOPE,
+ POWER_SUPPLY_PROP_PRECHARGE_CURRENT,
POWER_SUPPLY_PROP_CHARGE_TERM_CURRENT,
POWER_SUPPLY_PROP_CALIBRATE,
/* Properties of type `const char *' */
@@ -381,6 +382,8 @@ static inline bool power_supply_is_amp_property(enum power_supply_property psp)
case POWER_SUPPLY_PROP_CHARGE_NOW:
case POWER_SUPPLY_PROP_CHARGE_AVG:
case POWER_SUPPLY_PROP_CHARGE_COUNTER:
+ case POWER_SUPPLY_PROP_PRECHARGE_CURRENT:
+ case POWER_SUPPLY_PROP_CHARGE_TERM_CURRENT:
case POWER_SUPPLY_PROP_CONSTANT_CHARGE_CURRENT:
case POWER_SUPPLY_PROP_CONSTANT_CHARGE_CURRENT_MAX:
case POWER_SUPPLY_PROP_CURRENT_MAX:
--
2.13.0
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH v14 05/11] dt-bindings: power: supply: bq27xxx: Add monitored-battery documentation
2017-06-07 18:37 [PATCH v14 00/11] devicetree simple-battery and client in bq27xxx_battery Liam Breck
` (3 preceding siblings ...)
2017-06-07 18:37 ` [PATCH v14 04/11] power: supply: core: Add power_supply_prop_precharge Liam Breck
@ 2017-06-07 18:37 ` Liam Breck
2017-06-07 18:37 ` [PATCH v14 06/11] power: supply: bq27xxx: Add bulk transfer bus methods Liam Breck
` (6 subsequent siblings)
11 siblings, 0 replies; 25+ messages in thread
From: Liam Breck @ 2017-06-07 18:37 UTC (permalink / raw)
To: Sebastian Reichel, Pali Rohar, linux-pm
Cc: Enric Balletbo, Paul Kocialkowski, Quentin Schulz, Rob Herring,
devicetree, Matt Ranostay, Liam Breck
From: Liam Breck <kernel@networkimprov.net>
Document monitored-battery = <&battery_node>
Cc: Rob Herring <robh@kernel.org>
Cc: devicetree@vger.kernel.org
Signed-off-by: Matt Ranostay <matt@ranostay.consulting>
Signed-off-by: Liam Breck <kernel@networkimprov.net>
Acked-by: Rob Herring <robh@kernel.org>
---
.../devicetree/bindings/power/supply/bq27xxx.txt | 31 +++++++++++++++++-----
1 file changed, 24 insertions(+), 7 deletions(-)
diff --git a/Documentation/devicetree/bindings/power/supply/bq27xxx.txt b/Documentation/devicetree/bindings/power/supply/bq27xxx.txt
index b0c95ef6..6858e1a8 100644
--- a/Documentation/devicetree/bindings/power/supply/bq27xxx.txt
+++ b/Documentation/devicetree/bindings/power/supply/bq27xxx.txt
@@ -1,7 +1,7 @@
-Binding for TI BQ27XXX fuel gauge family
+TI BQ27XXX fuel gauge family
Required properties:
-- compatible: Should contain one of the following:
+- compatible: contains one of the following:
* "ti,bq27200" - BQ27200
* "ti,bq27210" - BQ27210
* "ti,bq27500" - deprecated, use revision specific property below
@@ -26,11 +26,28 @@ Required properties:
* "ti,bq27425" - BQ27425
* "ti,bq27441" - BQ27441
* "ti,bq27621" - BQ27621
-- reg: integer, i2c address of the device.
+- reg: integer, I2C address of the fuel gauge.
+
+Optional properties:
+- monitored-battery: phandle of battery characteristics node
+ The fuel gauge uses the following battery properties:
+ + energy-full-design-microwatt-hours
+ + charge-full-design-microamp-hours
+ + voltage-min-design-microvolt
+ Both or neither of the *-full-design-*-hours properties must be set.
+ See Documentation/devicetree/bindings/power/supply/battery.txt
Example:
-bq27510g3 {
- compatible = "ti,bq27510g3";
- reg = <0x55>;
-};
+ bat: battery {
+ compatible = "simple-battery";
+ voltage-min-design-microvolt = <3200000>;
+ energy-full-design-microwatt-hours = <5290000>;
+ charge-full-design-microamp-hours = <1430000>;
+ };
+
+ bq27510g3: fuel-gauge@55 {
+ compatible = "ti,bq27510g3";
+ reg = <0x55>;
+ monitored-battery = <&bat>;
+ };
--
2.13.0
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH v14 06/11] power: supply: bq27xxx: Add bulk transfer bus methods
2017-06-07 18:37 [PATCH v14 00/11] devicetree simple-battery and client in bq27xxx_battery Liam Breck
` (4 preceding siblings ...)
2017-06-07 18:37 ` [PATCH v14 05/11] dt-bindings: power: supply: bq27xxx: Add monitored-battery documentation Liam Breck
@ 2017-06-07 18:37 ` Liam Breck
2017-06-07 18:37 ` [PATCH v14 07/11] power: supply: bq27xxx: Add chip data memory read/write support Liam Breck
` (5 subsequent siblings)
11 siblings, 0 replies; 25+ messages in thread
From: Liam Breck @ 2017-06-07 18:37 UTC (permalink / raw)
To: Sebastian Reichel, Pali Rohar, linux-pm
Cc: Enric Balletbo, Paul Kocialkowski, Quentin Schulz, Matt Ranostay,
Liam Breck
From: Matt Ranostay <matt@ranostay.consulting>
Declare bus.write/read_bulk/write_bulk().
Add I2C write/read_bulk/write_bulk() to implement the above.
Add bq27xxx_write/read_block/write_block() helpers to call the above.
Signed-off-by: Matt Ranostay <matt@ranostay.consulting>
Signed-off-by: Liam Breck <kernel@networkimprov.net>
Acked-by: "Andrew F. Davis" <afd@ti.com>
---
drivers/power/supply/bq27xxx_battery.c | 67 +++++++++++++++++++++++-
drivers/power/supply/bq27xxx_battery_i2c.c | 82 +++++++++++++++++++++++++++++-
include/linux/power/bq27xxx_battery.h | 3 ++
3 files changed, 149 insertions(+), 3 deletions(-)
diff --git a/drivers/power/supply/bq27xxx_battery.c b/drivers/power/supply/bq27xxx_battery.c
index 398801a2..a11dfad7 100644
--- a/drivers/power/supply/bq27xxx_battery.c
+++ b/drivers/power/supply/bq27xxx_battery.c
@@ -794,11 +794,74 @@ 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;
- 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);
+
+ return ret;
+}
+
+static inline int bq27xxx_write(struct bq27xxx_device_info *di, int reg_index,
+ u16 value, bool single)
+{
+ int ret;
+
+ if (!di || di->regs[reg_index] == INVALID_REG_ADDR)
+ return -EINVAL;
+
+ if (!di->bus.write)
+ return -EPERM;
+
+ ret = di->bus.write(di, di->regs[reg_index], value, single);
+ 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_read_block(struct bq27xxx_device_info *di, int reg_index,
+ u8 *data, int len)
+{
+ int ret;
+
+ if (!di || di->regs[reg_index] == INVALID_REG_ADDR)
+ return -EINVAL;
+
+ if (!di->bus.read_bulk)
+ return -EPERM;
+
+ ret = di->bus.read_bulk(di, di->regs[reg_index], data, len);
+ if (ret < 0)
+ dev_dbg(di->dev, "failed to read_bulk register 0x%02x (index %d)\n",
+ di->regs[reg_index], reg_index);
+
+ return ret;
+}
+
+static inline int bq27xxx_write_block(struct bq27xxx_device_info *di, int reg_index,
+ u8 *data, int len)
+{
+ int ret;
+
+ if (!di || di->regs[reg_index] == INVALID_REG_ADDR)
+ return -EINVAL;
+
+ if (!di->bus.write_bulk)
+ return -EPERM;
+
+ ret = di->bus.write_bulk(di, di->regs[reg_index], data, len);
+ if (ret < 0)
+ dev_dbg(di->dev, "failed to write_bulk register 0x%02x (index %d)\n",
+ di->regs[reg_index], reg_index);
+
+ return ret;
}
/*
diff --git a/drivers/power/supply/bq27xxx_battery_i2c.c b/drivers/power/supply/bq27xxx_battery_i2c.c
index c68fbc3f..a5972214 100644
--- a/drivers/power/supply/bq27xxx_battery_i2c.c
+++ b/drivers/power/supply/bq27xxx_battery_i2c.c
@@ -38,7 +38,7 @@ static int bq27xxx_battery_i2c_read(struct bq27xxx_device_info *di, u8 reg,
{
struct i2c_client *client = to_i2c_client(di->dev);
struct i2c_msg msg[2];
- unsigned char data[2];
+ u8 data[2];
int ret;
if (!client->adapter)
@@ -68,6 +68,82 @@ 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;
+ u8 data[4];
+ int ret;
+
+ if (!client->adapter)
+ return -ENODEV;
+
+ data[0] = reg;
+ if (single) {
+ data[1] = (u8) value;
+ msg.len = 2;
+ } else {
+ put_unaligned_le16(value, &data[1]);
+ msg.len = 3;
+ }
+
+ msg.buf = data;
+ msg.addr = client->addr;
+ msg.flags = 0;
+
+ ret = i2c_transfer(client->adapter, &msg, 1);
+ if (ret < 0)
+ return ret;
+ if (ret != 1)
+ return -EINVAL;
+ return 0;
+}
+
+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);
+ int ret;
+
+ if (!client->adapter)
+ return -ENODEV;
+
+ ret = i2c_smbus_read_i2c_block_data(client, reg, len, data);
+ if (ret < 0)
+ return ret;
+ if (ret != len)
+ return -EINVAL;
+ return 0;
+}
+
+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];
+ int ret;
+
+ 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;
+
+ ret = i2c_transfer(client->adapter, &msg, 1);
+ if (ret < 0)
+ return ret;
+ if (ret != 1)
+ return -EINVAL;
+ return 0;
+}
+
static int bq27xxx_battery_i2c_probe(struct i2c_client *client,
const struct i2c_device_id *id)
{
@@ -95,7 +171,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)
diff --git a/include/linux/power/bq27xxx_battery.h b/include/linux/power/bq27xxx_battery.h
index b312bcef..c3369fa6 100644
--- a/include/linux/power/bq27xxx_battery.h
+++ b/include/linux/power/bq27xxx_battery.h
@@ -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 {
--
2.13.0
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH v14 07/11] power: supply: bq27xxx: Add chip data memory read/write support
2017-06-07 18:37 [PATCH v14 00/11] devicetree simple-battery and client in bq27xxx_battery Liam Breck
` (5 preceding siblings ...)
2017-06-07 18:37 ` [PATCH v14 06/11] power: supply: bq27xxx: Add bulk transfer bus methods Liam Breck
@ 2017-06-07 18:37 ` Liam Breck
2017-06-07 18:37 ` [PATCH v14 08/11] power: supply: bq27xxx: Add power_supply_battery_info support Liam Breck
` (4 subsequent siblings)
11 siblings, 0 replies; 25+ messages in thread
From: Liam Breck @ 2017-06-07 18:37 UTC (permalink / raw)
To: Sebastian Reichel, Pali Rohar, linux-pm
Cc: Enric Balletbo, Paul Kocialkowski, Quentin Schulz, Matt Ranostay,
Liam Breck
From: Liam Breck <kernel@networkimprov.net>
Add these to enable read/write of chip data memory RAM/NVM/flash:
bq27xxx_battery_seal()
bq27xxx_battery_unseal()
bq27xxx_battery_set_cfgupdate()
bq27xxx_battery_soft_reset()
bq27xxx_battery_read_dm_block()
bq27xxx_battery_write_dm_block()
bq27xxx_battery_checksum_dm_block()
Signed-off-by: Matt Ranostay <matt@ranostay.consulting>
Signed-off-by: Liam Breck <kernel@networkimprov.net>
---
drivers/power/supply/bq27xxx_battery.c | 265 +++++++++++++++++++++++++++++++++
include/linux/power/bq27xxx_battery.h | 1 +
2 files changed, 266 insertions(+)
diff --git a/drivers/power/supply/bq27xxx_battery.c b/drivers/power/supply/bq27xxx_battery.c
index a11dfad7..6a4ac14c 100644
--- a/drivers/power/supply/bq27xxx_battery.c
+++ b/drivers/power/supply/bq27xxx_battery.c
@@ -5,6 +5,7 @@
* Copyright (C) 2008 Eurotech S.p.A. <info@eurotech.it>
* Copyright (C) 2010-2011 Lars-Peter Clausen <lars@metafoo.de>
* Copyright (C) 2011 Pali Rohár <pali.rohar@gmail.com>
+ * Copyright (C) 2017 Liam Breck <kernel@networkimprov.net>
*
* Based on a previous work by Copyright (C) 2008 Texas Instruments, Inc.
*
@@ -65,6 +66,7 @@
#define BQ27XXX_FLAG_DSC BIT(0)
#define BQ27XXX_FLAG_SOCF BIT(1) /* State-of-Charge threshold final */
#define BQ27XXX_FLAG_SOC1 BIT(2) /* State-of-Charge threshold 1 */
+#define BQ27XXX_FLAG_CFGUP BIT(4)
#define BQ27XXX_FLAG_FC BIT(9)
#define BQ27XXX_FLAG_OTD BIT(14)
#define BQ27XXX_FLAG_OTC BIT(15)
@@ -78,6 +80,12 @@
#define BQ27000_FLAG_FC BIT(5)
#define BQ27000_FLAG_CHGS BIT(7) /* Charge state flag */
+/* control register params */
+#define BQ27XXX_SEALED 0x20
+#define BQ27XXX_SET_CFGUPDATE 0x13
+#define BQ27XXX_SOFT_RESET 0x42
+#define BQ27XXX_RESET 0x41
+
#define BQ27XXX_RS (20) /* Resistor sense mOhm */
#define BQ27XXX_POWER_CONSTANT (29200) /* 29.2 µV^2 * 1000 */
#define BQ27XXX_CURRENT_CONSTANT (3570) /* 3.57 µV * 1000 */
@@ -108,9 +116,21 @@ enum bq27xxx_reg_index {
BQ27XXX_REG_SOC, /* State-of-Charge */
BQ27XXX_REG_DCAP, /* Design Capacity */
BQ27XXX_REG_AP, /* Average Power */
+ BQ27XXX_DM_CTRL, /* Block Data Control */
+ BQ27XXX_DM_CLASS, /* Data Class */
+ BQ27XXX_DM_BLOCK, /* Data Block */
+ BQ27XXX_DM_DATA, /* Block Data */
+ BQ27XXX_DM_CKSUM, /* Block Data Checksum */
BQ27XXX_REG_MAX, /* sentinel */
};
+#define BQ27XXX_DM_REG_ROWS \
+ [BQ27XXX_DM_CTRL] = 0x61, \
+ [BQ27XXX_DM_CLASS] = 0x3e, \
+ [BQ27XXX_DM_BLOCK] = 0x3f, \
+ [BQ27XXX_DM_DATA] = 0x40, \
+ [BQ27XXX_DM_CKSUM] = 0x60
+
/* Register mappings */
static u8 bq27xxx_regs[][BQ27XXX_REG_MAX] = {
[BQ27000] = {
@@ -131,6 +151,11 @@ static u8 bq27xxx_regs[][BQ27XXX_REG_MAX] = {
[BQ27XXX_REG_SOC] = 0x0b,
[BQ27XXX_REG_DCAP] = 0x76,
[BQ27XXX_REG_AP] = 0x24,
+ [BQ27XXX_DM_CTRL] = INVALID_REG_ADDR,
+ [BQ27XXX_DM_CLASS] = INVALID_REG_ADDR,
+ [BQ27XXX_DM_BLOCK] = INVALID_REG_ADDR,
+ [BQ27XXX_DM_DATA] = INVALID_REG_ADDR,
+ [BQ27XXX_DM_CKSUM] = INVALID_REG_ADDR,
},
[BQ27010] = {
[BQ27XXX_REG_CTRL] = 0x00,
@@ -150,6 +175,11 @@ static u8 bq27xxx_regs[][BQ27XXX_REG_MAX] = {
[BQ27XXX_REG_SOC] = 0x0b,
[BQ27XXX_REG_DCAP] = 0x76,
[BQ27XXX_REG_AP] = INVALID_REG_ADDR,
+ [BQ27XXX_DM_CTRL] = INVALID_REG_ADDR,
+ [BQ27XXX_DM_CLASS] = INVALID_REG_ADDR,
+ [BQ27XXX_DM_BLOCK] = INVALID_REG_ADDR,
+ [BQ27XXX_DM_DATA] = INVALID_REG_ADDR,
+ [BQ27XXX_DM_CKSUM] = INVALID_REG_ADDR,
},
[BQ2750X] = {
[BQ27XXX_REG_CTRL] = 0x00,
@@ -169,6 +199,7 @@ static u8 bq27xxx_regs[][BQ27XXX_REG_MAX] = {
[BQ27XXX_REG_SOC] = 0x2c,
[BQ27XXX_REG_DCAP] = 0x3c,
[BQ27XXX_REG_AP] = INVALID_REG_ADDR,
+ BQ27XXX_DM_REG_ROWS,
},
[BQ2751X] = {
[BQ27XXX_REG_CTRL] = 0x00,
@@ -188,6 +219,7 @@ static u8 bq27xxx_regs[][BQ27XXX_REG_MAX] = {
[BQ27XXX_REG_SOC] = 0x20,
[BQ27XXX_REG_DCAP] = 0x2e,
[BQ27XXX_REG_AP] = INVALID_REG_ADDR,
+ BQ27XXX_DM_REG_ROWS,
},
[BQ27500] = {
[BQ27XXX_REG_CTRL] = 0x00,
@@ -207,6 +239,7 @@ static u8 bq27xxx_regs[][BQ27XXX_REG_MAX] = {
[BQ27XXX_REG_SOC] = 0x2c,
[BQ27XXX_REG_DCAP] = 0x3c,
[BQ27XXX_REG_AP] = 0x24,
+ BQ27XXX_DM_REG_ROWS,
},
[BQ27510G1] = {
[BQ27XXX_REG_CTRL] = 0x00,
@@ -226,6 +259,7 @@ static u8 bq27xxx_regs[][BQ27XXX_REG_MAX] = {
[BQ27XXX_REG_SOC] = 0x2c,
[BQ27XXX_REG_DCAP] = 0x3c,
[BQ27XXX_REG_AP] = 0x24,
+ BQ27XXX_DM_REG_ROWS,
},
[BQ27510G2] = {
[BQ27XXX_REG_CTRL] = 0x00,
@@ -245,6 +279,7 @@ static u8 bq27xxx_regs[][BQ27XXX_REG_MAX] = {
[BQ27XXX_REG_SOC] = 0x2c,
[BQ27XXX_REG_DCAP] = 0x3c,
[BQ27XXX_REG_AP] = 0x24,
+ BQ27XXX_DM_REG_ROWS,
},
[BQ27510G3] = {
[BQ27XXX_REG_CTRL] = 0x00,
@@ -264,6 +299,7 @@ static u8 bq27xxx_regs[][BQ27XXX_REG_MAX] = {
[BQ27XXX_REG_SOC] = 0x20,
[BQ27XXX_REG_DCAP] = 0x2e,
[BQ27XXX_REG_AP] = INVALID_REG_ADDR,
+ BQ27XXX_DM_REG_ROWS,
},
[BQ27520G1] = {
[BQ27XXX_REG_CTRL] = 0x00,
@@ -283,6 +319,7 @@ static u8 bq27xxx_regs[][BQ27XXX_REG_MAX] = {
[BQ27XXX_REG_SOC] = 0x2c,
[BQ27XXX_REG_DCAP] = 0x3c,
[BQ27XXX_REG_AP] = 0x24,
+ BQ27XXX_DM_REG_ROWS,
},
[BQ27520G2] = {
[BQ27XXX_REG_CTRL] = 0x00,
@@ -302,6 +339,7 @@ static u8 bq27xxx_regs[][BQ27XXX_REG_MAX] = {
[BQ27XXX_REG_SOC] = 0x2c,
[BQ27XXX_REG_DCAP] = 0x3c,
[BQ27XXX_REG_AP] = 0x24,
+ BQ27XXX_DM_REG_ROWS,
},
[BQ27520G3] = {
[BQ27XXX_REG_CTRL] = 0x00,
@@ -321,6 +359,7 @@ static u8 bq27xxx_regs[][BQ27XXX_REG_MAX] = {
[BQ27XXX_REG_SOC] = 0x2c,
[BQ27XXX_REG_DCAP] = 0x3c,
[BQ27XXX_REG_AP] = 0x24,
+ BQ27XXX_DM_REG_ROWS,
},
[BQ27520G4] = {
[BQ27XXX_REG_CTRL] = 0x00,
@@ -340,6 +379,7 @@ static u8 bq27xxx_regs[][BQ27XXX_REG_MAX] = {
[BQ27XXX_REG_SOC] = 0x20,
[BQ27XXX_REG_DCAP] = INVALID_REG_ADDR,
[BQ27XXX_REG_AP] = INVALID_REG_ADDR,
+ BQ27XXX_DM_REG_ROWS,
},
[BQ27530] = {
[BQ27XXX_REG_CTRL] = 0x00,
@@ -359,6 +399,7 @@ static u8 bq27xxx_regs[][BQ27XXX_REG_MAX] = {
[BQ27XXX_REG_SOC] = 0x2c,
[BQ27XXX_REG_DCAP] = INVALID_REG_ADDR,
[BQ27XXX_REG_AP] = 0x24,
+ BQ27XXX_DM_REG_ROWS,
},
[BQ27541] = {
[BQ27XXX_REG_CTRL] = 0x00,
@@ -378,6 +419,7 @@ static u8 bq27xxx_regs[][BQ27XXX_REG_MAX] = {
[BQ27XXX_REG_SOC] = 0x2c,
[BQ27XXX_REG_DCAP] = 0x3c,
[BQ27XXX_REG_AP] = 0x24,
+ BQ27XXX_DM_REG_ROWS,
},
[BQ27545] = {
[BQ27XXX_REG_CTRL] = 0x00,
@@ -397,6 +439,7 @@ static u8 bq27xxx_regs[][BQ27XXX_REG_MAX] = {
[BQ27XXX_REG_SOC] = 0x2c,
[BQ27XXX_REG_DCAP] = INVALID_REG_ADDR,
[BQ27XXX_REG_AP] = 0x24,
+ BQ27XXX_DM_REG_ROWS,
},
[BQ27421] = {
[BQ27XXX_REG_CTRL] = 0x00,
@@ -416,6 +459,7 @@ static u8 bq27xxx_regs[][BQ27XXX_REG_MAX] = {
[BQ27XXX_REG_SOC] = 0x1c,
[BQ27XXX_REG_DCAP] = 0x3c,
[BQ27XXX_REG_AP] = 0x18,
+ BQ27XXX_DM_REG_ROWS,
},
};
@@ -757,6 +801,28 @@ static struct {
static DEFINE_MUTEX(bq27xxx_list_lock);
static LIST_HEAD(bq27xxx_battery_devices);
+#define BQ27XXX_MSLEEP(i) usleep_range((i)*1000, (i)*1000+500)
+
+#define BQ27XXX_DM_SZ 32
+
+/**
+ * struct bq27xxx_dm_buf - chip data memory buffer
+ * @class: data memory subclass_id
+ * @block: data memory block number
+ * @data: data from/for the block
+ * @has_data: true if data has been filled by read
+ * @dirty: true if data has changed since last read/write
+ *
+ * Encapsulates info required to manage chip data memory blocks.
+ */
+struct bq27xxx_dm_buf {
+ u8 class;
+ u8 block;
+ u8 data[BQ27XXX_DM_SZ];
+ bool has_data, dirty;
+};
+
+
static int poll_interval_param_set(const char *val, const struct kernel_param *kp)
{
struct bq27xxx_device_info *di;
@@ -864,6 +930,205 @@ static inline int bq27xxx_write_block(struct bq27xxx_device_info *di, int reg_in
return ret;
}
+static int bq27xxx_battery_seal(struct bq27xxx_device_info *di)
+{
+ int ret;
+
+ ret = bq27xxx_write(di, BQ27XXX_REG_CTRL, BQ27XXX_SEALED, false);
+ if (ret < 0) {
+ dev_err(di->dev, "bus error on seal: %d\n", ret);
+ return ret;
+ }
+
+ return 0;
+}
+
+static int bq27xxx_battery_unseal(struct bq27xxx_device_info *di)
+{
+ int ret;
+
+ if (di->unseal_key == 0) {
+ dev_err(di->dev, "unseal failed due to missing key\n");
+ return -EINVAL;
+ }
+
+ ret = bq27xxx_write(di, BQ27XXX_REG_CTRL, (u16)(di->unseal_key >> 16), false);
+ if (ret < 0)
+ goto out;
+
+ ret = bq27xxx_write(di, BQ27XXX_REG_CTRL, (u16)di->unseal_key, false);
+ if (ret < 0)
+ goto out;
+
+ return 0;
+
+out:
+ dev_err(di->dev, "bus error on unseal: %d\n", ret);
+ return ret;
+}
+
+static u8 bq27xxx_battery_checksum_dm_block(struct bq27xxx_dm_buf *buf)
+{
+ u16 sum = 0;
+ int i;
+
+ for (i = 0; i < BQ27XXX_DM_SZ; i++)
+ sum += buf->data[i];
+ sum &= 0xff;
+
+ return 0xff - sum;
+}
+
+static int bq27xxx_battery_read_dm_block(struct bq27xxx_device_info *di,
+ struct bq27xxx_dm_buf *buf)
+{
+ int ret;
+
+ buf->has_data = false;
+
+ ret = bq27xxx_write(di, BQ27XXX_DM_CLASS, buf->class, true);
+ if (ret < 0)
+ goto out;
+
+ ret = bq27xxx_write(di, BQ27XXX_DM_BLOCK, buf->block, true);
+ if (ret < 0)
+ goto out;
+
+ BQ27XXX_MSLEEP(1);
+
+ ret = bq27xxx_read_block(di, BQ27XXX_DM_DATA, buf->data, BQ27XXX_DM_SZ);
+ if (ret < 0)
+ goto out;
+
+ ret = bq27xxx_read(di, BQ27XXX_DM_CKSUM, true);
+ if (ret < 0)
+ goto out;
+
+ if ((u8)ret != bq27xxx_battery_checksum_dm_block(buf)) {
+ ret = -EINVAL;
+ goto out;
+ }
+
+ buf->has_data = true;
+ buf->dirty = false;
+
+ return 0;
+
+out:
+ dev_err(di->dev, "bus error reading chip memory: %d\n", ret);
+ return ret;
+}
+
+static int bq27xxx_battery_cfgupdate_priv(struct bq27xxx_device_info *di, bool active)
+{
+ const int limit = 100;
+ u16 cmd = active ? BQ27XXX_SET_CFGUPDATE : BQ27XXX_SOFT_RESET;
+ int ret, try = limit;
+
+ ret = bq27xxx_write(di, BQ27XXX_REG_CTRL, cmd, false);
+ if (ret < 0)
+ return ret;
+
+ do {
+ BQ27XXX_MSLEEP(25);
+ ret = bq27xxx_read(di, BQ27XXX_REG_FLAGS, false);
+ if (ret < 0)
+ return ret;
+ } while (!!(ret & BQ27XXX_FLAG_CFGUP) != active && --try);
+
+ if (!try) {
+ dev_err(di->dev, "timed out waiting for cfgupdate flag %d\n", active);
+ return -EINVAL;
+ }
+
+ if (limit - try > 3)
+ dev_warn(di->dev, "cfgupdate %d, retries %d\n", active, limit - try);
+
+ return 0;
+}
+
+static inline int bq27xxx_battery_set_cfgupdate(struct bq27xxx_device_info *di)
+{
+ int ret = bq27xxx_battery_cfgupdate_priv(di, true);
+ if (ret < 0 && ret != -EINVAL)
+ dev_err(di->dev, "bus error on set_cfgupdate: %d\n", ret);
+
+ return ret;
+}
+
+static inline int bq27xxx_battery_soft_reset(struct bq27xxx_device_info *di)
+{
+ int ret = bq27xxx_battery_cfgupdate_priv(di, false);
+ if (ret < 0 && ret != -EINVAL)
+ dev_err(di->dev, "bus error on soft_reset: %d\n", ret);
+
+ return ret;
+}
+
+static int bq27xxx_battery_write_dm_block(struct bq27xxx_device_info *di,
+ struct bq27xxx_dm_buf *buf)
+{
+ bool cfgup = di->chip == BQ27421; /* assume related chips need cfgupdate */
+ int ret;
+
+ if (!buf->dirty)
+ return 0;
+
+ if (cfgup) {
+ ret = bq27xxx_battery_set_cfgupdate(di);
+ if (ret < 0)
+ return ret;
+ }
+
+ ret = bq27xxx_write(di, BQ27XXX_DM_CTRL, 0, true);
+ if (ret < 0)
+ goto out;
+
+ ret = bq27xxx_write(di, BQ27XXX_DM_CLASS, buf->class, true);
+ if (ret < 0)
+ goto out;
+
+ ret = bq27xxx_write(di, BQ27XXX_DM_BLOCK, buf->block, true);
+ if (ret < 0)
+ goto out;
+
+ BQ27XXX_MSLEEP(1);
+
+ ret = bq27xxx_write_block(di, BQ27XXX_DM_DATA, buf->data, BQ27XXX_DM_SZ);
+ if (ret < 0)
+ goto out;
+
+ ret = bq27xxx_write(di, BQ27XXX_DM_CKSUM,
+ bq27xxx_battery_checksum_dm_block(buf), true);
+ if (ret < 0)
+ goto out;
+
+ /* DO NOT read BQ27XXX_DM_CKSUM here to verify it! That may cause NVM
+ * corruption on the '425 chip (and perhaps others), which can damage
+ * the chip.
+ */
+
+ if (cfgup) {
+ BQ27XXX_MSLEEP(1);
+ ret = bq27xxx_battery_soft_reset(di);
+ if (ret < 0)
+ return ret;
+ } else {
+ BQ27XXX_MSLEEP(100); /* flash DM updates in <100ms */
+ }
+
+ buf->dirty = false;
+
+ return 0;
+
+out:
+ if (cfgup)
+ bq27xxx_battery_soft_reset(di);
+
+ dev_err(di->dev, "bus error writing chip memory: %d\n", ret);
+ return ret;
+}
+
/*
* Return the battery State-of-Charge
* Or < 0 if something fails.
diff --git a/include/linux/power/bq27xxx_battery.h b/include/linux/power/bq27xxx_battery.h
index c3369fa6..b1defb86 100644
--- a/include/linux/power/bq27xxx_battery.h
+++ b/include/linux/power/bq27xxx_battery.h
@@ -64,6 +64,7 @@ struct bq27xxx_device_info {
int id;
enum bq27xxx_chip chip;
const char *name;
+ u32 unseal_key;
struct bq27xxx_access_methods bus;
struct bq27xxx_reg_cache cache;
int charge_design_full;
--
2.13.0
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH v14 08/11] power: supply: bq27xxx: Add power_supply_battery_info support
2017-06-07 18:37 [PATCH v14 00/11] devicetree simple-battery and client in bq27xxx_battery Liam Breck
` (6 preceding siblings ...)
2017-06-07 18:37 ` [PATCH v14 07/11] power: supply: bq27xxx: Add chip data memory read/write support Liam Breck
@ 2017-06-07 18:37 ` Liam Breck
2017-06-07 18:37 ` [PATCH v14 09/11] power: supply: bq27xxx: Enable data memory update for certain chips Liam Breck
` (3 subsequent siblings)
11 siblings, 0 replies; 25+ messages in thread
From: Liam Breck @ 2017-06-07 18:37 UTC (permalink / raw)
To: Sebastian Reichel, Pali Rohar, linux-pm
Cc: Enric Balletbo, Paul Kocialkowski, Quentin Schulz, Matt Ranostay,
Liam Breck
From: Liam Breck <kernel@networkimprov.net>
Previously there was no way to configure these chips in the event that the
defaults didn't match the battery in question.
For chips with RAM data memory (and also those with flash/NVM data memory
if CONFIG_BATTERY_BQ27XXX_DT_UPDATES_NVM is defined and the user has not
set module param dt_monitored_battery_updates_nvm=0) we now call
power_supply_get_battery_info(), check its values, and write battery
properties to chip data memory if there is a dm_regs table for the chip.
Signed-off-by: Matt Ranostay <matt@ranostay.consulting>
Signed-off-by: Liam Breck <kernel@networkimprov.net>
---
drivers/power/supply/Kconfig | 11 ++
drivers/power/supply/bq27xxx_battery.c | 204 ++++++++++++++++++++++++++++++++-
include/linux/power/bq27xxx_battery.h | 2 +
3 files changed, 216 insertions(+), 1 deletion(-)
diff --git a/drivers/power/supply/Kconfig b/drivers/power/supply/Kconfig
index 76806a0b..38fae108 100644
--- a/drivers/power/supply/Kconfig
+++ b/drivers/power/supply/Kconfig
@@ -178,6 +178,17 @@ config BATTERY_BQ27XXX_I2C
Say Y here to enable support for batteries with BQ27xxx chips
connected over an I2C bus.
+config BATTERY_BQ27XXX_DT_UPDATES_NVM
+ bool "BQ27xxx support for update of NVM/flash data memory"
+ depends on BATTERY_BQ27XXX_I2C
+ help
+ Say Y here to enable devicetree monitored-battery config to update
+ NVM/flash data memory. Only enable this option for devices with a
+ fuel gauge mounted on the circuit board, and a battery that cannot
+ easily be replaced with one of a different type. Not for
+ general-purpose kernels, as this can cause misconfiguration of a
+ smart battery with embedded NVM/flash.
+
config BATTERY_DA9030
tristate "DA9030 battery driver"
depends on PMIC_DA903X
diff --git a/drivers/power/supply/bq27xxx_battery.c b/drivers/power/supply/bq27xxx_battery.c
index 6a4ac14c..ed44439d 100644
--- a/drivers/power/supply/bq27xxx_battery.c
+++ b/drivers/power/supply/bq27xxx_battery.c
@@ -805,6 +805,13 @@ static LIST_HEAD(bq27xxx_battery_devices);
#define BQ27XXX_DM_SZ 32
+struct bq27xxx_dm_reg {
+ u8 subclass_id;
+ u8 offset;
+ u8 bytes;
+ u16 min, max;
+};
+
/**
* struct bq27xxx_dm_buf - chip data memory buffer
* @class: data memory subclass_id
@@ -822,6 +829,44 @@ struct bq27xxx_dm_buf {
bool has_data, dirty;
};
+#define BQ27XXX_DM_BUF(di, i) { \
+ .class = (di)->dm_regs[i].subclass_id, \
+ .block = (di)->dm_regs[i].offset / BQ27XXX_DM_SZ, \
+}
+
+static inline u16 *bq27xxx_dm_reg_ptr(struct bq27xxx_dm_buf *buf,
+ struct bq27xxx_dm_reg *reg)
+{
+ if (buf->class == reg->subclass_id &&
+ buf->block == reg->offset / BQ27XXX_DM_SZ)
+ return (u16 *) (buf->data + reg->offset % BQ27XXX_DM_SZ);
+
+ return NULL;
+}
+
+enum bq27xxx_dm_reg_id {
+ BQ27XXX_DM_DESIGN_CAPACITY = 0,
+ BQ27XXX_DM_DESIGN_ENERGY,
+ BQ27XXX_DM_TERMINATE_VOLTAGE,
+};
+
+static const char * const bq27xxx_dm_reg_name[] = {
+ [BQ27XXX_DM_DESIGN_CAPACITY] = "design-capacity",
+ [BQ27XXX_DM_DESIGN_ENERGY] = "design-energy",
+ [BQ27XXX_DM_TERMINATE_VOLTAGE] = "terminate-voltage",
+};
+
+
+static bool bq27xxx_dt_to_nvm = true;
+module_param_named(dt_monitored_battery_updates_nvm, bq27xxx_dt_to_nvm, bool, 0444);
+MODULE_PARM_DESC(dt_monitored_battery_updates_nvm,
+ "Devicetree monitored-battery config updates data memory on NVM/flash chips.\n"
+ "Users must set this =0 when installing a different type of battery!\n"
+ "Default is =1."
+#ifndef CONFIG_BATTERY_BQ27XXX_DT_UPDATES_NVM
+ "\nSetting this affects future kernel updates, not the current configuration."
+#endif
+);
static int poll_interval_param_set(const char *val, const struct kernel_param *kp)
{
@@ -1019,6 +1064,55 @@ static int bq27xxx_battery_read_dm_block(struct bq27xxx_device_info *di,
return ret;
}
+static void 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 = &di->dm_regs[reg_id];
+ const char *str = bq27xxx_dm_reg_name[reg_id];
+ u16 *prev = bq27xxx_dm_reg_ptr(buf, reg);
+
+ if (prev == NULL) {
+ dev_warn(di->dev, "buffer does not match %s dm spec\n", str);
+ return;
+ }
+
+ if (reg->bytes != 2) {
+ dev_warn(di->dev, "%s dm spec has unsupported byte size\n", str);
+ return;
+ }
+
+ if (!buf->has_data)
+ return;
+
+ if (be16_to_cpup(prev) == val) {
+ dev_info(di->dev, "%s has %u\n", str, val);
+ return;
+ }
+
+#ifdef CONFIG_BATTERY_BQ27XXX_DT_UPDATES_NVM
+ if (!di->ram_chip && !bq27xxx_dt_to_nvm) {
+#else
+ if (!di->ram_chip) {
+#endif
+ /* devicetree and NVM differ; defer to NVM */
+ dev_warn(di->dev, "%s has %u; update to %u disallowed "
+#ifdef CONFIG_BATTERY_BQ27XXX_DT_UPDATES_NVM
+ "by dt_monitored_battery_updates_nvm=0"
+#else
+ "for flash/NVM data memory"
+#endif
+ "\n", str, be16_to_cpup(prev), val);
+ return;
+ }
+
+ dev_info(di->dev, "update %s to %u\n", str, val);
+
+ *prev = cpu_to_be16(val);
+ buf->dirty = true;
+}
+
static int bq27xxx_battery_cfgupdate_priv(struct bq27xxx_device_info *di, bool active)
{
const int limit = 100;
@@ -1129,6 +1223,103 @@ static int bq27xxx_battery_write_dm_block(struct bq27xxx_device_info *di,
return ret;
}
+static void bq27xxx_battery_set_config(struct bq27xxx_device_info *di,
+ struct power_supply_battery_info *info)
+{
+ struct bq27xxx_dm_buf bd = BQ27XXX_DM_BUF(di, BQ27XXX_DM_DESIGN_CAPACITY);
+ struct bq27xxx_dm_buf bt = BQ27XXX_DM_BUF(di, BQ27XXX_DM_TERMINATE_VOLTAGE);
+ bool updated;
+
+ if (bq27xxx_battery_unseal(di) < 0)
+ return;
+
+ if (info->charge_full_design_uah != -EINVAL &&
+ info->energy_full_design_uwh != -EINVAL) {
+ bq27xxx_battery_read_dm_block(di, &bd);
+ /* assume design energy & capacity are in same block */
+ bq27xxx_battery_update_dm_block(di, &bd,
+ BQ27XXX_DM_DESIGN_CAPACITY,
+ info->charge_full_design_uah / 1000);
+ bq27xxx_battery_update_dm_block(di, &bd,
+ BQ27XXX_DM_DESIGN_ENERGY,
+ info->energy_full_design_uwh / 1000);
+ }
+
+ if (info->voltage_min_design_uv != -EINVAL) {
+ bool same = bd.class == bt.class && bd.block == bt.block;
+ if (!same)
+ bq27xxx_battery_read_dm_block(di, &bt);
+ bq27xxx_battery_update_dm_block(di, same ? &bd : &bt,
+ BQ27XXX_DM_TERMINATE_VOLTAGE,
+ info->voltage_min_design_uv / 1000);
+ }
+
+ updated = bd.dirty || bt.dirty;
+
+ bq27xxx_battery_write_dm_block(di, &bd);
+ bq27xxx_battery_write_dm_block(di, &bt);
+
+ bq27xxx_battery_seal(di);
+
+ if (updated && di->chip != BQ27421) { /* not a cfgupdate chip, so reset */
+ bq27xxx_write(di, BQ27XXX_REG_CTRL, BQ27XXX_RESET, false);
+ BQ27XXX_MSLEEP(300); /* reset time is not documented */
+ }
+ /* assume bq27xxx_battery_update() is called hereafter */
+}
+
+static void bq27xxx_battery_settings(struct bq27xxx_device_info *di)
+{
+ struct power_supply_battery_info info = {};
+ unsigned int min, max;
+
+ if (power_supply_get_battery_info(di->bat, &info) < 0)
+ return;
+
+ if (!di->dm_regs) {
+ dev_warn(di->dev, "data memory update not supported for chip\n");
+ return;
+ }
+
+ 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");
+ }
+
+ /* assume min == 0 */
+ max = di->dm_regs[BQ27XXX_DM_DESIGN_ENERGY].max;
+ if (info.energy_full_design_uwh > max * 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;
+ }
+
+ /* assume min == 0 */
+ max = di->dm_regs[BQ27XXX_DM_DESIGN_CAPACITY].max;
+ if (info.charge_full_design_uah > max * 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;
+ }
+
+ min = di->dm_regs[BQ27XXX_DM_TERMINATE_VOLTAGE].min;
+ max = di->dm_regs[BQ27XXX_DM_TERMINATE_VOLTAGE].max;
+ if ((info.voltage_min_design_uv < min * 1000 ||
+ info.voltage_min_design_uv > 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)
+ bq27xxx_battery_set_config(di, &info);
+}
+
/*
* Return the battery State-of-Charge
* Or < 0 if something fails.
@@ -1646,6 +1837,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;
@@ -1679,7 +1877,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);
@@ -1704,6 +1905,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);
diff --git a/include/linux/power/bq27xxx_battery.h b/include/linux/power/bq27xxx_battery.h
index b1defb86..11e11685 100644
--- a/include/linux/power/bq27xxx_battery.h
+++ b/include/linux/power/bq27xxx_battery.h
@@ -63,7 +63,9 @@ struct bq27xxx_device_info {
struct device *dev;
int id;
enum bq27xxx_chip chip;
+ bool ram_chip;
const char *name;
+ struct bq27xxx_dm_reg *dm_regs;
u32 unseal_key;
struct bq27xxx_access_methods bus;
struct bq27xxx_reg_cache cache;
--
2.13.0
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH v14 09/11] power: supply: bq27xxx: Enable data memory update for certain chips
2017-06-07 18:37 [PATCH v14 00/11] devicetree simple-battery and client in bq27xxx_battery Liam Breck
` (7 preceding siblings ...)
2017-06-07 18:37 ` [PATCH v14 08/11] power: supply: bq27xxx: Add power_supply_battery_info support Liam Breck
@ 2017-06-07 18:37 ` Liam Breck
2017-06-15 16:02 ` Sebastian Reichel
2017-06-07 18:37 ` [PATCH v14 10/11] power: supply: bq27xxx: Flag identical register maps when in debug mode Liam Breck
` (2 subsequent siblings)
11 siblings, 1 reply; 25+ messages in thread
From: Liam Breck @ 2017-06-07 18:37 UTC (permalink / raw)
To: Sebastian Reichel, Pali Rohar, linux-pm
Cc: Enric Balletbo, Paul Kocialkowski, Quentin Schulz, Liam Breck
From: Liam Breck <kernel@networkimprov.net>
Support data memory update on BQ27500, 545, 425, 421, 441, 621.
With exception of BQ27425, these are only enabled #ifdef DEBUG,
as they are not tested.
Create IDs for for previously unID'd chips, to index arrays for unseal keys
and data memory register tables.
Signed-off-by: Liam Breck <kernel@networkimprov.net>
---
drivers/power/supply/bq27xxx_battery.c | 75 ++++++++++++++++++++++++++++--
drivers/power/supply/bq27xxx_battery_i2c.c | 52 +++++++++++----------
include/linux/power/bq27xxx_battery.h | 14 ++++++
3 files changed, 111 insertions(+), 30 deletions(-)
diff --git a/drivers/power/supply/bq27xxx_battery.c b/drivers/power/supply/bq27xxx_battery.c
index ed44439d..a7014525 100644
--- a/drivers/power/supply/bq27xxx_battery.c
+++ b/drivers/power/supply/bq27xxx_battery.c
@@ -58,7 +58,7 @@
#include <linux/power/bq27xxx_battery.h>
-#define DRIVER_VERSION "1.2.0"
+#define DRIVER_VERSION "1.3.0"
#define BQ27XXX_MANUFACTURER "Texas Instruments"
@@ -132,7 +132,7 @@ enum bq27xxx_reg_index {
[BQ27XXX_DM_CKSUM] = 0x60
/* Register mappings */
-static u8 bq27xxx_regs[][BQ27XXX_REG_MAX] = {
+static u8 bq27xxx_regs[BQ27MAX][BQ27XXX_REG_MAX] = {
[BQ27000] = {
[BQ27XXX_REG_CTRL] = 0x00,
[BQ27XXX_REG_TEMP] = 0x06,
@@ -779,7 +779,7 @@ static enum power_supply_property bq27421_battery_props[] = {
static struct {
enum power_supply_property *props;
size_t size;
-} bq27xxx_battery_props[] = {
+} bq27xxx_battery_props[BQ27MAX] = {
BQ27XXX_PROP(BQ27000, bq27000_battery_props),
BQ27XXX_PROP(BQ27010, bq27010_battery_props),
BQ27XXX_PROP(BQ2750X, bq2750x_battery_props),
@@ -856,6 +856,63 @@ static const char * const bq27xxx_dm_reg_name[] = {
[BQ27XXX_DM_TERMINATE_VOLTAGE] = "terminate-voltage",
};
+static struct bq27xxx_dm_reg bq27425_dm_regs[] = {
+ [BQ27XXX_DM_DESIGN_CAPACITY] = { 82, 12, 2, 0, 32767 },
+ [BQ27XXX_DM_DESIGN_ENERGY] = { 82, 14, 2, 0, 32767 },
+ [BQ27XXX_DM_TERMINATE_VOLTAGE] = { 82, 18, 2, 2800, 3700 },
+};
+
+#ifdef DEBUG
+/* these have not been tested */
+
+static struct bq27xxx_dm_reg bq27500_dm_regs[] = {
+ [BQ27XXX_DM_DESIGN_CAPACITY] = { 48, 10, 2, 0, 65535 },
+ [BQ27XXX_DM_DESIGN_ENERGY] = { }, /* missing on chip */
+ [BQ27XXX_DM_TERMINATE_VOLTAGE] = { 80, 48, 2, 1000, 32767 },
+};
+
+static struct bq27xxx_dm_reg bq27545_dm_regs[] = {
+ [BQ27XXX_DM_DESIGN_CAPACITY] = { 48, 23, 2, 0, 32767 },
+ [BQ27XXX_DM_DESIGN_ENERGY] = { 48, 25, 2, 0, 32767 },
+ [BQ27XXX_DM_TERMINATE_VOLTAGE] = { 80, 67, 2, 2800, 3700 },
+};
+
+static struct bq27xxx_dm_reg bq27421_dm_regs[] = {
+ [BQ27XXX_DM_DESIGN_CAPACITY] = { 82, 10, 2, 0, 8000 },
+ [BQ27XXX_DM_DESIGN_ENERGY] = { 82, 12, 2, 0, 32767 },
+ [BQ27XXX_DM_TERMINATE_VOLTAGE] = { 82, 16, 2, 2500, 3700 },
+};
+
+static struct bq27xxx_dm_reg bq27621_dm_regs[] = {
+ [BQ27XXX_DM_DESIGN_CAPACITY] = { 82, 3, 2, 0, 8000 },
+ [BQ27XXX_DM_DESIGN_ENERGY] = { 82, 5, 2, 0, 32767 },
+ [BQ27XXX_DM_TERMINATE_VOLTAGE] = { 82, 9, 2, 2500, 3700 },
+};
+
+#endif
+
+static struct bq27xxx_dm_reg *bq27xxx_dm_regs[] = {
+ [BQ27425] = bq27425_dm_regs,
+#ifdef DEBUG
+ [BQ27500] = bq27500_dm_regs,
+ [BQ27545] = bq27545_dm_regs,
+ [BQ27421] = bq27421_dm_regs,
+ [BQ27441] = bq27421_dm_regs,
+ [BQ27621] = bq27621_dm_regs,
+#else
+ [BQ27621] = 0 /* must have highest ID */
+#endif
+};
+
+static u32 bq27xxx_unseal_keys[] = {
+ [BQ27500] = 0x04143672,
+ [BQ27545] = 0x04143672,
+ [BQ27421] = 0x80008000,
+ [BQ27425] = 0x04143672,
+ [BQ27441] = 0x80008000,
+ [BQ27621] = 0x80008000,
+};
+
static bool bq27xxx_dt_to_nvm = true;
module_param_named(dt_monitored_battery_updates_nvm, bq27xxx_dt_to_nvm, bool, 0444);
@@ -1882,9 +1939,17 @@ int bq27xxx_battery_setup(struct bq27xxx_device_info *di)
.drv_data = di,
};
+ di->ram_chip = di->real_chip == BQ27421 ||
+ di->real_chip == BQ27441 ||
+ di->real_chip == BQ27621;
+
+ di->unseal_key = bq27xxx_unseal_keys[di->real_chip];
+ di->dm_regs = bq27xxx_dm_regs[di->real_chip];
+
+ di->regs = bq27xxx_regs[di->chip];
+
INIT_DELAYED_WORK(&di->work, bq27xxx_battery_poll);
mutex_init(&di->lock);
- di->regs = bq27xxx_regs[di->chip];
psy_desc = devm_kzalloc(di->dev, sizeof(*psy_desc), GFP_KERNEL);
if (!psy_desc)
@@ -1999,7 +2064,7 @@ static int bq27xxx_battery_platform_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, di);
di->dev = &pdev->dev;
- di->chip = pdata->chip;
+ di->chip = di->real_chip = pdata->chip;
di->name = pdata->name ?: dev_name(&pdev->dev);
di->bus.read = bq27xxx_battery_platform_read;
diff --git a/drivers/power/supply/bq27xxx_battery_i2c.c b/drivers/power/supply/bq27xxx_battery_i2c.c
index a5972214..a3656508 100644
--- a/drivers/power/supply/bq27xxx_battery_i2c.c
+++ b/drivers/power/supply/bq27xxx_battery_i2c.c
@@ -169,7 +169,8 @@ static int bq27xxx_battery_i2c_probe(struct i2c_client *client,
di->id = num;
di->dev = &client->dev;
- di->chip = id->driver_data;
+ di->real_chip = id->driver_data >> 16;
+ di->chip = (u16) id->driver_data;
di->name = name;
di->bus.read = bq27xxx_battery_i2c_read;
@@ -226,30 +227,31 @@ static int bq27xxx_battery_i2c_remove(struct i2c_client *client)
}
static const struct i2c_device_id bq27xxx_i2c_id_table[] = {
- { "bq27200", BQ27000 },
- { "bq27210", BQ27010 },
- { "bq27500", BQ2750X },
- { "bq27510", BQ2751X },
- { "bq27520", BQ2751X },
- { "bq27500-1", BQ27500 },
- { "bq27510g1", BQ27510G1 },
- { "bq27510g2", BQ27510G2 },
- { "bq27510g3", BQ27510G3 },
- { "bq27520g1", BQ27520G1 },
- { "bq27520g2", BQ27520G2 },
- { "bq27520g3", BQ27520G3 },
- { "bq27520g4", BQ27520G4 },
- { "bq27530", BQ27530 },
- { "bq27531", BQ27530 },
- { "bq27541", BQ27541 },
- { "bq27542", BQ27541 },
- { "bq27546", BQ27541 },
- { "bq27742", BQ27541 },
- { "bq27545", BQ27545 },
- { "bq27421", BQ27421 },
- { "bq27425", BQ27421 },
- { "bq27441", BQ27421 },
- { "bq27621", BQ27421 },
+ /* dest. di->real_chip di->chip */
+ { "bq27200", (BQ27000 << 16) | BQ27000 },
+ { "bq27210", (BQ27010 << 16) | BQ27010 },
+ { "bq27500", (BQ2750X << 16) | BQ2750X },
+ { "bq27510", (BQ2751X << 16) | BQ2751X },
+ { "bq27520", (BQ2752X << 16) | BQ2751X },
+ { "bq27500-1", (BQ27500 << 16) | BQ27500 },
+ { "bq27510g1", (BQ27510G1 << 16) | BQ27510G1 },
+ { "bq27510g2", (BQ27510G2 << 16) | BQ27510G2 },
+ { "bq27510g3", (BQ27510G3 << 16) | BQ27510G3 },
+ { "bq27520g1", (BQ27520G1 << 16) | BQ27520G1 },
+ { "bq27520g2", (BQ27520G2 << 16) | BQ27520G2 },
+ { "bq27520g3", (BQ27520G3 << 16) | BQ27520G3 },
+ { "bq27520g4", (BQ27520G4 << 16) | BQ27520G4 },
+ { "bq27530", (BQ27530 << 16) | BQ27530 },
+ { "bq27531", (BQ27531 << 16) | BQ27530 },
+ { "bq27541", (BQ27541 << 16) | BQ27541 },
+ { "bq27542", (BQ27542 << 16) | BQ27541 },
+ { "bq27546", (BQ27546 << 16) | BQ27541 },
+ { "bq27742", (BQ27742 << 16) | BQ27541 },
+ { "bq27545", (BQ27545 << 16) | BQ27545 },
+ { "bq27421", (BQ27421 << 16) | BQ27421 },
+ { "bq27425", (BQ27425 << 16) | BQ27421 },
+ { "bq27441", (BQ27441 << 16) | BQ27421 },
+ { "bq27621", (BQ27621 << 16) | BQ27421 },
{},
};
MODULE_DEVICE_TABLE(i2c, bq27xxx_i2c_id_table);
diff --git a/include/linux/power/bq27xxx_battery.h b/include/linux/power/bq27xxx_battery.h
index 11e11685..bec13b7a 100644
--- a/include/linux/power/bq27xxx_battery.h
+++ b/include/linux/power/bq27xxx_battery.h
@@ -2,6 +2,9 @@
#define __LINUX_BQ27X00_BATTERY_H__
enum bq27xxx_chip {
+ /* all index bq27xxx_unseal_keys[] & bq27xxx_dm_regs[] */
+
+ /* these index bq27xxx_regs[] & bq27xxx_battery_props[] */
BQ27000 = 1, /* bq27000, bq27200 */
BQ27010, /* bq27010, bq27210 */
BQ2750X, /* bq27500 deprecated alias */
@@ -18,6 +21,16 @@ enum bq27xxx_chip {
BQ27541, /* bq27541, bq27542, bq27546, bq27742 */
BQ27545, /* bq27545 */
BQ27421, /* bq27421, bq27425, bq27441, bq27621 */
+ BQ27MAX,
+
+ BQ2752X, /* deprecated alias */
+ BQ27531,
+ BQ27542,
+ BQ27546,
+ BQ27742,
+ BQ27425,
+ BQ27441,
+ BQ27621,
};
/**
@@ -62,6 +75,7 @@ struct bq27xxx_reg_cache {
struct bq27xxx_device_info {
struct device *dev;
int id;
+ enum bq27xxx_chip real_chip;
enum bq27xxx_chip chip;
bool ram_chip;
const char *name;
--
2.13.0
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH v14 10/11] power: supply: bq27xxx: Flag identical register maps when in debug mode
2017-06-07 18:37 [PATCH v14 00/11] devicetree simple-battery and client in bq27xxx_battery Liam Breck
` (8 preceding siblings ...)
2017-06-07 18:37 ` [PATCH v14 09/11] power: supply: bq27xxx: Enable data memory update for certain chips Liam Breck
@ 2017-06-07 18:37 ` Liam Breck
2017-06-15 16:06 ` Sebastian Reichel
2017-06-07 18:37 ` [PATCH v14 11/11] power: supply: bq27xxx: Remove duplicate register arrays Liam Breck
2017-06-08 16:42 ` [PATCH v14 00/11] devicetree simple-battery and client in bq27xxx_battery Sebastian Reichel
11 siblings, 1 reply; 25+ messages in thread
From: Liam Breck @ 2017-06-07 18:37 UTC (permalink / raw)
To: Sebastian Reichel, Pali Rohar, linux-pm
Cc: Enric Balletbo, Paul Kocialkowski, Quentin Schulz, Liam Breck
From: Liam Breck <kernel@networkimprov.net>
We tie multiple chips to unique register maps. When supporting a new chip,
it's easy to add a duplicate map by mistake.
In debug mode we now scan the register maps for duplicates.
Signed-off-by: Liam Breck <kernel@networkimprov.net>
---
drivers/power/supply/bq27xxx_battery.c | 21 +++++++++++++++++++++
1 file changed, 21 insertions(+)
diff --git a/drivers/power/supply/bq27xxx_battery.c b/drivers/power/supply/bq27xxx_battery.c
index a7014525..f4449aba 100644
--- a/drivers/power/supply/bq27xxx_battery.c
+++ b/drivers/power/supply/bq27xxx_battery.c
@@ -1931,6 +1931,25 @@ static void bq27xxx_external_power_changed(struct power_supply *psy)
schedule_delayed_work(&di->work, 0);
}
+#ifdef DEBUG
+static void bq27xxx_battery_dbg_regs_dupes(struct bq27xxx_device_info *di)
+{
+ const size_t max = ARRAY_SIZE(bq27xxx_regs);
+ int a, b;
+
+ for (a = 1; a < max-1; a++) {
+ for (b = a+1; b < max; b++) {
+ if (!memcmp(bq27xxx_regs[a], bq27xxx_regs[b],
+ sizeof(bq27xxx_regs[0])))
+ dev_warn(di->dev,
+ "bq27xxx_regs[%d] & [%d] are identical\n", a, b);
+ }
+ }
+}
+#else
+static inline void bq27xxx_battery_dbg_regs_dupes(struct bq27xxx_device_info *di) {}
+#endif
+
int bq27xxx_battery_setup(struct bq27xxx_device_info *di)
{
struct power_supply_desc *psy_desc;
@@ -1939,6 +1958,8 @@ int bq27xxx_battery_setup(struct bq27xxx_device_info *di)
.drv_data = di,
};
+ bq27xxx_battery_dbg_regs_dupes(di);
+
di->ram_chip = di->real_chip == BQ27421 ||
di->real_chip == BQ27441 ||
di->real_chip == BQ27621;
--
2.13.0
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH v14 11/11] power: supply: bq27xxx: Remove duplicate register arrays
2017-06-07 18:37 [PATCH v14 00/11] devicetree simple-battery and client in bq27xxx_battery Liam Breck
` (9 preceding siblings ...)
2017-06-07 18:37 ` [PATCH v14 10/11] power: supply: bq27xxx: Flag identical register maps when in debug mode Liam Breck
@ 2017-06-07 18:37 ` Liam Breck
2017-06-08 16:42 ` [PATCH v14 00/11] devicetree simple-battery and client in bq27xxx_battery Sebastian Reichel
11 siblings, 0 replies; 25+ messages in thread
From: Liam Breck @ 2017-06-07 18:37 UTC (permalink / raw)
To: Sebastian Reichel, Pali Rohar, linux-pm
Cc: Enric Balletbo, Paul Kocialkowski, Quentin Schulz, Liam Breck
From: Liam Breck <kernel@networkimprov.net>
BQ2751X & BQ27510G3 are identical.
BQ27500 & BQ27510G1 & BQ27510G2 are identical.
Make BQ2751X & BQ27510G1/2 aliases.
Remove the duplicate arrays, etc.
Signed-off-by: Liam Breck <kernel@networkimprov.net>
---
drivers/power/supply/bq27xxx_battery.c | 126 -----------------------------
drivers/power/supply/bq27xxx_battery_i2c.c | 8 +-
include/linux/power/bq27xxx_battery.h | 12 +--
3 files changed, 10 insertions(+), 136 deletions(-)
diff --git a/drivers/power/supply/bq27xxx_battery.c b/drivers/power/supply/bq27xxx_battery.c
index f4449aba..b81f986d 100644
--- a/drivers/power/supply/bq27xxx_battery.c
+++ b/drivers/power/supply/bq27xxx_battery.c
@@ -201,26 +201,6 @@ static u8 bq27xxx_regs[BQ27MAX][BQ27XXX_REG_MAX] = {
[BQ27XXX_REG_AP] = INVALID_REG_ADDR,
BQ27XXX_DM_REG_ROWS,
},
- [BQ2751X] = {
- [BQ27XXX_REG_CTRL] = 0x00,
- [BQ27XXX_REG_TEMP] = 0x06,
- [BQ27XXX_REG_INT_TEMP] = 0x28,
- [BQ27XXX_REG_VOLT] = 0x08,
- [BQ27XXX_REG_AI] = 0x14,
- [BQ27XXX_REG_FLAGS] = 0x0a,
- [BQ27XXX_REG_TTE] = 0x16,
- [BQ27XXX_REG_TTF] = INVALID_REG_ADDR,
- [BQ27XXX_REG_TTES] = 0x1a,
- [BQ27XXX_REG_TTECP] = INVALID_REG_ADDR,
- [BQ27XXX_REG_NAC] = 0x0c,
- [BQ27XXX_REG_FCC] = 0x12,
- [BQ27XXX_REG_CYCT] = 0x1e,
- [BQ27XXX_REG_AE] = INVALID_REG_ADDR,
- [BQ27XXX_REG_SOC] = 0x20,
- [BQ27XXX_REG_DCAP] = 0x2e,
- [BQ27XXX_REG_AP] = INVALID_REG_ADDR,
- BQ27XXX_DM_REG_ROWS,
- },
[BQ27500] = {
[BQ27XXX_REG_CTRL] = 0x00,
[BQ27XXX_REG_TEMP] = 0x06,
@@ -241,46 +221,6 @@ static u8 bq27xxx_regs[BQ27MAX][BQ27XXX_REG_MAX] = {
[BQ27XXX_REG_AP] = 0x24,
BQ27XXX_DM_REG_ROWS,
},
- [BQ27510G1] = {
- [BQ27XXX_REG_CTRL] = 0x00,
- [BQ27XXX_REG_TEMP] = 0x06,
- [BQ27XXX_REG_INT_TEMP] = INVALID_REG_ADDR,
- [BQ27XXX_REG_VOLT] = 0x08,
- [BQ27XXX_REG_AI] = 0x14,
- [BQ27XXX_REG_FLAGS] = 0x0a,
- [BQ27XXX_REG_TTE] = 0x16,
- [BQ27XXX_REG_TTF] = 0x18,
- [BQ27XXX_REG_TTES] = 0x1c,
- [BQ27XXX_REG_TTECP] = 0x26,
- [BQ27XXX_REG_NAC] = 0x0c,
- [BQ27XXX_REG_FCC] = 0x12,
- [BQ27XXX_REG_CYCT] = 0x2a,
- [BQ27XXX_REG_AE] = 0x22,
- [BQ27XXX_REG_SOC] = 0x2c,
- [BQ27XXX_REG_DCAP] = 0x3c,
- [BQ27XXX_REG_AP] = 0x24,
- BQ27XXX_DM_REG_ROWS,
- },
- [BQ27510G2] = {
- [BQ27XXX_REG_CTRL] = 0x00,
- [BQ27XXX_REG_TEMP] = 0x06,
- [BQ27XXX_REG_INT_TEMP] = INVALID_REG_ADDR,
- [BQ27XXX_REG_VOLT] = 0x08,
- [BQ27XXX_REG_AI] = 0x14,
- [BQ27XXX_REG_FLAGS] = 0x0a,
- [BQ27XXX_REG_TTE] = 0x16,
- [BQ27XXX_REG_TTF] = 0x18,
- [BQ27XXX_REG_TTES] = 0x1c,
- [BQ27XXX_REG_TTECP] = 0x26,
- [BQ27XXX_REG_NAC] = 0x0c,
- [BQ27XXX_REG_FCC] = 0x12,
- [BQ27XXX_REG_CYCT] = 0x2a,
- [BQ27XXX_REG_AE] = 0x22,
- [BQ27XXX_REG_SOC] = 0x2c,
- [BQ27XXX_REG_DCAP] = 0x3c,
- [BQ27XXX_REG_AP] = 0x24,
- BQ27XXX_DM_REG_ROWS,
- },
[BQ27510G3] = {
[BQ27XXX_REG_CTRL] = 0x00,
[BQ27XXX_REG_TEMP] = 0x06,
@@ -523,24 +463,6 @@ static enum power_supply_property bq2750x_battery_props[] = {
POWER_SUPPLY_PROP_MANUFACTURER,
};
-static enum power_supply_property bq2751x_battery_props[] = {
- POWER_SUPPLY_PROP_STATUS,
- POWER_SUPPLY_PROP_PRESENT,
- POWER_SUPPLY_PROP_VOLTAGE_NOW,
- POWER_SUPPLY_PROP_CURRENT_NOW,
- POWER_SUPPLY_PROP_CAPACITY,
- POWER_SUPPLY_PROP_CAPACITY_LEVEL,
- POWER_SUPPLY_PROP_TEMP,
- POWER_SUPPLY_PROP_TIME_TO_EMPTY_NOW,
- POWER_SUPPLY_PROP_TECHNOLOGY,
- POWER_SUPPLY_PROP_CHARGE_FULL,
- POWER_SUPPLY_PROP_CHARGE_NOW,
- POWER_SUPPLY_PROP_CHARGE_FULL_DESIGN,
- POWER_SUPPLY_PROP_CYCLE_COUNT,
- POWER_SUPPLY_PROP_HEALTH,
- POWER_SUPPLY_PROP_MANUFACTURER,
-};
-
static enum power_supply_property bq27500_battery_props[] = {
POWER_SUPPLY_PROP_STATUS,
POWER_SUPPLY_PROP_PRESENT,
@@ -562,48 +484,6 @@ static enum power_supply_property bq27500_battery_props[] = {
POWER_SUPPLY_PROP_MANUFACTURER,
};
-static enum power_supply_property bq27510g1_battery_props[] = {
- POWER_SUPPLY_PROP_STATUS,
- POWER_SUPPLY_PROP_PRESENT,
- POWER_SUPPLY_PROP_VOLTAGE_NOW,
- POWER_SUPPLY_PROP_CURRENT_NOW,
- POWER_SUPPLY_PROP_CAPACITY,
- POWER_SUPPLY_PROP_CAPACITY_LEVEL,
- POWER_SUPPLY_PROP_TEMP,
- POWER_SUPPLY_PROP_TIME_TO_EMPTY_NOW,
- POWER_SUPPLY_PROP_TIME_TO_FULL_NOW,
- POWER_SUPPLY_PROP_TECHNOLOGY,
- POWER_SUPPLY_PROP_CHARGE_FULL,
- POWER_SUPPLY_PROP_CHARGE_NOW,
- POWER_SUPPLY_PROP_CHARGE_FULL_DESIGN,
- POWER_SUPPLY_PROP_CYCLE_COUNT,
- POWER_SUPPLY_PROP_ENERGY_NOW,
- POWER_SUPPLY_PROP_POWER_AVG,
- POWER_SUPPLY_PROP_HEALTH,
- POWER_SUPPLY_PROP_MANUFACTURER,
-};
-
-static enum power_supply_property bq27510g2_battery_props[] = {
- POWER_SUPPLY_PROP_STATUS,
- POWER_SUPPLY_PROP_PRESENT,
- POWER_SUPPLY_PROP_VOLTAGE_NOW,
- POWER_SUPPLY_PROP_CURRENT_NOW,
- POWER_SUPPLY_PROP_CAPACITY,
- POWER_SUPPLY_PROP_CAPACITY_LEVEL,
- POWER_SUPPLY_PROP_TEMP,
- POWER_SUPPLY_PROP_TIME_TO_EMPTY_NOW,
- POWER_SUPPLY_PROP_TIME_TO_FULL_NOW,
- POWER_SUPPLY_PROP_TECHNOLOGY,
- POWER_SUPPLY_PROP_CHARGE_FULL,
- POWER_SUPPLY_PROP_CHARGE_NOW,
- POWER_SUPPLY_PROP_CHARGE_FULL_DESIGN,
- POWER_SUPPLY_PROP_CYCLE_COUNT,
- POWER_SUPPLY_PROP_ENERGY_NOW,
- POWER_SUPPLY_PROP_POWER_AVG,
- POWER_SUPPLY_PROP_HEALTH,
- POWER_SUPPLY_PROP_MANUFACTURER,
-};
-
static enum power_supply_property bq27510g3_battery_props[] = {
POWER_SUPPLY_PROP_STATUS,
POWER_SUPPLY_PROP_PRESENT,
@@ -783,10 +663,7 @@ static struct {
BQ27XXX_PROP(BQ27000, bq27000_battery_props),
BQ27XXX_PROP(BQ27010, bq27010_battery_props),
BQ27XXX_PROP(BQ2750X, bq2750x_battery_props),
- BQ27XXX_PROP(BQ2751X, bq2751x_battery_props),
BQ27XXX_PROP(BQ27500, bq27500_battery_props),
- BQ27XXX_PROP(BQ27510G1, bq27510g1_battery_props),
- BQ27XXX_PROP(BQ27510G2, bq27510g2_battery_props),
BQ27XXX_PROP(BQ27510G3, bq27510g3_battery_props),
BQ27XXX_PROP(BQ27520G1, bq27520g1_battery_props),
BQ27XXX_PROP(BQ27520G2, bq27520g2_battery_props),
@@ -1577,10 +1454,7 @@ static bool bq27xxx_battery_overtemp(struct bq27xxx_device_info *di, u16 flags)
{
switch (di->chip) {
case BQ2750X:
- case BQ2751X:
case BQ27500:
- case BQ27510G1:
- case BQ27510G2:
case BQ27510G3:
case BQ27520G1:
case BQ27520G2:
diff --git a/drivers/power/supply/bq27xxx_battery_i2c.c b/drivers/power/supply/bq27xxx_battery_i2c.c
index a3656508..0ef0fca3 100644
--- a/drivers/power/supply/bq27xxx_battery_i2c.c
+++ b/drivers/power/supply/bq27xxx_battery_i2c.c
@@ -231,11 +231,11 @@ static const struct i2c_device_id bq27xxx_i2c_id_table[] = {
{ "bq27200", (BQ27000 << 16) | BQ27000 },
{ "bq27210", (BQ27010 << 16) | BQ27010 },
{ "bq27500", (BQ2750X << 16) | BQ2750X },
- { "bq27510", (BQ2751X << 16) | BQ2751X },
- { "bq27520", (BQ2752X << 16) | BQ2751X },
+ { "bq27510", (BQ2751X << 16) | BQ27510G3 },
+ { "bq27520", (BQ2752X << 16) | BQ27510G3 },
{ "bq27500-1", (BQ27500 << 16) | BQ27500 },
- { "bq27510g1", (BQ27510G1 << 16) | BQ27510G1 },
- { "bq27510g2", (BQ27510G2 << 16) | BQ27510G2 },
+ { "bq27510g1", (BQ27510G1 << 16) | BQ27500 },
+ { "bq27510g2", (BQ27510G2 << 16) | BQ27500 },
{ "bq27510g3", (BQ27510G3 << 16) | BQ27510G3 },
{ "bq27520g1", (BQ27520G1 << 16) | BQ27520G1 },
{ "bq27520g2", (BQ27520G2 << 16) | BQ27520G2 },
diff --git a/include/linux/power/bq27xxx_battery.h b/include/linux/power/bq27xxx_battery.h
index bec13b7a..acbe52e6 100644
--- a/include/linux/power/bq27xxx_battery.h
+++ b/include/linux/power/bq27xxx_battery.h
@@ -8,11 +8,8 @@ enum bq27xxx_chip {
BQ27000 = 1, /* bq27000, bq27200 */
BQ27010, /* bq27010, bq27210 */
BQ2750X, /* bq27500 deprecated alias */
- BQ2751X, /* bq27510, bq27520 deprecated alias */
- BQ27500, /* bq27500/1 */
- BQ27510G1, /* bq27510G1 */
- BQ27510G2, /* bq27510G2 */
- BQ27510G3, /* bq27510G3 */
+ BQ27500, /* bq27500/1, bq27510G1, bq27510G2 */
+ BQ27510G3, /* bq27510G3, bq27510, bq27520 */
BQ27520G1, /* bq27520G1 */
BQ27520G2, /* bq27520G2 */
BQ27520G3, /* bq27520G3 */
@@ -23,7 +20,10 @@ enum bq27xxx_chip {
BQ27421, /* bq27421, bq27425, bq27441, bq27621 */
BQ27MAX,
- BQ2752X, /* deprecated alias */
+ BQ2751X, /* bq27510 deprecated alias */
+ BQ2752X, /* bq27520 deprecated alias */
+ BQ27510G1,
+ BQ27510G2,
BQ27531,
BQ27542,
BQ27546,
--
2.13.0
^ permalink raw reply related [flat|nested] 25+ messages in thread
* Re: [PATCH v14 00/11] devicetree simple-battery and client in bq27xxx_battery
2017-06-07 18:37 [PATCH v14 00/11] devicetree simple-battery and client in bq27xxx_battery Liam Breck
` (10 preceding siblings ...)
2017-06-07 18:37 ` [PATCH v14 11/11] power: supply: bq27xxx: Remove duplicate register arrays Liam Breck
@ 2017-06-08 16:42 ` Sebastian Reichel
11 siblings, 0 replies; 25+ messages in thread
From: Sebastian Reichel @ 2017-06-08 16:42 UTC (permalink / raw)
To: Liam Breck
Cc: Pali Rohar, linux-pm, Enric Balletbo, Paul Kocialkowski, Quentin Schulz
[-- Attachment #1: Type: text/plain, Size: 7859 bytes --]
Hi,
On Wed, Jun 07, 2017 at 11:37:48AM -0700, Liam Breck wrote:
> Hi Sebastian, I think this is finally ready to go. I'll make a final
> test pass while awaiting any further comments. There's a lot here, so
> I'd appreciate it if you could look closely, as it's been a while since
> you had any feedback. Also note comment about Rob's ack in v13 change list.
Ok, I had some hours for reviewing stuff today and finished
and queued everything up to patch 8, which looked fine to me.
I will have a look at the remaining 3 sometime this week.
> Enric, Paul, Quentin, and I all have patchsets waiting on this one.
> Thanks!
I also queued some of those, so that stuff can be
properly tested in linux-next. Thanks for your work.
Also: If any issues come up for the patches I want
follow-up patches instead of replacing them.
-- Sebastian
> 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 RAM/NVM for params
> essential to correct operation: energy-full-design-microwatt-hours,
> charge-full-design-microamp-hours, voltage-min-design-microvolt
>
> Changes in v14:
> Doc...bindings/power/supply/ & power_supply_core
> * add constant-charge-{current,voltage}-max-* for Enric's patchset
> bq27xxx_battery:
> * separate set_cfgupdate() and soft_reset() functions
> * module param dt_monitored_battery_updates_nvm visible even when disabled
> * real_chip & chip IDs combined in i2c_id_table[]
> * hide untested chip params behind ifdef DEBUG
>
> Changes in v13:
> Doc...bindings/power/supply/*
> * add fields in battery.txt, see below
> * caution about changing battery type in battery.txt
> * note: Rob acked v11 battery.txt containing URL to Linux header
> * more detail in bq27xxx.txt
> power_supply_core:
> * add battery_info fields precharge-current-microamp & charge-term-current-microamp
> * new patch for power_supply_prop_precharge_current
> bq27xxx_battery:
> * add copyright notice
> * for flash/NVM chips, emit warning instead of doing update
> * config_battery_bq27xxx_dt_updates_nvm enables update of flash/NVM
> * module param dt_monitored_battery_updates_nvm lets user disallow update
> * new patch to flag dupes in bq27xxx_regs[]
> * drop patch to clean up error reporting
> * polishing from Andrew's feedback
> * resolve checkpatch errors
> * refactor patchset
> * fix missing static keyword flagged by Colin
>
> Changes in v12: (several partial series posts; see above)
>
> Changes in v11:
> power_supply_core:
> * switch to compatible = "simple-battery"
> * add docs to power_supply_class.txt
> Documentation/devicetree/.../battery.txt:
> * add description, drop refs to Linux, "simple-battery"
> bq27xxx_battery:
> * reset flash chips after DM update
> * add bq27xxx_write/xfer()
> * polishing from Andrew's feedback
> * new patch to clean up error reporting
> * new patch to consolidate duplicate register/property arrays
>
> Changes in v10:
> bq27xxx_battery:
> * pass actual chip ID into _setup()
> * add di->unseal_key & di->dm_regs; drop static arrays
> * support bq27425, 441, 621
>
> Changes in v9:
> bq27xxx_battery:
> * fix set_cfgupdate()
> * support bq27500, 545, 421; defer others
> * drop print_dm_blocks() patch
> * minor polishing
> Documentation/devicetree/.../battery.txt:
> * describe rationale for enum power_supply_property names
>
> Changes in v8:
> bq27xxx_battery:
> * wait on flag after set_cfgupdate & soft_reset
> * drop print_config(), report status in update_dm_block()
> * clarify error messages
> * cleanup from Andrew's feedback; minor polishing
>
> Changes in v7:
> bq27xxx_battery:
> * support chips where terminate_voltage & design_* live in separate blocks
> * draft support for 421, 441, 621 chips
> * new patch to log chip memory fields
> * report bus I/O errors; return error code in bq27xxx_battery_i2c
> * verify checksum in read_dm_block()
> * use set_cfgupdate only if chip provides it, soft_reset on I/O error
> * block_data_control=0 only in write_dm_block()
> * note toxic code from TI bqtool in write_dm_block()
> * lots of functionally neutral polishing
> Documentation/devicetree/.../battery.txt:
> * mention power_supply_get_battery_info()
>
> 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
>
>
> Liam Breck (9):
> dt-bindings: power: supply: Add battery.txt with simple-battery binding
> power: supply: core: Add power_supply_battery_info and API
> power: supply: core: Add power_supply_prop_precharge
> dt-bindings: power: supply: bq27xxx: Add monitored-battery documentation
> power: supply: bq27xxx: Add chip data memory read/write support
> power: supply: bq27xxx: Add power_supply_battery_info support
> power: supply: bq27xxx: Enable data memory update for certain chips
> power: supply: bq27xxx: Flag identical register maps when in debug mode
> power: supply: bq27xxx: Remove duplicate register arrays
>
> Matt Ranostay (2):
> devicetree: property-units: Add uWh and uAh units
> power: supply: bq27xxx: Add bulk transfer bus methods
>
> .../devicetree/bindings/power/supply/battery.txt | 57 ++
> .../devicetree/bindings/power/supply/bq27xxx.txt | 31 +-
> .../devicetree/bindings/property-units.txt | 2 +
> Documentation/power/power_supply_class.txt | 31 +-
> drivers/power/supply/Kconfig | 11 +
> drivers/power/supply/bq27xxx_battery.c | 752 +++++++++++++++++----
> drivers/power/supply/bq27xxx_battery_i2c.c | 134 +++-
> drivers/power/supply/power_supply_core.c | 57 ++
> drivers/power/supply/power_supply_sysfs.c | 1 +
> include/linux/power/bq27xxx_battery.h | 30 +-
> include/linux/power_supply.h | 25 +
> 11 files changed, 955 insertions(+), 176 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/power/supply/battery.txt
>
> --
> 2.13.0
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v14 09/11] power: supply: bq27xxx: Enable data memory update for certain chips
2017-06-07 18:37 ` [PATCH v14 09/11] power: supply: bq27xxx: Enable data memory update for certain chips Liam Breck
@ 2017-06-15 16:02 ` Sebastian Reichel
2017-06-16 9:21 ` Liam Breck
0 siblings, 1 reply; 25+ messages in thread
From: Sebastian Reichel @ 2017-06-15 16:02 UTC (permalink / raw)
To: Liam Breck
Cc: Pali Rohar, linux-pm, Enric Balletbo, Paul Kocialkowski,
Quentin Schulz, Liam Breck
[-- Attachment #1: Type: text/plain, Size: 2729 bytes --]
Hi,
On Wed, Jun 07, 2017 at 11:37:57AM -0700, Liam Breck wrote:
> static const struct i2c_device_id bq27xxx_i2c_id_table[] = {
> - { "bq27200", BQ27000 },
> - { "bq27210", BQ27010 },
> - { "bq27500", BQ2750X },
> - { "bq27510", BQ2751X },
> - { "bq27520", BQ2751X },
> - { "bq27500-1", BQ27500 },
> - { "bq27510g1", BQ27510G1 },
> - { "bq27510g2", BQ27510G2 },
> - { "bq27510g3", BQ27510G3 },
> - { "bq27520g1", BQ27520G1 },
> - { "bq27520g2", BQ27520G2 },
> - { "bq27520g3", BQ27520G3 },
> - { "bq27520g4", BQ27520G4 },
> - { "bq27530", BQ27530 },
> - { "bq27531", BQ27530 },
> - { "bq27541", BQ27541 },
> - { "bq27542", BQ27541 },
> - { "bq27546", BQ27541 },
> - { "bq27742", BQ27541 },
> - { "bq27545", BQ27545 },
> - { "bq27421", BQ27421 },
> - { "bq27425", BQ27421 },
> - { "bq27441", BQ27421 },
> - { "bq27621", BQ27421 },
> + /* dest. di->real_chip di->chip */
> + { "bq27200", (BQ27000 << 16) | BQ27000 },
> + { "bq27210", (BQ27010 << 16) | BQ27010 },
> + { "bq27500", (BQ2750X << 16) | BQ2750X },
> + { "bq27510", (BQ2751X << 16) | BQ2751X },
> + { "bq27520", (BQ2752X << 16) | BQ2751X },
> + { "bq27500-1", (BQ27500 << 16) | BQ27500 },
> + { "bq27510g1", (BQ27510G1 << 16) | BQ27510G1 },
> + { "bq27510g2", (BQ27510G2 << 16) | BQ27510G2 },
> + { "bq27510g3", (BQ27510G3 << 16) | BQ27510G3 },
> + { "bq27520g1", (BQ27520G1 << 16) | BQ27520G1 },
> + { "bq27520g2", (BQ27520G2 << 16) | BQ27520G2 },
> + { "bq27520g3", (BQ27520G3 << 16) | BQ27520G3 },
> + { "bq27520g4", (BQ27520G4 << 16) | BQ27520G4 },
> + { "bq27530", (BQ27530 << 16) | BQ27530 },
> + { "bq27531", (BQ27531 << 16) | BQ27530 },
> + { "bq27541", (BQ27541 << 16) | BQ27541 },
> + { "bq27542", (BQ27542 << 16) | BQ27541 },
> + { "bq27546", (BQ27546 << 16) | BQ27541 },
> + { "bq27742", (BQ27742 << 16) | BQ27541 },
> + { "bq27545", (BQ27545 << 16) | BQ27545 },
> + { "bq27421", (BQ27421 << 16) | BQ27421 },
> + { "bq27425", (BQ27425 << 16) | BQ27421 },
> + { "bq27441", (BQ27441 << 16) | BQ27421 },
> + { "bq27621", (BQ27621 << 16) | BQ27421 },
This is ugly. The proper way to do this is by providing a pointer
to a structure with all required information. E.g.:
static const struct bq27xxx_pdata chip_info_bq27530 {
.reg_layout = ®_info_bq27530,
.feature = false,
/* more stuff */
};
static const struct bq27xxx_pdata chip_info_bq27531 {
.reg_layout = ®_info_bq27530,
.feature = true,
/* more stuff */
};
/* ... */
static const struct i2c_device_id bq27xxx_i2c_id_table[] = {
/* ... */
{ "bq27530", &chip_info_bq27530 },
{ "bq27531", &chip_info_bq27531 },
/* ... */
}
-- Sebastian
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v14 10/11] power: supply: bq27xxx: Flag identical register maps when in debug mode
2017-06-07 18:37 ` [PATCH v14 10/11] power: supply: bq27xxx: Flag identical register maps when in debug mode Liam Breck
@ 2017-06-15 16:06 ` Sebastian Reichel
2017-06-16 9:44 ` Liam Breck
0 siblings, 1 reply; 25+ messages in thread
From: Sebastian Reichel @ 2017-06-15 16:06 UTC (permalink / raw)
To: Liam Breck
Cc: Pali Rohar, linux-pm, Enric Balletbo, Paul Kocialkowski,
Quentin Schulz, Liam Breck
[-- Attachment #1: Type: text/plain, Size: 1931 bytes --]
Hi,
On Wed, Jun 07, 2017 at 11:37:58AM -0700, Liam Breck wrote:
> From: Liam Breck <kernel@networkimprov.net>
>
> We tie multiple chips to unique register maps. When supporting a new chip,
> it's easy to add a duplicate map by mistake.
>
> In debug mode we now scan the register maps for duplicates.
>
> Signed-off-by: Liam Breck <kernel@networkimprov.net>
This does not belong into the kernel, but into some static analyzer
tool.
-- Sebastian
> drivers/power/supply/bq27xxx_battery.c | 21 +++++++++++++++++++++
> 1 file changed, 21 insertions(+)
>
> diff --git a/drivers/power/supply/bq27xxx_battery.c b/drivers/power/supply/bq27xxx_battery.c
> index a7014525..f4449aba 100644
> --- a/drivers/power/supply/bq27xxx_battery.c
> +++ b/drivers/power/supply/bq27xxx_battery.c
> @@ -1931,6 +1931,25 @@ static void bq27xxx_external_power_changed(struct power_supply *psy)
> schedule_delayed_work(&di->work, 0);
> }
>
> +#ifdef DEBUG
> +static void bq27xxx_battery_dbg_regs_dupes(struct bq27xxx_device_info *di)
> +{
> + const size_t max = ARRAY_SIZE(bq27xxx_regs);
> + int a, b;
> +
> + for (a = 1; a < max-1; a++) {
> + for (b = a+1; b < max; b++) {
> + if (!memcmp(bq27xxx_regs[a], bq27xxx_regs[b],
> + sizeof(bq27xxx_regs[0])))
> + dev_warn(di->dev,
> + "bq27xxx_regs[%d] & [%d] are identical\n", a, b);
> + }
> + }
> +}
> +#else
> +static inline void bq27xxx_battery_dbg_regs_dupes(struct bq27xxx_device_info *di) {}
> +#endif
> +
> int bq27xxx_battery_setup(struct bq27xxx_device_info *di)
> {
> struct power_supply_desc *psy_desc;
> @@ -1939,6 +1958,8 @@ int bq27xxx_battery_setup(struct bq27xxx_device_info *di)
> .drv_data = di,
> };
>
> + bq27xxx_battery_dbg_regs_dupes(di);
> +
> di->ram_chip = di->real_chip == BQ27421 ||
> di->real_chip == BQ27441 ||
> di->real_chip == BQ27621;
> --
> 2.13.0
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v14 09/11] power: supply: bq27xxx: Enable data memory update for certain chips
2017-06-15 16:02 ` Sebastian Reichel
@ 2017-06-16 9:21 ` Liam Breck
2017-06-16 10:33 ` Sebastian Reichel
0 siblings, 1 reply; 25+ messages in thread
From: Liam Breck @ 2017-06-16 9:21 UTC (permalink / raw)
To: Sebastian Reichel
Cc: Pali Rohar, linux-pm, Enric Balletbo, Paul Kocialkowski,
Quentin Schulz, Liam Breck
Hi Sebastian,
On Thu, Jun 15, 2017 at 9:02 AM, Sebastian Reichel
<sebastian.reichel@collabora.co.uk> wrote:
> Hi,
>
> On Wed, Jun 07, 2017 at 11:37:57AM -0700, Liam Breck wrote:
>> static const struct i2c_device_id bq27xxx_i2c_id_table[] = {
>> - { "bq27200", BQ27000 },
>> - { "bq27210", BQ27010 },
>> - { "bq27500", BQ2750X },
>> - { "bq27510", BQ2751X },
>> - { "bq27520", BQ2751X },
>> - { "bq27500-1", BQ27500 },
>> - { "bq27510g1", BQ27510G1 },
>> - { "bq27510g2", BQ27510G2 },
>> - { "bq27510g3", BQ27510G3 },
>> - { "bq27520g1", BQ27520G1 },
>> - { "bq27520g2", BQ27520G2 },
>> - { "bq27520g3", BQ27520G3 },
>> - { "bq27520g4", BQ27520G4 },
>> - { "bq27530", BQ27530 },
>> - { "bq27531", BQ27530 },
>> - { "bq27541", BQ27541 },
>> - { "bq27542", BQ27541 },
>> - { "bq27546", BQ27541 },
>> - { "bq27742", BQ27541 },
>> - { "bq27545", BQ27545 },
>> - { "bq27421", BQ27421 },
>> - { "bq27425", BQ27421 },
>> - { "bq27441", BQ27421 },
>> - { "bq27621", BQ27421 },
>> + /* dest. di->real_chip di->chip */
>> + { "bq27200", (BQ27000 << 16) | BQ27000 },
>> + { "bq27210", (BQ27010 << 16) | BQ27010 },
>> + { "bq27500", (BQ2750X << 16) | BQ2750X },
>> + { "bq27510", (BQ2751X << 16) | BQ2751X },
>> + { "bq27520", (BQ2752X << 16) | BQ2751X },
>> + { "bq27500-1", (BQ27500 << 16) | BQ27500 },
>> + { "bq27510g1", (BQ27510G1 << 16) | BQ27510G1 },
>> + { "bq27510g2", (BQ27510G2 << 16) | BQ27510G2 },
>> + { "bq27510g3", (BQ27510G3 << 16) | BQ27510G3 },
>> + { "bq27520g1", (BQ27520G1 << 16) | BQ27520G1 },
>> + { "bq27520g2", (BQ27520G2 << 16) | BQ27520G2 },
>> + { "bq27520g3", (BQ27520G3 << 16) | BQ27520G3 },
>> + { "bq27520g4", (BQ27520G4 << 16) | BQ27520G4 },
>> + { "bq27530", (BQ27530 << 16) | BQ27530 },
>> + { "bq27531", (BQ27531 << 16) | BQ27530 },
>> + { "bq27541", (BQ27541 << 16) | BQ27541 },
>> + { "bq27542", (BQ27542 << 16) | BQ27541 },
>> + { "bq27546", (BQ27546 << 16) | BQ27541 },
>> + { "bq27742", (BQ27742 << 16) | BQ27541 },
>> + { "bq27545", (BQ27545 << 16) | BQ27545 },
>> + { "bq27421", (BQ27421 << 16) | BQ27421 },
>> + { "bq27425", (BQ27425 << 16) | BQ27421 },
>> + { "bq27441", (BQ27441 << 16) | BQ27421 },
>> + { "bq27621", (BQ27621 << 16) | BQ27421 },
>
> This is ugly. The proper way to do this is by providing a pointer
> to a structure with all required information. E.g.:
>
> static const struct bq27xxx_pdata chip_info_bq27530 {
> .reg_layout = ®_info_bq27530,
> .feature = false,
> /* more stuff */
> };
>
> static const struct bq27xxx_pdata chip_info_bq27531 {
> .reg_layout = ®_info_bq27530,
> .feature = true,
> /* more stuff */
> };
>
> /* ... */
>
> static const struct i2c_device_id bq27xxx_i2c_id_table[] = {
> /* ... */
> { "bq27530", &chip_info_bq27530 },
> { "bq27531", &chip_info_bq27531 },
> /* ... */
> }
How's this...
static const struct bq27xxx_chip_ids[] = {
[BQ27425] = { .chip = BQ27421, .real_chip = BQ27425 },
...
}
static const struct i2c_device_id bq27xxx_i2c_id_table[] = {
{ "bq27425", &bq27xxx_chip_ids[BQ27425] },
...
.
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v14 10/11] power: supply: bq27xxx: Flag identical register maps when in debug mode
2017-06-15 16:06 ` Sebastian Reichel
@ 2017-06-16 9:44 ` Liam Breck
0 siblings, 0 replies; 25+ messages in thread
From: Liam Breck @ 2017-06-16 9:44 UTC (permalink / raw)
To: Sebastian Reichel
Cc: Pali Rohar, linux-pm, Enric Balletbo, Paul Kocialkowski,
Quentin Schulz, Liam Breck
Hi Sebastian,
On Thu, Jun 15, 2017 at 9:06 AM, Sebastian Reichel
<sebastian.reichel@collabora.co.uk> wrote:
> Hi,
>
> On Wed, Jun 07, 2017 at 11:37:58AM -0700, Liam Breck wrote:
>> From: Liam Breck <kernel@networkimprov.net>
>>
>> We tie multiple chips to unique register maps. When supporting a new chip,
>> it's easy to add a duplicate map by mistake.
>>
>> In debug mode we now scan the register maps for duplicates.
>>
>> Signed-off-by: Liam Breck <kernel@networkimprov.net>
>
> This does not belong into the kernel, but into some static analyzer
> tool.
More than a dozen chip definitions are in this driver, spanning 300
lines. We need these 18 lines to manage them; I think this would be
overlooked in an external utility.
I put this code behind ifdef DEBUG; if that's not OK, could we put it
behind a kernel option or other define?
>> drivers/power/supply/bq27xxx_battery.c | 21 +++++++++++++++++++++
>> 1 file changed, 21 insertions(+)
>>
>> diff --git a/drivers/power/supply/bq27xxx_battery.c b/drivers/power/supply/bq27xxx_battery.c
>> index a7014525..f4449aba 100644
>> --- a/drivers/power/supply/bq27xxx_battery.c
>> +++ b/drivers/power/supply/bq27xxx_battery.c
>> @@ -1931,6 +1931,25 @@ static void bq27xxx_external_power_changed(struct power_supply *psy)
>> schedule_delayed_work(&di->work, 0);
>> }
>>
>> +#ifdef DEBUG
>> +static void bq27xxx_battery_dbg_regs_dupes(struct bq27xxx_device_info *di)
>> +{
>> + const size_t max = ARRAY_SIZE(bq27xxx_regs);
>> + int a, b;
>> +
>> + for (a = 1; a < max-1; a++) {
>> + for (b = a+1; b < max; b++) {
>> + if (!memcmp(bq27xxx_regs[a], bq27xxx_regs[b],
>> + sizeof(bq27xxx_regs[0])))
>> + dev_warn(di->dev,
>> + "bq27xxx_regs[%d] & [%d] are identical\n", a, b);
>> + }
>> + }
>> +}
>> +#else
>> +static inline void bq27xxx_battery_dbg_regs_dupes(struct bq27xxx_device_info *di) {}
>> +#endif
>> +
>> int bq27xxx_battery_setup(struct bq27xxx_device_info *di)
>> {
>> struct power_supply_desc *psy_desc;
>> @@ -1939,6 +1958,8 @@ int bq27xxx_battery_setup(struct bq27xxx_device_info *di)
>> .drv_data = di,
>> };
>>
>> + bq27xxx_battery_dbg_regs_dupes(di);
>> +
>> di->ram_chip = di->real_chip == BQ27421 ||
>> di->real_chip == BQ27441 ||
>> di->real_chip == BQ27621;
>> --
>> 2.13.0
>>
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v14 09/11] power: supply: bq27xxx: Enable data memory update for certain chips
2017-06-16 9:21 ` Liam Breck
@ 2017-06-16 10:33 ` Sebastian Reichel
2017-06-16 11:32 ` Liam Breck
0 siblings, 1 reply; 25+ messages in thread
From: Sebastian Reichel @ 2017-06-16 10:33 UTC (permalink / raw)
To: Liam Breck
Cc: Pali Rohar, linux-pm, Enric Balletbo, Paul Kocialkowski,
Quentin Schulz, Liam Breck
[-- Attachment #1: Type: text/plain, Size: 3822 bytes --]
Hi Liam,
On Fri, Jun 16, 2017 at 02:21:42AM -0700, Liam Breck wrote:
> On Thu, Jun 15, 2017 at 9:02 AM, Sebastian Reichel
> <sebastian.reichel@collabora.co.uk> wrote:
> > On Wed, Jun 07, 2017 at 11:37:57AM -0700, Liam Breck wrote:
> >> static const struct i2c_device_id bq27xxx_i2c_id_table[] = {
> >> - { "bq27200", BQ27000 },
> >> - { "bq27210", BQ27010 },
> >> - { "bq27500", BQ2750X },
> >> - { "bq27510", BQ2751X },
> >> - { "bq27520", BQ2751X },
> >> - { "bq27500-1", BQ27500 },
> >> - { "bq27510g1", BQ27510G1 },
> >> - { "bq27510g2", BQ27510G2 },
> >> - { "bq27510g3", BQ27510G3 },
> >> - { "bq27520g1", BQ27520G1 },
> >> - { "bq27520g2", BQ27520G2 },
> >> - { "bq27520g3", BQ27520G3 },
> >> - { "bq27520g4", BQ27520G4 },
> >> - { "bq27530", BQ27530 },
> >> - { "bq27531", BQ27530 },
> >> - { "bq27541", BQ27541 },
> >> - { "bq27542", BQ27541 },
> >> - { "bq27546", BQ27541 },
> >> - { "bq27742", BQ27541 },
> >> - { "bq27545", BQ27545 },
> >> - { "bq27421", BQ27421 },
> >> - { "bq27425", BQ27421 },
> >> - { "bq27441", BQ27421 },
> >> - { "bq27621", BQ27421 },
> >> + /* dest. di->real_chip di->chip */
> >> + { "bq27200", (BQ27000 << 16) | BQ27000 },
> >> + { "bq27210", (BQ27010 << 16) | BQ27010 },
> >> + { "bq27500", (BQ2750X << 16) | BQ2750X },
> >> + { "bq27510", (BQ2751X << 16) | BQ2751X },
> >> + { "bq27520", (BQ2752X << 16) | BQ2751X },
> >> + { "bq27500-1", (BQ27500 << 16) | BQ27500 },
> >> + { "bq27510g1", (BQ27510G1 << 16) | BQ27510G1 },
> >> + { "bq27510g2", (BQ27510G2 << 16) | BQ27510G2 },
> >> + { "bq27510g3", (BQ27510G3 << 16) | BQ27510G3 },
> >> + { "bq27520g1", (BQ27520G1 << 16) | BQ27520G1 },
> >> + { "bq27520g2", (BQ27520G2 << 16) | BQ27520G2 },
> >> + { "bq27520g3", (BQ27520G3 << 16) | BQ27520G3 },
> >> + { "bq27520g4", (BQ27520G4 << 16) | BQ27520G4 },
> >> + { "bq27530", (BQ27530 << 16) | BQ27530 },
> >> + { "bq27531", (BQ27531 << 16) | BQ27530 },
> >> + { "bq27541", (BQ27541 << 16) | BQ27541 },
> >> + { "bq27542", (BQ27542 << 16) | BQ27541 },
> >> + { "bq27546", (BQ27546 << 16) | BQ27541 },
> >> + { "bq27742", (BQ27742 << 16) | BQ27541 },
> >> + { "bq27545", (BQ27545 << 16) | BQ27545 },
> >> + { "bq27421", (BQ27421 << 16) | BQ27421 },
> >> + { "bq27425", (BQ27425 << 16) | BQ27421 },
> >> + { "bq27441", (BQ27441 << 16) | BQ27421 },
> >> + { "bq27621", (BQ27621 << 16) | BQ27421 },
> >
> > This is ugly. The proper way to do this is by providing a pointer
> > to a structure with all required information. E.g.:
> >
> > static const struct bq27xxx_pdata chip_info_bq27530 {
> > .reg_layout = ®_info_bq27530,
> > .feature = false,
> > /* more stuff */
> > };
> >
> > static const struct bq27xxx_pdata chip_info_bq27531 {
> > .reg_layout = ®_info_bq27530,
> > .feature = true,
> > /* more stuff */
> > };
> >
> > /* ... */
> >
> > static const struct i2c_device_id bq27xxx_i2c_id_table[] = {
> > /* ... */
> > { "bq27530", &chip_info_bq27530 },
> > { "bq27531", &chip_info_bq27531 },
> > /* ... */
> > }
>
> How's this...
>
> static const struct bq27xxx_chip_ids[] = {
> [BQ27425] = { .chip = BQ27421, .real_chip = BQ27425 },
> ...
> }
>
> static const struct i2c_device_id bq27xxx_i2c_id_table[] = {
> { "bq27425", &bq27xxx_chip_ids[BQ27425] },
> ...
That's better, but let's get rid of real_chip. Just add the required
information directly to the above struct (e.g. seal_key, ramtype).
-- Sebastian
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v14 09/11] power: supply: bq27xxx: Enable data memory update for certain chips
2017-06-16 10:33 ` Sebastian Reichel
@ 2017-06-16 11:32 ` Liam Breck
2017-06-21 20:55 ` Liam Breck
2017-07-03 16:48 ` Sebastian Reichel
0 siblings, 2 replies; 25+ messages in thread
From: Liam Breck @ 2017-06-16 11:32 UTC (permalink / raw)
To: Sebastian Reichel
Cc: Pali Rohar, linux-pm, Enric Balletbo, Paul Kocialkowski,
Quentin Schulz, Liam Breck
Hi Sebastian,
On Fri, Jun 16, 2017 at 3:33 AM, Sebastian Reichel
<sebastian.reichel@collabora.co.uk> wrote:
> Hi Liam,
>
> On Fri, Jun 16, 2017 at 02:21:42AM -0700, Liam Breck wrote:
>> On Thu, Jun 15, 2017 at 9:02 AM, Sebastian Reichel
>> <sebastian.reichel@collabora.co.uk> wrote:
>> > On Wed, Jun 07, 2017 at 11:37:57AM -0700, Liam Breck wrote:
>> >> static const struct i2c_device_id bq27xxx_i2c_id_table[] = {
>> >> - { "bq27200", BQ27000 },
>> >> - { "bq27210", BQ27010 },
>> >> - { "bq27500", BQ2750X },
>> >> - { "bq27510", BQ2751X },
>> >> - { "bq27520", BQ2751X },
>> >> - { "bq27500-1", BQ27500 },
>> >> - { "bq27510g1", BQ27510G1 },
>> >> - { "bq27510g2", BQ27510G2 },
>> >> - { "bq27510g3", BQ27510G3 },
>> >> - { "bq27520g1", BQ27520G1 },
>> >> - { "bq27520g2", BQ27520G2 },
>> >> - { "bq27520g3", BQ27520G3 },
>> >> - { "bq27520g4", BQ27520G4 },
>> >> - { "bq27530", BQ27530 },
>> >> - { "bq27531", BQ27530 },
>> >> - { "bq27541", BQ27541 },
>> >> - { "bq27542", BQ27541 },
>> >> - { "bq27546", BQ27541 },
>> >> - { "bq27742", BQ27541 },
>> >> - { "bq27545", BQ27545 },
>> >> - { "bq27421", BQ27421 },
>> >> - { "bq27425", BQ27421 },
>> >> - { "bq27441", BQ27421 },
>> >> - { "bq27621", BQ27421 },
>> >> + /* dest. di->real_chip di->chip */
>> >> + { "bq27200", (BQ27000 << 16) | BQ27000 },
>> >> + { "bq27210", (BQ27010 << 16) | BQ27010 },
>> >> + { "bq27500", (BQ2750X << 16) | BQ2750X },
>> >> + { "bq27510", (BQ2751X << 16) | BQ2751X },
>> >> + { "bq27520", (BQ2752X << 16) | BQ2751X },
>> >> + { "bq27500-1", (BQ27500 << 16) | BQ27500 },
>> >> + { "bq27510g1", (BQ27510G1 << 16) | BQ27510G1 },
>> >> + { "bq27510g2", (BQ27510G2 << 16) | BQ27510G2 },
>> >> + { "bq27510g3", (BQ27510G3 << 16) | BQ27510G3 },
>> >> + { "bq27520g1", (BQ27520G1 << 16) | BQ27520G1 },
>> >> + { "bq27520g2", (BQ27520G2 << 16) | BQ27520G2 },
>> >> + { "bq27520g3", (BQ27520G3 << 16) | BQ27520G3 },
>> >> + { "bq27520g4", (BQ27520G4 << 16) | BQ27520G4 },
>> >> + { "bq27530", (BQ27530 << 16) | BQ27530 },
>> >> + { "bq27531", (BQ27531 << 16) | BQ27530 },
>> >> + { "bq27541", (BQ27541 << 16) | BQ27541 },
>> >> + { "bq27542", (BQ27542 << 16) | BQ27541 },
>> >> + { "bq27546", (BQ27546 << 16) | BQ27541 },
>> >> + { "bq27742", (BQ27742 << 16) | BQ27541 },
>> >> + { "bq27545", (BQ27545 << 16) | BQ27545 },
>> >> + { "bq27421", (BQ27421 << 16) | BQ27421 },
>> >> + { "bq27425", (BQ27425 << 16) | BQ27421 },
>> >> + { "bq27441", (BQ27441 << 16) | BQ27421 },
>> >> + { "bq27621", (BQ27621 << 16) | BQ27421 },
>> >
>> > This is ugly. The proper way to do this is by providing a pointer
>> > to a structure with all required information. E.g.:
>> >
>> > static const struct bq27xxx_pdata chip_info_bq27530 {
>> > .reg_layout = ®_info_bq27530,
>> > .feature = false,
>> > /* more stuff */
>> > };
>> >
>> > static const struct bq27xxx_pdata chip_info_bq27531 {
>> > .reg_layout = ®_info_bq27530,
>> > .feature = true,
>> > /* more stuff */
>> > };
>> >
>> > /* ... */
>> >
>> > static const struct i2c_device_id bq27xxx_i2c_id_table[] = {
>> > /* ... */
>> > { "bq27530", &chip_info_bq27530 },
>> > { "bq27531", &chip_info_bq27531 },
>> > /* ... */
>> > }
>>
>> How's this...
>>
>> static const struct bq27xxx_chip_ids[] = {
>> [BQ27425] = { .chip = BQ27421, .real_chip = BQ27425 },
>> ...
>> }
>>
>> static const struct i2c_device_id bq27xxx_i2c_id_table[] = {
>> { "bq27425", &bq27xxx_chip_ids[BQ27425] },
>> ...
>
> That's better, but let's get rid of real_chip. Just add the required
> information directly to the above struct (e.g. seal_key, ramtype).
I don't think the new chip data belongs in bq27*_i2c.c. All the
(extensive) existing chip data is in the main file.
A later patch could rework the chip data scheme; I was trying to
minimize changes in this one. The original i2c table did a real_chip
-> chip mapping. Now we're just carrying real_chip forward.
The easiest fix would be to define a macro which does << and |
{ "bq27621", BQ27XXX_ID_PAIR(BQ27621, BQ27421) },
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v14 09/11] power: supply: bq27xxx: Enable data memory update for certain chips
2017-06-16 11:32 ` Liam Breck
@ 2017-06-21 20:55 ` Liam Breck
2017-07-03 16:48 ` Sebastian Reichel
1 sibling, 0 replies; 25+ messages in thread
From: Liam Breck @ 2017-06-21 20:55 UTC (permalink / raw)
To: Sebastian Reichel; +Cc: Pali Rohar, linux-pm, Paul Kocialkowski, Liam Breck
Hi Sebastian, can we resolve the two outstanding issues this week? I
sent comments on both last week...
On Fri, Jun 16, 2017 at 4:32 AM, Liam Breck <liam@networkimprov.net> wrote:
> Hi Sebastian,
>
>
> On Fri, Jun 16, 2017 at 3:33 AM, Sebastian Reichel
> <sebastian.reichel@collabora.co.uk> wrote:
>> Hi Liam,
>>
>> On Fri, Jun 16, 2017 at 02:21:42AM -0700, Liam Breck wrote:
>>> On Thu, Jun 15, 2017 at 9:02 AM, Sebastian Reichel
>>> <sebastian.reichel@collabora.co.uk> wrote:
>>> > On Wed, Jun 07, 2017 at 11:37:57AM -0700, Liam Breck wrote:
>>> >> static const struct i2c_device_id bq27xxx_i2c_id_table[] = {
>>> >> - { "bq27200", BQ27000 },
>>> >> - { "bq27210", BQ27010 },
>>> >> - { "bq27500", BQ2750X },
>>> >> - { "bq27510", BQ2751X },
>>> >> - { "bq27520", BQ2751X },
>>> >> - { "bq27500-1", BQ27500 },
>>> >> - { "bq27510g1", BQ27510G1 },
>>> >> - { "bq27510g2", BQ27510G2 },
>>> >> - { "bq27510g3", BQ27510G3 },
>>> >> - { "bq27520g1", BQ27520G1 },
>>> >> - { "bq27520g2", BQ27520G2 },
>>> >> - { "bq27520g3", BQ27520G3 },
>>> >> - { "bq27520g4", BQ27520G4 },
>>> >> - { "bq27530", BQ27530 },
>>> >> - { "bq27531", BQ27530 },
>>> >> - { "bq27541", BQ27541 },
>>> >> - { "bq27542", BQ27541 },
>>> >> - { "bq27546", BQ27541 },
>>> >> - { "bq27742", BQ27541 },
>>> >> - { "bq27545", BQ27545 },
>>> >> - { "bq27421", BQ27421 },
>>> >> - { "bq27425", BQ27421 },
>>> >> - { "bq27441", BQ27421 },
>>> >> - { "bq27621", BQ27421 },
>>> >> + /* dest. di->real_chip di->chip */
>>> >> + { "bq27200", (BQ27000 << 16) | BQ27000 },
>>> >> + { "bq27210", (BQ27010 << 16) | BQ27010 },
>>> >> + { "bq27500", (BQ2750X << 16) | BQ2750X },
>>> >> + { "bq27510", (BQ2751X << 16) | BQ2751X },
>>> >> + { "bq27520", (BQ2752X << 16) | BQ2751X },
>>> >> + { "bq27500-1", (BQ27500 << 16) | BQ27500 },
>>> >> + { "bq27510g1", (BQ27510G1 << 16) | BQ27510G1 },
>>> >> + { "bq27510g2", (BQ27510G2 << 16) | BQ27510G2 },
>>> >> + { "bq27510g3", (BQ27510G3 << 16) | BQ27510G3 },
>>> >> + { "bq27520g1", (BQ27520G1 << 16) | BQ27520G1 },
>>> >> + { "bq27520g2", (BQ27520G2 << 16) | BQ27520G2 },
>>> >> + { "bq27520g3", (BQ27520G3 << 16) | BQ27520G3 },
>>> >> + { "bq27520g4", (BQ27520G4 << 16) | BQ27520G4 },
>>> >> + { "bq27530", (BQ27530 << 16) | BQ27530 },
>>> >> + { "bq27531", (BQ27531 << 16) | BQ27530 },
>>> >> + { "bq27541", (BQ27541 << 16) | BQ27541 },
>>> >> + { "bq27542", (BQ27542 << 16) | BQ27541 },
>>> >> + { "bq27546", (BQ27546 << 16) | BQ27541 },
>>> >> + { "bq27742", (BQ27742 << 16) | BQ27541 },
>>> >> + { "bq27545", (BQ27545 << 16) | BQ27545 },
>>> >> + { "bq27421", (BQ27421 << 16) | BQ27421 },
>>> >> + { "bq27425", (BQ27425 << 16) | BQ27421 },
>>> >> + { "bq27441", (BQ27441 << 16) | BQ27421 },
>>> >> + { "bq27621", (BQ27621 << 16) | BQ27421 },
>>> >
>>> > This is ugly. The proper way to do this is by providing a pointer
>>> > to a structure with all required information. E.g.:
>>> >
>>> > static const struct bq27xxx_pdata chip_info_bq27530 {
>>> > .reg_layout = ®_info_bq27530,
>>> > .feature = false,
>>> > /* more stuff */
>>> > };
>>> >
>>> > static const struct bq27xxx_pdata chip_info_bq27531 {
>>> > .reg_layout = ®_info_bq27530,
>>> > .feature = true,
>>> > /* more stuff */
>>> > };
>>> >
>>> > /* ... */
>>> >
>>> > static const struct i2c_device_id bq27xxx_i2c_id_table[] = {
>>> > /* ... */
>>> > { "bq27530", &chip_info_bq27530 },
>>> > { "bq27531", &chip_info_bq27531 },
>>> > /* ... */
>>> > }
>>>
>>> How's this...
>>>
>>> static const struct bq27xxx_chip_ids[] = {
>>> [BQ27425] = { .chip = BQ27421, .real_chip = BQ27425 },
>>> ...
>>> }
>>>
>>> static const struct i2c_device_id bq27xxx_i2c_id_table[] = {
>>> { "bq27425", &bq27xxx_chip_ids[BQ27425] },
>>> ...
>>
>> That's better, but let's get rid of real_chip. Just add the required
>> information directly to the above struct (e.g. seal_key, ramtype).
>
> I don't think the new chip data belongs in bq27*_i2c.c. All the
> (extensive) existing chip data is in the main file.
>
> A later patch could rework the chip data scheme; I was trying to
> minimize changes in this one. The original i2c table did a real_chip
> -> chip mapping. Now we're just carrying real_chip forward.
>
> The easiest fix would be to define a macro which does << and |
>
> { "bq27621", BQ27XXX_ID_PAIR(BQ27621, BQ27421) },
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v14 09/11] power: supply: bq27xxx: Enable data memory update for certain chips
2017-06-16 11:32 ` Liam Breck
2017-06-21 20:55 ` Liam Breck
@ 2017-07-03 16:48 ` Sebastian Reichel
2017-07-04 23:24 ` Liam Breck
1 sibling, 1 reply; 25+ messages in thread
From: Sebastian Reichel @ 2017-07-03 16:48 UTC (permalink / raw)
To: Liam Breck
Cc: Pali Rohar, linux-pm, Enric Balletbo, Paul Kocialkowski,
Quentin Schulz, Liam Breck
[-- Attachment #1: Type: text/plain, Size: 5156 bytes --]
Hi,
On Fri, Jun 16, 2017 at 04:32:13AM -0700, Liam Breck wrote:
> On Fri, Jun 16, 2017 at 3:33 AM, Sebastian Reichel
> <sebastian.reichel@collabora.co.uk> wrote:
> > Hi Liam,
> >
> > On Fri, Jun 16, 2017 at 02:21:42AM -0700, Liam Breck wrote:
> >> On Thu, Jun 15, 2017 at 9:02 AM, Sebastian Reichel
> >> <sebastian.reichel@collabora.co.uk> wrote:
> >> > On Wed, Jun 07, 2017 at 11:37:57AM -0700, Liam Breck wrote:
> >> >> static const struct i2c_device_id bq27xxx_i2c_id_table[] = {
> >> >> - { "bq27200", BQ27000 },
> >> >> - { "bq27210", BQ27010 },
> >> >> - { "bq27500", BQ2750X },
> >> >> - { "bq27510", BQ2751X },
> >> >> - { "bq27520", BQ2751X },
> >> >> - { "bq27500-1", BQ27500 },
> >> >> - { "bq27510g1", BQ27510G1 },
> >> >> - { "bq27510g2", BQ27510G2 },
> >> >> - { "bq27510g3", BQ27510G3 },
> >> >> - { "bq27520g1", BQ27520G1 },
> >> >> - { "bq27520g2", BQ27520G2 },
> >> >> - { "bq27520g3", BQ27520G3 },
> >> >> - { "bq27520g4", BQ27520G4 },
> >> >> - { "bq27530", BQ27530 },
> >> >> - { "bq27531", BQ27530 },
> >> >> - { "bq27541", BQ27541 },
> >> >> - { "bq27542", BQ27541 },
> >> >> - { "bq27546", BQ27541 },
> >> >> - { "bq27742", BQ27541 },
> >> >> - { "bq27545", BQ27545 },
> >> >> - { "bq27421", BQ27421 },
> >> >> - { "bq27425", BQ27421 },
> >> >> - { "bq27441", BQ27421 },
> >> >> - { "bq27621", BQ27421 },
> >> >> + /* dest. di->real_chip di->chip */
> >> >> + { "bq27200", (BQ27000 << 16) | BQ27000 },
> >> >> + { "bq27210", (BQ27010 << 16) | BQ27010 },
> >> >> + { "bq27500", (BQ2750X << 16) | BQ2750X },
> >> >> + { "bq27510", (BQ2751X << 16) | BQ2751X },
> >> >> + { "bq27520", (BQ2752X << 16) | BQ2751X },
> >> >> + { "bq27500-1", (BQ27500 << 16) | BQ27500 },
> >> >> + { "bq27510g1", (BQ27510G1 << 16) | BQ27510G1 },
> >> >> + { "bq27510g2", (BQ27510G2 << 16) | BQ27510G2 },
> >> >> + { "bq27510g3", (BQ27510G3 << 16) | BQ27510G3 },
> >> >> + { "bq27520g1", (BQ27520G1 << 16) | BQ27520G1 },
> >> >> + { "bq27520g2", (BQ27520G2 << 16) | BQ27520G2 },
> >> >> + { "bq27520g3", (BQ27520G3 << 16) | BQ27520G3 },
> >> >> + { "bq27520g4", (BQ27520G4 << 16) | BQ27520G4 },
> >> >> + { "bq27530", (BQ27530 << 16) | BQ27530 },
> >> >> + { "bq27531", (BQ27531 << 16) | BQ27530 },
> >> >> + { "bq27541", (BQ27541 << 16) | BQ27541 },
> >> >> + { "bq27542", (BQ27542 << 16) | BQ27541 },
> >> >> + { "bq27546", (BQ27546 << 16) | BQ27541 },
> >> >> + { "bq27742", (BQ27742 << 16) | BQ27541 },
> >> >> + { "bq27545", (BQ27545 << 16) | BQ27545 },
> >> >> + { "bq27421", (BQ27421 << 16) | BQ27421 },
> >> >> + { "bq27425", (BQ27425 << 16) | BQ27421 },
> >> >> + { "bq27441", (BQ27441 << 16) | BQ27421 },
> >> >> + { "bq27621", (BQ27621 << 16) | BQ27421 },
> >> >
> >> > This is ugly. The proper way to do this is by providing a pointer
> >> > to a structure with all required information. E.g.:
> >> >
> >> > static const struct bq27xxx_pdata chip_info_bq27530 {
> >> > .reg_layout = ®_info_bq27530,
> >> > .feature = false,
> >> > /* more stuff */
> >> > };
> >> >
> >> > static const struct bq27xxx_pdata chip_info_bq27531 {
> >> > .reg_layout = ®_info_bq27530,
> >> > .feature = true,
> >> > /* more stuff */
> >> > };
> >> >
> >> > /* ... */
> >> >
> >> > static const struct i2c_device_id bq27xxx_i2c_id_table[] = {
> >> > /* ... */
> >> > { "bq27530", &chip_info_bq27530 },
> >> > { "bq27531", &chip_info_bq27531 },
> >> > /* ... */
> >> > }
> >>
> >> How's this...
> >>
> >> static const struct bq27xxx_chip_ids[] = {
> >> [BQ27425] = { .chip = BQ27421, .real_chip = BQ27425 },
> >> ...
> >> }
> >>
> >> static const struct i2c_device_id bq27xxx_i2c_id_table[] = {
> >> { "bq27425", &bq27xxx_chip_ids[BQ27425] },
> >> ...
> >
> > That's better, but let's get rid of real_chip. Just add the required
> > information directly to the above struct (e.g. seal_key, ramtype).
>
> I don't think the new chip data belongs in bq27*_i2c.c.
> All the (extensive) existing chip data is in the main file.
Obviously. That's how it works when you supply a chip_id instead
of a pointer to the chip_info. bq27xxx outgrew being simple enough
to just provide a chip_id.
> A later patch could rework the chip data scheme; I was trying to
> minimize changes in this one.
Trying to generate small changes is appreciated, creating hacks is
not.
> The original i2c table did a real_chip
> -> chip mapping. Now we're just carrying real_chip forward.
>
> The easiest fix would be to define a macro which does << and |
>
> { "bq27621", BQ27XXX_ID_PAIR(BQ27621, BQ27421) },
We do not want the easiest hack adding support, but a clean solution
acceptable for the mainline kernel. I consider the real_chip stuff
not very readable^W^W^W a huge mess.
-- Sebastian
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v14 09/11] power: supply: bq27xxx: Enable data memory update for certain chips
2017-07-03 16:48 ` Sebastian Reichel
@ 2017-07-04 23:24 ` Liam Breck
2017-07-05 9:47 ` Sebastian Reichel
0 siblings, 1 reply; 25+ messages in thread
From: Liam Breck @ 2017-07-04 23:24 UTC (permalink / raw)
To: Sebastian Reichel; +Cc: Pali Rohar, linux-pm, Paul Kocialkowski, Liam Breck
Hi Sebastian,
On Mon, Jul 3, 2017 at 9:48 AM, Sebastian Reichel
<sebastian.reichel@collabora.co.uk> wrote:
> ...
>> >> >> + { "bq27621", (BQ27621 << 16) | BQ27421 },
>> >> >
>> >> > This is ugly. The proper way to do this is by providing a pointer
>> >> > to a structure with all required information. E.g.:
>> >> >
>> >> > static const struct bq27xxx_pdata chip_info_bq27530 {
>> >> > .reg_layout = ®_info_bq27530,
>> >> > .feature = false,
>> >> > /* more stuff */
>> >> > };
>> >> >
>> >> > static const struct bq27xxx_pdata chip_info_bq27531 {
>> >> > .reg_layout = ®_info_bq27530,
>> >> > .feature = true,
>> >> > /* more stuff */
>> >> > };
>> >> >
>> >> > /* ... */
>> >> >
>> >> > static const struct i2c_device_id bq27xxx_i2c_id_table[] = {
>> >> > /* ... */
>> >> > { "bq27530", &chip_info_bq27530 },
>> >> > { "bq27531", &chip_info_bq27531 },
>> >> > /* ... */
>> >> > }
>> >>
>> >> How's this...
>> >>
>> >> static const struct bq27xxx_chip_ids[] = {
>> >> [BQ27425] = { .chip = BQ27421, .real_chip = BQ27425 },
>> >> ...
>> >> }
>> >>
>> >> static const struct i2c_device_id bq27xxx_i2c_id_table[] = {
>> >> { "bq27425", &bq27xxx_chip_ids[BQ27425] },
>> >> ...
>> >
>> > That's better, but let's get rid of real_chip. Just add the required
>> > information directly to the above struct (e.g. seal_key, ramtype).
>>
>> I don't think the new chip data belongs in bq27*_i2c.c.
>> All the (extensive) existing chip data is in the main file.
>
> Obviously. That's how it works when you supply a chip_id instead
> of a pointer to the chip_info. bq27xxx outgrew being simple enough
> to just provide a chip_id.
>
>> A later patch could rework the chip data scheme; I was trying to
>> minimize changes in this one.
>
> Trying to generate small changes is appreciated, creating hacks is
> not.
>
>> The original i2c table did a real_chip
>> -> chip mapping. Now we're just carrying real_chip forward.
>>
>> The easiest fix would be to define a macro which does << and |
>>
>> { "bq27621", BQ27XXX_ID_PAIR(BQ27621, BQ27421) },
>
> We do not want the easiest hack adding support, but a clean solution
> acceptable for the mainline kernel. I consider the real_chip stuff
> not very readable^W^W^W a huge mess.
OK, below is a minimal change to bq27xxx_battery.c creating a single
data table for all chips. It can easily be extended with new fields
for my patchset.
This is probably safe to upstream without testing on all the chips, as
it was mostly search/replace. However various conditionals test the
chip ID, so when adding the rest of the IDs (previously only in
real_chip) in the next patch, I'd have to touch that code, but I have
no way of testing all the affected parts. I could use .acts_like =
OTHER_ID in the data table to minimize that risk. Thoughts?
From: Liam Breck <kernel@networkimprov.net>
Subject: [PATCH] power: supply: bq27xxx: create single chip data table
Unify data previously in bq27xxx_regs & bq27xxx_battery_props into a
single table.
There is no functional change to the driver.
---
drivers/power/supply/bq27xxx_battery.c | 121 ++++++++++++++++-----------------
1 file changed, 60 insertions(+), 61 deletions(-)
diff --git a/drivers/power/supply/bq27xxx_battery.c
b/drivers/power/supply/bq27xxx_battery.c
index ed44439d0112c..db42e4961c68f 100644
--- a/drivers/power/supply/bq27xxx_battery.c
+++ b/drivers/power/supply/bq27xxx_battery.c
@@ -132,8 +132,8 @@ enum bq27xxx_reg_index {
[BQ27XXX_DM_CKSUM] = 0x60
/* Register mappings */
-static u8 bq27xxx_regs[][BQ27XXX_REG_MAX] = {
- [BQ27000] = {
+static u8
+ bq27000_regs[BQ27XXX_REG_MAX] = {
[BQ27XXX_REG_CTRL] = 0x00,
[BQ27XXX_REG_TEMP] = 0x06,
[BQ27XXX_REG_INT_TEMP] = INVALID_REG_ADDR,
@@ -157,7 +157,7 @@ static u8 bq27xxx_regs[][BQ27XXX_REG_MAX] = {
[BQ27XXX_DM_DATA] = INVALID_REG_ADDR,
[BQ27XXX_DM_CKSUM] = INVALID_REG_ADDR,
},
- [BQ27010] = {
+ bq27010_regs[BQ27XXX_REG_MAX] = {
[BQ27XXX_REG_CTRL] = 0x00,
[BQ27XXX_REG_TEMP] = 0x06,
[BQ27XXX_REG_INT_TEMP] = INVALID_REG_ADDR,
@@ -181,7 +181,7 @@ static u8 bq27xxx_regs[][BQ27XXX_REG_MAX] = {
[BQ27XXX_DM_DATA] = INVALID_REG_ADDR,
[BQ27XXX_DM_CKSUM] = INVALID_REG_ADDR,
},
- [BQ2750X] = {
+ bq2750x_regs[BQ27XXX_REG_MAX] = {
[BQ27XXX_REG_CTRL] = 0x00,
[BQ27XXX_REG_TEMP] = 0x06,
[BQ27XXX_REG_INT_TEMP] = 0x28,
@@ -201,7 +201,7 @@ static u8 bq27xxx_regs[][BQ27XXX_REG_MAX] = {
[BQ27XXX_REG_AP] = INVALID_REG_ADDR,
BQ27XXX_DM_REG_ROWS,
},
- [BQ2751X] = {
+ bq2751x_regs[BQ27XXX_REG_MAX] = {
[BQ27XXX_REG_CTRL] = 0x00,
[BQ27XXX_REG_TEMP] = 0x06,
[BQ27XXX_REG_INT_TEMP] = 0x28,
@@ -221,7 +221,7 @@ static u8 bq27xxx_regs[][BQ27XXX_REG_MAX] = {
[BQ27XXX_REG_AP] = INVALID_REG_ADDR,
BQ27XXX_DM_REG_ROWS,
},
- [BQ27500] = {
+ bq27500_regs[BQ27XXX_REG_MAX] = {
[BQ27XXX_REG_CTRL] = 0x00,
[BQ27XXX_REG_TEMP] = 0x06,
[BQ27XXX_REG_INT_TEMP] = INVALID_REG_ADDR,
@@ -241,7 +241,7 @@ static u8 bq27xxx_regs[][BQ27XXX_REG_MAX] = {
[BQ27XXX_REG_AP] = 0x24,
BQ27XXX_DM_REG_ROWS,
},
- [BQ27510G1] = {
+ bq27510g1_regs[BQ27XXX_REG_MAX] = {
[BQ27XXX_REG_CTRL] = 0x00,
[BQ27XXX_REG_TEMP] = 0x06,
[BQ27XXX_REG_INT_TEMP] = INVALID_REG_ADDR,
@@ -261,7 +261,7 @@ static u8 bq27xxx_regs[][BQ27XXX_REG_MAX] = {
[BQ27XXX_REG_AP] = 0x24,
BQ27XXX_DM_REG_ROWS,
},
- [BQ27510G2] = {
+ bq27510g2_regs[BQ27XXX_REG_MAX] = {
[BQ27XXX_REG_CTRL] = 0x00,
[BQ27XXX_REG_TEMP] = 0x06,
[BQ27XXX_REG_INT_TEMP] = INVALID_REG_ADDR,
@@ -281,7 +281,7 @@ static u8 bq27xxx_regs[][BQ27XXX_REG_MAX] = {
[BQ27XXX_REG_AP] = 0x24,
BQ27XXX_DM_REG_ROWS,
},
- [BQ27510G3] = {
+ bq27510g3_regs[BQ27XXX_REG_MAX] = {
[BQ27XXX_REG_CTRL] = 0x00,
[BQ27XXX_REG_TEMP] = 0x06,
[BQ27XXX_REG_INT_TEMP] = 0x28,
@@ -301,7 +301,7 @@ static u8 bq27xxx_regs[][BQ27XXX_REG_MAX] = {
[BQ27XXX_REG_AP] = INVALID_REG_ADDR,
BQ27XXX_DM_REG_ROWS,
},
- [BQ27520G1] = {
+ bq27520g1_regs[BQ27XXX_REG_MAX] = {
[BQ27XXX_REG_CTRL] = 0x00,
[BQ27XXX_REG_TEMP] = 0x06,
[BQ27XXX_REG_INT_TEMP] = INVALID_REG_ADDR,
@@ -321,7 +321,7 @@ static u8 bq27xxx_regs[][BQ27XXX_REG_MAX] = {
[BQ27XXX_REG_AP] = 0x24,
BQ27XXX_DM_REG_ROWS,
},
- [BQ27520G2] = {
+ bq27520g2_regs[BQ27XXX_REG_MAX] = {
[BQ27XXX_REG_CTRL] = 0x00,
[BQ27XXX_REG_TEMP] = 0x06,
[BQ27XXX_REG_INT_TEMP] = 0x36,
@@ -341,7 +341,7 @@ static u8 bq27xxx_regs[][BQ27XXX_REG_MAX] = {
[BQ27XXX_REG_AP] = 0x24,
BQ27XXX_DM_REG_ROWS,
},
- [BQ27520G3] = {
+ bq27520g3_regs[BQ27XXX_REG_MAX] = {
[BQ27XXX_REG_CTRL] = 0x00,
[BQ27XXX_REG_TEMP] = 0x06,
[BQ27XXX_REG_INT_TEMP] = 0x36,
@@ -361,7 +361,7 @@ static u8 bq27xxx_regs[][BQ27XXX_REG_MAX] = {
[BQ27XXX_REG_AP] = 0x24,
BQ27XXX_DM_REG_ROWS,
},
- [BQ27520G4] = {
+ bq27520g4_regs[BQ27XXX_REG_MAX] = {
[BQ27XXX_REG_CTRL] = 0x00,
[BQ27XXX_REG_TEMP] = 0x06,
[BQ27XXX_REG_INT_TEMP] = 0x28,
@@ -381,7 +381,7 @@ static u8 bq27xxx_regs[][BQ27XXX_REG_MAX] = {
[BQ27XXX_REG_AP] = INVALID_REG_ADDR,
BQ27XXX_DM_REG_ROWS,
},
- [BQ27530] = {
+ bq27530_regs[BQ27XXX_REG_MAX] = {
[BQ27XXX_REG_CTRL] = 0x00,
[BQ27XXX_REG_TEMP] = 0x06,
[BQ27XXX_REG_INT_TEMP] = 0x32,
@@ -401,7 +401,7 @@ static u8 bq27xxx_regs[][BQ27XXX_REG_MAX] = {
[BQ27XXX_REG_AP] = 0x24,
BQ27XXX_DM_REG_ROWS,
},
- [BQ27541] = {
+ bq27541_regs[BQ27XXX_REG_MAX] = {
[BQ27XXX_REG_CTRL] = 0x00,
[BQ27XXX_REG_TEMP] = 0x06,
[BQ27XXX_REG_INT_TEMP] = 0x28,
@@ -421,7 +421,7 @@ static u8 bq27xxx_regs[][BQ27XXX_REG_MAX] = {
[BQ27XXX_REG_AP] = 0x24,
BQ27XXX_DM_REG_ROWS,
},
- [BQ27545] = {
+ bq27545_regs[BQ27XXX_REG_MAX] = {
[BQ27XXX_REG_CTRL] = 0x00,
[BQ27XXX_REG_TEMP] = 0x06,
[BQ27XXX_REG_INT_TEMP] = 0x28,
@@ -441,7 +441,7 @@ static u8 bq27xxx_regs[][BQ27XXX_REG_MAX] = {
[BQ27XXX_REG_AP] = 0x24,
BQ27XXX_DM_REG_ROWS,
},
- [BQ27421] = {
+ bq27421_regs[BQ27XXX_REG_MAX] = {
[BQ27XXX_REG_CTRL] = 0x00,
[BQ27XXX_REG_TEMP] = 0x02,
[BQ27XXX_REG_INT_TEMP] = 0x1e,
@@ -460,10 +460,9 @@ static u8 bq27xxx_regs[][BQ27XXX_REG_MAX] = {
[BQ27XXX_REG_DCAP] = 0x3c,
[BQ27XXX_REG_AP] = 0x18,
BQ27XXX_DM_REG_ROWS,
- },
-};
+ };
-static enum power_supply_property bq27000_battery_props[] = {
+static enum power_supply_property bq27000_props[] = {
POWER_SUPPLY_PROP_STATUS,
POWER_SUPPLY_PROP_PRESENT,
POWER_SUPPLY_PROP_VOLTAGE_NOW,
@@ -485,7 +484,7 @@ static enum power_supply_property
bq27000_battery_props[] = {
POWER_SUPPLY_PROP_MANUFACTURER,
};
-static enum power_supply_property bq27010_battery_props[] = {
+static enum power_supply_property bq27010_props[] = {
POWER_SUPPLY_PROP_STATUS,
POWER_SUPPLY_PROP_PRESENT,
POWER_SUPPLY_PROP_VOLTAGE_NOW,
@@ -505,7 +504,7 @@ static enum power_supply_property
bq27010_battery_props[] = {
POWER_SUPPLY_PROP_MANUFACTURER,
};
-static enum power_supply_property bq2750x_battery_props[] = {
+static enum power_supply_property bq2750x_props[] = {
POWER_SUPPLY_PROP_STATUS,
POWER_SUPPLY_PROP_PRESENT,
POWER_SUPPLY_PROP_VOLTAGE_NOW,
@@ -523,7 +522,7 @@ static enum power_supply_property
bq2750x_battery_props[] = {
POWER_SUPPLY_PROP_MANUFACTURER,
};
-static enum power_supply_property bq2751x_battery_props[] = {
+static enum power_supply_property bq2751x_props[] = {
POWER_SUPPLY_PROP_STATUS,
POWER_SUPPLY_PROP_PRESENT,
POWER_SUPPLY_PROP_VOLTAGE_NOW,
@@ -541,7 +540,7 @@ static enum power_supply_property
bq2751x_battery_props[] = {
POWER_SUPPLY_PROP_MANUFACTURER,
};
-static enum power_supply_property bq27500_battery_props[] = {
+static enum power_supply_property bq27500_props[] = {
POWER_SUPPLY_PROP_STATUS,
POWER_SUPPLY_PROP_PRESENT,
POWER_SUPPLY_PROP_VOLTAGE_NOW,
@@ -562,7 +561,7 @@ static enum power_supply_property
bq27500_battery_props[] = {
POWER_SUPPLY_PROP_MANUFACTURER,
};
-static enum power_supply_property bq27510g1_battery_props[] = {
+static enum power_supply_property bq27510g1_props[] = {
POWER_SUPPLY_PROP_STATUS,
POWER_SUPPLY_PROP_PRESENT,
POWER_SUPPLY_PROP_VOLTAGE_NOW,
@@ -583,7 +582,7 @@ static enum power_supply_property
bq27510g1_battery_props[] = {
POWER_SUPPLY_PROP_MANUFACTURER,
};
-static enum power_supply_property bq27510g2_battery_props[] = {
+static enum power_supply_property bq27510g2_props[] = {
POWER_SUPPLY_PROP_STATUS,
POWER_SUPPLY_PROP_PRESENT,
POWER_SUPPLY_PROP_VOLTAGE_NOW,
@@ -604,7 +603,7 @@ static enum power_supply_property
bq27510g2_battery_props[] = {
POWER_SUPPLY_PROP_MANUFACTURER,
};
-static enum power_supply_property bq27510g3_battery_props[] = {
+static enum power_supply_property bq27510g3_props[] = {
POWER_SUPPLY_PROP_STATUS,
POWER_SUPPLY_PROP_PRESENT,
POWER_SUPPLY_PROP_VOLTAGE_NOW,
@@ -622,7 +621,7 @@ static enum power_supply_property
bq27510g3_battery_props[] = {
POWER_SUPPLY_PROP_MANUFACTURER,
};
-static enum power_supply_property bq27520g1_battery_props[] = {
+static enum power_supply_property bq27520g1_props[] = {
POWER_SUPPLY_PROP_STATUS,
POWER_SUPPLY_PROP_PRESENT,
POWER_SUPPLY_PROP_VOLTAGE_NOW,
@@ -642,7 +641,7 @@ static enum power_supply_property
bq27520g1_battery_props[] = {
POWER_SUPPLY_PROP_MANUFACTURER,
};
-static enum power_supply_property bq27520g2_battery_props[] = {
+static enum power_supply_property bq27520g2_props[] = {
POWER_SUPPLY_PROP_STATUS,
POWER_SUPPLY_PROP_PRESENT,
POWER_SUPPLY_PROP_VOLTAGE_NOW,
@@ -663,7 +662,7 @@ static enum power_supply_property
bq27520g2_battery_props[] = {
POWER_SUPPLY_PROP_MANUFACTURER,
};
-static enum power_supply_property bq27520g3_battery_props[] = {
+static enum power_supply_property bq27520g3_props[] = {
POWER_SUPPLY_PROP_STATUS,
POWER_SUPPLY_PROP_PRESENT,
POWER_SUPPLY_PROP_VOLTAGE_NOW,
@@ -683,7 +682,7 @@ static enum power_supply_property
bq27520g3_battery_props[] = {
POWER_SUPPLY_PROP_MANUFACTURER,
};
-static enum power_supply_property bq27520g4_battery_props[] = {
+static enum power_supply_property bq27520g4_props[] = {
POWER_SUPPLY_PROP_STATUS,
POWER_SUPPLY_PROP_PRESENT,
POWER_SUPPLY_PROP_VOLTAGE_NOW,
@@ -700,7 +699,7 @@ static enum power_supply_property
bq27520g4_battery_props[] = {
POWER_SUPPLY_PROP_MANUFACTURER,
};
-static enum power_supply_property bq27530_battery_props[] = {
+static enum power_supply_property bq27530_props[] = {
POWER_SUPPLY_PROP_STATUS,
POWER_SUPPLY_PROP_PRESENT,
POWER_SUPPLY_PROP_VOLTAGE_NOW,
@@ -718,7 +717,7 @@ static enum power_supply_property
bq27530_battery_props[] = {
POWER_SUPPLY_PROP_MANUFACTURER,
};
-static enum power_supply_property bq27541_battery_props[] = {
+static enum power_supply_property bq27541_props[] = {
POWER_SUPPLY_PROP_STATUS,
POWER_SUPPLY_PROP_PRESENT,
POWER_SUPPLY_PROP_VOLTAGE_NOW,
@@ -737,7 +736,7 @@ static enum power_supply_property
bq27541_battery_props[] = {
POWER_SUPPLY_PROP_MANUFACTURER,
};
-static enum power_supply_property bq27545_battery_props[] = {
+static enum power_supply_property bq27545_props[] = {
POWER_SUPPLY_PROP_STATUS,
POWER_SUPPLY_PROP_PRESENT,
POWER_SUPPLY_PROP_VOLTAGE_NOW,
@@ -755,7 +754,7 @@ static enum power_supply_property
bq27545_battery_props[] = {
POWER_SUPPLY_PROP_MANUFACTURER,
};
-static enum power_supply_property bq27421_battery_props[] = {
+static enum power_supply_property bq27421_props[] = {
POWER_SUPPLY_PROP_STATUS,
POWER_SUPPLY_PROP_PRESENT,
POWER_SUPPLY_PROP_VOLTAGE_NOW,
@@ -770,32 +769,32 @@ static enum power_supply_property
bq27421_battery_props[] = {
POWER_SUPPLY_PROP_MANUFACTURER,
};
-#define BQ27XXX_PROP(_id, _prop) \
- [_id] = { \
- .props = _prop, \
- .size = ARRAY_SIZE(_prop), \
- }
+#define BQ27XXX_DATA(ref) { \
+ .regs = ref##_regs, \
+ .props = ref##_props, \
+ .props_size = ARRAY_SIZE(ref##_props), }
static struct {
+ u8 *regs;
enum power_supply_property *props;
- size_t size;
-} bq27xxx_battery_props[] = {
- BQ27XXX_PROP(BQ27000, bq27000_battery_props),
- BQ27XXX_PROP(BQ27010, bq27010_battery_props),
- BQ27XXX_PROP(BQ2750X, bq2750x_battery_props),
- BQ27XXX_PROP(BQ2751X, bq2751x_battery_props),
- BQ27XXX_PROP(BQ27500, bq27500_battery_props),
- BQ27XXX_PROP(BQ27510G1, bq27510g1_battery_props),
- BQ27XXX_PROP(BQ27510G2, bq27510g2_battery_props),
- BQ27XXX_PROP(BQ27510G3, bq27510g3_battery_props),
- BQ27XXX_PROP(BQ27520G1, bq27520g1_battery_props),
- BQ27XXX_PROP(BQ27520G2, bq27520g2_battery_props),
- BQ27XXX_PROP(BQ27520G3, bq27520g3_battery_props),
- BQ27XXX_PROP(BQ27520G4, bq27520g4_battery_props),
- BQ27XXX_PROP(BQ27530, bq27530_battery_props),
- BQ27XXX_PROP(BQ27541, bq27541_battery_props),
- BQ27XXX_PROP(BQ27545, bq27545_battery_props),
- BQ27XXX_PROP(BQ27421, bq27421_battery_props),
+ size_t props_size;
+} bq27xxx_battery_data[] = {
+ [BQ27000] = BQ27XXX_DATA(bq27000),
+ [BQ27010] = BQ27XXX_DATA(bq27010),
+ [BQ2750X] = BQ27XXX_DATA(bq2750x),
+ [BQ2751X] = BQ27XXX_DATA(bq2751x),
+ [BQ27500] = BQ27XXX_DATA(bq27500),
+ [BQ27510G1] = BQ27XXX_DATA(bq27510g1),
+ [BQ27510G2] = BQ27XXX_DATA(bq27510g2),
+ [BQ27510G3] = BQ27XXX_DATA(bq27510g3),
+ [BQ27520G1] = BQ27XXX_DATA(bq27520g1),
+ [BQ27520G2] = BQ27XXX_DATA(bq27520g2),
+ [BQ27520G3] = BQ27XXX_DATA(bq27520g3),
+ [BQ27520G4] = BQ27XXX_DATA(bq27520g4),
+ [BQ27530] = BQ27XXX_DATA(bq27530),
+ [BQ27541] = BQ27XXX_DATA(bq27541),
+ [BQ27545] = BQ27XXX_DATA(bq27545),
+ [BQ27421] = BQ27XXX_DATA(bq27421),
};
static DEFINE_MUTEX(bq27xxx_list_lock);
@@ -1884,7 +1883,7 @@ int bq27xxx_battery_setup(struct bq27xxx_device_info *di)
INIT_DELAYED_WORK(&di->work, bq27xxx_battery_poll);
mutex_init(&di->lock);
- di->regs = bq27xxx_regs[di->chip];
+ di->regs = bq27xxx_battery_data[di->chip].regs;
psy_desc = devm_kzalloc(di->dev, sizeof(*psy_desc), GFP_KERNEL);
if (!psy_desc)
@@ -1892,8 +1891,8 @@ int bq27xxx_battery_setup(struct bq27xxx_device_info *di)
psy_desc->name = di->name;
psy_desc->type = POWER_SUPPLY_TYPE_BATTERY;
- psy_desc->properties = bq27xxx_battery_props[di->chip].props;
- psy_desc->num_properties = bq27xxx_battery_props[di->chip].size;
+ psy_desc->properties = bq27xxx_battery_data[di->chip].props;
+ psy_desc->num_properties = bq27xxx_battery_data[di->chip].props_size;
psy_desc->get_property = bq27xxx_battery_get_property;
psy_desc->external_power_changed = bq27xxx_external_power_changed;
^ permalink raw reply related [flat|nested] 25+ messages in thread
* Re: [PATCH v14 09/11] power: supply: bq27xxx: Enable data memory update for certain chips
2017-07-04 23:24 ` Liam Breck
@ 2017-07-05 9:47 ` Sebastian Reichel
2017-07-05 17:49 ` Liam Breck
0 siblings, 1 reply; 25+ messages in thread
From: Sebastian Reichel @ 2017-07-05 9:47 UTC (permalink / raw)
To: Liam Breck; +Cc: Pali Rohar, linux-pm, Paul Kocialkowski, Liam Breck
[-- Attachment #1: Type: text/plain, Size: 16710 bytes --]
Hi,
On Tue, Jul 04, 2017 at 04:24:05PM -0700, Liam Breck wrote:
> OK, below is a minimal change to bq27xxx_battery.c creating a single
> data table for all chips. It can easily be extended with new fields
> for my patchset.
Patch looks good.
> This is probably safe to upstream without testing on all the chips, as
> it was mostly search/replace. However various conditionals test the
> chip ID, so when adding the rest of the IDs (previously only in
> real_chip) in the next patch, I'd have to touch that code, but I have
> no way of testing all the affected parts. I could use .acts_like =
> OTHER_ID in the data table to minimize that risk. Thoughts?
As far as I can see there are only a couple of different things
di->chip is used for:
di->chip == BQ27421 => flag_cfgupdate,
di->chip == BQ27000 || di->chip == BQ27010 => flag_single,
di->chip == BQ27530 || di->chip == BQ27421 => flag_temp_ot_ut,
If we add those flags to bq27xxx_battery_data we do not need
to check di->chip in the driver at all.
-- Sebastian
> From: Liam Breck <kernel@networkimprov.net>
> Subject: [PATCH] power: supply: bq27xxx: create single chip data table
>
> Unify data previously in bq27xxx_regs & bq27xxx_battery_props into a
> single table.
> There is no functional change to the driver.
>
> ---
> drivers/power/supply/bq27xxx_battery.c | 121 ++++++++++++++++-----------------
> 1 file changed, 60 insertions(+), 61 deletions(-)
>
> diff --git a/drivers/power/supply/bq27xxx_battery.c
> b/drivers/power/supply/bq27xxx_battery.c
> index ed44439d0112c..db42e4961c68f 100644
> --- a/drivers/power/supply/bq27xxx_battery.c
> +++ b/drivers/power/supply/bq27xxx_battery.c
> @@ -132,8 +132,8 @@ enum bq27xxx_reg_index {
> [BQ27XXX_DM_CKSUM] = 0x60
>
> /* Register mappings */
> -static u8 bq27xxx_regs[][BQ27XXX_REG_MAX] = {
> - [BQ27000] = {
> +static u8
> + bq27000_regs[BQ27XXX_REG_MAX] = {
> [BQ27XXX_REG_CTRL] = 0x00,
> [BQ27XXX_REG_TEMP] = 0x06,
> [BQ27XXX_REG_INT_TEMP] = INVALID_REG_ADDR,
> @@ -157,7 +157,7 @@ static u8 bq27xxx_regs[][BQ27XXX_REG_MAX] = {
> [BQ27XXX_DM_DATA] = INVALID_REG_ADDR,
> [BQ27XXX_DM_CKSUM] = INVALID_REG_ADDR,
> },
> - [BQ27010] = {
> + bq27010_regs[BQ27XXX_REG_MAX] = {
> [BQ27XXX_REG_CTRL] = 0x00,
> [BQ27XXX_REG_TEMP] = 0x06,
> [BQ27XXX_REG_INT_TEMP] = INVALID_REG_ADDR,
> @@ -181,7 +181,7 @@ static u8 bq27xxx_regs[][BQ27XXX_REG_MAX] = {
> [BQ27XXX_DM_DATA] = INVALID_REG_ADDR,
> [BQ27XXX_DM_CKSUM] = INVALID_REG_ADDR,
> },
> - [BQ2750X] = {
> + bq2750x_regs[BQ27XXX_REG_MAX] = {
> [BQ27XXX_REG_CTRL] = 0x00,
> [BQ27XXX_REG_TEMP] = 0x06,
> [BQ27XXX_REG_INT_TEMP] = 0x28,
> @@ -201,7 +201,7 @@ static u8 bq27xxx_regs[][BQ27XXX_REG_MAX] = {
> [BQ27XXX_REG_AP] = INVALID_REG_ADDR,
> BQ27XXX_DM_REG_ROWS,
> },
> - [BQ2751X] = {
> + bq2751x_regs[BQ27XXX_REG_MAX] = {
> [BQ27XXX_REG_CTRL] = 0x00,
> [BQ27XXX_REG_TEMP] = 0x06,
> [BQ27XXX_REG_INT_TEMP] = 0x28,
> @@ -221,7 +221,7 @@ static u8 bq27xxx_regs[][BQ27XXX_REG_MAX] = {
> [BQ27XXX_REG_AP] = INVALID_REG_ADDR,
> BQ27XXX_DM_REG_ROWS,
> },
> - [BQ27500] = {
> + bq27500_regs[BQ27XXX_REG_MAX] = {
> [BQ27XXX_REG_CTRL] = 0x00,
> [BQ27XXX_REG_TEMP] = 0x06,
> [BQ27XXX_REG_INT_TEMP] = INVALID_REG_ADDR,
> @@ -241,7 +241,7 @@ static u8 bq27xxx_regs[][BQ27XXX_REG_MAX] = {
> [BQ27XXX_REG_AP] = 0x24,
> BQ27XXX_DM_REG_ROWS,
> },
> - [BQ27510G1] = {
> + bq27510g1_regs[BQ27XXX_REG_MAX] = {
> [BQ27XXX_REG_CTRL] = 0x00,
> [BQ27XXX_REG_TEMP] = 0x06,
> [BQ27XXX_REG_INT_TEMP] = INVALID_REG_ADDR,
> @@ -261,7 +261,7 @@ static u8 bq27xxx_regs[][BQ27XXX_REG_MAX] = {
> [BQ27XXX_REG_AP] = 0x24,
> BQ27XXX_DM_REG_ROWS,
> },
> - [BQ27510G2] = {
> + bq27510g2_regs[BQ27XXX_REG_MAX] = {
> [BQ27XXX_REG_CTRL] = 0x00,
> [BQ27XXX_REG_TEMP] = 0x06,
> [BQ27XXX_REG_INT_TEMP] = INVALID_REG_ADDR,
> @@ -281,7 +281,7 @@ static u8 bq27xxx_regs[][BQ27XXX_REG_MAX] = {
> [BQ27XXX_REG_AP] = 0x24,
> BQ27XXX_DM_REG_ROWS,
> },
> - [BQ27510G3] = {
> + bq27510g3_regs[BQ27XXX_REG_MAX] = {
> [BQ27XXX_REG_CTRL] = 0x00,
> [BQ27XXX_REG_TEMP] = 0x06,
> [BQ27XXX_REG_INT_TEMP] = 0x28,
> @@ -301,7 +301,7 @@ static u8 bq27xxx_regs[][BQ27XXX_REG_MAX] = {
> [BQ27XXX_REG_AP] = INVALID_REG_ADDR,
> BQ27XXX_DM_REG_ROWS,
> },
> - [BQ27520G1] = {
> + bq27520g1_regs[BQ27XXX_REG_MAX] = {
> [BQ27XXX_REG_CTRL] = 0x00,
> [BQ27XXX_REG_TEMP] = 0x06,
> [BQ27XXX_REG_INT_TEMP] = INVALID_REG_ADDR,
> @@ -321,7 +321,7 @@ static u8 bq27xxx_regs[][BQ27XXX_REG_MAX] = {
> [BQ27XXX_REG_AP] = 0x24,
> BQ27XXX_DM_REG_ROWS,
> },
> - [BQ27520G2] = {
> + bq27520g2_regs[BQ27XXX_REG_MAX] = {
> [BQ27XXX_REG_CTRL] = 0x00,
> [BQ27XXX_REG_TEMP] = 0x06,
> [BQ27XXX_REG_INT_TEMP] = 0x36,
> @@ -341,7 +341,7 @@ static u8 bq27xxx_regs[][BQ27XXX_REG_MAX] = {
> [BQ27XXX_REG_AP] = 0x24,
> BQ27XXX_DM_REG_ROWS,
> },
> - [BQ27520G3] = {
> + bq27520g3_regs[BQ27XXX_REG_MAX] = {
> [BQ27XXX_REG_CTRL] = 0x00,
> [BQ27XXX_REG_TEMP] = 0x06,
> [BQ27XXX_REG_INT_TEMP] = 0x36,
> @@ -361,7 +361,7 @@ static u8 bq27xxx_regs[][BQ27XXX_REG_MAX] = {
> [BQ27XXX_REG_AP] = 0x24,
> BQ27XXX_DM_REG_ROWS,
> },
> - [BQ27520G4] = {
> + bq27520g4_regs[BQ27XXX_REG_MAX] = {
> [BQ27XXX_REG_CTRL] = 0x00,
> [BQ27XXX_REG_TEMP] = 0x06,
> [BQ27XXX_REG_INT_TEMP] = 0x28,
> @@ -381,7 +381,7 @@ static u8 bq27xxx_regs[][BQ27XXX_REG_MAX] = {
> [BQ27XXX_REG_AP] = INVALID_REG_ADDR,
> BQ27XXX_DM_REG_ROWS,
> },
> - [BQ27530] = {
> + bq27530_regs[BQ27XXX_REG_MAX] = {
> [BQ27XXX_REG_CTRL] = 0x00,
> [BQ27XXX_REG_TEMP] = 0x06,
> [BQ27XXX_REG_INT_TEMP] = 0x32,
> @@ -401,7 +401,7 @@ static u8 bq27xxx_regs[][BQ27XXX_REG_MAX] = {
> [BQ27XXX_REG_AP] = 0x24,
> BQ27XXX_DM_REG_ROWS,
> },
> - [BQ27541] = {
> + bq27541_regs[BQ27XXX_REG_MAX] = {
> [BQ27XXX_REG_CTRL] = 0x00,
> [BQ27XXX_REG_TEMP] = 0x06,
> [BQ27XXX_REG_INT_TEMP] = 0x28,
> @@ -421,7 +421,7 @@ static u8 bq27xxx_regs[][BQ27XXX_REG_MAX] = {
> [BQ27XXX_REG_AP] = 0x24,
> BQ27XXX_DM_REG_ROWS,
> },
> - [BQ27545] = {
> + bq27545_regs[BQ27XXX_REG_MAX] = {
> [BQ27XXX_REG_CTRL] = 0x00,
> [BQ27XXX_REG_TEMP] = 0x06,
> [BQ27XXX_REG_INT_TEMP] = 0x28,
> @@ -441,7 +441,7 @@ static u8 bq27xxx_regs[][BQ27XXX_REG_MAX] = {
> [BQ27XXX_REG_AP] = 0x24,
> BQ27XXX_DM_REG_ROWS,
> },
> - [BQ27421] = {
> + bq27421_regs[BQ27XXX_REG_MAX] = {
> [BQ27XXX_REG_CTRL] = 0x00,
> [BQ27XXX_REG_TEMP] = 0x02,
> [BQ27XXX_REG_INT_TEMP] = 0x1e,
> @@ -460,10 +460,9 @@ static u8 bq27xxx_regs[][BQ27XXX_REG_MAX] = {
> [BQ27XXX_REG_DCAP] = 0x3c,
> [BQ27XXX_REG_AP] = 0x18,
> BQ27XXX_DM_REG_ROWS,
> - },
> -};
> + };
>
> -static enum power_supply_property bq27000_battery_props[] = {
> +static enum power_supply_property bq27000_props[] = {
> POWER_SUPPLY_PROP_STATUS,
> POWER_SUPPLY_PROP_PRESENT,
> POWER_SUPPLY_PROP_VOLTAGE_NOW,
> @@ -485,7 +484,7 @@ static enum power_supply_property
> bq27000_battery_props[] = {
> POWER_SUPPLY_PROP_MANUFACTURER,
> };
>
> -static enum power_supply_property bq27010_battery_props[] = {
> +static enum power_supply_property bq27010_props[] = {
> POWER_SUPPLY_PROP_STATUS,
> POWER_SUPPLY_PROP_PRESENT,
> POWER_SUPPLY_PROP_VOLTAGE_NOW,
> @@ -505,7 +504,7 @@ static enum power_supply_property
> bq27010_battery_props[] = {
> POWER_SUPPLY_PROP_MANUFACTURER,
> };
>
> -static enum power_supply_property bq2750x_battery_props[] = {
> +static enum power_supply_property bq2750x_props[] = {
> POWER_SUPPLY_PROP_STATUS,
> POWER_SUPPLY_PROP_PRESENT,
> POWER_SUPPLY_PROP_VOLTAGE_NOW,
> @@ -523,7 +522,7 @@ static enum power_supply_property
> bq2750x_battery_props[] = {
> POWER_SUPPLY_PROP_MANUFACTURER,
> };
>
> -static enum power_supply_property bq2751x_battery_props[] = {
> +static enum power_supply_property bq2751x_props[] = {
> POWER_SUPPLY_PROP_STATUS,
> POWER_SUPPLY_PROP_PRESENT,
> POWER_SUPPLY_PROP_VOLTAGE_NOW,
> @@ -541,7 +540,7 @@ static enum power_supply_property
> bq2751x_battery_props[] = {
> POWER_SUPPLY_PROP_MANUFACTURER,
> };
>
> -static enum power_supply_property bq27500_battery_props[] = {
> +static enum power_supply_property bq27500_props[] = {
> POWER_SUPPLY_PROP_STATUS,
> POWER_SUPPLY_PROP_PRESENT,
> POWER_SUPPLY_PROP_VOLTAGE_NOW,
> @@ -562,7 +561,7 @@ static enum power_supply_property
> bq27500_battery_props[] = {
> POWER_SUPPLY_PROP_MANUFACTURER,
> };
>
> -static enum power_supply_property bq27510g1_battery_props[] = {
> +static enum power_supply_property bq27510g1_props[] = {
> POWER_SUPPLY_PROP_STATUS,
> POWER_SUPPLY_PROP_PRESENT,
> POWER_SUPPLY_PROP_VOLTAGE_NOW,
> @@ -583,7 +582,7 @@ static enum power_supply_property
> bq27510g1_battery_props[] = {
> POWER_SUPPLY_PROP_MANUFACTURER,
> };
>
> -static enum power_supply_property bq27510g2_battery_props[] = {
> +static enum power_supply_property bq27510g2_props[] = {
> POWER_SUPPLY_PROP_STATUS,
> POWER_SUPPLY_PROP_PRESENT,
> POWER_SUPPLY_PROP_VOLTAGE_NOW,
> @@ -604,7 +603,7 @@ static enum power_supply_property
> bq27510g2_battery_props[] = {
> POWER_SUPPLY_PROP_MANUFACTURER,
> };
>
> -static enum power_supply_property bq27510g3_battery_props[] = {
> +static enum power_supply_property bq27510g3_props[] = {
> POWER_SUPPLY_PROP_STATUS,
> POWER_SUPPLY_PROP_PRESENT,
> POWER_SUPPLY_PROP_VOLTAGE_NOW,
> @@ -622,7 +621,7 @@ static enum power_supply_property
> bq27510g3_battery_props[] = {
> POWER_SUPPLY_PROP_MANUFACTURER,
> };
>
> -static enum power_supply_property bq27520g1_battery_props[] = {
> +static enum power_supply_property bq27520g1_props[] = {
> POWER_SUPPLY_PROP_STATUS,
> POWER_SUPPLY_PROP_PRESENT,
> POWER_SUPPLY_PROP_VOLTAGE_NOW,
> @@ -642,7 +641,7 @@ static enum power_supply_property
> bq27520g1_battery_props[] = {
> POWER_SUPPLY_PROP_MANUFACTURER,
> };
>
> -static enum power_supply_property bq27520g2_battery_props[] = {
> +static enum power_supply_property bq27520g2_props[] = {
> POWER_SUPPLY_PROP_STATUS,
> POWER_SUPPLY_PROP_PRESENT,
> POWER_SUPPLY_PROP_VOLTAGE_NOW,
> @@ -663,7 +662,7 @@ static enum power_supply_property
> bq27520g2_battery_props[] = {
> POWER_SUPPLY_PROP_MANUFACTURER,
> };
>
> -static enum power_supply_property bq27520g3_battery_props[] = {
> +static enum power_supply_property bq27520g3_props[] = {
> POWER_SUPPLY_PROP_STATUS,
> POWER_SUPPLY_PROP_PRESENT,
> POWER_SUPPLY_PROP_VOLTAGE_NOW,
> @@ -683,7 +682,7 @@ static enum power_supply_property
> bq27520g3_battery_props[] = {
> POWER_SUPPLY_PROP_MANUFACTURER,
> };
>
> -static enum power_supply_property bq27520g4_battery_props[] = {
> +static enum power_supply_property bq27520g4_props[] = {
> POWER_SUPPLY_PROP_STATUS,
> POWER_SUPPLY_PROP_PRESENT,
> POWER_SUPPLY_PROP_VOLTAGE_NOW,
> @@ -700,7 +699,7 @@ static enum power_supply_property
> bq27520g4_battery_props[] = {
> POWER_SUPPLY_PROP_MANUFACTURER,
> };
>
> -static enum power_supply_property bq27530_battery_props[] = {
> +static enum power_supply_property bq27530_props[] = {
> POWER_SUPPLY_PROP_STATUS,
> POWER_SUPPLY_PROP_PRESENT,
> POWER_SUPPLY_PROP_VOLTAGE_NOW,
> @@ -718,7 +717,7 @@ static enum power_supply_property
> bq27530_battery_props[] = {
> POWER_SUPPLY_PROP_MANUFACTURER,
> };
>
> -static enum power_supply_property bq27541_battery_props[] = {
> +static enum power_supply_property bq27541_props[] = {
> POWER_SUPPLY_PROP_STATUS,
> POWER_SUPPLY_PROP_PRESENT,
> POWER_SUPPLY_PROP_VOLTAGE_NOW,
> @@ -737,7 +736,7 @@ static enum power_supply_property
> bq27541_battery_props[] = {
> POWER_SUPPLY_PROP_MANUFACTURER,
> };
>
> -static enum power_supply_property bq27545_battery_props[] = {
> +static enum power_supply_property bq27545_props[] = {
> POWER_SUPPLY_PROP_STATUS,
> POWER_SUPPLY_PROP_PRESENT,
> POWER_SUPPLY_PROP_VOLTAGE_NOW,
> @@ -755,7 +754,7 @@ static enum power_supply_property
> bq27545_battery_props[] = {
> POWER_SUPPLY_PROP_MANUFACTURER,
> };
>
> -static enum power_supply_property bq27421_battery_props[] = {
> +static enum power_supply_property bq27421_props[] = {
> POWER_SUPPLY_PROP_STATUS,
> POWER_SUPPLY_PROP_PRESENT,
> POWER_SUPPLY_PROP_VOLTAGE_NOW,
> @@ -770,32 +769,32 @@ static enum power_supply_property
> bq27421_battery_props[] = {
> POWER_SUPPLY_PROP_MANUFACTURER,
> };
>
> -#define BQ27XXX_PROP(_id, _prop) \
> - [_id] = { \
> - .props = _prop, \
> - .size = ARRAY_SIZE(_prop), \
> - }
> +#define BQ27XXX_DATA(ref) { \
> + .regs = ref##_regs, \
> + .props = ref##_props, \
> + .props_size = ARRAY_SIZE(ref##_props), }
>
> static struct {
> + u8 *regs;
> enum power_supply_property *props;
> - size_t size;
> -} bq27xxx_battery_props[] = {
> - BQ27XXX_PROP(BQ27000, bq27000_battery_props),
> - BQ27XXX_PROP(BQ27010, bq27010_battery_props),
> - BQ27XXX_PROP(BQ2750X, bq2750x_battery_props),
> - BQ27XXX_PROP(BQ2751X, bq2751x_battery_props),
> - BQ27XXX_PROP(BQ27500, bq27500_battery_props),
> - BQ27XXX_PROP(BQ27510G1, bq27510g1_battery_props),
> - BQ27XXX_PROP(BQ27510G2, bq27510g2_battery_props),
> - BQ27XXX_PROP(BQ27510G3, bq27510g3_battery_props),
> - BQ27XXX_PROP(BQ27520G1, bq27520g1_battery_props),
> - BQ27XXX_PROP(BQ27520G2, bq27520g2_battery_props),
> - BQ27XXX_PROP(BQ27520G3, bq27520g3_battery_props),
> - BQ27XXX_PROP(BQ27520G4, bq27520g4_battery_props),
> - BQ27XXX_PROP(BQ27530, bq27530_battery_props),
> - BQ27XXX_PROP(BQ27541, bq27541_battery_props),
> - BQ27XXX_PROP(BQ27545, bq27545_battery_props),
> - BQ27XXX_PROP(BQ27421, bq27421_battery_props),
> + size_t props_size;
> +} bq27xxx_battery_data[] = {
> + [BQ27000] = BQ27XXX_DATA(bq27000),
> + [BQ27010] = BQ27XXX_DATA(bq27010),
> + [BQ2750X] = BQ27XXX_DATA(bq2750x),
> + [BQ2751X] = BQ27XXX_DATA(bq2751x),
> + [BQ27500] = BQ27XXX_DATA(bq27500),
> + [BQ27510G1] = BQ27XXX_DATA(bq27510g1),
> + [BQ27510G2] = BQ27XXX_DATA(bq27510g2),
> + [BQ27510G3] = BQ27XXX_DATA(bq27510g3),
> + [BQ27520G1] = BQ27XXX_DATA(bq27520g1),
> + [BQ27520G2] = BQ27XXX_DATA(bq27520g2),
> + [BQ27520G3] = BQ27XXX_DATA(bq27520g3),
> + [BQ27520G4] = BQ27XXX_DATA(bq27520g4),
> + [BQ27530] = BQ27XXX_DATA(bq27530),
> + [BQ27541] = BQ27XXX_DATA(bq27541),
> + [BQ27545] = BQ27XXX_DATA(bq27545),
> + [BQ27421] = BQ27XXX_DATA(bq27421),
> };
>
> static DEFINE_MUTEX(bq27xxx_list_lock);
> @@ -1884,7 +1883,7 @@ int bq27xxx_battery_setup(struct bq27xxx_device_info *di)
>
> INIT_DELAYED_WORK(&di->work, bq27xxx_battery_poll);
> mutex_init(&di->lock);
> - di->regs = bq27xxx_regs[di->chip];
> + di->regs = bq27xxx_battery_data[di->chip].regs;
>
> psy_desc = devm_kzalloc(di->dev, sizeof(*psy_desc), GFP_KERNEL);
> if (!psy_desc)
> @@ -1892,8 +1891,8 @@ int bq27xxx_battery_setup(struct bq27xxx_device_info *di)
>
> psy_desc->name = di->name;
> psy_desc->type = POWER_SUPPLY_TYPE_BATTERY;
> - psy_desc->properties = bq27xxx_battery_props[di->chip].props;
> - psy_desc->num_properties = bq27xxx_battery_props[di->chip].size;
> + psy_desc->properties = bq27xxx_battery_data[di->chip].props;
> + psy_desc->num_properties = bq27xxx_battery_data[di->chip].props_size;
> psy_desc->get_property = bq27xxx_battery_get_property;
> psy_desc->external_power_changed = bq27xxx_external_power_changed;
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v14 09/11] power: supply: bq27xxx: Enable data memory update for certain chips
2017-07-05 9:47 ` Sebastian Reichel
@ 2017-07-05 17:49 ` Liam Breck
2017-07-06 6:06 ` Liam Breck
0 siblings, 1 reply; 25+ messages in thread
From: Liam Breck @ 2017-07-05 17:49 UTC (permalink / raw)
To: Sebastian Reichel; +Cc: Pali Rohar, linux-pm, Paul Kocialkowski, Liam Breck
Hi Sebastian,
On Wed, Jul 5, 2017 at 2:47 AM, Sebastian Reichel
<sebastian.reichel@collabora.co.uk> wrote:
> Hi,
>
> On Tue, Jul 04, 2017 at 04:24:05PM -0700, Liam Breck wrote:
>> OK, below is a minimal change to bq27xxx_battery.c creating a single
>> data table for all chips. It can easily be extended with new fields
>> for my patchset.
>
> Patch looks good.
>
>> This is probably safe to upstream without testing on all the chips, as
>> it was mostly search/replace. However various conditionals test the
>> chip ID, so when adding the rest of the IDs (previously only in
>> real_chip) in the next patch, I'd have to touch that code, but I have
>> no way of testing all the affected parts. I could use .acts_like =
>> OTHER_ID in the data table to minimize that risk. Thoughts?
>
> As far as I can see there are only a couple of different things
> di->chip is used for:
>
> di->chip == BQ27421 => flag_cfgupdate,
> di->chip == BQ27000 || di->chip == BQ27010 => flag_single,
> di->chip == BQ27530 || di->chip == BQ27421 => flag_temp_ot_ut,
There are two more cases. Also there's a lot more which could be added
to the driver down the road. Therefore I'd suggest a single u32 field
and bitwise flags. Then the table has:
[BQ27421] = BQ27XXX_DATA(bq27421, BQ27OPT_CFGUP | BQ27OPT_OTUT | BQ27OPT_RAM)
> If we add those flags to bq27xxx_battery_data we do not need
> to check di->chip in the driver at all.
I can define .opts in the table, and constants for the two cases I'm
responsible for. Outside of those, this idea has me changing a LOT of
logic for chips I cannot test, which isn't fail-safe. I would like to
leave that logic as-is and do the following after setup() is done with
bq27xxx_battery_data[di->chip].*
di->chip = bq27xxx_battery_data[di->chip].acts_like
//todo migrate this and other di->chip uses to di->opts
>> From: Liam Breck <kernel@networkimprov.net>
>> Subject: [PATCH] power: supply: bq27xxx: create single chip data table
>>
>> Unify data previously in bq27xxx_regs & bq27xxx_battery_props into a
>> single table.
>> There is no functional change to the driver.
>>
>> ---
>> drivers/power/supply/bq27xxx_battery.c | 121 ++++++++++++++++-----------------
>> 1 file changed, 60 insertions(+), 61 deletions(-)
>>
>> diff --git a/drivers/power/supply/bq27xxx_battery.c
>> b/drivers/power/supply/bq27xxx_battery.c
>> index ed44439d0112c..db42e4961c68f 100644
>> --- a/drivers/power/supply/bq27xxx_battery.c
>> +++ b/drivers/power/supply/bq27xxx_battery.c
>> @@ -132,8 +132,8 @@ enum bq27xxx_reg_index {
>> [BQ27XXX_DM_CKSUM] = 0x60
>>
>> /* Register mappings */
>> -static u8 bq27xxx_regs[][BQ27XXX_REG_MAX] = {
>> - [BQ27000] = {
>> +static u8
>> + bq27000_regs[BQ27XXX_REG_MAX] = {
>> [BQ27XXX_REG_CTRL] = 0x00,
>> [BQ27XXX_REG_TEMP] = 0x06,
>> [BQ27XXX_REG_INT_TEMP] = INVALID_REG_ADDR,
>> @@ -157,7 +157,7 @@ static u8 bq27xxx_regs[][BQ27XXX_REG_MAX] = {
>> [BQ27XXX_DM_DATA] = INVALID_REG_ADDR,
>> [BQ27XXX_DM_CKSUM] = INVALID_REG_ADDR,
>> },
>> - [BQ27010] = {
>> + bq27010_regs[BQ27XXX_REG_MAX] = {
>> [BQ27XXX_REG_CTRL] = 0x00,
>> [BQ27XXX_REG_TEMP] = 0x06,
>> [BQ27XXX_REG_INT_TEMP] = INVALID_REG_ADDR,
>> @@ -181,7 +181,7 @@ static u8 bq27xxx_regs[][BQ27XXX_REG_MAX] = {
>> [BQ27XXX_DM_DATA] = INVALID_REG_ADDR,
>> [BQ27XXX_DM_CKSUM] = INVALID_REG_ADDR,
>> },
>> - [BQ2750X] = {
>> + bq2750x_regs[BQ27XXX_REG_MAX] = {
>> [BQ27XXX_REG_CTRL] = 0x00,
>> [BQ27XXX_REG_TEMP] = 0x06,
>> [BQ27XXX_REG_INT_TEMP] = 0x28,
>> @@ -201,7 +201,7 @@ static u8 bq27xxx_regs[][BQ27XXX_REG_MAX] = {
>> [BQ27XXX_REG_AP] = INVALID_REG_ADDR,
>> BQ27XXX_DM_REG_ROWS,
>> },
>> - [BQ2751X] = {
>> + bq2751x_regs[BQ27XXX_REG_MAX] = {
>> [BQ27XXX_REG_CTRL] = 0x00,
>> [BQ27XXX_REG_TEMP] = 0x06,
>> [BQ27XXX_REG_INT_TEMP] = 0x28,
>> @@ -221,7 +221,7 @@ static u8 bq27xxx_regs[][BQ27XXX_REG_MAX] = {
>> [BQ27XXX_REG_AP] = INVALID_REG_ADDR,
>> BQ27XXX_DM_REG_ROWS,
>> },
>> - [BQ27500] = {
>> + bq27500_regs[BQ27XXX_REG_MAX] = {
>> [BQ27XXX_REG_CTRL] = 0x00,
>> [BQ27XXX_REG_TEMP] = 0x06,
>> [BQ27XXX_REG_INT_TEMP] = INVALID_REG_ADDR,
>> @@ -241,7 +241,7 @@ static u8 bq27xxx_regs[][BQ27XXX_REG_MAX] = {
>> [BQ27XXX_REG_AP] = 0x24,
>> BQ27XXX_DM_REG_ROWS,
>> },
>> - [BQ27510G1] = {
>> + bq27510g1_regs[BQ27XXX_REG_MAX] = {
>> [BQ27XXX_REG_CTRL] = 0x00,
>> [BQ27XXX_REG_TEMP] = 0x06,
>> [BQ27XXX_REG_INT_TEMP] = INVALID_REG_ADDR,
>> @@ -261,7 +261,7 @@ static u8 bq27xxx_regs[][BQ27XXX_REG_MAX] = {
>> [BQ27XXX_REG_AP] = 0x24,
>> BQ27XXX_DM_REG_ROWS,
>> },
>> - [BQ27510G2] = {
>> + bq27510g2_regs[BQ27XXX_REG_MAX] = {
>> [BQ27XXX_REG_CTRL] = 0x00,
>> [BQ27XXX_REG_TEMP] = 0x06,
>> [BQ27XXX_REG_INT_TEMP] = INVALID_REG_ADDR,
>> @@ -281,7 +281,7 @@ static u8 bq27xxx_regs[][BQ27XXX_REG_MAX] = {
>> [BQ27XXX_REG_AP] = 0x24,
>> BQ27XXX_DM_REG_ROWS,
>> },
>> - [BQ27510G3] = {
>> + bq27510g3_regs[BQ27XXX_REG_MAX] = {
>> [BQ27XXX_REG_CTRL] = 0x00,
>> [BQ27XXX_REG_TEMP] = 0x06,
>> [BQ27XXX_REG_INT_TEMP] = 0x28,
>> @@ -301,7 +301,7 @@ static u8 bq27xxx_regs[][BQ27XXX_REG_MAX] = {
>> [BQ27XXX_REG_AP] = INVALID_REG_ADDR,
>> BQ27XXX_DM_REG_ROWS,
>> },
>> - [BQ27520G1] = {
>> + bq27520g1_regs[BQ27XXX_REG_MAX] = {
>> [BQ27XXX_REG_CTRL] = 0x00,
>> [BQ27XXX_REG_TEMP] = 0x06,
>> [BQ27XXX_REG_INT_TEMP] = INVALID_REG_ADDR,
>> @@ -321,7 +321,7 @@ static u8 bq27xxx_regs[][BQ27XXX_REG_MAX] = {
>> [BQ27XXX_REG_AP] = 0x24,
>> BQ27XXX_DM_REG_ROWS,
>> },
>> - [BQ27520G2] = {
>> + bq27520g2_regs[BQ27XXX_REG_MAX] = {
>> [BQ27XXX_REG_CTRL] = 0x00,
>> [BQ27XXX_REG_TEMP] = 0x06,
>> [BQ27XXX_REG_INT_TEMP] = 0x36,
>> @@ -341,7 +341,7 @@ static u8 bq27xxx_regs[][BQ27XXX_REG_MAX] = {
>> [BQ27XXX_REG_AP] = 0x24,
>> BQ27XXX_DM_REG_ROWS,
>> },
>> - [BQ27520G3] = {
>> + bq27520g3_regs[BQ27XXX_REG_MAX] = {
>> [BQ27XXX_REG_CTRL] = 0x00,
>> [BQ27XXX_REG_TEMP] = 0x06,
>> [BQ27XXX_REG_INT_TEMP] = 0x36,
>> @@ -361,7 +361,7 @@ static u8 bq27xxx_regs[][BQ27XXX_REG_MAX] = {
>> [BQ27XXX_REG_AP] = 0x24,
>> BQ27XXX_DM_REG_ROWS,
>> },
>> - [BQ27520G4] = {
>> + bq27520g4_regs[BQ27XXX_REG_MAX] = {
>> [BQ27XXX_REG_CTRL] = 0x00,
>> [BQ27XXX_REG_TEMP] = 0x06,
>> [BQ27XXX_REG_INT_TEMP] = 0x28,
>> @@ -381,7 +381,7 @@ static u8 bq27xxx_regs[][BQ27XXX_REG_MAX] = {
>> [BQ27XXX_REG_AP] = INVALID_REG_ADDR,
>> BQ27XXX_DM_REG_ROWS,
>> },
>> - [BQ27530] = {
>> + bq27530_regs[BQ27XXX_REG_MAX] = {
>> [BQ27XXX_REG_CTRL] = 0x00,
>> [BQ27XXX_REG_TEMP] = 0x06,
>> [BQ27XXX_REG_INT_TEMP] = 0x32,
>> @@ -401,7 +401,7 @@ static u8 bq27xxx_regs[][BQ27XXX_REG_MAX] = {
>> [BQ27XXX_REG_AP] = 0x24,
>> BQ27XXX_DM_REG_ROWS,
>> },
>> - [BQ27541] = {
>> + bq27541_regs[BQ27XXX_REG_MAX] = {
>> [BQ27XXX_REG_CTRL] = 0x00,
>> [BQ27XXX_REG_TEMP] = 0x06,
>> [BQ27XXX_REG_INT_TEMP] = 0x28,
>> @@ -421,7 +421,7 @@ static u8 bq27xxx_regs[][BQ27XXX_REG_MAX] = {
>> [BQ27XXX_REG_AP] = 0x24,
>> BQ27XXX_DM_REG_ROWS,
>> },
>> - [BQ27545] = {
>> + bq27545_regs[BQ27XXX_REG_MAX] = {
>> [BQ27XXX_REG_CTRL] = 0x00,
>> [BQ27XXX_REG_TEMP] = 0x06,
>> [BQ27XXX_REG_INT_TEMP] = 0x28,
>> @@ -441,7 +441,7 @@ static u8 bq27xxx_regs[][BQ27XXX_REG_MAX] = {
>> [BQ27XXX_REG_AP] = 0x24,
>> BQ27XXX_DM_REG_ROWS,
>> },
>> - [BQ27421] = {
>> + bq27421_regs[BQ27XXX_REG_MAX] = {
>> [BQ27XXX_REG_CTRL] = 0x00,
>> [BQ27XXX_REG_TEMP] = 0x02,
>> [BQ27XXX_REG_INT_TEMP] = 0x1e,
>> @@ -460,10 +460,9 @@ static u8 bq27xxx_regs[][BQ27XXX_REG_MAX] = {
>> [BQ27XXX_REG_DCAP] = 0x3c,
>> [BQ27XXX_REG_AP] = 0x18,
>> BQ27XXX_DM_REG_ROWS,
>> - },
>> -};
>> + };
>>
>> -static enum power_supply_property bq27000_battery_props[] = {
>> +static enum power_supply_property bq27000_props[] = {
>> POWER_SUPPLY_PROP_STATUS,
>> POWER_SUPPLY_PROP_PRESENT,
>> POWER_SUPPLY_PROP_VOLTAGE_NOW,
>> @@ -485,7 +484,7 @@ static enum power_supply_property
>> bq27000_battery_props[] = {
>> POWER_SUPPLY_PROP_MANUFACTURER,
>> };
>>
>> -static enum power_supply_property bq27010_battery_props[] = {
>> +static enum power_supply_property bq27010_props[] = {
>> POWER_SUPPLY_PROP_STATUS,
>> POWER_SUPPLY_PROP_PRESENT,
>> POWER_SUPPLY_PROP_VOLTAGE_NOW,
>> @@ -505,7 +504,7 @@ static enum power_supply_property
>> bq27010_battery_props[] = {
>> POWER_SUPPLY_PROP_MANUFACTURER,
>> };
>>
>> -static enum power_supply_property bq2750x_battery_props[] = {
>> +static enum power_supply_property bq2750x_props[] = {
>> POWER_SUPPLY_PROP_STATUS,
>> POWER_SUPPLY_PROP_PRESENT,
>> POWER_SUPPLY_PROP_VOLTAGE_NOW,
>> @@ -523,7 +522,7 @@ static enum power_supply_property
>> bq2750x_battery_props[] = {
>> POWER_SUPPLY_PROP_MANUFACTURER,
>> };
>>
>> -static enum power_supply_property bq2751x_battery_props[] = {
>> +static enum power_supply_property bq2751x_props[] = {
>> POWER_SUPPLY_PROP_STATUS,
>> POWER_SUPPLY_PROP_PRESENT,
>> POWER_SUPPLY_PROP_VOLTAGE_NOW,
>> @@ -541,7 +540,7 @@ static enum power_supply_property
>> bq2751x_battery_props[] = {
>> POWER_SUPPLY_PROP_MANUFACTURER,
>> };
>>
>> -static enum power_supply_property bq27500_battery_props[] = {
>> +static enum power_supply_property bq27500_props[] = {
>> POWER_SUPPLY_PROP_STATUS,
>> POWER_SUPPLY_PROP_PRESENT,
>> POWER_SUPPLY_PROP_VOLTAGE_NOW,
>> @@ -562,7 +561,7 @@ static enum power_supply_property
>> bq27500_battery_props[] = {
>> POWER_SUPPLY_PROP_MANUFACTURER,
>> };
>>
>> -static enum power_supply_property bq27510g1_battery_props[] = {
>> +static enum power_supply_property bq27510g1_props[] = {
>> POWER_SUPPLY_PROP_STATUS,
>> POWER_SUPPLY_PROP_PRESENT,
>> POWER_SUPPLY_PROP_VOLTAGE_NOW,
>> @@ -583,7 +582,7 @@ static enum power_supply_property
>> bq27510g1_battery_props[] = {
>> POWER_SUPPLY_PROP_MANUFACTURER,
>> };
>>
>> -static enum power_supply_property bq27510g2_battery_props[] = {
>> +static enum power_supply_property bq27510g2_props[] = {
>> POWER_SUPPLY_PROP_STATUS,
>> POWER_SUPPLY_PROP_PRESENT,
>> POWER_SUPPLY_PROP_VOLTAGE_NOW,
>> @@ -604,7 +603,7 @@ static enum power_supply_property
>> bq27510g2_battery_props[] = {
>> POWER_SUPPLY_PROP_MANUFACTURER,
>> };
>>
>> -static enum power_supply_property bq27510g3_battery_props[] = {
>> +static enum power_supply_property bq27510g3_props[] = {
>> POWER_SUPPLY_PROP_STATUS,
>> POWER_SUPPLY_PROP_PRESENT,
>> POWER_SUPPLY_PROP_VOLTAGE_NOW,
>> @@ -622,7 +621,7 @@ static enum power_supply_property
>> bq27510g3_battery_props[] = {
>> POWER_SUPPLY_PROP_MANUFACTURER,
>> };
>>
>> -static enum power_supply_property bq27520g1_battery_props[] = {
>> +static enum power_supply_property bq27520g1_props[] = {
>> POWER_SUPPLY_PROP_STATUS,
>> POWER_SUPPLY_PROP_PRESENT,
>> POWER_SUPPLY_PROP_VOLTAGE_NOW,
>> @@ -642,7 +641,7 @@ static enum power_supply_property
>> bq27520g1_battery_props[] = {
>> POWER_SUPPLY_PROP_MANUFACTURER,
>> };
>>
>> -static enum power_supply_property bq27520g2_battery_props[] = {
>> +static enum power_supply_property bq27520g2_props[] = {
>> POWER_SUPPLY_PROP_STATUS,
>> POWER_SUPPLY_PROP_PRESENT,
>> POWER_SUPPLY_PROP_VOLTAGE_NOW,
>> @@ -663,7 +662,7 @@ static enum power_supply_property
>> bq27520g2_battery_props[] = {
>> POWER_SUPPLY_PROP_MANUFACTURER,
>> };
>>
>> -static enum power_supply_property bq27520g3_battery_props[] = {
>> +static enum power_supply_property bq27520g3_props[] = {
>> POWER_SUPPLY_PROP_STATUS,
>> POWER_SUPPLY_PROP_PRESENT,
>> POWER_SUPPLY_PROP_VOLTAGE_NOW,
>> @@ -683,7 +682,7 @@ static enum power_supply_property
>> bq27520g3_battery_props[] = {
>> POWER_SUPPLY_PROP_MANUFACTURER,
>> };
>>
>> -static enum power_supply_property bq27520g4_battery_props[] = {
>> +static enum power_supply_property bq27520g4_props[] = {
>> POWER_SUPPLY_PROP_STATUS,
>> POWER_SUPPLY_PROP_PRESENT,
>> POWER_SUPPLY_PROP_VOLTAGE_NOW,
>> @@ -700,7 +699,7 @@ static enum power_supply_property
>> bq27520g4_battery_props[] = {
>> POWER_SUPPLY_PROP_MANUFACTURER,
>> };
>>
>> -static enum power_supply_property bq27530_battery_props[] = {
>> +static enum power_supply_property bq27530_props[] = {
>> POWER_SUPPLY_PROP_STATUS,
>> POWER_SUPPLY_PROP_PRESENT,
>> POWER_SUPPLY_PROP_VOLTAGE_NOW,
>> @@ -718,7 +717,7 @@ static enum power_supply_property
>> bq27530_battery_props[] = {
>> POWER_SUPPLY_PROP_MANUFACTURER,
>> };
>>
>> -static enum power_supply_property bq27541_battery_props[] = {
>> +static enum power_supply_property bq27541_props[] = {
>> POWER_SUPPLY_PROP_STATUS,
>> POWER_SUPPLY_PROP_PRESENT,
>> POWER_SUPPLY_PROP_VOLTAGE_NOW,
>> @@ -737,7 +736,7 @@ static enum power_supply_property
>> bq27541_battery_props[] = {
>> POWER_SUPPLY_PROP_MANUFACTURER,
>> };
>>
>> -static enum power_supply_property bq27545_battery_props[] = {
>> +static enum power_supply_property bq27545_props[] = {
>> POWER_SUPPLY_PROP_STATUS,
>> POWER_SUPPLY_PROP_PRESENT,
>> POWER_SUPPLY_PROP_VOLTAGE_NOW,
>> @@ -755,7 +754,7 @@ static enum power_supply_property
>> bq27545_battery_props[] = {
>> POWER_SUPPLY_PROP_MANUFACTURER,
>> };
>>
>> -static enum power_supply_property bq27421_battery_props[] = {
>> +static enum power_supply_property bq27421_props[] = {
>> POWER_SUPPLY_PROP_STATUS,
>> POWER_SUPPLY_PROP_PRESENT,
>> POWER_SUPPLY_PROP_VOLTAGE_NOW,
>> @@ -770,32 +769,32 @@ static enum power_supply_property
>> bq27421_battery_props[] = {
>> POWER_SUPPLY_PROP_MANUFACTURER,
>> };
>>
>> -#define BQ27XXX_PROP(_id, _prop) \
>> - [_id] = { \
>> - .props = _prop, \
>> - .size = ARRAY_SIZE(_prop), \
>> - }
>> +#define BQ27XXX_DATA(ref) { \
>> + .regs = ref##_regs, \
>> + .props = ref##_props, \
>> + .props_size = ARRAY_SIZE(ref##_props), }
>>
>> static struct {
>> + u8 *regs;
>> enum power_supply_property *props;
>> - size_t size;
>> -} bq27xxx_battery_props[] = {
>> - BQ27XXX_PROP(BQ27000, bq27000_battery_props),
>> - BQ27XXX_PROP(BQ27010, bq27010_battery_props),
>> - BQ27XXX_PROP(BQ2750X, bq2750x_battery_props),
>> - BQ27XXX_PROP(BQ2751X, bq2751x_battery_props),
>> - BQ27XXX_PROP(BQ27500, bq27500_battery_props),
>> - BQ27XXX_PROP(BQ27510G1, bq27510g1_battery_props),
>> - BQ27XXX_PROP(BQ27510G2, bq27510g2_battery_props),
>> - BQ27XXX_PROP(BQ27510G3, bq27510g3_battery_props),
>> - BQ27XXX_PROP(BQ27520G1, bq27520g1_battery_props),
>> - BQ27XXX_PROP(BQ27520G2, bq27520g2_battery_props),
>> - BQ27XXX_PROP(BQ27520G3, bq27520g3_battery_props),
>> - BQ27XXX_PROP(BQ27520G4, bq27520g4_battery_props),
>> - BQ27XXX_PROP(BQ27530, bq27530_battery_props),
>> - BQ27XXX_PROP(BQ27541, bq27541_battery_props),
>> - BQ27XXX_PROP(BQ27545, bq27545_battery_props),
>> - BQ27XXX_PROP(BQ27421, bq27421_battery_props),
>> + size_t props_size;
>> +} bq27xxx_battery_data[] = {
>> + [BQ27000] = BQ27XXX_DATA(bq27000),
>> + [BQ27010] = BQ27XXX_DATA(bq27010),
>> + [BQ2750X] = BQ27XXX_DATA(bq2750x),
>> + [BQ2751X] = BQ27XXX_DATA(bq2751x),
>> + [BQ27500] = BQ27XXX_DATA(bq27500),
>> + [BQ27510G1] = BQ27XXX_DATA(bq27510g1),
>> + [BQ27510G2] = BQ27XXX_DATA(bq27510g2),
>> + [BQ27510G3] = BQ27XXX_DATA(bq27510g3),
>> + [BQ27520G1] = BQ27XXX_DATA(bq27520g1),
>> + [BQ27520G2] = BQ27XXX_DATA(bq27520g2),
>> + [BQ27520G3] = BQ27XXX_DATA(bq27520g3),
>> + [BQ27520G4] = BQ27XXX_DATA(bq27520g4),
>> + [BQ27530] = BQ27XXX_DATA(bq27530),
>> + [BQ27541] = BQ27XXX_DATA(bq27541),
>> + [BQ27545] = BQ27XXX_DATA(bq27545),
>> + [BQ27421] = BQ27XXX_DATA(bq27421),
>> };
>>
>> static DEFINE_MUTEX(bq27xxx_list_lock);
>> @@ -1884,7 +1883,7 @@ int bq27xxx_battery_setup(struct bq27xxx_device_info *di)
>>
>> INIT_DELAYED_WORK(&di->work, bq27xxx_battery_poll);
>> mutex_init(&di->lock);
>> - di->regs = bq27xxx_regs[di->chip];
>> + di->regs = bq27xxx_battery_data[di->chip].regs;
>>
>> psy_desc = devm_kzalloc(di->dev, sizeof(*psy_desc), GFP_KERNEL);
>> if (!psy_desc)
>> @@ -1892,8 +1891,8 @@ int bq27xxx_battery_setup(struct bq27xxx_device_info *di)
>>
>> psy_desc->name = di->name;
>> psy_desc->type = POWER_SUPPLY_TYPE_BATTERY;
>> - psy_desc->properties = bq27xxx_battery_props[di->chip].props;
>> - psy_desc->num_properties = bq27xxx_battery_props[di->chip].size;
>> + psy_desc->properties = bq27xxx_battery_data[di->chip].props;
>> + psy_desc->num_properties = bq27xxx_battery_data[di->chip].props_size;
>> psy_desc->get_property = bq27xxx_battery_get_property;
>> psy_desc->external_power_changed = bq27xxx_external_power_changed;
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v14 09/11] power: supply: bq27xxx: Enable data memory update for certain chips
2017-07-05 17:49 ` Liam Breck
@ 2017-07-06 6:06 ` Liam Breck
0 siblings, 0 replies; 25+ messages in thread
From: Liam Breck @ 2017-07-06 6:06 UTC (permalink / raw)
To: Sebastian Reichel; +Cc: Pali Rohar, linux-pm, Paul Kocialkowski, Liam Breck
Hi Sebastian,
On Wed, Jul 5, 2017 at 10:49 AM, Liam Breck <liam@networkimprov.net> wrote:
> Hi Sebastian,
>
> On Wed, Jul 5, 2017 at 2:47 AM, Sebastian Reichel
> <sebastian.reichel@collabora.co.uk> wrote:
>> Hi,
>>
>> On Tue, Jul 04, 2017 at 04:24:05PM -0700, Liam Breck wrote:
>>> OK, below is a minimal change to bq27xxx_battery.c creating a single
>>> data table for all chips. It can easily be extended with new fields
>>> for my patchset.
>>
>> Patch looks good.
>>
>>> This is probably safe to upstream without testing on all the chips, as
>>> it was mostly search/replace. However various conditionals test the
>>> chip ID, so when adding the rest of the IDs (previously only in
>>> real_chip) in the next patch, I'd have to touch that code, but I have
>>> no way of testing all the affected parts. I could use .acts_like =
>>> OTHER_ID in the data table to minimize that risk. Thoughts?
>>
>> As far as I can see there are only a couple of different things
>> di->chip is used for:
>>
>> di->chip == BQ27421 => flag_cfgupdate,
>> di->chip == BQ27000 || di->chip == BQ27010 => flag_single,
>> di->chip == BQ27530 || di->chip == BQ27421 => flag_temp_ot_ut,
>
> There are two more cases. Also there's a lot more which could be added
> to the driver down the road. Therefore I'd suggest a single u32 field
> and bitwise flags. Then the table has:
>
> [BQ27421] = BQ27XXX_DATA(bq27421, BQ27OPT_CFGUP | BQ27OPT_OTUT | BQ27OPT_RAM)
>
>> If we add those flags to bq27xxx_battery_data we do not need
>> to check di->chip in the driver at all.
>
> I can define .opts in the table, and constants for the two cases I'm
> responsible for. Outside of those, this idea has me changing a LOT of
> logic for chips I cannot test, which isn't fail-safe. I would like to
> leave that logic as-is and do the following after setup() is done with
> bq27xxx_battery_data[di->chip].*
>
> di->chip = bq27xxx_battery_data[di->chip].acts_like
> //todo migrate this and other di->chip uses to di->opts
Below is a second patch which adds the missing IDs and enables the
.opts field in table. Specific opts would be defined in subsequent
patches.
Re the defines at the top of the patch, I first tried pointers to the
referenced arrays, but C rejects that in a static initializer.
From: Liam Breck <kernel@networkimprov.net>
power: supply: bq27xxx: add chip IDs for previously unlisted supported chips
No functional change to the driver.
---
drivers/power/supply/bq27xxx_battery.c | 65 ++++++++++++++++++++++--------
drivers/power/supply/bq27xxx_battery_i2c.c | 16 ++++----
include/linux/power/bq27xxx_battery.h | 9 +++++
3 files changed, 65 insertions(+), 25 deletions(-)
diff --git a/drivers/power/supply/bq27xxx_battery.c
b/drivers/power/supply/bq27xxx_battery.c
index 9769a83cd2db9..c7039d4ba6d07 100644
--- a/drivers/power/supply/bq27xxx_battery.c
+++ b/drivers/power/supply/bq27xxx_battery.c
@@ -221,6 +221,7 @@ static u8
[BQ27XXX_REG_AP] = INVALID_REG_ADDR,
BQ27XXX_DM_REG_ROWS,
},
+#define bq2752x_regs bq2751x_regs
bq27500_regs[BQ27XXX_REG_MAX] = {
[BQ27XXX_REG_CTRL] = 0x00,
[BQ27XXX_REG_TEMP] = 0x06,
@@ -401,6 +402,7 @@ static u8
[BQ27XXX_REG_AP] = 0x24,
BQ27XXX_DM_REG_ROWS,
},
+#define bq27531_regs bq27530_regs
bq27541_regs[BQ27XXX_REG_MAX] = {
[BQ27XXX_REG_CTRL] = 0x00,
[BQ27XXX_REG_TEMP] = 0x06,
@@ -421,6 +423,9 @@ static u8
[BQ27XXX_REG_AP] = 0x24,
BQ27XXX_DM_REG_ROWS,
},
+#define bq27542_regs bq27541_regs
+#define bq27546_regs bq27541_regs
+#define bq27742_regs bq27541_regs
bq27545_regs[BQ27XXX_REG_MAX] = {
[BQ27XXX_REG_CTRL] = 0x00,
[BQ27XXX_REG_TEMP] = 0x06,
@@ -461,6 +466,9 @@ static u8
[BQ27XXX_REG_AP] = 0x18,
BQ27XXX_DM_REG_ROWS,
};
+#define bq27425_regs bq27421_regs
+#define bq27441_regs bq27421_regs
+#define bq27621_regs bq27421_regs
static enum power_supply_property bq27000_props[] = {
POWER_SUPPLY_PROP_STATUS,
@@ -539,6 +547,7 @@ static enum power_supply_property bq2751x_props[] = {
POWER_SUPPLY_PROP_HEALTH,
POWER_SUPPLY_PROP_MANUFACTURER,
};
+#define bq2752x_props bq2751x_props
static enum power_supply_property bq27500_props[] = {
POWER_SUPPLY_PROP_STATUS,
@@ -716,6 +725,7 @@ static enum power_supply_property bq27530_props[] = {
POWER_SUPPLY_PROP_CYCLE_COUNT,
POWER_SUPPLY_PROP_MANUFACTURER,
};
+#define bq27531_props bq27530_props
static enum power_supply_property bq27541_props[] = {
POWER_SUPPLY_PROP_STATUS,
@@ -735,6 +745,9 @@ static enum power_supply_property bq27541_props[] = {
POWER_SUPPLY_PROP_HEALTH,
POWER_SUPPLY_PROP_MANUFACTURER,
};
+#define bq27542_props bq27541_props
+#define bq27546_props bq27541_props
+#define bq27742_props bq27541_props
static enum power_supply_property bq27545_props[] = {
POWER_SUPPLY_PROP_STATUS,
@@ -768,33 +781,48 @@ static enum power_supply_property bq27421_props[] = {
POWER_SUPPLY_PROP_CHARGE_FULL_DESIGN,
POWER_SUPPLY_PROP_MANUFACTURER,
};
+#define bq27425_props bq27421_props
+#define bq27441_props bq27421_props
+#define bq27621_props bq27421_props
-#define BQ27XXX_DATA(ref) { \
+#define BQ27XXX_DATA(ref, act, opt) { \
+ .opts = (opt), \
+ .acts_like = act, \
.regs = bq27##ref##_regs, \
.props = bq27##ref##_props, \
.props_size = ARRAY_SIZE(bq27##ref##_props) }
static struct {
+ u32 opts;
+ int acts_like; //todo drop this when opts fully implemented
u8 *regs;
enum power_supply_property *props;
size_t props_size;
} bq27xxx_battery_data[] = {
- [BQ27000] = BQ27XXX_DATA(000),
- [BQ27010] = BQ27XXX_DATA(010),
- [BQ2750X] = BQ27XXX_DATA(50x),
- [BQ2751X] = BQ27XXX_DATA(51x),
- [BQ27500] = BQ27XXX_DATA(500),
- [BQ27510G1] = BQ27XXX_DATA(510g1),
- [BQ27510G2] = BQ27XXX_DATA(510g2),
- [BQ27510G3] = BQ27XXX_DATA(510g3),
- [BQ27520G1] = BQ27XXX_DATA(520g1),
- [BQ27520G2] = BQ27XXX_DATA(520g2),
- [BQ27520G3] = BQ27XXX_DATA(520g3),
- [BQ27520G4] = BQ27XXX_DATA(520g4),
- [BQ27530] = BQ27XXX_DATA(530),
- [BQ27541] = BQ27XXX_DATA(541),
- [BQ27545] = BQ27XXX_DATA(545),
- [BQ27421] = BQ27XXX_DATA(421),
+ [BQ27000] = BQ27XXX_DATA(000, BQ27000, 0),
+ [BQ27010] = BQ27XXX_DATA(010, BQ27010, 0),
+ [BQ2750X] = BQ27XXX_DATA(50x, BQ2750X, 0),
+ [BQ2751X] = BQ27XXX_DATA(51x, BQ2751X, 0),
+ [BQ2752X] = BQ27XXX_DATA(52x, BQ2751X, 0),
+ [BQ27500] = BQ27XXX_DATA(500, BQ27500, 0),
+ [BQ27510G1] = BQ27XXX_DATA(510g1, BQ27510G1, 0),
+ [BQ27510G2] = BQ27XXX_DATA(510g2, BQ27510G2, 0),
+ [BQ27510G3] = BQ27XXX_DATA(510g3, BQ27510G3, 0),
+ [BQ27520G1] = BQ27XXX_DATA(520g1, BQ27520G1, 0),
+ [BQ27520G2] = BQ27XXX_DATA(520g2, BQ27520G2, 0),
+ [BQ27520G3] = BQ27XXX_DATA(520g3, BQ27520G3, 0),
+ [BQ27520G4] = BQ27XXX_DATA(520g4, BQ27520G4, 0),
+ [BQ27530] = BQ27XXX_DATA(530, BQ27530, 0),
+ [BQ27531] = BQ27XXX_DATA(531, BQ27530, 0),
+ [BQ27541] = BQ27XXX_DATA(541, BQ27541, 0),
+ [BQ27542] = BQ27XXX_DATA(542, BQ27541, 0),
+ [BQ27546] = BQ27XXX_DATA(546, BQ27541, 0),
+ [BQ27742] = BQ27XXX_DATA(742, BQ27541, 0),
+ [BQ27545] = BQ27XXX_DATA(545, BQ27545, 0),
+ [BQ27421] = BQ27XXX_DATA(421, BQ27421, 0),
+ [BQ27425] = BQ27XXX_DATA(425, BQ27421, 0),
+ [BQ27441] = BQ27XXX_DATA(441, BQ27421, 0),
+ [BQ27621] = BQ27XXX_DATA(621, BQ27421, 0),
};
static DEFINE_MUTEX(bq27xxx_list_lock);
@@ -1902,6 +1930,9 @@ int bq27xxx_battery_setup(struct bq27xxx_device_info *di)
return PTR_ERR(di->bat);
}
+ di->opts = bq27xxx_battery_data[di->chip].opts;
+ di->chip = bq27xxx_battery_data[di->chip].acts_like;
+
dev_info(di->dev, "support ver. %s enabled\n", DRIVER_VERSION);
bq27xxx_battery_settings(di);
diff --git a/drivers/power/supply/bq27xxx_battery_i2c.c
b/drivers/power/supply/bq27xxx_battery_i2c.c
index a5972214f0742..0b11ed472f338 100644
--- a/drivers/power/supply/bq27xxx_battery_i2c.c
+++ b/drivers/power/supply/bq27xxx_battery_i2c.c
@@ -230,7 +230,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 },
@@ -240,16 +240,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);
diff --git a/include/linux/power/bq27xxx_battery.h
b/include/linux/power/bq27xxx_battery.h
index 11e11685dd1d5..77fe94f1d5f25 100644
--- a/include/linux/power/bq27xxx_battery.h
+++ b/include/linux/power/bq27xxx_battery.h
@@ -6,6 +6,7 @@ enum bq27xxx_chip {
BQ27010, /* bq27010, bq27210 */
BQ2750X, /* bq27500 deprecated alias */
BQ2751X, /* bq27510, bq27520 deprecated alias */
+ BQ2752X,
BQ27500, /* bq27500/1 */
BQ27510G1, /* bq27510G1 */
BQ27510G2, /* bq27510G2 */
@@ -15,9 +16,16 @@ enum bq27xxx_chip {
BQ27520G3, /* bq27520G3 */
BQ27520G4, /* bq27520G4 */
BQ27530, /* bq27530, bq27531 */
+ BQ27531,
BQ27541, /* bq27541, bq27542, bq27546, bq27742 */
+ BQ27542,
+ BQ27546,
+ BQ27742,
BQ27545, /* bq27545 */
BQ27421, /* bq27421, bq27425, bq27441, bq27621 */
+ BQ27425,
+ BQ27441,
+ BQ27621,
};
/**
@@ -64,6 +72,7 @@ struct bq27xxx_device_info {
int id;
enum bq27xxx_chip chip;
bool ram_chip;
+ u32 opts;
const char *name;
struct bq27xxx_dm_reg *dm_regs;
u32 unseal_key;
^ permalink raw reply related [flat|nested] 25+ messages in thread
end of thread, other threads:[~2017-07-06 6:06 UTC | newest]
Thread overview: 25+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-06-07 18:37 [PATCH v14 00/11] devicetree simple-battery and client in bq27xxx_battery Liam Breck
2017-06-07 18:37 ` [PATCH v14 01/11] devicetree: property-units: Add uWh and uAh units Liam Breck
2017-06-07 18:37 ` [PATCH v14 02/11] dt-bindings: power: supply: Add battery.txt with simple-battery binding Liam Breck
2017-06-07 18:37 ` [PATCH v14 03/11] power: supply: core: Add power_supply_battery_info and API Liam Breck
2017-06-07 18:37 ` [PATCH v14 04/11] power: supply: core: Add power_supply_prop_precharge Liam Breck
2017-06-07 18:37 ` [PATCH v14 05/11] dt-bindings: power: supply: bq27xxx: Add monitored-battery documentation Liam Breck
2017-06-07 18:37 ` [PATCH v14 06/11] power: supply: bq27xxx: Add bulk transfer bus methods Liam Breck
2017-06-07 18:37 ` [PATCH v14 07/11] power: supply: bq27xxx: Add chip data memory read/write support Liam Breck
2017-06-07 18:37 ` [PATCH v14 08/11] power: supply: bq27xxx: Add power_supply_battery_info support Liam Breck
2017-06-07 18:37 ` [PATCH v14 09/11] power: supply: bq27xxx: Enable data memory update for certain chips Liam Breck
2017-06-15 16:02 ` Sebastian Reichel
2017-06-16 9:21 ` Liam Breck
2017-06-16 10:33 ` Sebastian Reichel
2017-06-16 11:32 ` Liam Breck
2017-06-21 20:55 ` Liam Breck
2017-07-03 16:48 ` Sebastian Reichel
2017-07-04 23:24 ` Liam Breck
2017-07-05 9:47 ` Sebastian Reichel
2017-07-05 17:49 ` Liam Breck
2017-07-06 6:06 ` Liam Breck
2017-06-07 18:37 ` [PATCH v14 10/11] power: supply: bq27xxx: Flag identical register maps when in debug mode Liam Breck
2017-06-15 16:06 ` Sebastian Reichel
2017-06-16 9:44 ` Liam Breck
2017-06-07 18:37 ` [PATCH v14 11/11] power: supply: bq27xxx: Remove duplicate register arrays Liam Breck
2017-06-08 16:42 ` [PATCH v14 00/11] devicetree simple-battery and client in bq27xxx_battery Sebastian Reichel
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.