diff mbox series

power: supply: ab8500_fg: Allocate wq in probe

Message ID 20220423172727.1197901-1-linus.walleij@linaro.org
State Accepted
Commit 010ddb813f3554cbbf8bd13b731452236a2c8017
Headers show
Series power: supply: ab8500_fg: Allocate wq in probe | expand

Commit Message

Linus Walleij April 23, 2022, 5:27 p.m. UTC
The workqueue is allocated in bind() but all interrupts are
registered in probe().

Some interrupts put work on the workqueue, which can have
bad side effects.

Allocate the workqueue in probe() instead, destroy it in
.remove() and make unbind() simply flush the workqueue.

Fixes: 1c1f13a006ed ("power: supply: ab8500: Move to componentized binding")
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 drivers/power/supply/ab8500_fg.c | 19 ++++++++++---------
 1 file changed, 10 insertions(+), 9 deletions(-)

Comments

Sebastian Reichel May 3, 2022, 3:38 p.m. UTC | #1
Hi,

On Sat, Apr 23, 2022 at 07:27:27PM +0200, Linus Walleij wrote:
> The workqueue is allocated in bind() but all interrupts are
> registered in probe().
> 
> Some interrupts put work on the workqueue, which can have
> bad side effects.
> 
> Allocate the workqueue in probe() instead, destroy it in
> .remove() and make unbind() simply flush the workqueue.
> 
> Fixes: 1c1f13a006ed ("power: supply: ab8500: Move to componentized binding")
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---

Thanks, queued to fixes branch.

-- Sebastian

>  drivers/power/supply/ab8500_fg.c | 19 ++++++++++---------
>  1 file changed, 10 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/power/supply/ab8500_fg.c b/drivers/power/supply/ab8500_fg.c
> index 97ac588a9e9c..ec8a404d71b4 100644
> --- a/drivers/power/supply/ab8500_fg.c
> +++ b/drivers/power/supply/ab8500_fg.c
> @@ -3037,13 +3037,6 @@ static int ab8500_fg_bind(struct device *dev, struct device *master,
>  {
>  	struct ab8500_fg *di = dev_get_drvdata(dev);
>  
> -	/* Create a work queue for running the FG algorithm */
> -	di->fg_wq = alloc_ordered_workqueue("ab8500_fg_wq", WQ_MEM_RECLAIM);
> -	if (di->fg_wq == NULL) {
> -		dev_err(dev, "failed to create work queue\n");
> -		return -ENOMEM;
> -	}
> -
>  	di->bat_cap.max_mah_design = di->bm->bi->charge_full_design_uah;
>  	di->bat_cap.max_mah = di->bat_cap.max_mah_design;
>  	di->vbat_nom_uv = di->bm->bi->voltage_max_design_uv;
> @@ -3067,8 +3060,7 @@ static void ab8500_fg_unbind(struct device *dev, struct device *master,
>  	if (ret)
>  		dev_err(dev, "failed to disable coulomb counter\n");
>  
> -	destroy_workqueue(di->fg_wq);
> -	flush_scheduled_work();
> +	flush_workqueue(di->fg_wq);
>  }
>  
>  static const struct component_ops ab8500_fg_component_ops = {
> @@ -3117,6 +3109,13 @@ static int ab8500_fg_probe(struct platform_device *pdev)
>  	ab8500_fg_charge_state_to(di, AB8500_FG_CHARGE_INIT);
>  	ab8500_fg_discharge_state_to(di, AB8500_FG_DISCHARGE_INIT);
>  
> +	/* Create a work queue for running the FG algorithm */
> +	di->fg_wq = alloc_ordered_workqueue("ab8500_fg_wq", WQ_MEM_RECLAIM);
> +	if (di->fg_wq == NULL) {
> +		dev_err(dev, "failed to create work queue\n");
> +		return -ENOMEM;
> +	}
> +
>  	/* Init work for running the fg algorithm instantly */
>  	INIT_WORK(&di->fg_work, ab8500_fg_instant_work);
>  
> @@ -3227,6 +3226,8 @@ static int ab8500_fg_remove(struct platform_device *pdev)
>  {
>  	struct ab8500_fg *di = platform_get_drvdata(pdev);
>  
> +	destroy_workqueue(di->fg_wq);
> +	flush_scheduled_work();
>  	component_del(&pdev->dev, &ab8500_fg_component_ops);
>  	list_del(&di->node);
>  	ab8500_fg_sysfs_exit(di);
> -- 
> 2.35.1
>
diff mbox series

Patch

diff --git a/drivers/power/supply/ab8500_fg.c b/drivers/power/supply/ab8500_fg.c
index 97ac588a9e9c..ec8a404d71b4 100644
--- a/drivers/power/supply/ab8500_fg.c
+++ b/drivers/power/supply/ab8500_fg.c
@@ -3037,13 +3037,6 @@  static int ab8500_fg_bind(struct device *dev, struct device *master,
 {
 	struct ab8500_fg *di = dev_get_drvdata(dev);
 
-	/* Create a work queue for running the FG algorithm */
-	di->fg_wq = alloc_ordered_workqueue("ab8500_fg_wq", WQ_MEM_RECLAIM);
-	if (di->fg_wq == NULL) {
-		dev_err(dev, "failed to create work queue\n");
-		return -ENOMEM;
-	}
-
 	di->bat_cap.max_mah_design = di->bm->bi->charge_full_design_uah;
 	di->bat_cap.max_mah = di->bat_cap.max_mah_design;
 	di->vbat_nom_uv = di->bm->bi->voltage_max_design_uv;
@@ -3067,8 +3060,7 @@  static void ab8500_fg_unbind(struct device *dev, struct device *master,
 	if (ret)
 		dev_err(dev, "failed to disable coulomb counter\n");
 
-	destroy_workqueue(di->fg_wq);
-	flush_scheduled_work();
+	flush_workqueue(di->fg_wq);
 }
 
 static const struct component_ops ab8500_fg_component_ops = {
@@ -3117,6 +3109,13 @@  static int ab8500_fg_probe(struct platform_device *pdev)
 	ab8500_fg_charge_state_to(di, AB8500_FG_CHARGE_INIT);
 	ab8500_fg_discharge_state_to(di, AB8500_FG_DISCHARGE_INIT);
 
+	/* Create a work queue for running the FG algorithm */
+	di->fg_wq = alloc_ordered_workqueue("ab8500_fg_wq", WQ_MEM_RECLAIM);
+	if (di->fg_wq == NULL) {
+		dev_err(dev, "failed to create work queue\n");
+		return -ENOMEM;
+	}
+
 	/* Init work for running the fg algorithm instantly */
 	INIT_WORK(&di->fg_work, ab8500_fg_instant_work);
 
@@ -3227,6 +3226,8 @@  static int ab8500_fg_remove(struct platform_device *pdev)
 {
 	struct ab8500_fg *di = platform_get_drvdata(pdev);
 
+	destroy_workqueue(di->fg_wq);
+	flush_scheduled_work();
 	component_del(&pdev->dev, &ab8500_fg_component_ops);
 	list_del(&di->node);
 	ab8500_fg_sysfs_exit(di);