public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/4] power: supply: axp288_charger: Do not cancel work before initializing it
@ 2026-02-20 17:49 Krzysztof Kozlowski
  2026-02-20 17:49 ` [PATCH 2/4] power: supply: axp288_charger: Simplify returns of dev_err_probe() Krzysztof Kozlowski
                   ` (5 more replies)
  0 siblings, 6 replies; 11+ messages in thread
From: Krzysztof Kozlowski @ 2026-02-20 17:49 UTC (permalink / raw)
  To: Sebastian Reichel, Hans de Goede, Chen-Yu Tsai, linux-pm,
	linux-kernel
  Cc: Krzysztof Kozlowski, stable

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>
---
 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,
-- 
2.51.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 2/4] power: supply: axp288_charger: Simplify returns of dev_err_probe()
  2026-02-20 17:49 [PATCH 1/4] power: supply: axp288_charger: Do not cancel work before initializing it Krzysztof Kozlowski
@ 2026-02-20 17:49 ` Krzysztof Kozlowski
  2026-02-21  5:05   ` Chen-Yu Tsai
  2026-02-21 15:19   ` Hans de Goede
  2026-02-20 17:49 ` [PATCH 3/4] power: supply: bq24190: Avoid rescheduling after cancelling work Krzysztof Kozlowski
                   ` (4 subsequent siblings)
  5 siblings, 2 replies; 11+ messages in thread
From: Krzysztof Kozlowski @ 2026-02-20 17:49 UTC (permalink / raw)
  To: Sebastian Reichel, Hans de Goede, Chen-Yu Tsai, linux-pm,
	linux-kernel
  Cc: Krzysztof Kozlowski

One of benefits of dev_err_probe() is that it returns the error value
greatly simplifying the error paths (e.g. three lines -> one line).

Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>

---

Context depends on previous patch
---
 drivers/power/supply/axp288_charger.c | 52 ++++++++++++---------------
 1 file changed, 22 insertions(+), 30 deletions(-)

diff --git a/drivers/power/supply/axp288_charger.c b/drivers/power/supply/axp288_charger.c
index ca52c2c82b2c..ea0f5caee8f0 100644
--- a/drivers/power/supply/axp288_charger.c
+++ b/drivers/power/supply/axp288_charger.c
@@ -859,12 +859,10 @@ static int axp288_charger_probe(struct platform_device *pdev)
 	info->regmap_irqc = axp20x->regmap_irqc;
 
 	info->cable.edev = extcon_get_extcon_dev(AXP288_EXTCON_DEV_NAME);
-	if (IS_ERR(info->cable.edev)) {
-		dev_err_probe(dev, PTR_ERR(info->cable.edev),
-			      "extcon_get_extcon_dev(%s) failed\n",
-			      AXP288_EXTCON_DEV_NAME);
-		return PTR_ERR(info->cable.edev);
-	}
+	if (IS_ERR(info->cable.edev))
+		return dev_err_probe(dev, PTR_ERR(info->cable.edev),
+				     "extcon_get_extcon_dev(%s) failed\n",
+				     AXP288_EXTCON_DEV_NAME);
 
 	/*
 	 * On devices with broken ACPI GPIO event handlers there also is no ACPI
@@ -878,12 +876,11 @@ static int axp288_charger_probe(struct platform_device *pdev)
 
 	if (extcon_name) {
 		info->otg.cable = extcon_get_extcon_dev(extcon_name);
-		if (IS_ERR(info->otg.cable)) {
-			dev_err_probe(dev, PTR_ERR(info->otg.cable),
-				      "extcon_get_extcon_dev(%s) failed\n",
-				      USB_HOST_EXTCON_NAME);
-			return PTR_ERR(info->otg.cable);
-		}
+		if (IS_ERR(info->otg.cable))
+			return dev_err_probe(dev, PTR_ERR(info->otg.cable),
+					     "extcon_get_extcon_dev(%s) failed\n",
+					     USB_HOST_EXTCON_NAME);
+
 		dev_info(dev, "Using " USB_HOST_EXTCON_HID " extcon for usb-id\n");
 	}
 
@@ -897,11 +894,9 @@ static int axp288_charger_probe(struct platform_device *pdev)
 	charger_cfg.drv_data = info;
 	info->psy_usb = devm_power_supply_register(dev, &axp288_charger_desc,
 						   &charger_cfg);
-	if (IS_ERR(info->psy_usb)) {
-		ret = PTR_ERR(info->psy_usb);
-		dev_err(dev, "failed to register power supply: %d\n", ret);
-		return ret;
-	}
+	if (IS_ERR(info->psy_usb))
+		return dev_err_probe(dev, PTR_ERR(info->psy_usb),
+				     "failed to register power supply: %d\n", ret);
 
 	/* Cancel our work on cleanup, register this before the notifiers */
 	ret = devm_work_autocancel(dev, &info->cable.work,
@@ -913,10 +908,9 @@ static int axp288_charger_probe(struct platform_device *pdev)
 	info->cable.nb.notifier_call = axp288_charger_handle_cable_evt;
 	ret = devm_extcon_register_notifier_all(dev, info->cable.edev,
 						&info->cable.nb);
-	if (ret) {
-		dev_err(dev, "failed to register cable extcon notifier\n");
-		return ret;
-	}
+	if (ret)
+		return dev_err_probe(dev, ret, "failed to register cable extcon notifier\n");
+
 	schedule_work(&info->cable.work);
 
 	ret = devm_work_autocancel(dev, &info->otg.work,
@@ -929,10 +923,10 @@ static int axp288_charger_probe(struct platform_device *pdev)
 	if (info->otg.cable) {
 		ret = devm_extcon_register_notifier(dev, info->otg.cable,
 					EXTCON_USB_HOST, &info->otg.id_nb);
-		if (ret) {
-			dev_err(dev, "failed to register EXTCON_USB_HOST notifier\n");
-			return ret;
-		}
+		if (ret)
+			return dev_err_probe(dev, ret,
+					     "failed to register EXTCON_USB_HOST notifier\n");
+
 		schedule_work(&info->otg.work);
 	}
 
@@ -951,11 +945,9 @@ static int axp288_charger_probe(struct platform_device *pdev)
 		ret = devm_request_threaded_irq(&info->pdev->dev, info->irq[i],
 					NULL, axp288_charger_irq_thread_handler,
 					IRQF_ONESHOT, info->pdev->name, info);
-		if (ret) {
-			dev_err(dev, "failed to request interrupt=%d\n",
-								info->irq[i]);
-			return ret;
-		}
+		if (ret)
+			return dev_err_probe(dev, ret, "failed to request interrupt=%d\n",
+					     info->irq[i]);
 	}
 
 	return 0;
-- 
2.51.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 3/4] power: supply: bq24190: Avoid rescheduling after cancelling work
  2026-02-20 17:49 [PATCH 1/4] power: supply: axp288_charger: Do not cancel work before initializing it Krzysztof Kozlowski
  2026-02-20 17:49 ` [PATCH 2/4] power: supply: axp288_charger: Simplify returns of dev_err_probe() Krzysztof Kozlowski
@ 2026-02-20 17:49 ` Krzysztof Kozlowski
  2026-02-21 15:19   ` Hans de Goede
  2026-02-20 17:49 ` [PATCH 4/4] power: supply: twl4030_madc: Drop unused header includes Krzysztof Kozlowski
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 11+ messages in thread
From: Krzysztof Kozlowski @ 2026-02-20 17:49 UTC (permalink / raw)
  To: Sebastian Reichel, Hans de Goede, Chen-Yu Tsai, linux-pm,
	linux-kernel
  Cc: Krzysztof Kozlowski

Driver initializes delayed work and then registers interrupt handler
with devm interface.  This means that device removal will not use a
reversed order, but first cancel pending work items and then, via devm
release handlers, free the interrupt.

The interrupt handler does not directly use/schedule work
items on the workqueue, however it updates the status of the battery
charger which might lead to calling power_supply_changed() and trigger
chain of calls leading to scheduling the work items.  If this happens
during short time window after cancel_delayed_work_sync() in remove()
callback, the work would be rescheduled.

Avoid this by using devm interface to initialize and cancel work item,
thus having exactly reverse order during remove() in respect to rest of
the probe/cleanup paths.  This is also more logical and readable code.

Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
---
 drivers/power/supply/bq24190_charger.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/power/supply/bq24190_charger.c b/drivers/power/supply/bq24190_charger.c
index ed0ceae8d90b..55da91bacc3e 100644
--- a/drivers/power/supply/bq24190_charger.c
+++ b/drivers/power/supply/bq24190_charger.c
@@ -9,6 +9,7 @@
 #include <linux/module.h>
 #include <linux/interrupt.h>
 #include <linux/delay.h>
+#include <linux/devm-helpers.h>
 #include <linux/pm_runtime.h>
 #include <linux/power_supply.h>
 #include <linux/power/bq24190_charger.h>
@@ -2087,8 +2088,11 @@ static int bq24190_probe(struct i2c_client *client)
 	bdi->charge_type = POWER_SUPPLY_CHARGE_TYPE_FAST;
 	bdi->f_reg = 0;
 	bdi->ss_reg = BQ24190_REG_SS_VBUS_STAT_MASK; /* impossible state */
-	INIT_DELAYED_WORK(&bdi->input_current_limit_work,
-			  bq24190_input_current_limit_work);
+
+	ret = devm_delayed_work_autocancel(dev, &bdi->input_current_limit_work,
+					   bq24190_input_current_limit_work);
+	if (ret)
+		return ret;
 
 	i2c_set_clientdata(client, bdi);
 
@@ -2198,7 +2202,6 @@ static void bq24190_remove(struct i2c_client *client)
 	struct bq24190_dev_info *bdi = i2c_get_clientdata(client);
 	int error;
 
-	cancel_delayed_work_sync(&bdi->input_current_limit_work);
 	error = pm_runtime_resume_and_get(bdi->dev);
 	if (error < 0)
 		dev_warn(bdi->dev, "pm_runtime_get failed: %i\n", error);
-- 
2.51.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 4/4] power: supply: twl4030_madc: Drop unused header includes
  2026-02-20 17:49 [PATCH 1/4] power: supply: axp288_charger: Do not cancel work before initializing it Krzysztof Kozlowski
  2026-02-20 17:49 ` [PATCH 2/4] power: supply: axp288_charger: Simplify returns of dev_err_probe() Krzysztof Kozlowski
  2026-02-20 17:49 ` [PATCH 3/4] power: supply: bq24190: Avoid rescheduling after cancelling work Krzysztof Kozlowski
@ 2026-02-20 17:49 ` Krzysztof Kozlowski
  2026-02-21 15:19   ` Hans de Goede
  2026-02-21  5:08 ` [PATCH 1/4] power: supply: axp288_charger: Do not cancel work before initializing it Chen-Yu Tsai
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 11+ messages in thread
From: Krzysztof Kozlowski @ 2026-02-20 17:49 UTC (permalink / raw)
  To: Sebastian Reichel, Hans de Goede, Chen-Yu Tsai, linux-pm,
	linux-kernel
  Cc: Krzysztof Kozlowski

Driver does not use any code from workqueue.h and param.h.

Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
---
 drivers/power/supply/twl4030_madc_battery.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/power/supply/twl4030_madc_battery.c b/drivers/power/supply/twl4030_madc_battery.c
index 3935162e350b..a99b3ff26929 100644
--- a/drivers/power/supply/twl4030_madc_battery.c
+++ b/drivers/power/supply/twl4030_madc_battery.c
@@ -11,9 +11,7 @@
  */
 
 #include <linux/module.h>
-#include <linux/param.h>
 #include <linux/delay.h>
-#include <linux/workqueue.h>
 #include <linux/platform_device.h>
 #include <linux/power_supply.h>
 #include <linux/slab.h>
-- 
2.51.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH 2/4] power: supply: axp288_charger: Simplify returns of dev_err_probe()
  2026-02-20 17:49 ` [PATCH 2/4] power: supply: axp288_charger: Simplify returns of dev_err_probe() Krzysztof Kozlowski
@ 2026-02-21  5:05   ` Chen-Yu Tsai
  2026-02-21 15:19   ` Hans de Goede
  1 sibling, 0 replies; 11+ messages in thread
From: Chen-Yu Tsai @ 2026-02-21  5:05 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Sebastian Reichel, Hans de Goede, linux-pm, linux-kernel

On Sat, Feb 21, 2026 at 1:49 AM Krzysztof Kozlowski
<krzysztof.kozlowski@oss.qualcomm.com> wrote:
>
> One of benefits of dev_err_probe() is that it returns the error value
> greatly simplifying the error paths (e.g. three lines -> one line).
>
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>

Reviewed-by: Chen-Yu Tsai <wens@kernel.org>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/4] power: supply: axp288_charger: Do not cancel work before initializing it
  2026-02-20 17:49 [PATCH 1/4] power: supply: axp288_charger: Do not cancel work before initializing it Krzysztof Kozlowski
                   ` (2 preceding siblings ...)
  2026-02-20 17:49 ` [PATCH 4/4] power: supply: twl4030_madc: Drop unused header includes Krzysztof Kozlowski
@ 2026-02-21  5:08 ` Chen-Yu Tsai
  2026-02-21 15:17 ` Hans de Goede
  2026-03-03  0:01 ` Sebastian Reichel
  5 siblings, 0 replies; 11+ messages in thread
From: Chen-Yu Tsai @ 2026-02-21  5:08 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Sebastian Reichel, Hans de Goede, linux-pm, linux-kernel, stable

On Sat, Feb 21, 2026 at 1:49 AM Krzysztof Kozlowski
<krzysztof.kozlowski@oss.qualcomm.com> 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>

Reviewed-by: Chen-Yu Tsai <wens@kernel.org>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/4] power: supply: axp288_charger: Do not cancel work before initializing it
  2026-02-20 17:49 [PATCH 1/4] power: supply: axp288_charger: Do not cancel work before initializing it Krzysztof Kozlowski
                   ` (3 preceding siblings ...)
  2026-02-21  5:08 ` [PATCH 1/4] power: supply: axp288_charger: Do not cancel work before initializing it Chen-Yu Tsai
@ 2026-02-21 15:17 ` Hans de Goede
  2026-03-03  0:01 ` Sebastian Reichel
  5 siblings, 0 replies; 11+ messages in thread
From: Hans de Goede @ 2026-02-21 15:17 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Sebastian Reichel, Chen-Yu Tsai, linux-pm,
	linux-kernel
  Cc: stable

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,


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 2/4] power: supply: axp288_charger: Simplify returns of dev_err_probe()
  2026-02-20 17:49 ` [PATCH 2/4] power: supply: axp288_charger: Simplify returns of dev_err_probe() Krzysztof Kozlowski
  2026-02-21  5:05   ` Chen-Yu Tsai
@ 2026-02-21 15:19   ` Hans de Goede
  1 sibling, 0 replies; 11+ messages in thread
From: Hans de Goede @ 2026-02-21 15:19 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Sebastian Reichel, Chen-Yu Tsai, linux-pm,
	linux-kernel

Hi,

On 20-Feb-26 18:49, Krzysztof Kozlowski wrote:
> One of benefits of dev_err_probe() is that it returns the error value
> greatly simplifying the error paths (e.g. three lines -> one line).
> 
> 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



> 
> ---
> 
> Context depends on previous patch
> ---
>  drivers/power/supply/axp288_charger.c | 52 ++++++++++++---------------
>  1 file changed, 22 insertions(+), 30 deletions(-)
> 
> diff --git a/drivers/power/supply/axp288_charger.c b/drivers/power/supply/axp288_charger.c
> index ca52c2c82b2c..ea0f5caee8f0 100644
> --- a/drivers/power/supply/axp288_charger.c
> +++ b/drivers/power/supply/axp288_charger.c
> @@ -859,12 +859,10 @@ static int axp288_charger_probe(struct platform_device *pdev)
>  	info->regmap_irqc = axp20x->regmap_irqc;
>  
>  	info->cable.edev = extcon_get_extcon_dev(AXP288_EXTCON_DEV_NAME);
> -	if (IS_ERR(info->cable.edev)) {
> -		dev_err_probe(dev, PTR_ERR(info->cable.edev),
> -			      "extcon_get_extcon_dev(%s) failed\n",
> -			      AXP288_EXTCON_DEV_NAME);
> -		return PTR_ERR(info->cable.edev);
> -	}
> +	if (IS_ERR(info->cable.edev))
> +		return dev_err_probe(dev, PTR_ERR(info->cable.edev),
> +				     "extcon_get_extcon_dev(%s) failed\n",
> +				     AXP288_EXTCON_DEV_NAME);
>  
>  	/*
>  	 * On devices with broken ACPI GPIO event handlers there also is no ACPI
> @@ -878,12 +876,11 @@ static int axp288_charger_probe(struct platform_device *pdev)
>  
>  	if (extcon_name) {
>  		info->otg.cable = extcon_get_extcon_dev(extcon_name);
> -		if (IS_ERR(info->otg.cable)) {
> -			dev_err_probe(dev, PTR_ERR(info->otg.cable),
> -				      "extcon_get_extcon_dev(%s) failed\n",
> -				      USB_HOST_EXTCON_NAME);
> -			return PTR_ERR(info->otg.cable);
> -		}
> +		if (IS_ERR(info->otg.cable))
> +			return dev_err_probe(dev, PTR_ERR(info->otg.cable),
> +					     "extcon_get_extcon_dev(%s) failed\n",
> +					     USB_HOST_EXTCON_NAME);
> +
>  		dev_info(dev, "Using " USB_HOST_EXTCON_HID " extcon for usb-id\n");
>  	}
>  
> @@ -897,11 +894,9 @@ static int axp288_charger_probe(struct platform_device *pdev)
>  	charger_cfg.drv_data = info;
>  	info->psy_usb = devm_power_supply_register(dev, &axp288_charger_desc,
>  						   &charger_cfg);
> -	if (IS_ERR(info->psy_usb)) {
> -		ret = PTR_ERR(info->psy_usb);
> -		dev_err(dev, "failed to register power supply: %d\n", ret);
> -		return ret;
> -	}
> +	if (IS_ERR(info->psy_usb))
> +		return dev_err_probe(dev, PTR_ERR(info->psy_usb),
> +				     "failed to register power supply: %d\n", ret);
>  
>  	/* Cancel our work on cleanup, register this before the notifiers */
>  	ret = devm_work_autocancel(dev, &info->cable.work,
> @@ -913,10 +908,9 @@ static int axp288_charger_probe(struct platform_device *pdev)
>  	info->cable.nb.notifier_call = axp288_charger_handle_cable_evt;
>  	ret = devm_extcon_register_notifier_all(dev, info->cable.edev,
>  						&info->cable.nb);
> -	if (ret) {
> -		dev_err(dev, "failed to register cable extcon notifier\n");
> -		return ret;
> -	}
> +	if (ret)
> +		return dev_err_probe(dev, ret, "failed to register cable extcon notifier\n");
> +
>  	schedule_work(&info->cable.work);
>  
>  	ret = devm_work_autocancel(dev, &info->otg.work,
> @@ -929,10 +923,10 @@ static int axp288_charger_probe(struct platform_device *pdev)
>  	if (info->otg.cable) {
>  		ret = devm_extcon_register_notifier(dev, info->otg.cable,
>  					EXTCON_USB_HOST, &info->otg.id_nb);
> -		if (ret) {
> -			dev_err(dev, "failed to register EXTCON_USB_HOST notifier\n");
> -			return ret;
> -		}
> +		if (ret)
> +			return dev_err_probe(dev, ret,
> +					     "failed to register EXTCON_USB_HOST notifier\n");
> +
>  		schedule_work(&info->otg.work);
>  	}
>  
> @@ -951,11 +945,9 @@ static int axp288_charger_probe(struct platform_device *pdev)
>  		ret = devm_request_threaded_irq(&info->pdev->dev, info->irq[i],
>  					NULL, axp288_charger_irq_thread_handler,
>  					IRQF_ONESHOT, info->pdev->name, info);
> -		if (ret) {
> -			dev_err(dev, "failed to request interrupt=%d\n",
> -								info->irq[i]);
> -			return ret;
> -		}
> +		if (ret)
> +			return dev_err_probe(dev, ret, "failed to request interrupt=%d\n",
> +					     info->irq[i]);
>  	}
>  
>  	return 0;


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 3/4] power: supply: bq24190: Avoid rescheduling after cancelling work
  2026-02-20 17:49 ` [PATCH 3/4] power: supply: bq24190: Avoid rescheduling after cancelling work Krzysztof Kozlowski
@ 2026-02-21 15:19   ` Hans de Goede
  0 siblings, 0 replies; 11+ messages in thread
From: Hans de Goede @ 2026-02-21 15:19 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Sebastian Reichel, Chen-Yu Tsai, linux-pm,
	linux-kernel

Hi,

On 20-Feb-26 18:49, Krzysztof Kozlowski wrote:
> Driver initializes delayed work and then registers interrupt handler
> with devm interface.  This means that device removal will not use a
> reversed order, but first cancel pending work items and then, via devm
> release handlers, free the interrupt.
> 
> The interrupt handler does not directly use/schedule work
> items on the workqueue, however it updates the status of the battery
> charger which might lead to calling power_supply_changed() and trigger
> chain of calls leading to scheduling the work items.  If this happens
> during short time window after cancel_delayed_work_sync() in remove()
> callback, the work would be rescheduled.
> 
> Avoid this by using devm interface to initialize and cancel work item,
> thus having exactly reverse order during remove() in respect to rest of
> the probe/cleanup paths.  This is also more logical and readable code.
> 
> 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/bq24190_charger.c | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/power/supply/bq24190_charger.c b/drivers/power/supply/bq24190_charger.c
> index ed0ceae8d90b..55da91bacc3e 100644
> --- a/drivers/power/supply/bq24190_charger.c
> +++ b/drivers/power/supply/bq24190_charger.c
> @@ -9,6 +9,7 @@
>  #include <linux/module.h>
>  #include <linux/interrupt.h>
>  #include <linux/delay.h>
> +#include <linux/devm-helpers.h>
>  #include <linux/pm_runtime.h>
>  #include <linux/power_supply.h>
>  #include <linux/power/bq24190_charger.h>
> @@ -2087,8 +2088,11 @@ static int bq24190_probe(struct i2c_client *client)
>  	bdi->charge_type = POWER_SUPPLY_CHARGE_TYPE_FAST;
>  	bdi->f_reg = 0;
>  	bdi->ss_reg = BQ24190_REG_SS_VBUS_STAT_MASK; /* impossible state */
> -	INIT_DELAYED_WORK(&bdi->input_current_limit_work,
> -			  bq24190_input_current_limit_work);
> +
> +	ret = devm_delayed_work_autocancel(dev, &bdi->input_current_limit_work,
> +					   bq24190_input_current_limit_work);
> +	if (ret)
> +		return ret;
>  
>  	i2c_set_clientdata(client, bdi);
>  
> @@ -2198,7 +2202,6 @@ static void bq24190_remove(struct i2c_client *client)
>  	struct bq24190_dev_info *bdi = i2c_get_clientdata(client);
>  	int error;
>  
> -	cancel_delayed_work_sync(&bdi->input_current_limit_work);
>  	error = pm_runtime_resume_and_get(bdi->dev);
>  	if (error < 0)
>  		dev_warn(bdi->dev, "pm_runtime_get failed: %i\n", error);


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 4/4] power: supply: twl4030_madc: Drop unused header includes
  2026-02-20 17:49 ` [PATCH 4/4] power: supply: twl4030_madc: Drop unused header includes Krzysztof Kozlowski
@ 2026-02-21 15:19   ` Hans de Goede
  0 siblings, 0 replies; 11+ messages in thread
From: Hans de Goede @ 2026-02-21 15:19 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Sebastian Reichel, Chen-Yu Tsai, linux-pm,
	linux-kernel

Hi,

On 20-Feb-26 18:49, Krzysztof Kozlowski wrote:
> Driver does not use any code from workqueue.h and param.h.
> 
> 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/twl4030_madc_battery.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/drivers/power/supply/twl4030_madc_battery.c b/drivers/power/supply/twl4030_madc_battery.c
> index 3935162e350b..a99b3ff26929 100644
> --- a/drivers/power/supply/twl4030_madc_battery.c
> +++ b/drivers/power/supply/twl4030_madc_battery.c
> @@ -11,9 +11,7 @@
>   */
>  
>  #include <linux/module.h>
> -#include <linux/param.h>
>  #include <linux/delay.h>
> -#include <linux/workqueue.h>
>  #include <linux/platform_device.h>
>  #include <linux/power_supply.h>
>  #include <linux/slab.h>


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/4] power: supply: axp288_charger: Do not cancel work before initializing it
  2026-02-20 17:49 [PATCH 1/4] power: supply: axp288_charger: Do not cancel work before initializing it Krzysztof Kozlowski
                   ` (4 preceding siblings ...)
  2026-02-21 15:17 ` Hans de Goede
@ 2026-03-03  0:01 ` Sebastian Reichel
  5 siblings, 0 replies; 11+ messages in thread
From: Sebastian Reichel @ 2026-03-03  0:01 UTC (permalink / raw)
  To: Sebastian Reichel, Hans de Goede, Chen-Yu Tsai, linux-pm,
	linux-kernel, Krzysztof Kozlowski
  Cc: stable


On Fri, 20 Feb 2026 18:49:39 +0100, 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.
> 
> [...]

Applied, thanks!

[1/4] power: supply: axp288_charger: Do not cancel work before initializing it
      commit: 3e2143c88b5c1e50439239693ba9994cc82d86c3
[2/4] power: supply: axp288_charger: Simplify returns of dev_err_probe()
      commit: c53266766ba5fb52f32f1766a71e0f96c5e51892
[3/4] power: supply: bq24190: Avoid rescheduling after cancelling work
      commit: ba4300a96fb2be99dd29939fd2ca84d67260deaa
[4/4] power: supply: twl4030_madc: Drop unused header includes
      commit: f14f741f9059a8d5492969e480453640cb5dbc85

Best regards,
-- 
Sebastian Reichel <sebastian.reichel@collabora.com>


^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2026-03-03  0:01 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-02-20 17:49 [PATCH 1/4] power: supply: axp288_charger: Do not cancel work before initializing it Krzysztof Kozlowski
2026-02-20 17:49 ` [PATCH 2/4] power: supply: axp288_charger: Simplify returns of dev_err_probe() Krzysztof Kozlowski
2026-02-21  5:05   ` Chen-Yu Tsai
2026-02-21 15:19   ` Hans de Goede
2026-02-20 17:49 ` [PATCH 3/4] power: supply: bq24190: Avoid rescheduling after cancelling work Krzysztof Kozlowski
2026-02-21 15:19   ` Hans de Goede
2026-02-20 17:49 ` [PATCH 4/4] power: supply: twl4030_madc: Drop unused header includes Krzysztof Kozlowski
2026-02-21 15:19   ` Hans de Goede
2026-02-21  5:08 ` [PATCH 1/4] power: supply: axp288_charger: Do not cancel work before initializing it Chen-Yu Tsai
2026-02-21 15:17 ` Hans de Goede
2026-03-03  0:01 ` Sebastian Reichel

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox