| Message ID | 20181201154151.14890-7-linus.walleij@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Regulator ena_gpiod fixups | expand |
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
diff --git a/drivers/regulator/max8973-regulator.c b/drivers/regulator/max8973-regulator.c index e7a58b509032..ef8f4789a517 100644 --- a/drivers/regulator/max8973-regulator.c +++ b/drivers/regulator/max8973-regulator.c @@ -632,7 +632,7 @@ static int max8973_probe(struct i2c_client *client, struct max8973_chip *max; bool pdata_from_dt = false; unsigned int chip_id; - struct gpio_desc *gpiod; + struct gpio_desc *gpiod = NULL; enum gpiod_flags gflags; int ret; @@ -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); 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); dev_err(max->dev, "Max8973 Init failed, err = %d\n", ret); return ret; }
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*) --- drivers/regulator/max8973-regulator.c | 23 ++++++++++++++++------- 1 file changed, 16 insertions(+), 7 deletions(-) -- 2.19.1