Skip to content

Commit c00bc80

Browse files
jwrdegoedesre
authored andcommitted
power: supply: bq27xxx: Fix poll_interval handling and races on remove
Before this patch bq27xxx_battery_teardown() was setting poll_interval = 0 to avoid bq27xxx_battery_update() requeuing the delayed_work item. There are 2 problems with this: 1. If the driver is unbound through sysfs, rather then the module being rmmod-ed, this changes poll_interval unexpectedly 2. This is racy, after it being set poll_interval could be changed before bq27xxx_battery_update() checks it through /sys/module/bq27xxx_battery/parameters/poll_interval Fix this by added a removed attribute to struct bq27xxx_device_info and using that instead of setting poll_interval to 0. There also is another poll_interval related race on remove(), writing /sys/module/bq27xxx_battery/parameters/poll_interval will requeue the delayed_work item for all devices on the bq27xxx_battery_devices list and the device being removed was only removed from that list after cancelling the delayed_work item. Fix this by moving the removal from the bq27xxx_battery_devices list to before cancelling the delayed_work item. Fixes: 8cfaaa8 ("bq27x00_battery: Fix OOPS caused by unregistring bq27x00 driver") Signed-off-by: Hans de Goede <hdegoede@redhat.com> Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
1 parent 444ff00 commit c00bc80

2 files changed

Lines changed: 10 additions & 13 deletions

File tree

drivers/power/supply/bq27xxx_battery.c

Lines changed: 9 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1801,7 +1801,7 @@ static void bq27xxx_battery_update_unlocked(struct bq27xxx_device_info *di)
18011801

18021802
di->last_update = jiffies;
18031803

1804-
if (poll_interval > 0)
1804+
if (!di->removed && poll_interval > 0)
18051805
mod_delayed_work(system_wq, &di->work, poll_interval * HZ);
18061806
}
18071807

@@ -2132,22 +2132,18 @@ EXPORT_SYMBOL_GPL(bq27xxx_battery_setup);
21322132

21332133
void bq27xxx_battery_teardown(struct bq27xxx_device_info *di)
21342134
{
2135-
/*
2136-
* power_supply_unregister call bq27xxx_battery_get_property which
2137-
* call bq27xxx_battery_poll.
2138-
* Make sure that bq27xxx_battery_poll will not call
2139-
* schedule_delayed_work again after unregister (which cause OOPS).
2140-
*/
2141-
poll_interval = 0;
2142-
2143-
cancel_delayed_work_sync(&di->work);
2144-
2145-
power_supply_unregister(di->bat);
2146-
21472135
mutex_lock(&bq27xxx_list_lock);
21482136
list_del(&di->list);
21492137
mutex_unlock(&bq27xxx_list_lock);
21502138

2139+
/* Set removed to avoid bq27xxx_battery_update() re-queuing the work */
2140+
mutex_lock(&di->lock);
2141+
di->removed = true;
2142+
mutex_unlock(&di->lock);
2143+
2144+
cancel_delayed_work_sync(&di->work);
2145+
2146+
power_supply_unregister(di->bat);
21512147
mutex_destroy(&di->lock);
21522148
}
21532149
EXPORT_SYMBOL_GPL(bq27xxx_battery_teardown);

include/linux/power/bq27xxx_battery.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,7 @@ struct bq27xxx_device_info {
6868
struct bq27xxx_access_methods bus;
6969
struct bq27xxx_reg_cache cache;
7070
int charge_design_full;
71+
bool removed;
7172
unsigned long last_update;
7273
struct delayed_work work;
7374
struct power_supply *bat;

0 commit comments

Comments
 (0)