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.westerb...@linux.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.westerb...@linux.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.
