Power/battery/BQ27xxx: some fixes and hot-plug support

Power/battery/BQ27xxx: some fixes and hot-plug support

[Juergen Borleis]

The BQ27425 support needs some fixes, as it differs from the main
BQ27000 and the BQ27500 somehow. Also we need to distinguish two existing
BQ27425 device types due to a firmware change.

And these kind of devices can be part of the attached battery or part of the
board with an externally attached battery. In both cases we must consider a
battery change can be handled and at least the battery connection state gets
reported correctly.

Please keep me on CC as I'm not subscribed to LKML.

Comments are welcome.
Juergen

[PATCH 1/7] bq27425: fix register offset for SOC on bq27425

[Juergen Borleis]

The SOC register is at offset 0x1C according to its datasheet. Due to the
fact the access routine subtracts BQ27425_REG_OFFSET from this value we need
0x20 to reach the real offset at 0x1C.

This offset is valid for the following devices:
 - BQ27425-G1, firmware device info 0x0410
 - BQ27425-G2a, firmware device info 0x0425

Signed-off-by: Juergen Borleis <jbe@pengutronix.de>
---
 drivers/power/bq27x00_battery.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/power/bq27x00_battery.c b/drivers/power/bq27x00_battery.c
index b309713..f3de29e 100644
--- a/drivers/power/bq27x00_battery.c
+++ b/drivers/power/bq27x00_battery.c
@@ -72,7 +72,7 @@
 
 /* bq27425 register addresses are same as bq27x00 addresses minus 4 */
 #define BQ27425_REG_OFFSET		0x04
-#define BQ27425_REG_SOC			0x18 /* Register address plus offset */
+#define BQ27425_REG_SOC			0x20 /* Register address plus offset */
 
 #define BQ27000_RS			20 /* Resistor sense */
 #define BQ27x00_POWER_CONSTANT		(256 * 29200 / 1000)
-- 
2.0.0.rc2

[PATCH 2/7] bq27425: add support to read the control registers

[Juergen Borleis]

These kind of registers need a special sequence to be accessed.

Signed-off-by: Juergen Borleis <jbe@pengutronix.de>
---
 drivers/power/bq27x00_battery.c | 42 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 42 insertions(+)

diff --git a/drivers/power/bq27x00_battery.c b/drivers/power/bq27x00_battery.c
index f3de29e..58644de 100644
--- a/drivers/power/bq27x00_battery.c
+++ b/drivers/power/bq27x00_battery.c
@@ -70,6 +70,9 @@
 #define BQ27500_FLAG_FC			BIT(9)
 #define BQ27500_FLAG_OTC		BIT(15)
 
+/* bq27425 control commands */
+#define BQ27425_CONTROL			0x00
+
 /* bq27425 register addresses are same as bq27x00 addresses minus 4 */
 #define BQ27425_REG_OFFSET		0x04
 #define BQ27425_REG_SOC			0x20 /* Register address plus offset */
@@ -80,6 +83,7 @@
 struct bq27x00_device_info;
 struct bq27x00_access_methods {
 	int (*read)(struct bq27x00_device_info *di, u8 reg, bool single);
+	int (*reads)(struct bq27x00_device_info *di, u16 creg);
 };
 
 enum bq27x00_chip { BQ27000, BQ27500, BQ27425};
@@ -782,6 +786,42 @@ static int bq27x00_read_i2c(struct bq27x00_device_info *di, u8 reg, bool single)
 	return ret;
 }
 
+static int bq27425_read_i2c_control(struct bq27x00_device_info *di, u16 creg)
+{
+	struct i2c_client *client = to_i2c_client(di->dev);
+	struct i2c_msg msg[2];
+	unsigned char cntl[3];
+	unsigned char data[2];
+	int ret;
+
+	if (!client->adapter)
+		return -ENODEV;
+
+	msg[0].addr = client->addr;
+	msg[0].flags = 0;
+	cntl[0] = BQ27425_CONTROL; /* CNTL command */
+	put_unaligned_le16(creg, &cntl[1]); /* CNTL DATA */
+	msg[0].buf = cntl;
+	msg[0].len = sizeof(cntl);
+	msg[1].addr = client->addr;
+	msg[1].flags = I2C_M_RD;
+	msg[1].buf = data;
+	msg[1].len = sizeof(data);
+
+	ret = i2c_transfer(client->adapter, msg, 1);
+	if (ret < 0)
+		return ret;
+
+	udelay(100); /* at least 66 µs pause */
+
+	msg[0].len = 1; /* only the command again, not the subcommand */
+
+	ret = i2c_transfer(client->adapter, msg, ARRAY_SIZE(msg));
+	if (ret < 0)
+		return ret;
+
+	return get_unaligned_le16(data);
+}
 static int bq27x00_battery_probe(struct i2c_client *client,
 				 const struct i2c_device_id *id)
 {
@@ -816,6 +856,8 @@ static int bq27x00_battery_probe(struct i2c_client *client,
 	di->chip = id->driver_data;
 	di->bat.name = name;
 	di->bus.read = &bq27x00_read_i2c;
+	if (di->chip == BQ27425)
+		di->bus.reads = bq27425_read_i2c_control;
 
 	retval = bq27x00_powersupply_init(di);
 	if (retval)
-- 
2.0.0.rc2

[PATCH 3/7] bq27425: add support for different firmware chip types

[Juergen Borleis]

Signed-off-by: Juergen Borleis <jbe@pengutronix.de>
---
 drivers/power/bq27x00_battery.c | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/drivers/power/bq27x00_battery.c b/drivers/power/bq27x00_battery.c
index 58644de..39b5194 100644
--- a/drivers/power/bq27x00_battery.c
+++ b/drivers/power/bq27x00_battery.c
@@ -72,6 +72,7 @@
 
 /* bq27425 control commands */
 #define BQ27425_CONTROL			0x00
+# define BQ27425_DEVICE_TYPE		0x0001
 
 /* bq27425 register addresses are same as bq27x00 addresses minus 4 */
 #define BQ27425_REG_OFFSET		0x04
@@ -87,6 +88,7 @@ struct bq27x00_access_methods {
 };
 
 enum bq27x00_chip { BQ27000, BQ27500, BQ27425};
+enum bq27425_chip_type { BQ27425_unknown, BQ27425_g1, BQ27425_g2a};
 
 struct bq27x00_reg_cache {
 	int temperature;
@@ -106,6 +108,7 @@ struct bq27x00_device_info {
 	struct device 		*dev;
 	int			id;
 	enum bq27x00_chip	chip;
+	enum bq27425_chip_type	chip_type;
 
 	struct bq27x00_reg_cache cache;
 	int charge_design_full;
@@ -698,6 +701,28 @@ static int bq27x00_powersupply_init(struct bq27x00_device_info *di)
 {
 	int ret;
 
+	if (di->chip == BQ27425) {
+		/* detect chip type */
+		ret = di->bus.reads(di, BQ27425_DEVICE_TYPE);
+		if (ret < 0) {
+			dev_err(di->dev, "Fail to detect BQ27425's type\n");
+			/* try to continue */
+		} else {
+			switch (ret) {
+			case 0x0410:
+				di->chip_type = BQ27425_g1;
+				break;
+			case 0x0425:
+				di->chip_type = BQ27425_g2a;
+				break;
+			default:
+				dev_err(di->dev, "Unknown BQ27425 type: "
+						"%04X\n", ret);
+				/* try to continue */
+			}
+		}
+	}
+
 	di->bat.type = POWER_SUPPLY_TYPE_BATTERY;
 	if (di->chip == BQ27425) {
 		di->bat.properties = bq27425_battery_props;
-- 
2.0.0.rc2

[PATCH 4/7] bq27425: the DesignCapacity register is at a different offset

[Juergen Borleis]

For the BQ27425-g1 this register does not exists at all.
For the BQ27425-g2a this register is at offset 0x3C.

Signed-off-by: Juergen Borleis <jbe@pengutronix.de>
---
 drivers/power/bq27x00_battery.c | 17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/drivers/power/bq27x00_battery.c b/drivers/power/bq27x00_battery.c
index 39b5194..a5a88f1 100644
--- a/drivers/power/bq27x00_battery.c
+++ b/drivers/power/bq27x00_battery.c
@@ -78,6 +78,9 @@
 #define BQ27425_REG_OFFSET		0x04
 #define BQ27425_REG_SOC			0x20 /* Register address plus offset */
 
+/* register offsets above 0x20 are DEVICE_TYPE 0x0425 specifc */
+#define BQ27425_REG_DCAP		0x40 /* Design capacity plus offset */
+
 #define BQ27000_RS			20 /* Resistor sense */
 #define BQ27x00_POWER_CONSTANT		(256 * 29200 / 1000)
 
@@ -265,9 +268,17 @@ static int bq27x00_battery_read_ilmd(struct bq27x00_device_info *di)
 {
 	int ilmd;
 
-	if (bq27xxx_is_chip_version_higher(di))
-		ilmd = bq27x00_read(di, BQ27500_REG_DCAP, false);
-	else
+	if (bq27xxx_is_chip_version_higher(di)) {
+		if (di->chip == BQ27425) {
+			if (di->chip_type == BQ27425_g2a)
+				ilmd = bq27x00_read(di, BQ27425_REG_DCAP,
+							false);
+			else
+				ilmd = -ENODATA;
+		} else {
+			ilmd = bq27x00_read(di, BQ27500_REG_DCAP, false);
+		}
+	} else
 		ilmd = bq27x00_read(di, BQ27000_REG_ILMD, true);
 
 	if (ilmd < 0) {
-- 
2.0.0.rc2

[PATCH 5/7] bq27xxx: suppress error messages

[Juergen Borleis]

Due to the support for hot-plug, the inaccessibility of the device if a regular use-case.

Signed-off-by: Juergen Borleis <jbe@pengutronix.de>
---
 drivers/power/bq27x00_battery.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/power/bq27x00_battery.c b/drivers/power/bq27x00_battery.c
index a5a88f1..3f3169e 100644
--- a/drivers/power/bq27x00_battery.c
+++ b/drivers/power/bq27x00_battery.c
@@ -382,7 +382,7 @@ static int bq27x00_battery_read_pwr_avg(struct bq27x00_device_info *di, u8 reg)
 
 	tval = bq27x00_read(di, reg, false);
 	if (tval < 0) {
-		dev_err(di->dev, "error reading power avg rgister  %02x: %d\n",
+		dev_dbg(di->dev, "error reading power avg register  %02x: %d\n",
 			reg, tval);
 		return tval;
 	}
@@ -403,7 +403,7 @@ static int bq27x00_battery_read_health(struct bq27x00_device_info *di)
 
 	tval = bq27x00_read(di, BQ27x00_REG_FLAGS, false);
 	if (tval < 0) {
-		dev_err(di->dev, "error reading flag register:%d\n", tval);
+		dev_dbg(di->dev, "error reading flag register:%d\n", tval);
 		return tval;
 	}
 
-- 
2.0.0.rc2

[PATCH 6/7] bq27xxx: init the cache first

[Juergen Borleis]

While registering the power supply the framework starts to query immediately.
This change ensures providing consistent information.

Signed-off-by: Juergen Borleis <jbe@pengutronix.de>
---
 drivers/power/bq27x00_battery.c | 30 ++++++++++++++++++++++++------
 1 file changed, 24 insertions(+), 6 deletions(-)

diff --git a/drivers/power/bq27x00_battery.c b/drivers/power/bq27x00_battery.c
index 3f3169e..0a02d86 100644
--- a/drivers/power/bq27x00_battery.c
+++ b/drivers/power/bq27x00_battery.c
@@ -124,6 +124,8 @@ struct bq27x00_device_info {
 	struct bq27x00_access_methods bus;
 
 	struct mutex lock;
+
+	bool dev_accessible; /* true when the device is accessible */
 };
 
 static enum power_supply_property bq27x00_battery_props[] = {
@@ -178,6 +180,19 @@ static inline int bq27x00_read(struct bq27x00_device_info *di, u8 reg,
 	return di->bus.read(di, reg, single);
 }
 
+static void bq27245_init_cache(struct bq27x00_reg_cache *cache)
+{
+	cache->temperature = cache->time_to_empty = cache->time_to_empty_avg =
+		cache->time_to_full = cache->charge_full = cache->cycle_count =
+		cache->capacity = cache->energy = cache->flags =
+		cache->power_avg = cache->health = -ENODATA;
+}
+
+static void bq27xxx_is_awake(struct bq27x00_device_info *di)
+{
+	di->dev_accessible = true;
+}
+
 /*
  * Higher versions of the chip like BQ27425 and BQ27500
  * differ from BQ27000 and BQ27200 in calculation of certain
@@ -748,17 +763,20 @@ static int bq27x00_powersupply_init(struct bq27x00_device_info *di)
 	INIT_DELAYED_WORK(&di->work, bq27x00_battery_poll);
 	mutex_init(&di->lock);
 
+	dev_info(di->dev, "support ver. %s enabled\n", DRIVER_VERSION);
+
+	/* expect the chip is accessible for now */
+	bq27xxx_is_awake(di);
+	/* we get queried immediately, so init our cache first */
+	bq27245_init_cache(&di->cache);
+
 	ret = power_supply_register(di->dev, &di->bat);
-	if (ret) {
+	if (ret)
 		dev_err(di->dev, "failed to register battery: %d\n", ret);
-		return ret;
-	}
-
-	dev_info(di->dev, "support ver. %s enabled\n", DRIVER_VERSION);
 
 	bq27x00_update(di);
 
-	return 0;
+	return ret;
 }
 
 static void bq27x00_powersupply_unregister(struct bq27x00_device_info *di)
-- 
2.0.0.rc2

[PATCH 7/7] bq27xxx: enable hot-plug support

[Juergen Borleis]

These kind of devices can be part of the attached battery or part of the board
with an externally attached battery. In both cases we must consider a battery
change can be handled and at least the battery connection state gets
reported correctly.

Signed-off-by: Juergen Borleis <jbe@pengutronix.de>
---
 drivers/power/bq27x00_battery.c | 58 ++++++++++++++++++++++++++++++++++++++---
 1 file changed, 54 insertions(+), 4 deletions(-)

diff --git a/drivers/power/bq27x00_battery.c b/drivers/power/bq27x00_battery.c
index 0a02d86..5a6c00d 100644
--- a/drivers/power/bq27x00_battery.c
+++ b/drivers/power/bq27x00_battery.c
@@ -188,6 +188,14 @@ static void bq27245_init_cache(struct bq27x00_reg_cache *cache)
 		cache->power_avg = cache->health = -ENODATA;
 }
 
+static void bq27xxx_is_gone(struct bq27x00_device_info *di)
+{
+	di->dev_accessible = false;
+	bq27245_init_cache(&di->cache);
+	di->last_update = jiffies;
+	power_supply_changed(&di->bat);
+}
+
 static void bq27xxx_is_awake(struct bq27x00_device_info *di)
 {
 	di->dev_accessible = true;
@@ -220,8 +228,10 @@ static int bq27x00_battery_read_rsoc(struct bq27x00_device_info *di)
 	else
 		rsoc = bq27x00_read(di, BQ27000_REG_RSOC, true);
 
-	if (rsoc < 0)
+	if (rsoc < 0) {
 		dev_dbg(di->dev, "error reading relative State-of-Charge\n");
+		bq27xxx_is_gone(di);
+	}
 
 	return rsoc;
 }
@@ -238,6 +248,7 @@ static int bq27x00_battery_read_charge(struct bq27x00_device_info *di, u8 reg)
 	if (charge < 0) {
 		dev_dbg(di->dev, "error reading charge register %02x: %d\n",
 			reg, charge);
+		bq27xxx_is_gone(di);
 		return charge;
 	}
 
@@ -260,6 +271,10 @@ static inline int bq27x00_battery_read_nac(struct bq27x00_device_info *di)
 	bool is_higher = bq27xxx_is_chip_version_higher(di);
 
 	flags = bq27x00_read(di, BQ27x00_REG_FLAGS, !is_bq27500);
+	if (flags < 0) {
+		bq27xxx_is_gone(di);
+		return flags;
+	}
 	if (flags >= 0 && !is_higher && (flags & BQ27000_FLAG_CI))
 		return -ENODATA;
 
@@ -272,7 +287,12 @@ static inline int bq27x00_battery_read_nac(struct bq27x00_device_info *di)
  */
 static inline int bq27x00_battery_read_lmd(struct bq27x00_device_info *di)
 {
-	return bq27x00_battery_read_charge(di, BQ27x00_REG_LMD);
+	int reg;
+
+	reg = bq27x00_battery_read_charge(di, BQ27x00_REG_LMD);
+	if (reg < 0)
+		bq27xxx_is_gone(di);
+	return reg;
 }
 
 /*
@@ -298,6 +318,7 @@ static int bq27x00_battery_read_ilmd(struct bq27x00_device_info *di)
 
 	if (ilmd < 0) {
 		dev_dbg(di->dev, "error reading initial last measured discharge\n");
+		bq27xxx_is_gone(di);
 		return ilmd;
 	}
 
@@ -320,6 +341,7 @@ static int bq27x00_battery_read_energy(struct bq27x00_device_info *di)
 	ae = bq27x00_read(di, BQ27x00_REG_AE, false);
 	if (ae < 0) {
 		dev_dbg(di->dev, "error reading available energy\n");
+		bq27xxx_is_gone(di);
 		return ae;
 	}
 
@@ -342,6 +364,7 @@ static int bq27x00_battery_read_temperature(struct bq27x00_device_info *di)
 	temp = bq27x00_read(di, BQ27x00_REG_TEMP, false);
 	if (temp < 0) {
 		dev_err(di->dev, "error reading temperature\n");
+		bq27xxx_is_gone(di);
 		return temp;
 	}
 
@@ -360,8 +383,10 @@ static int bq27x00_battery_read_cyct(struct bq27x00_device_info *di)
 	int cyct;
 
 	cyct = bq27x00_read(di, BQ27x00_REG_CYCT, false);
-	if (cyct < 0)
+	if (cyct < 0) {
 		dev_err(di->dev, "error reading cycle count total\n");
+		bq27xxx_is_gone(di);
+	}
 
 	return cyct;
 }
@@ -378,6 +403,7 @@ static int bq27x00_battery_read_time(struct bq27x00_device_info *di, u8 reg)
 	if (tval < 0) {
 		dev_dbg(di->dev, "error reading time register %02x: %d\n",
 			reg, tval);
+		bq27xxx_is_gone(di);
 		return tval;
 	}
 
@@ -399,6 +425,7 @@ static int bq27x00_battery_read_pwr_avg(struct bq27x00_device_info *di, u8 reg)
 	if (tval < 0) {
 		dev_dbg(di->dev, "error reading power avg register  %02x: %d\n",
 			reg, tval);
+		bq27xxx_is_gone(di);
 		return tval;
 	}
 
@@ -419,6 +446,7 @@ static int bq27x00_battery_read_health(struct bq27x00_device_info *di)
 	tval = bq27x00_read(di, BQ27x00_REG_FLAGS, false);
 	if (tval < 0) {
 		dev_dbg(di->dev, "error reading flag register:%d\n", tval);
+		bq27xxx_is_gone(di);
 		return tval;
 	}
 
@@ -446,8 +474,29 @@ static void bq27x00_update(struct bq27x00_device_info *di)
 	struct bq27x00_reg_cache cache = {0, };
 	bool is_bq27500 = di->chip == BQ27500;
 	bool is_bq27425 = di->chip == BQ27425;
+	int reg;
+
+	reg = bq27x00_read(di, BQ27x00_REG_FLAGS, !is_bq27500);
+	if (di->dev_accessible == false) {
+		/* check, if the device is present now */
+		if (reg < 0) {
+			dev_dbg(di->dev, "BQ27xxx still not present\n");
+			goto out; /* no, still not present */
+		}
+
+		/* it seems present now */
+		bq27xxx_is_awake(di);
+		dev_dbg(di->dev, "BQ27xxx detected\n");
+	}
+
+	if (reg < 0) {
+		/* it seems gone now */
+		dev_dbg(di->dev, "BQ27xxx is gone\n");
+		bq27xxx_is_gone(di);
+		return;
+	}
 
-	cache.flags = bq27x00_read(di, BQ27x00_REG_FLAGS, !is_bq27500);
+	cache.flags = reg;
 	if (cache.flags >= 0) {
 		if (!is_bq27500 && !is_bq27425
 				&& (cache.flags & BQ27000_FLAG_CI)) {
@@ -492,6 +541,7 @@ static void bq27x00_update(struct bq27x00_device_info *di)
 		power_supply_changed(&di->bat);
 	}
 
+out:
 	di->last_update = jiffies;
 }
 
-- 
2.0.0.rc2

Re: Power/battery/BQ27xxx: some fixes and hot-plug support

[Steffen Trumtrar]

Hi!

Juergen Borleis <jbe@pengutronix.de> writes:
> The BQ27425 support needs some fixes, as it differs from the main
> BQ27000 and the BQ27500 somehow. Also we need to distinguish two existing
> BQ27425 device types due to a firmware change.
>
> And these kind of devices can be part of the attached battery or part of the
> board with an externally attached battery. In both cases we must consider a
> battery change can be handled and at least the battery connection state gets
> reported correctly.
>

ping!

Any comments on this series!?

Regards,
Steffen

-- 
Pengutronix e.K.                           | Steffen Trumtrar            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |