Messages in this thread | | | Subject | Re: [PATCH 1/3] regulator: wm8994: Revert back to using devres | From | Marek Szyprowski <> | Date | Fri, 23 Nov 2018 10:24:28 +0100 |
| |
Hi Charles,
On 2018-11-22 18:30, Charles Keepax wrote: > This reverts commit 466affa06703 ("regulator: wm8994: Don't > use devres for enable GPIOs"). Whilst that did work around the > issue in question there are complications on the error paths of > regulator_register. In the success case clearly the regulator > core owns the GPIO, however, in the failure case it is unclear > what should happen. The regulator core could always free the > GPIO but that may not make sense from the perspective of the > calling code. Alternatively the regulator core could never free > the GPIO but that makes the handling in the regulator core rather > complex. Ultimately it makes more sense to handle the shared GPIO > in the core. > > Reported-by: Marek Szyprowski <m.szyprowski@samsung.com> > Signed-off-by: Charles Keepax <ckeepax@opensource.cirrus.com>
Thanks for sorting this out! Works fine on Trats2:
Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>
> --- > drivers/regulator/wm8994-regulator.c | 14 +++++--------- > 1 file changed, 5 insertions(+), 9 deletions(-) > > diff --git a/drivers/regulator/wm8994-regulator.c b/drivers/regulator/wm8994-regulator.c > index 46e6b4ee1491a..d7fec533c4032 100644 > --- a/drivers/regulator/wm8994-regulator.c > +++ b/drivers/regulator/wm8994-regulator.c > @@ -147,14 +147,11 @@ static int wm8994_ldo_probe(struct platform_device *pdev) > config.regmap = wm8994->regmap; > config.init_data = &ldo->init_data; > > - /* > - * Look up LDO enable GPIO from the parent device node, we don't > - * use devm because the regulator core will free the GPIO > - */ > - gpiod = gpiod_get_optional(pdev->dev.parent, > - id ? "wlf,ldo2ena" : "wlf,ldo1ena", > - GPIOD_OUT_LOW | > - GPIOD_FLAGS_BIT_NONEXCLUSIVE); > + /* Look up LDO enable GPIO from the parent device node */ > + gpiod = devm_gpiod_get_optional(pdev->dev.parent, > + id ? "wlf,ldo2ena" : "wlf,ldo1ena", > + GPIOD_OUT_LOW | > + GPIOD_FLAGS_BIT_NONEXCLUSIVE); > if (IS_ERR(gpiod)) > return PTR_ERR(gpiod); > config.ena_gpiod = gpiod; > @@ -187,7 +184,6 @@ static int wm8994_ldo_probe(struct platform_device *pdev) > return 0; > > err: > - gpiod_put(gpiod); > return ret; > } >
Best regards -- Marek Szyprowski, PhD Samsung R&D Institute Poland
| |