* [PATCH v12 0/5] bq27xxx_battery partial series
@ 2017-03-27 7:41 Liam Breck
2017-03-27 7:41 ` [PATCH v12 5/9] power: bq27xxx_battery: Add bulk transfer bus methods Liam Breck
` (4 more replies)
0 siblings, 5 replies; 16+ messages in thread
From: Liam Breck @ 2017-03-27 7:41 UTC (permalink / raw)
To: Andrew F. Davis, linux-pm
Andrew, here are the bq27xxx related patches, revised and refactored from your feedback. I
refactored with a somewhat diff mix than you suggested for the sake of readability.
Changes prior to refactoring:
https://github.com/networkimprov/linux/commit/a9c984f0
https://github.com/networkimprov/linux/commit/59fb17ac
I didn't take these 5 suggestions:
/* BlockDataControl() -> Block Data Control, etc
I prefer terms from the docs so that searches in docs for the comment string work.
/* Reports EINVAL ...
This is obvious from the code.
Error reporting
You and Sebastian differ, so I dropped the patch, leaving dev_dbg in I/O methods. Pls resolve in a
future patchset.
checksum_dm_block()
That code came from bqtool, which I don't want to second guess.
/* This sequence is toxic
It's essential to document a specific feature which must not be added to the code. I revised the
comment.
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v12 5/9] power: bq27xxx_battery: Add bulk transfer bus methods
2017-03-27 7:41 [PATCH v12 0/5] bq27xxx_battery partial series Liam Breck
@ 2017-03-27 7:41 ` Liam Breck
2017-03-27 14:49 ` Andrew F. Davis
2017-03-27 7:41 ` [PATCH v12 6/9] power: bq27xxx_battery: Add chip data memory read/write support Liam Breck
` (3 subsequent siblings)
4 siblings, 1 reply; 16+ messages in thread
From: Liam Breck @ 2017-03-27 7:41 UTC (permalink / raw)
To: Andrew F. Davis, linux-pm; +Cc: Matt Ranostay, Liam Breck
From: Matt Ranostay <matt@ranostay.consulting>
Declare bus.write/read_bulk/write_bulk().
Add I2C write/read_bulk/write_bulk() to implement the above.
Add bq27xxx_write/read_block/write_block() helper functions to call the above.
Signed-off-by: Matt Ranostay <matt@ranostay.consulting>
Signed-off-by: Liam Breck <kernel@networkimprov.net>
---
drivers/power/supply/bq27xxx_battery.c | 55 +++++++++++++++++++-
drivers/power/supply/bq27xxx_battery_i2c.c | 82 +++++++++++++++++++++++++++++-
include/linux/power/bq27xxx_battery.h | 3 ++
3 files changed, 137 insertions(+), 3 deletions(-)
diff --git a/drivers/power/supply/bq27xxx_battery.c b/drivers/power/supply/bq27xxx_battery.c
index 398801a..f8c919e 100644
--- a/drivers/power/supply/bq27xxx_battery.c
+++ b/drivers/power/supply/bq27xxx_battery.c
@@ -794,11 +794,62 @@ 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 || !di->bus.write)
+ return -EINVAL;
+
+ 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 int bq27xxx_xfer(struct bq27xxx_device_info *di,
+ int (*xfer)(struct bq27xxx_device_info*, u8, u8*, int),
+ u8* data, int len)
+{
+ int ret;
+
+ if (!di || di->regs[BQ27XXX_DM_DATA] == INVALID_REG_ADDR || !xfer)
+ return -EINVAL;
+
+ ret = xfer(di, di->regs[BQ27XXX_DM_DATA], data, len);
+ if (ret < 0)
+ dev_dbg(di->dev, "failed to xfer register 0x%02x (index %d)\n",
+ di->regs[BQ27XXX_DM_DATA], BQ27XXX_DM_DATA);
+
+ return ret;
+}
+
+static inline int bq27xxx_read_block(struct bq27xxx_device_info *di,
+ u8* data, int len)
+{
+ return bq27xxx_xfer(di, di ? di->bus.read_bulk : 0, data, len);
+}
+
+static inline int bq27xxx_write_block(struct bq27xxx_device_info *di,
+ u8* data, int len)
+{
+ return bq27xxx_xfer(di, di ? di->bus.write_bulk : 0, data, len);
}
/*
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
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v12 6/9] power: bq27xxx_battery: Add chip data memory read/write support
2017-03-27 7:41 [PATCH v12 0/5] bq27xxx_battery partial series Liam Breck
2017-03-27 7:41 ` [PATCH v12 5/9] power: bq27xxx_battery: Add bulk transfer bus methods Liam Breck
@ 2017-03-27 7:41 ` Liam Breck
2017-03-27 14:58 ` Andrew F. Davis
2017-03-27 7:41 ` [PATCH v12 7/9] power: bq27xxx_battery: Add power_supply_battery_info support Liam Breck
` (2 subsequent siblings)
4 siblings, 1 reply; 16+ messages in thread
From: Liam Breck @ 2017-03-27 7:41 UTC (permalink / raw)
To: Andrew F. Davis, linux-pm; +Cc: Matt Ranostay, 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_update_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 | 360 ++++++++++++++++++++++++++++++++-
include/linux/power/bq27xxx_battery.h | 2 +
2 files changed, 361 insertions(+), 1 deletion(-)
diff --git a/drivers/power/supply/bq27xxx_battery.c b/drivers/power/supply/bq27xxx_battery.c
index f8c919e..ad152cc 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"
@@ -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,6 +115,11 @@ enum bq27xxx_reg_index {
BQ27XXX_REG_SOC, /* State-of-Charge */
BQ27XXX_REG_DCAP, /* Design Capacity */
BQ27XXX_REG_AP, /* Average Power */
+ BQ27XXX_DM_CTRL, /* BlockDataControl() */
+ BQ27XXX_DM_CLASS, /* DataClass() */
+ BQ27XXX_DM_BLOCK, /* DataBlock() */
+ BQ27XXX_DM_DATA, /* BlockData() */
+ BQ27XXX_DM_CKSUM, /* BlockDataChecksum() */
BQ27XXX_REG_MAX, /* sentinel */
};
@@ -131,6 +143,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 +167,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 +191,11 @@ static u8 bq27xxx_regs[][BQ27XXX_REG_MAX] = {
[BQ27XXX_REG_SOC] = 0x2c,
[BQ27XXX_REG_DCAP] = 0x3c,
[BQ27XXX_REG_AP] = INVALID_REG_ADDR,
+ [BQ27XXX_DM_CTRL] = 0x61,
+ [BQ27XXX_DM_CLASS] = 0x3e,
+ [BQ27XXX_DM_BLOCK] = 0x3f,
+ [BQ27XXX_DM_DATA] = 0x40,
+ [BQ27XXX_DM_CKSUM] = 0x60,
},
[BQ2751X] = {
[BQ27XXX_REG_CTRL] = 0x00,
@@ -188,6 +215,11 @@ static u8 bq27xxx_regs[][BQ27XXX_REG_MAX] = {
[BQ27XXX_REG_SOC] = 0x20,
[BQ27XXX_REG_DCAP] = 0x2e,
[BQ27XXX_REG_AP] = INVALID_REG_ADDR,
+ [BQ27XXX_DM_CTRL] = 0x61,
+ [BQ27XXX_DM_CLASS] = 0x3e,
+ [BQ27XXX_DM_BLOCK] = 0x3f,
+ [BQ27XXX_DM_DATA] = 0x40,
+ [BQ27XXX_DM_CKSUM] = 0x60,
},
[BQ27500] = {
[BQ27XXX_REG_CTRL] = 0x00,
@@ -207,6 +239,11 @@ static u8 bq27xxx_regs[][BQ27XXX_REG_MAX] = {
[BQ27XXX_REG_SOC] = 0x2c,
[BQ27XXX_REG_DCAP] = 0x3c,
[BQ27XXX_REG_AP] = 0x24,
+ [BQ27XXX_DM_CTRL] = 0x61,
+ [BQ27XXX_DM_CLASS] = 0x3e,
+ [BQ27XXX_DM_BLOCK] = 0x3f,
+ [BQ27XXX_DM_DATA] = 0x40,
+ [BQ27XXX_DM_CKSUM] = 0x60,
},
[BQ27510G1] = {
[BQ27XXX_REG_CTRL] = 0x00,
@@ -226,6 +263,11 @@ static u8 bq27xxx_regs[][BQ27XXX_REG_MAX] = {
[BQ27XXX_REG_SOC] = 0x2c,
[BQ27XXX_REG_DCAP] = 0x3c,
[BQ27XXX_REG_AP] = 0x24,
+ [BQ27XXX_DM_CTRL] = 0x61,
+ [BQ27XXX_DM_CLASS] = 0x3e,
+ [BQ27XXX_DM_BLOCK] = 0x3f,
+ [BQ27XXX_DM_DATA] = 0x40,
+ [BQ27XXX_DM_CKSUM] = 0x60,
},
[BQ27510G2] = {
[BQ27XXX_REG_CTRL] = 0x00,
@@ -245,6 +287,11 @@ static u8 bq27xxx_regs[][BQ27XXX_REG_MAX] = {
[BQ27XXX_REG_SOC] = 0x2c,
[BQ27XXX_REG_DCAP] = 0x3c,
[BQ27XXX_REG_AP] = 0x24,
+ [BQ27XXX_DM_CTRL] = 0x61,
+ [BQ27XXX_DM_CLASS] = 0x3e,
+ [BQ27XXX_DM_BLOCK] = 0x3f,
+ [BQ27XXX_DM_DATA] = 0x40,
+ [BQ27XXX_DM_CKSUM] = 0x60,
},
[BQ27510G3] = {
[BQ27XXX_REG_CTRL] = 0x00,
@@ -264,6 +311,11 @@ static u8 bq27xxx_regs[][BQ27XXX_REG_MAX] = {
[BQ27XXX_REG_SOC] = 0x20,
[BQ27XXX_REG_DCAP] = 0x2e,
[BQ27XXX_REG_AP] = INVALID_REG_ADDR,
+ [BQ27XXX_DM_CTRL] = 0x61,
+ [BQ27XXX_DM_CLASS] = 0x3e,
+ [BQ27XXX_DM_BLOCK] = 0x3f,
+ [BQ27XXX_DM_DATA] = 0x40,
+ [BQ27XXX_DM_CKSUM] = 0x60,
},
[BQ27520G1] = {
[BQ27XXX_REG_CTRL] = 0x00,
@@ -283,6 +335,11 @@ static u8 bq27xxx_regs[][BQ27XXX_REG_MAX] = {
[BQ27XXX_REG_SOC] = 0x2c,
[BQ27XXX_REG_DCAP] = 0x3c,
[BQ27XXX_REG_AP] = 0x24,
+ [BQ27XXX_DM_CTRL] = 0x61,
+ [BQ27XXX_DM_CLASS] = 0x3e,
+ [BQ27XXX_DM_BLOCK] = 0x3f,
+ [BQ27XXX_DM_DATA] = 0x40,
+ [BQ27XXX_DM_CKSUM] = 0x60,
},
[BQ27520G2] = {
[BQ27XXX_REG_CTRL] = 0x00,
@@ -302,6 +359,11 @@ static u8 bq27xxx_regs[][BQ27XXX_REG_MAX] = {
[BQ27XXX_REG_SOC] = 0x2c,
[BQ27XXX_REG_DCAP] = 0x3c,
[BQ27XXX_REG_AP] = 0x24,
+ [BQ27XXX_DM_CTRL] = 0x61,
+ [BQ27XXX_DM_CLASS] = 0x3e,
+ [BQ27XXX_DM_BLOCK] = 0x3f,
+ [BQ27XXX_DM_DATA] = 0x40,
+ [BQ27XXX_DM_CKSUM] = 0x60,
},
[BQ27520G3] = {
[BQ27XXX_REG_CTRL] = 0x00,
@@ -321,6 +383,11 @@ static u8 bq27xxx_regs[][BQ27XXX_REG_MAX] = {
[BQ27XXX_REG_SOC] = 0x2c,
[BQ27XXX_REG_DCAP] = 0x3c,
[BQ27XXX_REG_AP] = 0x24,
+ [BQ27XXX_DM_CTRL] = 0x61,
+ [BQ27XXX_DM_CLASS] = 0x3e,
+ [BQ27XXX_DM_BLOCK] = 0x3f,
+ [BQ27XXX_DM_DATA] = 0x40,
+ [BQ27XXX_DM_CKSUM] = 0x60,
},
[BQ27520G4] = {
[BQ27XXX_REG_CTRL] = 0x00,
@@ -340,6 +407,11 @@ 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_CTRL] = 0x61,
+ [BQ27XXX_DM_CLASS] = 0x3e,
+ [BQ27XXX_DM_BLOCK] = 0x3f,
+ [BQ27XXX_DM_DATA] = 0x40,
+ [BQ27XXX_DM_CKSUM] = 0x60,
},
[BQ27530] = {
[BQ27XXX_REG_CTRL] = 0x00,
@@ -359,6 +431,11 @@ static u8 bq27xxx_regs[][BQ27XXX_REG_MAX] = {
[BQ27XXX_REG_SOC] = 0x2c,
[BQ27XXX_REG_DCAP] = INVALID_REG_ADDR,
[BQ27XXX_REG_AP] = 0x24,
+ [BQ27XXX_DM_CTRL] = 0x61,
+ [BQ27XXX_DM_CLASS] = 0x3e,
+ [BQ27XXX_DM_BLOCK] = 0x3f,
+ [BQ27XXX_DM_DATA] = 0x40,
+ [BQ27XXX_DM_CKSUM] = 0x60,
},
[BQ27541] = {
[BQ27XXX_REG_CTRL] = 0x00,
@@ -378,6 +455,11 @@ static u8 bq27xxx_regs[][BQ27XXX_REG_MAX] = {
[BQ27XXX_REG_SOC] = 0x2c,
[BQ27XXX_REG_DCAP] = 0x3c,
[BQ27XXX_REG_AP] = 0x24,
+ [BQ27XXX_DM_CTRL] = 0x61,
+ [BQ27XXX_DM_CLASS] = 0x3e,
+ [BQ27XXX_DM_BLOCK] = 0x3f,
+ [BQ27XXX_DM_DATA] = 0x40,
+ [BQ27XXX_DM_CKSUM] = 0x60,
},
[BQ27545] = {
[BQ27XXX_REG_CTRL] = 0x00,
@@ -397,6 +479,11 @@ static u8 bq27xxx_regs[][BQ27XXX_REG_MAX] = {
[BQ27XXX_REG_SOC] = 0x2c,
[BQ27XXX_REG_DCAP] = INVALID_REG_ADDR,
[BQ27XXX_REG_AP] = 0x24,
+ [BQ27XXX_DM_CTRL] = 0x61,
+ [BQ27XXX_DM_CLASS] = 0x3e,
+ [BQ27XXX_DM_BLOCK] = 0x3f,
+ [BQ27XXX_DM_DATA] = 0x40,
+ [BQ27XXX_DM_CKSUM] = 0x60,
},
[BQ27421] = {
[BQ27XXX_REG_CTRL] = 0x00,
@@ -416,6 +503,11 @@ static u8 bq27xxx_regs[][BQ27XXX_REG_MAX] = {
[BQ27XXX_REG_SOC] = 0x1c,
[BQ27XXX_REG_DCAP] = 0x3c,
[BQ27XXX_REG_AP] = 0x18,
+ [BQ27XXX_DM_CTRL] = 0x61,
+ [BQ27XXX_DM_CLASS] = 0x3e,
+ [BQ27XXX_DM_BLOCK] = 0x3f,
+ [BQ27XXX_DM_DATA] = 0x40,
+ [BQ27XXX_DM_CKSUM] = 0x60,
},
};
@@ -757,6 +849,52 @@ 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_reg {
+ u8 subclass_id;
+ u8 offset;
+ u8 bytes;
+ u16 min, max;
+};
+
+struct bq27xxx_dm_buf {
+ u8 class;
+ u8 block;
+ u8 data[BQ27XXX_DM_SZ];
+ bool full, updt;
+};
+
+#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)
{
struct bq27xxx_device_info *di;
@@ -852,6 +990,226 @@ static inline int bq27xxx_write_block(struct bq27xxx_device_info *di,
return bq27xxx_xfer(di, di ? di->bus.write_bulk : 0, data, len);
}
+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)
+ return 0;
+
+ dev_err(di->dev, "bus error on seal: %d\n", ret);
+ return ret;
+}
+
+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;
+
+ 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, 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->full = true;
+ buf->updt = false;
+
+ return 0;
+
+out:
+ dev_err(di->dev, "bus error reading chip memory: %d\n", ret);
+ 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->full)
+ 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->updt = true;
+}
+
+static int bq27xxx_battery_set_cfgupdate(struct bq27xxx_device_info *di, u16 flag)
+{
+ const int limit = 100;
+ int ret, try = limit;
+
+ ret = bq27xxx_write(di, BQ27XXX_REG_CTRL,
+ flag ? 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) != flag && --try);
+
+ if (!try) {
+ dev_err(di->dev, "timed out waiting for cfgupdate flag %d\n", !!flag);
+ return -EINVAL;
+ }
+
+ if (limit-try > 3)
+ dev_warn(di->dev, "cfgupdate %d, retries %d\n", !!flag, limit-try);
+
+ return 0;
+
+out:
+ dev_err(di->dev, "bus error on %s: %d\n", flag ? "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->updt)
+ 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, 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 ADD THIS FEATURE, IT IS TOXIC.
+ * TI's bqtool reads checksum after above write:
+ *
+ * BQ27XXX_MSLEEP(time)
+ * bq27xxx_write(BQ27XXX_DM_BLOCK, buf->block)
+ * sum = bq27xxx_read(BQ27XXX_DM_CKSUM)
+ * if (sum != bq27xxx_battery_checksum_dm_block(buf))
+ * report error
+ *
+ * If the 'time' delay is insufficient, DM corruption results on
+ * the '425 chip (and perhaps others), which could damage the chip.
+ * 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->updt = 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..227eb08 100644
--- a/include/linux/power/bq27xxx_battery.h
+++ b/include/linux/power/bq27xxx_battery.h
@@ -64,6 +64,8 @@ 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;
int charge_design_full;
--
2.9.3
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v12 7/9] power: bq27xxx_battery: Add power_supply_battery_info support
2017-03-27 7:41 [PATCH v12 0/5] bq27xxx_battery partial series Liam Breck
2017-03-27 7:41 ` [PATCH v12 5/9] power: bq27xxx_battery: Add bulk transfer bus methods Liam Breck
2017-03-27 7:41 ` [PATCH v12 6/9] power: bq27xxx_battery: Add chip data memory read/write support Liam Breck
@ 2017-03-27 7:41 ` Liam Breck
2017-03-27 15:01 ` Andrew F. Davis
2017-03-27 7:41 ` [PATCH v12 8/9] power: bq27xxx_battery: Enable chip data memory update for certain chips Liam Breck
2017-03-27 7:41 ` [PATCH v12 9/9] power: bq27xxx_battery: Remove duplicate register arrays Liam Breck
4 siblings, 1 reply; 16+ messages in thread
From: Liam Breck @ 2017-03-27 7:41 UTC (permalink / raw)
To: 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.
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 | 105 ++++++++++++++++++++++++++++++++-
1 file changed, 104 insertions(+), 1 deletion(-)
diff --git a/drivers/power/supply/bq27xxx_battery.c b/drivers/power/supply/bq27xxx_battery.c
index ad152cc..d845ade 100644
--- a/drivers/power/supply/bq27xxx_battery.c
+++ b/drivers/power/supply/bq27xxx_battery.c
@@ -1210,6 +1210,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.
@@ -1727,6 +1819,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;
@@ -1760,7 +1859,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);
@@ -1785,6 +1887,7 @@ int bq27xxx_battery_setup(struct bq27xxx_device_info *di)
dev_info(di->dev, "support ver. %s enabled\n", DRIVER_VERSION);
+ bq27xxx_battery_settings(di);
bq27xxx_battery_update(di);
mutex_lock(&bq27xxx_list_lock);
--
2.9.3
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v12 8/9] power: bq27xxx_battery: Enable chip data memory update for certain chips
2017-03-27 7:41 [PATCH v12 0/5] bq27xxx_battery partial series Liam Breck
` (2 preceding siblings ...)
2017-03-27 7:41 ` [PATCH v12 7/9] power: bq27xxx_battery: Add power_supply_battery_info support Liam Breck
@ 2017-03-27 7:41 ` Liam Breck
2017-03-27 15:05 ` Andrew F. Davis
2017-03-27 7:41 ` [PATCH v12 9/9] power: bq27xxx_battery: Remove duplicate register arrays Liam Breck
4 siblings, 1 reply; 16+ messages in thread
From: Liam Breck @ 2017-03-27 7:41 UTC (permalink / raw)
To: Andrew F. Davis, linux-pm; +Cc: Liam Breck
From: Liam Breck <kernel@networkimprov.net>
Support data memory update of BQ27500, 545, 421, 425, 441, 621.
Pass actual chip ID into _setup() to allow support for previously unID'd chips.
Signed-off-by: Liam Breck <kernel@networkimprov.net>
---
drivers/power/supply/bq27xxx_battery.c | 70 ++++++++++++++++++++++++++++--
drivers/power/supply/bq27xxx_battery_i2c.c | 19 ++++----
include/linux/power/bq27xxx_battery.h | 13 +++++-
3 files changed, 88 insertions(+), 14 deletions(-)
diff --git a/drivers/power/supply/bq27xxx_battery.c b/drivers/power/supply/bq27xxx_battery.c
index d845ade..95f86ad 100644
--- a/drivers/power/supply/bq27xxx_battery.c
+++ b/drivers/power/supply/bq27xxx_battery.c
@@ -894,6 +894,36 @@ 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 int poll_interval_param_set(const char *val, const struct kernel_param *kp)
{
@@ -1856,7 +1886,12 @@ static void bq27xxx_external_power_changed(struct power_supply *psy)
schedule_delayed_work(&di->work, 0);
}
-int bq27xxx_battery_setup(struct bq27xxx_device_info *di)
+#define BQ27XXX_INIT(c,d,k) \
+ di->chip = (c); \
+ di->dm_regs = (d); \
+ di->unseal_key = (k)
+
+int bq27xxx_battery_setup(struct bq27xxx_device_info *di, enum bq27xxx_chip real_chip)
{
struct power_supply_desc *psy_desc;
struct power_supply_config psy_cfg = {
@@ -1864,6 +1899,36 @@ int bq27xxx_battery_setup(struct bq27xxx_device_info *di)
.drv_data = di,
};
+ switch(real_chip) {
+ /* categories */
+ case BQ27000: BQ27XXX_INIT(real_chip, 0, 0); break;
+ case BQ27010: BQ27XXX_INIT(real_chip, 0, 0); break;
+ case BQ2750X: BQ27XXX_INIT(real_chip, 0, 0); break;
+ case BQ27500: BQ27XXX_INIT(real_chip, bq27500_dm_regs, 0x04143672); break;
+ case BQ2751X: BQ27XXX_INIT(real_chip, 0, 0); break;
+ case BQ27510G1: BQ27XXX_INIT(real_chip, 0, 0); break;
+ case BQ27510G2: BQ27XXX_INIT(real_chip, 0, 0); break;
+ case BQ27510G3: BQ27XXX_INIT(real_chip, 0, 0); break;
+ case BQ27520G1: BQ27XXX_INIT(real_chip, 0, 0); break;
+ case BQ27520G2: BQ27XXX_INIT(real_chip, 0, 0); break;
+ case BQ27520G3: BQ27XXX_INIT(real_chip, 0, 0); break;
+ case BQ27520G4: BQ27XXX_INIT(real_chip, 0, 0); break;
+ case BQ27530: BQ27XXX_INIT(real_chip, 0, 0); break;
+ case BQ27541: BQ27XXX_INIT(real_chip, 0, 0); break;
+ case BQ27545: BQ27XXX_INIT(real_chip, bq27545_dm_regs, 0x04143672); break;
+ case BQ27421: BQ27XXX_INIT(real_chip, bq27421_dm_regs, 0x80008000); break;
+
+ /* members of categories */
+ case BQ2752X: BQ27XXX_INIT(BQ27510G3, 0, 0); break;
+ case BQ27531: BQ27XXX_INIT(BQ27530, 0, 0); break;
+ case BQ27542: BQ27XXX_INIT(BQ27541, 0, 0); break;
+ case BQ27546: BQ27XXX_INIT(BQ27541, 0, 0); break;
+ case BQ27742: BQ27XXX_INIT(BQ27541, 0, 0); break;
+ case BQ27425: BQ27XXX_INIT(BQ27421, bq27425_dm_regs, 0x04143672); break;
+ case BQ27441: BQ27XXX_INIT(BQ27421, bq27421_dm_regs, 0x80008000); break;
+ case BQ27621: BQ27XXX_INIT(BQ27421, bq27621_dm_regs, 0x80008000); break;
+ }
+
INIT_DELAYED_WORK(&di->work, bq27xxx_battery_poll);
mutex_init(&di->lock);
di->regs = bq27xxx_regs[di->chip];
@@ -1981,11 +2046,10 @@ static int bq27xxx_battery_platform_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, di);
di->dev = &pdev->dev;
- di->chip = pdata->chip;
di->name = pdata->name ?: dev_name(&pdev->dev);
di->bus.read = bq27xxx_battery_platform_read;
- return bq27xxx_battery_setup(di);
+ return bq27xxx_battery_setup(di, pdata->chip);
}
static int bq27xxx_battery_platform_remove(struct platform_device *pdev)
diff --git a/drivers/power/supply/bq27xxx_battery_i2c.c b/drivers/power/supply/bq27xxx_battery_i2c.c
index a597221..aaeddc0 100644
--- a/drivers/power/supply/bq27xxx_battery_i2c.c
+++ b/drivers/power/supply/bq27xxx_battery_i2c.c
@@ -169,7 +169,6 @@ static int bq27xxx_battery_i2c_probe(struct i2c_client *client,
di->id = num;
di->dev = &client->dev;
- di->chip = id->driver_data;
di->name = name;
di->bus.read = bq27xxx_battery_i2c_read;
@@ -177,7 +176,7 @@ static int bq27xxx_battery_i2c_probe(struct i2c_client *client,
di->bus.read_bulk = bq27xxx_battery_i2c_bulk_read;
di->bus.write_bulk = bq27xxx_battery_i2c_bulk_write;
- ret = bq27xxx_battery_setup(di);
+ ret = bq27xxx_battery_setup(di, id->driver_data);
if (ret)
goto err_failed;
@@ -230,7 +229,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 +239,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..4577727 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 {
+ /* categories; index for bq27xxx_regs[] */
BQ27000 = 1, /* bq27000, bq27200 */
BQ27010, /* bq27010, bq27210 */
BQ2750X, /* bq27500 deprecated alias */
@@ -18,6 +19,16 @@ enum bq27xxx_chip {
BQ27541, /* bq27541, bq27542, bq27546, bq27742 */
BQ27545, /* bq27545 */
BQ27421, /* bq27421, bq27425, bq27441, bq27621 */
+
+ /* members of categories; translate these to category in _setup() */
+ BQ2752X, /* deprecated alias */
+ BQ27531,
+ BQ27542,
+ BQ27546,
+ BQ27742,
+ BQ27425,
+ BQ27441,
+ BQ27621,
};
/**
@@ -78,7 +89,7 @@ struct bq27xxx_device_info {
};
void bq27xxx_battery_update(struct bq27xxx_device_info *di);
-int bq27xxx_battery_setup(struct bq27xxx_device_info *di);
+int bq27xxx_battery_setup(struct bq27xxx_device_info *di, enum bq27xxx_chip real_chip);
void bq27xxx_battery_teardown(struct bq27xxx_device_info *di);
#endif
--
2.9.3
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v12 9/9] power: bq27xxx_battery: Remove duplicate register arrays
2017-03-27 7:41 [PATCH v12 0/5] bq27xxx_battery partial series Liam Breck
` (3 preceding siblings ...)
2017-03-27 7:41 ` [PATCH v12 8/9] power: bq27xxx_battery: Enable chip data memory update for certain chips Liam Breck
@ 2017-03-27 7:41 ` Liam Breck
4 siblings, 0 replies; 16+ messages in thread
From: Liam Breck @ 2017-03-27 7:41 UTC (permalink / raw)
To: Andrew F. Davis, linux-pm; +Cc: 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 | 144 +--------------------------------
include/linux/power/bq27xxx_battery.h | 12 +--
2 files changed, 9 insertions(+), 147 deletions(-)
diff --git a/drivers/power/supply/bq27xxx_battery.c b/drivers/power/supply/bq27xxx_battery.c
index 95f86ad..c8c311f 100644
--- a/drivers/power/supply/bq27xxx_battery.c
+++ b/drivers/power/supply/bq27xxx_battery.c
@@ -197,30 +197,6 @@ static u8 bq27xxx_regs[][BQ27XXX_REG_MAX] = {
[BQ27XXX_DM_DATA] = 0x40,
[BQ27XXX_DM_CKSUM] = 0x60,
},
- [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_CTRL] = 0x61,
- [BQ27XXX_DM_CLASS] = 0x3e,
- [BQ27XXX_DM_BLOCK] = 0x3f,
- [BQ27XXX_DM_DATA] = 0x40,
- [BQ27XXX_DM_CKSUM] = 0x60,
- },
[BQ27500] = {
[BQ27XXX_REG_CTRL] = 0x00,
[BQ27XXX_REG_TEMP] = 0x06,
@@ -245,54 +221,6 @@ static u8 bq27xxx_regs[][BQ27XXX_REG_MAX] = {
[BQ27XXX_DM_DATA] = 0x40,
[BQ27XXX_DM_CKSUM] = 0x60,
},
- [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_CTRL] = 0x61,
- [BQ27XXX_DM_CLASS] = 0x3e,
- [BQ27XXX_DM_BLOCK] = 0x3f,
- [BQ27XXX_DM_DATA] = 0x40,
- [BQ27XXX_DM_CKSUM] = 0x60,
- },
- [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_CTRL] = 0x61,
- [BQ27XXX_DM_CLASS] = 0x3e,
- [BQ27XXX_DM_BLOCK] = 0x3f,
- [BQ27XXX_DM_DATA] = 0x40,
- [BQ27XXX_DM_CKSUM] = 0x60,
- },
[BQ27510G3] = {
[BQ27XXX_REG_CTRL] = 0x00,
[BQ27XXX_REG_TEMP] = 0x06,
@@ -571,24 +499,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,
@@ -610,48 +520,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,
@@ -831,10 +699,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),
@@ -1532,10 +1397,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,9 +1767,6 @@ int bq27xxx_battery_setup(struct bq27xxx_device_info *di, enum bq27xxx_chip real
case BQ27010: BQ27XXX_INIT(real_chip, 0, 0); break;
case BQ2750X: BQ27XXX_INIT(real_chip, 0, 0); break;
case BQ27500: BQ27XXX_INIT(real_chip, bq27500_dm_regs, 0x04143672); break;
- case BQ2751X: BQ27XXX_INIT(real_chip, 0, 0); break;
- case BQ27510G1: BQ27XXX_INIT(real_chip, 0, 0); break;
- case BQ27510G2: BQ27XXX_INIT(real_chip, 0, 0); break;
case BQ27510G3: BQ27XXX_INIT(real_chip, 0, 0); break;
case BQ27520G1: BQ27XXX_INIT(real_chip, 0, 0); break;
case BQ27520G2: BQ27XXX_INIT(real_chip, 0, 0); break;
@@ -1919,7 +1778,10 @@ int bq27xxx_battery_setup(struct bq27xxx_device_info *di, enum bq27xxx_chip real
case BQ27421: BQ27XXX_INIT(real_chip, bq27421_dm_regs, 0x80008000); break;
/* members of categories */
+ case BQ2751X: BQ27XXX_INIT(BQ27510G3, 0, 0); break;
case BQ2752X: BQ27XXX_INIT(BQ27510G3, 0, 0); break;
+ case BQ27510G1: BQ27XXX_INIT(BQ27500, 0, 0); break;
+ case BQ27510G2: BQ27XXX_INIT(BQ27500, 0, 0); break;
case BQ27531: BQ27XXX_INIT(BQ27530, 0, 0); break;
case BQ27542: BQ27XXX_INIT(BQ27541, 0, 0); break;
case BQ27546: BQ27XXX_INIT(BQ27541, 0, 0); break;
diff --git a/include/linux/power/bq27xxx_battery.h b/include/linux/power/bq27xxx_battery.h
index 4577727..485fbdf 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 */
/* members of categories; translate these to category in _setup() */
- BQ2752X, /* deprecated alias */
+ BQ2751X, /* bq27510 deprecated alias */
+ BQ2752X, /* bq27520 deprecated alias */
+ BQ27510G1,
+ BQ27510G2,
BQ27531,
BQ27542,
BQ27546,
--
2.9.3
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH v12 5/9] power: bq27xxx_battery: Add bulk transfer bus methods
2017-03-27 7:41 ` [PATCH v12 5/9] power: bq27xxx_battery: Add bulk transfer bus methods Liam Breck
@ 2017-03-27 14:49 ` Andrew F. Davis
2017-03-27 19:34 ` Liam Breck
0 siblings, 1 reply; 16+ messages in thread
From: Andrew F. Davis @ 2017-03-27 14:49 UTC (permalink / raw)
To: Liam Breck, linux-pm; +Cc: Matt Ranostay, Liam Breck
On 03/27/2017 02:41 AM, Liam Breck wrote:
> 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.
>
> Signed-off-by: Matt Ranostay <matt@ranostay.consulting>
> Signed-off-by: Liam Breck <kernel@networkimprov.net>
> ---
> drivers/power/supply/bq27xxx_battery.c | 55 +++++++++++++++++++-
> drivers/power/supply/bq27xxx_battery_i2c.c | 82 +++++++++++++++++++++++++++++-
> include/linux/power/bq27xxx_battery.h | 3 ++
> 3 files changed, 137 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/power/supply/bq27xxx_battery.c b/drivers/power/supply/bq27xxx_battery.c
> index 398801a..f8c919e 100644
> --- a/drivers/power/supply/bq27xxx_battery.c
> +++ b/drivers/power/supply/bq27xxx_battery.c
> @@ -794,11 +794,62 @@ 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 || !di->bus.write)
> + return -EINVAL;
> +
> + 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 int bq27xxx_xfer(struct bq27xxx_device_info *di,
> + int (*xfer)(struct bq27xxx_device_info*, u8, u8*, int),
> + u8* data, int len)
> +{
I'm not sure why you reverted this change from my suggested patch, it
only adds a couple lines to have a separate _read/write functions, but
it makes it 100x more readable. This way includes passing functions and
all kinds of ternary operators, this isn't a contest to see how clever
you can code, please keep the readable version.
> + int ret;
> +
> + if (!di || di->regs[BQ27XXX_DM_DATA] == INVALID_REG_ADDR || !xfer)
NAK, I removed the hard-coded register for a reason, BQ27XXX_DM_DATA is
not the only register you can bulk read. Let the caller pass in the
requested register like I showed you.
> + return -EINVAL;
> +
> + ret = xfer(di, di->regs[BQ27XXX_DM_DATA], data, len);
> + if (ret < 0)
> + dev_dbg(di->dev, "failed to xfer register 0x%02x (index %d)\n",
> + di->regs[BQ27XXX_DM_DATA], BQ27XXX_DM_DATA);
> +
> + return ret;
> +}
> +
> +static inline int bq27xxx_read_block(struct bq27xxx_device_info *di,
> + u8* data, int len)
> +{
> + return bq27xxx_xfer(di, di ? di->bus.read_bulk : 0, data, len);
> +}
> +
> +static inline int bq27xxx_write_block(struct bq27xxx_device_info *di,
> + u8* data, int len)
> +{
> + return bq27xxx_xfer(di, di ? di->bus.write_bulk : 0, data, len);
> }
>
> /*
> 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 {
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v12 6/9] power: bq27xxx_battery: Add chip data memory read/write support
2017-03-27 7:41 ` [PATCH v12 6/9] power: bq27xxx_battery: Add chip data memory read/write support Liam Breck
@ 2017-03-27 14:58 ` Andrew F. Davis
2017-03-27 19:52 ` Liam Breck
0 siblings, 1 reply; 16+ messages in thread
From: Andrew F. Davis @ 2017-03-27 14:58 UTC (permalink / raw)
To: Liam Breck, linux-pm; +Cc: Matt Ranostay, Liam Breck
On 03/27/2017 02:41 AM, Liam Breck wrote:
> 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_update_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 | 360 ++++++++++++++++++++++++++++++++-
> include/linux/power/bq27xxx_battery.h | 2 +
> 2 files changed, 361 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/power/supply/bq27xxx_battery.c b/drivers/power/supply/bq27xxx_battery.c
> index f8c919e..ad152cc 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"
>
Lets hold of on this until we change functionality, this patch should
not add functional changes.
> #define BQ27XXX_MANUFACTURER "Texas Instruments"
>
> @@ -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,6 +115,11 @@ enum bq27xxx_reg_index {
> BQ27XXX_REG_SOC, /* State-of-Charge */
> BQ27XXX_REG_DCAP, /* Design Capacity */
> BQ27XXX_REG_AP, /* Average Power */
> + BQ27XXX_DM_CTRL, /* BlockDataControl() */
> + BQ27XXX_DM_CLASS, /* DataClass() */
> + BQ27XXX_DM_BLOCK, /* DataBlock() */
> + BQ27XXX_DM_DATA, /* BlockData() */
> + BQ27XXX_DM_CKSUM, /* BlockDataChecksum() */
Not all data sheets will refer to them in this way, all the other
registers explain what they do, not how they are referred in some
version of some parts datasheet. This makes adding new parts easier as
we look for the register that does what the comment says it does, not
necessarily what the register is named.
> BQ27XXX_REG_MAX, /* sentinel */
> };
>
> @@ -131,6 +143,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 +167,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 +191,11 @@ static u8 bq27xxx_regs[][BQ27XXX_REG_MAX] = {
> [BQ27XXX_REG_SOC] = 0x2c,
> [BQ27XXX_REG_DCAP] = 0x3c,
> [BQ27XXX_REG_AP] = INVALID_REG_ADDR,
> + [BQ27XXX_DM_CTRL] = 0x61,
> + [BQ27XXX_DM_CLASS] = 0x3e,
> + [BQ27XXX_DM_BLOCK] = 0x3f,
> + [BQ27XXX_DM_DATA] = 0x40,
> + [BQ27XXX_DM_CKSUM] = 0x60,
> },
> [BQ2751X] = {
> [BQ27XXX_REG_CTRL] = 0x00,
> @@ -188,6 +215,11 @@ static u8 bq27xxx_regs[][BQ27XXX_REG_MAX] = {
> [BQ27XXX_REG_SOC] = 0x20,
> [BQ27XXX_REG_DCAP] = 0x2e,
> [BQ27XXX_REG_AP] = INVALID_REG_ADDR,
> + [BQ27XXX_DM_CTRL] = 0x61,
> + [BQ27XXX_DM_CLASS] = 0x3e,
> + [BQ27XXX_DM_BLOCK] = 0x3f,
> + [BQ27XXX_DM_DATA] = 0x40,
> + [BQ27XXX_DM_CKSUM] = 0x60,
> },
> [BQ27500] = {
> [BQ27XXX_REG_CTRL] = 0x00,
> @@ -207,6 +239,11 @@ static u8 bq27xxx_regs[][BQ27XXX_REG_MAX] = {
> [BQ27XXX_REG_SOC] = 0x2c,
> [BQ27XXX_REG_DCAP] = 0x3c,
> [BQ27XXX_REG_AP] = 0x24,
> + [BQ27XXX_DM_CTRL] = 0x61,
> + [BQ27XXX_DM_CLASS] = 0x3e,
> + [BQ27XXX_DM_BLOCK] = 0x3f,
> + [BQ27XXX_DM_DATA] = 0x40,
> + [BQ27XXX_DM_CKSUM] = 0x60,
> },
> [BQ27510G1] = {
> [BQ27XXX_REG_CTRL] = 0x00,
> @@ -226,6 +263,11 @@ static u8 bq27xxx_regs[][BQ27XXX_REG_MAX] = {
> [BQ27XXX_REG_SOC] = 0x2c,
> [BQ27XXX_REG_DCAP] = 0x3c,
> [BQ27XXX_REG_AP] = 0x24,
> + [BQ27XXX_DM_CTRL] = 0x61,
> + [BQ27XXX_DM_CLASS] = 0x3e,
> + [BQ27XXX_DM_BLOCK] = 0x3f,
> + [BQ27XXX_DM_DATA] = 0x40,
> + [BQ27XXX_DM_CKSUM] = 0x60,
> },
> [BQ27510G2] = {
> [BQ27XXX_REG_CTRL] = 0x00,
> @@ -245,6 +287,11 @@ static u8 bq27xxx_regs[][BQ27XXX_REG_MAX] = {
> [BQ27XXX_REG_SOC] = 0x2c,
> [BQ27XXX_REG_DCAP] = 0x3c,
> [BQ27XXX_REG_AP] = 0x24,
> + [BQ27XXX_DM_CTRL] = 0x61,
> + [BQ27XXX_DM_CLASS] = 0x3e,
> + [BQ27XXX_DM_BLOCK] = 0x3f,
> + [BQ27XXX_DM_DATA] = 0x40,
> + [BQ27XXX_DM_CKSUM] = 0x60,
> },
> [BQ27510G3] = {
> [BQ27XXX_REG_CTRL] = 0x00,
> @@ -264,6 +311,11 @@ static u8 bq27xxx_regs[][BQ27XXX_REG_MAX] = {
> [BQ27XXX_REG_SOC] = 0x20,
> [BQ27XXX_REG_DCAP] = 0x2e,
> [BQ27XXX_REG_AP] = INVALID_REG_ADDR,
> + [BQ27XXX_DM_CTRL] = 0x61,
> + [BQ27XXX_DM_CLASS] = 0x3e,
> + [BQ27XXX_DM_BLOCK] = 0x3f,
> + [BQ27XXX_DM_DATA] = 0x40,
> + [BQ27XXX_DM_CKSUM] = 0x60,
> },
> [BQ27520G1] = {
> [BQ27XXX_REG_CTRL] = 0x00,
> @@ -283,6 +335,11 @@ static u8 bq27xxx_regs[][BQ27XXX_REG_MAX] = {
> [BQ27XXX_REG_SOC] = 0x2c,
> [BQ27XXX_REG_DCAP] = 0x3c,
> [BQ27XXX_REG_AP] = 0x24,
> + [BQ27XXX_DM_CTRL] = 0x61,
> + [BQ27XXX_DM_CLASS] = 0x3e,
> + [BQ27XXX_DM_BLOCK] = 0x3f,
> + [BQ27XXX_DM_DATA] = 0x40,
> + [BQ27XXX_DM_CKSUM] = 0x60,
> },
> [BQ27520G2] = {
> [BQ27XXX_REG_CTRL] = 0x00,
> @@ -302,6 +359,11 @@ static u8 bq27xxx_regs[][BQ27XXX_REG_MAX] = {
> [BQ27XXX_REG_SOC] = 0x2c,
> [BQ27XXX_REG_DCAP] = 0x3c,
> [BQ27XXX_REG_AP] = 0x24,
> + [BQ27XXX_DM_CTRL] = 0x61,
> + [BQ27XXX_DM_CLASS] = 0x3e,
> + [BQ27XXX_DM_BLOCK] = 0x3f,
> + [BQ27XXX_DM_DATA] = 0x40,
> + [BQ27XXX_DM_CKSUM] = 0x60,
> },
> [BQ27520G3] = {
> [BQ27XXX_REG_CTRL] = 0x00,
> @@ -321,6 +383,11 @@ static u8 bq27xxx_regs[][BQ27XXX_REG_MAX] = {
> [BQ27XXX_REG_SOC] = 0x2c,
> [BQ27XXX_REG_DCAP] = 0x3c,
> [BQ27XXX_REG_AP] = 0x24,
> + [BQ27XXX_DM_CTRL] = 0x61,
> + [BQ27XXX_DM_CLASS] = 0x3e,
> + [BQ27XXX_DM_BLOCK] = 0x3f,
> + [BQ27XXX_DM_DATA] = 0x40,
> + [BQ27XXX_DM_CKSUM] = 0x60,
> },
> [BQ27520G4] = {
> [BQ27XXX_REG_CTRL] = 0x00,
> @@ -340,6 +407,11 @@ 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_CTRL] = 0x61,
> + [BQ27XXX_DM_CLASS] = 0x3e,
> + [BQ27XXX_DM_BLOCK] = 0x3f,
> + [BQ27XXX_DM_DATA] = 0x40,
> + [BQ27XXX_DM_CKSUM] = 0x60,
> },
> [BQ27530] = {
> [BQ27XXX_REG_CTRL] = 0x00,
> @@ -359,6 +431,11 @@ static u8 bq27xxx_regs[][BQ27XXX_REG_MAX] = {
> [BQ27XXX_REG_SOC] = 0x2c,
> [BQ27XXX_REG_DCAP] = INVALID_REG_ADDR,
> [BQ27XXX_REG_AP] = 0x24,
> + [BQ27XXX_DM_CTRL] = 0x61,
> + [BQ27XXX_DM_CLASS] = 0x3e,
> + [BQ27XXX_DM_BLOCK] = 0x3f,
> + [BQ27XXX_DM_DATA] = 0x40,
> + [BQ27XXX_DM_CKSUM] = 0x60,
> },
> [BQ27541] = {
> [BQ27XXX_REG_CTRL] = 0x00,
> @@ -378,6 +455,11 @@ static u8 bq27xxx_regs[][BQ27XXX_REG_MAX] = {
> [BQ27XXX_REG_SOC] = 0x2c,
> [BQ27XXX_REG_DCAP] = 0x3c,
> [BQ27XXX_REG_AP] = 0x24,
> + [BQ27XXX_DM_CTRL] = 0x61,
> + [BQ27XXX_DM_CLASS] = 0x3e,
> + [BQ27XXX_DM_BLOCK] = 0x3f,
> + [BQ27XXX_DM_DATA] = 0x40,
> + [BQ27XXX_DM_CKSUM] = 0x60,
> },
> [BQ27545] = {
> [BQ27XXX_REG_CTRL] = 0x00,
> @@ -397,6 +479,11 @@ static u8 bq27xxx_regs[][BQ27XXX_REG_MAX] = {
> [BQ27XXX_REG_SOC] = 0x2c,
> [BQ27XXX_REG_DCAP] = INVALID_REG_ADDR,
> [BQ27XXX_REG_AP] = 0x24,
> + [BQ27XXX_DM_CTRL] = 0x61,
> + [BQ27XXX_DM_CLASS] = 0x3e,
> + [BQ27XXX_DM_BLOCK] = 0x3f,
> + [BQ27XXX_DM_DATA] = 0x40,
> + [BQ27XXX_DM_CKSUM] = 0x60,
> },
> [BQ27421] = {
> [BQ27XXX_REG_CTRL] = 0x00,
> @@ -416,6 +503,11 @@ static u8 bq27xxx_regs[][BQ27XXX_REG_MAX] = {
> [BQ27XXX_REG_SOC] = 0x1c,
> [BQ27XXX_REG_DCAP] = 0x3c,
> [BQ27XXX_REG_AP] = 0x18,
> + [BQ27XXX_DM_CTRL] = 0x61,
> + [BQ27XXX_DM_CLASS] = 0x3e,
> + [BQ27XXX_DM_BLOCK] = 0x3f,
> + [BQ27XXX_DM_DATA] = 0x40,
> + [BQ27XXX_DM_CKSUM] = 0x60,
> },
> };
>
> @@ -757,6 +849,52 @@ 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_reg {
> + u8 subclass_id;
> + u8 offset;
> + u8 bytes;
> + u16 min, max;
> +};
> +
> +struct bq27xxx_dm_buf {
> + u8 class;
> + u8 block;
> + u8 data[BQ27XXX_DM_SZ];
> + bool full, updt;
> +};
> +
> +#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)
> {
> struct bq27xxx_device_info *di;
> @@ -852,6 +990,226 @@ static inline int bq27xxx_write_block(struct bq27xxx_device_info *di,
> return bq27xxx_xfer(di, di ? di->bus.write_bulk : 0, data, len);
> }
>
> +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)
> + return 0;
> +
> + dev_err(di->dev, "bus error on seal: %d\n", ret);
> + return ret;
> +}
> +
> +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;
> +
> + 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, 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->full = true;
> + buf->updt = false;
> +
> + return 0;
> +
> +out:
> + dev_err(di->dev, "bus error reading chip memory: %d\n", ret);
> + 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->full)
> + 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->updt = true;
> +}
> +
> +static int bq27xxx_battery_set_cfgupdate(struct bq27xxx_device_info *di, u16 flag)
> +{
> + const int limit = 100;
> + int ret, try = limit;
> +
> + ret = bq27xxx_write(di, BQ27XXX_REG_CTRL,
> + flag ? 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) != flag && --try);
> +
> + if (!try) {
> + dev_err(di->dev, "timed out waiting for cfgupdate flag %d\n", !!flag);
> + return -EINVAL;
> + }
> +
> + if (limit-try > 3)
> + dev_warn(di->dev, "cfgupdate %d, retries %d\n", !!flag, limit-try);
> +
> + return 0;
> +
> +out:
> + dev_err(di->dev, "bus error on %s: %d\n", flag ? "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->updt)
> + 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, 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 ADD THIS FEATURE, IT IS TOXIC.
DONT RUN system("rm -rf /"); !!!!
We know drivers can break hardware, simply say:
/*
* If the 'time' delay is insufficient, DM corruption results on
* the '425 chip (and perhaps others), which could damage the chip.
*/
This is more that sufficient, a working example of erasing your device's
NVM is not needed.
> + * TI's bqtool reads checksum after above write:
> + *
> + * BQ27XXX_MSLEEP(time)
> + * bq27xxx_write(BQ27XXX_DM_BLOCK, buf->block)
> + * sum = bq27xxx_read(BQ27XXX_DM_CKSUM)
> + * if (sum != bq27xxx_battery_checksum_dm_block(buf))
> + * report error
> + *
> + * If the 'time' delay is insufficient, DM corruption results on
> + * the '425 chip (and perhaps others), which could damage the chip.
> + * 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->updt = 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..227eb08 100644
> --- a/include/linux/power/bq27xxx_battery.h
> +++ b/include/linux/power/bq27xxx_battery.h
> @@ -64,6 +64,8 @@ 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;
> int charge_design_full;
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v12 7/9] power: bq27xxx_battery: Add power_supply_battery_info support
2017-03-27 7:41 ` [PATCH v12 7/9] power: bq27xxx_battery: Add power_supply_battery_info support Liam Breck
@ 2017-03-27 15:01 ` Andrew F. Davis
2017-03-27 20:52 ` Liam Breck
0 siblings, 1 reply; 16+ messages in thread
From: Andrew F. Davis @ 2017-03-27 15:01 UTC (permalink / raw)
To: Liam Breck, linux-pm; +Cc: Matt Ranostay, Liam Breck
On 03/27/2017 02:41 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).
>
How do we know if that is the correct thing to do? Maybe the values in
NVM are more correct than the DT and the DT values are just for
reference when NVM is not present. Updating NVM with DT values needs to
be controllable by the user, not automatic every time this driver probes.
> Signed-off-by: Matt Ranostay <matt@ranostay.consulting>
> Signed-off-by: Liam Breck <kernel@networkimprov.net>
> ---
> drivers/power/supply/bq27xxx_battery.c | 105 ++++++++++++++++++++++++++++++++-
> 1 file changed, 104 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/power/supply/bq27xxx_battery.c b/drivers/power/supply/bq27xxx_battery.c
> index ad152cc..d845ade 100644
> --- a/drivers/power/supply/bq27xxx_battery.c
> +++ b/drivers/power/supply/bq27xxx_battery.c
> @@ -1210,6 +1210,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.
> @@ -1727,6 +1819,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;
> @@ -1760,7 +1859,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);
> @@ -1785,6 +1887,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);
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v12 8/9] power: bq27xxx_battery: Enable chip data memory update for certain chips
2017-03-27 7:41 ` [PATCH v12 8/9] power: bq27xxx_battery: Enable chip data memory update for certain chips Liam Breck
@ 2017-03-27 15:05 ` Andrew F. Davis
2017-03-27 21:12 ` Liam Breck
0 siblings, 1 reply; 16+ messages in thread
From: Andrew F. Davis @ 2017-03-27 15:05 UTC (permalink / raw)
To: Liam Breck, linux-pm; +Cc: Liam Breck
On 03/27/2017 02:41 AM, Liam Breck wrote:
> From: Liam Breck <kernel@networkimprov.net>
>
> Support data memory update of BQ27500, 545, 421, 425, 441, 621.
>
> Pass actual chip ID into _setup() to allow support for previously unID'd chips.
>
> Signed-off-by: Liam Breck <kernel@networkimprov.net>
> ---
> drivers/power/supply/bq27xxx_battery.c | 70 ++++++++++++++++++++++++++++--
> drivers/power/supply/bq27xxx_battery_i2c.c | 19 ++++----
> include/linux/power/bq27xxx_battery.h | 13 +++++-
> 3 files changed, 88 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/power/supply/bq27xxx_battery.c b/drivers/power/supply/bq27xxx_battery.c
> index d845ade..95f86ad 100644
> --- a/drivers/power/supply/bq27xxx_battery.c
> +++ b/drivers/power/supply/bq27xxx_battery.c
> @@ -894,6 +894,36 @@ 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 int poll_interval_param_set(const char *val, const struct kernel_param *kp)
> {
> @@ -1856,7 +1886,12 @@ static void bq27xxx_external_power_changed(struct power_supply *psy)
> schedule_delayed_work(&di->work, 0);
> }
>
> -int bq27xxx_battery_setup(struct bq27xxx_device_info *di)
> +#define BQ27XXX_INIT(c,d,k) \
> + di->chip = (c); \
> + di->dm_regs = (d); \
> + di->unseal_key = (k)
These are static values per device, use a mapping table from chip ->
dm_regs/unseal_key, no need to fill in a static table at run-time.
> +
> +int bq27xxx_battery_setup(struct bq27xxx_device_info *di, enum bq27xxx_chip real_chip)
> {
> struct power_supply_desc *psy_desc;
> struct power_supply_config psy_cfg = {
> @@ -1864,6 +1899,36 @@ int bq27xxx_battery_setup(struct bq27xxx_device_info *di)
> .drv_data = di,
> };
>
> + switch(real_chip) {
> + /* categories */
> + case BQ27000: BQ27XXX_INIT(real_chip, 0, 0); break;
> + case BQ27010: BQ27XXX_INIT(real_chip, 0, 0); break;
> + case BQ2750X: BQ27XXX_INIT(real_chip, 0, 0); break;
> + case BQ27500: BQ27XXX_INIT(real_chip, bq27500_dm_regs, 0x04143672); break;
> + case BQ2751X: BQ27XXX_INIT(real_chip, 0, 0); break;
> + case BQ27510G1: BQ27XXX_INIT(real_chip, 0, 0); break;
> + case BQ27510G2: BQ27XXX_INIT(real_chip, 0, 0); break;
> + case BQ27510G3: BQ27XXX_INIT(real_chip, 0, 0); break;
> + case BQ27520G1: BQ27XXX_INIT(real_chip, 0, 0); break;
> + case BQ27520G2: BQ27XXX_INIT(real_chip, 0, 0); break;
> + case BQ27520G3: BQ27XXX_INIT(real_chip, 0, 0); break;
> + case BQ27520G4: BQ27XXX_INIT(real_chip, 0, 0); break;
> + case BQ27530: BQ27XXX_INIT(real_chip, 0, 0); break;
> + case BQ27541: BQ27XXX_INIT(real_chip, 0, 0); break;
> + case BQ27545: BQ27XXX_INIT(real_chip, bq27545_dm_regs, 0x04143672); break;
> + case BQ27421: BQ27XXX_INIT(real_chip, bq27421_dm_regs, 0x80008000); break;
> +
> + /* members of categories */
> + case BQ2752X: BQ27XXX_INIT(BQ27510G3, 0, 0); break;
> + case BQ27531: BQ27XXX_INIT(BQ27530, 0, 0); break;
> + case BQ27542: BQ27XXX_INIT(BQ27541, 0, 0); break;
> + case BQ27546: BQ27XXX_INIT(BQ27541, 0, 0); break;
> + case BQ27742: BQ27XXX_INIT(BQ27541, 0, 0); break;
> + case BQ27425: BQ27XXX_INIT(BQ27421, bq27425_dm_regs, 0x04143672); break;
> + case BQ27441: BQ27XXX_INIT(BQ27421, bq27421_dm_regs, 0x80008000); break;
> + case BQ27621: BQ27XXX_INIT(BQ27421, bq27621_dm_regs, 0x80008000); break;
> + }
> +
> INIT_DELAYED_WORK(&di->work, bq27xxx_battery_poll);
> mutex_init(&di->lock);
> di->regs = bq27xxx_regs[di->chip];
> @@ -1981,11 +2046,10 @@ static int bq27xxx_battery_platform_probe(struct platform_device *pdev)
> platform_set_drvdata(pdev, di);
>
> di->dev = &pdev->dev;
> - di->chip = pdata->chip;
> di->name = pdata->name ?: dev_name(&pdev->dev);
> di->bus.read = bq27xxx_battery_platform_read;
>
> - return bq27xxx_battery_setup(di);
> + return bq27xxx_battery_setup(di, pdata->chip);
> }
>
> static int bq27xxx_battery_platform_remove(struct platform_device *pdev)
> diff --git a/drivers/power/supply/bq27xxx_battery_i2c.c b/drivers/power/supply/bq27xxx_battery_i2c.c
> index a597221..aaeddc0 100644
> --- a/drivers/power/supply/bq27xxx_battery_i2c.c
> +++ b/drivers/power/supply/bq27xxx_battery_i2c.c
> @@ -169,7 +169,6 @@ static int bq27xxx_battery_i2c_probe(struct i2c_client *client,
>
> di->id = num;
> di->dev = &client->dev;
> - di->chip = id->driver_data;
> di->name = name;
>
> di->bus.read = bq27xxx_battery_i2c_read;
> @@ -177,7 +176,7 @@ static int bq27xxx_battery_i2c_probe(struct i2c_client *client,
> di->bus.read_bulk = bq27xxx_battery_i2c_bulk_read;
> di->bus.write_bulk = bq27xxx_battery_i2c_bulk_write;
>
> - ret = bq27xxx_battery_setup(di);
> + ret = bq27xxx_battery_setup(di, id->driver_data);
> if (ret)
> goto err_failed;
>
> @@ -230,7 +229,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 +239,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..4577727 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 {
> + /* categories; index for bq27xxx_regs[] */
> BQ27000 = 1, /* bq27000, bq27200 */
> BQ27010, /* bq27010, bq27210 */
> BQ2750X, /* bq27500 deprecated alias */
> @@ -18,6 +19,16 @@ enum bq27xxx_chip {
> BQ27541, /* bq27541, bq27542, bq27546, bq27742 */
> BQ27545, /* bq27545 */
> BQ27421, /* bq27421, bq27425, bq27441, bq27621 */
> +
> + /* members of categories; translate these to category in _setup() */
> + BQ2752X, /* deprecated alias */
> + BQ27531,
> + BQ27542,
> + BQ27546,
> + BQ27742,
> + BQ27425,
> + BQ27441,
> + BQ27621,
> };
>
> /**
> @@ -78,7 +89,7 @@ struct bq27xxx_device_info {
> };
>
> void bq27xxx_battery_update(struct bq27xxx_device_info *di);
> -int bq27xxx_battery_setup(struct bq27xxx_device_info *di);
> +int bq27xxx_battery_setup(struct bq27xxx_device_info *di, enum bq27xxx_chip real_chip);
struct bq27xxx_device_info contains all needed info, if you want more
info passed into battery_setup() then add that into bq27xxx_device_info.
> void bq27xxx_battery_teardown(struct bq27xxx_device_info *di);
>
> #endif
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v12 5/9] power: bq27xxx_battery: Add bulk transfer bus methods
2017-03-27 14:49 ` Andrew F. Davis
@ 2017-03-27 19:34 ` Liam Breck
0 siblings, 0 replies; 16+ messages in thread
From: Liam Breck @ 2017-03-27 19:34 UTC (permalink / raw)
To: Andrew F. Davis; +Cc: linux-pm, Matt Ranostay, Liam Breck
On Mon, Mar 27, 2017 at 7:49 AM, Andrew F. Davis <afd@ti.com> wrote:
> On 03/27/2017 02:41 AM, Liam Breck wrote:
>> 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.
>>
>> Signed-off-by: Matt Ranostay <matt@ranostay.consulting>
>> Signed-off-by: Liam Breck <kernel@networkimprov.net>
>> ---
>> drivers/power/supply/bq27xxx_battery.c | 55 +++++++++++++++++++-
>> drivers/power/supply/bq27xxx_battery_i2c.c | 82 +++++++++++++++++++++++++++++-
>> include/linux/power/bq27xxx_battery.h | 3 ++
>> 3 files changed, 137 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/power/supply/bq27xxx_battery.c b/drivers/power/supply/bq27xxx_battery.c
>> index 398801a..f8c919e 100644
>> --- a/drivers/power/supply/bq27xxx_battery.c
>> +++ b/drivers/power/supply/bq27xxx_battery.c
>> @@ -794,11 +794,62 @@ 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 || !di->bus.write)
>> + return -EINVAL;
>> +
>> + 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 int bq27xxx_xfer(struct bq27xxx_device_info *di,
>> + int (*xfer)(struct bq27xxx_device_info*, u8, u8*, int),
>> + u8* data, int len)
>> +{
>
> I'm not sure why you reverted this change from my suggested patch, it
So you know, I didn't revert anything per se; I copied changes out of a diff.
> only adds a couple lines to have a separate _read/write functions, but
> it makes it 100x more readable. This way includes passing functions and
> all kinds of ternary operators, this isn't a contest to see how clever
> you can code, please keep the readable version.
>
>> + int ret;
>> +
>> + if (!di || di->regs[BQ27XXX_DM_DATA] == INVALID_REG_ADDR || !xfer)
>
> NAK, I removed the hard-coded register for a reason, BQ27XXX_DM_DATA is
> not the only register you can bulk read. Let the caller pass in the
> requested register like I showed you.
>
>> + return -EINVAL;
>> +
>> + ret = xfer(di, di->regs[BQ27XXX_DM_DATA], data, len);
>> + if (ret < 0)
>> + dev_dbg(di->dev, "failed to xfer register 0x%02x (index %d)\n",
>> + di->regs[BQ27XXX_DM_DATA], BQ27XXX_DM_DATA);
>> +
>> + return ret;
>> +}
>> +
>> +static inline int bq27xxx_read_block(struct bq27xxx_device_info *di,
>> + u8* data, int len)
>> +{
>> + return bq27xxx_xfer(di, di ? di->bus.read_bulk : 0, data, len);
>> +}
>> +
>> +static inline int bq27xxx_write_block(struct bq27xxx_device_info *di,
>> + u8* data, int len)
>> +{
>> + return bq27xxx_xfer(di, di ? di->bus.write_bulk : 0, data, len);
>> }
>>
>> /*
>> 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 {
>>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v12 6/9] power: bq27xxx_battery: Add chip data memory read/write support
2017-03-27 14:58 ` Andrew F. Davis
@ 2017-03-27 19:52 ` Liam Breck
0 siblings, 0 replies; 16+ messages in thread
From: Liam Breck @ 2017-03-27 19:52 UTC (permalink / raw)
To: Andrew F. Davis; +Cc: linux-pm, Matt Ranostay, Liam Breck
On Mon, Mar 27, 2017 at 7:58 AM, Andrew F. Davis <afd@ti.com> wrote:
> On 03/27/2017 02:41 AM, Liam Breck wrote:
>> 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_update_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 | 360 ++++++++++++++++++++++++++++++++-
>> include/linux/power/bq27xxx_battery.h | 2 +
>> 2 files changed, 361 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/power/supply/bq27xxx_battery.c b/drivers/power/supply/bq27xxx_battery.c
>> index f8c919e..ad152cc 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"
>>
>
> Lets hold of on this until we change functionality, this patch should
> not add functional changes.
I'll move it to "Enable chip data memory update..." patch
>> #define BQ27XXX_MANUFACTURER "Texas Instruments"
>>
>> @@ -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,6 +115,11 @@ enum bq27xxx_reg_index {
>> BQ27XXX_REG_SOC, /* State-of-Charge */
>> BQ27XXX_REG_DCAP, /* Design Capacity */
>> BQ27XXX_REG_AP, /* Average Power */
>> + BQ27XXX_DM_CTRL, /* BlockDataControl() */
>> + BQ27XXX_DM_CLASS, /* DataClass() */
>> + BQ27XXX_DM_BLOCK, /* DataBlock() */
>> + BQ27XXX_DM_DATA, /* BlockData() */
>> + BQ27XXX_DM_CKSUM, /* BlockDataChecksum() */
>
> Not all data sheets will refer to them in this way, all the other
> registers explain what they do, not how they are referred in some
> version of some parts datasheet. This makes adding new parts easier as
> we look for the register that does what the comment says it does, not
> necessarily what the register is named.
>
>> BQ27XXX_REG_MAX, /* sentinel */
>> };
>>
>> @@ -131,6 +143,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 +167,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 +191,11 @@ static u8 bq27xxx_regs[][BQ27XXX_REG_MAX] = {
>> [BQ27XXX_REG_SOC] = 0x2c,
>> [BQ27XXX_REG_DCAP] = 0x3c,
>> [BQ27XXX_REG_AP] = INVALID_REG_ADDR,
>> + [BQ27XXX_DM_CTRL] = 0x61,
>> + [BQ27XXX_DM_CLASS] = 0x3e,
>> + [BQ27XXX_DM_BLOCK] = 0x3f,
>> + [BQ27XXX_DM_DATA] = 0x40,
>> + [BQ27XXX_DM_CKSUM] = 0x60,
>> },
>> [BQ2751X] = {
>> [BQ27XXX_REG_CTRL] = 0x00,
>> @@ -188,6 +215,11 @@ static u8 bq27xxx_regs[][BQ27XXX_REG_MAX] = {
>> [BQ27XXX_REG_SOC] = 0x20,
>> [BQ27XXX_REG_DCAP] = 0x2e,
>> [BQ27XXX_REG_AP] = INVALID_REG_ADDR,
>> + [BQ27XXX_DM_CTRL] = 0x61,
>> + [BQ27XXX_DM_CLASS] = 0x3e,
>> + [BQ27XXX_DM_BLOCK] = 0x3f,
>> + [BQ27XXX_DM_DATA] = 0x40,
>> + [BQ27XXX_DM_CKSUM] = 0x60,
>> },
>> [BQ27500] = {
>> [BQ27XXX_REG_CTRL] = 0x00,
>> @@ -207,6 +239,11 @@ static u8 bq27xxx_regs[][BQ27XXX_REG_MAX] = {
>> [BQ27XXX_REG_SOC] = 0x2c,
>> [BQ27XXX_REG_DCAP] = 0x3c,
>> [BQ27XXX_REG_AP] = 0x24,
>> + [BQ27XXX_DM_CTRL] = 0x61,
>> + [BQ27XXX_DM_CLASS] = 0x3e,
>> + [BQ27XXX_DM_BLOCK] = 0x3f,
>> + [BQ27XXX_DM_DATA] = 0x40,
>> + [BQ27XXX_DM_CKSUM] = 0x60,
>> },
>> [BQ27510G1] = {
>> [BQ27XXX_REG_CTRL] = 0x00,
>> @@ -226,6 +263,11 @@ static u8 bq27xxx_regs[][BQ27XXX_REG_MAX] = {
>> [BQ27XXX_REG_SOC] = 0x2c,
>> [BQ27XXX_REG_DCAP] = 0x3c,
>> [BQ27XXX_REG_AP] = 0x24,
>> + [BQ27XXX_DM_CTRL] = 0x61,
>> + [BQ27XXX_DM_CLASS] = 0x3e,
>> + [BQ27XXX_DM_BLOCK] = 0x3f,
>> + [BQ27XXX_DM_DATA] = 0x40,
>> + [BQ27XXX_DM_CKSUM] = 0x60,
>> },
>> [BQ27510G2] = {
>> [BQ27XXX_REG_CTRL] = 0x00,
>> @@ -245,6 +287,11 @@ static u8 bq27xxx_regs[][BQ27XXX_REG_MAX] = {
>> [BQ27XXX_REG_SOC] = 0x2c,
>> [BQ27XXX_REG_DCAP] = 0x3c,
>> [BQ27XXX_REG_AP] = 0x24,
>> + [BQ27XXX_DM_CTRL] = 0x61,
>> + [BQ27XXX_DM_CLASS] = 0x3e,
>> + [BQ27XXX_DM_BLOCK] = 0x3f,
>> + [BQ27XXX_DM_DATA] = 0x40,
>> + [BQ27XXX_DM_CKSUM] = 0x60,
>> },
>> [BQ27510G3] = {
>> [BQ27XXX_REG_CTRL] = 0x00,
>> @@ -264,6 +311,11 @@ static u8 bq27xxx_regs[][BQ27XXX_REG_MAX] = {
>> [BQ27XXX_REG_SOC] = 0x20,
>> [BQ27XXX_REG_DCAP] = 0x2e,
>> [BQ27XXX_REG_AP] = INVALID_REG_ADDR,
>> + [BQ27XXX_DM_CTRL] = 0x61,
>> + [BQ27XXX_DM_CLASS] = 0x3e,
>> + [BQ27XXX_DM_BLOCK] = 0x3f,
>> + [BQ27XXX_DM_DATA] = 0x40,
>> + [BQ27XXX_DM_CKSUM] = 0x60,
>> },
>> [BQ27520G1] = {
>> [BQ27XXX_REG_CTRL] = 0x00,
>> @@ -283,6 +335,11 @@ static u8 bq27xxx_regs[][BQ27XXX_REG_MAX] = {
>> [BQ27XXX_REG_SOC] = 0x2c,
>> [BQ27XXX_REG_DCAP] = 0x3c,
>> [BQ27XXX_REG_AP] = 0x24,
>> + [BQ27XXX_DM_CTRL] = 0x61,
>> + [BQ27XXX_DM_CLASS] = 0x3e,
>> + [BQ27XXX_DM_BLOCK] = 0x3f,
>> + [BQ27XXX_DM_DATA] = 0x40,
>> + [BQ27XXX_DM_CKSUM] = 0x60,
>> },
>> [BQ27520G2] = {
>> [BQ27XXX_REG_CTRL] = 0x00,
>> @@ -302,6 +359,11 @@ static u8 bq27xxx_regs[][BQ27XXX_REG_MAX] = {
>> [BQ27XXX_REG_SOC] = 0x2c,
>> [BQ27XXX_REG_DCAP] = 0x3c,
>> [BQ27XXX_REG_AP] = 0x24,
>> + [BQ27XXX_DM_CTRL] = 0x61,
>> + [BQ27XXX_DM_CLASS] = 0x3e,
>> + [BQ27XXX_DM_BLOCK] = 0x3f,
>> + [BQ27XXX_DM_DATA] = 0x40,
>> + [BQ27XXX_DM_CKSUM] = 0x60,
>> },
>> [BQ27520G3] = {
>> [BQ27XXX_REG_CTRL] = 0x00,
>> @@ -321,6 +383,11 @@ static u8 bq27xxx_regs[][BQ27XXX_REG_MAX] = {
>> [BQ27XXX_REG_SOC] = 0x2c,
>> [BQ27XXX_REG_DCAP] = 0x3c,
>> [BQ27XXX_REG_AP] = 0x24,
>> + [BQ27XXX_DM_CTRL] = 0x61,
>> + [BQ27XXX_DM_CLASS] = 0x3e,
>> + [BQ27XXX_DM_BLOCK] = 0x3f,
>> + [BQ27XXX_DM_DATA] = 0x40,
>> + [BQ27XXX_DM_CKSUM] = 0x60,
>> },
>> [BQ27520G4] = {
>> [BQ27XXX_REG_CTRL] = 0x00,
>> @@ -340,6 +407,11 @@ 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_CTRL] = 0x61,
>> + [BQ27XXX_DM_CLASS] = 0x3e,
>> + [BQ27XXX_DM_BLOCK] = 0x3f,
>> + [BQ27XXX_DM_DATA] = 0x40,
>> + [BQ27XXX_DM_CKSUM] = 0x60,
>> },
>> [BQ27530] = {
>> [BQ27XXX_REG_CTRL] = 0x00,
>> @@ -359,6 +431,11 @@ static u8 bq27xxx_regs[][BQ27XXX_REG_MAX] = {
>> [BQ27XXX_REG_SOC] = 0x2c,
>> [BQ27XXX_REG_DCAP] = INVALID_REG_ADDR,
>> [BQ27XXX_REG_AP] = 0x24,
>> + [BQ27XXX_DM_CTRL] = 0x61,
>> + [BQ27XXX_DM_CLASS] = 0x3e,
>> + [BQ27XXX_DM_BLOCK] = 0x3f,
>> + [BQ27XXX_DM_DATA] = 0x40,
>> + [BQ27XXX_DM_CKSUM] = 0x60,
>> },
>> [BQ27541] = {
>> [BQ27XXX_REG_CTRL] = 0x00,
>> @@ -378,6 +455,11 @@ static u8 bq27xxx_regs[][BQ27XXX_REG_MAX] = {
>> [BQ27XXX_REG_SOC] = 0x2c,
>> [BQ27XXX_REG_DCAP] = 0x3c,
>> [BQ27XXX_REG_AP] = 0x24,
>> + [BQ27XXX_DM_CTRL] = 0x61,
>> + [BQ27XXX_DM_CLASS] = 0x3e,
>> + [BQ27XXX_DM_BLOCK] = 0x3f,
>> + [BQ27XXX_DM_DATA] = 0x40,
>> + [BQ27XXX_DM_CKSUM] = 0x60,
>> },
>> [BQ27545] = {
>> [BQ27XXX_REG_CTRL] = 0x00,
>> @@ -397,6 +479,11 @@ static u8 bq27xxx_regs[][BQ27XXX_REG_MAX] = {
>> [BQ27XXX_REG_SOC] = 0x2c,
>> [BQ27XXX_REG_DCAP] = INVALID_REG_ADDR,
>> [BQ27XXX_REG_AP] = 0x24,
>> + [BQ27XXX_DM_CTRL] = 0x61,
>> + [BQ27XXX_DM_CLASS] = 0x3e,
>> + [BQ27XXX_DM_BLOCK] = 0x3f,
>> + [BQ27XXX_DM_DATA] = 0x40,
>> + [BQ27XXX_DM_CKSUM] = 0x60,
>> },
>> [BQ27421] = {
>> [BQ27XXX_REG_CTRL] = 0x00,
>> @@ -416,6 +503,11 @@ static u8 bq27xxx_regs[][BQ27XXX_REG_MAX] = {
>> [BQ27XXX_REG_SOC] = 0x1c,
>> [BQ27XXX_REG_DCAP] = 0x3c,
>> [BQ27XXX_REG_AP] = 0x18,
>> + [BQ27XXX_DM_CTRL] = 0x61,
>> + [BQ27XXX_DM_CLASS] = 0x3e,
>> + [BQ27XXX_DM_BLOCK] = 0x3f,
>> + [BQ27XXX_DM_DATA] = 0x40,
>> + [BQ27XXX_DM_CKSUM] = 0x60,
>> },
>> };
>>
>> @@ -757,6 +849,52 @@ 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_reg {
>> + u8 subclass_id;
>> + u8 offset;
>> + u8 bytes;
>> + u16 min, max;
>> +};
>> +
>> +struct bq27xxx_dm_buf {
>> + u8 class;
>> + u8 block;
>> + u8 data[BQ27XXX_DM_SZ];
>> + bool full, updt;
>> +};
>> +
>> +#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)
>> {
>> struct bq27xxx_device_info *di;
>> @@ -852,6 +990,226 @@ static inline int bq27xxx_write_block(struct bq27xxx_device_info *di,
>> return bq27xxx_xfer(di, di ? di->bus.write_bulk : 0, data, len);
>> }
>>
>> +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)
>> + return 0;
>> +
>> + dev_err(di->dev, "bus error on seal: %d\n", ret);
>> + return ret;
>> +}
>> +
>> +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;
>> +
>> + 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, 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->full = true;
>> + buf->updt = false;
>> +
>> + return 0;
>> +
>> +out:
>> + dev_err(di->dev, "bus error reading chip memory: %d\n", ret);
>> + 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->full)
>> + 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->updt = true;
>> +}
>> +
>> +static int bq27xxx_battery_set_cfgupdate(struct bq27xxx_device_info *di, u16 flag)
>> +{
>> + const int limit = 100;
>> + int ret, try = limit;
>> +
>> + ret = bq27xxx_write(di, BQ27XXX_REG_CTRL,
>> + flag ? 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) != flag && --try);
>> +
>> + if (!try) {
>> + dev_err(di->dev, "timed out waiting for cfgupdate flag %d\n", !!flag);
>> + return -EINVAL;
>> + }
>> +
>> + if (limit-try > 3)
>> + dev_warn(di->dev, "cfgupdate %d, retries %d\n", !!flag, limit-try);
>> +
>> + return 0;
>> +
>> +out:
>> + dev_err(di->dev, "bus error on %s: %d\n", flag ? "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->updt)
>> + 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, 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 ADD THIS FEATURE, IT IS TOXIC.
>
> DONT RUN system("rm -rf /"); !!!!
>
> We know drivers can break hardware, simply say:
>
> /*
> * If the 'time' delay is insufficient, DM corruption results on
> * the '425 chip (and perhaps others), which could damage the chip.
> */
>
> This is more that sufficient, a working example of erasing your device's
> NVM is not needed.
The comment without code would be, "Don't read BQ27XXX_DM_CKSUM here.
It may trigger DM corruption..."
>> + * TI's bqtool reads checksum after above write:
>> + *
>> + * BQ27XXX_MSLEEP(time)
>> + * bq27xxx_write(BQ27XXX_DM_BLOCK, buf->block)
>> + * sum = bq27xxx_read(BQ27XXX_DM_CKSUM)
>> + * if (sum != bq27xxx_battery_checksum_dm_block(buf))
>> + * report error
>> + *
>> + * If the 'time' delay is insufficient, DM corruption results on
>> + * the '425 chip (and perhaps others), which could damage the chip.
>> + * 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->updt = 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..227eb08 100644
>> --- a/include/linux/power/bq27xxx_battery.h
>> +++ b/include/linux/power/bq27xxx_battery.h
>> @@ -64,6 +64,8 @@ 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;
>> int charge_design_full;
>>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v12 7/9] power: bq27xxx_battery: Add power_supply_battery_info support
2017-03-27 15:01 ` Andrew F. Davis
@ 2017-03-27 20:52 ` Liam Breck
2017-03-28 6:29 ` Liam Breck
2017-03-28 19:19 ` Liam Breck
0 siblings, 2 replies; 16+ messages in thread
From: Liam Breck @ 2017-03-27 20:52 UTC (permalink / raw)
To: Andrew F. Davis; +Cc: linux-pm, Matt Ranostay, Liam Breck
On Mon, Mar 27, 2017 at 8:01 AM, Andrew F. Davis <afd@ti.com> wrote:
> On 03/27/2017 02:41 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).
>>
>
> How do we know if that is the correct thing to do? Maybe the values in
> NVM are more correct than the DT and the DT values are just for
> reference when NVM is not present. Updating NVM with DT values needs to
> be controllable by the user, not automatic every time this driver probes.
We trust the OS package to contain correct config for RAM-only chips,
but not NVM? A battery config would not be set in mainline dts files,
unless the battery is non-removable in the device.
If NVM is unexpectedly present, then the entire device is unexpected,
and you have bigger issues than battery config.
We could prevent a DT referencing a different gauge than is present
from taking effect by checking device_type via control register. But
it's not clear that second-guessing the DT is wise.
>> Signed-off-by: Matt Ranostay <matt@ranostay.consulting>
>> Signed-off-by: Liam Breck <kernel@networkimprov.net>
>> ---
>> drivers/power/supply/bq27xxx_battery.c | 105 ++++++++++++++++++++++++++++++++-
>> 1 file changed, 104 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/power/supply/bq27xxx_battery.c b/drivers/power/supply/bq27xxx_battery.c
>> index ad152cc..d845ade 100644
>> --- a/drivers/power/supply/bq27xxx_battery.c
>> +++ b/drivers/power/supply/bq27xxx_battery.c
>> @@ -1210,6 +1210,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.
>> @@ -1727,6 +1819,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;
>> @@ -1760,7 +1859,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);
>> @@ -1785,6 +1887,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);
>>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v12 8/9] power: bq27xxx_battery: Enable chip data memory update for certain chips
2017-03-27 15:05 ` Andrew F. Davis
@ 2017-03-27 21:12 ` Liam Breck
0 siblings, 0 replies; 16+ messages in thread
From: Liam Breck @ 2017-03-27 21:12 UTC (permalink / raw)
To: Andrew F. Davis; +Cc: linux-pm, Liam Breck
On Mon, Mar 27, 2017 at 8:05 AM, Andrew F. Davis <afd@ti.com> wrote:
> On 03/27/2017 02:41 AM, Liam Breck wrote:
>> From: Liam Breck <kernel@networkimprov.net>
>>
>> Support data memory update of BQ27500, 545, 421, 425, 441, 621.
>>
>> Pass actual chip ID into _setup() to allow support for previously unID'd chips.
>>
>> Signed-off-by: Liam Breck <kernel@networkimprov.net>
>> ---
>> drivers/power/supply/bq27xxx_battery.c | 70 ++++++++++++++++++++++++++++--
>> drivers/power/supply/bq27xxx_battery_i2c.c | 19 ++++----
>> include/linux/power/bq27xxx_battery.h | 13 +++++-
>> 3 files changed, 88 insertions(+), 14 deletions(-)
>>
>> diff --git a/drivers/power/supply/bq27xxx_battery.c b/drivers/power/supply/bq27xxx_battery.c
>> index d845ade..95f86ad 100644
>> --- a/drivers/power/supply/bq27xxx_battery.c
>> +++ b/drivers/power/supply/bq27xxx_battery.c
>> @@ -894,6 +894,36 @@ 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 int poll_interval_param_set(const char *val, const struct kernel_param *kp)
>> {
>> @@ -1856,7 +1886,12 @@ static void bq27xxx_external_power_changed(struct power_supply *psy)
>> schedule_delayed_work(&di->work, 0);
>> }
>>
>> -int bq27xxx_battery_setup(struct bq27xxx_device_info *di)
>> +#define BQ27XXX_INIT(c,d,k) \
>> + di->chip = (c); \
>> + di->dm_regs = (d); \
>> + di->unseal_key = (k)
>
> These are static values per device, use a mapping table from chip ->
> dm_regs/unseal_key, no need to fill in a static table at run-time.
>
>> +
>> +int bq27xxx_battery_setup(struct bq27xxx_device_info *di, enum bq27xxx_chip real_chip)
>> {
>> struct power_supply_desc *psy_desc;
>> struct power_supply_config psy_cfg = {
>> @@ -1864,6 +1899,36 @@ int bq27xxx_battery_setup(struct bq27xxx_device_info *di)
>> .drv_data = di,
>> };
>>
>> + switch(real_chip) {
>> + /* categories */
>> + case BQ27000: BQ27XXX_INIT(real_chip, 0, 0); break;
>> + case BQ27010: BQ27XXX_INIT(real_chip, 0, 0); break;
>> + case BQ2750X: BQ27XXX_INIT(real_chip, 0, 0); break;
>> + case BQ27500: BQ27XXX_INIT(real_chip, bq27500_dm_regs, 0x04143672); break;
>> + case BQ2751X: BQ27XXX_INIT(real_chip, 0, 0); break;
>> + case BQ27510G1: BQ27XXX_INIT(real_chip, 0, 0); break;
>> + case BQ27510G2: BQ27XXX_INIT(real_chip, 0, 0); break;
>> + case BQ27510G3: BQ27XXX_INIT(real_chip, 0, 0); break;
>> + case BQ27520G1: BQ27XXX_INIT(real_chip, 0, 0); break;
>> + case BQ27520G2: BQ27XXX_INIT(real_chip, 0, 0); break;
>> + case BQ27520G3: BQ27XXX_INIT(real_chip, 0, 0); break;
>> + case BQ27520G4: BQ27XXX_INIT(real_chip, 0, 0); break;
>> + case BQ27530: BQ27XXX_INIT(real_chip, 0, 0); break;
>> + case BQ27541: BQ27XXX_INIT(real_chip, 0, 0); break;
>> + case BQ27545: BQ27XXX_INIT(real_chip, bq27545_dm_regs, 0x04143672); break;
>> + case BQ27421: BQ27XXX_INIT(real_chip, bq27421_dm_regs, 0x80008000); break;
>> +
>> + /* members of categories */
>> + case BQ2752X: BQ27XXX_INIT(BQ27510G3, 0, 0); break;
>> + case BQ27531: BQ27XXX_INIT(BQ27530, 0, 0); break;
>> + case BQ27542: BQ27XXX_INIT(BQ27541, 0, 0); break;
>> + case BQ27546: BQ27XXX_INIT(BQ27541, 0, 0); break;
>> + case BQ27742: BQ27XXX_INIT(BQ27541, 0, 0); break;
>> + case BQ27425: BQ27XXX_INIT(BQ27421, bq27425_dm_regs, 0x04143672); break;
>> + case BQ27441: BQ27XXX_INIT(BQ27421, bq27421_dm_regs, 0x80008000); break;
>> + case BQ27621: BQ27XXX_INIT(BQ27421, bq27621_dm_regs, 0x80008000); break;
>> + }
>> +
>> INIT_DELAYED_WORK(&di->work, bq27xxx_battery_poll);
>> mutex_init(&di->lock);
>> di->regs = bq27xxx_regs[di->chip];
>> @@ -1981,11 +2046,10 @@ static int bq27xxx_battery_platform_probe(struct platform_device *pdev)
>> platform_set_drvdata(pdev, di);
>>
>> di->dev = &pdev->dev;
>> - di->chip = pdata->chip;
>> di->name = pdata->name ?: dev_name(&pdev->dev);
>> di->bus.read = bq27xxx_battery_platform_read;
>>
>> - return bq27xxx_battery_setup(di);
>> + return bq27xxx_battery_setup(di, pdata->chip);
>> }
>>
>> static int bq27xxx_battery_platform_remove(struct platform_device *pdev)
>> diff --git a/drivers/power/supply/bq27xxx_battery_i2c.c b/drivers/power/supply/bq27xxx_battery_i2c.c
>> index a597221..aaeddc0 100644
>> --- a/drivers/power/supply/bq27xxx_battery_i2c.c
>> +++ b/drivers/power/supply/bq27xxx_battery_i2c.c
>> @@ -169,7 +169,6 @@ static int bq27xxx_battery_i2c_probe(struct i2c_client *client,
>>
>> di->id = num;
>> di->dev = &client->dev;
>> - di->chip = id->driver_data;
>> di->name = name;
>>
>> di->bus.read = bq27xxx_battery_i2c_read;
>> @@ -177,7 +176,7 @@ static int bq27xxx_battery_i2c_probe(struct i2c_client *client,
>> di->bus.read_bulk = bq27xxx_battery_i2c_bulk_read;
>> di->bus.write_bulk = bq27xxx_battery_i2c_bulk_write;
>>
>> - ret = bq27xxx_battery_setup(di);
>> + ret = bq27xxx_battery_setup(di, id->driver_data);
>> if (ret)
>> goto err_failed;
>>
>> @@ -230,7 +229,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 +239,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..4577727 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 {
>> + /* categories; index for bq27xxx_regs[] */
>> BQ27000 = 1, /* bq27000, bq27200 */
>> BQ27010, /* bq27010, bq27210 */
>> BQ2750X, /* bq27500 deprecated alias */
>> @@ -18,6 +19,16 @@ enum bq27xxx_chip {
>> BQ27541, /* bq27541, bq27542, bq27546, bq27742 */
>> BQ27545, /* bq27545 */
>> BQ27421, /* bq27421, bq27425, bq27441, bq27621 */
>> +
>> + /* members of categories; translate these to category in _setup() */
>> + BQ2752X, /* deprecated alias */
>> + BQ27531,
>> + BQ27542,
>> + BQ27546,
>> + BQ27742,
>> + BQ27425,
>> + BQ27441,
>> + BQ27621,
>> };
>>
>> /**
>> @@ -78,7 +89,7 @@ struct bq27xxx_device_info {
>> };
>>
>> void bq27xxx_battery_update(struct bq27xxx_device_info *di);
>> -int bq27xxx_battery_setup(struct bq27xxx_device_info *di);
>> +int bq27xxx_battery_setup(struct bq27xxx_device_info *di, enum bq27xxx_chip real_chip);
>
> struct bq27xxx_device_info contains all needed info, if you want more
> info passed into battery_setup() then add that into bq27xxx_device_info.
So add di->realchip, even tho it isn't used outside _setup() ? It's
easier to use di->chip and alter it in setup, but Sebastian complained
about that.
>> void bq27xxx_battery_teardown(struct bq27xxx_device_info *di);
>>
>> #endif
>>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v12 7/9] power: bq27xxx_battery: Add power_supply_battery_info support
2017-03-27 20:52 ` Liam Breck
@ 2017-03-28 6:29 ` Liam Breck
2017-03-28 19:19 ` Liam Breck
1 sibling, 0 replies; 16+ messages in thread
From: Liam Breck @ 2017-03-28 6:29 UTC (permalink / raw)
To: Andrew F. Davis; +Cc: linux-pm, Matt Ranostay, Liam Breck
On Mon, Mar 27, 2017 at 1:52 PM, Liam Breck <liam@networkimprov.net> wrote:
> On Mon, Mar 27, 2017 at 8:01 AM, Andrew F. Davis <afd@ti.com> wrote:
>> On 03/27/2017 02:41 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).
>>>
>>
>> How do we know if that is the correct thing to do? Maybe the values in
>> NVM are more correct than the DT and the DT values are just for
>> reference when NVM is not present. Updating NVM with DT values needs to
>> be controllable by the user, not automatic every time this driver probes.
>
> We trust the OS package to contain correct config for RAM-only chips,
> but not NVM? A battery config would not be set in mainline dts files,
> unless the battery is non-removable in the device.
>
> If NVM is unexpectedly present, then the entire device is unexpected,
> and you have bigger issues than battery config.
>
> We could prevent a DT referencing a different gauge than is present
> from taking effect by checking device_type via control register. But
> it's not clear that second-guessing the DT is wise.
But the control register device_type values are shared among diff
chips, so that won't work.
I addressed all your other comments. Latest draft at
https://github.com/networkimprov/linux/commits/anvl-v4.9-bq27x-dt-12.1
>>> Signed-off-by: Matt Ranostay <matt@ranostay.consulting>
>>> Signed-off-by: Liam Breck <kernel@networkimprov.net>
>>> ---
>>> drivers/power/supply/bq27xxx_battery.c | 105 ++++++++++++++++++++++++++++++++-
>>> 1 file changed, 104 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/power/supply/bq27xxx_battery.c b/drivers/power/supply/bq27xxx_battery.c
>>> index ad152cc..d845ade 100644
>>> --- a/drivers/power/supply/bq27xxx_battery.c
>>> +++ b/drivers/power/supply/bq27xxx_battery.c
>>> @@ -1210,6 +1210,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.
>>> @@ -1727,6 +1819,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;
>>> @@ -1760,7 +1859,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);
>>> @@ -1785,6 +1887,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);
>>>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v12 7/9] power: bq27xxx_battery: Add power_supply_battery_info support
2017-03-27 20:52 ` Liam Breck
2017-03-28 6:29 ` Liam Breck
@ 2017-03-28 19:19 ` Liam Breck
1 sibling, 0 replies; 16+ messages in thread
From: Liam Breck @ 2017-03-28 19:19 UTC (permalink / raw)
To: Andrew F. Davis; +Cc: linux-pm
On Mon, Mar 27, 2017 at 1:52 PM, Liam Breck <liam@networkimprov.net> wrote:
> On Mon, Mar 27, 2017 at 8:01 AM, Andrew F. Davis <afd@ti.com> wrote:
>> On 03/27/2017 02:41 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).
>>>
>>
>> How do we know if that is the correct thing to do? Maybe the values in
>> NVM are more correct than the DT and the DT values are just for
>> reference when NVM is not present. Updating NVM with DT values needs to
>> be controllable by the user, not automatic every time this driver probes.
>
> We trust the OS package to contain correct config for RAM-only chips,
> but not NVM? A battery config would not be set in mainline dts files,
> unless the battery is non-removable in the device.
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 -- an experienced kernel
maintainer whom I work with gave the DT for a prototype a wrong
voltage value and fried its eMMC chip.
So if the DT says use xyz gauge config, we jolly well better do it :-)
> If NVM is unexpectedly present, then the entire device is unexpected,
> and you have bigger issues than battery config.
>
> We could prevent a DT referencing a different gauge than is present
> from taking effect by checking device_type via control register. But
> it's not clear that second-guessing the DT is wise.
>
>
>>> Signed-off-by: Matt Ranostay <matt@ranostay.consulting>
>>> Signed-off-by: Liam Breck <kernel@networkimprov.net>
>>> ---
>>> drivers/power/supply/bq27xxx_battery.c | 105 ++++++++++++++++++++++++++++++++-
>>> 1 file changed, 104 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/power/supply/bq27xxx_battery.c b/drivers/power/supply/bq27xxx_battery.c
>>> index ad152cc..d845ade 100644
>>> --- a/drivers/power/supply/bq27xxx_battery.c
>>> +++ b/drivers/power/supply/bq27xxx_battery.c
>>> @@ -1210,6 +1210,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.
>>> @@ -1727,6 +1819,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;
>>> @@ -1760,7 +1859,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);
>>> @@ -1785,6 +1887,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);
>>>
^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2017-03-28 19:19 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-03-27 7:41 [PATCH v12 0/5] bq27xxx_battery partial series Liam Breck
2017-03-27 7:41 ` [PATCH v12 5/9] power: bq27xxx_battery: Add bulk transfer bus methods Liam Breck
2017-03-27 14:49 ` Andrew F. Davis
2017-03-27 19:34 ` Liam Breck
2017-03-27 7:41 ` [PATCH v12 6/9] power: bq27xxx_battery: Add chip data memory read/write support Liam Breck
2017-03-27 14:58 ` Andrew F. Davis
2017-03-27 19:52 ` Liam Breck
2017-03-27 7:41 ` [PATCH v12 7/9] power: bq27xxx_battery: Add power_supply_battery_info support Liam Breck
2017-03-27 15:01 ` Andrew F. Davis
2017-03-27 20:52 ` Liam Breck
2017-03-28 6:29 ` Liam Breck
2017-03-28 19:19 ` Liam Breck
2017-03-27 7:41 ` [PATCH v12 8/9] power: bq27xxx_battery: Enable chip data memory update for certain chips Liam Breck
2017-03-27 15:05 ` Andrew F. Davis
2017-03-27 21:12 ` Liam Breck
2017-03-27 7:41 ` [PATCH v12 9/9] power: bq27xxx_battery: Remove duplicate register arrays 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.