power: supply: bq27xxx: Fix poll_interval handling and races on remove
commit c00bc80462afc7963f449d7f21d896d2f629cacc upstream.
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: 8cfaaa8118
("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>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
This commit is contained in:
parent
e65fee4568
commit
d952a1eaaf
@ -1801,7 +1801,7 @@ static void bq27xxx_battery_update_unlocked(struct bq27xxx_device_info *di)
|
|||||||
|
|
||||||
di->last_update = jiffies;
|
di->last_update = jiffies;
|
||||||
|
|
||||||
if (poll_interval > 0)
|
if (!di->removed && poll_interval > 0)
|
||||||
mod_delayed_work(system_wq, &di->work, poll_interval * HZ);
|
mod_delayed_work(system_wq, &di->work, poll_interval * HZ);
|
||||||
}
|
}
|
||||||
|
|
||||||
@ -2132,22 +2132,18 @@ EXPORT_SYMBOL_GPL(bq27xxx_battery_setup);
|
|||||||
|
|
||||||
void bq27xxx_battery_teardown(struct bq27xxx_device_info *di)
|
void bq27xxx_battery_teardown(struct bq27xxx_device_info *di)
|
||||||
{
|
{
|
||||||
/*
|
|
||||||
* power_supply_unregister call bq27xxx_battery_get_property which
|
|
||||||
* call bq27xxx_battery_poll.
|
|
||||||
* Make sure that bq27xxx_battery_poll will not call
|
|
||||||
* schedule_delayed_work again after unregister (which cause OOPS).
|
|
||||||
*/
|
|
||||||
poll_interval = 0;
|
|
||||||
|
|
||||||
cancel_delayed_work_sync(&di->work);
|
|
||||||
|
|
||||||
power_supply_unregister(di->bat);
|
|
||||||
|
|
||||||
mutex_lock(&bq27xxx_list_lock);
|
mutex_lock(&bq27xxx_list_lock);
|
||||||
list_del(&di->list);
|
list_del(&di->list);
|
||||||
mutex_unlock(&bq27xxx_list_lock);
|
mutex_unlock(&bq27xxx_list_lock);
|
||||||
|
|
||||||
|
/* Set removed to avoid bq27xxx_battery_update() re-queuing the work */
|
||||||
|
mutex_lock(&di->lock);
|
||||||
|
di->removed = true;
|
||||||
|
mutex_unlock(&di->lock);
|
||||||
|
|
||||||
|
cancel_delayed_work_sync(&di->work);
|
||||||
|
|
||||||
|
power_supply_unregister(di->bat);
|
||||||
mutex_destroy(&di->lock);
|
mutex_destroy(&di->lock);
|
||||||
}
|
}
|
||||||
EXPORT_SYMBOL_GPL(bq27xxx_battery_teardown);
|
EXPORT_SYMBOL_GPL(bq27xxx_battery_teardown);
|
||||||
|
@ -68,6 +68,7 @@ struct bq27xxx_device_info {
|
|||||||
struct bq27xxx_access_methods bus;
|
struct bq27xxx_access_methods bus;
|
||||||
struct bq27xxx_reg_cache cache;
|
struct bq27xxx_reg_cache cache;
|
||||||
int charge_design_full;
|
int charge_design_full;
|
||||||
|
bool removed;
|
||||||
unsigned long last_update;
|
unsigned long last_update;
|
||||||
struct delayed_work work;
|
struct delayed_work work;
|
||||||
struct power_supply *bat;
|
struct power_supply *bat;
|
||||||
|
Loading…
Reference in New Issue
Block a user