linux-kernel - Re: [PATCH 06/15 v3] regulator: max8973: Let core handle GPIO descriptor

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20181205153331.GG16508@imbe.wolfsonmicro.main>
Date:   Wed, 5 Dec 2018 15:33:31 +0000
From:   Charles Keepax <ckeepax@...nsource.cirrus.com>
To:     Linus Walleij <linus.walleij@...aro.org>
CC:     Liam Girdwood <lgirdwood@...il.com>,
        Mark Brown <broonie@...nel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Bartosz Golaszewski <bgolaszewski@...libre.com>,
        Marek Szyprowski <m.szyprowski@...sung.com>
Subject: Re: [PATCH 06/15 v3] regulator: max8973: Let core handle GPIO
 descriptor

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@...nsource.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