From: Lars-Peter Clausen <lars@metafoo.de>
To: Anton Vorontsov <cbouatmailru@gmail.com>
Cc: Rodolfo Giometti <giometti@linux.it>,
Grazvydas Ignotas <notasas@gmail.com>,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 07/14] bq27X00: Cache battery registers
Date: Sat, 12 Feb 2011 20:52:43 +0100 [thread overview]
Message-ID: <4D56E50B.4000602@metafoo.de> (raw)
In-Reply-To: <1297539554-13957-9-git-send-email-lars@metafoo.de>
-----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-----
next prev parent reply other threads:[~2011-02-12 19:53 UTC|newest]
Thread overview: 28+ messages / expand[flat|nested] mbox.gz Atom feed top
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 [this message]
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
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4D56E50B.4000602@metafoo.de \
--to=lars@metafoo.de \
--cc=cbouatmailru@gmail.com \
--cc=giometti@linux.it \
--cc=linux-kernel@vger.kernel.org \
--cc=notasas@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox