| Message ID | 50d84193-a933-1301-b9d9-bf6cc01ee126@I-love.SAKURA.ne.jp (mailing list archive) |
|---|---|
| State | Handled Elsewhere, archived |
| Headers | show |
| Series | power: ab8500: Remove flush_scheduled_work() call. | expand |
Hi, On Thu, Jun 09, 2022 at 01:58:04PM +0900, Tetsuo Handa wrote: > It seems to me that ab8500 driver is using dedicated workqueues and > is not calling schedule{,_delayed}_work{,_on}(). Then, there will be > no work to flush using flush_scheduled_work(). > > Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> > --- > Please see commit c4f135d643823a86 ("workqueue: Wrap flush_workqueue() > using a macro") for background. Looks sensible to me. Adding Linus to Cc and waiting a bit so that he has time to review/test. @Linus I think it makes sense to add something like this to MAINTAINERS or add the files to the "ARM/NOMADIK/Ux500 ARCHITECTURES" entry, so that you will be CC'd. AB8500 BATTERY AND CHARGER DRIVERS M: Linus Walleij <linus.walleij@linaro.org> S: Maintained F: Documentation/devicetree/bindings/power/supply/*ab8500* F: Documentation/devicetree/bindings/power/supply/*ab8500* -- Sebastian > drivers/power/supply/ab8500_btemp.c | 1 - > drivers/power/supply/ab8500_chargalg.c | 1 - > drivers/power/supply/ab8500_charger.c | 2 -- > drivers/power/supply/ab8500_fg.c | 1 - > 4 files changed, 5 deletions(-) > > diff --git a/drivers/power/supply/ab8500_btemp.c b/drivers/power/supply/ab8500_btemp.c > index b7e842dff567..863fabe05bdc 100644 > --- a/drivers/power/supply/ab8500_btemp.c > +++ b/drivers/power/supply/ab8500_btemp.c > @@ -697,7 +697,6 @@ static void ab8500_btemp_unbind(struct device *dev, struct device *master, > > /* Delete the work queue */ > destroy_workqueue(di->btemp_wq); > - flush_scheduled_work(); > } > > static const struct component_ops ab8500_btemp_component_ops = { > diff --git a/drivers/power/supply/ab8500_chargalg.c b/drivers/power/supply/ab8500_chargalg.c > index 431bbc352d1b..454acb1964fc 100644 > --- a/drivers/power/supply/ab8500_chargalg.c > +++ b/drivers/power/supply/ab8500_chargalg.c > @@ -1746,7 +1746,6 @@ static void ab8500_chargalg_unbind(struct device *dev, struct device *master, > > /* Delete the work queue */ > destroy_workqueue(di->chargalg_wq); > - flush_scheduled_work(); > } > > static const struct component_ops ab8500_chargalg_component_ops = { > diff --git a/drivers/power/supply/ab8500_charger.c b/drivers/power/supply/ab8500_charger.c > index d04d087caa50..0510e0ee4c60 100644 > --- a/drivers/power/supply/ab8500_charger.c > +++ b/drivers/power/supply/ab8500_charger.c > @@ -3404,8 +3404,6 @@ static void ab8500_charger_unbind(struct device *dev) > /* Delete the work queue */ > destroy_workqueue(di->charger_wq); > > - flush_scheduled_work(); > - > /* Unbind fg, btemp, algorithm */ > component_unbind_all(dev, di); > } > diff --git a/drivers/power/supply/ab8500_fg.c b/drivers/power/supply/ab8500_fg.c > index ec8a404d71b4..e49f9b679b6c 100644 > --- a/drivers/power/supply/ab8500_fg.c > +++ b/drivers/power/supply/ab8500_fg.c > @@ -3227,7 +3227,6 @@ 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.18.4 >
Linus, is this patch OK? On 2022/06/10 4:43, Sebastian Reichel wrote: > Hi, > > On Thu, Jun 09, 2022 at 01:58:04PM +0900, Tetsuo Handa wrote: >> It seems to me that ab8500 driver is using dedicated workqueues and >> is not calling schedule{,_delayed}_work{,_on}(). Then, there will be >> no work to flush using flush_scheduled_work(). >> >> Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> >> --- >> Please see commit c4f135d643823a86 ("workqueue: Wrap flush_workqueue() >> using a macro") for background. > > Looks sensible to me. Adding Linus to Cc and waiting a bit so that > he has time to review/test. > > @Linus I think it makes sense to add something like this to > MAINTAINERS or add the files to the "ARM/NOMADIK/Ux500 ARCHITECTURES" > entry, so that you will be CC'd. > > AB8500 BATTERY AND CHARGER DRIVERS > M: Linus Walleij <linus.walleij@linaro.org> > S: Maintained > F: Documentation/devicetree/bindings/power/supply/*ab8500* > F: Documentation/devicetree/bindings/power/supply/*ab8500* > > -- Sebastian > >> drivers/power/supply/ab8500_btemp.c | 1 - >> drivers/power/supply/ab8500_chargalg.c | 1 - >> drivers/power/supply/ab8500_charger.c | 2 -- >> drivers/power/supply/ab8500_fg.c | 1 - >> 4 files changed, 5 deletions(-) >>
On Thu, Jun 9, 2022 at 9:43 PM Sebastian Reichel <sebastian.reichel@collabora.com> wrote: > On Thu, Jun 09, 2022 at 01:58:04PM +0900, Tetsuo Handa wrote: > > It seems to me that ab8500 driver is using dedicated workqueues and > > is not calling schedule{,_delayed}_work{,_on}(). Then, there will be > > no work to flush using flush_scheduled_work(). > > > > Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> > > --- > > Please see commit c4f135d643823a86 ("workqueue: Wrap flush_workqueue() > > using a macro") for background. > > Looks sensible to me. Adding Linus to Cc and waiting a bit so that > he has time to review/test. Makes perfect sense. Reviewed-by: Linus Walleij <linus.walleij@linaro.org> > @Linus I think it makes sense to add something like this to > MAINTAINERS or add the files to the "ARM/NOMADIK/Ux500 ARCHITECTURES" > entry, so that you will be CC'd. > > AB8500 BATTERY AND CHARGER DRIVERS > M: Linus Walleij <linus.walleij@linaro.org> > S: Maintained > F: Documentation/devicetree/bindings/power/supply/*ab8500* > F: Documentation/devicetree/bindings/power/supply/*ab8500* OK I fix something. Yours, Linus Walleij
Hi, On Thu, Jun 23, 2022 at 04:24:09PM +0200, Linus Walleij wrote: > On Thu, Jun 9, 2022 at 9:43 PM Sebastian Reichel > <sebastian.reichel@collabora.com> wrote: > > On Thu, Jun 09, 2022 at 01:58:04PM +0900, Tetsuo Handa wrote: > > > It seems to me that ab8500 driver is using dedicated workqueues and > > > is not calling schedule{,_delayed}_work{,_on}(). Then, there will be > > > no work to flush using flush_scheduled_work(). > > > > > > Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> > > > --- > > > Please see commit c4f135d643823a86 ("workqueue: Wrap flush_workqueue() > > > using a macro") for background. > > > > Looks sensible to me. Adding Linus to Cc and waiting a bit so that > > he has time to review/test. > > Makes perfect sense. > Reviewed-by: Linus Walleij <linus.walleij@linaro.org> Thanks, queued. > > @Linus I think it makes sense to add something like this to > > MAINTAINERS or add the files to the "ARM/NOMADIK/Ux500 ARCHITECTURES" > > entry, so that you will be CC'd. > > > > AB8500 BATTERY AND CHARGER DRIVERS > > M: Linus Walleij <linus.walleij@linaro.org> > > S: Maintained > > F: Documentation/devicetree/bindings/power/supply/*ab8500* > > F: Documentation/devicetree/bindings/power/supply/*ab8500* > > OK I fix something. Thanks. -- Sebastian
diff --git a/drivers/power/supply/ab8500_btemp.c b/drivers/power/supply/ab8500_btemp.c index b7e842dff567..863fabe05bdc 100644 --- a/drivers/power/supply/ab8500_btemp.c +++ b/drivers/power/supply/ab8500_btemp.c @@ -697,7 +697,6 @@ static void ab8500_btemp_unbind(struct device *dev, struct device *master, /* Delete the work queue */ destroy_workqueue(di->btemp_wq); - flush_scheduled_work(); } static const struct component_ops ab8500_btemp_component_ops = { diff --git a/drivers/power/supply/ab8500_chargalg.c b/drivers/power/supply/ab8500_chargalg.c index 431bbc352d1b..454acb1964fc 100644 --- a/drivers/power/supply/ab8500_chargalg.c +++ b/drivers/power/supply/ab8500_chargalg.c @@ -1746,7 +1746,6 @@ static void ab8500_chargalg_unbind(struct device *dev, struct device *master, /* Delete the work queue */ destroy_workqueue(di->chargalg_wq); - flush_scheduled_work(); } static const struct component_ops ab8500_chargalg_component_ops = { diff --git a/drivers/power/supply/ab8500_charger.c b/drivers/power/supply/ab8500_charger.c index d04d087caa50..0510e0ee4c60 100644 --- a/drivers/power/supply/ab8500_charger.c +++ b/drivers/power/supply/ab8500_charger.c @@ -3404,8 +3404,6 @@ static void ab8500_charger_unbind(struct device *dev) /* Delete the work queue */ destroy_workqueue(di->charger_wq); - flush_scheduled_work(); - /* Unbind fg, btemp, algorithm */ component_unbind_all(dev, di); } diff --git a/drivers/power/supply/ab8500_fg.c b/drivers/power/supply/ab8500_fg.c index ec8a404d71b4..e49f9b679b6c 100644 --- a/drivers/power/supply/ab8500_fg.c +++ b/drivers/power/supply/ab8500_fg.c @@ -3227,7 +3227,6 @@ 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);
It seems to me that ab8500 driver is using dedicated workqueues and is not calling schedule{,_delayed}_work{,_on}(). Then, there will be no work to flush using flush_scheduled_work(). Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> --- Please see commit c4f135d643823a86 ("workqueue: Wrap flush_workqueue() using a macro") for background. drivers/power/supply/ab8500_btemp.c | 1 - drivers/power/supply/ab8500_chargalg.c | 1 - drivers/power/supply/ab8500_charger.c | 2 -- drivers/power/supply/ab8500_fg.c | 1 - 4 files changed, 5 deletions(-)