* [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* 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 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
* [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* 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
* [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 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
` (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 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