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 simple GPIO-operated devices in DT, and using the GPIO
Aggregator as a generic GPIO driver for userspace, which is useful
for industrial control.
Changes compared to v5[2]:
- Convert raw gpiod_lookup users to GPIO_LOOKUP*(),
- Update Documentation/driver-api/gpio/board.rst for gpiod_lookup
changes,
- Reword rationale behing looking up GPIOs by line name,
- Introduce gpiod_set_config(),
- Use gpiod_to_chip() instead of open-coding,
- Drop debug print of gpio_desc.label, as it usually points to the
GPIO Aggregator itself,
- Drop no longer needed #include "gpiolib.h",
- Fix missing offset translation in gpio_fwd_set_config(),
- Fix "allows" without object,
- Drop "gpiochipN" support,
- Extend example,
Changes compared to v4[3]:
- Add Reviewed-by, Tested-by,
- Fix inconsistent indentation in documentation.
Changes compared to v3[4] (more details in the individual patches):
- Drop controversial GPIO repeater,
- Drop support for legacy sysfs interface based name matching,
- Drop applied "gpiolib: Add GPIOCHIP_NAME definition",
- Documentation improvements,
- Lots of small cleanups.
Changes compared to v2[5] (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[6]:
- 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[7][8].
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 gpio-aggregator.0: gpio 1 => gpio-954
gpiochip_find_base: found new base at 758
gpio gpiochip12: (gpio-aggregator.0): added GPIO chardev (254:13)
gpiochip_setup_dev: registered GPIOs 758 to 759 on device: gpiochip12 (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 gpio-aggregator.1: gpio 1 => gpio-1012
gpio-aggregator gpio-aggregator.1: gpio 2 => gpio-1013
gpio-aggregator gpio-aggregator.1: gpio 3 => gpio-1014
gpiochip_find_base: found new base at 754
gpio gpiochip13: (gpio-aggregator.1): added GPIO chardev (254:13)
gpiochip_setup_dev: registered GPIOs 754 to 757 on device: gpiochip13 (gpio-aggregator.1)
- Adjust permissions on /dev/gpiochip1[23] (optional)
- Control LEDs:
$ gpioset gpiochip12 0=0 1=1 # LED6 OFF, LED7 ON
$ gpioset gpiochip12 0=1 1=0 # LED6 ON, LED7 OFF
$ gpioset gpiochip13 0=1 # LED8 ON
$ gpioset gpiochip13 0=0 # LED8 OFF
- 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
To ease testing, I have pushed this series to the
topic/gpio-aggregator-v6 branch of my renesas-drivers repository at
git://git.kernel.org/pub/scm/linux/kernel/git/geert/renesas-drivers.git.
Kbuild test robot <[email protected]> reported no issues.
Thanks!
References:
[1] "[PATCH V4 2/2] gpio: inverter: document the inverter bindings"
(https://lore.kernel.org/r/[email protected]/)
[2] "[PATCH v5 0/5] gpio: Add GPIO Aggregator"
(https://lore.kernel.org/r/[email protected]/)
[3] "[PATCH v4 0/5] gpio: Add GPIO Aggregator"
(https://lore.kernel.org/r/[email protected])
[4] "[PATCH v3 0/7] gpio: Add GPIO Aggregator/Repeater"
(https://lore.kernel.org/r/[email protected]/)
[5] "[PATCH/RFC v2 0/5] gpio: Add GPIO Aggregator Driver"
(https://lore.kernel.org/r/[email protected]/)
[6] "[PATCH RFC] gpio: Add Virtual Aggregator GPIO Driver"
(https://lore.kernel.org/r/[email protected]/)
[7] "[PATCH QEMU POC] Add a GPIO backend"
(https://lore.kernel.org/r/[email protected]/)
[8] "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 (8):
ARM: integrator: impd1: Use GPIO_LOOKUP() helper macro
i2c: i801: Use GPIO_LOOKUP() helper macro
mfd: sm501: Use GPIO_LOOKUP_IDX() helper macro
gpiolib: Add support for GPIO lookup by line name
gpiolib: Introduce gpiod_set_config()
gpio: Add GPIO Aggregator
docs: gpio: Add GPIO Aggregator documentation
MAINTAINERS: Add GPIO Aggregator section
.../admin-guide/gpio/gpio-aggregator.rst | 111 ++++
Documentation/admin-guide/gpio/index.rst | 1 +
Documentation/driver-api/gpio/board.rst | 10 +-
MAINTAINERS | 7 +
arch/arm/mach-integrator/impd1.c | 11 +-
drivers/gpio/Kconfig | 12 +
drivers/gpio/Makefile | 1 +
drivers/gpio/gpio-aggregator.c | 568 ++++++++++++++++++
drivers/gpio/gpiolib.c | 50 +-
drivers/i2c/busses/i2c-i801.c | 6 +-
drivers/mfd/sm501.c | 24 +-
include/linux/gpio/consumer.h | 8 +
include/linux/gpio/machine.h | 15 +-
13 files changed, 776 insertions(+), 48 deletions(-)
create mode 100644 Documentation/admin-guide/gpio/gpio-aggregator.rst
create mode 100644 drivers/gpio/gpio-aggregator.c
--
2.17.1
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
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
impd1_probe() fills in the GPIO lookup table by manually populating an
array of gpiod_lookup structures. Use the existing GPIO_LOOKUP() helper
macro instead, to relax a dependency on the gpiod_lookup structure's
member names.
Signed-off-by: Geert Uytterhoeven <[email protected]>
Cc: [email protected]
---
While this patch is a dependency for "[PATCH v6 4/8] gpiolib: Add
support for GPIO lookup by line name", it can be applied independently.
But an Acked-by would be nice, too.
Cover letter and full series at
https://lore.kernel.org/r/[email protected]/
v6:
- New.
---
arch/arm/mach-integrator/impd1.c | 11 ++++-------
1 file changed, 4 insertions(+), 7 deletions(-)
diff --git a/arch/arm/mach-integrator/impd1.c b/arch/arm/mach-integrator/impd1.c
index 1ecbea5331d6ed8b..6f875ded841924d8 100644
--- a/arch/arm/mach-integrator/impd1.c
+++ b/arch/arm/mach-integrator/impd1.c
@@ -410,13 +410,10 @@ static int __ref impd1_probe(struct lm_device *dev)
* 5 = Key lower right
*/
/* We need the two MMCI GPIO entries */
- lookup->table[0].chip_label = chipname;
- lookup->table[0].chip_hwnum = 3;
- lookup->table[0].con_id = "wp";
- lookup->table[1].chip_label = chipname;
- lookup->table[1].chip_hwnum = 4;
- lookup->table[1].con_id = "cd";
- lookup->table[1].flags = GPIO_ACTIVE_LOW;
+ lookup->table[0] = (struct gpiod_lookup)
+ GPIO_LOOKUP(chipname, 3, "wp", 0);
+ lookup->table[1] = (struct gpiod_lookup)
+ GPIO_LOOKUP(chipname, 4, "cd", GPIO_ACTIVE_LOW);
gpiod_add_lookup_table(lookup);
}
--
2.17.1
Add a maintainership section for the GPIO Aggregator, covering
documentation and driver source code.
Signed-off-by: Geert Uytterhoeven <[email protected]>
Reviewed-by: Eugeniu Rosca <[email protected]>
Tested-by: Eugeniu Rosca <[email protected]>
---
v6:
- No changes,
v5:
- Add Reviewed-by, Tested-by,
v4:
- Drop controversial GPIO repeater,
v3:
- New.
---
MAINTAINERS | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index fcd79fc38928fafc..1fad69b956df1162 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -7127,6 +7127,13 @@ F: Documentation/firmware-guide/acpi/gpio-properties.rst
F: drivers/gpio/gpiolib-acpi.c
F: drivers/gpio/gpiolib-acpi.h
+GPIO AGGREGATOR
+M: Geert Uytterhoeven <[email protected]>
+L: [email protected]
+S: Maintained
+F: Documentation/admin-guide/gpio/gpio-aggregator.rst
+F: drivers/gpio/gpio-aggregator.c
+
GPIO IR Transmitter
M: Sean Young <[email protected]>
L: [email protected]
--
2.17.1
The GPIO Aggregator will need a method to forward a .set_config() call
to its parent gpiochip. This requires obtaining the gpio_chip and
offset for a given gpio_desc. While gpiod_to_chip() is public,
gpio_chip_hwgpio() is not, so there is currently no method to obtain the
needed GPIO offset parameter.
Hence introduce a public gpiod_set_config() helper, which invokes the
.set_config() callback through a gpio_desc pointer, like is done for
most other gpio_chip callbacks.
Rewrite the existing gpiod_set_debounce() helper as a wrapper around
gpiod_set_config(), to avoid duplication.
Signed-off-by: Geert Uytterhoeven <[email protected]>
---
v6:
- New.
---
drivers/gpio/gpiolib.c | 28 ++++++++++++++++++++++------
include/linux/gpio/consumer.h | 8 ++++++++
2 files changed, 30 insertions(+), 6 deletions(-)
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index c756602e249c052e..30ea75e972b5a3b1 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -3478,6 +3478,26 @@ int gpiod_direction_output(struct gpio_desc *desc, int value)
}
EXPORT_SYMBOL_GPL(gpiod_direction_output);
+/**
+ * gpiod_set_config - sets @config for a GPIO
+ * @desc: descriptor of the GPIO for which to set the configuration
+ * @config: Same packed config format as generic pinconf
+ *
+ * Returns:
+ * 0 on success, %-ENOTSUPP if the controller doesn't support setting the
+ * configuration.
+ */
+int gpiod_set_config(struct gpio_desc *desc, unsigned long config)
+{
+ struct gpio_chip *chip;
+
+ VALIDATE_DESC(desc);
+ chip = desc->gdev->chip;
+
+ return gpio_do_set_config(chip, gpio_chip_hwgpio(desc), config);
+}
+EXPORT_SYMBOL_GPL(gpiod_set_config);
+
/**
* gpiod_set_debounce - sets @debounce time for a GPIO
* @desc: descriptor of the GPIO for which to set debounce time
@@ -3489,14 +3509,10 @@ EXPORT_SYMBOL_GPL(gpiod_direction_output);
*/
int gpiod_set_debounce(struct gpio_desc *desc, unsigned debounce)
{
- struct gpio_chip *chip;
- unsigned long config;
-
- VALIDATE_DESC(desc);
- chip = desc->gdev->chip;
+ unsigned long config;
config = pinconf_to_config_packed(PIN_CONFIG_INPUT_DEBOUNCE, debounce);
- return gpio_do_set_config(chip, gpio_chip_hwgpio(desc), config);
+ return gpiod_set_config(desc, config);
}
EXPORT_SYMBOL_GPL(gpiod_set_debounce);
diff --git a/include/linux/gpio/consumer.h b/include/linux/gpio/consumer.h
index 0a72fccf60fff230..901aab89d025f3ff 100644
--- a/include/linux/gpio/consumer.h
+++ b/include/linux/gpio/consumer.h
@@ -157,6 +157,7 @@ int gpiod_set_raw_array_value_cansleep(unsigned int array_size,
struct gpio_array *array_info,
unsigned long *value_bitmap);
+int gpiod_set_config(struct gpio_desc *desc, unsigned long config);
int gpiod_set_debounce(struct gpio_desc *desc, unsigned debounce);
int gpiod_set_transitory(struct gpio_desc *desc, bool transitory);
void gpiod_toggle_active_low(struct gpio_desc *desc);
@@ -473,6 +474,13 @@ static inline int gpiod_set_raw_array_value_cansleep(unsigned int array_size,
return 0;
}
+static inline int gpiod_set_config(struct gpio_desc *desc, unsigned long config)
+{
+ /* GPIO can never have been requested */
+ WARN_ON(desc);
+ return -ENOSYS;
+}
+
static inline int gpiod_set_debounce(struct gpio_desc *desc, unsigned debounce)
{
/* GPIO can never have been requested */
--
2.17.1
On Tue, Mar 24, 2020 at 2:57 PM Geert Uytterhoeven
<[email protected]> wrote:
> impd1_probe() fills in the GPIO lookup table by manually populating an
> array of gpiod_lookup structures. Use the existing GPIO_LOOKUP() helper
> macro instead, to relax a dependency on the gpiod_lookup structure's
> member names.
>
> Signed-off-by: Geert Uytterhoeven <[email protected]>
> Cc: [email protected]
> ---
> While this patch is a dependency for "[PATCH v6 4/8] gpiolib: Add
> support for GPIO lookup by line name", it can be applied independently.
> But an Acked-by would be nice, too.
I simply applied this patch for v5.7 in the GPIO tree since I am the
maintainer of this platform, and I might want to change stuff around
Integrator next cycle so it's good to have this covered.
Yours,
Linus Walleij
On Tue, Mar 24, 2020 at 2:57 PM Geert Uytterhoeven
<[email protected]> wrote:
> The GPIO Aggregator will need a method to forward a .set_config() call
> to its parent gpiochip. This requires obtaining the gpio_chip and
> offset for a given gpio_desc. While gpiod_to_chip() is public,
> gpio_chip_hwgpio() is not, so there is currently no method to obtain the
> needed GPIO offset parameter.
>
> Hence introduce a public gpiod_set_config() helper, which invokes the
> .set_config() callback through a gpio_desc pointer, like is done for
> most other gpio_chip callbacks.
>
> Rewrite the existing gpiod_set_debounce() helper as a wrapper around
> gpiod_set_config(), to avoid duplication.
>
> Signed-off-by: Geert Uytterhoeven <[email protected]>
> ---
> v6:
> - New.
This is nice, I tried to actually just apply this (you also sent some
two cleanups that I tried to apply) byt Yue's cleanup patch
commit d18fddff061d2796525e6d4a958cb3d30aed8efd
"gpiolib: Remove duplicated function gpio_do_set_config()"
makes none of them apply :/
Yours,
Linus Walleij
Hi Linus,
On Thu, Mar 26, 2020 at 10:26 PM Linus Walleij <[email protected]> wrote:
> On Tue, Mar 24, 2020 at 2:57 PM Geert Uytterhoeven
> <[email protected]> wrote:
> > The GPIO Aggregator will need a method to forward a .set_config() call
> > to its parent gpiochip. This requires obtaining the gpio_chip and
> > offset for a given gpio_desc. While gpiod_to_chip() is public,
> > gpio_chip_hwgpio() is not, so there is currently no method to obtain the
> > needed GPIO offset parameter.
> >
> > Hence introduce a public gpiod_set_config() helper, which invokes the
> > .set_config() callback through a gpio_desc pointer, like is done for
> > most other gpio_chip callbacks.
> >
> > Rewrite the existing gpiod_set_debounce() helper as a wrapper around
> > gpiod_set_config(), to avoid duplication.
> >
> > Signed-off-by: Geert Uytterhoeven <[email protected]>
> > ---
> > v6:
> > - New.
>
> This is nice, I tried to actually just apply this (you also sent some
> two cleanups that I tried to apply) byt Yue's cleanup patch
> commit d18fddff061d2796525e6d4a958cb3d30aed8efd
> "gpiolib: Remove duplicated function gpio_do_set_config()"
> makes none of them apply :/
/me confused.
That commit was reverted later, so it shouldn't matter.
I have just verified, and both my full series and just this single
patch, do apply fine to all of current gpio/for-next, linus/master, and
next-20200327. They even apply fine to gpio/for-next before or after
the two cleanups I sent, too.
What am I missing?
Thanks!
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
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
On Fri, Mar 27, 2020 at 9:45 AM Geert Uytterhoeven <[email protected]> wrote:
> On Thu, Mar 26, 2020 at 10:26 PM Linus Walleij <[email protected]> wrote:
> > On Tue, Mar 24, 2020 at 2:57 PM Geert Uytterhoeven
> > <[email protected]> wrote:
> > > The GPIO Aggregator will need a method to forward a .set_config() call
> > > to its parent gpiochip. This requires obtaining the gpio_chip and
> > > offset for a given gpio_desc. While gpiod_to_chip() is public,
> > > gpio_chip_hwgpio() is not, so there is currently no method to obtain the
> > > needed GPIO offset parameter.
> > >
> > > Hence introduce a public gpiod_set_config() helper, which invokes the
> > > .set_config() callback through a gpio_desc pointer, like is done for
> > > most other gpio_chip callbacks.
> > >
> > > Rewrite the existing gpiod_set_debounce() helper as a wrapper around
> > > gpiod_set_config(), to avoid duplication.
> > >
> > > Signed-off-by: Geert Uytterhoeven <[email protected]>
> > > ---
> > > v6:
> > > - New.
> >
> > This is nice, I tried to actually just apply this (you also sent some
> > two cleanups that I tried to apply) byt Yue's cleanup patch
> > commit d18fddff061d2796525e6d4a958cb3d30aed8efd
> > "gpiolib: Remove duplicated function gpio_do_set_config()"
> > makes none of them apply :/
>
> /me confused.
>
> That commit was reverted later, so it shouldn't matter.
>
> I have just verified, and both my full series and just this single
> patch, do apply fine to all of current gpio/for-next, linus/master, and
> next-20200327. They even apply fine to gpio/for-next before or after
> the two cleanups I sent, too.
>
> What am I missing?
Ah I see, it is because my development branch is based on
v5.6-rc1. So I have to merge in a later -rc where this revert
is applied so that this applies.
Yours,
Linus Walleij
On Tue, Mar 24, 2020 at 2:57 PM Geert Uytterhoeven
<[email protected]> wrote:
> The GPIO Aggregator will need a method to forward a .set_config() call
> to its parent gpiochip. This requires obtaining the gpio_chip and
> offset for a given gpio_desc. While gpiod_to_chip() is public,
> gpio_chip_hwgpio() is not, so there is currently no method to obtain the
> needed GPIO offset parameter.
>
> Hence introduce a public gpiod_set_config() helper, which invokes the
> .set_config() callback through a gpio_desc pointer, like is done for
> most other gpio_chip callbacks.
>
> Rewrite the existing gpiod_set_debounce() helper as a wrapper around
> gpiod_set_config(), to avoid duplication.
>
> Signed-off-by: Geert Uytterhoeven <[email protected]>
> ---
> v6:
> - New.
Patch applied in preparation for the next kernel cycle
so we get Geert's patch stack down.
Yours,
Linus Walleij