power: supply: bq27xxx: Fix bq27xxx_battery_update() race condition
commit 5c34c0aef185dcd10881847b9ebf20046aa77cb4 upstream.
bq27xxx_battery_update() assumes / requires that it is only run once,
not multiple times at the same time. But there are 3 possible callers:
1. bq27xxx_battery_poll() delayed_work item handler
2. bq27xxx_battery_irq_handler_thread() I2C IRQ handler
3. bq27xxx_battery_setup()
And there is no protection against these racing with each other,
fix this race condition by making all callers take di->lock:
- Rename bq27xxx_battery_update() to bq27xxx_battery_update_unlocked()
- Add new bq27xxx_battery_update() which takes di->lock and then calls
bq27xxx_battery_update_unlocked()
- Make stale cache check code in bq27xxx_battery_get_property(), which
already takes di->lock directly to check the jiffies, call
bq27xxx_battery_update_unlocked() instead of messing with
the delayed_work item
- Make bq27xxx_battery_update_unlocked() mod the delayed-work item
so that the next poll is delayed to poll_interval milliseconds after
the last update independent of the source of the update
Fixes: 740b755a3b
("bq27x00: Poll battery state")
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
This commit is contained in:
parent
96bfafbc7d
commit
7b3b119649
@ -1547,7 +1547,7 @@ static int bq27xxx_battery_read_health(struct bq27xxx_device_info *di)
|
|||||||
return POWER_SUPPLY_HEALTH_GOOD;
|
return POWER_SUPPLY_HEALTH_GOOD;
|
||||||
}
|
}
|
||||||
|
|
||||||
void bq27xxx_battery_update(struct bq27xxx_device_info *di)
|
static void bq27xxx_battery_update_unlocked(struct bq27xxx_device_info *di)
|
||||||
{
|
{
|
||||||
struct bq27xxx_reg_cache cache = {0, };
|
struct bq27xxx_reg_cache cache = {0, };
|
||||||
bool has_ci_flag = di->opts & BQ27XXX_O_ZERO;
|
bool has_ci_flag = di->opts & BQ27XXX_O_ZERO;
|
||||||
@ -1597,6 +1597,16 @@ void bq27xxx_battery_update(struct bq27xxx_device_info *di)
|
|||||||
di->cache = cache;
|
di->cache = cache;
|
||||||
|
|
||||||
di->last_update = jiffies;
|
di->last_update = jiffies;
|
||||||
|
|
||||||
|
if (poll_interval > 0)
|
||||||
|
mod_delayed_work(system_wq, &di->work, poll_interval * HZ);
|
||||||
|
}
|
||||||
|
|
||||||
|
void bq27xxx_battery_update(struct bq27xxx_device_info *di)
|
||||||
|
{
|
||||||
|
mutex_lock(&di->lock);
|
||||||
|
bq27xxx_battery_update_unlocked(di);
|
||||||
|
mutex_unlock(&di->lock);
|
||||||
}
|
}
|
||||||
EXPORT_SYMBOL_GPL(bq27xxx_battery_update);
|
EXPORT_SYMBOL_GPL(bq27xxx_battery_update);
|
||||||
|
|
||||||
@ -1607,9 +1617,6 @@ static void bq27xxx_battery_poll(struct work_struct *work)
|
|||||||
work.work);
|
work.work);
|
||||||
|
|
||||||
bq27xxx_battery_update(di);
|
bq27xxx_battery_update(di);
|
||||||
|
|
||||||
if (poll_interval > 0)
|
|
||||||
schedule_delayed_work(&di->work, poll_interval * HZ);
|
|
||||||
}
|
}
|
||||||
|
|
||||||
/*
|
/*
|
||||||
@ -1770,10 +1777,8 @@ static int bq27xxx_battery_get_property(struct power_supply *psy,
|
|||||||
struct bq27xxx_device_info *di = power_supply_get_drvdata(psy);
|
struct bq27xxx_device_info *di = power_supply_get_drvdata(psy);
|
||||||
|
|
||||||
mutex_lock(&di->lock);
|
mutex_lock(&di->lock);
|
||||||
if (time_is_before_jiffies(di->last_update + 5 * HZ)) {
|
if (time_is_before_jiffies(di->last_update + 5 * HZ))
|
||||||
cancel_delayed_work_sync(&di->work);
|
bq27xxx_battery_update_unlocked(di);
|
||||||
bq27xxx_battery_poll(&di->work.work);
|
|
||||||
}
|
|
||||||
mutex_unlock(&di->lock);
|
mutex_unlock(&di->lock);
|
||||||
|
|
||||||
if (psp != POWER_SUPPLY_PROP_PRESENT && di->cache.flags < 0)
|
if (psp != POWER_SUPPLY_PROP_PRESENT && di->cache.flags < 0)
|
||||||
|
Loading…
Reference in New Issue
Block a user