[PATCH v12.4 0/7] bq27xxx_battery partial series

[PATCH v12.4 0/7] bq27xxx_battery partial series

[Liam Breck]

Latest changes:

Devicetree docs mention relevant battery node fields.

Clean up patch "Flag identical register maps..."

^.^ - I can has test?

  power: bq27xxx_battery: Add bulk transfer bus methods
  power: bq27xxx_battery: Add chip data memory read/write support
  power: bq27xxx_battery: Add power_supply_battery_info support
  power: bq27xxx_battery: Enable chip data memory update for certain chips
  devicetree: power: bq27xxx: Add monitored-battery documentation
  power: bq27xxx_battery: Flag identical register maps when in debug mode
  power: bq27xxx_battery: Remove duplicate register arrays

 .../devicetree/bindings/power/supply/bq27xxx.txt   |  31 +-
 drivers/power/supply/bq27xxx_battery.c             | 704 +++++++++++++++++----
 drivers/power/supply/bq27xxx_battery_i2c.c         |  98 ++-
 include/linux/power/bq27xxx_battery.h              |  26 +-
 4 files changed, 710 insertions(+), 149 deletions(-)

-- 
2.9.3

[PATCH v12.4 4/10] power: bq27xxx_battery: Add bulk transfer bus methods

[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() helper functions to call the above.

Acked-by: "Andrew F. Davis" <afd@ti.com>
Signed-off-by: Matt Ranostay <matt@ranostay.consulting>
Signed-off-by: Liam Breck <kernel@networkimprov.net>
---
 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 398801a..ede7fac 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 -ENOSYS;
+
+	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 -ENOSYS;
+
+	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 -ENOSYS;
+
+	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 c68fbc3..a597221 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 b312bce..c3369fa 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.9.3

[PATCH v12.4 5/10] power: bq27xxx_battery: Add chip data memory read/write support

[Liam Breck]

From: Liam Breck <kernel@networkimprov.net>

Add the following to enable read/write of chip data memory (DM) RAM/NVM/flash:
  bq27xxx_battery_seal()
  bq27xxx_battery_unseal()
  bq27xxx_battery_set_cfgupdate()
  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 | 245 +++++++++++++++++++++++++++++++++
 include/linux/power/bq27xxx_battery.h  |   1 +
 2 files changed, 246 insertions(+)

diff --git a/drivers/power/supply/bq27xxx_battery.c b/drivers/power/supply/bq27xxx_battery.c
index ede7fac..5f692b1 100644
--- a/drivers/power/supply/bq27xxx_battery.c
+++ b/drivers/power/supply/bq27xxx_battery.c
@@ -65,6 +65,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 +79,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 +115,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 +150,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 +174,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 +198,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 +218,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 +238,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 +258,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 +278,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 +298,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 +318,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 +338,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 +358,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 +378,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 +398,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 +418,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 +438,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 +458,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 +800,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 +929,186 @@ 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 < 0) ? ret : 0;
+}
+
+static int bq27xxx_battery_unseal(struct bq27xxx_device_info *di)
+{
+	int ret;
+
+	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_set_cfgupdate(struct bq27xxx_device_info *di, u16 state)
+{
+	const int limit = 100;
+	int ret, try = limit;
+
+	ret = bq27xxx_write(di, BQ27XXX_REG_CTRL,
+			    state ? BQ27XXX_SET_CFGUPDATE : BQ27XXX_SOFT_RESET,
+			    false);
+	if (ret < 0)
+		goto out;
+
+	do {
+		BQ27XXX_MSLEEP(25);
+		ret = di->bus.read(di, di->regs[BQ27XXX_REG_FLAGS], false);
+		if (ret < 0)
+			goto out;
+	} while ((ret & BQ27XXX_FLAG_CFGUP) != state && --try);
+
+	if (!try) {
+		dev_err(di->dev, "timed out waiting for cfgupdate flag %d\n", !!state);
+		return -EINVAL;
+	}
+
+	if (limit-try > 3)
+		dev_warn(di->dev, "cfgupdate %d, retries %d\n", !!state, limit-try);
+
+	return 0;
+
+out:
+	dev_err(di->dev, "bus error on %s: %d\n", state ? "set_cfgupdate" : "soft_reset", 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 group supports cfgupdate */
+	int ret;
+
+	if (!buf->dirty)
+		return 0;
+
+	if (cfgup) {
+		ret = bq27xxx_battery_set_cfgupdate(di, BQ27XXX_FLAG_CFGUP);
+		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. See TI bqtool for what not to do:
+	 * http://git.ti.com/bms-linux/bqtool/blobs/master/gauge.c#line328
+	 */
+
+	if (cfgup) {
+		BQ27XXX_MSLEEP(1);
+		ret = bq27xxx_battery_set_cfgupdate(di, 0);
+		if (ret < 0)
+			return ret;
+	} else {
+		BQ27XXX_MSLEEP(100); /* flash DM updates in <100ms */
+	}
+
+	buf->dirty = false;
+
+	return 0;
+
+out:
+	if (cfgup)
+		bq27xxx_battery_set_cfgupdate(di, 0);
+
+	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

[PATCH v12.4 6/10] power: bq27xxx_battery: Add power_supply_battery_info support

[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.

We now call power_supply_get_battery_info(), check its values, and write
battery data to chip data memory (flash/RAM/NVM).

Signed-off-by: Matt Ranostay <matt@ranostay.consulting>
Signed-off-by: Liam Breck <kernel@networkimprov.net>
---
 drivers/power/supply/bq27xxx_battery.c | 172 ++++++++++++++++++++++++++++++++-
 include/linux/power/bq27xxx_battery.h  |   1 +
 2 files changed, 172 insertions(+), 1 deletion(-)

diff --git a/drivers/power/supply/bq27xxx_battery.c b/drivers/power/supply/bq27xxx_battery.c
index 5f692b1..41eb5b7 100644
--- a/drivers/power/supply/bq27xxx_battery.c
+++ b/drivers/power/supply/bq27xxx_battery.c
@@ -804,6 +804,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
@@ -821,6 +828,33 @@ 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* bq27xxx_dm_reg_name[] = {
+	[BQ27XXX_DM_DESIGN_CAPACITY] = "design-capacity",
+	[BQ27XXX_DM_DESIGN_ENERGY] = "design-energy",
+	[BQ27XXX_DM_TERMINATE_VOLTAGE] = "terminate-voltage",
+};
+
 
 static int poll_interval_param_set(const char *val, const struct kernel_param *kp)
 {
@@ -1011,6 +1045,39 @@ 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;
+	}
+
+	dev_info(di->dev, "update %s to %u\n", str, val);
+
+	*prev = cpu_to_be16(val);
+	buf->dirty = true;
+}
+
 static int bq27xxx_battery_set_cfgupdate(struct bq27xxx_device_info *di, u16 state)
 {
 	const int limit = 100;
@@ -1109,6 +1176,98 @@ 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);
+
+	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);
+	}
+
+	bq27xxx_battery_write_dm_block(di, &bd);
+	bq27xxx_battery_write_dm_block(di, &bt);
+
+	bq27xxx_battery_seal(di);
+
+	if (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 */
+}
+
+void bq27xxx_battery_settings(struct bq27xxx_device_info *di)
+{
+	struct power_supply_battery_info info = {};
+	unsigned int min, max;
+
+	if (!di->dm_regs)
+		return;
+
+	if (power_supply_get_battery_info(di->bat, &info) < 0)
+		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.
@@ -1626,6 +1785,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;
@@ -1659,7 +1825,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);
@@ -1684,6 +1853,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..227eb08 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;
+	struct bq27xxx_dm_reg *dm_regs;
 	u32 unseal_key;
 	struct bq27xxx_access_methods bus;
 	struct bq27xxx_reg_cache cache;
-- 
2.9.3

Re: [PATCH v12.4 6/10] power: bq27xxx_battery: Add power_supply_battery_info support

[Andrew F. Davis]

On 04/04/2017 03:57 AM, Liam Breck 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.
> 
> We now call power_supply_get_battery_info(), check its values, and write
> battery data to chip data memory (flash/RAM/NVM).
> 

I'm almost convinced now this is *not* the correct thing to do, we
should not be writing to NVM. If the DT states different values than in
the device we need to trust the device. DT is static for a board, the
battery data may not be.

At most we could have an override flag as a driver param that causes us
to ignore NVM and use the DT values, but always overwriting the device's
NVM with the DT values is not what most want.

> Signed-off-by: Matt Ranostay <matt@ranostay.consulting>
> Signed-off-by: Liam Breck <kernel@networkimprov.net>
> ---
>  drivers/power/supply/bq27xxx_battery.c | 172 ++++++++++++++++++++++++++++++++-
>  include/linux/power/bq27xxx_battery.h  |   1 +
>  2 files changed, 172 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/power/supply/bq27xxx_battery.c b/drivers/power/supply/bq27xxx_battery.c
> index 5f692b1..41eb5b7 100644
> --- a/drivers/power/supply/bq27xxx_battery.c
> +++ b/drivers/power/supply/bq27xxx_battery.c
> @@ -804,6 +804,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
> @@ -821,6 +828,33 @@ 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* bq27xxx_dm_reg_name[] = {
> +	[BQ27XXX_DM_DESIGN_CAPACITY] = "design-capacity",
> +	[BQ27XXX_DM_DESIGN_ENERGY] = "design-energy",
> +	[BQ27XXX_DM_TERMINATE_VOLTAGE] = "terminate-voltage",
> +};
> +
>  
>  static int poll_interval_param_set(const char *val, const struct kernel_param *kp)
>  {
> @@ -1011,6 +1045,39 @@ 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;
> +	}
> +
> +	dev_info(di->dev, "update %s to %u\n", str, val);
> +
> +	*prev = cpu_to_be16(val);
> +	buf->dirty = true;
> +}
> +
>  static int bq27xxx_battery_set_cfgupdate(struct bq27xxx_device_info *di, u16 state)
>  {
>  	const int limit = 100;
> @@ -1109,6 +1176,98 @@ 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);
> +
> +	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);
> +	}
> +
> +	bq27xxx_battery_write_dm_block(di, &bd);
> +	bq27xxx_battery_write_dm_block(di, &bt);
> +
> +	bq27xxx_battery_seal(di);
> +
> +	if (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 */
> +}
> +
> +void bq27xxx_battery_settings(struct bq27xxx_device_info *di)
> +{
> +	struct power_supply_battery_info info = {};
> +	unsigned int min, max;
> +
> +	if (!di->dm_regs)
> +		return;
> +
> +	if (power_supply_get_battery_info(di->bat, &info) < 0)
> +		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.
> @@ -1626,6 +1785,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;
> @@ -1659,7 +1825,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);
> @@ -1684,6 +1853,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..227eb08 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;
> +	struct bq27xxx_dm_reg *dm_regs;
>  	u32 unseal_key;
>  	struct bq27xxx_access_methods bus;
>  	struct bq27xxx_reg_cache cache;
> 

Re: [PATCH v12.4 6/10] power: bq27xxx_battery: Add power_supply_battery_info support

[Liam Breck]

On Thu, Apr 6, 2017 at 7:30 AM, Andrew F. Davis <afd@ti.com> wrote:
> On 04/04/2017 03:57 AM, Liam Breck 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.
>>
>> We now call power_supply_get_battery_info(), check its values, and write
>> battery data to chip data memory (flash/RAM/NVM).
>>
>
> I'm almost convinced now this is *not* the correct thing to do, we
> should not be writing to NVM. If the DT states different values than in
> the device we need to trust the device. DT is static for a board, the
> battery data may not be.
>
> At most we could have an override flag as a driver param that causes us
> to ignore NVM and use the DT values, but always overwriting the device's
> NVM with the DT values is not what most want.

Reprising my previous response, with addenda...

We trust the distro/device kernel package to contain correct config
for RAM-only chips, but not NVM? A battery config would not be set in
mainline dts files, unless a battery is soldered to the board, or
similar. I will note this in our DT binding.

Re DT provisioning... Device makers and distros are extremely careful
about what they put into DTs and what dtbs are included with a kernel
package. They know that a misconfigured DT is a showstopper, and could
be catastrophic. I've seen the latter -- a guru-level kernel
maintainer whom I work with set a DT voltage level wrong on a
prototype and fried its eMMC chip. If the DT has a gauge config, the
default assumption is that it's correct.

It is wrong to force distros and device makers to include
/etc/modprobe.d/xyz.conf to fully configure the hardware.

However we should indeed let users override the DT config with a
module param. How about use-on-chip-config=1, type bool?




>> Signed-off-by: Matt Ranostay <matt@ranostay.consulting>
>> Signed-off-by: Liam Breck <kernel@networkimprov.net>
>> ---
>>  drivers/power/supply/bq27xxx_battery.c | 172 ++++++++++++++++++++++++++++++++-
>>  include/linux/power/bq27xxx_battery.h  |   1 +
>>  2 files changed, 172 insertions(+), 1 deletion(-)

Re: [PATCH v12.4 6/10] power: bq27xxx_battery: Add power_supply_battery_info support

[Liam Breck]

On Thu, Apr 6, 2017 at 11:23 AM, Liam Breck <liam@networkimprov.net> wrote:
> On Thu, Apr 6, 2017 at 7:30 AM, Andrew F. Davis <afd@ti.com> wrote:
>> On 04/04/2017 03:57 AM, Liam Breck 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.
>>>
>>> We now call power_supply_get_battery_info(), check its values, and write
>>> battery data to chip data memory (flash/RAM/NVM).
>>>
>>
>> I'm almost convinced now this is *not* the correct thing to do, we
>> should not be writing to NVM. If the DT states different values than in
>> the device we need to trust the device. DT is static for a board, the
>> battery data may not be.
>>
>> At most we could have an override flag as a driver param that causes us
>> to ignore NVM and use the DT values, but always overwriting the device's
>> NVM with the DT values is not what most want.
>
> Reprising my previous response, with addenda...
>
> We trust the distro/device kernel package to contain correct config
> for RAM-only chips, but not NVM? A battery config would not be set in
> mainline dts files, unless a battery is soldered to the board, or
> similar. I will note this in our DT binding.
>
> Re DT provisioning... Device makers and distros are extremely careful
> about what they put into DTs and what dtbs are included with a kernel
> package. They know that a misconfigured DT is a showstopper, and could
> be catastrophic. I've seen the latter -- a guru-level kernel
> maintainer whom I work with set a DT voltage level wrong on a
> prototype and fried its eMMC chip. If the DT has a gauge config, the
> default assumption is that it's correct.
>
> It is wrong to force distros and device makers to include
> /etc/modprobe.d/xyz.conf to fully configure the hardware.
>
> However we should indeed let users override the DT config with a
> module param. How about use-on-chip-config=1, type bool?

I came up with...

static bool bq27xxx_dt_battery = true;
module_param_named(use_dt_battery, bq27xxx_dt_battery, bool, S_IRUGO);
MODULE_PARM_DESC(use_dt_battery,
 "Use devicetree monitored-battery config if present. Default is 1/Y.");

After we check for DT monitored-battery, we test bq27xxx_dt_battery,
and if false, emit a warning that we are skipping the DT config.

Re: [PATCH v12.4 6/10] power: bq27xxx_battery: Add power_supply_battery_info support

[Andrew F. Davis]

On 04/07/2017 01:58 PM, Liam Breck wrote:
> On Thu, Apr 6, 2017 at 11:23 AM, Liam Breck <liam@networkimprov.net> wrote:
>> On Thu, Apr 6, 2017 at 7:30 AM, Andrew F. Davis <afd@ti.com> wrote:
>>> On 04/04/2017 03:57 AM, Liam Breck 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.
>>>>
>>>> We now call power_supply_get_battery_info(), check its values, and write
>>>> battery data to chip data memory (flash/RAM/NVM).
>>>>
>>>
>>> I'm almost convinced now this is *not* the correct thing to do, we
>>> should not be writing to NVM. If the DT states different values than in
>>> the device we need to trust the device. DT is static for a board, the
>>> battery data may not be.
>>>
>>> At most we could have an override flag as a driver param that causes us
>>> to ignore NVM and use the DT values, but always overwriting the device's
>>> NVM with the DT values is not what most want.
>>
>> Reprising my previous response, with addenda...
>>
>> We trust the distro/device kernel package to contain correct config
>> for RAM-only chips, but not NVM? A battery config would not be set in
>> mainline dts files, unless a battery is soldered to the board, or
>> similar. I will note this in our DT binding.
>>

DT is for hardware descriptions, in cases where the hardware cannot be
automatically detected. If we find we can get the battery info from the
battery gauge why would we not only ignore it, but go and overwrite it
with the info from DT? This is backwards.

>> Re DT provisioning... Device makers and distros are extremely careful
>> about what they put into DTs and what dtbs are included with a kernel
>> package. They know that a misconfigured DT is a showstopper, and could
>> be catastrophic. I've seen the latter -- a guru-level kernel
>> maintainer whom I work with set a DT voltage level wrong on a
>> prototype and fried its eMMC chip. If the DT has a gauge config, the
>> default assumption is that it's correct.
>>

And if that eMMC chip could communicate its requested voltage we would
not need to describe it in DT, and we certainly would not say "sorry
eMMC, you requested 3v but DT says you need 33v, so thats what you're
getting".

>> It is wrong to force distros and device makers to include
>> /etc/modprobe.d/xyz.conf to fully configure the hardware.
>>

It's either that or add a driver sysfs entry, we don't configure
hardware in DT, thats rule #1. You got an ack for a binding that
describes a "simple-battery"(one that cannot self report), you are
misusing this binding as input configuration data to feed to batteries
that *can* report their configuration.

>> However we should indeed let users override the DT config with a
>> module param. How about use-on-chip-config=1, type bool?
> 

If we do add a param it needs to be default "off". It should not update
saved battery data on the chip in either case, that needs to be a
manually triggered through sysfs or similar.

> I came up with...
> 
> static bool bq27xxx_dt_battery = true;
> module_param_named(use_dt_battery, bq27xxx_dt_battery, bool, S_IRUGO);
> MODULE_PARM_DESC(use_dt_battery,
>  "Use devicetree monitored-battery config if present. Default is 1/Y.");
> 
> After we check for DT monitored-battery, we test bq27xxx_dt_battery,
> and if false, emit a warning that we are skipping the DT config.
> 

Re: [PATCH v12.4 6/10] power: bq27xxx_battery: Add power_supply_battery_info support

[Liam Breck]

On Fri, Apr 7, 2017 at 12:19 PM, Andrew F. Davis <afd@ti.com> wrote:
> On 04/07/2017 01:58 PM, Liam Breck wrote:
>> On Thu, Apr 6, 2017 at 11:23 AM, Liam Breck <liam@networkimprov.net> wrote:
>>> On Thu, Apr 6, 2017 at 7:30 AM, Andrew F. Davis <afd@ti.com> wrote:
>>>> On 04/04/2017 03:57 AM, Liam Breck 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.
>>>>>
>>>>> We now call power_supply_get_battery_info(), check its values, and write
>>>>> battery data to chip data memory (flash/RAM/NVM).
>>>>>
>>>>
>>>> I'm almost convinced now this is *not* the correct thing to do, we
>>>> should not be writing to NVM. If the DT states different values than in
>>>> the device we need to trust the device. DT is static for a board, the
>>>> battery data may not be.
>>>>
>>>> At most we could have an override flag as a driver param that causes us
>>>> to ignore NVM and use the DT values, but always overwriting the device's
>>>> NVM with the DT values is not what most want.
>>>
>>> Reprising my previous response, with addenda...
>>>
>>> We trust the distro/device kernel package to contain correct config
>>> for RAM-only chips, but not NVM? A battery config would not be set in
>>> mainline dts files, unless a battery is soldered to the board, or
>>> similar. I will note this in our DT binding.
>>>
>
> DT is for hardware descriptions, in cases where the hardware cannot be
> automatically detected. If we find we can get the battery info from the
> battery gauge why would we not only ignore it, but go and overwrite it
> with the info from DT? This is backwards.

Can this driver manage gauges packaged in batteries? I thought it
could only see those wired onto main boards. I agree we should not
update battery-packaged gauges by default. Is it possible to tell the
difference?

On the other hand, no gauge can detect battery characteristics by its
own circuitry. It has to be programmed by human hands. A dtb + this
patch makes a tool for those hands.

And you don't trust device vendors? If a vendor records DT properties
for a gauge, it's specifically because the gauge-resident config isn't
correct.

>>> Re DT provisioning... Device makers and distros are extremely careful
>>> about what they put into DTs and what dtbs are included with a kernel
>>> package. They know that a misconfigured DT is a showstopper, and could
>>> be catastrophic. I've seen the latter -- a guru-level kernel
>>> maintainer whom I work with set a DT voltage level wrong on a
>>> prototype and fried its eMMC chip. If the DT has a gauge config, the
>>> default assumption is that it's correct.
>>>
>
> And if that eMMC chip could communicate its requested voltage we would
> not need to describe it in DT, and we certainly would not say "sorry
> eMMC, you requested 3v but DT says you need 33v, so thats what you're
> getting".

>>> It is wrong to force distros and device makers to include
>>> /etc/modprobe.d/xyz.conf to fully configure the hardware.
>>>
>
> It's either that or add a driver sysfs entry, we don't configure
> hardware in DT, thats rule #1. You got an ack for a binding that

Setting regulator voltage levels is not "configuring hardware"?

> describes a "simple-battery"(one that cannot self report), you are
> misusing this binding as input configuration data to feed to batteries
> that *can* report their configuration.

>>> However we should indeed let users override the DT config with a
>>> module param. How about use-on-chip-config=1, type bool?
>>
>
> If we do add a param it needs to be default "off". It should not update
> saved battery data on the chip in either case, that needs to be a
> manually triggered through sysfs or similar.
>
>> I came up with...
>>
>> static bool bq27xxx_dt_battery = true;
>> module_param_named(use_dt_battery, bq27xxx_dt_battery, bool, S_IRUGO);
>> MODULE_PARM_DESC(use_dt_battery,
>>  "Use devicetree monitored-battery config if present. Default is 1/Y.");
>>
>> After we check for DT monitored-battery, we test bq27xxx_dt_battery,
>> and if false, emit a warning that we are skipping the DT config.
>>

Re: [PATCH v12.4 6/10] power: bq27xxx_battery: Add power_supply_battery_info support

[Andrew F. Davis]

On 04/07/2017 02:56 PM, Liam Breck wrote:
> On Fri, Apr 7, 2017 at 12:19 PM, Andrew F. Davis <afd@ti.com> wrote:
>> On 04/07/2017 01:58 PM, Liam Breck wrote:
>>> On Thu, Apr 6, 2017 at 11:23 AM, Liam Breck <liam@networkimprov.net> wrote:
>>>> On Thu, Apr 6, 2017 at 7:30 AM, Andrew F. Davis <afd@ti.com> wrote:
>>>>> On 04/04/2017 03:57 AM, Liam Breck 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.
>>>>>>
>>>>>> We now call power_supply_get_battery_info(), check its values, and write
>>>>>> battery data to chip data memory (flash/RAM/NVM).
>>>>>>
>>>>>
>>>>> I'm almost convinced now this is *not* the correct thing to do, we
>>>>> should not be writing to NVM. If the DT states different values than in
>>>>> the device we need to trust the device. DT is static for a board, the
>>>>> battery data may not be.
>>>>>
>>>>> At most we could have an override flag as a driver param that causes us
>>>>> to ignore NVM and use the DT values, but always overwriting the device's
>>>>> NVM with the DT values is not what most want.
>>>>
>>>> Reprising my previous response, with addenda...
>>>>
>>>> We trust the distro/device kernel package to contain correct config
>>>> for RAM-only chips, but not NVM? A battery config would not be set in
>>>> mainline dts files, unless a battery is soldered to the board, or
>>>> similar. I will note this in our DT binding.
>>>>
>>
>> DT is for hardware descriptions, in cases where the hardware cannot be
>> automatically detected. If we find we can get the battery info from the
>> battery gauge why would we not only ignore it, but go and overwrite it
>> with the info from DT? This is backwards.
> 
> Can this driver manage gauges packaged in batteries? I thought it
> could only see those wired onto main boards. I agree we should not
> update battery-packaged gauges by default. Is it possible to tell the
> difference?
> 

Yes, this is actually the most common configuration. (You probably have
a BQ27xxx in this config in your pocket right now if you have a cell phone).

Some BQ27xxx parts have a battery detect, this is usually only needed
for gauges that do not ride along with the battery, but in practice you
cannot tell the difference except by looking at how it is wired.

> On the other hand, no gauge can detect battery characteristics by its
> own circuitry. It has to be programmed by human hands. A dtb + this
> patch makes a tool for those hands.
> 

We don't need a tool for this, we have several open source tools for
this already for battery and device manufactures. This is the kernel
driver we are working on, it is for the user of hardware, not producer.

> And you don't trust device vendors? If a vendor records DT properties
> for a gauge, it's specifically because the gauge-resident config isn't
> correct.
> 

I trust the manufacturer of the device more than the Linux port I just
downloaded for the device, yes. Sometimes the manufacturer gets it
wrong, but this should be the exception, not the expected case.

>>>> Re DT provisioning... Device makers and distros are extremely careful
>>>> about what they put into DTs and what dtbs are included with a kernel
>>>> package. They know that a misconfigured DT is a showstopper, and could
>>>> be catastrophic. I've seen the latter -- a guru-level kernel
>>>> maintainer whom I work with set a DT voltage level wrong on a
>>>> prototype and fried its eMMC chip. If the DT has a gauge config, the
>>>> default assumption is that it's correct.
>>>>
>>
>> And if that eMMC chip could communicate its requested voltage we would
>> not need to describe it in DT, and we certainly would not say "sorry
>> eMMC, you requested 3v but DT says you need 33v, so thats what you're
>> getting".
> 
>>>> It is wrong to force distros and device makers to include
>>>> /etc/modprobe.d/xyz.conf to fully configure the hardware.
>>>>
>>
>> It's either that or add a driver sysfs entry, we don't configure
>> hardware in DT, thats rule #1. You got an ack for a binding that
> 
> Setting regulator voltage levels is not "configuring hardware"?
> 

No it is not, look at a regulator binding, in the regulator node we
describe the device's voltage limits, these voltage limits are a
physical property of the regulator. Then in the node for a device
powered by this regulator we describe the voltage it is designed to
operate at, this also is a physical description of a device, not
something we chose.

>> describes a "simple-battery"(one that cannot self report), you are
>> misusing this binding as input configuration data to feed to batteries
>> that *can* report their configuration.
> 
>>>> However we should indeed let users override the DT config with a
>>>> module param. How about use-on-chip-config=1, type bool?
>>>
>>
>> If we do add a param it needs to be default "off". It should not update
>> saved battery data on the chip in either case, that needs to be a
>> manually triggered through sysfs or similar.
>>
>>> I came up with...
>>>
>>> static bool bq27xxx_dt_battery = true;
>>> module_param_named(use_dt_battery, bq27xxx_dt_battery, bool, S_IRUGO);
>>> MODULE_PARM_DESC(use_dt_battery,
>>>  "Use devicetree monitored-battery config if present. Default is 1/Y.");
>>>
>>> After we check for DT monitored-battery, we test bq27xxx_dt_battery,
>>> and if false, emit a warning that we are skipping the DT config.
>>>

[PATCH v12.4 7/10] power: bq27xxx_battery: Enable chip data memory update for certain chips

[Liam Breck]

From: Liam Breck <kernel@networkimprov.net>

Support data memory update of BQ27500, 545, 421, 425, 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     | 73 +++++++++++++++++++++++++++++-
 drivers/power/supply/bq27xxx_battery_i2c.c | 16 +++----
 include/linux/power/bq27xxx_battery.h      | 11 +++++
 3 files changed, 90 insertions(+), 10 deletions(-)

diff --git a/drivers/power/supply/bq27xxx_battery.c b/drivers/power/supply/bq27xxx_battery.c
index 41eb5b7..3ff854e 100644
--- a/drivers/power/supply/bq27xxx_battery.c
+++ b/drivers/power/supply/bq27xxx_battery.c
@@ -57,7 +57,7 @@
 
 #include <linux/power/bq27xxx_battery.h>
 
-#define DRIVER_VERSION		"1.2.0"
+#define DRIVER_VERSION		"1.3.0"
 
 #define BQ27XXX_MANUFACTURER	"Texas Instruments"
 
@@ -855,6 +855,54 @@ static const char* 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 int poll_interval_param_set(const char *val, const struct kernel_param *kp)
 {
@@ -1830,9 +1878,30 @@ int bq27xxx_battery_setup(struct bq27xxx_device_info *di)
 		.drv_data = di,
 	};
 
+	di->unseal_key = bq27xxx_unseal_keys[di->chip];
+	di->dm_regs = bq27xxx_dm_regs[di->chip];
+
+	switch(di->chip) {  /* translate category members */
+	case BQ2752X:
+		di->chip = BQ27510G3; break;
+	case BQ27531:
+		di->chip = BQ27530;   break;
+	case BQ27542:
+	case BQ27546:
+	case BQ27742:
+		di->chip = BQ27541;   break;
+	case BQ27425:
+	case BQ27441:
+	case BQ27621:
+		di->chip = BQ27421;   break;
+	default:
+		break;
+	}
+
+	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)
diff --git a/drivers/power/supply/bq27xxx_battery_i2c.c b/drivers/power/supply/bq27xxx_battery_i2c.c
index a597221..0b11ed4 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 227eb08..7c9bf03 100644
--- a/include/linux/power/bq27xxx_battery.h
+++ b/include/linux/power/bq27xxx_battery.h
@@ -2,6 +2,7 @@
 #define __LINUX_BQ27X00_BATTERY_H__
 
 enum bq27xxx_chip {
+	/* category prototypes; index for bq27xxx_regs[] */
 	BQ27000 = 1, /* bq27000, bq27200 */
 	BQ27010, /* bq27010, bq27210 */
 	BQ2750X, /* bq27500 deprecated alias */
@@ -18,6 +19,16 @@ enum bq27xxx_chip {
 	BQ27541, /* bq27541, bq27542, bq27546, bq27742 */
 	BQ27545, /* bq27545 */
 	BQ27421, /* bq27421, bq27425, bq27441, bq27621 */
+
+	/* category members; translated to category prototype in _setup() */
+	BQ2752X, /* deprecated alias */
+	BQ27531,
+	BQ27542,
+	BQ27546,
+	BQ27742,
+	BQ27425,
+	BQ27441,
+	BQ27621,
 };
 
 /**
-- 
2.9.3

[PATCH v12.4 8/10] devicetree: power: bq27xxx: Add monitored-battery documentation

[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>
Acked-by: Sebastian Reichel <sre@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 b0c95ef..a4b62b3 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 devicetree 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.9.3

[PATCH v12.4 9/10] power: bq27xxx_battery: Flag identical register maps when in debug mode

[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 accident.

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 | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/drivers/power/supply/bq27xxx_battery.c b/drivers/power/supply/bq27xxx_battery.c
index 3ff854e..cc87984 100644
--- a/drivers/power/supply/bq27xxx_battery.c
+++ b/drivers/power/supply/bq27xxx_battery.c
@@ -1870,6 +1870,27 @@ static void bq27xxx_external_power_changed(struct power_supply *psy)
 	schedule_delayed_work(&di->work, 0);
 }
 
+#ifndef DEBUG /* ifdef after testing */
+static void bq27xxx_battery_find_reg_dupes(struct bq27xxx_device_info *di)
+{
+	const size_t max = ARRAY_SIZE(bq27xxx_regs);
+	int a, b;
+
+	dev_warn(di->dev, "checking %d reg maps\n", max);
+
+	for (a = 1; a < max; 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_find_reg_dupes(struct bq27xxx_device_info *di) {}
+#endif
+
 int bq27xxx_battery_setup(struct bq27xxx_device_info *di)
 {
 	struct power_supply_desc *psy_desc;
@@ -1878,6 +1899,8 @@ int bq27xxx_battery_setup(struct bq27xxx_device_info *di)
 		.drv_data = di,
 	};
 
+	bq27xxx_battery_find_reg_dupes(di);
+
 	di->unseal_key = bq27xxx_unseal_keys[di->chip];
 	di->dm_regs = bq27xxx_dm_regs[di->chip];
 
-- 
2.9.3

[PATCH v12.4 10/10] power: bq27xxx_battery: Remove duplicate register arrays

[Liam Breck]

From: Liam Breck <kernel@networkimprov.net>

BQ2751X & BQ27510G3 are identical.
BQ27500 & BQ27510G1 & BQ27510G2 are identical.
Make BQ2751X & BQ27510G1/2 category members.
Remove the duplicate arrays, etc.

Signed-off-by: Liam Breck <kernel@networkimprov.net>
---
 drivers/power/supply/bq27xxx_battery.c | 130 +--------------------------------
 include/linux/power/bq27xxx_battery.h  |  12 +--
 2 files changed, 10 insertions(+), 132 deletions(-)

diff --git a/drivers/power/supply/bq27xxx_battery.c b/drivers/power/supply/bq27xxx_battery.c
index cc87984..15eff77 100644
--- a/drivers/power/supply/bq27xxx_battery.c
+++ b/drivers/power/supply/bq27xxx_battery.c
@@ -200,26 +200,6 @@ static u8 bq27xxx_regs[][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,
@@ -240,46 +220,6 @@ static u8 bq27xxx_regs[][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,
@@ -522,24 +462,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,
@@ -561,48 +483,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,
@@ -782,10 +662,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),
@@ -1516,10 +1393,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:
@@ -1905,8 +1779,12 @@ int bq27xxx_battery_setup(struct bq27xxx_device_info *di)
 	di->dm_regs = bq27xxx_dm_regs[di->chip];
 
 	switch(di->chip) {  /* translate category members */
+	case BQ2751X:
 	case BQ2752X:
 		di->chip = BQ27510G3; break;
+	case BQ27510G1:
+	case BQ27510G2:
+		di->chip = BQ27500;   break;
 	case BQ27531:
 		di->chip = BQ27530;   break;
 	case BQ27542:
diff --git a/include/linux/power/bq27xxx_battery.h b/include/linux/power/bq27xxx_battery.h
index 7c9bf03..4233cae 100644
--- a/include/linux/power/bq27xxx_battery.h
+++ b/include/linux/power/bq27xxx_battery.h
@@ -6,11 +6,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 */
@@ -21,7 +18,10 @@ enum bq27xxx_chip {
 	BQ27421, /* bq27421, bq27425, bq27441, bq27621 */
 
 	/* category members; translated to category prototype in _setup() */
-	BQ2752X, /* deprecated alias */
+	BQ2751X, /* bq27510 deprecated alias */
+	BQ2752X, /* bq27520 deprecated alias */
+	BQ27510G1,
+	BQ27510G2,
 	BQ27531,
 	BQ27542,
 	BQ27546,
-- 
2.9.3

Re: [PATCH v12.4 0/7] bq27xxx_battery partial series

[Liam Breck]

Hi Andrew, any other feedback on this 12.4 series?

On Tue, Apr 4, 2017 at 1:56 AM, Liam Breck <liam@networkimprov.net> wrote:
> Latest changes:
>
> Devicetree docs mention relevant battery node fields.
>
> Clean up patch "Flag identical register maps..."
>
> ^.^ - I can has test?
>
>   power: bq27xxx_battery: Add bulk transfer bus methods
>   power: bq27xxx_battery: Add chip data memory read/write support
>   power: bq27xxx_battery: Add power_supply_battery_info support
>   power: bq27xxx_battery: Enable chip data memory update for certain chips
>   devicetree: power: bq27xxx: Add monitored-battery documentation
>   power: bq27xxx_battery: Flag identical register maps when in debug mode
>   power: bq27xxx_battery: Remove duplicate register arrays
>
>  .../devicetree/bindings/power/supply/bq27xxx.txt   |  31 +-
>  drivers/power/supply/bq27xxx_battery.c             | 704 +++++++++++++++++----
>  drivers/power/supply/bq27xxx_battery_i2c.c         |  98 ++-
>  include/linux/power/bq27xxx_battery.h              |  26 +-
>  4 files changed, 710 insertions(+), 149 deletions(-)
>
> --
> 2.9.3
>