Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Wed, 5 Dec 2018 15:33:31 +0000
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
Message-ID: <20181205153331.GG16508@imbe.wolfsonmicro.main>
References: <20181205124721.26624-1-linus.walleij@linaro.org>
 <20181205124721.26624-7-linus.walleij@linaro.org>
 <20181205134345.GF16508@imbe.wolfsonmicro.main>
 <CACRpkdbJsTc2tAyTriz33a75d-1tJdcvdock0fxi7HdJ+EH_Dw@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <CACRpkdbJsTc2tAyTriz33a75d-1tJdcvdock0fxi7HdJ+EH_Dw@mail.gmail.com>
User-Agent: Mutt/1.5.20 (2009-12-10)
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

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