diff mbox series

[v2,1/9] power: supply: bq25890: Ensure pump_express_work is cancelled on remove

Message ID 20221128092856.71619-2-hdegoede@redhat.com (mailing list archive)
State Handled Elsewhere, archived
Headers show
Series power: supply: bq25890: Fixes for 6.2 + further work for 6.3 | expand

Commit Message

Hans de Goede Nov. 28, 2022, 9:28 a.m. UTC
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(+)

Comments

Sebastian Reichel Dec. 3, 2022, 2:09 a.m. UTC | #1
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 mbox series

Patch

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;