[PATCH] power: supply: rt9455: quiesce delayed work before teardown

[PATCH] power: supply: rt9455: quiesce delayed work before teardown

[Fan Wu]

The threaded IRQ handler can queue pwr_rdy_work,
max_charging_time_work and batt_presence_work.  pwr_rdy_work and
batt_presence_work can also queue max_charging_time_work, while
batt_presence_work can requeue itself.

rt9455_remove() cancels max_charging_time_work before
batt_presence_work.  The latter can therefore queue
max_charging_time_work after it has already been cancelled:

  rt9455_remove()                   workqueue
    cancel pwr_rdy_work
    cancel max_charging_time_work
                                      batt_presence_work queues
                                        max_charging_time_work
    cancel batt_presence_work
    return
    devres frees rt9455_info
                                      max_charging_time_work dereferences
                                        rt9455_info

The IRQ also remains registered until devres cleanup and can queue more
work after any of the cancellation calls.  If rt9455_hw_init() fails
after the IRQ has been requested, probe returns without cancelling work
that may already have been queued.  A pending callback can then access
rt9455_info after it has been freed.

Explicitly free the managed IRQ before cancelling the delayed works in
both paths.  This waits for the threaded handler and prevents it from
queuing more work.  Cancel pwr_rdy_work and batt_presence_work before
max_charging_time_work because both can queue the latter.

This issue was found by an in-house static analysis tool.

Fixes: e86d69dd786e ("power_supply: Add support for Richtek RT9455 battery charger")
Cc: stable@vger.kernel.org
Assisted-by: Codex:gpt-5.6
Signed-off-by: Fan Wu <fanwu01@zju.edu.cn>
---
 drivers/power/supply/rt9455_charger.c | 25 ++++++++++++++++++++-----
 1 file changed, 20 insertions(+), 5 deletions(-)

diff --git a/drivers/power/supply/rt9455_charger.c b/drivers/power/supply/rt9455_charger.c
index 7045d2908148..3c64591b5934 100644
--- a/drivers/power/supply/rt9455_charger.c
+++ b/drivers/power/supply/rt9455_charger.c
@@ -1582,6 +1582,17 @@ static const struct regmap_config rt9455_regmap_config = {
 	.cache_type	= REGCACHE_MAPLE,
 };
 
+static void rt9455_cancel_all_delayed_works(struct rt9455_info *info)
+{
+	/*
+	 * Both pwr_rdy_work and batt_presence_work can queue
+	 * max_charging_time_work, so cancel them first.
+	 */
+	cancel_delayed_work_sync(&info->pwr_rdy_work);
+	cancel_delayed_work_sync(&info->batt_presence_work);
+	cancel_delayed_work_sync(&info->max_charging_time_work);
+}
+
 static int rt9455_probe(struct i2c_client *client)
 {
 	struct i2c_adapter *adapter = client->adapter;
@@ -1684,11 +1695,15 @@ static int rt9455_probe(struct i2c_client *client)
 	ret = rt9455_hw_init(info, ichrg, ieoc_percentage, mivr, iaicr);
 	if (ret) {
 		dev_err(dev, "Failed to set charger to its default values\n");
-		goto put_usb_notifier;
+		goto free_irq_and_drain;
 	}
 
 	return 0;
 
+free_irq_and_drain:
+	/* Stop new work before draining work queued during probe. */
+	devm_free_irq(dev, client->irq, info);
+	rt9455_cancel_all_delayed_works(info);
 put_usb_notifier:
 #if IS_ENABLED(CONFIG_USB_PHY)
 	if (info->nb.notifier_call)  {
@@ -1704,6 +1719,10 @@ static void rt9455_remove(struct i2c_client *client)
 	int ret;
 	struct rt9455_info *info = i2c_get_clientdata(client);
 
+	/* Stop the IRQ handler from queuing work during teardown. */
+	devm_free_irq(&client->dev, client->irq, info);
+	rt9455_cancel_all_delayed_works(info);
+
 	ret = rt9455_register_reset(info);
 	if (ret)
 		dev_err(&info->client->dev, "Failed to set charger to its default values\n");
@@ -1712,10 +1731,6 @@ static void rt9455_remove(struct i2c_client *client)
 	if (info->nb.notifier_call)
 		usb_unregister_notifier(info->usb_phy, &info->nb);
 #endif
-
-	cancel_delayed_work_sync(&info->pwr_rdy_work);
-	cancel_delayed_work_sync(&info->max_charging_time_work);
-	cancel_delayed_work_sync(&info->batt_presence_work);
 }
 
 static const struct i2c_device_id rt9455_i2c_id_table[] = {
-- 
2.34.1

Re: [PATCH] power: supply: rt9455: quiesce delayed work before teardown

[Sebastian Reichel]

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

Hi,

On Thu, Jul 23, 2026 at 02:53:52PM +0000, Fan Wu wrote:
> The threaded IRQ handler can queue pwr_rdy_work,
> max_charging_time_work and batt_presence_work.  pwr_rdy_work and
> batt_presence_work can also queue max_charging_time_work, while
> batt_presence_work can requeue itself.
> 
> rt9455_remove() cancels max_charging_time_work before
> batt_presence_work.  The latter can therefore queue
> max_charging_time_work after it has already been cancelled:
> 
>   rt9455_remove()                   workqueue
>     cancel pwr_rdy_work
>     cancel max_charging_time_work
>                                       batt_presence_work queues
>                                         max_charging_time_work
>     cancel batt_presence_work
>     return
>     devres frees rt9455_info
>                                       max_charging_time_work dereferences
>                                         rt9455_info
> 
> The IRQ also remains registered until devres cleanup and can queue more
> work after any of the cancellation calls.  If rt9455_hw_init() fails
> after the IRQ has been requested, probe returns without cancelling work
> that may already have been queued.  A pending callback can then access
> rt9455_info after it has been freed.
> 
> Explicitly free the managed IRQ before cancelling the delayed works in
> both paths.  This waits for the threaded handler and prevents it from
> queuing more work.  Cancel pwr_rdy_work and batt_presence_work before
> max_charging_time_work because both can queue the latter.
> 
> This issue was found by an in-house static analysis tool.
> 
> Fixes: e86d69dd786e ("power_supply: Add support for Richtek RT9455 battery charger")
> Cc: stable@vger.kernel.org
> Assisted-by: Codex:gpt-5.6
> Signed-off-by: Fan Wu <fanwu01@zju.edu.cn>
> ---
>  drivers/power/supply/rt9455_charger.c | 25 ++++++++++++++++++++-----
>  1 file changed, 20 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/power/supply/rt9455_charger.c b/drivers/power/supply/rt9455_charger.c
> index 7045d2908148..3c64591b5934 100644
> --- a/drivers/power/supply/rt9455_charger.c
> +++ b/drivers/power/supply/rt9455_charger.c
> @@ -1582,6 +1582,17 @@ static const struct regmap_config rt9455_regmap_config = {
>  	.cache_type	= REGCACHE_MAPLE,
>  };
>  
> +static void rt9455_cancel_all_delayed_works(struct rt9455_info *info)
> +{
> +	/*
> +	 * Both pwr_rdy_work and batt_presence_work can queue
> +	 * max_charging_time_work, so cancel them first.
> +	 */
> +	cancel_delayed_work_sync(&info->pwr_rdy_work);
> +	cancel_delayed_work_sync(&info->batt_presence_work);
> +	cancel_delayed_work_sync(&info->max_charging_time_work);
> +}
> +
>  static int rt9455_probe(struct i2c_client *client)
>  {
>  	struct i2c_adapter *adapter = client->adapter;
> @@ -1684,11 +1695,15 @@ static int rt9455_probe(struct i2c_client *client)
>  	ret = rt9455_hw_init(info, ichrg, ieoc_percentage, mivr, iaicr);
>  	if (ret) {
>  		dev_err(dev, "Failed to set charger to its default values\n");
> -		goto put_usb_notifier;
> +		goto free_irq_and_drain;
>  	}
>  
>  	return 0;
>  
> +free_irq_and_drain:
> +	/* Stop new work before draining work queued during probe. */
> +	devm_free_irq(dev, client->irq, info);
> +	rt9455_cancel_all_delayed_works(info);

Manually freeing device managed resources is ugly. Register the
rt9455_cancel_all_delayed_works() function via devm_add_action_or_reset()
after the devm_power_supply_register() call in the probe function
instead.

Greetings,

-- Sebastian

>  put_usb_notifier:
>  #if IS_ENABLED(CONFIG_USB_PHY)
>  	if (info->nb.notifier_call)  {
> @@ -1704,6 +1719,10 @@ static void rt9455_remove(struct i2c_client *client)
>  	int ret;
>  	struct rt9455_info *info = i2c_get_clientdata(client);
>  
> +	/* Stop the IRQ handler from queuing work during teardown. */
> +	devm_free_irq(&client->dev, client->irq, info);
> +	rt9455_cancel_all_delayed_works(info);
> +
>  	ret = rt9455_register_reset(info);
>  	if (ret)
>  		dev_err(&info->client->dev, "Failed to set charger to its default values\n");
> @@ -1712,10 +1731,6 @@ static void rt9455_remove(struct i2c_client *client)
>  	if (info->nb.notifier_call)
>  		usb_unregister_notifier(info->usb_phy, &info->nb);
>  #endif
> -
> -	cancel_delayed_work_sync(&info->pwr_rdy_work);
> -	cancel_delayed_work_sync(&info->max_charging_time_work);
> -	cancel_delayed_work_sync(&info->batt_presence_work);
>  }
>  
>  static const struct i2c_device_id rt9455_i2c_id_table[] = {
> -- 
> 2.34.1
> 
> 

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

[PATCH v2] power: supply: rt9455: quiesce delayed work before teardown

[Fan Wu]

The threaded IRQ handler can queue pwr_rdy_work,
max_charging_time_work and batt_presence_work.  pwr_rdy_work and
batt_presence_work can also queue max_charging_time_work, while
batt_presence_work can requeue itself.

rt9455_remove() cancels max_charging_time_work before
batt_presence_work.  The latter can therefore queue
max_charging_time_work after it has already been cancelled:

  rt9455_remove()                   workqueue
    cancel pwr_rdy_work
    cancel max_charging_time_work
                                      batt_presence_work queues
                                        max_charging_time_work
    cancel batt_presence_work
    return
    devres frees rt9455_info
                                      max_charging_time_work dereferences
                                        rt9455_info

The IRQ also remains registered until devres cleanup and can queue more
work after any of the cancellation calls.  If rt9455_hw_init() fails
after the IRQ has been requested, probe returns without cancelling work
that may already have been queued.  A pending callback can then access
rt9455_info after it has been freed.

Register rt9455_cancel_all_delayed_works() through
devm_add_action_or_reset() right after devm_power_supply_register().
devres invokes the action in reverse registration order, after the
managed IRQ has been freed and before rt9455_info is released, so the
delayed works are drained in both rt9455_remove() and the probe error
path.  Cancel pwr_rdy_work and batt_presence_work before
max_charging_time_work because both can queue the latter.

This issue was found by an in-house static analysis tool.

Fixes: e86d69dd786e ("power_supply: Add support for Richtek RT9455 battery charger")
Cc: stable@vger.kernel.org
Assisted-by: Codex:gpt-5.6
Signed-off-by: Fan Wu <fanwu01@zju.edu.cn>
---
 drivers/power/supply/rt9455_charger.c | 22 ++++++++++++++++++----
 1 file changed, 18 insertions(+), 4 deletions(-)

diff --git a/drivers/power/supply/rt9455_charger.c b/drivers/power/supply/rt9455_charger.c
index 7045d2908..9fa343e1e 100644
--- a/drivers/power/supply/rt9455_charger.c
+++ b/drivers/power/supply/rt9455_charger.c
@@ -1582,6 +1582,19 @@ static const struct regmap_config rt9455_regmap_config = {
 	.cache_type	= REGCACHE_MAPLE,
 };
 
+static void rt9455_cancel_all_delayed_works(void *data)
+{
+	struct rt9455_info *info = data;
+
+	/*
+	 * Both pwr_rdy_work and batt_presence_work can queue
+	 * max_charging_time_work, so cancel them first.
+	 */
+	cancel_delayed_work_sync(&info->pwr_rdy_work);
+	cancel_delayed_work_sync(&info->batt_presence_work);
+	cancel_delayed_work_sync(&info->max_charging_time_work);
+}
+
 static int rt9455_probe(struct i2c_client *client)
 {
 	struct i2c_adapter *adapter = client->adapter;
@@ -1672,6 +1685,11 @@ static int rt9455_probe(struct i2c_client *client)
 		goto put_usb_notifier;
 	}
 
+	ret = devm_add_action_or_reset(dev, rt9455_cancel_all_delayed_works,
+				       info);
+	if (ret)
+		goto put_usb_notifier;
+
 	ret = devm_request_threaded_irq(dev, client->irq, NULL,
 					rt9455_irq_handler_thread,
 					IRQF_TRIGGER_LOW | IRQF_ONESHOT,
@@ -1712,10 +1730,6 @@ static void rt9455_remove(struct i2c_client *client)
 	if (info->nb.notifier_call)
 		usb_unregister_notifier(info->usb_phy, &info->nb);
 #endif
-
-	cancel_delayed_work_sync(&info->pwr_rdy_work);
-	cancel_delayed_work_sync(&info->max_charging_time_work);
-	cancel_delayed_work_sync(&info->batt_presence_work);
 }
 
 static const struct i2c_device_id rt9455_i2c_id_table[] = {
-- 
2.34.1