Hi all,

GPIO controllers are exported to userspace using /dev/gpiochip*
character devices.  Access control to these devices is provided by
standard UNIX file system permissions, on an all-or-nothing basis:
either a GPIO controller is accessible for a user, or it is not.
Currently no mechanism exists to control access to individual GPIOs.

Hence this adds a GPIO driver to aggregate existing GPIOs, and expose
them as a new gpiochip.  This is useful for implementing access control,
and assigning a set of GPIOs to a specific user.  Furthermore, this
simplifies and hardens exporting GPIOs to a virtual machine, as the VM
can just grab the full GPIO controller, and no longer needs to care
about which GPIOs to grab and which not, reducing the attack surface.

Recently, other use cases have been discovered[1]:
  - Describing GPIO inverters in DT, as a generic GPIO Repeater,
  - Describing simple GPIO-operated devices in DT, and using the GPIO
    Aggregator as a generic GPIO driver for userspace.

Changes compared to v2[2] (more details in the individual patches):
  - Integrate GPIO Repeater functionality,
  - Absorb GPIO forwarder library, as the Aggregator and Repeater are
    now a single driver,
  - Use the aggregator parameters to create a GPIO lookup table instead
    of an array of GPIO descriptors,
  - Add documentation,
  - New patches:
      - "gpiolib: Add GPIOCHIP_NAME definition",
      - "gpiolib: Add support for gpiochipN-based table lookup",
      - "gpiolib: Add support for GPIO line table lookup",
      - "dt-bindings: gpio: Add gpio-repeater bindings",
      - "docs: gpio: Add GPIO Aggregator/Repeater documentation",
      - "MAINTAINERS: Add GPIO Aggregator/Repeater section".
  - Dropped patches:
      - "gpio: Export gpiod_{request,free}() to modular GPIO code",
      - "gpio: Export gpiochip_get_desc() to modular GPIO code",
      - "gpio: Export gpio_name_to_desc() to modular GPIO code",
      - "gpio: Add GPIO Forwarder Helper".

Changes compared to v1[3]:
  - Drop "virtual", rename to gpio-aggregator,
  - Create and use new GPIO Forwarder Helper, to allow sharing code with
    the GPIO inverter,
  - Lift limit on the maximum number of GPIOs,
  - Improve parsing of GPIO specifiers,
  - Fix modular build.

Aggregating GPIOs and exposing them as a new gpiochip was suggested in
response to my proof-of-concept for GPIO virtualization with QEMU[4][5].

For the first use case, aggregated GPIO controllers are instantiated and
destroyed by writing to atribute files in sysfs.
Sample session on the Renesas Koelsch development board:

  - Unbind LEDs from leds-gpio driver:

        echo leds > /sys/bus/platform/drivers/leds-gpio/unbind

  - Create aggregators:

    $ echo e6052000.gpio 19,20 \
        > /sys/bus/platform/drivers/gpio-aggregator/new_device

    gpio-aggregator gpio-aggregator.0: gpio 0 => gpio-953 (gpio-aggregator.0)
    gpio-aggregator gpio-aggregator.0: gpio 1 => gpio-954 (gpio-aggregator.0)
    gpiochip_find_base: found new base at 778
    gpio gpiochip8: (gpio-aggregator.0): added GPIO chardev (254:8)
    gpiochip_setup_dev: registered GPIOs 778 to 779 on device: gpiochip8 (gpio-aggregator.0)

    $ echo e6052000.gpio 21 e6050000.gpio 20-22 \
        > /sys/bus/platform/drivers/gpio-aggregator/new_device

    gpio-aggregator gpio-aggregator.1: gpio 0 => gpio-955 (gpio-aggregator.1)
    gpio-aggregator gpio-aggregator.1: gpio 1 => gpio-1012 (gpio-aggregator.1)
    gpio-aggregator gpio-aggregator.1: gpio 2 => gpio-1013 (gpio-aggregator.1)
    gpio-aggregator gpio-aggregator.1: gpio 3 => gpio-1014 (gpio-aggregator.1)
    gpiochip_find_base: found new base at 774
    gpio gpiochip9: (gpio-aggregator.1): added GPIO chardev (254:9)
    gpiochip_setup_dev: registered GPIOs 774 to 777 on device: gpiochip9 (gpio-aggregator.1)

  - Adjust permissions on /dev/gpiochip[89] (optional)

  - Control LEDs:

    $ gpioset gpiochip8 0=0 1=1 # LED6 OFF, LED7 ON
    $ gpioset gpiochip8 0=1 1=0 # LED6 ON, LED7 OFF
    $ gpioset gpiochip9 0=0     # LED8 OFF
    $ gpioset gpiochip9 0=1     # LED8 ON

  - Destroy aggregators:

    $ echo gpio-aggregator.0 \
            > /sys/bus/platform/drivers/gpio-aggregator/delete_device
    $ echo gpio-aggregator.1 \
            > /sys/bus/platform/drivers/gpio-aggregator/delete_device

Thanks for your comments!

References:
  [1] "[PATCH V4 2/2] gpio: inverter: document the inverter bindings"
      (https://lore.kernel.org/linux-gpio/1561699236-18620-3-git-send-email-harish_kandiga@mentor.com/)
  [2] "[PATCH/RFC v2 0/5] gpio: Add GPIO Aggregator Driver"
      (https://lore.kernel.org/linux-gpio/20190911143858.13024-1-geert+renesas@glider.be/)
  [3] "[PATCH RFC] gpio: Add Virtual Aggregator GPIO Driver"
      (https://lore.kernel.org/lkml/20190705160536.12047-1-geert+renesas@glider.be/)
  [4] "[PATCH QEMU POC] Add a GPIO backend"
      (https://lore.kernel.org/linux-renesas-soc/20181003152521.23144-1-geert+renesas@glider.be/)
  [5] "Getting To Blinky: Virt Edition / Making device pass-through
       work on embedded ARM"
      (https://fosdem.org/2019/schedule/event/vai_getting_to_blinky/)

Geert Uytterhoeven (7):
  gpiolib: Add GPIOCHIP_NAME definition
  gpiolib: Add support for gpiochipN-based table lookup
  gpiolib: Add support for GPIO line table lookup
  dt-bindings: gpio: Add gpio-repeater bindings
  gpio: Add GPIO Aggregator/Repeater driver
  docs: gpio: Add GPIO Aggregator/Repeater documentation
  MAINTAINERS: Add GPIO Aggregator/Repeater section

 .../admin-guide/gpio/gpio-aggregator.rst      | 111 ++++
 Documentation/admin-guide/gpio/index.rst      |   1 +
 .../bindings/gpio/gpio-repeater.yaml          |  53 ++
 MAINTAINERS                                   |   8 +
 drivers/gpio/Kconfig                          |  13 +
 drivers/gpio/Makefile                         |   1 +
 drivers/gpio/gpio-aggregator.c                | 587 ++++++++++++++++++
 drivers/gpio/gpiolib-sysfs.c                  |   7 +-
 drivers/gpio/gpiolib.c                        |  38 +-
 drivers/gpio/gpiolib.h                        |   2 +
 include/linux/gpio/machine.h                  |   2 +-
 11 files changed, 815 insertions(+), 8 deletions(-)
 create mode 100644 Documentation/admin-guide/gpio/gpio-aggregator.rst
 create mode 100644 Documentation/devicetree/bindings/gpio/gpio-repeater.yaml
 create mode 100644 drivers/gpio/gpio-aggregator.c

-- 
2.17.1

Hi Geert,

On Wed, Nov 27, 2019 at 09:42:46AM +0100, Geert Uytterhoeven wrote:
>   - Create aggregators:
> 
>     $ echo e6052000.gpio 19,20 \
>         > /sys/bus/platform/drivers/gpio-aggregator/new_device
> 
>     gpio-aggregator gpio-aggregator.0: gpio 0 => gpio-953 (gpio-aggregator.0)
>     gpio-aggregator gpio-aggregator.0: gpio 1 => gpio-954 (gpio-aggregator.0)
>     gpiochip_find_base: found new base at 778
>     gpio gpiochip8: (gpio-aggregator.0): added GPIO chardev (254:8)
>     gpiochip_setup_dev: registered GPIOs 778 to 779 on device: gpiochip8 (gpio-aggregator.0)
> 
>     $ echo e6052000.gpio 21 e6050000.gpio 20-22 \
>         > /sys/bus/platform/drivers/gpio-aggregator/new_device
> 
>     gpio-aggregator gpio-aggregator.1: gpio 0 => gpio-955 (gpio-aggregator.1)
>     gpio-aggregator gpio-aggregator.1: gpio 1 => gpio-1012 (gpio-aggregator.1)
>     gpio-aggregator gpio-aggregator.1: gpio 2 => gpio-1013 (gpio-aggregator.1)
>     gpio-aggregator gpio-aggregator.1: gpio 3 => gpio-1014 (gpio-aggregator.1)
>     gpiochip_find_base: found new base at 774
>     gpio gpiochip9: (gpio-aggregator.1): added GPIO chardev (254:9)
>     gpiochip_setup_dev: registered GPIOs 774 to 777 on device: gpiochip9 (gpio-aggregator.1)
> 
>   - Adjust permissions on /dev/gpiochip[89] (optional)
> 
>   - Control LEDs:
> 
>     $ gpioset gpiochip8 0=0 1=1 # LED6 OFF, LED7 ON
>     $ gpioset gpiochip8 0=1 1=0 # LED6 ON, LED7 OFF
>     $ gpioset gpiochip9 0=0     # LED8 OFF
>     $ gpioset gpiochip9 0=1     # LED8 ON
> 
>   - Destroy aggregators:
> 
>     $ echo gpio-aggregator.0 \
>             > /sys/bus/platform/drivers/gpio-aggregator/delete_device
>     $ echo gpio-aggregator.1 \
>             > /sys/bus/platform/drivers/gpio-aggregator/delete_device

Thanks for describing the test procedure in detail. It helps a lot.

Using similar commands on H3ULCB, I could successfully trigger the
gpiochip6-{12,13} leds on and off. 

The only unexpected thing is seeing below messages (where gpiochip99 and
gpiochip22 are inexisting gpiochip names, mistakenly provided on command
line prior to passing the correct name):

root@rcar-gen3:~# echo gpiochip6 12-13 > /sys/bus/platform/drivers/gpio-aggregator/new_device                                                                                                                                                                                                                                                                 
[  915.572905] gpio-aggregator gpio-aggregator.0: cannot find GPIO chip gpiochip99, deferring
[  915.584224] gpio-aggregator gpio-aggregator.2: cannot find GPIO chip gpiochip99, deferring
[  915.865281] gpio-aggregator gpio-aggregator.29: cannot find GPIO chip gpiochip22, deferring

Obviously, in the above case, due to a typo in the names, the gpio
chips will never be found, no matter how long gpio-aggregator defers
their probing. Unfortunately, the driver will continuously emit those
messages, upon each successfully created/aggregated gpiochip. I built
gpio-aggregator as a loadable module, if that's relevant.

Another comment is that, while the series _does_ allow specifying
gpio lines in the DTS (this would require a common compatible string
in gpio_aggregator_dt_ids[] and in the DTS node) and while those lines
are indeed exposed to userspace, based on my testing, these same gpio
lines are marked as "used/reserved" by the kernel. This means that
operating on those gpio pins from userspace will not be possible.
For instance, gpioget/gpioset return "Device or resource busy":

gpioget: error reading GPIO values: Device or resource busy
gpioset: error setting the GPIO line values: Device or resource busy

I guess Harish will be unhappy about that, as his expectation was that
upon merging gpio-aggregator with gpio-inverter, he will be able to
describe GPIO polarity and names in DTS without "hogging" the pins.
Perhaps this can be supplemented via an add-on patch later on?

For the whole series (leaving the above findings to your discretion):

Reviewed-by: Eugeniu Rosca <erosca@de.adit-jv.com>
Tested-by: Eugeniu Rosca <erosca@de.adit-jv.com>

Thanks!

-- 
Best Regards,
Eugeniu

Hi Eugeniu,

On Sat, Jan 18, 2020 at 2:46 AM Eugeniu Rosca <erosca@de.adit-jv.com> wrote:
> On Wed, Nov 27, 2019 at 09:42:46AM +0100, Geert Uytterhoeven wrote:
> >   - Create aggregators:
> >
> >     $ echo e6052000.gpio 19,20 \
> >         > /sys/bus/platform/drivers/gpio-aggregator/new_device

> The only unexpected thing is seeing below messages (where gpiochip99 and
> gpiochip22 are inexisting gpiochip names, mistakenly provided on command
> line prior to passing the correct name):
>
> root@rcar-gen3:~# echo gpiochip6 12-13 > /sys/bus/platform/drivers/gpio-aggregator/new_device
> [  915.572905] gpio-aggregator gpio-aggregator.0: cannot find GPIO chip gpiochip99, deferring
> [  915.584224] gpio-aggregator gpio-aggregator.2: cannot find GPIO chip gpiochip99, deferring
> [  915.865281] gpio-aggregator gpio-aggregator.29: cannot find GPIO chip gpiochip22, deferring
>
> Obviously, in the above case, due to a typo in the names, the gpio
> chips will never be found, no matter how long gpio-aggregator defers

Indeed, that is expected behavior: you have created platform devices
referring to resources that are not available.

> their probing. Unfortunately, the driver will continuously emit those
> messages, upon each successfully created/aggregated gpiochip. I built

That is expected behavior, too: every time the driver core manages to
bind a device to a driver, it will retry all previously deferred probes,
in the hope they can be satisfied by the just bound device.

Note that you can destroy these bogus devices, using e.g.

    # echo gpio-aggregator.0 > \
    /sys/bus/platform/drivers/gpio-aggregator/delete_device

> gpio-aggregator as a loadable module, if that's relevant.

Modular or non-modular shouldn't matter w.r.t. this behavior.
Although unloading the module should get rid of the cruft.

> Another comment is that, while the series _does_ allow specifying
> gpio lines in the DTS (this would require a common compatible string
> in gpio_aggregator_dt_ids[] and in the DTS node) and while those lines
> are indeed exposed to userspace, based on my testing, these same gpio
> lines are marked as "used/reserved" by the kernel. This means that
> operating on those gpio pins from userspace will not be possible.
> For instance, gpioget/gpioset return "Device or resource busy":
>
> gpioget: error reading GPIO values: Device or resource busy
> gpioset: error setting the GPIO line values: Device or resource busy
>
> I guess Harish will be unhappy about that, as his expectation was that
> upon merging gpio-aggregator with gpio-inverter, he will be able to
> describe GPIO polarity and names in DTS without "hogging" the pins.
> Perhaps this can be supplemented via an add-on patch later on?

When aggregating GPIO lines, the original GPIO lines are indeed marked
used/reserved, so you cannot use them from userspace.
However, you are expected to use them through the newly created virtual
gpiochip representing the aggregated GPIO lines.

You can try this using the "door" example in
Documentation/admin-guide/gpio/gpio-aggregator.rst, after replacing
gpio2 {19,20} by gpio6 {12,13} to suit your H3ULCB.

> For the whole series (leaving the above findings to your discretion):
>
> Reviewed-by: Eugeniu Rosca <erosca@de.adit-jv.com>
> Tested-by: Eugeniu Rosca <erosca@de.adit-jv.com>

Thanks!

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Hi Geert,

On Mon, Jan 20, 2020 at 10:33:53AM +0100, Geert Uytterhoeven wrote:
> On Sat, Jan 18, 2020 at 2:46 AM Eugeniu Rosca <erosca@de.adit-jv.com> wrote:
> > The only unexpected thing is seeing below messages (where gpiochip99 and
> > gpiochip22 are inexisting gpiochip names, mistakenly provided on command
> > line prior to passing the correct name):
> >
> > root@rcar-gen3:~# echo gpiochip6 12-13 > /sys/bus/platform/drivers/gpio-aggregator/new_device
> > [  915.572905] gpio-aggregator gpio-aggregator.0: cannot find GPIO chip gpiochip99, deferring
> > [  915.584224] gpio-aggregator gpio-aggregator.2: cannot find GPIO chip gpiochip99, deferring
> > [  915.865281] gpio-aggregator gpio-aggregator.29: cannot find GPIO chip gpiochip22, deferring
> >
> > Obviously, in the above case, due to a typo in the names, the gpio
> > chips will never be found, no matter how long gpio-aggregator defers
> 
> Indeed, that is expected behavior: you have created platform devices
> referring to resources that are not available.

Got it. Sounds reasonable to me.

> 
> > their probing. Unfortunately, the driver will continuously emit those
> > messages, upon each successfully created/aggregated gpiochip. I built
> 
> That is expected behavior, too: every time the driver core manages to
> bind a device to a driver, it will retry all previously deferred probes,
> in the hope they can be satisfied by the just bound device.
> 
> Note that you can destroy these bogus devices, using e.g.
> 
>     # echo gpio-aggregator.0 > \
>     /sys/bus/platform/drivers/gpio-aggregator/delete_device

Yep, I can get rid of the bogus devices this way. Thanks!

> 
> > gpio-aggregator as a loadable module, if that's relevant.
> 
> Modular or non-modular shouldn't matter w.r.t. this behavior.
> Although unloading the module should get rid of the cruft.

Yes, indeed!

> 
> > Another comment is that, while the series _does_ allow specifying
> > gpio lines in the DTS (this would require a common compatible string
> > in gpio_aggregator_dt_ids[] and in the DTS node) and while those lines
> > are indeed exposed to userspace, based on my testing, these same gpio
> > lines are marked as "used/reserved" by the kernel. This means that
> > operating on those gpio pins from userspace will not be possible.
> > For instance, gpioget/gpioset return "Device or resource busy":
> >
> > gpioget: error reading GPIO values: Device or resource busy
> > gpioset: error setting the GPIO line values: Device or resource busy
> >
> > I guess Harish will be unhappy about that, as his expectation was that
> > upon merging gpio-aggregator with gpio-inverter, he will be able to
> > describe GPIO polarity and names in DTS without "hogging" the pins.
> > Perhaps this can be supplemented via an add-on patch later on?
> 
> When aggregating GPIO lines, the original GPIO lines are indeed marked
> used/reserved, so you cannot use them from userspace.
> However, you are expected to use them through the newly created virtual
> gpiochip representing the aggregated GPIO lines.
> 
> You can try this using the "door" example in
> Documentation/admin-guide/gpio/gpio-aggregator.rst, after replacing
> gpio2 {19,20} by gpio6 {12,13} to suit your H3ULCB.

Confirmed. The example works like a charm. One difference between
the runtime-created and DTS-created gpiochips is the name:
 - gpio-aggregator.<number>, for the ones created via sysfs
 - <name-of-DTS-node>, for the ones created via DTS

Seeing this behavior on my target, I believe the expectations of
Harish should be met w/o any major limitations.

> 
> > For the whole series (leaving the above findings to your discretion):
> >
> > Reviewed-by: Eugeniu Rosca <erosca@de.adit-jv.com>
> > Tested-by: Eugeniu Rosca <erosca@de.adit-jv.com>

The recent [v3] discussion actually applies to [v4], for which I did
review and testing. Will relay the signatures to the latest version.

Thank you very much.

-- 
Best Regards,
Eugeniu