2011-02-06 19:36:36

by Lars-Peter Clausen

[permalink] [raw]
Subject: [PATCH 00/14] POWER: BQ27x00: New Properties, fixes, bq27000 support

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.


Lars-Peter Clausen (10):
POWER: bq27x00: Add type property
POWER: bq27x00: Improve temperature precession
POWER: bq27x00: Return -ENODEV for properties if the battery is not
present
POWER: bq27x00: Prepare code for addition of bq27000 platform driver
POWER: bq27x00: Add bq27000 support
POWER: bq27X00: Cache battery registers
POWER: bq27x00: Poll battery state
POWER: bq27x00: Give more specific reports on battery status
POWER: bq27x00: Cleanup bq27x00_i2c_read
POWER: Ignore -ENODATA errors when generating a uevent

Pali Rohár (4):
POWER: bq27x00: Fix current now property
POWER: bq27x00: Add new properties
POWER: bq27x00: Add MODULE_DEVICE_TABLE
POWER: bq27x00: Minor cleanups

drivers/power/Kconfig | 14 +
drivers/power/bq27x00_battery.c | 725 +++++++++++++++++++++++++--------
drivers/power/power_supply_sysfs.c | 2 +-
include/linux/power/bq27x00_battery.h | 19 +
4 files changed, 590 insertions(+), 170 deletions(-)
create mode 100644 include/linux/power/bq27x00_battery.h

--
1.7.2.3


2011-02-06 16:34:33

by Lars-Peter Clausen

[permalink] [raw]
Subject: [PATCH 11/14] POWER: bq27x00: Give more specific reports on battery status

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 <[email protected]>
---
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 7540364..961238f 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 */
@@ -395,8 +396,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

2011-02-06 16:38:28

by Rodolfo Giometti

[permalink] [raw]
Subject: Re: [PATCH 11/14] POWER: bq27x00: Give more specific reports on battery status

On Sun, Feb 06, 2011 at 01:48:08AM +0100, Lars-Peter Clausen wrote:
> 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 <[email protected]>
> ---
> 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 7540364..961238f 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 */
> @@ -395,8 +396,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
>

Acked-by: Rodolfo Giometti <[email protected]>

--

GNU/Linux Solutions e-mail: [email protected]
Linux Device Driver [email protected]
Embedded Systems phone: +39 349 2432127
UNIX programming skype: rodolfo.giometti
Freelance ICT Italia - Consulente ICT Italia - http://www.consulenti-ict.it

2011-02-06 16:40:40

by Rodolfo Giometti

[permalink] [raw]
Subject: Re: [PATCH 04/14] POWER: bq27x00: Return -ENODEV for properties if the battery is not present

On Sun, Feb 06, 2011 at 01:48:01AM +0100, Lars-Peter Clausen wrote:
> 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 <[email protected]>
> ---
> 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
>

Acked-by: Rodolfo Giometti <[email protected]>

--

GNU/Linux Solutions e-mail: [email protected]
Linux Device Driver [email protected]
Embedded Systems phone: +39 349 2432127
UNIX programming skype: rodolfo.giometti
Freelance ICT Italia - Consulente ICT Italia - http://www.consulenti-ict.it

2011-02-06 16:43:19

by Rodolfo Giometti

[permalink] [raw]
Subject: Re: [PATCH 01/14] POWER: bq27x00: Add type property

On Sun, Feb 06, 2011 at 01:47:58AM +0100, Lars-Peter Clausen wrote:
> This patch adds the type property to the bq27x00 battery driver.
> All bq27x00 are lithium ion batteries.
>
> Signed-off-by: Lars-Peter Clausen <[email protected]>
> ---
> 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
>

Acked-by: Rodolfo Giometti <[email protected]>

--

GNU/Linux Solutions e-mail: [email protected]
Linux Device Driver [email protected]
Embedded Systems phone: +39 349 2432127
UNIX programming skype: rodolfo.giometti
Freelance ICT Italia - Consulente ICT Italia - http://www.consulenti-ict.it

2011-02-06 16:47:47

by Lars-Peter Clausen

[permalink] [raw]
Subject: [PATCH 01/14] POWER: bq27x00: Add type property

This patch adds the type property to the bq27x00 battery driver.
All bq27x00 are lithium ion batteries.

Signed-off-by: Lars-Peter Clausen <[email protected]>
---
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

2011-02-06 16:49:12

by Rodolfo Giometti

[permalink] [raw]
Subject: Re: [PATCH 10/14] POWER: bq27x00: Add MODULE_DEVICE_TABLE

On Sun, Feb 06, 2011 at 01:48:07AM +0100, Lars-Peter Clausen wrote:
> From: Pali Roh?r <[email protected]>
>
> This patch adds MODULE_DEVICE_TABLE for module bq27x00_battery.
>
> Signed-off-by: Pali Roh?r <[email protected]>
> Tested-by: Pali Roh?r <[email protected]>
> Signed-off-by: Lars-Peter Clausen <[email protected]>
> ---
> 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 49affd6..7540364 100644
> --- a/drivers/power/bq27x00_battery.c
> +++ b/drivers/power/bq27x00_battery.c
> @@ -665,6 +665,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
>

Acked-by: Rodolfo Giometti <[email protected]>

--

GNU/Linux Solutions e-mail: [email protected]
Linux Device Driver [email protected]
Embedded Systems phone: +39 349 2432127
UNIX programming skype: rodolfo.giometti
Freelance ICT Italia - Consulente ICT Italia - http://www.consulenti-ict.it

2011-02-06 19:33:25

by Lars-Peter Clausen

[permalink] [raw]
Subject: [PATCH 06/14] POWER: bq27x00: Add bq27000 support

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 <[email protected]>
---
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 eee0120..ae4677f 100644
--- a/drivers/power/bq27x00_battery.c
+++ b/drivers/power/bq27x00_battery.c
@@ -3,6 +3,7 @@
*
* Copyright (C) 2008 Rodolfo Giometti <[email protected]>
* Copyright (C) 2008 Eurotech S.p.A. <[email protected]>
+ * Copyright (C) 2010-2011 Lars-Peter Clausen <[email protected]>
*
* 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

2011-02-06 19:33:38

by Lars-Peter Clausen

[permalink] [raw]
Subject: [PATCH 14/14] POWER: Ignore -ENODATA errors when generating a uevent

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

2011-02-06 19:33:45

by Lars-Peter Clausen

[permalink] [raw]
Subject: [PATCH 02/14] POWER: bq27x00: Improve temperature precession

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

2011-02-06 19:34:20

by Lars-Peter Clausen

[permalink] [raw]
Subject: [PATCH 08/14] POWER: bq27x00: Poll battery state

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 <[email protected]>
---
drivers/power/bq27x00_battery.c | 47 +++++++++++++++++++++++++++++++++++---
1 files changed, 43 insertions(+), 4 deletions(-)

diff --git a/drivers/power/bq27x00_battery.c b/drivers/power/bq27x00_battery.c
index cabc7c9..0dc7771 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>
@@ -74,6 +75,7 @@ struct bq27x00_device_info {

struct bq27x00_reg_cache cache;
unsigned long last_update;
+ struct delayed_work work;

struct power_supply bat;

@@ -93,6 +95,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
*/
@@ -282,7 +289,7 @@ static int bq27x00_battery_get_property(struct power_supply *psy,
struct bq27x00_device_info *di = to_bq27x00_device_info(psy);

if (time_is_before_jiffies(di->last_update + 6 * HZ))
- bq27x00_update(di);
+ flush_delayed_work_sync(&di->work);

if (psp != POWER_SUPPLY_PROP_PRESENT && di->cache.voltage <= 0)
return -ENODEV;
@@ -325,6 +332,28 @@ static int bq27x00_battery_get_property(struct power_supply *psy,
return ret;
}

+static void bq27x00_battery_work(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);
+ }
+}
+
+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;
@@ -333,7 +362,9 @@ 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_work);

ret = power_supply_register(di->dev, &di->bat);
if (ret) {
@@ -348,6 +379,13 @@ 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);
+}
+

/* i2c specific code */
#ifdef CONFIG_BATTERY_BQ27X00_I2C
@@ -455,7 +493,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);

@@ -588,7 +626,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

2011-02-06 19:34:33

by Lars-Peter Clausen

[permalink] [raw]
Subject: [PATCH 13/14] POWER: bq27x00: Cleanup bq27x00_i2c_read

Some minor stylistic cleanups.

Signed-off-by: Lars-Peter Clausen <[email protected]>
---
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 83904a2..03d073c 100644
--- a/drivers/power/bq27x00_battery.c
+++ b/drivers/power/bq27x00_battery.c
@@ -558,36 +558,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 err, 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;
- err = i2c_transfer(client->adapter, msg, 1);
+ err = i2c_transfer(client->adapter, &msg, 1);
+
+ if (err < 0)
+ return err;
+
+ if (single)
+ msg.len = 1;
+ else
+ msg.len = 2;
+
+ msg.flags = I2C_M_RD;
+ err = i2c_transfer(client->adapter, &msg, 1);
+ if (err < 0)
+ return err;
+
+ if (!single)
+ ret = get_unaligned_le16(data);
+ else
+ ret = data[0];

- if (err >= 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) {
- if (!single)
- ret = get_unaligned_le16(data);
- else
- ret = data[0];
- }
- }
return ret;
}

--
1.7.2.3

2011-02-06 19:36:47

by Lars-Peter Clausen

[permalink] [raw]
Subject: [PATCH 05/14] POWER: bq27x00: Prepare code for addition of bq27000 platform driver

This patch simplifies the drivers data structure and moves code to be shared by
both init functions into a common function.
This patch has no functional changes, it only moves code around.

Signed-off-by: Lars-Peter Clausen <[email protected]>
---
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..eee0120 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(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

2011-02-06 19:36:37

by Lars-Peter Clausen

[permalink] [raw]
Subject: [PATCH 12/14] POWER: bq27x00: Minor cleanups

From: Pali Rohár <[email protected]>

* Consistently use uppercase for hexadecimal values.
* Clarify/fix the unit of functions return value in its comment.

Signed-off-by: Pali Rohár <[email protected]>
Signed-off-by: Lars-Peter Clausen <[email protected]>
---
drivers/power/bq27x00_battery.c | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/power/bq27x00_battery.c b/drivers/power/bq27x00_battery.c
index 961238f..83904a2 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)
@@ -136,7 +136,7 @@ static inline int bq27x00_read(struct bq27x00_device_info *di, u8 reg,
}

/*
- * Return the battery Voltage in milivolts
+ * Return the battery Voltage in µV
* Or < 0 if something fails.
*/
static int bq27x00_battery_read_voltage(struct bq27x00_device_info *di)
@@ -349,7 +349,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

2011-02-06 19:39:05

by Lars-Peter Clausen

[permalink] [raw]
Subject: [PATCH 07/14] POWER: bq27X00: Cache battery registers

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.

Furthermore the registers of the bq27x00 are only updated at discrete
intervals, for how long the cache is valid is set to roughly match
the batteries internal update interval.

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 <[email protected]>
---
drivers/power/bq27x00_battery.c | 263 +++++++++++++++++++++-----------------
1 files changed, 145 insertions(+), 118 deletions(-)

diff --git a/drivers/power/bq27x00_battery.c b/drivers/power/bq27x00_battery.c
index ae4677f..cabc7c9 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,31 @@

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 voltage;
+ 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,107 @@ 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 Voltage in milivolts
* Or < 0 if something fails.
*/
-static int bq27x00_battery_temperature(struct bq27x00_device_info *di)
+static int bq27x00_battery_read_voltage(struct bq27x00_device_info *di)
{
- int ret;
- int temp = 0;
+ int volt;

- ret = bq27x00_read(di, BQ27x00_REG_TEMP, &temp, false);
- if (ret) {
- dev_err(di->dev, "error reading temperature\n");
- return ret;
- }
+ volt = bq27x00_read(di, BQ27x00_REG_VOLT, false);
+ if (volt < 0)
+ return volt;
+
+ return volt * 1000;
+}
+
+/*
+ * Return the battery Relative State-of-Charge
+ * Or < 0 if something fails.
+ */
+static int bq27x00_battery_read_rsoc(struct bq27x00_device_info *di)
+{
+ 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.voltage = bq27x00_battery_read_voltage(di);
+ if (cache.voltage >= 0) {
+ cache.flags = bq27x00_read(di, BQ27x00_REG_FLAGS, is_bq27500);
+ cache.current_now = bq27x00_read(di, BQ27x00_REG_AI, false);
+ 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);
+ }
+
+ /* Ignore those values which are snapshots of the current battery state
+ * and are likely to be different even between two consecutive reads */
+ if (memcmp(&di->cache, &cache, sizeof(cache) - 2*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 +206,68 @@ 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;

- ret = bq27x00_read(di, BQ27x00_REG_AI, &curr, false);
- if (ret) {
- dev_err(di->dev, "error reading current\n");
- return 0;
- }
+ if (di->cache.current_now < 0)
+ return di->cache.current_now;

if (di->chip == BQ27500) {
/* bq27500 returns signed value */
- curr = (int)((s16)curr) * 1000;
+ val->intval = (int)((s16)di->cache.current_now) * 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 < 0)
+ return di->cache.flags;
+
+ curr = di->cache.current_now;
+ 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->cache.flags < 0)
+ return di->cache.flags;

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.
- */
-static int bq27x00_battery_time(struct bq27x00_device_info *di, int reg,
- union power_supply_propval *val)
+static int bq27x00_simple_value(int value,
+ union power_supply_propval *val)
{
- int tval = 0;
- int ret;
-
- ret = bq27x00_read(reg, &tval, false);
- if (ret) {
- dev_err(di->dev, "error reading register %02x\n", reg);
- return ret;
- }
+ if (value < 0)
+ return value;

- if (tval == 65535)
- return -ENODATA;
+ val->intval = value;

- val->intval = tval * 60;
return 0;
}

@@ -249,9 +280,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 + 6 * HZ))
+ bq27x00_update(di);
+
+ if (psp != POWER_SUPPLY_PROP_PRESENT && di->cache.voltage <= 0)
return -ENODEV;

switch (psp) {
@@ -259,29 +292,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_simple_value(di->cache.voltage, val);
break;
case POWER_SUPPLY_PROP_PRESENT:
- if (psp == POWER_SUPPLY_PROP_PRESENT)
- val->intval = voltage <= 0 ? 0 : 1;
+ val->intval = di->cache.voltage <= 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 +343,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 +358,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 err, ret;

if (!client->adapter)
return -ENODEV;
@@ -353,14 +386,12 @@ static int bq27x00_read_i2c(struct bq27x00_device_info *di, u8 reg,
err = i2c_transfer(client->adapter, msg, 1);
if (err >= 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 +508,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 +535,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

2011-02-06 19:39:34

by Lars-Peter Clausen

[permalink] [raw]
Subject: [PATCH 10/14] POWER: bq27x00: Add MODULE_DEVICE_TABLE

From: Pali Rohár <[email protected]>

This patch adds MODULE_DEVICE_TABLE for module bq27x00_battery.

Signed-off-by: Pali Rohár <[email protected]>
Tested-by: Pali Rohár <[email protected]>
Signed-off-by: Lars-Peter Clausen <[email protected]>
---
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 49affd6..7540364 100644
--- a/drivers/power/bq27x00_battery.c
+++ b/drivers/power/bq27x00_battery.c
@@ -665,6 +665,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

2011-02-06 19:39:44

by Lars-Peter Clausen

[permalink] [raw]
Subject: [PATCH 04/14] POWER: bq27x00: Return -ENODEV for properties if the battery is not present

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

2011-02-06 20:03:40

by Lars-Peter Clausen

[permalink] [raw]
Subject: [PATCH 03/14] POWER: bq27x00: Fix current now property

From: Pali Rohár <[email protected]>

According to the bq27000 datasheet to obtain the current value in Micro-ampere
from the AI register it has to be transformed by the following formula:
current = AI * 3570 / 20

This patch adjust the drivers code accordingly.

Signed-off-by: Pali Rohár <[email protected]>
Signed-off-by: Lars-Peter Clausen <[email protected]>
---
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

2011-02-06 20:44:06

by Lars-Peter Clausen

[permalink] [raw]
Subject: [PATCH 09/14] POWER: bq27x00: Add new properties

From: Pali Rohár <[email protected]>

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 <[email protected]>
Signed-off-by: Lars-Peter Clausen <[email protected]>
---
drivers/power/bq27x00_battery.c | 158 ++++++++++++++++++++++++++++++++++++++-
1 files changed, 156 insertions(+), 2 deletions(-)

diff --git a/drivers/power/bq27x00_battery.c b/drivers/power/bq27x00_battery.c
index 0dc7771..49affd6 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,11 +74,15 @@ 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;

int voltage;
+ int charge_now;
int current_now;
+ int energy_now;
};

struct bq27x00_device_info {
@@ -74,6 +91,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;

@@ -93,6 +112,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;
@@ -145,6 +169,113 @@ static int bq27x00_battery_read_rsoc(struct bq27x00_device_info *di)
}

/*
+ * Return the battery Nominal available capaciy in µAh
+ * Or < 0 if something fails.
+ */
+static int bq27x00_battery_read_nac(struct bq27x00_device_info *di)
+{
+ int nac;
+
+ nac = bq27x00_read(di, BQ27x00_REG_NAC, false);
+ if (nac < 0) {
+ dev_err(di->dev, "error reading nominal available capacity\n");
+ return nac;
+ }
+
+ if (di->chip == BQ27500)
+ nac *= 1000;
+ else
+ nac = nac * 3570 / BQ27000_RS;
+
+ return nac;
+}
+
+/*
+ * Return the battery Last measured discharge in µAh
+ * Or < 0 if something fails.
+ */
+static int bq27x00_battery_read_lmd(struct bq27x00_device_info *di)
+{
+ int lmd;
+
+ lmd = bq27x00_read(di, BQ27x00_REG_LMD, false);
+ if (lmd < 0) {
+ dev_err(di->dev, "error reading last measured discharge\n");
+ return lmd;
+ }
+
+ if (di->chip == BQ27500)
+ lmd *= 1000;
+ else
+ lmd = lmd * 3570 / BQ27000_RS;
+
+ return 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;
+}
+
+/*
+ * Return the battery Available energy in µWh
+ * Or < 0 if something fails.
+ */
+static int bq27x00_battery_read_energy(struct bq27x00_device_info *di)
+{
+ 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;
+
+ return ae;
+}
+
+/*
* Read a time register.
* Return < 0 if something fails.
*/
@@ -178,11 +309,19 @@ 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_now = bq27x00_battery_read_nac(di);
+ cache.charge_full = bq27x00_battery_read_lmd(di);
+ cache.charge_counter = bq27x00_battery_read_cyct(di);
+ cache.energy_now = bq27x00_battery_read_energy(di);
+
+ /* 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 those values which are snapshots of the current battery state
* and are likely to be different even between two consecutive reads */
- if (memcmp(&di->cache, &cache, sizeof(cache) - 2*sizeof(int)) != 0) {
+ if (memcmp(&di->cache, &cache, sizeof(cache) - 4*sizeof(int)) != 0) {
di->cache = cache;
power_supply_changed(&di->bat);
}
@@ -325,6 +464,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(di->cache.charge_now, 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_simple_value(di->cache.energy_now, val);
+ break;
default:
return -EINVAL;
}
--
1.7.2.3

2011-02-06 22:47:12

by Felipe Contreras

[permalink] [raw]
Subject: Re: [PATCH 00/14] POWER: BQ27x00: New Properties, fixes, bq27000 support

Hi,

On Sun, Feb 6, 2011 at 2:47 AM, Lars-Peter Clausen <[email protected]> wrote:
> 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.

This might be a bit off-topic, but I have tried to get the battery on
the Nokia N900 working, and so far I'm only able to query the
remaining capacity, but not really to enabling charging. This is very
annoying, as I can only hack a certain amount of time, and then I have
to reflash, reboot, and wait for the batter to charge with the
official system (if there's enough power to flash).

Do you have any hints how to do that?

--
Felipe Contreras

2011-02-06 23:08:18

by Lars-Peter Clausen

[permalink] [raw]
Subject: Re: [PATCH 00/14] POWER: BQ27x00: New Properties, fixes, bq27000 support

On 02/06/2011 11:47 PM, Felipe Contreras wrote:
> Hi,
>
> On Sun, Feb 6, 2011 at 2:47 AM, Lars-Peter Clausen <[email protected]> wrote:
>> 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.
>
> This might be a bit off-topic, but I have tried to get the battery on
> the Nokia N900 working, and so far I'm only able to query the
> remaining capacity, but not really to enabling charging. This is very
> annoying, as I can only hack a certain amount of time, and then I have
> to reflash, reboot, and wait for the batter to charge with the
> official system (if there's enough power to flash).
>
> Do you have any hints how to do that?
>

Hi

Sorry, I've never worked with the N900, so I have no idea. But Pali has tested this
patch series on his N900, so he might have an idea.

- Lars

2011-02-07 00:00:53

by Felipe Contreras

[permalink] [raw]
Subject: Re: [PATCH 00/14] POWER: BQ27x00: New Properties, fixes, bq27000 support

On Mon, Feb 7, 2011 at 1:09 AM, Lars-Peter Clausen <[email protected]> wrote:
> On 02/06/2011 11:47 PM, Felipe Contreras wrote:
>> Hi,
>>
>> On Sun, Feb 6, 2011 at 2:47 AM, Lars-Peter Clausen <[email protected]> wrote:
>>> 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.
>>
>> This might be a bit off-topic, but I have tried to get the battery on
>> the Nokia N900 working, and so far I'm only able to query the
>> remaining capacity, but not really to enabling charging. This is very
>> annoying, as I can only hack a certain amount of time, and then I have
>> to reflash, reboot, and wait for the batter to charge with the
>> official system (if there's enough power to flash).
>>
>> Do you have any hints how to do that?
>
> Sorry, I've never worked with the N900, so I have no idea. But Pali has tested this
> patch series on his N900, so he might have an idea.

Well, it has a bq27200 AFAIK.

--
Felipe Contreras

2011-02-07 00:11:01

by Lars-Peter Clausen

[permalink] [raw]
Subject: Re: [PATCH 00/14] POWER: BQ27x00: New Properties, fixes, bq27000 support

On 02/07/2011 01:00 AM, Felipe Contreras wrote:
> On Mon, Feb 7, 2011 at 1:09 AM, Lars-Peter Clausen <[email protected]> wrote:
>> On 02/06/2011 11:47 PM, Felipe Contreras wrote:
>>> Hi,
>>>
>>> On Sun, Feb 6, 2011 at 2:47 AM, Lars-Peter Clausen <[email protected]> wrote:
>>>> 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.
>>>
>>> This might be a bit off-topic, but I have tried to get the battery on
>>> the Nokia N900 working, and so far I'm only able to query the
>>> remaining capacity, but not really to enabling charging. This is very
>>> annoying, as I can only hack a certain amount of time, and then I have
>>> to reflash, reboot, and wait for the batter to charge with the
>>> official system (if there's enough power to flash).
>>>
>>> Do you have any hints how to do that?
>>
>> Sorry, I've never worked with the N900, so I have no idea. But Pali has tested this
>> patch series on his N900, so he might have an idea.
>
> Well, it has a bq27200 AFAIK.
>

It has, but that doesn't tell you anything about the charging circuit.

- Lars

2011-02-07 00:58:33

by Grazvydas Ignotas

[permalink] [raw]
Subject: Re: [PATCH 00/14] POWER: BQ27x00: New Properties, fixes, bq27000 support

On Sun, Feb 6, 2011 at 2:47 AM, Lars-Peter Clausen <[email protected]> wrote:
> 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.

Trying this on pandora board (bq27500 over i2c) and it seems there is
something wrong with the cache, I'm always getting the old values. I'm
reading them manually over sysfs, uptime is over 25min but values
still match ones on boot, even after turning off the backlight.

Looking at the code delayed_work handling looks suspicious,
bq27x00_battery_get_property() flushes it and nothing ever reschedules
it. Other than that all new property values looks sane.

2011-02-07 01:28:30

by Lars-Peter Clausen

[permalink] [raw]
Subject: Re: [PATCH 00/14] POWER: BQ27x00: New Properties, fixes, bq27000 support

On 02/07/2011 01:58 AM, Grazvydas Ignotas wrote:
> On Sun, Feb 6, 2011 at 2:47 AM, Lars-Peter Clausen <[email protected]> wrote:
>> 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.
>
> Trying this on pandora board (bq27500 over i2c) and it seems there is
> something wrong with the cache, I'm always getting the old values. I'm
> reading them manually over sysfs, uptime is over 25min but values
> still match ones on boot, even after turning off the backlight.
>
> Looking at the code delayed_work handling looks suspicious,
> bq27x00_battery_get_property() flushes it and nothing ever reschedules
> it. Other than that all new property values looks sane.

Hi

Hm, I tought that rescheduling from withing the work function would work, even when
flushing the work, at least it did here.
But the current code is broken if poll_interval is 0 anyway, cause then it wont
reschedule itself and is never run again.

Could you try:

if (time_is_before_jiffies(di->last_update + 6 * HZ)) {
cancel_delayed_work_sync(&di->work);
bq27x00_battery_work(&di->work);
}

- Lars

2011-02-07 08:07:12

by Rodolfo Giometti

[permalink] [raw]
Subject: Re: [PATCH 02/14] POWER: bq27x00: Improve temperature precession

On Sun, Feb 06, 2011 at 01:47:59AM +0100, Lars-Peter Clausen wrote:
> 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 <[email protected]>
> ---
> 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

Acked-by: Rodolfo Giometti <[email protected]>

--

GNU/Linux Solutions e-mail: [email protected]
Linux Device Driver [email protected]
Embedded Systems phone: +39 349 2432127
UNIX programming skype: rodolfo.giometti
Freelance ICT Italia - Consulente ICT Italia - http://www.consulenti-ict.it

2011-02-07 08:07:51

by Rodolfo Giometti

[permalink] [raw]
Subject: Re: [PATCH 03/14] POWER: bq27x00: Fix current now property

On Sun, Feb 06, 2011 at 01:48:00AM +0100, Lars-Peter Clausen wrote:
> From: Pali Roh?r <[email protected]>
>
> According to the bq27000 datasheet to obtain the current value in Micro-ampere
> from the AI register it has to be transformed by the following formula:
> current = AI * 3570 / 20
>
> This patch adjust the drivers code accordingly.
>
> Signed-off-by: Pali Roh?r <[email protected]>
> Signed-off-by: Lars-Peter Clausen <[email protected]>
> ---
> 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
>

Acked-by: Rodolfo Giometti <[email protected]>

--

GNU/Linux Solutions e-mail: [email protected]
Linux Device Driver [email protected]
Embedded Systems phone: +39 349 2432127
UNIX programming skype: rodolfo.giometti
Freelance ICT Italia - Consulente ICT Italia - http://www.consulenti-ict.it

2011-02-07 08:10:40

by Rodolfo Giometti

[permalink] [raw]
Subject: Re: [PATCH 05/14] POWER: bq27x00: Prepare code for addition of bq27000 platform driver

On Sun, Feb 06, 2011 at 01:48:02AM +0100, Lars-Peter Clausen wrote:
> This patch simplifies the drivers data structure and moves code to be shared by
> both init functions into a common function.
> This patch has no functional changes, it only moves code around.
>
> Signed-off-by: Lars-Peter Clausen <[email protected]>
> ---
> 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..eee0120 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(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
>

Acked-by: Rodolfo Giometti <[email protected]>

--

GNU/Linux Solutions e-mail: [email protected]
Linux Device Driver [email protected]
Embedded Systems phone: +39 349 2432127
UNIX programming skype: rodolfo.giometti
Freelance ICT Italia - Consulente ICT Italia - http://www.consulenti-ict.it

2011-02-07 10:53:03

by Pali Rohár

[permalink] [raw]
Subject: Re: [PATCH 00/14] POWER: BQ27x00: New Properties, fixes, bq27000 support

2011/2/7 Lars-Peter Clausen <[email protected]>:
> On 02/06/2011 11:47 PM, Felipe Contreras wrote:
>> Hi,
>>
>> On Sun, Feb 6, 2011 at 2:47 AM, Lars-Peter Clausen <[email protected]> wrote:
>>> 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.
>>
>> This might be a bit off-topic, but I have tried to get the battery on
>> the Nokia N900 working, and so far I'm only able to query the
>> remaining capacity, but not really to enabling charging. This is very
>> annoying, as I can only hack a certain amount of time, and then I have
>> to reflash, reboot, and wait for the batter to charge with the
>> official system (if there's enough power to flash).
>>
>> Do you have any hints how to do that?
>>
>
> Hi
>
> Sorry, I've never worked with the N900, so I have no idea. But Pali has tested this
> patch series on his N900, so he might have an idea.
>
> - Lars
>

bq27x00 module does not support battery charging on Nokia N900. Only
can report battery state and some properties.

Battery Managment Entity (BME) is proprietary Nokia software which can
charge battery in Nokia N900. Alternative solution for charging
battery you can find on: http://enivax.net/jk/n900/charge21.sh.txt

--
Pali Rohár
[email protected]

2011-02-08 11:40:26

by Grazvydas Ignotas

[permalink] [raw]
Subject: Re: [PATCH 00/14] POWER: BQ27x00: New Properties, fixes, bq27000 support

On Mon, Feb 7, 2011 at 3:30 AM, Lars-Peter Clausen <[email protected]> wrote:
> On 02/07/2011 01:58 AM, Grazvydas Ignotas wrote:
>> On Sun, Feb 6, 2011 at 2:47 AM, Lars-Peter Clausen <[email protected]> wrote:
>>> 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.
>>
>> Trying this on pandora board (bq27500 over i2c) and it seems there is
>> something wrong with the cache, I'm always getting the old values. I'm
>> reading them manually over sysfs, uptime is over 25min but values
>> still match ones on boot, even after turning off the backlight.
>>
>> Looking at the code delayed_work handling looks suspicious,
>> bq27x00_battery_get_property() flushes it and nothing ever reschedules
>> it. Other than that all new property values looks sane.
>
> Hi
>
> Hm, I tought that rescheduling from withing the work function would work, even when
> flushing the work, at least it did here.
> But the current code is broken if poll_interval is 0 anyway, cause then it wont
> reschedule itself and is never run again.
>
> Could you try:
>
> if (time_is_before_jiffies(di->last_update + 6 * HZ)) {
> ? ? ? ?cancel_delayed_work_sync(&di->work);
> ? ? ? ?bq27x00_battery_work(&di->work);
> }

That works. However I wonder about the delay, I've experimentally
determined that bq27500 refreshes in about a second, so before this
series I could for example run 'watch -n 1 cat current_now' and see
current changes almost in realtime (when toggling backlight, for
example). Maybe we could have different cache timeout for different
chips or have quickly changing properties (like current and voltage)
bypass the cache.

2011-02-08 16:12:20

by Lars-Peter Clausen

[permalink] [raw]
Subject: Re: [PATCH 00/14] POWER: BQ27x00: New Properties, fixes, bq27000 support

On 02/08/2011 12:40 PM, Grazvydas Ignotas wrote:
> That works. However I wonder about the delay, I've experimentally
> determined that bq27500 refreshes in about a second, so before this
> series I could for example run 'watch -n 1 cat current_now' and see
> current changes almost in realtime (when toggling backlight, for
> example). Maybe we could have different cache timeout for different
> chips or have quickly changing properties (like current and voltage)
> bypass the cache.

Looking at the datasheet confirms that the bq27500 updates the current register
every second. The bq27000 updates it only every 5.12 seconds.
I think reducing the the cache time for bq27500 to 1 second would render the
cache mostly useless, but maybe it makes sense to bypass the cache for voltage
and current on the bq27500.

- Lars

2011-02-08 16:28:10

by Anton Vorontsov

[permalink] [raw]
Subject: Re: [PATCH 00/14] POWER: BQ27x00: New Properties, fixes, bq27000 support

On Tue, Feb 08, 2011 at 05:13:47PM +0100, Lars-Peter Clausen wrote:
> On 02/08/2011 12:40 PM, Grazvydas Ignotas wrote:
> > That works. However I wonder about the delay, I've experimentally
> > determined that bq27500 refreshes in about a second, so before this
> > series I could for example run 'watch -n 1 cat current_now' and see
> > current changes almost in realtime (when toggling backlight, for
> > example). Maybe we could have different cache timeout for different
> > chips or have quickly changing properties (like current and voltage)
> > bypass the cache.
>
> Looking at the datasheet confirms that the bq27500 updates the current register
> every second. The bq27000 updates it only every 5.12 seconds.
> I think reducing the the cache time for bq27500 to 1 second would render the
> cache mostly useless, but maybe it makes sense to bypass the cache for voltage
> and current on the bq27500.

Nope, it's not useless. I.e. for `cat /sys/class/power_supply/*/*`.
Actually, it was only the purpose for caching the battery values.

So, 1 second is OK.

p.s. Lars, on the next resend, can you please remove "POWER:"
prefix from the patch description line, bound the commit message
width to 76 symbols, and add all the collected Acked-by lines.
That way I can just pull your tree.

Thanks!

--
Anton Vorontsov
Email: [email protected]

2011-02-08 17:07:45

by Lars-Peter Clausen

[permalink] [raw]
Subject: Re: [PATCH 00/14] POWER: BQ27x00: New Properties, fixes, bq27000 support

On 02/08/2011 05:28 PM, Anton Vorontsov wrote:
> On Tue, Feb 08, 2011 at 05:13:47PM +0100, Lars-Peter Clausen wrote:
>> On 02/08/2011 12:40 PM, Grazvydas Ignotas wrote:
>>> That works. However I wonder about the delay, I've experimentally
>>> determined that bq27500 refreshes in about a second, so before this
>>> series I could for example run 'watch -n 1 cat current_now' and see
>>> current changes almost in realtime (when toggling backlight, for
>>> example). Maybe we could have different cache timeout for different
>>> chips or have quickly changing properties (like current and voltage)
>>> bypass the cache.
>>
>> Looking at the datasheet confirms that the bq27500 updates the current register
>> every second. The bq27000 updates it only every 5.12 seconds.
>> I think reducing the the cache time for bq27500 to 1 second would render the
>> cache mostly useless, but maybe it makes sense to bypass the cache for voltage
>> and current on the bq27500.
>
> Nope, it's not useless. I.e. for `cat /sys/class/power_supply/*/*`.
> Actually, it was only the purpose for caching the battery values.
>
> So, 1 second is OK.

Yes, the purpose of the register cache was that registers aren't read multiple
times if multiple properties are queried at once. Which is what I consider the
standard use case, since this is what a power monitor application would
normally do.
The other use case is constantly monitor a single property like CURRENT_NOW.
Since we would now read all properties instead of just one and would increase
the load on the bus by 12 times.

The other purpose of the register cache was to be able to tell whether
registers have changed when polling the battery. Since we already ignore the
*_NOW properties when comparing the the old and the new state it would make
sense to bypass the cache completely for them.
This would reduce the load when monitoring such a property and it would also
reduce the load when polling the battery since we would only read 8 instead of
12 registers.

>
> p.s. Lars, on the next resend, can you please remove "POWER:"
> prefix from the patch description line, bound the commit message
> width to 76 symbols, and add all the collected Acked-by lines.
> That way I can just pull your tree.

Sure.

- Lars