* [PATCH v13.2 00/11] bq27xxx_battery partial series
@ 2017-05-09 8:21 Liam Breck
2017-05-09 8:21 ` [PATCH v13.2 07/11] power: supply: bq27xxx_battery: Add chip data memory read/write support Liam Breck
` (3 more replies)
0 siblings, 4 replies; 8+ messages in thread
From: Liam Breck @ 2017-05-09 8:21 UTC (permalink / raw)
To: Sebastian Reichel, Andrew F. Davis, linux-pm
Hi Sebastian,
Andrew and I need your help in resolving three issues in this series.
I describe the issues in replies to the following patches. (The first
patch is included for context; there are no issues with it.)
Thanks!
power: supply: bq27xxx_battery: Add chip data memory read/write support
power: supply: bq27xxx_battery: Add power_supply_battery_info support
power: supply: bq27xxx_battery: Enable data memory update for certain chips
power: supply: bq27xxx_battery: Flag identical register maps when in debug mode
--
2.9.3
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v13.2 07/11] power: supply: bq27xxx_battery: Add chip data memory read/write support
2017-05-09 8:21 [PATCH v13.2 00/11] bq27xxx_battery partial series Liam Breck
@ 2017-05-09 8:21 ` Liam Breck
2017-05-09 8:21 ` [PATCH v13.2 08/11] power: supply: bq27xxx_battery: Add power_supply_battery_info support Liam Breck
` (2 subsequent siblings)
3 siblings, 0 replies; 8+ messages in thread
From: Liam Breck @ 2017-05-09 8:21 UTC (permalink / raw)
To: Sebastian Reichel, Andrew F. Davis, linux-pm; +Cc: 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 a11dfad..6a4ac14 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 c3369fa..b1defb8 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.9.3
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v13.2 08/11] power: supply: bq27xxx_battery: Add power_supply_battery_info support
2017-05-09 8:21 [PATCH v13.2 00/11] bq27xxx_battery partial series Liam Breck
2017-05-09 8:21 ` [PATCH v13.2 07/11] power: supply: bq27xxx_battery: Add chip data memory read/write support Liam Breck
@ 2017-05-09 8:21 ` Liam Breck
2017-05-09 10:18 ` Liam Breck
2017-05-09 8:21 ` [PATCH v13.2 09/11] power: supply: bq27xxx_battery: Enable data memory update for certain chips Liam Breck
2017-05-09 8:21 ` [PATCH v13.2 10/11] power: supply: bq27xxx_battery: Flag identical register maps when in debug mode Liam Breck
3 siblings, 1 reply; 8+ messages in thread
From: Liam Breck @ 2017-05-09 8:21 UTC (permalink / raw)
To: Sebastian Reichel, Andrew F. Davis, linux-pm; +Cc: 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 76806a0..38fae10 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 6a4ac14..ed44439 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 b1defb8..11e1168 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.9.3
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v13.2 09/11] power: supply: bq27xxx_battery: Enable data memory update for certain chips
2017-05-09 8:21 [PATCH v13.2 00/11] bq27xxx_battery partial series Liam Breck
2017-05-09 8:21 ` [PATCH v13.2 07/11] power: supply: bq27xxx_battery: Add chip data memory read/write support Liam Breck
2017-05-09 8:21 ` [PATCH v13.2 08/11] power: supply: bq27xxx_battery: Add power_supply_battery_info support Liam Breck
@ 2017-05-09 8:21 ` Liam Breck
2017-05-09 11:09 ` Liam Breck
2017-05-09 8:21 ` [PATCH v13.2 10/11] power: supply: bq27xxx_battery: Flag identical register maps when in debug mode Liam Breck
3 siblings, 1 reply; 8+ messages in thread
From: Liam Breck @ 2017-05-09 8:21 UTC (permalink / raw)
To: Sebastian Reichel, Andrew F. Davis, linux-pm; +Cc: Liam Breck
From: Liam Breck <kernel@networkimprov.net>
Support data memory update on BQ27500, 545, 425, 421, 441, 621.
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 | 64 +++++++++++++++++++++++++++---
drivers/power/supply/bq27xxx_battery_i2c.c | 52 ++++++++++++------------
include/linux/power/bq27xxx_battery.h | 14 +++++++
3 files changed, 100 insertions(+), 30 deletions(-)
diff --git a/drivers/power/supply/bq27xxx_battery.c b/drivers/power/supply/bq27xxx_battery.c
index ed44439..76cb181 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,54 @@ static const char * const bq27xxx_dm_reg_name[] = {
[BQ27XXX_DM_TERMINATE_VOLTAGE] = "terminate-voltage",
};
+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 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 },
+};
+
+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 },
+};
+
+static struct bq27xxx_dm_reg *bq27xxx_dm_regs[] = {
+ [BQ27500] = bq27500_dm_regs,
+ [BQ27545] = bq27545_dm_regs,
+ [BQ27421] = bq27421_dm_regs,
+ [BQ27425] = bq27425_dm_regs,
+ [BQ27441] = bq27421_dm_regs,
+ [BQ27621] = bq27621_dm_regs,
+};
+
+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 +1930,15 @@ int bq27xxx_battery_setup(struct bq27xxx_device_info *di)
.drv_data = di,
};
+ di->ram_chip = di->chip == BQ27421 || di->chip == BQ27441 || di->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 +2053,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 a597221..a365650 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 11e1168..bec13b7 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.9.3
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v13.2 10/11] power: supply: bq27xxx_battery: Flag identical register maps when in debug mode
2017-05-09 8:21 [PATCH v13.2 00/11] bq27xxx_battery partial series Liam Breck
` (2 preceding siblings ...)
2017-05-09 8:21 ` [PATCH v13.2 09/11] power: supply: bq27xxx_battery: Enable data memory update for certain chips Liam Breck
@ 2017-05-09 8:21 ` Liam Breck
2017-05-09 11:27 ` Liam Breck
3 siblings, 1 reply; 8+ messages in thread
From: Liam Breck @ 2017-05-09 8:21 UTC (permalink / raw)
To: Sebastian Reichel, Andrew F. Davis, linux-pm; +Cc: 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 76cb181..0492f33 100644
--- a/drivers/power/supply/bq27xxx_battery.c
+++ b/drivers/power/supply/bq27xxx_battery.c
@@ -1922,6 +1922,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;
@@ -1930,6 +1949,8 @@ int bq27xxx_battery_setup(struct bq27xxx_device_info *di)
.drv_data = di,
};
+ bq27xxx_battery_dbg_regs_dupes(di);
+
di->ram_chip = di->chip == BQ27421 || di->chip == BQ27441 || di->chip == BQ27621;
di->unseal_key = bq27xxx_unseal_keys[di->real_chip];
--
2.9.3
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v13.2 08/11] power: supply: bq27xxx_battery: Add power_supply_battery_info support
2017-05-09 8:21 ` [PATCH v13.2 08/11] power: supply: bq27xxx_battery: Add power_supply_battery_info support Liam Breck
@ 2017-05-09 10:18 ` Liam Breck
0 siblings, 0 replies; 8+ messages in thread
From: Liam Breck @ 2017-05-09 10:18 UTC (permalink / raw)
To: Sebastian Reichel, Andrew F. Davis, linux-pm; +Cc: Liam Breck
Hi Sebastian,
This patch only allows update of fuel gauge flash/NVM data if the
kernel is built with CONFIG_BATTERY_BQ27XXX_DT_UPDATES_NVM=y and a dtb
with monitored-battery config is packaged with the kernel. We expect
some device vendors to field-upgrade the fuel gauge this way.
The patch provides a module param dt_monitored_battery_updates_nvm
(default true) so the user can disable flash/NVM updates if he changes
the battery type. The param has no effect if config option is not =y.
Andrew is concerned that a distro could "by accident" enable the
config option and ship a dtb which causes some smart batteries to be
misconfigured. He believes that the module param should default false
to prevent this. However, that can cause the following scenario:
1) user changes battery type, sets param dt_monitored_battery_updates_nvm=0
2) device vendor ships gauge update in kernel package with
/etc/modprobe.d/xyz containing dt_monitored_battery_updates_nvm=1
3) gauge stops working correctly due to incompatible update
So making the module param default false is a bug.
I think the likelihood of a distro shipping a kernel package with
config option =y and a dtb with monitored-battery config where it does
not belong is vanishingly small. But to prevent a config accident we
could add a second option which also must be =y.
Some more comments inline below...
What do you think?
On Tue, May 9, 2017 at 1:21 AM, Liam Breck <liam@networkimprov.net> wrote:
> 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 76806a0..38fae10 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.
Above I have documented why distros should not set this option.
> 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 6a4ac14..ed44439 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
> +);
The module param has no effect when config option is not set, but I
define the param so the user can see the docs for it with modinfo
either way.
> 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;
> + }
The above is how the config option and module param control NVM update.
> + 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 b1defb8..11e1168 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.9.3
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v13.2 09/11] power: supply: bq27xxx_battery: Enable data memory update for certain chips
2017-05-09 8:21 ` [PATCH v13.2 09/11] power: supply: bq27xxx_battery: Enable data memory update for certain chips Liam Breck
@ 2017-05-09 11:09 ` Liam Breck
0 siblings, 0 replies; 8+ messages in thread
From: Liam Breck @ 2017-05-09 11:09 UTC (permalink / raw)
To: Sebastian Reichel, Andrew F. Davis, linux-pm; +Cc: Liam Breck
Hi Sebastian,
This patch makes the minimum changes required to support certain fuel
gauge chips.
Andrew dislikes that it stores two IDs for each chip -- a new "real
ID" which is unique and the original "chip ID" which is shared in some
cases. So he asked me to change the ID logic throughout the driver, a
significant effort. He has a concept for the change, but it's still
developing.
Changing the ID scheme is worth considering, but I do not believe it
belongs in this patch, and I do not wish to code and test it. I
suggested he do that in a patch following this series.
I point out things that Andrew took issue with inline below.
Do you think the patch is OK as-is?
On Tue, May 9, 2017 at 1:21 AM, Liam Breck <liam@networkimprov.net> wrote:
> From: Liam Breck <kernel@networkimprov.net>
>
> Support data memory update on BQ27500, 545, 425, 421, 441, 621.
>
> 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 | 64 +++++++++++++++++++++++++++---
> drivers/power/supply/bq27xxx_battery_i2c.c | 52 ++++++++++++------------
> include/linux/power/bq27xxx_battery.h | 14 +++++++
> 3 files changed, 100 insertions(+), 30 deletions(-)
>
> diff --git a/drivers/power/supply/bq27xxx_battery.c b/drivers/power/supply/bq27xxx_battery.c
> index ed44439..76cb181 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,54 @@ static const char * const bq27xxx_dm_reg_name[] = {
> [BQ27XXX_DM_TERMINATE_VOLTAGE] = "terminate-voltage",
> };
>
> +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 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 },
> +};
> +
> +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 },
> +};
> +
> +static struct bq27xxx_dm_reg *bq27xxx_dm_regs[] = {
> + [BQ27500] = bq27500_dm_regs,
> + [BQ27545] = bq27545_dm_regs,
> + [BQ27421] = bq27421_dm_regs,
> + [BQ27425] = bq27425_dm_regs,
> + [BQ27441] = bq27421_dm_regs,
> + [BQ27621] = bq27621_dm_regs,
> +};
> +
> +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 +1930,15 @@ int bq27xxx_battery_setup(struct bq27xxx_device_info *di)
> .drv_data = di,
> };
>
> + di->ram_chip = di->chip == BQ27421 || di->chip == BQ27441 || di->chip == BQ27621;
> +
> + di->unseal_key = bq27xxx_unseal_keys[di->real_chip];
> + di->dm_regs = bq27xxx_dm_regs[di->real_chip];
Above are the only two uses of the new real_chip ID.
> +
> + di->regs = bq27xxx_regs[di->chip];
This is from the original source, just moved up.
> 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 +2053,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 a597221..a365650 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 },
I combine real ID and chip ID in the I2C table, which used to carry
only the chip ID.
> {},
> };
> 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 11e1168..bec13b7 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.9.3
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v13.2 10/11] power: supply: bq27xxx_battery: Flag identical register maps when in debug mode
2017-05-09 8:21 ` [PATCH v13.2 10/11] power: supply: bq27xxx_battery: Flag identical register maps when in debug mode Liam Breck
@ 2017-05-09 11:27 ` Liam Breck
0 siblings, 0 replies; 8+ messages in thread
From: Liam Breck @ 2017-05-09 11:27 UTC (permalink / raw)
To: Sebastian Reichel, Andrew F. Davis, linux-pm; +Cc: Liam Breck
Hi Sebastian,
This patch checks for dupes in the large bq27xxx_regs[][] array of
register tables. It's easy to mistakenly add a duplicate table when
supporting a new chip; 3 such dupes were added recently.
Andrew suggested that this code belongs in a separate tools directory
program. That may be overkill for how short this is. Also the dupe
checker needs access to static module data.
What do you think?
On Tue, May 9, 2017 at 1:21 AM, Liam Breck <liam@networkimprov.net> 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>
> ---
> 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 76cb181..0492f33 100644
> --- a/drivers/power/supply/bq27xxx_battery.c
> +++ b/drivers/power/supply/bq27xxx_battery.c
> @@ -1922,6 +1922,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;
> @@ -1930,6 +1949,8 @@ int bq27xxx_battery_setup(struct bq27xxx_device_info *di)
> .drv_data = di,
> };
>
> + bq27xxx_battery_dbg_regs_dupes(di);
> +
> di->ram_chip = di->chip == BQ27421 || di->chip == BQ27441 || di->chip == BQ27621;
>
> di->unseal_key = bq27xxx_unseal_keys[di->real_chip];
> --
> 2.9.3
>
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2017-05-09 11:27 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-05-09 8:21 [PATCH v13.2 00/11] bq27xxx_battery partial series Liam Breck
2017-05-09 8:21 ` [PATCH v13.2 07/11] power: supply: bq27xxx_battery: Add chip data memory read/write support Liam Breck
2017-05-09 8:21 ` [PATCH v13.2 08/11] power: supply: bq27xxx_battery: Add power_supply_battery_info support Liam Breck
2017-05-09 10:18 ` Liam Breck
2017-05-09 8:21 ` [PATCH v13.2 09/11] power: supply: bq27xxx_battery: Enable data memory update for certain chips Liam Breck
2017-05-09 11:09 ` Liam Breck
2017-05-09 8:21 ` [PATCH v13.2 10/11] power: supply: bq27xxx_battery: Flag identical register maps when in debug mode Liam Breck
2017-05-09 11:27 ` Liam Breck
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.