On Wed, Dec 5, 2018 at 4:33 PM Charles Keepax <ckee...@opensource.cirrus.com> wrote: > On Wed, Dec 05, 2018 at 03:42:06PM +0100, Linus Walleij wrote: > > On Wed, Dec 5, 2018 at 2:43 PM Charles Keepax > > <ckee...@opensource.cirrus.com> wrote: > > > On Wed, Dec 05, 2018 at 01:47:12PM +0100, Linus Walleij wrote: > > > > @@ -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); > > > > > > My comment on v2 still stands here, the GPIO is not passed to > > > the regulator core on this path. > > > > Patch 01 should take care of that, did you check it? > > Yeah, patch 1 makes the regulator core consistently handle GPIOs > that are passed into it.
Sorry. I confused this patch for the max77686 patch. That one was fixed with patch 01... > However, on the MAX77621 path in this > switch statement the GPIO is never passed into the regulator > core, so the core never takes ownership of it, so the driver still > needs to manage the lifetime. > > It should be a pretty easy fix though as commented on v2, again > apologies for the slow review. OK I switch it to devm_ as you suggested, as we implemented gpiod_unhinge it's a piece of cake nowadays. Thanks a lot! Linus Walleij
