From patchwork Tue Jan 29 14:29:54 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: [1/1] power: bq27541 driver - serialize get_property Date: Tue, 29 Jan 2013 04:29:54 -0000 From: Alex Hornung X-Patchwork-Id: 216566 Message-Id: <39a5ee5b03a884334640e9c65bd8ec7a@alexhornung.com> To: Hi, this patch fixes bug 1093543[1]. Explanation is further down, as per commit message. It is not obvious to me what your upstream repository (that includes the bq27541 driver) is. The MAINTAINERS file also includes no information on this file. I would suspect it's android kernel/tegra[2], but I'm not sure since it only contains release branches (and an empty master branch). If it is, which one is the branch you based your master branch on? If not, which one is it? I'd like to get the patch upstream. * Currently, get_property() and its callees are not thread-safe, since they share some global state, which they access without serialization or guaranteed atomicity. * get_property() can be called in a reentrant fashion from either several concurrent sysfs accesses, or a sysfs access concurrently with an access from power_supply_core.c, which in turn are triggered from a work queue in the driver itself. * This fixes bogus readings affecting the capacity and the charge_now values - possibly others as well. Cheers, Alex [1]: https://bugs.launchpad.net/ubuntu-nexus7/+bug/1093543 [2]: https://android.googlesource.com/kernel/tegra/ >From 0164dc98a311137bdda9c39dc20cfcb89079ac12 Mon Sep 17 00:00:00 2001 From: Alex Hornung Date: Mon, 28 Jan 2013 20:43:08 +0000 Subject: [PATCH 1/1] power: bq27541 driver - serialize get_property * Currently, get_property() and its callees are not thread-safe, since they share some global state, which they access without serialization or guaranteed atomicity. * get_property() can be called in a reentrant fashion from either several concurrent sysfs accesses, or a sysfs access concurrently with an access from power_supply_core.c, which in turn are triggered from a work queue in the driver itself. * This fixes bogus readings affecting the capacity and the charge_now values - possibly others as well. Signed-off-by: Alex Hornung Acked-by: Andy Whitcroft Acked-by: Leann Ogasawara --- drivers/power/bq27541_battery.c | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/drivers/power/bq27541_battery.c b/drivers/power/bq27541_battery.c index 9a86cc7..9ce5c2d 100755 --- a/drivers/power/bq27541_battery.c +++ b/drivers/power/bq27541_battery.c @@ -34,6 +34,7 @@ #include #include #include +#include #include #include #include @@ -265,7 +266,7 @@ static struct bq27541_device_info { unsigned int old_temperature; unsigned int temp_err; unsigned int prj_id; - spinlock_t lock; + struct mutex lock; } *bq27541_device; static int bq27541_read_i2c(u8 reg, int *rt_value, int b_single) @@ -689,6 +690,9 @@ static int bq27541_get_property(struct power_supply *psy, union power_supply_propval *val) { u8 count; + + mutex_lock(&bq27541_device->lock); + switch (psp) { case POWER_SUPPLY_PROP_PRESENT: case POWER_SUPPLY_PROP_HEALTH: @@ -717,19 +721,21 @@ static int bq27541_get_property(struct power_supply *psy, } if (bq27541_get_psp(count, psp, val)) - return -EINVAL; + goto error; break; default: dev_err(&bq27541_device->client->dev, "%s: INVALID property psp=%u\n", __func__,psp); - return -EINVAL; + goto error; } + mutex_unlock(&bq27541_device->lock); return 0; error: + mutex_unlock(&bq27541_device->lock); return -EINVAL; } @@ -757,6 +763,8 @@ static int bq27541_probe(struct i2c_client *client, bq27541_device->shutdown_disable = 1; bq27541_device->cap_zero_count = 0; + mutex_init(&bq27541_device->lock); + for (i = 0; i < ARRAY_SIZE(bq27541_supply); i++) { ret = power_supply_register(&client->dev, &bq27541_supply[i]); if (ret) { @@ -775,7 +783,6 @@ static int bq27541_probe(struct i2c_client *client, INIT_DELAYED_WORK(&bq27541_device->shutdown_en_work, shutdown_enable_set); cancel_delayed_work(&bq27541_device->status_poll_work); - spin_lock_init(&bq27541_device->lock); wake_lock_init(&bq27541_device->low_battery_wake_lock, WAKE_LOCK_SUSPEND, "low_battery_detection"); wake_lock_init(&bq27541_device->cable_wake_lock, WAKE_LOCK_SUSPEND, "cable_state_changed");