Re: [PATCH 1/4] power: supply: axp288_charger: Do not cancel work before initializing it

[Hans de Goede <hansg@kernel.org>]

Hi,

On 20-Feb-26 18:49, Krzysztof Kozlowski wrote:
> Driver registered devm handler to cancel_work_sync() before even the
> work was initialized, thus leading to possible warning from
> kernel/workqueue.c on (!work->func) check, if the error path was hit
> before the initialization happened.
> 
> Use devm_work_autocancel() on each work item independently, which
> handles the initialization and handler to cancel work.
> 
> Fixes: 165c2357744e ("power: supply: axp288_charger: Properly stop work on probe-error / remove")
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>

Thanks, patch looks good to me:

Reviewed-by: Hans de Goede <johannes.goede@oss.qualcomm.com>

Regards,

Hans



> ---
>  drivers/power/supply/axp288_charger.c | 19 ++++++++-----------
>  1 file changed, 8 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/power/supply/axp288_charger.c b/drivers/power/supply/axp288_charger.c
> index ac05942e4e6a..ca52c2c82b2c 100644
> --- a/drivers/power/supply/axp288_charger.c
> +++ b/drivers/power/supply/axp288_charger.c
> @@ -10,6 +10,7 @@
>  #include <linux/acpi.h>
>  #include <linux/bitops.h>
>  #include <linux/module.h>
> +#include <linux/devm-helpers.h>
>  #include <linux/device.h>
>  #include <linux/regmap.h>
>  #include <linux/workqueue.h>
> @@ -821,14 +822,6 @@ static int charger_init_hw_regs(struct axp288_chrg_info *info)
>  	return 0;
>  }
>  
> -static void axp288_charger_cancel_work(void *data)
> -{
> -	struct axp288_chrg_info *info = data;
> -
> -	cancel_work_sync(&info->otg.work);
> -	cancel_work_sync(&info->cable.work);
> -}
> -
>  static int axp288_charger_probe(struct platform_device *pdev)
>  {
>  	int ret, i, pirq;
> @@ -911,12 +904,12 @@ static int axp288_charger_probe(struct platform_device *pdev)
>  	}
>  
>  	/* Cancel our work on cleanup, register this before the notifiers */
> -	ret = devm_add_action(dev, axp288_charger_cancel_work, info);
> +	ret = devm_work_autocancel(dev, &info->cable.work,
> +				   axp288_charger_extcon_evt_worker);
>  	if (ret)
>  		return ret;
>  
>  	/* Register for extcon notification */
> -	INIT_WORK(&info->cable.work, axp288_charger_extcon_evt_worker);
>  	info->cable.nb.notifier_call = axp288_charger_handle_cable_evt;
>  	ret = devm_extcon_register_notifier_all(dev, info->cable.edev,
>  						&info->cable.nb);
> @@ -926,8 +919,12 @@ static int axp288_charger_probe(struct platform_device *pdev)
>  	}
>  	schedule_work(&info->cable.work);
>  
> +	ret = devm_work_autocancel(dev, &info->otg.work,
> +				   axp288_charger_otg_evt_worker);
> +	if (ret)
> +		return ret;
> +
>  	/* Register for OTG notification */
> -	INIT_WORK(&info->otg.work, axp288_charger_otg_evt_worker);
>  	info->otg.id_nb.notifier_call = axp288_charger_handle_otg_evt;
>  	if (info->otg.cable) {
>  		ret = devm_extcon_register_notifier(dev, info->otg.cable,