linux-kernel - Re: [PATCH v2 3/6] pinctrl: Add a possibility to configure pins from a gpiolib based drivers

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]

Date:   Wed, 11 Jan 2017 15:33:04 +0200
From:   Mika Westerberg <mika.westerberg@...ux.intel.com>
To:     Linus Walleij <linus.walleij@...aro.org>
Cc:     Heikki Krogerus <heikki.krogerus@...ux.intel.com>,
        "David E . Box" <david.e.box@...ux.intel.com>,
        Andy Shevchenko <andy.shevchenko@...il.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-gpio@...r.kernel.org" <linux-gpio@...r.kernel.org>
Subject: Re: [PATCH v2 3/6] pinctrl: Add a possibility to configure pins from
 a gpiolib based drivers

On Wed, Jan 11, 2017 at 02:06:56PM +0100, Linus Walleij wrote:
> On Tue, Jan 10, 2017 at 3:31 PM, Mika Westerberg
> <mika.westerberg@...ux.intel.com> wrote:
> 
> > When a GPIO driver is backed by a pinctrl driver the GPIO driver often
> > needs to call the pinctrl driver to configure certain things, like
> > whether the pin is used as input or output. In addition to this there
> > are other pin configurations applicable to GPIOs such as debounce
> > timeout.
> >
> > To support this we introduce a new function pinctrl_gpio_set_config()
> > that can be used by gpiolib based driver to pass configuration requests
> > to the backing pinctrl driver.
> >
> > Signed-off-by: Mika Westerberg <mika.westerberg@...ux.intel.com>
> 
> OK so this is needed.

The alternative would be to just handle this internally in
pinctrl-intel.c but I thought it is better to make the functionality
available for other drivers as well.

> But let's first pause and discuss this, because I have some stuff on my
> mind here.
> 
> First this kernel-internal ABI from <linux/gpio/driver.h>:
> 
> struct gpio_chip {
> (...)
>         int                     (*set_debounce)(struct gpio_chip *chip,
>                                                 unsigned offset,
>                                                 unsigned debounce);
>         int                     (*set_single_ended)(struct gpio_chip *chip,
>                                                 unsigned offset,
>                                                 enum single_ended_mode mode);
> (...)
> 
> It's not going to scale. We need to replace this with something like
> 
> int (*set_config)(struct gpio_chip *chip, unsigned offset, unsigned
> long config);
> 
> Where "config" takes the packed format described in
> <linux/pinctrl/pinconf-generic.h>
> and nothing else, anything else is just inviting disaster.
> 
> We can also later add:
> 
> int (*get_config)(struct gpio_chip *chip, unsigned offset, unsigned
> long *config);
> 
> We can then  set and get arbitrary configs on GPIO lines, and the
> drivers can simply implement a switch() for the configs they handle
> else return -ENOTSUPP.
> 
> But right now only set_config() would be enough.
> 
> Maybe stuff needs to be split out of that header to be shared between
> GPIO and pinctrl but hopefully you could just include it.
> 
> Then we change all in-kernel users of these two APIs over to set_config().
> 
> THEN we can think about cross-calling to pin control using the API
> from this patch. It should be a simple matter of just passing along the
> same config argument since we're using generic pin config.
> 
> It's not like it's impossible to merge this patch first, but I want to get some
> order here.
> 
> Are you convenient with doing the above patch as part of this series, or
> shall I do it first so you can rebase on it? (Will take some time if I
> do it...)

Sure, I can take a look at it.

> We need this because GPIO is going to need more and more config
> to be done by pinctrl on its behalf, and it will have to go all the
> way to userspace in many cases, so we need this infrastructure in
> place.

OK.