Re: [PATCH 02/10] power: supply: bq25890: Ensure pump_express_work is cancelled on remove

From: Sebastian Reichel <sebastian.reichel@collabora.com>
To: Hans de Goede <hdegoede@redhat.com>
Cc: Marek Vasut <marex@denx.de>, linux-pm@vger.kernel.org
Subject: Re: [PATCH 02/10] power: supply: bq25890: Ensure pump_express_work is cancelled on remove
Date: Mon, 28 Nov 2022 00:17:16 +0100	[thread overview]
Message-ID: <20221127231716.dtt5nrpvv7dheeqd@mercury.elektranox.org> (raw)
In-Reply-To: <20221127180233.103678-3-hdegoede@redhat.com>

[-- Attachment #1: Type: text/plain, Size: 1971 bytes --]

Hi,

On Sun, Nov 27, 2022 at 07:02:25PM +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")
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  drivers/power/supply/bq25890_charger.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/drivers/power/supply/bq25890_charger.c b/drivers/power/supply/bq25890_charger.c
> index 512c81662eea..30d77afab839 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,10 @@ static int bq25890_probe(struct i2c_client *client)
>  	/* OTG reporting */
>  	bq->usb_phy = devm_usb_get_phy(dev, USB_PHY_TYPE_USB2);
>  
> +	ret = devm_add_action_or_reset(dev, bq25890_non_devm_cleanup, bq);
> +	if (ret)
> +		return ret;

ret = devm_delayed_work_autocancel(dev, &bq->pump_express_work, bq25890_pump_express_work);
if (ret)
    return ret;

(+ removing the INIT_DELAYED_WORK)

-- Sebastian

> +
>  	ret = bq25890_register_regulator(bq);
>  	if (ret)
>  		return ret;
> -- 
> 2.38.1
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]