| Message ID | 20191004150738.6542-1-krzk@kernel.org (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
| Series | [RFT,1/3] power: supply: ab8500: Cleanup probe in reverse order | expand |
On Fri, Oct 4, 2019 at 5:07 PM Krzysztof Kozlowski <krzk@kernel.org> wrote: > It is logical to cleanup in probe's error path in reverse order to > previous actions. It also makes easier to add additional goto labels > within this error path. > > Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org> For all 3 patches: Acked-by: Linus Walleij <linus.walleij@linaro.org> The battery charging code is currently disabled on ux500 simply because no platforms with batteries were available for testing or supported by any device trees. This is getting fix: PostmarketOS is brewing patches for enabling all Ux500-based Samsung phones, all with batteries. So we will soon be able to test and turn this on. The patches are fine to merge, however notice that we are refactoring all drivers using ADC through the IIO tree: https://lore.kernel.org/linux-iio/20191011071805.5554-4-linus.walleij@linaro.org/ https://lore.kernel.org/linux-iio/20191011071805.5554-2-linus.walleij@linaro.org/ https://lore.kernel.org/linux-iio/20191011071805.5554-3-linus.walleij@linaro.org/ It would be nice if we could avoid colissions. Yours, Linus Walleij
Hi, On Wed, Oct 16, 2019 at 10:33:12AM +0200, Linus Walleij wrote: > On Fri, Oct 4, 2019 at 5:07 PM Krzysztof Kozlowski <krzk@kernel.org> wrote: > > > It is logical to cleanup in probe's error path in reverse order to > > previous actions. It also makes easier to add additional goto labels > > within this error path. > > > > Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org> > > For all 3 patches: > Acked-by: Linus Walleij <linus.walleij@linaro.org> > > The battery charging code is currently disabled on ux500 simply > because no platforms with batteries were available for testing > or supported by any device trees. > > This is getting fix: PostmarketOS is brewing patches for enabling > all Ux500-based Samsung phones, all with batteries. So we will > soon be able to test and turn this on. > > The patches are fine to merge, however notice that we are > refactoring all drivers using ADC through the IIO tree: > https://lore.kernel.org/linux-iio/20191011071805.5554-4-linus.walleij@linaro.org/ > https://lore.kernel.org/linux-iio/20191011071805.5554-2-linus.walleij@linaro.org/ > https://lore.kernel.org/linux-iio/20191011071805.5554-3-linus.walleij@linaro.org/ > > It would be nice if we could avoid colissions. Thanks, I merged your immutable branch for the ADC work and this patchset. -- Sebastian
diff --git a/drivers/power/supply/ab8500_btemp.c b/drivers/power/supply/ab8500_btemp.c index 8fe81259bfd9..a167938e32f2 100644 --- a/drivers/power/supply/ab8500_btemp.c +++ b/drivers/power/supply/ab8500_btemp.c @@ -1104,13 +1104,13 @@ static int ab8500_btemp_probe(struct platform_device *pdev) return ret; free_irq: - power_supply_unregister(di->btemp_psy); - /* We also have to free all successfully registered irqs */ for (i = i - 1; i >= 0; i--) { irq = platform_get_irq_byname(pdev, ab8500_btemp_irq[i].name); free_irq(irq, di); } + + power_supply_unregister(di->btemp_psy); free_btemp_wq: destroy_workqueue(di->btemp_wq); return ret; diff --git a/drivers/power/supply/ab8500_fg.c b/drivers/power/supply/ab8500_fg.c index 6fc4bc30644c..36bbb8ea05da 100644 --- a/drivers/power/supply/ab8500_fg.c +++ b/drivers/power/supply/ab8500_fg.c @@ -3212,15 +3212,15 @@ static int ab8500_fg_probe(struct platform_device *pdev) return ret; free_irq: - power_supply_unregister(di->fg_psy); - /* We also have to free all registered irqs */ + irq = platform_get_irq_byname(pdev, ab8500_fg_irq_bh[0].name); + free_irq(irq, di); for (i = 0; i < ARRAY_SIZE(ab8500_fg_irq_th); i++) { irq = platform_get_irq_byname(pdev, ab8500_fg_irq_th[i].name); free_irq(irq, di); } - irq = platform_get_irq_byname(pdev, ab8500_fg_irq_bh[0].name); - free_irq(irq, di); + + power_supply_unregister(di->fg_psy); free_inst_curr_wq: destroy_workqueue(di->fg_wq); return ret;
It is logical to cleanup in probe's error path in reverse order to previous actions. It also makes easier to add additional goto labels within this error path. Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org> --- Not marking as cc-stable as this was not reproduced and not tested. --- drivers/power/supply/ab8500_btemp.c | 4 ++-- drivers/power/supply/ab8500_fg.c | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-)