Skip to content

Commit 5c34c0a

Browse files
jwrdegoedesre
authored andcommitted
power: supply: bq27xxx: Fix bq27xxx_battery_update() race condition
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: 740b755 ("bq27x00: Poll battery state") Signed-off-by: Hans de Goede <hdegoede@redhat.com> Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
1 parent e448464 commit 5c34c0a

1 file changed

Lines changed: 13 additions & 8 deletions

File tree

drivers/power/supply/bq27xxx_battery.c

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1761,7 +1761,7 @@ static int bq27xxx_battery_read_health(struct bq27xxx_device_info *di)
17611761
return POWER_SUPPLY_HEALTH_GOOD;
17621762
}
17631763

1764-
void bq27xxx_battery_update(struct bq27xxx_device_info *di)
1764+
static void bq27xxx_battery_update_unlocked(struct bq27xxx_device_info *di)
17651765
{
17661766
struct bq27xxx_reg_cache cache = {0, };
17671767
bool has_singe_flag = di->opts & BQ27XXX_O_ZERO;
@@ -1800,6 +1800,16 @@ void bq27xxx_battery_update(struct bq27xxx_device_info *di)
18001800
di->cache = cache;
18011801

18021802
di->last_update = jiffies;
1803+
1804+
if (poll_interval > 0)
1805+
mod_delayed_work(system_wq, &di->work, poll_interval * HZ);
1806+
}
1807+
1808+
void bq27xxx_battery_update(struct bq27xxx_device_info *di)
1809+
{
1810+
mutex_lock(&di->lock);
1811+
bq27xxx_battery_update_unlocked(di);
1812+
mutex_unlock(&di->lock);
18031813
}
18041814
EXPORT_SYMBOL_GPL(bq27xxx_battery_update);
18051815

@@ -1810,9 +1820,6 @@ static void bq27xxx_battery_poll(struct work_struct *work)
18101820
work.work);
18111821

18121822
bq27xxx_battery_update(di);
1813-
1814-
if (poll_interval > 0)
1815-
schedule_delayed_work(&di->work, poll_interval * HZ);
18161823
}
18171824

18181825
static bool bq27xxx_battery_is_full(struct bq27xxx_device_info *di, int flags)
@@ -1985,10 +1992,8 @@ static int bq27xxx_battery_get_property(struct power_supply *psy,
19851992
struct bq27xxx_device_info *di = power_supply_get_drvdata(psy);
19861993

19871994
mutex_lock(&di->lock);
1988-
if (time_is_before_jiffies(di->last_update + 5 * HZ)) {
1989-
cancel_delayed_work_sync(&di->work);
1990-
bq27xxx_battery_poll(&di->work.work);
1991-
}
1995+
if (time_is_before_jiffies(di->last_update + 5 * HZ))
1996+
bq27xxx_battery_update_unlocked(di);
19921997
mutex_unlock(&di->lock);
19931998

19941999
if (psp != POWER_SUPPLY_PROP_PRESENT && di->cache.flags < 0)

0 commit comments

Comments
 (0)