Re: API for pull-up/pull-down configuration

[Linus Walleij <linus.walleij@xxxxxxxxxx>]

Hi Thomas,

weird that you write this just after my note to Marek (who just made a
tremendous
job modernizing this driver):
https://marc.info/?l=linux-gpio&m=154479904816811&w=2

On Fri, Dec 14, 2018 at 5:39 PM Thomas Petazzoni
<thomas.petazzoni@xxxxxxxxxxx> wrote:

> I have a GPIO expander over I2C,
> https://www.nxp.com/docs/en/data-sheet/PCAL6416A.pdf, which is
> supported by the existing gpio-pca953x.c driver. This GPIO expander has
> built-in pull-up and pull-down resistors, which can be configured by
> the PULL_EN and PULL_SET registers:
>
> #define PCAL953X_PULL_EN        0x23
> #define PCAL953X_PULL_SEL       0x24
>
> Unfortunately, these are not used anywhere in the driver, and I'm
> wondering what is the right interface for configuring this?
>
> Since this GPIO controller is not tied to any pinmux controller, there
> is no place to define the configuration of pins.
>
> What would be the upstream-acceptable way of providing a mechanism to
> configure such GPIO configuration ? A vendor-specific DT property ? A
> generic DT property ? Something else ?
>
> Of course, I could solve that on my side with a quick non-upstream hack
> in gpio-pca953x, but if there's a reasonable solution that could be
> accepted upstream, I'd very much prefer to go in this direction.

There are essentially two ways.

1. The really complex way and refactor it into a GPIO+pin controller
combo as we did with
drivers/pinctrl/pinctrl-sx150x.c
drivers/pinctrl/pinctrl-mcp23s08.c
If the chip logic is "complicated enough" this is definately the way to
go. If the chip support muxing in other functions than GPIO it is the
only reasonable way to go.

2. The simple way. We already support a whole bunch of standard
pin config in the .set_config() callback. Debounce (which this silicon
seem to support) can be implemented right off to begin with.
We also support this callback for open drain/source configuration.
But in theory any of the stuff from include/linux/pinctrl/pinconf-generic.h
can be passed to this function (that is why we unified it).

For this (2) approach, it would be possible to add some more pin
config options, which I think is all right for anything that is
GPIO only (no muxing). There is no stopping it.

Paradoxically you can probably support any config with board files,
and even userspace access for this approach, but you will run into
a problem when you get to device tree:
-
foo-gpios = <&gpio0 12 GPIO_FOO>;

This last flag needs to encode all options and we have only 32
bits in it I suppose.

Of course it is simple to just support GPIO_PULL_UP and
GPIO_PULL_DOWN, but I have seen so much silicon supporting
setting the number of Ohm for this. This is not the only
pin config that may need an argument: drive strength is the
same (I think this hardware supports that too...)

So a compromise would be to allow GPIO_PULL_UP and
GPIO_PULL_DOWN, GPIO_DRIVE_EXTRA_JUICE flags
if and only if they are just off/on
switches (boolean) and any more complicated hardware will
be required to implement proper pin control.

This would have to be discussed by the DT maintainers though.
It might be necessary to introduce fourcell GPIO specifiers
to handle the arguments.

foo-gpios = <&gpio0 12 GPIO_PULL_UP 8>; // 8 Ohm

What do you think?

Yours,
Linus Walleij