| Message ID | 20221128092856.71619-2-hdegoede@redhat.com |
|---|---|
| State | Accepted |
| Commit | a7aaa80098d5b7608b2dc1e883e3c3f929415243 |
| Headers | show |
| Series | [v2,1/9] power: supply: bq25890: Ensure pump_express_work is cancelled on remove | expand |
Hi, On Mon, Nov 28, 2022 at 10:28:48AM +0100, Hans de Goede wrote: > The pump_express_work which gets queued from an external_power_changed > callback might be pending / running on remove() (or on probe failure). > > Add a devm action cancelling the work, to ensure that it is cancelled. > > Note the devm action is added before devm_power_supply_register(), making > it run after devm unregisters the power_supply, so that the work cannot > be queued anymore (this is also why a devm action is used for this). > > Fixes: 48f45b094dbb ("power: supply: bq25890: Support higher charging voltages through Pump Express+ protocol") > Reviewed-by: Marek Vasut <marex@denx.de> > Signed-off-by: Hans de Goede <hdegoede@redhat.com> > --- Thanks, queued. -- Sebastian > Changes in v2: > - Add comment that the devm_add_action() call must be done before > devm registering the power_supply, to guarantee it running after > the unregister > --- > drivers/power/supply/bq25890_charger.c | 15 +++++++++++++++ > 1 file changed, 15 insertions(+) > > diff --git a/drivers/power/supply/bq25890_charger.c b/drivers/power/supply/bq25890_charger.c > index 512c81662eea..866c475bb735 100644 > --- a/drivers/power/supply/bq25890_charger.c > +++ b/drivers/power/supply/bq25890_charger.c > @@ -1317,6 +1317,13 @@ static int bq25890_fw_probe(struct bq25890_device *bq) > return 0; > } > > +static void bq25890_non_devm_cleanup(void *data) > +{ > + struct bq25890_device *bq = data; > + > + cancel_delayed_work_sync(&bq->pump_express_work); > +} > + > static int bq25890_probe(struct i2c_client *client) > { > struct device *dev = &client->dev; > @@ -1372,6 +1379,14 @@ static int bq25890_probe(struct i2c_client *client) > /* OTG reporting */ > bq->usb_phy = devm_usb_get_phy(dev, USB_PHY_TYPE_USB2); > > + /* > + * This must be before bq25890_power_supply_init(), so that it runs > + * after devm unregisters the power_supply. > + */ > + ret = devm_add_action_or_reset(dev, bq25890_non_devm_cleanup, bq); > + if (ret) > + return ret; > + > ret = bq25890_register_regulator(bq); > if (ret) > return ret; > -- > 2.37.3 >
diff --git a/drivers/power/supply/bq25890_charger.c b/drivers/power/supply/bq25890_charger.c index 512c81662eea..866c475bb735 100644 --- a/drivers/power/supply/bq25890_charger.c +++ b/drivers/power/supply/bq25890_charger.c @@ -1317,6 +1317,13 @@ static int bq25890_fw_probe(struct bq25890_device *bq) return 0; } +static void bq25890_non_devm_cleanup(void *data) +{ + struct bq25890_device *bq = data; + + cancel_delayed_work_sync(&bq->pump_express_work); +} + static int bq25890_probe(struct i2c_client *client) { struct device *dev = &client->dev; @@ -1372,6 +1379,14 @@ static int bq25890_probe(struct i2c_client *client) /* OTG reporting */ bq->usb_phy = devm_usb_get_phy(dev, USB_PHY_TYPE_USB2); + /* + * This must be before bq25890_power_supply_init(), so that it runs + * after devm unregisters the power_supply. + */ + ret = devm_add_action_or_reset(dev, bq25890_non_devm_cleanup, bq); + if (ret) + return ret; + ret = bq25890_register_regulator(bq); if (ret) return ret;
The pump_express_work which gets queued from an external_power_changed callback might be pending / running on remove() (or on probe failure). Add a devm action cancelling the work, to ensure that it is cancelled. Note the devm action is added before devm_power_supply_register(), making it run after devm unregisters the power_supply, so that the work cannot be queued anymore (this is also why a devm action is used for this). Fixes: 48f45b094dbb ("power: supply: bq25890: Support higher charging voltages through Pump Express+ protocol") Reviewed-by: Marek Vasut <marex@denx.de> Signed-off-by: Hans de Goede <hdegoede@redhat.com> --- Changes in v2: - Add comment that the devm_add_action() call must be done before devm registering the power_supply, to guarantee it running after the unregister --- drivers/power/supply/bq25890_charger.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+)