2014-06-10 09:45:17

by Juergen Borleis

[permalink] [raw]
Subject: Power/battery/BQ27xxx: some fixes and hot-plug support

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


2014-06-10 09:45:12

by Juergen Borleis

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

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

Signed-off-by: Juergen Borleis <[email protected]>
---
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

2014-06-10 09:45:26

by Juergen Borleis

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

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 <[email protected]>
---
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

2014-06-10 09:45:23

by Juergen Borleis

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

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

Signed-off-by: Juergen Borleis <[email protected]>
---
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

2014-06-10 09:45:56

by Juergen Borleis

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

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 <[email protected]>
---
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

2014-06-10 09:45:21

by Juergen Borleis

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

Signed-off-by: Juergen Borleis <[email protected]>
---
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

2014-06-10 09:45:19

by Juergen Borleis

[permalink] [raw]
Subject: [PATCH 5/7] bq27xxx: suppress error messages

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

Signed-off-by: Juergen Borleis <[email protected]>
---
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

2014-06-10 09:45:16

by Juergen Borleis

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

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 <[email protected]>
---
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

2014-07-08 08:28:52

by Steffen Trumtrar

[permalink] [raw]
Subject: Re: Power/battery/BQ27xxx: some fixes and hot-plug support


Hi!

Juergen Borleis <[email protected]> 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 |