linux-kernel - Re: [PATCH 1/2] mfd: arizona: Add GPIO maintain state flag

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] [day] [month] [year] [list]

Date:   Thu, 13 Apr 2017 14:07:21 +0100
From:   Charles Keepax <ckeepax@...nsource.wolfsonmicro.com>
To:     Linus Walleij <linus.walleij@...aro.org>
CC:     Richard Fitzgerald <rf@...nsource.wolfsonmicro.com>,
        Rob Herring <robh@...nel.org>,
        Lee Jones <lee.jones@...aro.org>,
        Alexandre Courbot <gnurou@...il.com>,
        Mark Rutland <mark.rutland@....com>,
        "linux-gpio@...r.kernel.org" <linux-gpio@...r.kernel.org>,
        "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "open list:WOLFSON MICROELECTRONICS DRIVERS" 
        <patches@...nsource.wolfsonmicro.com>
Subject: Re: [PATCH 1/2] mfd: arizona: Add GPIO maintain state flag

On Thu, Apr 13, 2017 at 02:48:45PM +0200, Linus Walleij wrote:
> On Thu, Apr 13, 2017 at 2:21 PM, Richard Fitzgerald
> <rf@...nsource.wolfsonmicro.com> wrote:
> > On Thu, 2017-04-13 at 14:14 +0200, Linus Walleij wrote:
> >> On Thu, Apr 13, 2017 at 11:15 AM, Charles Keepax
> >> <ckeepax@...nsource.wolfsonmicro.com> wrote:
> >> > On Tue, Apr 11, 2017 at 10:34:27AM +0100, Richard Fitzgerald wrote:
> >>
> >> >> 3) The codec only has to be kept awake while any such GPIO is actually
> >> >> in use. See (2)
> >> >
> >> > Yeah option 3 is the primary issue here, we only want to keep the
> >> > CODEC enabled whilst specific GPIOs are in use. As GPIOs can be
> >> > dynamically requested/released by things in the kernel we want to
> >> > know which GPIOs require the CODEC to be kept alive. Also in the
> >> > future one might be tempted to add maintain whilst high and
> >> > maintain whilst low options for lines with pulls on them to
> >> > further optimise power.
> >>
> >> Why does this have to be encoded as information in the device
> >> tree? Isn't it better to use a static table in the driver?
> >>
> >> I don't see what use system integrators and others playing
> >> around with the device tree has of this, it will just be confusing
> >> to them if it is a chip-internal detail.
> >>
> >
> > They are GPIOs for connecting to external hardware, we don't know what
> > people are going to connect them to so they have to tell us how they
> > need them to behave.
> 
> Aha it is a consumer configuration thing, then I see it.
> 
> I think it seems a bit odd that it is assumed that the default is that
> we should *not* preserve the GPIO output value if we go to sleep.
> Should the flag be inverted?
> 

I agree that is a bit odd, my thinking was keeping the behaviour
the same for existing systems. But it only introduces a power
regression perhaps it is ok to require people to update their DT
to avoid that?

> Also, why can't we just use a generic flag for this, it seems very
> very generic.
> 
> Look in:
> include/dt-bindings/gpio/gpio.h
> 
> Is there any reasons why we can't have:
> /* Bit 3 express GPIO suspend/resume persistance in low power mode */
> #define GPIO_MUST_KEEP_VALUE 0
> #define GPIO_MAY_LOOSE_VALUE_DURING_SLEEP 8
> 
> Yeah it's talkative but informative. This way you can mark up lines
> that are OK to loose their value during low-power/sleep using
> just (new) standard bindings that can be reused by others,
> also optionally making it possible for the gpiolib core to take action
> on these properties if need be.
> 

I certainly have no objections to making this a core feature if
you are comfortable with that. I will have a look at what that
would look like.

Thanks,
Charles