Messages in this thread | | | Date | Wed, 5 Dec 2018 13:08:26 +0000 | From | Charles Keepax <> | Subject | Re: [PATCH 06/13 v2] regulator: max8973: Let core handle GPIO descriptor |
| |
On Sat, Dec 01, 2018 at 04:41:44PM +0100, Linus Walleij wrote: > Use the gpiod_get() rather than the devm_* version so that the > regulator core can handle the lifecycle of these descriptors. > > Fixes: e7d2be696faa ("regulator: max8973: Pass descriptor instead of GPIO number") > Signed-off-by: Linus Walleij <linus.walleij@linaro.org> > --- > ChangeLog v1->v2: > - Drop the gpiod_put() on the errorpath after devm_regulator_register() > as this will be handled by the regulator core. > - Fix the second case of devm_gpiod_get() > - Put a comment in the code so maintainers knows not to > use managed resources (devm*) > --- > @@ -759,9 +759,13 @@ static int max8973_probe(struct i2c_client *client, > else > gflags = GPIOD_OUT_LOW; > gflags |= GPIOD_FLAGS_BIT_NONEXCLUSIVE; > - gpiod = devm_gpiod_get_optional(&client->dev, > - "maxim,enable", > - gflags); > + /* > + * Do not use devm* here: the regulator core takes over the > + * lifecycle management of the GPIO descriptor. > + */ > + gpiod = gpiod_get_optional(&client->dev, > + "maxim,enable", > + gflags); > if (IS_ERR(gpiod)) > return PTR_ERR(gpiod); > if (gpiod) { > @@ -775,10 +779,13 @@ static int max8973_probe(struct i2c_client *client, > /* > * We do not let the core switch this regulator on/off, > * we just leave it on. > + * > + * Do not use devm* here: the regulator core takes over the > + * lifecycle management of the GPIO descriptor. > */ > - gpiod = devm_gpiod_get_optional(&client->dev, > - "maxim,enable", > - GPIOD_OUT_HIGH); > + gpiod = gpiod_get_optional(&client->dev, > + "maxim,enable", > + GPIOD_OUT_HIGH);
Need to be careful here this path actually never passes the GPIO to the regulator core. I suspect you want to leave this one as a devm_
> if (IS_ERR(gpiod)) > return PTR_ERR(gpiod); > if (gpiod) > @@ -798,6 +805,8 @@ static int max8973_probe(struct i2c_client *client, > > ret = max8973_init_dcdc(max, pdata); > if (ret < 0) { > + if (gpiod) > + gpiod_put(gpiod);
And make this use config.ena_gpiod instead.
Thanks, Charles
| |