linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Charles Keepax <ckeepax@opensource.cirrus.com>
To: Linus Walleij <linus.walleij@linaro.org>
Cc: Liam Girdwood <lgirdwood@gmail.com>,
	Mark Brown <broonie@kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	Bartosz Golaszewski <bgolaszewski@baylibre.com>,
	Marek Szyprowski <m.szyprowski@samsung.com>
Subject: Re: [PATCH 06/15 v3] regulator: max8973: Let core handle GPIO descriptor
Date: Wed, 5 Dec 2018 15:33:31 +0000	[thread overview]
Message-ID: <20181205153331.GG16508@imbe.wolfsonmicro.main> (raw)
In-Reply-To: <CACRpkdbJsTc2tAyTriz33a75d-1tJdcvdock0fxi7HdJ+EH_Dw@mail.gmail.com>

On Wed, Dec 05, 2018 at 03:42:06PM +0100, Linus Walleij wrote:
> On Wed, Dec 5, 2018 at 2:43 PM Charles Keepax
> <ckeepax@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. 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.

Thanks,
Charles

  reply	other threads:[~2018-12-05 15:33 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-12-05 12:47 [PATCH 00/15 v3] Regulator ena_gpiod fixups Linus Walleij
2018-12-05 12:47 ` [PATCH 01/15 v3] regulator: core: Track dangling GPIO descriptors Linus Walleij
2018-12-05 12:47 ` [PATCH 02/15 v3] regulator: fixed: Let core handle GPIO descriptor Linus Walleij
2018-12-05 12:47 ` [PATCH 03/15 v3] regulator: lm363x: " Linus Walleij
2018-12-05 12:47 ` [PATCH 04/15 v3] regulator: lp8788-ldo: " Linus Walleij
2018-12-05 12:47 ` [PATCH 05/15 v3] regulator: max8952: " Linus Walleij
2018-12-05 12:47 ` [PATCH 06/15 v3] regulator: max8973: " Linus Walleij
2018-12-05 13:43   ` Charles Keepax
2018-12-05 14:42     ` Linus Walleij
2018-12-05 15:33       ` Charles Keepax [this message]
2018-12-06  8:58         ` Linus Walleij
2018-12-06 10:34           ` Charles Keepax
2018-12-06 11:47             ` Linus Walleij
2018-12-06 11:53               ` Charles Keepax
2018-12-05 12:47 ` [PATCH 07/15 v3] gpio: Export gpiod_get_from_of_node() Linus Walleij
2018-12-05 12:47 ` [PATCH 08/15 v3] regulator: max77686: Let core handle GPIO descriptor Linus Walleij
2018-12-05 12:47 ` [PATCH 09/15 v3] gpio: Enable nonexclusive gpiods from DT nodes Linus Walleij
2018-12-05 12:47 ` [PATCH 10/15 v3] gpio: devres: Handle nonexclusive GPIOs Linus Walleij
2018-12-05 14:34   ` Marek Szyprowski
2018-12-05 12:47 ` [PATCH 11/15 v3] gpio: Add devm_gpiod_unhinge() Linus Walleij
2018-12-05 14:34   ` Marek Szyprowski
2018-12-05 12:47 ` [PATCH 12/15 v3] regulator: da9211: Hand over GPIO to regulator core Linus Walleij
2018-12-05 12:47 ` [PATCH 13/15 v3] regulator: s5m8767: " Linus Walleij
2018-12-05 12:47 ` [PATCH 14/15 v3] regulator: tps65090: " Linus Walleij
2018-12-05 12:47 ` [PATCH 15/15 v3] regulator: s2mps11: " Linus Walleij

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20181205153331.GG16508@imbe.wolfsonmicro.main \
    --to=ckeepax@opensource.cirrus.com \
    --cc=bgolaszewski@baylibre.com \
    --cc=broonie@kernel.org \
    --cc=lgirdwood@gmail.com \
    --cc=linus.walleij@linaro.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=m.szyprowski@samsung.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).