* [PATCH v2 00/14] POWER: BQ27x00: New Properties, fixes, bq27000 support
@ 2011-02-12 19:38 Lars-Peter Clausen
2011-02-12 19:39 ` [PATCH 01/14] bq27x00: Add type property Lars-Peter Clausen
` (14 more replies)
0 siblings, 15 replies; 28+ messages in thread
From: Lars-Peter Clausen @ 2011-02-12 19:38 UTC (permalink / raw)
To: Anton Vorontsov
Cc: Rodolfo Giometti, Grazvydas Ignotas, linux-kernel, Lars-Peter Clausen
This patch series contains a few updates for the bq27x00 driver:
* Support for additional power supply properties
* Support for the bq27000 battery which is identical to the bq27200 but is
connected through the HDQ bus.
* Adds a register cache to the driver and introduces polling the batteries state
* Minor improvements and cleanups
The last patch in this series is not specific to the bq27x00 driver but is
required for uevents to be generated properly for this driver.
The patch makes properties which return -ENODATA to be ignored when generating
uevents. Previously in such a case uevent generation would have been aborted
with an error. But since the bq27x00 return -ENODATA for the TIME_TO_FULL
property when the battery is not charging and for the TIME_TO_EMPTY property
when the battery is not discharging and at least one of them is always true
uevent generation would always fail.
This series has so far been tested with the bq27000 and the bq27200 battery,
but not with the bq27500 battery, so it would be nice if somebody with a board
containing such a battery could test the patches to make sure that there are no
regressions.
Changes since v1:
Don't cache registers for fast changing properties like VOLTAGE_NOW,
CURRENT_NOW, ENERGY_NOW and CHARGE_NOW which can be obtained by reading
only a single register.
These were ignored anyway when comparing two battery states and since
their register is not shared between different properties we gain nothing
by caching them. On the other hand it allows the property to be queried at
a higher rate then the caching time.
This series is also available at
git://git.metafoo.de/linux-2.6 bq27x00-for-upstream
Lars-Peter Clausen (10):
bq27x00: Add type property
bq27x00: Improve temperature property precession
bq27x00: Return -ENODEV for properties if the battery is not present
bq27x00: Prepare code for addition of bq27000 platform driver
bq27x00: Add bq27000 support
bq27X00: Cache battery registers
bq27x00: Poll battery state
bq27x00: Give more specific reports on battery status
bq27x00: Cleanup bq27x00_i2c_read
Ignore -ENODATA errors when generating a uevent
Pali Rohár (4):
bq27x00: Fix CURRENT_NOW property
bq27x00: Add new properties
bq27x00: Add MODULE_DEVICE_TABLE
bq27x00: Minor cleanups
drivers/power/Kconfig | 14 +
drivers/power/bq27x00_battery.c | 726 +++++++++++++++++++++++++--------
drivers/power/power_supply_sysfs.c | 2 +-
include/linux/power/bq27x00_battery.h | 19 +
4 files changed, 594 insertions(+), 167 deletions(-)
create mode 100644 include/linux/power/bq27x00_battery.h
--
1.7.2.3
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH 01/14] bq27x00: Add type property
2011-02-12 19:38 [PATCH v2 00/14] POWER: BQ27x00: New Properties, fixes, bq27000 support Lars-Peter Clausen
@ 2011-02-12 19:39 ` Lars-Peter Clausen
2011-02-12 19:39 ` [PATCH 02/14] bq27x00: Improve temperature property precession Lars-Peter Clausen
` (13 subsequent siblings)
14 siblings, 0 replies; 28+ messages in thread
From: Lars-Peter Clausen @ 2011-02-12 19:39 UTC (permalink / raw)
To: Anton Vorontsov
Cc: Rodolfo Giometti, Grazvydas Ignotas, linux-kernel, Lars-Peter Clausen
This patch adds the type property to the bq27x00 battery driver.
All bq27x00 are lithium ion batteries.
Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
Acked-by: Rodolfo Giometti <giometti@linux.it>
---
drivers/power/bq27x00_battery.c | 4 ++++
1 files changed, 4 insertions(+), 0 deletions(-)
diff --git a/drivers/power/bq27x00_battery.c b/drivers/power/bq27x00_battery.c
index eff0273..bb043f9 100644
--- a/drivers/power/bq27x00_battery.c
+++ b/drivers/power/bq27x00_battery.c
@@ -78,6 +78,7 @@ static enum power_supply_property bq27x00_battery_props[] = {
POWER_SUPPLY_PROP_TIME_TO_EMPTY_NOW,
POWER_SUPPLY_PROP_TIME_TO_EMPTY_AVG,
POWER_SUPPLY_PROP_TIME_TO_FULL_NOW,
+ POWER_SUPPLY_PROP_TECHNOLOGY,
};
/*
@@ -277,6 +278,9 @@ static int bq27x00_battery_get_property(struct power_supply *psy,
case POWER_SUPPLY_PROP_TIME_TO_FULL_NOW:
ret = bq27x00_battery_time(di, BQ27x00_REG_TTF, val);
break;
+ case POWER_SUPPLY_PROP_TECHNOLOGY:
+ val->intval = POWER_SUPPLY_TECHNOLOGY_LION;
+ break;
default:
return -EINVAL;
}
--
1.7.2.3
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH 02/14] bq27x00: Improve temperature property precession
2011-02-12 19:38 [PATCH v2 00/14] POWER: BQ27x00: New Properties, fixes, bq27000 support Lars-Peter Clausen
2011-02-12 19:39 ` [PATCH 01/14] bq27x00: Add type property Lars-Peter Clausen
@ 2011-02-12 19:39 ` Lars-Peter Clausen
2011-02-12 19:39 ` [PATCH 03/14] bq27x00: Fix CURRENT_NOW property Lars-Peter Clausen
` (12 subsequent siblings)
14 siblings, 0 replies; 28+ messages in thread
From: Lars-Peter Clausen @ 2011-02-12 19:39 UTC (permalink / raw)
To: Anton Vorontsov
Cc: Rodolfo Giometti, Grazvydas Ignotas, linux-kernel, Lars-Peter Clausen
This patch improves the precession of the temperature property of the
bq27x00 driver.
By dividing before multiplying the current code effectively cuts of the
last decimal digit. This patch fixes it by multiplying before dividing.
Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
Acked-by: Rodolfo Giometti <giometti@linux.it>
---
drivers/power/bq27x00_battery.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/power/bq27x00_battery.c b/drivers/power/bq27x00_battery.c
index bb043f9..4f74659 100644
--- a/drivers/power/bq27x00_battery.c
+++ b/drivers/power/bq27x00_battery.c
@@ -109,7 +109,7 @@ static int bq27x00_battery_temperature(struct bq27x00_device_info *di)
if (di->chip == BQ27500)
return temp - 2731;
else
- return ((temp >> 2) - 273) * 10;
+ return ((temp * 5) - 5463) / 2;
}
/*
--
1.7.2.3
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH 03/14] bq27x00: Fix CURRENT_NOW property
2011-02-12 19:38 [PATCH v2 00/14] POWER: BQ27x00: New Properties, fixes, bq27000 support Lars-Peter Clausen
2011-02-12 19:39 ` [PATCH 01/14] bq27x00: Add type property Lars-Peter Clausen
2011-02-12 19:39 ` [PATCH 02/14] bq27x00: Improve temperature property precession Lars-Peter Clausen
@ 2011-02-12 19:39 ` Lars-Peter Clausen
2011-02-12 19:39 ` [PATCH 04/14] bq27x00: Return -ENODEV for properties if the battery is not present Lars-Peter Clausen
` (11 subsequent siblings)
14 siblings, 0 replies; 28+ messages in thread
From: Lars-Peter Clausen @ 2011-02-12 19:39 UTC (permalink / raw)
To: Anton Vorontsov
Cc: Rodolfo Giometti, Grazvydas Ignotas, linux-kernel,
Pali Rohár, Lars-Peter Clausen
From: Pali Rohár <pali.rohar@gmail.com>
According to the bq27000 datasheet the current should be calculated by
the following formula:
current = AI * 3570 / 20
This patch adjust the drivers code accordingly.
Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
Acked-by: Rodolfo Giometti <giometti@linux.it>
---
drivers/power/bq27x00_battery.c | 7 +++++--
1 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/power/bq27x00_battery.c b/drivers/power/bq27x00_battery.c
index 4f74659..1b06134 100644
--- a/drivers/power/bq27x00_battery.c
+++ b/drivers/power/bq27x00_battery.c
@@ -44,6 +44,8 @@
#define BQ27500_FLAG_DSC BIT(0)
#define BQ27500_FLAG_FC BIT(9)
+#define BQ27000_RS 20 /* Resistor sense */
+
/* If the system has several batteries we need a different name for each
* of them...
*/
@@ -149,7 +151,7 @@ static int bq27x00_battery_current(struct bq27x00_device_info *di)
if (di->chip == BQ27500) {
/* bq27500 returns signed value */
- curr = (int)(s16)curr;
+ curr = (int)((s16)curr) * 1000;
} else {
ret = bq27x00_read(BQ27x00_REG_FLAGS, &flags, 0, di);
if (ret < 0) {
@@ -160,9 +162,10 @@ static int bq27x00_battery_current(struct bq27x00_device_info *di)
dev_dbg(di->dev, "negative current!\n");
curr = -curr;
}
+ curr = curr * 3570 / BQ27000_RS;
}
- return curr * 1000;
+ return curr;
}
/*
--
1.7.2.3
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH 04/14] bq27x00: Return -ENODEV for properties if the battery is not present
2011-02-12 19:38 [PATCH v2 00/14] POWER: BQ27x00: New Properties, fixes, bq27000 support Lars-Peter Clausen
` (2 preceding siblings ...)
2011-02-12 19:39 ` [PATCH 03/14] bq27x00: Fix CURRENT_NOW property Lars-Peter Clausen
@ 2011-02-12 19:39 ` Lars-Peter Clausen
2011-02-12 19:39 ` [PATCH 05/14] bq27x00: Prepare code for addition of bq27000 platform driver Lars-Peter Clausen
` (10 subsequent siblings)
14 siblings, 0 replies; 28+ messages in thread
From: Lars-Peter Clausen @ 2011-02-12 19:39 UTC (permalink / raw)
To: Anton Vorontsov
Cc: Rodolfo Giometti, Grazvydas Ignotas, linux-kernel, Lars-Peter Clausen
This patch changes get_property callback of the bq27x00 battery to return
-ENODEV for properties other then the PROP_PRESENT if the battery is not
present.
The power subsystem core expects a driver to behave that way.
Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
Acked-by: Rodolfo Giometti <giometti@linux.it>
---
drivers/power/bq27x00_battery.c | 9 +++++++--
1 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/power/bq27x00_battery.c b/drivers/power/bq27x00_battery.c
index 1b06134..9f16666 100644
--- a/drivers/power/bq27x00_battery.c
+++ b/drivers/power/bq27x00_battery.c
@@ -252,16 +252,21 @@ static int bq27x00_battery_get_property(struct power_supply *psy,
{
int ret = 0;
struct bq27x00_device_info *di = to_bq27x00_device_info(psy);
+ int voltage = bq27x00_battery_voltage(di);
+
+ if (psp != POWER_SUPPLY_PROP_PRESENT && voltage <= 0)
+ return -ENODEV;
switch (psp) {
case POWER_SUPPLY_PROP_STATUS:
ret = bq27x00_battery_status(di, val);
break;
case POWER_SUPPLY_PROP_VOLTAGE_NOW:
+ val->intval = voltage;
+ break;
case POWER_SUPPLY_PROP_PRESENT:
- val->intval = bq27x00_battery_voltage(di);
if (psp == POWER_SUPPLY_PROP_PRESENT)
- val->intval = val->intval <= 0 ? 0 : 1;
+ val->intval = voltage <= 0 ? 0 : 1;
break;
case POWER_SUPPLY_PROP_CURRENT_NOW:
val->intval = bq27x00_battery_current(di);
--
1.7.2.3
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH 05/14] bq27x00: Prepare code for addition of bq27000 platform driver
2011-02-12 19:38 [PATCH v2 00/14] POWER: BQ27x00: New Properties, fixes, bq27000 support Lars-Peter Clausen
` (3 preceding siblings ...)
2011-02-12 19:39 ` [PATCH 04/14] bq27x00: Return -ENODEV for properties if the battery is not present Lars-Peter Clausen
@ 2011-02-12 19:39 ` Lars-Peter Clausen
2011-02-12 19:39 ` [PATCH 06/14] bq27x00: Add bq27000 support Lars-Peter Clausen
` (9 subsequent siblings)
14 siblings, 0 replies; 28+ messages in thread
From: Lars-Peter Clausen @ 2011-02-12 19:39 UTC (permalink / raw)
To: Anton Vorontsov
Cc: Rodolfo Giometti, Grazvydas Ignotas, linux-kernel, Lars-Peter Clausen
This patch simplifies the drivers data structure and moves code to be
shared by the bq27000 and bq27200/bq27500 init functions into a common
function.
This patch has no functional changes, it only moves code around.
Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
Acked-by: Rodolfo Giometti <giometti@linux.it>
---
drivers/power/bq27x00_battery.c | 86 +++++++++++++++++---------------------
1 files changed, 39 insertions(+), 47 deletions(-)
diff --git a/drivers/power/bq27x00_battery.c b/drivers/power/bq27x00_battery.c
index 9f16666..def951d 100644
--- a/drivers/power/bq27x00_battery.c
+++ b/drivers/power/bq27x00_battery.c
@@ -54,8 +54,8 @@ static DEFINE_MUTEX(battery_mutex);
struct bq27x00_device_info;
struct bq27x00_access_methods {
- int (*read)(u8 reg, int *rt_value, int b_single,
- struct bq27x00_device_info *di);
+ int (*read)(struct bq27x00_device_info *, u8 reg, int *rt_value,
+ bool single);
};
enum bq27x00_chip { BQ27000, BQ27500 };
@@ -63,11 +63,11 @@ enum bq27x00_chip { BQ27000, BQ27500 };
struct bq27x00_device_info {
struct device *dev;
int id;
- struct bq27x00_access_methods *bus;
- struct power_supply bat;
enum bq27x00_chip chip;
- struct i2c_client *client;
+ struct power_supply bat;
+
+ struct bq27x00_access_methods bus;
};
static enum power_supply_property bq27x00_battery_props[] = {
@@ -87,10 +87,10 @@ static enum power_supply_property bq27x00_battery_props[] = {
* Common code for BQ27x00 devices
*/
-static int bq27x00_read(u8 reg, int *rt_value, int b_single,
- struct bq27x00_device_info *di)
+static inline int bq27x00_read(struct bq27x00_device_info *di, u8 reg,
+ int *rt_value, bool single)
{
- return di->bus->read(reg, rt_value, b_single, di);
+ return di->bus.read(di, reg, rt_value, single);
}
/*
@@ -102,7 +102,7 @@ static int bq27x00_battery_temperature(struct bq27x00_device_info *di)
int ret;
int temp = 0;
- ret = bq27x00_read(BQ27x00_REG_TEMP, &temp, 0, di);
+ ret = bq27x00_read(di, BQ27x00_REG_TEMP, &temp, false);
if (ret) {
dev_err(di->dev, "error reading temperature\n");
return ret;
@@ -123,7 +123,7 @@ static int bq27x00_battery_voltage(struct bq27x00_device_info *di)
int ret;
int volt = 0;
- ret = bq27x00_read(BQ27x00_REG_VOLT, &volt, 0, di);
+ ret = bq27x00_read(di, BQ27x00_REG_VOLT, &volt, false);
if (ret) {
dev_err(di->dev, "error reading voltage\n");
return ret;
@@ -143,7 +143,7 @@ static int bq27x00_battery_current(struct bq27x00_device_info *di)
int curr = 0;
int flags = 0;
- ret = bq27x00_read(BQ27x00_REG_AI, &curr, 0, di);
+ ret = bq27x00_read(di, BQ27x00_REG_AI, &curr, false);
if (ret) {
dev_err(di->dev, "error reading current\n");
return 0;
@@ -153,7 +153,7 @@ static int bq27x00_battery_current(struct bq27x00_device_info *di)
/* bq27500 returns signed value */
curr = (int)((s16)curr) * 1000;
} else {
- ret = bq27x00_read(BQ27x00_REG_FLAGS, &flags, 0, di);
+ ret = bq27x00_read(di, BQ27x00_REG_FLAGS, &flags, false);
if (ret < 0) {
dev_err(di->dev, "error reading flags\n");
return 0;
@@ -178,9 +178,9 @@ static int bq27x00_battery_rsoc(struct bq27x00_device_info *di)
int rsoc = 0;
if (di->chip == BQ27500)
- ret = bq27x00_read(BQ27500_REG_SOC, &rsoc, 0, di);
+ ret = bq27x00_read(di, BQ27500_REG_SOC, &rsoc, false);
else
- ret = bq27x00_read(BQ27000_REG_RSOC, &rsoc, 1, di);
+ ret = bq27x00_read(di, BQ27000_REG_RSOC, &rsoc, true);
if (ret) {
dev_err(di->dev, "error reading relative State-of-Charge\n");
return ret;
@@ -196,7 +196,7 @@ static int bq27x00_battery_status(struct bq27x00_device_info *di,
int status;
int ret;
- ret = bq27x00_read(BQ27x00_REG_FLAGS, &flags, 0, di);
+ ret = bq27x00_read(di, BQ27x00_REG_FLAGS, &flags, false);
if (ret < 0) {
dev_err(di->dev, "error reading flags\n");
return ret;
@@ -230,7 +230,7 @@ static int bq27x00_battery_time(struct bq27x00_device_info *di, int reg,
int tval = 0;
int ret;
- ret = bq27x00_read(reg, &tval, 0, di);
+ ret = bq27x00_read(di, reg, &tval, false);
if (ret) {
dev_err(di->dev, "error reading register %02x\n", reg);
return ret;
@@ -296,23 +296,35 @@ static int bq27x00_battery_get_property(struct power_supply *psy,
return ret;
}
-static void bq27x00_powersupply_init(struct bq27x00_device_info *di)
+static int bq27x00_powersupply_init(struct bq27x00_device_info *di)
{
+ int ret;
+
di->bat.type = POWER_SUPPLY_TYPE_BATTERY;
di->bat.properties = bq27x00_battery_props;
di->bat.num_properties = ARRAY_SIZE(bq27x00_battery_props);
di->bat.get_property = bq27x00_battery_get_property;
di->bat.external_power_changed = NULL;
+
+ ret = power_supply_register(di->dev, &di->bat);
+ 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);
+
+ return 0;
}
/*
* i2c specific code
*/
-static int bq27x00_read_i2c(u8 reg, int *rt_value, int b_single,
- struct bq27x00_device_info *di)
+static int bq27x00_read_i2c(struct bq27x00_device_info *di, u8 reg,
+ int *rt_value, bool single)
{
- struct i2c_client *client = di->client;
+ struct i2c_client *client = to_i2c_client(di->dev);
struct i2c_msg msg[1];
unsigned char data[2];
int err;
@@ -329,7 +341,7 @@ static int bq27x00_read_i2c(u8 reg, int *rt_value, int b_single,
err = i2c_transfer(client->adapter, msg, 1);
if (err >= 0) {
- if (!b_single)
+ if (!single)
msg->len = 2;
else
msg->len = 1;
@@ -337,7 +349,7 @@ static int bq27x00_read_i2c(u8 reg, int *rt_value, int b_single,
msg->flags = I2C_M_RD;
err = i2c_transfer(client->adapter, msg, 1);
if (err >= 0) {
- if (!b_single)
+ if (!single)
*rt_value = get_unaligned_le16(data);
else
*rt_value = data[0];
@@ -353,7 +365,6 @@ static int bq27x00_battery_probe(struct i2c_client *client,
{
char *name;
struct bq27x00_device_info *di;
- struct bq27x00_access_methods *bus;
int num;
int retval = 0;
@@ -380,38 +391,20 @@ static int bq27x00_battery_probe(struct i2c_client *client,
retval = -ENOMEM;
goto batt_failed_2;
}
+
di->id = num;
+ di->dev = &client->dev;
di->chip = id->driver_data;
+ di->bat.name = name;
+ di->bus.read = &bq27x00_read_i2c;
- bus = kzalloc(sizeof(*bus), GFP_KERNEL);
- if (!bus) {
- dev_err(&client->dev, "failed to allocate access method "
- "data\n");
- retval = -ENOMEM;
+ if (bq27x00_powersupply_init(di))
goto batt_failed_3;
- }
i2c_set_clientdata(client, di);
- di->dev = &client->dev;
- di->bat.name = name;
- bus->read = &bq27x00_read_i2c;
- di->bus = bus;
- di->client = client;
-
- bq27x00_powersupply_init(di);
-
- retval = power_supply_register(&client->dev, &di->bat);
- if (retval) {
- dev_err(&client->dev, "failed to register battery\n");
- goto batt_failed_4;
- }
-
- dev_info(&client->dev, "support ver. %s enabled\n", DRIVER_VERSION);
return 0;
-batt_failed_4:
- kfree(bus);
batt_failed_3:
kfree(di);
batt_failed_2:
@@ -430,7 +423,6 @@ static int bq27x00_battery_remove(struct i2c_client *client)
power_supply_unregister(&di->bat);
- kfree(di->bus);
kfree(di->bat.name);
mutex_lock(&battery_mutex);
--
1.7.2.3
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH 06/14] bq27x00: Add bq27000 support
2011-02-12 19:38 [PATCH v2 00/14] POWER: BQ27x00: New Properties, fixes, bq27000 support Lars-Peter Clausen
` (4 preceding siblings ...)
2011-02-12 19:39 ` [PATCH 05/14] bq27x00: Prepare code for addition of bq27000 platform driver Lars-Peter Clausen
@ 2011-02-12 19:39 ` Lars-Peter Clausen
2011-02-12 19:39 ` [PATCH 07/14] bq27x00: Cache battery registers Lars-Peter Clausen
` (8 subsequent siblings)
14 siblings, 0 replies; 28+ messages in thread
From: Lars-Peter Clausen @ 2011-02-12 19:39 UTC (permalink / raw)
To: Anton Vorontsov
Cc: Rodolfo Giometti, Grazvydas Ignotas, linux-kernel, Lars-Peter Clausen
This patch adds support for the bq27000 battery to the bq27x00 driver.
The bq27000 is similar to the bq27200 except that it uses the HDQ bus
instead of I2C to communicate with the host system.
The driver is implemented as a platform driver. The driver expects to be
provided with a read callback function through its platform data. The read
function is assumed to do the lowlevel HDQ handling and read out the value
of a certain register.
Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
drivers/power/Kconfig | 14 +++
drivers/power/bq27x00_battery.c | 184 ++++++++++++++++++++++++++++++---
include/linux/power/bq27x00_battery.h | 19 ++++
3 files changed, 202 insertions(+), 15 deletions(-)
create mode 100644 include/linux/power/bq27x00_battery.h
diff --git a/drivers/power/Kconfig b/drivers/power/Kconfig
index 61bf5d7..52a462f 100644
--- a/drivers/power/Kconfig
+++ b/drivers/power/Kconfig
@@ -117,10 +117,24 @@ config BATTERY_BQ20Z75
config BATTERY_BQ27x00
tristate "BQ27x00 battery driver"
+ help
+ Say Y here to enable support for batteries with BQ27x00 (I2C/HDQ) chips.
+
+config BATTERY_BQ27X00_I2C
+ bool "BQ27200/BQ27500 support"
+ depends on BATTERY_BQ27x00
depends on I2C
+ default y
help
Say Y here to enable support for batteries with BQ27x00 (I2C) chips.
+config BATTERY_BQ27X00_PLATFORM
+ bool "BQ27000 support"
+ depends on BATTERY_BQ27x00
+ default y
+ help
+ Say Y here to enable support for batteries with BQ27000 (HDQ) chips.
+
config BATTERY_DA9030
tristate "DA9030 battery driver"
depends on PMIC_DA903X
diff --git a/drivers/power/bq27x00_battery.c b/drivers/power/bq27x00_battery.c
index def951d..44bc76b 100644
--- a/drivers/power/bq27x00_battery.c
+++ b/drivers/power/bq27x00_battery.c
@@ -3,6 +3,7 @@
*
* Copyright (C) 2008 Rodolfo Giometti <giometti@linux.it>
* Copyright (C) 2008 Eurotech S.p.A. <info@eurotech.it>
+ * Copyright (C) 2010-2011 Lars-Peter Clausen <lars@metafoo.de>
*
* Based on a previous work by Copyright (C) 2008 Texas Instruments, Inc.
*
@@ -27,6 +28,8 @@
#include <linux/slab.h>
#include <asm/unaligned.h>
+#include <linux/power/bq27x00_battery.h>
+
#define DRIVER_VERSION "1.1.0"
#define BQ27x00_REG_TEMP 0x06
@@ -46,12 +49,6 @@
#define BQ27000_RS 20 /* Resistor sense */
-/* If the system has several batteries we need a different name for each
- * of them...
- */
-static DEFINE_IDR(battery_id);
-static DEFINE_MUTEX(battery_mutex);
-
struct bq27x00_device_info;
struct bq27x00_access_methods {
int (*read)(struct bq27x00_device_info *, u8 reg, int *rt_value,
@@ -317,9 +314,15 @@ static int bq27x00_powersupply_init(struct bq27x00_device_info *di)
return 0;
}
-/*
- * i2c specific code
+
+/* i2c specific code */
+#ifdef CONFIG_BATTERY_BQ27X00_I2C
+
+/* If the system has several batteries we need a different name for each
+ * of them...
*/
+static DEFINE_IDR(battery_id);
+static DEFINE_MUTEX(battery_mutex);
static int bq27x00_read_i2c(struct bq27x00_device_info *di, u8 reg,
int *rt_value, bool single)
@@ -434,10 +437,6 @@ static int bq27x00_battery_remove(struct i2c_client *client)
return 0;
}
-/*
- * Module stuff
- */
-
static const struct i2c_device_id bq27x00_id[] = {
{ "bq27200", BQ27000 }, /* bq27200 is same as bq27000, but with i2c */
{ "bq27500", BQ27500 },
@@ -453,13 +452,167 @@ static struct i2c_driver bq27x00_battery_driver = {
.id_table = bq27x00_id,
};
+static inline int bq27x00_battery_i2c_init(void)
+{
+ int ret = i2c_add_driver(&bq27x00_battery_driver);
+ if (ret)
+ printk(KERN_ERR "Unable to register BQ27x00 i2c driver\n");
+
+ return ret;
+}
+
+static inline void bq27x00_battery_i2c_exit(void)
+{
+ i2c_del_driver(&bq27x00_battery_driver);
+}
+
+#else
+
+static inline int bq27x00_battery_i2c_init(void) { return 0; }
+static inline void bq27x00_battery_i2c_exit(void) {};
+
+#endif
+
+/* platform specific code */
+#ifdef CONFIG_BATTERY_BQ27X00_PLATFORM
+
+static int bq27000_read_platform(struct bq27x00_device_info *di, u8 reg,
+ int *rt_value, bool single)
+{
+ struct device *dev = di->dev;
+ struct bq27000_platform_data *pdata = dev->platform_data;
+ unsigned int timeout = 3;
+ int upper, lower;
+ int temp;
+
+ if (!single) {
+ /* Make sure the value has not changed in between reading the
+ * lower and the upper part */
+ upper = pdata->read(dev, reg + 1);
+ do {
+ temp = upper;
+ if (upper < 0)
+ return upper;
+
+ lower = pdata->read(dev, reg);
+ if (lower < 0)
+ return lower;
+
+ upper = pdata->read(dev, reg + 1);
+ } while (temp != upper && --timeout);
+
+ if (timeout == 0)
+ return -EIO;
+
+ *rt_value = (upper << 8) | lower;
+ } else {
+ lower = pdata->read(dev, reg);
+ if (lower < 0)
+ return lower;
+ *rt_value = lower;
+ }
+ return 0;
+}
+
+static int __devinit bq27000_battery_probe(struct platform_device *pdev)
+{
+ struct bq27x00_device_info *di;
+ struct bq27000_platform_data *pdata = pdev->dev.platform_data;
+ int ret;
+
+ if (!pdata) {
+ dev_err(&pdev->dev, "no platform_data supplied\n");
+ return -EINVAL;
+ }
+
+ if (!pdata->read) {
+ dev_err(&pdev->dev, "no hdq read callback supplied\n");
+ return -EINVAL;
+ }
+
+ di = kzalloc(sizeof(*di), GFP_KERNEL);
+ if (!di) {
+ dev_err(&pdev->dev, "failed to allocate device info data\n");
+ return -ENOMEM;
+ }
+
+ platform_set_drvdata(pdev, di);
+
+ di->dev = &pdev->dev;
+ di->chip = BQ27000;
+
+ di->bat.name = pdata->name ?: dev_name(&pdev->dev);
+ di->bus.read = &bq27000_read_platform;
+
+ ret = bq27x00_powersupply_init(di);
+ if (ret)
+ goto err_free;
+
+ return 0;
+
+err_free:
+ platform_set_drvdata(pdev, NULL);
+ kfree(di);
+
+ return ret;
+}
+
+static int __devexit bq27000_battery_remove(struct platform_device *pdev)
+{
+ struct bq27x00_device_info *di = platform_get_drvdata(pdev);
+
+ power_supply_unregister(&di->bat);
+ platform_set_drvdata(pdev, NULL);
+ kfree(di);
+
+ return 0;
+}
+
+static struct platform_driver bq27000_battery_driver = {
+ .probe = bq27000_battery_probe,
+ .remove = __devexit_p(bq27000_battery_remove),
+ .driver = {
+ .name = "bq27000-battery",
+ .owner = THIS_MODULE,
+ },
+};
+
+static inline int bq27x00_battery_platform_init(void)
+{
+ int ret = platform_driver_register(&bq27000_battery_driver);
+ if (ret)
+ printk(KERN_ERR "Unable to register BQ27000 platform driver\n");
+
+ return ret;
+}
+
+static inline void bq27x00_battery_platform_exit(void)
+{
+ platform_driver_unregister(&bq27000_battery_driver);
+}
+
+#else
+
+static inline int bq27x00_battery_platform_init(void) { return 0; }
+static inline void bq27x00_battery_platform_exit(void) {};
+
+#endif
+
+/*
+ * Module stuff
+ */
+
static int __init bq27x00_battery_init(void)
{
int ret;
- ret = i2c_add_driver(&bq27x00_battery_driver);
+ ret = bq27x00_battery_i2c_init();
+ if (ret)
+ return ret;
+
+ ret = bq27x00_battery_platform_init();
if (ret)
- printk(KERN_ERR "Unable to register BQ27x00 driver\n");
+ bq27x00_battery_i2c_exit();
return ret;
}
@@ -467,7 +620,8 @@ module_init(bq27x00_battery_init);
static void __exit bq27x00_battery_exit(void)
{
- i2c_del_driver(&bq27x00_battery_driver);
+ bq27x00_battery_platform_exit();
+ bq27x00_battery_i2c_exit();
}
module_exit(bq27x00_battery_exit);
diff --git a/include/linux/power/bq27x00_battery.h b/include/linux/power/bq27x00_battery.h
new file mode 100644
index 0000000..a857f71
--- /dev/null
+++ b/include/linux/power/bq27x00_battery.h
@@ -0,0 +1,19 @@
+#ifndef __LINUX_BQ27X00_BATTERY_H__
+#define __LINUX_BQ27X00_BATTERY_H__
+
+/**
+ * struct bq27000_plaform_data - Platform data for bq27000 devices
+ * @name: Name of the battery. If NULL the driver will fallback to "bq27000".
+ * @read: HDQ read callback.
+ * This function should provide access to the HDQ bus the battery is
+ * connected to.
+ * The first parameter is a pointer to the battery device, the second the
+ * register to be read. The return value should either be the content of
+ * the passed register or an error value.
+ */
+struct bq27000_platform_data {
+ const char *name;
+ int (*read)(struct device *dev, unsigned int);
+};
+
+#endif
--
1.7.2.3
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH 07/14] bq27x00: Cache battery registers
2011-02-12 19:38 [PATCH v2 00/14] POWER: BQ27x00: New Properties, fixes, bq27000 support Lars-Peter Clausen
` (5 preceding siblings ...)
2011-02-12 19:39 ` [PATCH 06/14] bq27x00: Add bq27000 support Lars-Peter Clausen
@ 2011-02-12 19:39 ` Lars-Peter Clausen
2011-02-13 15:14 ` Grazvydas Ignotas
2011-02-14 3:01 ` [PATCH 07/14 v3] " Lars-Peter Clausen
2011-02-12 19:39 ` [PATCH v2 07/14] bq27X00: " Lars-Peter Clausen
` (7 subsequent siblings)
14 siblings, 2 replies; 28+ messages in thread
From: Lars-Peter Clausen @ 2011-02-12 19:39 UTC (permalink / raw)
To: Anton Vorontsov
Cc: Rodolfo Giometti, Grazvydas Ignotas, linux-kernel, Lars-Peter Clausen
This patch adds a register cache to the bq27x00 battery driver.
Usually multiple, if not all, power_supply properties are queried at once,
for example when an uevent is generated. Since some registers are used by
multiple properties caching the registers should reduce the number of
reads.
The cache is valid for 5 seconds this roughly matches the internal update
interval of the current register for the bq27000/bq27200.
Fast changing properties(*_NOW) which can be obtained by reading a single
register are not cached.
It will also be used in the follow up patch to check if the battery status
has been changed since the last update to emit power_supply_changed events.
Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
drivers/power/bq27x00_battery.c | 272 +++++++++++++++++++++-----------------
1 files changed, 150 insertions(+), 122 deletions(-)
diff --git a/drivers/power/bq27x00_battery.c b/drivers/power/bq27x00_battery.c
index 44bc76b..0c94693 100644
--- a/drivers/power/bq27x00_battery.c
+++ b/drivers/power/bq27x00_battery.c
@@ -19,7 +19,6 @@
#include <linux/module.h>
#include <linux/param.h>
#include <linux/jiffies.h>
-#include <linux/workqueue.h>
#include <linux/delay.h>
#include <linux/platform_device.h>
#include <linux/power_supply.h>
@@ -51,17 +50,30 @@
struct bq27x00_device_info;
struct bq27x00_access_methods {
- int (*read)(struct bq27x00_device_info *, u8 reg, int *rt_value,
- bool single);
+ int (*read)(struct bq27x00_device_info *di, u8 reg, bool single);
};
enum bq27x00_chip { BQ27000, BQ27500 };
+struct bq27x00_reg_cache {
+ int temperature;
+ int time_to_empty;
+ int time_to_empty_avg;
+ int time_to_full;
+ int capacity;
+ int flags;
+
+ int current_now;
+};
+
struct bq27x00_device_info {
struct device *dev;
int id;
enum bq27x00_chip chip;
+ struct bq27x00_reg_cache cache;
+ unsigned long last_update;
+
struct power_supply bat;
struct bq27x00_access_methods bus;
@@ -85,48 +97,93 @@ static enum power_supply_property bq27x00_battery_props[] = {
*/
static inline int bq27x00_read(struct bq27x00_device_info *di, u8 reg,
- int *rt_value, bool single)
+ bool single)
{
- return di->bus.read(di, reg, rt_value, single);
+ return di->bus.read(di, reg, single);
}
/*
- * Return the battery temperature in tenths of degree Celsius
+ * Return the battery Relative State-of-Charge
* Or < 0 if something fails.
*/
-static int bq27x00_battery_temperature(struct bq27x00_device_info *di)
+static int bq27x00_battery_read_rsoc(struct bq27x00_device_info *di)
{
- int ret;
- int temp = 0;
-
- ret = bq27x00_read(di, BQ27x00_REG_TEMP, &temp, false);
- if (ret) {
- dev_err(di->dev, "error reading temperature\n");
- return ret;
- }
+ int rsoc;
if (di->chip == BQ27500)
- return temp - 2731;
+ rsoc = bq27x00_read(di, BQ27500_REG_SOC, false);
else
- return ((temp * 5) - 5463) / 2;
+ rsoc = bq27x00_read(di, BQ27000_REG_RSOC, true);
+
+ if (rsoc < 0)
+ dev_err(di->dev, "error reading relative State-of-Charge\n");
+
+ return rsoc;
}
/*
- * Return the battery Voltage in milivolts
- * Or < 0 if something fails.
+ * Read a time register.
+ * Return < 0 if something fails.
*/
-static int bq27x00_battery_voltage(struct bq27x00_device_info *di)
+static int bq27x00_battery_read_time(struct bq27x00_device_info *di, u8 reg)
{
- int ret;
- int volt = 0;
+ int tval;
- ret = bq27x00_read(di, BQ27x00_REG_VOLT, &volt, false);
- if (ret) {
- dev_err(di->dev, "error reading voltage\n");
- return ret;
+ tval = bq27x00_read(di, reg, false);
+ if (tval < 0) {
+ dev_err(di->dev, "error reading register %02x: %d\n", reg, tval);
+ return tval;
+ }
+
+ if (tval == 65535)
+ return -ENODATA;
+
+ return tval * 60;
+}
+
+static void bq27x00_update(struct bq27x00_device_info *di)
+{
+ struct bq27x00_reg_cache cache = {0, };
+ bool is_bq27500 = di->chip == BQ27500;
+
+ cache.flags = bq27x00_read(di, BQ27x00_REG_FLAGS, is_bq27500);
+ if (cache.flags >= 0) {
+ cache.capacity = bq27x00_battery_read_rsoc(di);
+ cache.temperature = bq27x00_read(di, BQ27x00_REG_TEMP, false);
+ cache.time_to_empty = bq27x00_battery_read_time(di, BQ27x00_REG_TTE);
+ cache.time_to_empty_avg = bq27x00_battery_read_time(di, BQ27x00_REG_TTECP);
+ cache.time_to_full = bq27x00_battery_read_time(di, BQ27x00_REG_TTF);
+
+ if (!is_bq27500)
+ cache.current_now = bq27x00_read(di, BQ27x00_REG_AI, false);
}
- return volt * 1000;
+ /* Ignore current_now which is a snapshot of the current battery state
+ * and is likely to be different even between two consecutive reads */
+ if (memcmp(&di->cache, &cache, sizeof(cache) - sizeof(int)) != 0) {
+ di->cache = cache;
+ power_supply_changed(&di->bat);
+ }
+
+ di->last_update = jiffies;
+}
+
+/*
+ * Return the battery temperature in tenths of degree Celsius
+ * Or < 0 if something fails.
+ */
+static int bq27x00_battery_temperature(struct bq27x00_device_info *di,
+ union power_supply_propval *val)
+{
+ if (di->cache.temperature < 0)
+ return di->cache.temperature;
+
+ if (di->chip == BQ27500)
+ val->intval = di->cache.temperature - 2731;
+ else
+ val->intval = ((di->cache.temperature * 5) - 5463) / 2;
+
+ return 0;
}
/*
@@ -134,109 +191,84 @@ static int bq27x00_battery_voltage(struct bq27x00_device_info *di)
* Note that current can be negative signed as well
* Or 0 if something fails.
*/
-static int bq27x00_battery_current(struct bq27x00_device_info *di)
+static int bq27x00_battery_current(struct bq27x00_device_info *di,
+ union power_supply_propval *val)
{
- int ret;
- int curr = 0;
- int flags = 0;
+ int curr;
- ret = bq27x00_read(di, BQ27x00_REG_AI, &curr, false);
- if (ret) {
- dev_err(di->dev, "error reading current\n");
- return 0;
- }
+ if (di->chip == BQ27000)
+ curr = bq27x00_read(di, BQ27x00_REG_AI, false);
+ else
+ curr = di->cache.current_now;
+
+ if (curr < 0)
+ return curr;
if (di->chip == BQ27500) {
/* bq27500 returns signed value */
- curr = (int)((s16)curr) * 1000;
+ val->intval = (int)((s16)curr) * 1000;
} else {
- ret = bq27x00_read(di, BQ27x00_REG_FLAGS, &flags, false);
- if (ret < 0) {
- dev_err(di->dev, "error reading flags\n");
- return 0;
- }
- if (flags & BQ27000_FLAG_CHGS) {
+ if (di->cache.flags & BQ27000_FLAG_CHGS) {
dev_dbg(di->dev, "negative current!\n");
curr = -curr;
}
- curr = curr * 3570 / BQ27000_RS;
- }
-
- return curr;
-}
-
-/*
- * Return the battery Relative State-of-Charge
- * Or < 0 if something fails.
- */
-static int bq27x00_battery_rsoc(struct bq27x00_device_info *di)
-{
- int ret;
- int rsoc = 0;
- if (di->chip == BQ27500)
- ret = bq27x00_read(di, BQ27500_REG_SOC, &rsoc, false);
- else
- ret = bq27x00_read(di, BQ27000_REG_RSOC, &rsoc, true);
- if (ret) {
- dev_err(di->dev, "error reading relative State-of-Charge\n");
- return ret;
+ val->intval = curr * 3570 / BQ27000_RS;
}
- return rsoc;
+ return 0;
}
static int bq27x00_battery_status(struct bq27x00_device_info *di,
- union power_supply_propval *val)
+ union power_supply_propval *val)
{
- int flags = 0;
int status;
- int ret;
-
- ret = bq27x00_read(di, BQ27x00_REG_FLAGS, &flags, false);
- if (ret < 0) {
- dev_err(di->dev, "error reading flags\n");
- return ret;
- }
if (di->chip == BQ27500) {
- if (flags & BQ27500_FLAG_FC)
+ if (di->cache.flags & BQ27500_FLAG_FC)
status = POWER_SUPPLY_STATUS_FULL;
- else if (flags & BQ27500_FLAG_DSC)
+ else if (di->cache.flags & BQ27500_FLAG_DSC)
status = POWER_SUPPLY_STATUS_DISCHARGING;
else
status = POWER_SUPPLY_STATUS_CHARGING;
} else {
- if (flags & BQ27000_FLAG_CHGS)
+ if (di->cache.flags & BQ27000_FLAG_CHGS)
status = POWER_SUPPLY_STATUS_CHARGING;
else
status = POWER_SUPPLY_STATUS_DISCHARGING;
}
val->intval = status;
+
return 0;
}
/*
- * Read a time register.
- * Return < 0 if something fails.
+ * Return the battery Voltage in milivolts
+ * Or < 0 if something fails.
*/
-static int bq27x00_battery_time(struct bq27x00_device_info *di, int reg,
- union power_supply_propval *val)
+static int bq27x00_battery_voltage(struct bq27x00_device_info *di,
+ union power_supply_propval *val)
{
- int tval = 0;
- int ret;
+ int volt;
- ret = bq27x00_read(di, reg, &tval, false);
- if (ret) {
- dev_err(di->dev, "error reading register %02x\n", reg);
- return ret;
- }
+ volt = bq27x00_read(di, BQ27x00_REG_VOLT, false);
+ if (volt < 0)
+ return volt;
- if (tval == 65535)
- return -ENODATA;
+ val->intval = volt * 1000;
+
+ return 0;
+}
+
+static int bq27x00_simple_value(int value,
+ union power_supply_propval *val)
+{
+ if (value < 0)
+ return value;
+
+ val->intval = value;
- val->intval = tval * 60;
return 0;
}
@@ -249,9 +281,11 @@ static int bq27x00_battery_get_property(struct power_supply *psy,
{
int ret = 0;
struct bq27x00_device_info *di = to_bq27x00_device_info(psy);
- int voltage = bq27x00_battery_voltage(di);
- if (psp != POWER_SUPPLY_PROP_PRESENT && voltage <= 0)
+ if (time_is_before_jiffies(di->last_update + 5 * HZ))
+ bq27x00_update(di);
+
+ if (psp != POWER_SUPPLY_PROP_PRESENT && di->cache.flags < 0)
return -ENODEV;
switch (psp) {
@@ -259,29 +293,28 @@ static int bq27x00_battery_get_property(struct power_supply *psy,
ret = bq27x00_battery_status(di, val);
break;
case POWER_SUPPLY_PROP_VOLTAGE_NOW:
- val->intval = voltage;
+ ret = bq27x00_battery_voltage(di, val);
break;
case POWER_SUPPLY_PROP_PRESENT:
- if (psp == POWER_SUPPLY_PROP_PRESENT)
- val->intval = voltage <= 0 ? 0 : 1;
+ val->intval = di->cache.flags < 0 ? 0 : 1;
break;
case POWER_SUPPLY_PROP_CURRENT_NOW:
- val->intval = bq27x00_battery_current(di);
+ ret = bq27x00_battery_current(di, val);
break;
case POWER_SUPPLY_PROP_CAPACITY:
- val->intval = bq27x00_battery_rsoc(di);
+ ret = bq27x00_simple_value(di->cache.capacity, val);
break;
case POWER_SUPPLY_PROP_TEMP:
- val->intval = bq27x00_battery_temperature(di);
+ ret = bq27x00_battery_temperature(di, val);
break;
case POWER_SUPPLY_PROP_TIME_TO_EMPTY_NOW:
- ret = bq27x00_battery_time(di, BQ27x00_REG_TTE, val);
+ ret = bq27x00_simple_value(di->cache.time_to_empty, val);
break;
case POWER_SUPPLY_PROP_TIME_TO_EMPTY_AVG:
- ret = bq27x00_battery_time(di, BQ27x00_REG_TTECP, val);
+ ret = bq27x00_simple_value(di->cache.time_to_empty_avg, val);
break;
case POWER_SUPPLY_PROP_TIME_TO_FULL_NOW:
- ret = bq27x00_battery_time(di, BQ27x00_REG_TTF, val);
+ ret = bq27x00_simple_value(di->cache.time_to_full, val);
break;
case POWER_SUPPLY_PROP_TECHNOLOGY:
val->intval = POWER_SUPPLY_TECHNOLOGY_LION;
@@ -311,6 +344,8 @@ static int bq27x00_powersupply_init(struct bq27x00_device_info *di)
dev_info(di->dev, "support ver. %s enabled\n", DRIVER_VERSION);
+ bq27x00_update(di);
+
return 0;
}
@@ -324,13 +359,12 @@ static int bq27x00_powersupply_init(struct bq27x00_device_info *di)
static DEFINE_IDR(battery_id);
static DEFINE_MUTEX(battery_mutex);
-static int bq27x00_read_i2c(struct bq27x00_device_info *di, u8 reg,
- int *rt_value, bool single)
+static int bq27x00_read_i2c(struct bq27x00_device_info *di, u8 reg, bool single)
{
struct i2c_client *client = to_i2c_client(di->dev);
struct i2c_msg msg[1];
unsigned char data[2];
- int err;
+ int ret;
if (!client->adapter)
return -ENODEV;
@@ -341,26 +375,24 @@ static int bq27x00_read_i2c(struct bq27x00_device_info *di, u8 reg,
msg->buf = data;
data[0] = reg;
- err = i2c_transfer(client->adapter, msg, 1);
+ ret = i2c_transfer(client->adapter, msg, 1);
- if (err >= 0) {
+ if (ret >= 0) {
if (!single)
msg->len = 2;
else
msg->len = 1;
msg->flags = I2C_M_RD;
- err = i2c_transfer(client->adapter, msg, 1);
- if (err >= 0) {
+ ret = i2c_transfer(client->adapter, msg, 1);
+ if (ret >= 0) {
if (!single)
- *rt_value = get_unaligned_le16(data);
+ ret = get_unaligned_le16(data);
else
- *rt_value = data[0];
-
- return 0;
+ ret = data[0];
}
}
- return err;
+ return ret;
}
static int bq27x00_battery_probe(struct i2c_client *client,
@@ -477,7 +509,7 @@ static inline void bq27x00_battery_i2c_exit(void) {};
#ifdef CONFIG_BATTERY_BQ27X00_PLATFORM
static int bq27000_read_platform(struct bq27x00_device_info *di, u8 reg,
- int *rt_value, bool single)
+ bool single)
{
struct device *dev = di->dev;
struct bq27000_platform_data *pdata = dev->platform_data;
@@ -504,14 +536,10 @@ static int bq27000_read_platform(struct bq27x00_device_info *di, u8 reg,
if (timeout == 0)
return -EIO;
- *rt_value = (upper << 8) | lower;
- } else {
- lower = pdata->read(dev, reg);
- if (lower < 0)
- return lower;
- *rt_value = lower;
+ return (upper << 8) | lower;
}
- return 0;
+
+ return pdata->read(dev, reg);
}
static int __devinit bq27000_battery_probe(struct platform_device *pdev)
--
1.7.2.3
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH v2 07/14] bq27X00: Cache battery registers
2011-02-12 19:38 [PATCH v2 00/14] POWER: BQ27x00: New Properties, fixes, bq27000 support Lars-Peter Clausen
` (6 preceding siblings ...)
2011-02-12 19:39 ` [PATCH 07/14] bq27x00: Cache battery registers Lars-Peter Clausen
@ 2011-02-12 19:39 ` Lars-Peter Clausen
2011-02-12 19:52 ` Lars-Peter Clausen
2011-02-12 19:39 ` [PATCH 08/14] bq27x00: Poll battery state Lars-Peter Clausen
` (6 subsequent siblings)
14 siblings, 1 reply; 28+ messages in thread
From: Lars-Peter Clausen @ 2011-02-12 19:39 UTC (permalink / raw)
To: Anton Vorontsov
Cc: Rodolfo Giometti, Grazvydas Ignotas, linux-kernel, Lars-Peter Clausen
This patch adds a register cache to the bq27x00 battery driver.
Usually multiple, if not all, power_supply properties are queried at once,
for example when an uevent is generated. Since some registers are used by
multiple properties caching the registers should reduce the number of
reads.
The cache is valid for 5 seconds this roughly matches the internal update
interval of the current register for the bq27000/bq27200.
Fast changing properties(*_NOW) which can be obtained by reading a single
register are not cached.
It will also be used in the follow up patch to check if the battery status
has been changed since the last update to emit power_supply_changed events.
Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
drivers/power/bq27x00_battery.c | 272 +++++++++++++++++++++-----------------
1 files changed, 150 insertions(+), 122 deletions(-)
diff --git a/drivers/power/bq27x00_battery.c b/drivers/power/bq27x00_battery.c
index ae4677f..0c94693 100644
--- a/drivers/power/bq27x00_battery.c
+++ b/drivers/power/bq27x00_battery.c
@@ -19,7 +19,6 @@
#include <linux/module.h>
#include <linux/param.h>
#include <linux/jiffies.h>
-#include <linux/workqueue.h>
#include <linux/delay.h>
#include <linux/platform_device.h>
#include <linux/power_supply.h>
@@ -51,17 +50,30 @@
struct bq27x00_device_info;
struct bq27x00_access_methods {
- int (*read)(struct bq27x00_device_info *, u8 reg, int *rt_value,
- bool single);
+ int (*read)(struct bq27x00_device_info *di, u8 reg, bool single);
};
enum bq27x00_chip { BQ27000, BQ27500 };
+struct bq27x00_reg_cache {
+ int temperature;
+ int time_to_empty;
+ int time_to_empty_avg;
+ int time_to_full;
+ int capacity;
+ int flags;
+
+ int current_now;
+};
+
struct bq27x00_device_info {
struct device *dev;
int id;
enum bq27x00_chip chip;
+ struct bq27x00_reg_cache cache;
+ unsigned long last_update;
+
struct power_supply bat;
struct bq27x00_access_methods bus;
@@ -85,48 +97,93 @@ static enum power_supply_property bq27x00_battery_props[] = {
*/
static inline int bq27x00_read(struct bq27x00_device_info *di, u8 reg,
- int *rt_value, bool single)
+ bool single)
{
- return di->bus.read(di, reg, rt_value, single);
+ return di->bus.read(di, reg, single);
}
/*
- * Return the battery temperature in tenths of degree Celsius
+ * Return the battery Relative State-of-Charge
* Or < 0 if something fails.
*/
-static int bq27x00_battery_temperature(struct bq27x00_device_info *di)
+static int bq27x00_battery_read_rsoc(struct bq27x00_device_info *di)
{
- int ret;
- int temp = 0;
-
- ret = bq27x00_read(di, BQ27x00_REG_TEMP, &temp, false);
- if (ret) {
- dev_err(di->dev, "error reading temperature\n");
- return ret;
- }
+ int rsoc;
if (di->chip == BQ27500)
- return temp - 2731;
+ rsoc = bq27x00_read(di, BQ27500_REG_SOC, false);
else
- return ((temp * 5) - 5463) / 2;
+ rsoc = bq27x00_read(di, BQ27000_REG_RSOC, true);
+
+ if (rsoc < 0)
+ dev_err(di->dev, "error reading relative State-of-Charge\n");
+
+ return rsoc;
}
/*
- * Return the battery Voltage in milivolts
- * Or < 0 if something fails.
+ * Read a time register.
+ * Return < 0 if something fails.
*/
-static int bq27x00_battery_voltage(struct bq27x00_device_info *di)
+static int bq27x00_battery_read_time(struct bq27x00_device_info *di, u8 reg)
{
- int ret;
- int volt = 0;
+ int tval;
- ret = bq27x00_read(di, BQ27x00_REG_VOLT, &volt, false);
- if (ret) {
- dev_err(di->dev, "error reading voltage\n");
- return ret;
+ tval = bq27x00_read(di, reg, false);
+ if (tval < 0) {
+ dev_err(di->dev, "error reading register %02x: %d\n", reg, tval);
+ return tval;
+ }
+
+ if (tval == 65535)
+ return -ENODATA;
+
+ return tval * 60;
+}
+
+static void bq27x00_update(struct bq27x00_device_info *di)
+{
+ struct bq27x00_reg_cache cache = {0, };
+ bool is_bq27500 = di->chip == BQ27500;
+
+ cache.flags = bq27x00_read(di, BQ27x00_REG_FLAGS, is_bq27500);
+ if (cache.flags >= 0) {
+ cache.capacity = bq27x00_battery_read_rsoc(di);
+ cache.temperature = bq27x00_read(di, BQ27x00_REG_TEMP, false);
+ cache.time_to_empty = bq27x00_battery_read_time(di, BQ27x00_REG_TTE);
+ cache.time_to_empty_avg = bq27x00_battery_read_time(di, BQ27x00_REG_TTECP);
+ cache.time_to_full = bq27x00_battery_read_time(di, BQ27x00_REG_TTF);
+
+ if (!is_bq27500)
+ cache.current_now = bq27x00_read(di, BQ27x00_REG_AI, false);
}
- return volt * 1000;
+ /* Ignore current_now which is a snapshot of the current battery state
+ * and is likely to be different even between two consecutive reads */
+ if (memcmp(&di->cache, &cache, sizeof(cache) - sizeof(int)) != 0) {
+ di->cache = cache;
+ power_supply_changed(&di->bat);
+ }
+
+ di->last_update = jiffies;
+}
+
+/*
+ * Return the battery temperature in tenths of degree Celsius
+ * Or < 0 if something fails.
+ */
+static int bq27x00_battery_temperature(struct bq27x00_device_info *di,
+ union power_supply_propval *val)
+{
+ if (di->cache.temperature < 0)
+ return di->cache.temperature;
+
+ if (di->chip == BQ27500)
+ val->intval = di->cache.temperature - 2731;
+ else
+ val->intval = ((di->cache.temperature * 5) - 5463) / 2;
+
+ return 0;
}
/*
@@ -134,109 +191,84 @@ static int bq27x00_battery_voltage(struct bq27x00_device_info *di)
* Note that current can be negative signed as well
* Or 0 if something fails.
*/
-static int bq27x00_battery_current(struct bq27x00_device_info *di)
+static int bq27x00_battery_current(struct bq27x00_device_info *di,
+ union power_supply_propval *val)
{
- int ret;
- int curr = 0;
- int flags = 0;
+ int curr;
- ret = bq27x00_read(di, BQ27x00_REG_AI, &curr, false);
- if (ret) {
- dev_err(di->dev, "error reading current\n");
- return 0;
- }
+ if (di->chip == BQ27000)
+ curr = bq27x00_read(di, BQ27x00_REG_AI, false);
+ else
+ curr = di->cache.current_now;
+
+ if (curr < 0)
+ return curr;
if (di->chip == BQ27500) {
/* bq27500 returns signed value */
- curr = (int)((s16)curr) * 1000;
+ val->intval = (int)((s16)curr) * 1000;
} else {
- ret = bq27x00_read(di, BQ27x00_REG_FLAGS, &flags, false);
- if (ret < 0) {
- dev_err(di->dev, "error reading flags\n");
- return 0;
- }
- if (flags & BQ27000_FLAG_CHGS) {
+ if (di->cache.flags & BQ27000_FLAG_CHGS) {
dev_dbg(di->dev, "negative current!\n");
curr = -curr;
}
- curr = curr * 3570 / BQ27000_RS;
- }
-
- return curr;
-}
-
-/*
- * Return the battery Relative State-of-Charge
- * Or < 0 if something fails.
- */
-static int bq27x00_battery_rsoc(struct bq27x00_device_info *di)
-{
- int ret;
- int rsoc = 0;
- if (di->chip == BQ27500)
- ret = bq27x00_read(di, BQ27500_REG_SOC, &rsoc, false);
- else
- ret = bq27x00_read(di, BQ27000_REG_RSOC, &rsoc, true);
- if (ret) {
- dev_err(di->dev, "error reading relative State-of-Charge\n");
- return ret;
+ val->intval = curr * 3570 / BQ27000_RS;
}
- return rsoc;
+ return 0;
}
static int bq27x00_battery_status(struct bq27x00_device_info *di,
- union power_supply_propval *val)
+ union power_supply_propval *val)
{
- int flags = 0;
int status;
- int ret;
-
- ret = bq27x00_read(di, BQ27x00_REG_FLAGS, &flags, false);
- if (ret < 0) {
- dev_err(di->dev, "error reading flags\n");
- return ret;
- }
if (di->chip == BQ27500) {
- if (flags & BQ27500_FLAG_FC)
+ if (di->cache.flags & BQ27500_FLAG_FC)
status = POWER_SUPPLY_STATUS_FULL;
- else if (flags & BQ27500_FLAG_DSC)
+ else if (di->cache.flags & BQ27500_FLAG_DSC)
status = POWER_SUPPLY_STATUS_DISCHARGING;
else
status = POWER_SUPPLY_STATUS_CHARGING;
} else {
- if (flags & BQ27000_FLAG_CHGS)
+ if (di->cache.flags & BQ27000_FLAG_CHGS)
status = POWER_SUPPLY_STATUS_CHARGING;
else
status = POWER_SUPPLY_STATUS_DISCHARGING;
}
val->intval = status;
+
return 0;
}
/*
- * Read a time register.
- * Return < 0 if something fails.
+ * Return the battery Voltage in milivolts
+ * Or < 0 if something fails.
*/
-static int bq27x00_battery_time(struct bq27x00_device_info *di, int reg,
- union power_supply_propval *val)
+static int bq27x00_battery_voltage(struct bq27x00_device_info *di,
+ union power_supply_propval *val)
{
- int tval = 0;
- int ret;
+ int volt;
- ret = bq27x00_read(reg, &tval, false);
- if (ret) {
- dev_err(di->dev, "error reading register %02x\n", reg);
- return ret;
- }
+ volt = bq27x00_read(di, BQ27x00_REG_VOLT, false);
+ if (volt < 0)
+ return volt;
- if (tval == 65535)
- return -ENODATA;
+ val->intval = volt * 1000;
+
+ return 0;
+}
+
+static int bq27x00_simple_value(int value,
+ union power_supply_propval *val)
+{
+ if (value < 0)
+ return value;
+
+ val->intval = value;
- val->intval = tval * 60;
return 0;
}
@@ -249,9 +281,11 @@ static int bq27x00_battery_get_property(struct power_supply *psy,
{
int ret = 0;
struct bq27x00_device_info *di = to_bq27x00_device_info(psy);
- int voltage = bq27x00_battery_voltage(di);
- if (psp != POWER_SUPPLY_PROP_PRESENT && voltage <= 0)
+ if (time_is_before_jiffies(di->last_update + 5 * HZ))
+ bq27x00_update(di);
+
+ if (psp != POWER_SUPPLY_PROP_PRESENT && di->cache.flags < 0)
return -ENODEV;
switch (psp) {
@@ -259,29 +293,28 @@ static int bq27x00_battery_get_property(struct power_supply *psy,
ret = bq27x00_battery_status(di, val);
break;
case POWER_SUPPLY_PROP_VOLTAGE_NOW:
- val->intval = voltage;
+ ret = bq27x00_battery_voltage(di, val);
break;
case POWER_SUPPLY_PROP_PRESENT:
- if (psp == POWER_SUPPLY_PROP_PRESENT)
- val->intval = voltage <= 0 ? 0 : 1;
+ val->intval = di->cache.flags < 0 ? 0 : 1;
break;
case POWER_SUPPLY_PROP_CURRENT_NOW:
- val->intval = bq27x00_battery_current(di);
+ ret = bq27x00_battery_current(di, val);
break;
case POWER_SUPPLY_PROP_CAPACITY:
- val->intval = bq27x00_battery_rsoc(di);
+ ret = bq27x00_simple_value(di->cache.capacity, val);
break;
case POWER_SUPPLY_PROP_TEMP:
- val->intval = bq27x00_battery_temperature(di);
+ ret = bq27x00_battery_temperature(di, val);
break;
case POWER_SUPPLY_PROP_TIME_TO_EMPTY_NOW:
- ret = bq27x00_battery_time(di, BQ27x00_REG_TTE, val);
+ ret = bq27x00_simple_value(di->cache.time_to_empty, val);
break;
case POWER_SUPPLY_PROP_TIME_TO_EMPTY_AVG:
- ret = bq27x00_battery_time(di, BQ27x00_REG_TTECP, val);
+ ret = bq27x00_simple_value(di->cache.time_to_empty_avg, val);
break;
case POWER_SUPPLY_PROP_TIME_TO_FULL_NOW:
- ret = bq27x00_battery_time(di, BQ27x00_REG_TTF, val);
+ ret = bq27x00_simple_value(di->cache.time_to_full, val);
break;
case POWER_SUPPLY_PROP_TECHNOLOGY:
val->intval = POWER_SUPPLY_TECHNOLOGY_LION;
@@ -311,6 +344,8 @@ static int bq27x00_powersupply_init(struct bq27x00_device_info *di)
dev_info(di->dev, "support ver. %s enabled\n", DRIVER_VERSION);
+ bq27x00_update(di);
+
return 0;
}
@@ -324,13 +359,12 @@ static int bq27x00_powersupply_init(struct bq27x00_device_info *di)
static DEFINE_IDR(battery_id);
static DEFINE_MUTEX(battery_mutex);
-static int bq27x00_read_i2c(struct bq27x00_device_info *di, u8 reg,
- int *rt_value, bool single)
+static int bq27x00_read_i2c(struct bq27x00_device_info *di, u8 reg, bool single)
{
struct i2c_client *client = to_i2c_client(di->dev);
struct i2c_msg msg[1];
unsigned char data[2];
- int err;
+ int ret;
if (!client->adapter)
return -ENODEV;
@@ -341,26 +375,24 @@ static int bq27x00_read_i2c(struct bq27x00_device_info *di, u8 reg,
msg->buf = data;
data[0] = reg;
- err = i2c_transfer(client->adapter, msg, 1);
+ ret = i2c_transfer(client->adapter, msg, 1);
- if (err >= 0) {
+ if (ret >= 0) {
if (!single)
msg->len = 2;
else
msg->len = 1;
msg->flags = I2C_M_RD;
- err = i2c_transfer(client->adapter, msg, 1);
- if (err >= 0) {
+ ret = i2c_transfer(client->adapter, msg, 1);
+ if (ret >= 0) {
if (!single)
- *rt_value = get_unaligned_le16(data);
+ ret = get_unaligned_le16(data);
else
- *rt_value = data[0];
-
- return 0;
+ ret = data[0];
}
}
- return err;
+ return ret;
}
static int bq27x00_battery_probe(struct i2c_client *client,
@@ -477,7 +509,7 @@ static inline void bq27x00_battery_i2c_exit(void) {};
#ifdef CONFIG_BATTERY_BQ27X00_PLATFORM
static int bq27000_read_platform(struct bq27x00_device_info *di, u8 reg,
- int *rt_value, bool single)
+ bool single)
{
struct device *dev = di->dev;
struct bq27000_platform_data *pdata = dev->platform_data;
@@ -504,14 +536,10 @@ static int bq27000_read_platform(struct bq27x00_device_info *di, u8 reg,
if (timeout == 0)
return -EIO;
- *rt_value = (upper << 8) | lower;
- } else {
- lower = pdata->read(dev, reg);
- if (lower < 0)
- return lower;
- *rt_value = lower;
+ return (upper << 8) | lower;
}
- return 0;
+
+ return pdata->read(dev, reg);
}
static int __devinit bq27000_battery_probe(struct platform_device *pdev)
--
1.7.2.3
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH 08/14] bq27x00: Poll battery state
2011-02-12 19:38 [PATCH v2 00/14] POWER: BQ27x00: New Properties, fixes, bq27000 support Lars-Peter Clausen
` (7 preceding siblings ...)
2011-02-12 19:39 ` [PATCH v2 07/14] bq27X00: " Lars-Peter Clausen
@ 2011-02-12 19:39 ` Lars-Peter Clausen
2011-02-12 19:39 ` [PATCH 09/14] bq27x00: Add new properties Lars-Peter Clausen
` (5 subsequent siblings)
14 siblings, 0 replies; 28+ messages in thread
From: Lars-Peter Clausen @ 2011-02-12 19:39 UTC (permalink / raw)
To: Anton Vorontsov
Cc: Rodolfo Giometti, Grazvydas Ignotas, linux-kernel, Lars-Peter Clausen
This patch adds support for polling the battery state and generating a
power_supply_changed() event if it has changed.
Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
drivers/power/bq27x00_battery.c | 59 +++++++++++++++++++++++++++++++++++---
1 files changed, 54 insertions(+), 5 deletions(-)
diff --git a/drivers/power/bq27x00_battery.c b/drivers/power/bq27x00_battery.c
index 0c94693..061c5a5 100644
--- a/drivers/power/bq27x00_battery.c
+++ b/drivers/power/bq27x00_battery.c
@@ -19,6 +19,7 @@
#include <linux/module.h>
#include <linux/param.h>
#include <linux/jiffies.h>
+#include <linux/workqueue.h>
#include <linux/delay.h>
#include <linux/platform_device.h>
#include <linux/power_supply.h>
@@ -73,10 +74,13 @@ struct bq27x00_device_info {
struct bq27x00_reg_cache cache;
unsigned long last_update;
+ struct delayed_work work;
struct power_supply bat;
struct bq27x00_access_methods bus;
+
+ struct mutex lock;
};
static enum power_supply_property bq27x00_battery_props[] = {
@@ -92,6 +96,11 @@ static enum power_supply_property bq27x00_battery_props[] = {
POWER_SUPPLY_PROP_TECHNOLOGY,
};
+static unsigned int poll_interval = 360;
+module_param(poll_interval, uint, 0644);
+MODULE_PARM_DESC(poll_interval, "battery poll interval in seconds - " \
+ "0 disables polling");
+
/*
* Common code for BQ27x00 devices
*/
@@ -168,6 +177,21 @@ static void bq27x00_update(struct bq27x00_device_info *di)
di->last_update = jiffies;
}
+static void bq27x00_battery_poll(struct work_struct *work)
+{
+ struct bq27x00_device_info *di =
+ container_of(work, struct bq27x00_device_info, work.work);
+
+ bq27x00_update(di);
+
+ if (poll_interval > 0) {
+ /* The timer does not have to be accurate. */
+ set_timer_slack(&di->work.timer, poll_interval * HZ / 4);
+ schedule_delayed_work(&di->work, poll_interval * HZ);
+ }
+}
+
+
/*
* Return the battery temperature in tenths of degree Celsius
* Or < 0 if something fails.
@@ -282,8 +306,12 @@ static int bq27x00_battery_get_property(struct power_supply *psy,
int ret = 0;
struct bq27x00_device_info *di = to_bq27x00_device_info(psy);
- if (time_is_before_jiffies(di->last_update + 5 * HZ))
- bq27x00_update(di);
+ mutex_lock(&di->lock);
+ if (time_is_before_jiffies(di->last_update + 5 * HZ)) {
+ cancel_delayed_work_sync(&di->work);
+ bq27x00_battery_poll(&di->work.work);
+ }
+ mutex_unlock(&di->lock);
if (psp != POWER_SUPPLY_PROP_PRESENT && di->cache.flags < 0)
return -ENODEV;
@@ -326,6 +354,14 @@ static int bq27x00_battery_get_property(struct power_supply *psy,
return ret;
}
+static void bq27x00_external_power_changed(struct power_supply *psy)
+{
+ struct bq27x00_device_info *di = to_bq27x00_device_info(psy);
+
+ cancel_delayed_work_sync(&di->work);
+ schedule_delayed_work(&di->work, 0);
+}
+
static int bq27x00_powersupply_init(struct bq27x00_device_info *di)
{
int ret;
@@ -334,7 +370,10 @@ static int bq27x00_powersupply_init(struct bq27x00_device_info *di)
di->bat.properties = bq27x00_battery_props;
di->bat.num_properties = ARRAY_SIZE(bq27x00_battery_props);
di->bat.get_property = bq27x00_battery_get_property;
- di->bat.external_power_changed = NULL;
+ di->bat.external_power_changed = bq27x00_external_power_changed;
+
+ INIT_DELAYED_WORK(&di->work, bq27x00_battery_poll);
+ mutex_init(&di->lock);
ret = power_supply_register(di->dev, &di->bat);
if (ret) {
@@ -349,6 +388,15 @@ static int bq27x00_powersupply_init(struct bq27x00_device_info *di)
return 0;
}
+static void bq27x00_powersupply_unregister(struct bq27x00_device_info *di)
+{
+ cancel_delayed_work_sync(&di->work);
+
+ power_supply_unregister(&di->bat);
+
+ mutex_destroy(&di->lock);
+}
+
/* i2c specific code */
#ifdef CONFIG_BATTERY_BQ27X00_I2C
@@ -456,7 +504,7 @@ static int bq27x00_battery_remove(struct i2c_client *client)
{
struct bq27x00_device_info *di = i2c_get_clientdata(client);
- power_supply_unregister(&di->bat);
+ bq27x00_powersupply_unregister(di);
kfree(di->bat.name);
@@ -589,7 +637,8 @@ static int __devexit bq27000_battery_remove(struct platform_device *pdev)
{
struct bq27x00_device_info *di = platform_get_drvdata(pdev);
- power_supply_unregister(&di->bat);
+ bq27x00_powersupply_unregister(di);
+
platform_set_drvdata(pdev, NULL);
kfree(di);
--
1.7.2.3
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH 09/14] bq27x00: Add new properties
2011-02-12 19:38 [PATCH v2 00/14] POWER: BQ27x00: New Properties, fixes, bq27000 support Lars-Peter Clausen
` (8 preceding siblings ...)
2011-02-12 19:39 ` [PATCH 08/14] bq27x00: Poll battery state Lars-Peter Clausen
@ 2011-02-12 19:39 ` Lars-Peter Clausen
2011-02-12 19:39 ` [PATCH 10/14] bq27x00: Add MODULE_DEVICE_TABLE Lars-Peter Clausen
` (4 subsequent siblings)
14 siblings, 0 replies; 28+ messages in thread
From: Lars-Peter Clausen @ 2011-02-12 19:39 UTC (permalink / raw)
To: Anton Vorontsov
Cc: Rodolfo Giometti, Grazvydas Ignotas, linux-kernel,
Pali Rohár, Lars-Peter Clausen
From: Pali Rohár <pali.rohar@gmail.com>
This patch add support for reporting properties
POWER_SUPPLY_PROP_CHARGE_NOW, POWER_SUPPLY_PROP_CHARGE_FULL,
POWER_SUPPLY_PROP_CHARGE_FULL_DESIGN,
POWER_SUPPLY_PROP_CHARGE_COUNTER, POWER_SUPPLY_PROP_ENERGY_NOW in
module bq27x00_battery.
Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
drivers/power/bq27x00_battery.c | 152 ++++++++++++++++++++++++++++++++++++++-
1 files changed, 151 insertions(+), 1 deletions(-)
diff --git a/drivers/power/bq27x00_battery.c b/drivers/power/bq27x00_battery.c
index 061c5a5..b7a8310 100644
--- a/drivers/power/bq27x00_battery.c
+++ b/drivers/power/bq27x00_battery.c
@@ -16,6 +16,13 @@
* WARRANTIES OF MERCHANTIBILITY AND FITNESS FOR A PARTICULAR PURPOSE.
*
*/
+
+/*
+ * Datasheets:
+ * http://focus.ti.com/docs/prod/folders/print/bq27000.html
+ * http://focus.ti.com/docs/prod/folders/print/bq27500.html
+ */
+
#include <linux/module.h>
#include <linux/param.h>
#include <linux/jiffies.h>
@@ -30,7 +37,7 @@
#include <linux/power/bq27x00_battery.h>
-#define DRIVER_VERSION "1.1.0"
+#define DRIVER_VERSION "1.2.0"
#define BQ27x00_REG_TEMP 0x06
#define BQ27x00_REG_VOLT 0x08
@@ -39,11 +46,17 @@
#define BQ27x00_REG_TTE 0x16
#define BQ27x00_REG_TTF 0x18
#define BQ27x00_REG_TTECP 0x26
+#define BQ27x00_REG_NAC 0x0C /* Nominal available capaciy */
+#define BQ27x00_REG_LMD 0x12 /* Last measured discharge */
+#define BQ27x00_REG_CYCT 0x2A /* Cycle count total */
+#define BQ27x00_REG_AE 0x22 /* Available enery */
#define BQ27000_REG_RSOC 0x0B /* Relative State-of-Charge */
+#define BQ27000_REG_ILMD 0x76 /* Initial last measured discharge */
#define BQ27000_FLAG_CHGS BIT(7)
#define BQ27500_REG_SOC 0x2c
+#define BQ27500_REG_DCAP 0x3C /* Design capacity */
#define BQ27500_FLAG_DSC BIT(0)
#define BQ27500_FLAG_FC BIT(9)
@@ -61,6 +74,8 @@ struct bq27x00_reg_cache {
int time_to_empty;
int time_to_empty_avg;
int time_to_full;
+ int charge_full;
+ int charge_counter;
int capacity;
int flags;
@@ -73,6 +88,8 @@ struct bq27x00_device_info {
enum bq27x00_chip chip;
struct bq27x00_reg_cache cache;
+ int charge_design_full;
+
unsigned long last_update;
struct delayed_work work;
@@ -94,6 +111,11 @@ static enum power_supply_property bq27x00_battery_props[] = {
POWER_SUPPLY_PROP_TIME_TO_EMPTY_AVG,
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_CHARGE_COUNTER,
+ POWER_SUPPLY_PROP_ENERGY_NOW,
};
static unsigned int poll_interval = 360;
@@ -131,6 +153,87 @@ static int bq27x00_battery_read_rsoc(struct bq27x00_device_info *di)
}
/*
+ * Return a battery charge value in µAh
+ * Or < 0 if something fails.
+ */
+static int bq27x00_battery_read_charge(struct bq27x00_device_info *di, u8 reg)
+{
+ int charge;
+
+ charge = bq27x00_read(di, reg, false);
+ if (charge < 0) {
+ dev_err(di->dev, "error reading nominal available capacity\n");
+ return charge;
+ }
+
+ if (di->chip == BQ27500)
+ charge *= 1000;
+ else
+ charge = charge * 3570 / BQ27000_RS;
+
+ return charge;
+}
+
+/*
+ * Return the battery Nominal available capaciy in µAh
+ * Or < 0 if something fails.
+ */
+static inline int bq27x00_battery_read_nac(struct bq27x00_device_info *di)
+{
+ return bq27x00_battery_read_charge(di, BQ27x00_REG_NAC);
+}
+
+/*
+ * Return the battery Last measured discharge in µAh
+ * Or < 0 if something fails.
+ */
+static inline int bq27x00_battery_read_lmd(struct bq27x00_device_info *di)
+{
+ return bq27x00_battery_read_charge(di, BQ27x00_REG_LMD);
+}
+
+/*
+ * Return the battery Initial last measured discharge in µAh
+ * Or < 0 if something fails.
+ */
+static int bq27x00_battery_read_ilmd(struct bq27x00_device_info *di)
+{
+ int ilmd;
+
+ if (di->chip == BQ27500)
+ ilmd = bq27x00_read(di, BQ27500_REG_DCAP, false);
+ else
+ ilmd = bq27x00_read(di, BQ27000_REG_ILMD, true);
+
+ if (ilmd < 0) {
+ dev_err(di->dev, "error reading initial last measured discharge\n");
+ return ilmd;
+ }
+
+ if (di->chip == BQ27500)
+ ilmd *= 1000;
+ else
+ ilmd = ilmd * 256 * 3570 / BQ27000_RS;
+
+ return ilmd;
+}
+
+/*
+ * Return the battery Cycle count total
+ * Or < 0 if something fails.
+ */
+static int bq27x00_battery_read_cyct(struct bq27x00_device_info *di)
+{
+ int cyct;
+
+ cyct = bq27x00_read(di, BQ27x00_REG_CYCT, false);
+ if (cyct < 0)
+ dev_err(di->dev, "error reading cycle count total\n");
+
+ return cyct;
+}
+
+/*
* Read a time register.
* Return < 0 if something fails.
*/
@@ -162,9 +265,15 @@ static void bq27x00_update(struct bq27x00_device_info *di)
cache.time_to_empty = bq27x00_battery_read_time(di, BQ27x00_REG_TTE);
cache.time_to_empty_avg = bq27x00_battery_read_time(di, BQ27x00_REG_TTECP);
cache.time_to_full = bq27x00_battery_read_time(di, BQ27x00_REG_TTF);
+ cache.charge_full = bq27x00_battery_read_lmd(di);
+ cache.charge_counter = bq27x00_battery_read_cyct(di);
if (!is_bq27500)
cache.current_now = bq27x00_read(di, BQ27x00_REG_AI, false);
+
+ /* We only have to read charge design full once */
+ if (di->charge_design_full <= 0)
+ di->charge_design_full = bq27x00_battery_read_ilmd(di);
}
/* Ignore current_now which is a snapshot of the current battery state
@@ -285,6 +394,32 @@ static int bq27x00_battery_voltage(struct bq27x00_device_info *di,
return 0;
}
+/*
+ * Return the battery Available energy in µWh
+ * Or < 0 if something fails.
+ */
+static int bq27x00_battery_energy(struct bq27x00_device_info *di,
+ union power_supply_propval *val)
+{
+ int ae;
+
+ ae = bq27x00_read(di, BQ27x00_REG_AE, false);
+ if (ae < 0) {
+ dev_err(di->dev, "error reading available energy\n");
+ return ae;
+ }
+
+ if (di->chip == BQ27500)
+ ae *= 1000;
+ else
+ ae = ae * 29200 / BQ27000_RS;
+
+ val->intval = ae;
+
+ return 0;
+}
+
+
static int bq27x00_simple_value(int value,
union power_supply_propval *val)
{
@@ -347,6 +482,21 @@ static int bq27x00_battery_get_property(struct power_supply *psy,
case POWER_SUPPLY_PROP_TECHNOLOGY:
val->intval = POWER_SUPPLY_TECHNOLOGY_LION;
break;
+ case POWER_SUPPLY_PROP_CHARGE_NOW:
+ ret = bq27x00_simple_value(bq27x00_battery_read_nac(di), val);
+ break;
+ case POWER_SUPPLY_PROP_CHARGE_FULL:
+ ret = bq27x00_simple_value(di->cache.charge_full, val);
+ break;
+ case POWER_SUPPLY_PROP_CHARGE_FULL_DESIGN:
+ ret = bq27x00_simple_value(di->charge_design_full, val);
+ break;
+ case POWER_SUPPLY_PROP_CHARGE_COUNTER:
+ ret = bq27x00_simple_value(di->cache.charge_counter, val);
+ break;
+ case POWER_SUPPLY_PROP_ENERGY_NOW:
+ ret = bq27x00_battery_energy(di, val);
+ break;
default:
return -EINVAL;
}
--
1.7.2.3
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH 10/14] bq27x00: Add MODULE_DEVICE_TABLE
2011-02-12 19:38 [PATCH v2 00/14] POWER: BQ27x00: New Properties, fixes, bq27000 support Lars-Peter Clausen
` (9 preceding siblings ...)
2011-02-12 19:39 ` [PATCH 09/14] bq27x00: Add new properties Lars-Peter Clausen
@ 2011-02-12 19:39 ` Lars-Peter Clausen
2011-02-12 19:39 ` [PATCH 11/14] bq27x00: Give more specific reports on battery status Lars-Peter Clausen
` (3 subsequent siblings)
14 siblings, 0 replies; 28+ messages in thread
From: Lars-Peter Clausen @ 2011-02-12 19:39 UTC (permalink / raw)
To: Anton Vorontsov
Cc: Rodolfo Giometti, Grazvydas Ignotas, linux-kernel,
Pali Rohár, Lars-Peter Clausen
From: Pali Rohár <pali.rohar@gmail.com>
This patch adds MODULE_DEVICE_TABLE for module bq27x00_battery.
Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
Tested-by: Pali Rohár <pali.rohar@gmail.com>
Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
Acked-by: Rodolfo Giometti <giometti@linux.it>
---
drivers/power/bq27x00_battery.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/drivers/power/bq27x00_battery.c b/drivers/power/bq27x00_battery.c
index b7a8310..d2e6423 100644
--- a/drivers/power/bq27x00_battery.c
+++ b/drivers/power/bq27x00_battery.c
@@ -672,6 +672,7 @@ static const struct i2c_device_id bq27x00_id[] = {
{ "bq27500", BQ27500 },
{},
};
+MODULE_DEVICE_TABLE(i2c, bq27x00_id);
static struct i2c_driver bq27x00_battery_driver = {
.driver = {
--
1.7.2.3
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH 11/14] bq27x00: Give more specific reports on battery status
2011-02-12 19:38 [PATCH v2 00/14] POWER: BQ27x00: New Properties, fixes, bq27000 support Lars-Peter Clausen
` (10 preceding siblings ...)
2011-02-12 19:39 ` [PATCH 10/14] bq27x00: Add MODULE_DEVICE_TABLE Lars-Peter Clausen
@ 2011-02-12 19:39 ` Lars-Peter Clausen
2011-02-12 19:39 ` [PATCH 12/14] bq27x00: Minor cleanups Lars-Peter Clausen
` (2 subsequent siblings)
14 siblings, 0 replies; 28+ messages in thread
From: Lars-Peter Clausen @ 2011-02-12 19:39 UTC (permalink / raw)
To: Anton Vorontsov
Cc: Rodolfo Giometti, Grazvydas Ignotas, linux-kernel, Lars-Peter Clausen
The current code only reports whether the battery is charging or
discharging. But the battery also reports whether it is fully charged,
furthermore by look at if the battery is supplied we can tell whether it
is discharging or not charging.
Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
Acked-by: Rodolfo Giometti <giometti@linux.it>
---
drivers/power/bq27x00_battery.c | 7 ++++++-
1 files changed, 6 insertions(+), 1 deletions(-)
diff --git a/drivers/power/bq27x00_battery.c b/drivers/power/bq27x00_battery.c
index d2e6423..e371b01 100644
--- a/drivers/power/bq27x00_battery.c
+++ b/drivers/power/bq27x00_battery.c
@@ -54,6 +54,7 @@
#define BQ27000_REG_RSOC 0x0B /* Relative State-of-Charge */
#define BQ27000_REG_ILMD 0x76 /* Initial last measured discharge */
#define BQ27000_FLAG_CHGS BIT(7)
+#define BQ27000_FLAG_FC BIT(5)
#define BQ27500_REG_SOC 0x2c
#define BQ27500_REG_DCAP 0x3C /* Design capacity */
@@ -365,8 +366,12 @@ static int bq27x00_battery_status(struct bq27x00_device_info *di,
else
status = POWER_SUPPLY_STATUS_CHARGING;
} else {
- if (di->cache.flags & BQ27000_FLAG_CHGS)
+ if (di->cache.flags & BQ27000_FLAG_FC)
+ status = POWER_SUPPLY_STATUS_FULL;
+ else if (di->cache.flags & BQ27000_FLAG_CHGS)
status = POWER_SUPPLY_STATUS_CHARGING;
+ else if (power_supply_am_i_supplied(&di->bat))
+ status = POWER_SUPPLY_STATUS_NOT_CHARGING;
else
status = POWER_SUPPLY_STATUS_DISCHARGING;
}
--
1.7.2.3
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH 12/14] bq27x00: Minor cleanups
2011-02-12 19:38 [PATCH v2 00/14] POWER: BQ27x00: New Properties, fixes, bq27000 support Lars-Peter Clausen
` (11 preceding siblings ...)
2011-02-12 19:39 ` [PATCH 11/14] bq27x00: Give more specific reports on battery status Lars-Peter Clausen
@ 2011-02-12 19:39 ` Lars-Peter Clausen
2011-02-12 19:39 ` [PATCH 13/14] bq27x00: Cleanup bq27x00_i2c_read Lars-Peter Clausen
2011-02-12 19:39 ` [PATCH 14/14] Ignore -ENODATA errors when generating a uevent Lars-Peter Clausen
14 siblings, 0 replies; 28+ messages in thread
From: Lars-Peter Clausen @ 2011-02-12 19:39 UTC (permalink / raw)
To: Anton Vorontsov
Cc: Rodolfo Giometti, Grazvydas Ignotas, linux-kernel,
Pali Rohár, Lars-Peter Clausen
From: Pali Rohár <pali.rohar@gmail.com>
* Consistently use uppercase for hexadecimal values.
* Clarify/fix the unit of functions return value in its comment.
Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
drivers/power/bq27x00_battery.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/power/bq27x00_battery.c b/drivers/power/bq27x00_battery.c
index e371b01..a64cc21 100644
--- a/drivers/power/bq27x00_battery.c
+++ b/drivers/power/bq27x00_battery.c
@@ -56,7 +56,7 @@
#define BQ27000_FLAG_CHGS BIT(7)
#define BQ27000_FLAG_FC BIT(5)
-#define BQ27500_REG_SOC 0x2c
+#define BQ27500_REG_SOC 0x2C
#define BQ27500_REG_DCAP 0x3C /* Design capacity */
#define BQ27500_FLAG_DSC BIT(0)
#define BQ27500_FLAG_FC BIT(9)
@@ -321,7 +321,7 @@ static int bq27x00_battery_temperature(struct bq27x00_device_info *di,
}
/*
- * Return the battery average current
+ * Return the battery average current in µA
* Note that current can be negative signed as well
* Or 0 if something fails.
*/
--
1.7.2.3
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH 13/14] bq27x00: Cleanup bq27x00_i2c_read
2011-02-12 19:38 [PATCH v2 00/14] POWER: BQ27x00: New Properties, fixes, bq27000 support Lars-Peter Clausen
` (12 preceding siblings ...)
2011-02-12 19:39 ` [PATCH 12/14] bq27x00: Minor cleanups Lars-Peter Clausen
@ 2011-02-12 19:39 ` Lars-Peter Clausen
2011-02-12 19:39 ` [PATCH 14/14] Ignore -ENODATA errors when generating a uevent Lars-Peter Clausen
14 siblings, 0 replies; 28+ messages in thread
From: Lars-Peter Clausen @ 2011-02-12 19:39 UTC (permalink / raw)
To: Anton Vorontsov
Cc: Rodolfo Giometti, Grazvydas Ignotas, linux-kernel, Lars-Peter Clausen
Some minor stylistic cleanups.
Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
drivers/power/bq27x00_battery.c | 45 ++++++++++++++++++++------------------
1 files changed, 24 insertions(+), 21 deletions(-)
diff --git a/drivers/power/bq27x00_battery.c b/drivers/power/bq27x00_battery.c
index a64cc21..2dee851 100644
--- a/drivers/power/bq27x00_battery.c
+++ b/drivers/power/bq27x00_battery.c
@@ -565,36 +565,39 @@ static DEFINE_MUTEX(battery_mutex);
static int bq27x00_read_i2c(struct bq27x00_device_info *di, u8 reg, bool single)
{
struct i2c_client *client = to_i2c_client(di->dev);
- struct i2c_msg msg[1];
+ struct i2c_msg msg;
unsigned char data[2];
int ret;
if (!client->adapter)
return -ENODEV;
- msg->addr = client->addr;
- msg->flags = 0;
- msg->len = 1;
- msg->buf = data;
+ msg.addr = client->addr;
+ msg.flags = 0;
+ msg.len = 1;
+ msg.buf = data;
data[0] = reg;
- ret = i2c_transfer(client->adapter, msg, 1);
+ ret = i2c_transfer(client->adapter, &msg, 1);
+
+ if (ret < 0)
+ return ret;
+
+ if (single)
+ msg.len = 1;
+ else
+ msg.len = 2;
+
+ msg.flags = I2C_M_RD;
+ ret = i2c_transfer(client->adapter, &msg, 1);
+ if (ret < 0)
+ return ret;
+
+ if (!single)
+ ret = get_unaligned_le16(data);
+ else
+ ret = data[0];
- if (ret >= 0) {
- if (!single)
- msg->len = 2;
- else
- msg->len = 1;
-
- msg->flags = I2C_M_RD;
- ret = i2c_transfer(client->adapter, msg, 1);
- if (ret >= 0) {
- if (!single)
- ret = get_unaligned_le16(data);
- else
- ret = data[0];
- }
- }
return ret;
}
--
1.7.2.3
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH 14/14] Ignore -ENODATA errors when generating a uevent
2011-02-12 19:38 [PATCH v2 00/14] POWER: BQ27x00: New Properties, fixes, bq27000 support Lars-Peter Clausen
` (13 preceding siblings ...)
2011-02-12 19:39 ` [PATCH 13/14] bq27x00: Cleanup bq27x00_i2c_read Lars-Peter Clausen
@ 2011-02-12 19:39 ` Lars-Peter Clausen
14 siblings, 0 replies; 28+ messages in thread
From: Lars-Peter Clausen @ 2011-02-12 19:39 UTC (permalink / raw)
To: Anton Vorontsov
Cc: Rodolfo Giometti, Grazvydas Ignotas, linux-kernel, Lars-Peter Clausen
Sometimes a driver can not report a meaningful value for a certain property
and returns -ENODATA.
Currently when generating a uevent and a property return -ENODATA it is
treated as an error an no uevent is generated at all. This is not an
desirable behavior.
This patch adds a special case for -ENODATA and ignores properties which
return this error code when generating the uevent.
Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
drivers/power/power_supply_sysfs.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/power/power_supply_sysfs.c b/drivers/power/power_supply_sysfs.c
index cd1f907..605514a 100644
--- a/drivers/power/power_supply_sysfs.c
+++ b/drivers/power/power_supply_sysfs.c
@@ -270,7 +270,7 @@ int power_supply_uevent(struct device *dev, struct kobj_uevent_env *env)
attr = &power_supply_attrs[psy->properties[j]];
ret = power_supply_show_property(dev, attr, prop_buf);
- if (ret == -ENODEV) {
+ if (ret == -ENODEV || ret == -ENODATA) {
/* When a battery is absent, we expect -ENODEV. Don't abort;
send the uevent with at least the the PRESENT=0 property */
ret = 0;
--
1.7.2.3
^ permalink raw reply related [flat|nested] 28+ messages in thread
* Re: [PATCH v2 07/14] bq27X00: Cache battery registers
2011-02-12 19:39 ` [PATCH v2 07/14] bq27X00: " Lars-Peter Clausen
@ 2011-02-12 19:52 ` Lars-Peter Clausen
0 siblings, 0 replies; 28+ messages in thread
From: Lars-Peter Clausen @ 2011-02-12 19:52 UTC (permalink / raw)
To: Anton Vorontsov; +Cc: Rodolfo Giometti, Grazvydas Ignotas, linux-kernel
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Sorry, this patch was included twice in the series, they are identical except
for the uppercase X in the subject, you should ignore this one.
On 02/12/2011 08:39 PM, Lars-Peter Clausen wrote:
> This patch adds a register cache to the bq27x00 battery driver.
> Usually multiple, if not all, power_supply properties are queried at once,
> for example when an uevent is generated. Since some registers are used by
> multiple properties caching the registers should reduce the number of
> reads.
>
> The cache is valid for 5 seconds this roughly matches the internal update
> interval of the current register for the bq27000/bq27200.
>
> Fast changing properties(*_NOW) which can be obtained by reading a single
> register are not cached.
>
> It will also be used in the follow up patch to check if the battery status
> has been changed since the last update to emit power_supply_changed events.
>
> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
> ---
> drivers/power/bq27x00_battery.c | 272 +++++++++++++++++++++-----------------
> 1 files changed, 150 insertions(+), 122 deletions(-)
>
> diff --git a/drivers/power/bq27x00_battery.c b/drivers/power/bq27x00_battery.c
> index ae4677f..0c94693 100644
> --- a/drivers/power/bq27x00_battery.c
> +++ b/drivers/power/bq27x00_battery.c
> @@ -19,7 +19,6 @@
> #include <linux/module.h>
> #include <linux/param.h>
> #include <linux/jiffies.h>
> -#include <linux/workqueue.h>
> #include <linux/delay.h>
> #include <linux/platform_device.h>
> #include <linux/power_supply.h>
> @@ -51,17 +50,30 @@
>
> struct bq27x00_device_info;
> struct bq27x00_access_methods {
> - int (*read)(struct bq27x00_device_info *, u8 reg, int *rt_value,
> - bool single);
> + int (*read)(struct bq27x00_device_info *di, u8 reg, bool single);
> };
>
> enum bq27x00_chip { BQ27000, BQ27500 };
>
> +struct bq27x00_reg_cache {
> + int temperature;
> + int time_to_empty;
> + int time_to_empty_avg;
> + int time_to_full;
> + int capacity;
> + int flags;
> +
> + int current_now;
> +};
> +
> struct bq27x00_device_info {
> struct device *dev;
> int id;
> enum bq27x00_chip chip;
>
> + struct bq27x00_reg_cache cache;
> + unsigned long last_update;
> +
> struct power_supply bat;
>
> struct bq27x00_access_methods bus;
> @@ -85,48 +97,93 @@ static enum power_supply_property bq27x00_battery_props[] = {
> */
>
> static inline int bq27x00_read(struct bq27x00_device_info *di, u8 reg,
> - int *rt_value, bool single)
> + bool single)
> {
> - return di->bus.read(di, reg, rt_value, single);
> + return di->bus.read(di, reg, single);
> }
>
> /*
> - * Return the battery temperature in tenths of degree Celsius
> + * Return the battery Relative State-of-Charge
> * Or < 0 if something fails.
> */
> -static int bq27x00_battery_temperature(struct bq27x00_device_info *di)
> +static int bq27x00_battery_read_rsoc(struct bq27x00_device_info *di)
> {
> - int ret;
> - int temp = 0;
> -
> - ret = bq27x00_read(di, BQ27x00_REG_TEMP, &temp, false);
> - if (ret) {
> - dev_err(di->dev, "error reading temperature\n");
> - return ret;
> - }
> + int rsoc;
>
> if (di->chip == BQ27500)
> - return temp - 2731;
> + rsoc = bq27x00_read(di, BQ27500_REG_SOC, false);
> else
> - return ((temp * 5) - 5463) / 2;
> + rsoc = bq27x00_read(di, BQ27000_REG_RSOC, true);
> +
> + if (rsoc < 0)
> + dev_err(di->dev, "error reading relative State-of-Charge\n");
> +
> + return rsoc;
> }
>
> /*
> - * Return the battery Voltage in milivolts
> - * Or < 0 if something fails.
> + * Read a time register.
> + * Return < 0 if something fails.
> */
> -static int bq27x00_battery_voltage(struct bq27x00_device_info *di)
> +static int bq27x00_battery_read_time(struct bq27x00_device_info *di, u8 reg)
> {
> - int ret;
> - int volt = 0;
> + int tval;
>
> - ret = bq27x00_read(di, BQ27x00_REG_VOLT, &volt, false);
> - if (ret) {
> - dev_err(di->dev, "error reading voltage\n");
> - return ret;
> + tval = bq27x00_read(di, reg, false);
> + if (tval < 0) {
> + dev_err(di->dev, "error reading register %02x: %d\n", reg, tval);
> + return tval;
> + }
> +
> + if (tval == 65535)
> + return -ENODATA;
> +
> + return tval * 60;
> +}
> +
> +static void bq27x00_update(struct bq27x00_device_info *di)
> +{
> + struct bq27x00_reg_cache cache = {0, };
> + bool is_bq27500 = di->chip == BQ27500;
> +
> + cache.flags = bq27x00_read(di, BQ27x00_REG_FLAGS, is_bq27500);
> + if (cache.flags >= 0) {
> + cache.capacity = bq27x00_battery_read_rsoc(di);
> + cache.temperature = bq27x00_read(di, BQ27x00_REG_TEMP, false);
> + cache.time_to_empty = bq27x00_battery_read_time(di, BQ27x00_REG_TTE);
> + cache.time_to_empty_avg = bq27x00_battery_read_time(di, BQ27x00_REG_TTECP);
> + cache.time_to_full = bq27x00_battery_read_time(di, BQ27x00_REG_TTF);
> +
> + if (!is_bq27500)
> + cache.current_now = bq27x00_read(di, BQ27x00_REG_AI, false);
> }
>
> - return volt * 1000;
> + /* Ignore current_now which is a snapshot of the current battery state
> + * and is likely to be different even between two consecutive reads */
> + if (memcmp(&di->cache, &cache, sizeof(cache) - sizeof(int)) != 0) {
> + di->cache = cache;
> + power_supply_changed(&di->bat);
> + }
> +
> + di->last_update = jiffies;
> +}
> +
> +/*
> + * Return the battery temperature in tenths of degree Celsius
> + * Or < 0 if something fails.
> + */
> +static int bq27x00_battery_temperature(struct bq27x00_device_info *di,
> + union power_supply_propval *val)
> +{
> + if (di->cache.temperature < 0)
> + return di->cache.temperature;
> +
> + if (di->chip == BQ27500)
> + val->intval = di->cache.temperature - 2731;
> + else
> + val->intval = ((di->cache.temperature * 5) - 5463) / 2;
> +
> + return 0;
> }
>
> /*
> @@ -134,109 +191,84 @@ static int bq27x00_battery_voltage(struct bq27x00_device_info *di)
> * Note that current can be negative signed as well
> * Or 0 if something fails.
> */
> -static int bq27x00_battery_current(struct bq27x00_device_info *di)
> +static int bq27x00_battery_current(struct bq27x00_device_info *di,
> + union power_supply_propval *val)
> {
> - int ret;
> - int curr = 0;
> - int flags = 0;
> + int curr;
>
> - ret = bq27x00_read(di, BQ27x00_REG_AI, &curr, false);
> - if (ret) {
> - dev_err(di->dev, "error reading current\n");
> - return 0;
> - }
> + if (di->chip == BQ27000)
> + curr = bq27x00_read(di, BQ27x00_REG_AI, false);
> + else
> + curr = di->cache.current_now;
> +
> + if (curr < 0)
> + return curr;
>
> if (di->chip == BQ27500) {
> /* bq27500 returns signed value */
> - curr = (int)((s16)curr) * 1000;
> + val->intval = (int)((s16)curr) * 1000;
> } else {
> - ret = bq27x00_read(di, BQ27x00_REG_FLAGS, &flags, false);
> - if (ret < 0) {
> - dev_err(di->dev, "error reading flags\n");
> - return 0;
> - }
> - if (flags & BQ27000_FLAG_CHGS) {
> + if (di->cache.flags & BQ27000_FLAG_CHGS) {
> dev_dbg(di->dev, "negative current!\n");
> curr = -curr;
> }
> - curr = curr * 3570 / BQ27000_RS;
> - }
> -
> - return curr;
> -}
> -
> -/*
> - * Return the battery Relative State-of-Charge
> - * Or < 0 if something fails.
> - */
> -static int bq27x00_battery_rsoc(struct bq27x00_device_info *di)
> -{
> - int ret;
> - int rsoc = 0;
>
> - if (di->chip == BQ27500)
> - ret = bq27x00_read(di, BQ27500_REG_SOC, &rsoc, false);
> - else
> - ret = bq27x00_read(di, BQ27000_REG_RSOC, &rsoc, true);
> - if (ret) {
> - dev_err(di->dev, "error reading relative State-of-Charge\n");
> - return ret;
> + val->intval = curr * 3570 / BQ27000_RS;
> }
>
> - return rsoc;
> + return 0;
> }
>
> static int bq27x00_battery_status(struct bq27x00_device_info *di,
> - union power_supply_propval *val)
> + union power_supply_propval *val)
> {
> - int flags = 0;
> int status;
> - int ret;
> -
> - ret = bq27x00_read(di, BQ27x00_REG_FLAGS, &flags, false);
> - if (ret < 0) {
> - dev_err(di->dev, "error reading flags\n");
> - return ret;
> - }
>
> if (di->chip == BQ27500) {
> - if (flags & BQ27500_FLAG_FC)
> + if (di->cache.flags & BQ27500_FLAG_FC)
> status = POWER_SUPPLY_STATUS_FULL;
> - else if (flags & BQ27500_FLAG_DSC)
> + else if (di->cache.flags & BQ27500_FLAG_DSC)
> status = POWER_SUPPLY_STATUS_DISCHARGING;
> else
> status = POWER_SUPPLY_STATUS_CHARGING;
> } else {
> - if (flags & BQ27000_FLAG_CHGS)
> + if (di->cache.flags & BQ27000_FLAG_CHGS)
> status = POWER_SUPPLY_STATUS_CHARGING;
> else
> status = POWER_SUPPLY_STATUS_DISCHARGING;
> }
>
> val->intval = status;
> +
> return 0;
> }
>
> /*
> - * Read a time register.
> - * Return < 0 if something fails.
> + * Return the battery Voltage in milivolts
> + * Or < 0 if something fails.
> */
> -static int bq27x00_battery_time(struct bq27x00_device_info *di, int reg,
> - union power_supply_propval *val)
> +static int bq27x00_battery_voltage(struct bq27x00_device_info *di,
> + union power_supply_propval *val)
> {
> - int tval = 0;
> - int ret;
> + int volt;
>
> - ret = bq27x00_read(reg, &tval, false);
> - if (ret) {
> - dev_err(di->dev, "error reading register %02x\n", reg);
> - return ret;
> - }
> + volt = bq27x00_read(di, BQ27x00_REG_VOLT, false);
> + if (volt < 0)
> + return volt;
>
> - if (tval == 65535)
> - return -ENODATA;
> + val->intval = volt * 1000;
> +
> + return 0;
> +}
> +
> +static int bq27x00_simple_value(int value,
> + union power_supply_propval *val)
> +{
> + if (value < 0)
> + return value;
> +
> + val->intval = value;
>
> - val->intval = tval * 60;
> return 0;
> }
>
> @@ -249,9 +281,11 @@ static int bq27x00_battery_get_property(struct power_supply *psy,
> {
> int ret = 0;
> struct bq27x00_device_info *di = to_bq27x00_device_info(psy);
> - int voltage = bq27x00_battery_voltage(di);
>
> - if (psp != POWER_SUPPLY_PROP_PRESENT && voltage <= 0)
> + if (time_is_before_jiffies(di->last_update + 5 * HZ))
> + bq27x00_update(di);
> +
> + if (psp != POWER_SUPPLY_PROP_PRESENT && di->cache.flags < 0)
> return -ENODEV;
>
> switch (psp) {
> @@ -259,29 +293,28 @@ static int bq27x00_battery_get_property(struct power_supply *psy,
> ret = bq27x00_battery_status(di, val);
> break;
> case POWER_SUPPLY_PROP_VOLTAGE_NOW:
> - val->intval = voltage;
> + ret = bq27x00_battery_voltage(di, val);
> break;
> case POWER_SUPPLY_PROP_PRESENT:
> - if (psp == POWER_SUPPLY_PROP_PRESENT)
> - val->intval = voltage <= 0 ? 0 : 1;
> + val->intval = di->cache.flags < 0 ? 0 : 1;
> break;
> case POWER_SUPPLY_PROP_CURRENT_NOW:
> - val->intval = bq27x00_battery_current(di);
> + ret = bq27x00_battery_current(di, val);
> break;
> case POWER_SUPPLY_PROP_CAPACITY:
> - val->intval = bq27x00_battery_rsoc(di);
> + ret = bq27x00_simple_value(di->cache.capacity, val);
> break;
> case POWER_SUPPLY_PROP_TEMP:
> - val->intval = bq27x00_battery_temperature(di);
> + ret = bq27x00_battery_temperature(di, val);
> break;
> case POWER_SUPPLY_PROP_TIME_TO_EMPTY_NOW:
> - ret = bq27x00_battery_time(di, BQ27x00_REG_TTE, val);
> + ret = bq27x00_simple_value(di->cache.time_to_empty, val);
> break;
> case POWER_SUPPLY_PROP_TIME_TO_EMPTY_AVG:
> - ret = bq27x00_battery_time(di, BQ27x00_REG_TTECP, val);
> + ret = bq27x00_simple_value(di->cache.time_to_empty_avg, val);
> break;
> case POWER_SUPPLY_PROP_TIME_TO_FULL_NOW:
> - ret = bq27x00_battery_time(di, BQ27x00_REG_TTF, val);
> + ret = bq27x00_simple_value(di->cache.time_to_full, val);
> break;
> case POWER_SUPPLY_PROP_TECHNOLOGY:
> val->intval = POWER_SUPPLY_TECHNOLOGY_LION;
> @@ -311,6 +344,8 @@ static int bq27x00_powersupply_init(struct bq27x00_device_info *di)
>
> dev_info(di->dev, "support ver. %s enabled\n", DRIVER_VERSION);
>
> + bq27x00_update(di);
> +
> return 0;
> }
>
> @@ -324,13 +359,12 @@ static int bq27x00_powersupply_init(struct bq27x00_device_info *di)
> static DEFINE_IDR(battery_id);
> static DEFINE_MUTEX(battery_mutex);
>
> -static int bq27x00_read_i2c(struct bq27x00_device_info *di, u8 reg,
> - int *rt_value, bool single)
> +static int bq27x00_read_i2c(struct bq27x00_device_info *di, u8 reg, bool single)
> {
> struct i2c_client *client = to_i2c_client(di->dev);
> struct i2c_msg msg[1];
> unsigned char data[2];
> - int err;
> + int ret;
>
> if (!client->adapter)
> return -ENODEV;
> @@ -341,26 +375,24 @@ static int bq27x00_read_i2c(struct bq27x00_device_info *di, u8 reg,
> msg->buf = data;
>
> data[0] = reg;
> - err = i2c_transfer(client->adapter, msg, 1);
> + ret = i2c_transfer(client->adapter, msg, 1);
>
> - if (err >= 0) {
> + if (ret >= 0) {
> if (!single)
> msg->len = 2;
> else
> msg->len = 1;
>
> msg->flags = I2C_M_RD;
> - err = i2c_transfer(client->adapter, msg, 1);
> - if (err >= 0) {
> + ret = i2c_transfer(client->adapter, msg, 1);
> + if (ret >= 0) {
> if (!single)
> - *rt_value = get_unaligned_le16(data);
> + ret = get_unaligned_le16(data);
> else
> - *rt_value = data[0];
> -
> - return 0;
> + ret = data[0];
> }
> }
> - return err;
> + return ret;
> }
>
> static int bq27x00_battery_probe(struct i2c_client *client,
> @@ -477,7 +509,7 @@ static inline void bq27x00_battery_i2c_exit(void) {};
> #ifdef CONFIG_BATTERY_BQ27X00_PLATFORM
>
> static int bq27000_read_platform(struct bq27x00_device_info *di, u8 reg,
> - int *rt_value, bool single)
> + bool single)
> {
> struct device *dev = di->dev;
> struct bq27000_platform_data *pdata = dev->platform_data;
> @@ -504,14 +536,10 @@ static int bq27000_read_platform(struct bq27x00_device_info *di, u8 reg,
> if (timeout == 0)
> return -EIO;
>
> - *rt_value = (upper << 8) | lower;
> - } else {
> - lower = pdata->read(dev, reg);
> - if (lower < 0)
> - return lower;
> - *rt_value = lower;
> + return (upper << 8) | lower;
> }
> - return 0;
> +
> + return pdata->read(dev, reg);
> }
>
> static int __devinit bq27000_battery_probe(struct platform_device *pdev)
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/
iEYEARECAAYFAk1W5QsACgkQBX4mSR26RiNJawCfVrOAsrxv1WQCmmIuOFN6Sk6h
L8cAn1KkrujtSGeiLya34WEWU75CYb4N
=A7NT
-----END PGP SIGNATURE-----
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 07/14] bq27x00: Cache battery registers
2011-02-12 19:39 ` [PATCH 07/14] bq27x00: Cache battery registers Lars-Peter Clausen
@ 2011-02-13 15:14 ` Grazvydas Ignotas
2011-02-13 18:56 ` Lars-Peter Clausen
2011-02-14 3:01 ` [PATCH 07/14 v3] " Lars-Peter Clausen
1 sibling, 1 reply; 28+ messages in thread
From: Grazvydas Ignotas @ 2011-02-13 15:14 UTC (permalink / raw)
To: Lars-Peter Clausen; +Cc: Anton Vorontsov, Rodolfo Giometti, linux-kernel
On Sat, Feb 12, 2011 at 9:39 PM, Lars-Peter Clausen <lars@metafoo.de> wrote:
> This patch adds a register cache to the bq27x00 battery driver.
> Usually multiple, if not all, power_supply properties are queried at once,
> for example when an uevent is generated. Since some registers are used by
> multiple properties caching the registers should reduce the number of
> reads.
>
> The cache is valid for 5 seconds this roughly matches the internal update
> interval of the current register for the bq27000/bq27200.
>
> Fast changing properties(*_NOW) which can be obtained by reading a single
> register are not cached.
>
> It will also be used in the follow up patch to check if the battery status
> has been changed since the last update to emit power_supply_changed events.
>
> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
> ---
> drivers/power/bq27x00_battery.c | 272 +++++++++++++++++++++-----------------
> 1 files changed, 150 insertions(+), 122 deletions(-)
>
> diff --git a/drivers/power/bq27x00_battery.c b/drivers/power/bq27x00_battery.c
> index 44bc76b..0c94693 100644
> --- a/drivers/power/bq27x00_battery.c
> +++ b/drivers/power/bq27x00_battery.c
> @@ -19,7 +19,6 @@
<snip>
> +static void bq27x00_update(struct bq27x00_device_info *di)
> +{
> + struct bq27x00_reg_cache cache = {0, };
> + bool is_bq27500 = di->chip == BQ27500;
> +
> + cache.flags = bq27x00_read(di, BQ27x00_REG_FLAGS, is_bq27500);
> + if (cache.flags >= 0) {
> + cache.capacity = bq27x00_battery_read_rsoc(di);
> + cache.temperature = bq27x00_read(di, BQ27x00_REG_TEMP, false);
> + cache.time_to_empty = bq27x00_battery_read_time(di, BQ27x00_REG_TTE);
> + cache.time_to_empty_avg = bq27x00_battery_read_time(di, BQ27x00_REG_TTECP);
> + cache.time_to_full = bq27x00_battery_read_time(di, BQ27x00_REG_TTF);
> +
> + if (!is_bq27500)
> + cache.current_now = bq27x00_read(di, BQ27x00_REG_AI, false);
> }
>
<snip>
> -static int bq27x00_battery_current(struct bq27x00_device_info *di)
> +static int bq27x00_battery_current(struct bq27x00_device_info *di,
> + union power_supply_propval *val)
> {
> - int ret;
> - int curr = 0;
> - int flags = 0;
> + int curr;
>
> - ret = bq27x00_read(di, BQ27x00_REG_AI, &curr, false);
> - if (ret) {
> - dev_err(di->dev, "error reading current\n");
> - return 0;
> - }
> + if (di->chip == BQ27000)
> + curr = bq27x00_read(di, BQ27x00_REG_AI, false);
> + else
> + curr = di->cache.current_now;
You're updating current_now in cache if it's not bq27500, but
bypassing cache for bq27000?
Why do you still want to cache the current while you are no longer
caching voltage, because it needs 2 register reads I guess?
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 07/14] bq27x00: Cache battery registers
2011-02-13 15:14 ` Grazvydas Ignotas
@ 2011-02-13 18:56 ` Lars-Peter Clausen
0 siblings, 0 replies; 28+ messages in thread
From: Lars-Peter Clausen @ 2011-02-13 18:56 UTC (permalink / raw)
To: Grazvydas Ignotas; +Cc: Anton Vorontsov, Rodolfo Giometti, linux-kernel
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
On 02/13/2011 04:14 PM, Grazvydas Ignotas wrote:
> On Sat, Feb 12, 2011 at 9:39 PM, Lars-Peter Clausen <lars@metafoo.de> wrote:
>> This patch adds a register cache to the bq27x00 battery driver.
>> Usually multiple, if not all, power_supply properties are queried at once,
>> for example when an uevent is generated. Since some registers are used by
>> multiple properties caching the registers should reduce the number of
>> reads.
>>
>> The cache is valid for 5 seconds this roughly matches the internal update
>> interval of the current register for the bq27000/bq27200.
>>
>> Fast changing properties(*_NOW) which can be obtained by reading a single
>> register are not cached.
>>
>> It will also be used in the follow up patch to check if the battery status
>> has been changed since the last update to emit power_supply_changed events.
>>
>> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
>> ---
>> drivers/power/bq27x00_battery.c | 272 +++++++++++++++++++++-----------------
>> 1 files changed, 150 insertions(+), 122 deletions(-)
>>
>> diff --git a/drivers/power/bq27x00_battery.c b/drivers/power/bq27x00_battery.c
>> index 44bc76b..0c94693 100644
>> --- a/drivers/power/bq27x00_battery.c
>> +++ b/drivers/power/bq27x00_battery.c
>> @@ -19,7 +19,6 @@
>
> <snip>
>
>> +static void bq27x00_update(struct bq27x00_device_info *di)
>> +{
>> + struct bq27x00_reg_cache cache = {0, };
>> + bool is_bq27500 = di->chip == BQ27500;
>> +
>> + cache.flags = bq27x00_read(di, BQ27x00_REG_FLAGS, is_bq27500);
>> + if (cache.flags >= 0) {
>> + cache.capacity = bq27x00_battery_read_rsoc(di);
>> + cache.temperature = bq27x00_read(di, BQ27x00_REG_TEMP, false);
>> + cache.time_to_empty = bq27x00_battery_read_time(di, BQ27x00_REG_TTE);
>> + cache.time_to_empty_avg = bq27x00_battery_read_time(di, BQ27x00_REG_TTECP);
>> + cache.time_to_full = bq27x00_battery_read_time(di, BQ27x00_REG_TTF);
>> +
>> + if (!is_bq27500)
>> + cache.current_now = bq27x00_read(di, BQ27x00_REG_AI, false);
>> }
>>
>
> <snip>
>
>> -static int bq27x00_battery_current(struct bq27x00_device_info *di)
>> +static int bq27x00_battery_current(struct bq27x00_device_info *di,
>> + union power_supply_propval *val)
>> {
>> - int ret;
>> - int curr = 0;
>> - int flags = 0;
>> + int curr;
>>
>> - ret = bq27x00_read(di, BQ27x00_REG_AI, &curr, false);
>> - if (ret) {
>> - dev_err(di->dev, "error reading current\n");
>> - return 0;
>> - }
>> + if (di->chip == BQ27000)
>> + curr = bq27x00_read(di, BQ27x00_REG_AI, false);
>> + else
>> + curr = di->cache.current_now;
>
> You're updating current_now in cache if it's not bq27500, but
> bypassing cache for bq27000?
Right, that should of course be ' == BQ27500'
>
> Why do you still want to cache the current while you are no longer
> caching voltage, because it needs 2 register reads I guess?
Yes. I've given it some though and I would rather avoid the chance of returning
bogus values, because the AI register is already updated, but flags still
contains the old state.
- - Lars
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/
iEYEARECAAYFAk1YKVgACgkQBX4mSR26RiNi6QCfRofHUac7YILVEA1LI9jZFodA
thcAnR6mTEN4/Mo+PUUuuqpa7msdRwyo
=YAr6
-----END PGP SIGNATURE-----
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH 07/14 v3] bq27x00: Cache battery registers
2011-02-12 19:39 ` [PATCH 07/14] bq27x00: Cache battery registers Lars-Peter Clausen
2011-02-13 15:14 ` Grazvydas Ignotas
@ 2011-02-14 3:01 ` Lars-Peter Clausen
2011-02-14 21:58 ` Grazvydas Ignotas
1 sibling, 1 reply; 28+ messages in thread
From: Lars-Peter Clausen @ 2011-02-14 3:01 UTC (permalink / raw)
To: Anton Vorontsov
Cc: Pali Rohar, Rodolfo Giometti, Grazvydas Ignotas, linux-kernel,
Lars-Peter Clausen
This patch adds a register cache to the bq27x00 battery driver.
Usually multiple, if not all, power_supply properties are queried at once,
for example when an uevent is generated. Since some registers are used by
multiple properties caching the registers should reduce the number of
reads.
The cache is valid for 5 seconds this roughly matches the internal update
interval of the current register for the bq27000/bq27200.
Fast changing properties(*_NOW) which can be obtained by reading a single
register are not cached.
It will also be used in the follow up patch to check if the battery status
has been changed since the last update to emit power_supply_changed events.
Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
drivers/power/bq27x00_battery.c | 271 +++++++++++++++++++++-----------------
1 files changed, 150 insertions(+), 121 deletions(-)
diff --git a/drivers/power/bq27x00_battery.c b/drivers/power/bq27x00_battery.c
index 44bc76b..dbe3fcb 100644
--- a/drivers/power/bq27x00_battery.c
+++ b/drivers/power/bq27x00_battery.c
@@ -51,17 +51,30 @@
struct bq27x00_device_info;
struct bq27x00_access_methods {
- int (*read)(struct bq27x00_device_info *, u8 reg, int *rt_value,
- bool single);
+ int (*read)(struct bq27x00_device_info *di, u8 reg, bool single);
};
enum bq27x00_chip { BQ27000, BQ27500 };
+struct bq27x00_reg_cache {
+ int temperature;
+ int time_to_empty;
+ int time_to_empty_avg;
+ int time_to_full;
+ int capacity;
+ int flags;
+
+ int current_now;
+};
+
struct bq27x00_device_info {
struct device *dev;
int id;
enum bq27x00_chip chip;
+ struct bq27x00_reg_cache cache;
+ unsigned long last_update;
+
struct power_supply bat;
struct bq27x00_access_methods bus;
@@ -85,48 +98,93 @@ static enum power_supply_property bq27x00_battery_props[] = {
*/
static inline int bq27x00_read(struct bq27x00_device_info *di, u8 reg,
- int *rt_value, bool single)
+ bool single)
{
- return di->bus.read(di, reg, rt_value, single);
+ return di->bus.read(di, reg, single);
}
/*
- * Return the battery temperature in tenths of degree Celsius
+ * Return the battery Relative State-of-Charge
* Or < 0 if something fails.
*/
-static int bq27x00_battery_temperature(struct bq27x00_device_info *di)
+static int bq27x00_battery_read_rsoc(struct bq27x00_device_info *di)
{
- int ret;
- int temp = 0;
-
- ret = bq27x00_read(di, BQ27x00_REG_TEMP, &temp, false);
- if (ret) {
- dev_err(di->dev, "error reading temperature\n");
- return ret;
- }
+ int rsoc;
if (di->chip == BQ27500)
- return temp - 2731;
+ rsoc = bq27x00_read(di, BQ27500_REG_SOC, false);
else
- return ((temp * 5) - 5463) / 2;
+ rsoc = bq27x00_read(di, BQ27000_REG_RSOC, true);
+
+ if (rsoc < 0)
+ dev_err(di->dev, "error reading relative State-of-Charge\n");
+
+ return rsoc;
}
/*
- * Return the battery Voltage in milivolts
- * Or < 0 if something fails.
+ * Read a time register.
+ * Return < 0 if something fails.
*/
-static int bq27x00_battery_voltage(struct bq27x00_device_info *di)
+static int bq27x00_battery_read_time(struct bq27x00_device_info *di, u8 reg)
{
- int ret;
- int volt = 0;
+ int tval;
- ret = bq27x00_read(di, BQ27x00_REG_VOLT, &volt, false);
- if (ret) {
- dev_err(di->dev, "error reading voltage\n");
- return ret;
+ tval = bq27x00_read(di, reg, false);
+ if (tval < 0) {
+ dev_err(di->dev, "error reading register %02x: %d\n", reg, tval);
+ return tval;
}
- return volt * 1000;
+ if (tval == 65535)
+ return -ENODATA;
+
+ return tval * 60;
+}
+
+static void bq27x00_update(struct bq27x00_device_info *di)
+{
+ struct bq27x00_reg_cache cache = {0, };
+ bool is_bq27500 = di->chip == BQ27500;
+
+ cache.flags = bq27x00_read(di, BQ27x00_REG_FLAGS, is_bq27500);
+ if (cache.flags >= 0) {
+ cache.capacity = bq27x00_battery_read_rsoc(di);
+ cache.temperature = bq27x00_read(di, BQ27x00_REG_TEMP, false);
+ cache.time_to_empty = bq27x00_battery_read_time(di, BQ27x00_REG_TTE);
+ cache.time_to_empty_avg = bq27x00_battery_read_time(di, BQ27x00_REG_TTECP);
+ cache.time_to_full = bq27x00_battery_read_time(di, BQ27x00_REG_TTF);
+
+ if (!is_bq27500)
+ cache.current_now = bq27x00_read(di, BQ27x00_REG_AI, false);
+ }
+
+ /* Ignore current_now which is a snapshot of the current battery state
+ * and is likely to be different even between two consecutive reads */
+ if (memcmp(&di->cache, &cache, sizeof(cache) - sizeof(int)) != 0) {
+ di->cache = cache;
+ power_supply_changed(&di->bat);
+ }
+
+ di->last_update = jiffies;
+}
+
+/*
+ * Return the battery temperature in tenths of degree Celsius
+ * Or < 0 if something fails.
+ */
+static int bq27x00_battery_temperature(struct bq27x00_device_info *di,
+ union power_supply_propval *val)
+{
+ if (di->cache.temperature < 0)
+ return di->cache.temperature;
+
+ if (di->chip == BQ27500)
+ val->intval = di->cache.temperature - 2731;
+ else
+ val->intval = ((di->cache.temperature * 5) - 5463) / 2;
+
+ return 0;
}
/*
@@ -134,109 +192,84 @@ static int bq27x00_battery_voltage(struct bq27x00_device_info *di)
* Note that current can be negative signed as well
* Or 0 if something fails.
*/
-static int bq27x00_battery_current(struct bq27x00_device_info *di)
+static int bq27x00_battery_current(struct bq27x00_device_info *di,
+ union power_supply_propval *val)
{
- int ret;
- int curr = 0;
- int flags = 0;
+ int curr;
- ret = bq27x00_read(di, BQ27x00_REG_AI, &curr, false);
- if (ret) {
- dev_err(di->dev, "error reading current\n");
- return 0;
- }
+ if (di->chip == BQ27500)
+ curr = bq27x00_read(di, BQ27x00_REG_AI, false);
+ else
+ curr = di->cache.current_now;
+
+ if (curr < 0)
+ return curr;
if (di->chip == BQ27500) {
/* bq27500 returns signed value */
- curr = (int)((s16)curr) * 1000;
+ val->intval = (int)((s16)curr) * 1000;
} else {
- ret = bq27x00_read(di, BQ27x00_REG_FLAGS, &flags, false);
- if (ret < 0) {
- dev_err(di->dev, "error reading flags\n");
- return 0;
- }
- if (flags & BQ27000_FLAG_CHGS) {
+ if (di->cache.flags & BQ27000_FLAG_CHGS) {
dev_dbg(di->dev, "negative current!\n");
curr = -curr;
}
- curr = curr * 3570 / BQ27000_RS;
- }
-
- return curr;
-}
-
-/*
- * Return the battery Relative State-of-Charge
- * Or < 0 if something fails.
- */
-static int bq27x00_battery_rsoc(struct bq27x00_device_info *di)
-{
- int ret;
- int rsoc = 0;
- if (di->chip == BQ27500)
- ret = bq27x00_read(di, BQ27500_REG_SOC, &rsoc, false);
- else
- ret = bq27x00_read(di, BQ27000_REG_RSOC, &rsoc, true);
- if (ret) {
- dev_err(di->dev, "error reading relative State-of-Charge\n");
- return ret;
+ val->intval = curr * 3570 / BQ27000_RS;
}
- return rsoc;
+ return 0;
}
static int bq27x00_battery_status(struct bq27x00_device_info *di,
- union power_supply_propval *val)
+ union power_supply_propval *val)
{
- int flags = 0;
int status;
- int ret;
-
- ret = bq27x00_read(di, BQ27x00_REG_FLAGS, &flags, false);
- if (ret < 0) {
- dev_err(di->dev, "error reading flags\n");
- return ret;
- }
if (di->chip == BQ27500) {
- if (flags & BQ27500_FLAG_FC)
+ if (di->cache.flags & BQ27500_FLAG_FC)
status = POWER_SUPPLY_STATUS_FULL;
- else if (flags & BQ27500_FLAG_DSC)
+ else if (di->cache.flags & BQ27500_FLAG_DSC)
status = POWER_SUPPLY_STATUS_DISCHARGING;
else
status = POWER_SUPPLY_STATUS_CHARGING;
} else {
- if (flags & BQ27000_FLAG_CHGS)
+ if (di->cache.flags & BQ27000_FLAG_CHGS)
status = POWER_SUPPLY_STATUS_CHARGING;
else
status = POWER_SUPPLY_STATUS_DISCHARGING;
}
val->intval = status;
+
return 0;
}
/*
- * Read a time register.
- * Return < 0 if something fails.
+ * Return the battery Voltage in milivolts
+ * Or < 0 if something fails.
*/
-static int bq27x00_battery_time(struct bq27x00_device_info *di, int reg,
- union power_supply_propval *val)
+static int bq27x00_battery_voltage(struct bq27x00_device_info *di,
+ union power_supply_propval *val)
{
- int tval = 0;
- int ret;
+ int volt;
- ret = bq27x00_read(di, reg, &tval, false);
- if (ret) {
- dev_err(di->dev, "error reading register %02x\n", reg);
- return ret;
- }
+ volt = bq27x00_read(di, BQ27x00_REG_VOLT, false);
+ if (volt < 0)
+ return volt;
- if (tval == 65535)
- return -ENODATA;
+ val->intval = volt * 1000;
+
+ return 0;
+}
+
+static int bq27x00_simple_value(int value,
+ union power_supply_propval *val)
+{
+ if (value < 0)
+ return value;
+
+ val->intval = value;
- val->intval = tval * 60;
return 0;
}
@@ -249,9 +282,11 @@ static int bq27x00_battery_get_property(struct power_supply *psy,
{
int ret = 0;
struct bq27x00_device_info *di = to_bq27x00_device_info(psy);
- int voltage = bq27x00_battery_voltage(di);
- if (psp != POWER_SUPPLY_PROP_PRESENT && voltage <= 0)
+ if (time_is_before_jiffies(di->last_update + 5 * HZ))
+ bq27x00_update(di);
+
+ if (psp != POWER_SUPPLY_PROP_PRESENT && di->cache.flags < 0)
return -ENODEV;
switch (psp) {
@@ -259,29 +294,28 @@ static int bq27x00_battery_get_property(struct power_supply *psy,
ret = bq27x00_battery_status(di, val);
break;
case POWER_SUPPLY_PROP_VOLTAGE_NOW:
- val->intval = voltage;
+ ret = bq27x00_battery_voltage(di, val);
break;
case POWER_SUPPLY_PROP_PRESENT:
- if (psp == POWER_SUPPLY_PROP_PRESENT)
- val->intval = voltage <= 0 ? 0 : 1;
+ val->intval = di->cache.flags < 0 ? 0 : 1;
break;
case POWER_SUPPLY_PROP_CURRENT_NOW:
- val->intval = bq27x00_battery_current(di);
+ ret = bq27x00_battery_current(di, val);
break;
case POWER_SUPPLY_PROP_CAPACITY:
- val->intval = bq27x00_battery_rsoc(di);
+ ret = bq27x00_simple_value(di->cache.capacity, val);
break;
case POWER_SUPPLY_PROP_TEMP:
- val->intval = bq27x00_battery_temperature(di);
+ ret = bq27x00_battery_temperature(di, val);
break;
case POWER_SUPPLY_PROP_TIME_TO_EMPTY_NOW:
- ret = bq27x00_battery_time(di, BQ27x00_REG_TTE, val);
+ ret = bq27x00_simple_value(di->cache.time_to_empty, val);
break;
case POWER_SUPPLY_PROP_TIME_TO_EMPTY_AVG:
- ret = bq27x00_battery_time(di, BQ27x00_REG_TTECP, val);
+ ret = bq27x00_simple_value(di->cache.time_to_empty_avg, val);
break;
case POWER_SUPPLY_PROP_TIME_TO_FULL_NOW:
- ret = bq27x00_battery_time(di, BQ27x00_REG_TTF, val);
+ ret = bq27x00_simple_value(di->cache.time_to_full, val);
break;
case POWER_SUPPLY_PROP_TECHNOLOGY:
val->intval = POWER_SUPPLY_TECHNOLOGY_LION;
@@ -311,6 +345,8 @@ static int bq27x00_powersupply_init(struct bq27x00_device_info *di)
dev_info(di->dev, "support ver. %s enabled\n", DRIVER_VERSION);
+ bq27x00_update(di);
+
return 0;
}
@@ -324,13 +360,12 @@ static int bq27x00_powersupply_init(struct bq27x00_device_info *di)
static DEFINE_IDR(battery_id);
static DEFINE_MUTEX(battery_mutex);
-static int bq27x00_read_i2c(struct bq27x00_device_info *di, u8 reg,
- int *rt_value, bool single)
+static int bq27x00_read_i2c(struct bq27x00_device_info *di, u8 reg, bool single)
{
struct i2c_client *client = to_i2c_client(di->dev);
struct i2c_msg msg[1];
unsigned char data[2];
- int err;
+ int ret;
if (!client->adapter)
return -ENODEV;
@@ -341,26 +376,24 @@ static int bq27x00_read_i2c(struct bq27x00_device_info *di, u8 reg,
msg->buf = data;
data[0] = reg;
- err = i2c_transfer(client->adapter, msg, 1);
+ ret = i2c_transfer(client->adapter, msg, 1);
- if (err >= 0) {
+ if (ret >= 0) {
if (!single)
msg->len = 2;
else
msg->len = 1;
msg->flags = I2C_M_RD;
- err = i2c_transfer(client->adapter, msg, 1);
- if (err >= 0) {
+ ret = i2c_transfer(client->adapter, msg, 1);
+ if (ret >= 0) {
if (!single)
- *rt_value = get_unaligned_le16(data);
+ ret = get_unaligned_le16(data);
else
- *rt_value = data[0];
-
- return 0;
+ ret = data[0];
}
}
- return err;
+ return ret;
}
static int bq27x00_battery_probe(struct i2c_client *client,
@@ -477,7 +510,7 @@ static inline void bq27x00_battery_i2c_exit(void) {};
#ifdef CONFIG_BATTERY_BQ27X00_PLATFORM
static int bq27000_read_platform(struct bq27x00_device_info *di, u8 reg,
- int *rt_value, bool single)
+ bool single)
{
struct device *dev = di->dev;
struct bq27000_platform_data *pdata = dev->platform_data;
@@ -504,14 +537,10 @@ static int bq27000_read_platform(struct bq27x00_device_info *di, u8 reg,
if (timeout == 0)
return -EIO;
- *rt_value = (upper << 8) | lower;
- } else {
- lower = pdata->read(dev, reg);
- if (lower < 0)
- return lower;
- *rt_value = lower;
+ return (upper << 8) | lower;
}
- return 0;
+
+ return pdata->read(dev, reg);
}
static int __devinit bq27000_battery_probe(struct platform_device *pdev)
--
1.7.2.3
^ permalink raw reply related [flat|nested] 28+ messages in thread
* Re: [PATCH 07/14 v3] bq27x00: Cache battery registers
2011-02-14 3:01 ` [PATCH 07/14 v3] " Lars-Peter Clausen
@ 2011-02-14 21:58 ` Grazvydas Ignotas
2011-02-14 22:14 ` Lars-Peter Clausen
0 siblings, 1 reply; 28+ messages in thread
From: Grazvydas Ignotas @ 2011-02-14 21:58 UTC (permalink / raw)
To: Lars-Peter Clausen
Cc: Anton Vorontsov, Pali Rohar, Rodolfo Giometti, linux-kernel
On Mon, Feb 14, 2011 at 5:01 AM, Lars-Peter Clausen <lars@metafoo.de> wrote:
> This patch adds a register cache to the bq27x00 battery driver.
> Usually multiple, if not all, power_supply properties are queried at once,
> for example when an uevent is generated. Since some registers are used by
> multiple properties caching the registers should reduce the number of
> reads.
>
> The cache is valid for 5 seconds this roughly matches the internal update
> interval of the current register for the bq27000/bq27200.
>
> Fast changing properties(*_NOW) which can be obtained by reading a single
> register are not cached.
>
> It will also be used in the follow up patch to check if the battery status
> has been changed since the last update to emit power_supply_changed events.
>
> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
> ---
> drivers/power/bq27x00_battery.c | 271 +++++++++++++++++++++-----------------
> 1 files changed, 150 insertions(+), 121 deletions(-)
>
> diff --git a/drivers/power/bq27x00_battery.c b/drivers/power/bq27x00_battery.c
> index 44bc76b..dbe3fcb 100644
> --- a/drivers/power/bq27x00_battery.c
> +++ b/drivers/power/bq27x00_battery.c
<snip>
> -static int bq27x00_battery_current(struct bq27x00_device_info *di)
> +static int bq27x00_battery_current(struct bq27x00_device_info *di,
> + union power_supply_propval *val)
> {
> - int ret;
> - int curr = 0;
> - int flags = 0;
> + int curr;
>
> - ret = bq27x00_read(di, BQ27x00_REG_AI, &curr, false);
> - if (ret) {
> - dev_err(di->dev, "error reading current\n");
> - return 0;
> - }
> + if (di->chip == BQ27500)
> + curr = bq27x00_read(di, BQ27x00_REG_AI, false);
> + else
> + curr = di->cache.current_now;
> +
> + if (curr < 0)
> + return curr;
This is wrong, as read function returns negative values for bq27500
when discharging. That's why read function used to pass value through
argument before your series (return value was for error code).
Sorry for not noticing this earlier, I only found this now during testing.
>
> if (di->chip == BQ27500) {
> /* bq27500 returns signed value */
> - curr = (int)((s16)curr) * 1000;
> + val->intval = (int)((s16)curr) * 1000;
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 07/14 v3] bq27x00: Cache battery registers
2011-02-14 21:58 ` Grazvydas Ignotas
@ 2011-02-14 22:14 ` Lars-Peter Clausen
2011-02-15 11:48 ` Grazvydas Ignotas
0 siblings, 1 reply; 28+ messages in thread
From: Lars-Peter Clausen @ 2011-02-14 22:14 UTC (permalink / raw)
To: Grazvydas Ignotas
Cc: Anton Vorontsov, Pali Rohar, Rodolfo Giometti, linux-kernel
On 02/14/2011 10:58 PM, Grazvydas Ignotas wrote:
> On Mon, Feb 14, 2011 at 5:01 AM, Lars-Peter Clausen <lars@metafoo.de> wrote:
>> This patch adds a register cache to the bq27x00 battery driver.
>> Usually multiple, if not all, power_supply properties are queried at once,
>> for example when an uevent is generated. Since some registers are used by
>> multiple properties caching the registers should reduce the number of
>> reads.
>>
>> The cache is valid for 5 seconds this roughly matches the internal update
>> interval of the current register for the bq27000/bq27200.
>>
>> Fast changing properties(*_NOW) which can be obtained by reading a single
>> register are not cached.
>>
>> It will also be used in the follow up patch to check if the battery status
>> has been changed since the last update to emit power_supply_changed events.
>>
>> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
>> ---
>> drivers/power/bq27x00_battery.c | 271 +++++++++++++++++++++-----------------
>> 1 files changed, 150 insertions(+), 121 deletions(-)
>>
>> diff --git a/drivers/power/bq27x00_battery.c b/drivers/power/bq27x00_battery.c
>> index 44bc76b..dbe3fcb 100644
>> --- a/drivers/power/bq27x00_battery.c
>> +++ b/drivers/power/bq27x00_battery.c
>
> <snip>
>
>> -static int bq27x00_battery_current(struct bq27x00_device_info *di)
>> +static int bq27x00_battery_current(struct bq27x00_device_info *di,
>> + union power_supply_propval *val)
>> {
>> - int ret;
>> - int curr = 0;
>> - int flags = 0;
>> + int curr;
>>
>> - ret = bq27x00_read(di, BQ27x00_REG_AI, &curr, false);
>> - if (ret) {
>> - dev_err(di->dev, "error reading current\n");
>> - return 0;
>> - }
>> + if (di->chip == BQ27500)
>> + curr = bq27x00_read(di, BQ27x00_REG_AI, false);
>> + else
>> + curr = di->cache.current_now;
>> +
>> + if (curr < 0)
>> + return curr;
>
> This is wrong, as read function returns negative values for bq27500
> when discharging. That's why read function used to pass value through
> argument before your series (return value was for error code).
I don't think so. The register is 16bit wide and it is read as a unsigned. So
in the non error case bq27x00_read will always return >= 0.
The value is later reinterpreted as a signed 16bit.(See the other lines you
quoted underneath).
Did you experience any actual problem with current being wrong?
>
> Sorry for not noticing this earlier, I only found this now during testing.
>
>>
>> if (di->chip == BQ27500) {
>> /* bq27500 returns signed value */
>> - curr = (int)((s16)curr) * 1000;
>> + val->intval = (int)((s16)curr) * 1000;
- Lars
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 07/14 v3] bq27x00: Cache battery registers
2011-02-14 22:14 ` Lars-Peter Clausen
@ 2011-02-15 11:48 ` Grazvydas Ignotas
2011-02-15 22:39 ` Grazvydas Ignotas
0 siblings, 1 reply; 28+ messages in thread
From: Grazvydas Ignotas @ 2011-02-15 11:48 UTC (permalink / raw)
To: Lars-Peter Clausen
Cc: Anton Vorontsov, Pali Rohar, Rodolfo Giometti, linux-kernel
On Tue, Feb 15, 2011 at 12:14 AM, Lars-Peter Clausen <lars@metafoo.de> wrote:
> On 02/14/2011 10:58 PM, Grazvydas Ignotas wrote:
>> On Mon, Feb 14, 2011 at 5:01 AM, Lars-Peter Clausen <lars@metafoo.de> wrote:
>>>
>>> - ret = bq27x00_read(di, BQ27x00_REG_AI, &curr, false);
>>> - if (ret) {
>>> - dev_err(di->dev, "error reading current\n");
>>> - return 0;
>>> - }
>>> + if (di->chip == BQ27500)
>>> + curr = bq27x00_read(di, BQ27x00_REG_AI, false);
>>> + else
>>> + curr = di->cache.current_now;
>>> +
>>> + if (curr < 0)
>>> + return curr;
>>
>> This is wrong, as read function returns negative values for bq27500
>> when discharging. That's why read function used to pass value through
>> argument before your series (return value was for error code).
>
> I don't think so. The register is 16bit wide and it is read as a unsigned. So
> in the non error case bq27x00_read will always return >= 0.
> The value is later reinterpreted as a signed 16bit.(See the other lines you
> quoted underneath).
Hmh, right..
> Did you experience any actual problem with current being wrong?
Yes, the returned current values were randomly jumping between -500000
and 600000 while the device was discharging, so I thought
uninitialized values were being returned (this never happened before
the series; no errors in dmesg). I'll need to debug a bit more I
guess..
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 07/14 v3] bq27x00: Cache battery registers
2011-02-15 11:48 ` Grazvydas Ignotas
@ 2011-02-15 22:39 ` Grazvydas Ignotas
2011-02-21 8:28 ` Lars-Peter Clausen
0 siblings, 1 reply; 28+ messages in thread
From: Grazvydas Ignotas @ 2011-02-15 22:39 UTC (permalink / raw)
To: Lars-Peter Clausen
Cc: Anton Vorontsov, Pali Rohar, Rodolfo Giometti, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 2104 bytes --]
On Tue, Feb 15, 2011 at 1:48 PM, Grazvydas Ignotas <notasas@gmail.com> wrote:
> On Tue, Feb 15, 2011 at 12:14 AM, Lars-Peter Clausen <lars@metafoo.de> wrote:
>> On 02/14/2011 10:58 PM, Grazvydas Ignotas wrote:
>>> This is wrong, as read function returns negative values for bq27500
>>> when discharging. That's why read function used to pass value through
>>> argument before your series (return value was for error code).
>>
>> I don't think so. The register is 16bit wide and it is read as a unsigned. So
>> in the non error case bq27x00_read will always return >= 0.
>> The value is later reinterpreted as a signed 16bit.(See the other lines you
>> quoted underneath).
>
> Hmh, right..
>
>> Did you experience any actual problem with current being wrong?
>
> Yes, the returned current values were randomly jumping between -500000
> and 600000 while the device was discharging, so I thought
> uninitialized values were being returned (this never happened before
> the series; no errors in dmesg). I'll need to debug a bit more I
> guess..
ok this series seem to trigger unrelated bug, probably due to lots of
reads when cache is being updated, the attached patch seems to help.
Feel free to integrate it or I'll send it separately after your
patches are merged.
However there is bigger problem, compiling as module and doing
insmod/rmmod/insmod causes kernel OOPS:
[ 882.575714] BUG: sleeping function called from invalid context at
arch/arm/mm/fault.c:295
[ 882.584350] in_atomic(): 0, irqs_disabled(): 128, pid: 1154, name: insmod
...
[ 882.959930] Unable to handle kernel NULL pointer dereference at
virtual address 00000000
[ 882.968444] pgd = c14c8000
[ 882.977905] Internal error: Oops: 817 [#1]
...
[ 883.304412] [<c007eed8>] (__queue_work+0x140/0x260) from
[<c007f044>] (queue_work_on+0x2c/0x34)
[ 883.313568] [<c007f044>] (queue_work_on+0x2c/0x34) from
[<bf008390>] (bq27x00_update+0x1f8/0x220 [bq27x00_battery])
[ 883.324584] [<bf008390>] (bq27x00_update+0x1f8/0x220
[bq27x00_battery]) from [<bf0084c8>] (bq27x00_battery_poll+0x14/0x48
[bq27x00_battery])
full backtrace attached.
[-- Attachment #2: backtrace --]
[-- Type: application/octet-stream, Size: 18766 bytes --]
[-- Attachment #3: 0001-bq27x00-Use-single-i2c_transfer-call-for-property-re.patch --]
[-- Type: text/x-diff, Size: 1651 bytes --]
From 8fc5bdaf033d834d1728cd70c240d31bc457336c Mon Sep 17 00:00:00 2001
From: Grazvydas Ignotas <notasas@gmail.com>
Date: Tue, 15 Feb 2011 23:27:35 +0200
Subject: [PATCH] bq27x00: Use single i2c_transfer call for property read
Doing this by using 2 calls sometimes results in unexpected
values being returned on OMAP3 i2c controller.
Signed-off-by: Grazvydas Ignotas <notasas@gmail.com>
---
drivers/power/bq27x00_battery.c | 27 +++++++++++----------------
1 files changed, 11 insertions(+), 16 deletions(-)
diff --git a/drivers/power/bq27x00_battery.c b/drivers/power/bq27x00_battery.c
index 740a0ac..59e68db 100644
--- a/drivers/power/bq27x00_battery.c
+++ b/drivers/power/bq27x00_battery.c
@@ -565,31 +565,26 @@ static DEFINE_MUTEX(battery_mutex);
static int bq27x00_read_i2c(struct bq27x00_device_info *di, u8 reg, bool single)
{
struct i2c_client *client = to_i2c_client(di->dev);
- struct i2c_msg msg;
+ struct i2c_msg msg[2];
unsigned char data[2];
int ret;
if (!client->adapter)
return -ENODEV;
- msg.addr = client->addr;
- msg.flags = 0;
- msg.len = 1;
- msg.buf = data;
-
- data[0] = reg;
- ret = i2c_transfer(client->adapter, &msg, 1);
-
- if (ret < 0)
- return ret;
-
+ msg[0].addr = client->addr;
+ msg[0].flags = 0;
+ msg[0].buf = ®
+ msg[0].len = sizeof(reg);
+ msg[1].addr = client->addr;
+ msg[1].flags = I2C_M_RD;
+ msg[1].buf = data;
if (single)
- msg.len = 1;
+ msg[1].len = 1;
else
- msg.len = 2;
+ msg[1].len = 2;
- msg.flags = I2C_M_RD;
- ret = i2c_transfer(client->adapter, &msg, 1);
+ ret = i2c_transfer(client->adapter, msg, ARRAY_SIZE(msg));
if (ret < 0)
return ret;
--
1.6.3.3
^ permalink raw reply related [flat|nested] 28+ messages in thread
* Re: [PATCH 07/14 v3] bq27x00: Cache battery registers
2011-02-15 22:39 ` Grazvydas Ignotas
@ 2011-02-21 8:28 ` Lars-Peter Clausen
2011-02-21 14:00 ` Grazvydas Ignotas
0 siblings, 1 reply; 28+ messages in thread
From: Lars-Peter Clausen @ 2011-02-21 8:28 UTC (permalink / raw)
To: Grazvydas Ignotas
Cc: Anton Vorontsov, Pali Rohar, Rodolfo Giometti, linux-kernel
Hi
Sorry, for not responding earlier I was traveling and didn't had my board with me.
On 02/15/2011 11:39 PM, Grazvydas Ignotas wrote:
> On Tue, Feb 15, 2011 at 1:48 PM, Grazvydas Ignotas <notasas@gmail.com> wrote:
>> On Tue, Feb 15, 2011 at 12:14 AM, Lars-Peter Clausen <lars@metafoo.de> wrote:
>>> On 02/14/2011 10:58 PM, Grazvydas Ignotas wrote:
>>>> This is wrong, as read function returns negative values for bq27500
>>>> when discharging. That's why read function used to pass value through
>>>> argument before your series (return value was for error code).
>>>
>>> I don't think so. The register is 16bit wide and it is read as a unsigned. So
>>> in the non error case bq27x00_read will always return >= 0.
>>> The value is later reinterpreted as a signed 16bit.(See the other lines you
>>> quoted underneath).
>>
>> Hmh, right..
>>
>>> Did you experience any actual problem with current being wrong?
>>
>> Yes, the returned current values were randomly jumping between -500000
>> and 600000 while the device was discharging, so I thought
>> uninitialized values were being returned (this never happened before
>> the series; no errors in dmesg). I'll need to debug a bit more I
>> guess..
>
> ok this series seem to trigger unrelated bug, probably due to lots of
> reads when cache is being updated, the attached patch seems to help.
> Feel free to integrate it or I'll send it separately after your
> patches are merged.
I've added the patch to my bq27x00-for-upstream tree.
>
> However there is bigger problem, compiling as module and doing
> insmod/rmmod/insmod causes kernel OOPS:
>
> [ 882.575714] BUG: sleeping function called from invalid context at
> arch/arm/mm/fault.c:295
> [ 882.584350] in_atomic(): 0, irqs_disabled(): 128, pid: 1154, name: insmod
> ...
> [ 882.959930] Unable to handle kernel NULL pointer dereference at
> virtual address 00000000
> [ 882.968444] pgd = c14c8000
> [ 882.977905] Internal error: Oops: 817 [#1]
> ...
> [ 883.304412] [<c007eed8>] (__queue_work+0x140/0x260) from
> [<c007f044>] (queue_work_on+0x2c/0x34)
> [ 883.313568] [<c007f044>] (queue_work_on+0x2c/0x34) from
> [<bf008390>] (bq27x00_update+0x1f8/0x220 [bq27x00_battery])
> [ 883.324584] [<bf008390>] (bq27x00_update+0x1f8/0x220
> [bq27x00_battery]) from [<bf0084c8>] (bq27x00_battery_poll+0x14/0x48
> [bq27x00_battery])
>
> full backtrace attached.
I can't reproduce that OOPS. And the backtrace looks a bit strange,
queue_work_on is not called from bq27x00_update.
Can you send me the disassembly of your bq27x00_update?
- Lars
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 07/14 v3] bq27x00: Cache battery registers
2011-02-21 8:28 ` Lars-Peter Clausen
@ 2011-02-21 14:00 ` Grazvydas Ignotas
2011-02-21 14:49 ` Lars-Peter Clausen
0 siblings, 1 reply; 28+ messages in thread
From: Grazvydas Ignotas @ 2011-02-21 14:00 UTC (permalink / raw)
To: Lars-Peter Clausen
Cc: Anton Vorontsov, Pali Rohar, Rodolfo Giometti, linux-kernel
> I've added the patch to my bq27x00-for-upstream tree.
Thanks.
>>
>> However there is bigger problem, compiling as module and doing
>> insmod/rmmod/insmod causes kernel OOPS:
>>
>> [ 882.575714] BUG: sleeping function called from invalid context at
>> arch/arm/mm/fault.c:295
>> [ 882.584350] in_atomic(): 0, irqs_disabled(): 128, pid: 1154, name: insmod
>> ...
>> [ 882.959930] Unable to handle kernel NULL pointer dereference at
>> virtual address 00000000
>> [ 882.968444] pgd = c14c8000
>> [ 882.977905] Internal error: Oops: 817 [#1]
>> ...
>> [ 883.304412] [<c007eed8>] (__queue_work+0x140/0x260) from
>> [<c007f044>] (queue_work_on+0x2c/0x34)
>> [ 883.313568] [<c007f044>] (queue_work_on+0x2c/0x34) from
>> [<bf008390>] (bq27x00_update+0x1f8/0x220 [bq27x00_battery])
>> [ 883.324584] [<bf008390>] (bq27x00_update+0x1f8/0x220
>> [bq27x00_battery]) from [<bf0084c8>] (bq27x00_battery_poll+0x14/0x48
>> [bq27x00_battery])
>>
>> full backtrace attached.
>
> I can't reproduce that OOPS. And the backtrace looks a bit strange,
> queue_work_on is not called from bq27x00_update.
> Can you send me the disassembly of your bq27x00_update?
It comes from power_supply_changed, probably omitted because
queue_work is a tail function of power_supply_changed. It's probably
something i2c specific, I could try bisecting it for you if you like,
I know it doesn't happen before this series.
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 07/14 v3] bq27x00: Cache battery registers
2011-02-21 14:00 ` Grazvydas Ignotas
@ 2011-02-21 14:49 ` Lars-Peter Clausen
2011-02-21 21:23 ` Grazvydas Ignotas
0 siblings, 1 reply; 28+ messages in thread
From: Lars-Peter Clausen @ 2011-02-21 14:49 UTC (permalink / raw)
To: Grazvydas Ignotas
Cc: Anton Vorontsov, Pali Rohar, Rodolfo Giometti, linux-kernel
On 02/21/2011 03:00 PM, Grazvydas Ignotas wrote:
>>> However there is bigger problem, compiling as module and doing
>>> insmod/rmmod/insmod causes kernel OOPS:
>>>
>>> [ 882.575714] BUG: sleeping function called from invalid context at
>>> arch/arm/mm/fault.c:295
>>> [ 882.584350] in_atomic(): 0, irqs_disabled(): 128, pid: 1154, name: insmod
>>> ...
>>> [ 882.959930] Unable to handle kernel NULL pointer dereference at
>>> virtual address 00000000
>>> [ 882.968444] pgd = c14c8000
>>> [ 882.977905] Internal error: Oops: 817 [#1]
>>> ...
>>> [ 883.304412] [<c007eed8>] (__queue_work+0x140/0x260) from
>>> [<c007f044>] (queue_work_on+0x2c/0x34)
>>> [ 883.313568] [<c007f044>] (queue_work_on+0x2c/0x34) from
>>> [<bf008390>] (bq27x00_update+0x1f8/0x220 [bq27x00_battery])
>>> [ 883.324584] [<bf008390>] (bq27x00_update+0x1f8/0x220
>>> [bq27x00_battery]) from [<bf0084c8>] (bq27x00_battery_poll+0x14/0x48
>>> [bq27x00_battery])
>>>
>>> full backtrace attached.
>>
>> I can't reproduce that OOPS. And the backtrace looks a bit strange,
>> queue_work_on is not called from bq27x00_update.
>> Can you send me the disassembly of your bq27x00_update?
>
> It comes from power_supply_changed, probably omitted because
> queue_work is a tail function of power_supply_changed. It's probably
> something i2c specific, I could try bisecting it for you if you like,
> I know it doesn't happen before this series.
Hi
The following patch should hopefully fix the issue.
- Lars
>From 860fc31cef00bd87085100825ddbff82ad601c33 Mon Sep 17 00:00:00 2001
From: Lars-Peter Clausen <lars@metafoo.de>
Date: Mon, 21 Feb 2011 15:34:19 +0100
Subject: [PATCH] Initialize power_supply changed_work before calling device_add
Calling device_add causes a uevent for that device to be generated.
The power_supply uevent function calls the drivers get_property function,
which might causes the driver to update its state, which again might causes
the driver to call power_supply_changed(). Since the power_supplys
changed_work has not been initialized at this point the behavior is
undefined and might result in a OOPS.
This patch fixes the issue by initializing the power_supplys changed_work
prior to adding the power_supplys device to the device tree.
Reported-by: Grazvydas Ignotas <notasas@gmail.com>
Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
drivers/power/power_supply_core.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/power/power_supply_core.c b/drivers/power/power_supply_core.c
index 970f733..329b46b 100644
--- a/drivers/power/power_supply_core.c
+++ b/drivers/power/power_supply_core.c
@@ -171,6 +171,8 @@ int power_supply_register(struct device *parent, struct
power_supply *psy)
dev_set_drvdata(dev, psy);
psy->dev = dev;
+ INIT_WORK(&psy->changed_work, power_supply_changed_work);
+
rc = kobject_set_name(&dev->kobj, "%s", psy->name);
if (rc)
goto kobject_set_name_failed;
@@ -179,8 +181,6 @@ int power_supply_register(struct device *parent, struct
power_supply *psy)
if (rc)
goto device_add_failed;
- INIT_WORK(&psy->changed_work, power_supply_changed_work);
-
rc = power_supply_create_triggers(psy);
if (rc)
goto create_triggers_failed;
--
1.7.2.3
^ permalink raw reply related [flat|nested] 28+ messages in thread
* Re: [PATCH 07/14 v3] bq27x00: Cache battery registers
2011-02-21 14:49 ` Lars-Peter Clausen
@ 2011-02-21 21:23 ` Grazvydas Ignotas
0 siblings, 0 replies; 28+ messages in thread
From: Grazvydas Ignotas @ 2011-02-21 21:23 UTC (permalink / raw)
To: Lars-Peter Clausen
Cc: Anton Vorontsov, Pali Rohar, Rodolfo Giometti, linux-kernel
On Mon, Feb 21, 2011 at 4:49 PM, Lars-Peter Clausen <lars@metafoo.de> wrote:
> On 02/21/2011 03:00 PM, Grazvydas Ignotas wrote:
>> It comes from power_supply_changed, probably omitted because
>> queue_work is a tail function of power_supply_changed. It's probably
>> something i2c specific, I could try bisecting it for you if you like,
>> I know it doesn't happen before this series.
>
> Hi
>
> The following patch should hopefully fix the issue.
Indeed it does, everything is working nicely now.
For the whole series:
Tested-by: Grazvydas Ignotas <notasas@gmail.com>
>
> From 860fc31cef00bd87085100825ddbff82ad601c33 Mon Sep 17 00:00:00 2001
> From: Lars-Peter Clausen <lars@metafoo.de>
> Date: Mon, 21 Feb 2011 15:34:19 +0100
> Subject: [PATCH] Initialize power_supply changed_work before calling device_add
>
> Calling device_add causes a uevent for that device to be generated.
> The power_supply uevent function calls the drivers get_property function,
> which might causes the driver to update its state, which again might causes
> the driver to call power_supply_changed(). Since the power_supplys
> changed_work has not been initialized at this point the behavior is
> undefined and might result in a OOPS.
>
> This patch fixes the issue by initializing the power_supplys changed_work
> prior to adding the power_supplys device to the device tree.
>
> Reported-by: Grazvydas Ignotas <notasas@gmail.com>
> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
> ---
> drivers/power/power_supply_core.c | 4 ++--
> 1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/power/power_supply_core.c b/drivers/power/power_supply_core.c
> index 970f733..329b46b 100644
> --- a/drivers/power/power_supply_core.c
> +++ b/drivers/power/power_supply_core.c
> @@ -171,6 +171,8 @@ int power_supply_register(struct device *parent, struct
> power_supply *psy)
> dev_set_drvdata(dev, psy);
> psy->dev = dev;
>
> + INIT_WORK(&psy->changed_work, power_supply_changed_work);
> +
> rc = kobject_set_name(&dev->kobj, "%s", psy->name);
> if (rc)
> goto kobject_set_name_failed;
> @@ -179,8 +181,6 @@ int power_supply_register(struct device *parent, struct
> power_supply *psy)
> if (rc)
> goto device_add_failed;
>
> - INIT_WORK(&psy->changed_work, power_supply_changed_work);
> -
> rc = power_supply_create_triggers(psy);
> if (rc)
> goto create_triggers_failed;
> --
> 1.7.2.3
>
>
^ permalink raw reply [flat|nested] 28+ messages in thread
end of thread, other threads:[~2011-02-21 21:23 UTC | newest]
Thread overview: 28+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-02-12 19:38 [PATCH v2 00/14] POWER: BQ27x00: New Properties, fixes, bq27000 support Lars-Peter Clausen
2011-02-12 19:39 ` [PATCH 01/14] bq27x00: Add type property Lars-Peter Clausen
2011-02-12 19:39 ` [PATCH 02/14] bq27x00: Improve temperature property precession Lars-Peter Clausen
2011-02-12 19:39 ` [PATCH 03/14] bq27x00: Fix CURRENT_NOW property Lars-Peter Clausen
2011-02-12 19:39 ` [PATCH 04/14] bq27x00: Return -ENODEV for properties if the battery is not present Lars-Peter Clausen
2011-02-12 19:39 ` [PATCH 05/14] bq27x00: Prepare code for addition of bq27000 platform driver Lars-Peter Clausen
2011-02-12 19:39 ` [PATCH 06/14] bq27x00: Add bq27000 support Lars-Peter Clausen
2011-02-12 19:39 ` [PATCH 07/14] bq27x00: Cache battery registers Lars-Peter Clausen
2011-02-13 15:14 ` Grazvydas Ignotas
2011-02-13 18:56 ` Lars-Peter Clausen
2011-02-14 3:01 ` [PATCH 07/14 v3] " Lars-Peter Clausen
2011-02-14 21:58 ` Grazvydas Ignotas
2011-02-14 22:14 ` Lars-Peter Clausen
2011-02-15 11:48 ` Grazvydas Ignotas
2011-02-15 22:39 ` Grazvydas Ignotas
2011-02-21 8:28 ` Lars-Peter Clausen
2011-02-21 14:00 ` Grazvydas Ignotas
2011-02-21 14:49 ` Lars-Peter Clausen
2011-02-21 21:23 ` Grazvydas Ignotas
2011-02-12 19:39 ` [PATCH v2 07/14] bq27X00: " Lars-Peter Clausen
2011-02-12 19:52 ` Lars-Peter Clausen
2011-02-12 19:39 ` [PATCH 08/14] bq27x00: Poll battery state Lars-Peter Clausen
2011-02-12 19:39 ` [PATCH 09/14] bq27x00: Add new properties Lars-Peter Clausen
2011-02-12 19:39 ` [PATCH 10/14] bq27x00: Add MODULE_DEVICE_TABLE Lars-Peter Clausen
2011-02-12 19:39 ` [PATCH 11/14] bq27x00: Give more specific reports on battery status Lars-Peter Clausen
2011-02-12 19:39 ` [PATCH 12/14] bq27x00: Minor cleanups Lars-Peter Clausen
2011-02-12 19:39 ` [PATCH 13/14] bq27x00: Cleanup bq27x00_i2c_read Lars-Peter Clausen
2011-02-12 19:39 ` [PATCH 14/14] Ignore -ENODATA errors when generating a uevent Lars-Peter Clausen
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.