* [PATCH v3 01/13] gpio: aggregator: reorder functions to prepare for configfs introduction
2025-02-16 12:58 [PATCH v3 00/13] Introduce configfs-based interface for gpio-aggregator Koichiro Den
@ 2025-02-16 12:58 ` Koichiro Den
2025-02-17 13:07 ` Geert Uytterhoeven
2025-02-16 12:58 ` [PATCH v3 02/13] gpio: aggregator: protect driver attr handlers against module unload Koichiro Den
` (12 subsequent siblings)
13 siblings, 1 reply; 24+ messages in thread
From: Koichiro Den @ 2025-02-16 12:58 UTC (permalink / raw)
To: linux-gpio
Cc: brgl, geert+renesas, linus.walleij, maciej.borzecki, linux-kernel
Reorder functions in drivers/gpio/gpio-aggregator.c to prepare for the
configfs-based interface additions in subsequent commits. Arrange the
code so that the configfs implementations will appear above the existing
sysfs-specific code, since the latter will partly depend on the configfs
interface implementations when it starts to expose the settings to
configfs.
The order in drivers/gpio/gpio-aggregator.c will be as follows:
* Basic gpio_aggregator/gpio_aggregator_line representations
* Common utility functions
* GPIO Forwarder implementations
* Configfs interface implementations
* Sysfs interface implementations
* Platform device implementations
* Module init/exit implementations
This separate commit ensures a clean diff for the subsequent commits.
No functional change.
Signed-off-by: Koichiro Den <koichiro.den@canonical.com>
---
drivers/gpio/gpio-aggregator.c | 352 +++++++++++++++++----------------
1 file changed, 178 insertions(+), 174 deletions(-)
diff --git a/drivers/gpio/gpio-aggregator.c b/drivers/gpio/gpio-aggregator.c
index 65f41cc3eafc..570cd1ff8cc2 100644
--- a/drivers/gpio/gpio-aggregator.c
+++ b/drivers/gpio/gpio-aggregator.c
@@ -61,180 +61,6 @@ static int aggr_add_gpio(struct gpio_aggregator *aggr, const char *key,
return 0;
}
-static int aggr_parse(struct gpio_aggregator *aggr)
-{
- char *args = skip_spaces(aggr->args);
- char *name, *offsets, *p;
- unsigned int i, n = 0;
- int error = 0;
-
- unsigned long *bitmap __free(bitmap) =
- bitmap_alloc(AGGREGATOR_MAX_GPIOS, GFP_KERNEL);
- if (!bitmap)
- return -ENOMEM;
-
- args = next_arg(args, &name, &p);
- while (*args) {
- args = next_arg(args, &offsets, &p);
-
- p = get_options(offsets, 0, &error);
- if (error == 0 || *p) {
- /* Named GPIO line */
- error = aggr_add_gpio(aggr, name, U16_MAX, &n);
- if (error)
- return error;
-
- name = offsets;
- continue;
- }
-
- /* GPIO chip + offset(s) */
- error = bitmap_parselist(offsets, bitmap, AGGREGATOR_MAX_GPIOS);
- if (error) {
- pr_err("Cannot parse %s: %d\n", offsets, error);
- return error;
- }
-
- for_each_set_bit(i, bitmap, AGGREGATOR_MAX_GPIOS) {
- error = aggr_add_gpio(aggr, name, i, &n);
- if (error)
- return error;
- }
-
- args = next_arg(args, &name, &p);
- }
-
- if (!n) {
- pr_err("No GPIOs specified\n");
- return -EINVAL;
- }
-
- return 0;
-}
-
-static ssize_t new_device_store(struct device_driver *driver, const char *buf,
- size_t count)
-{
- struct gpio_aggregator *aggr;
- struct platform_device *pdev;
- int res, id;
-
- /* kernfs guarantees string termination, so count + 1 is safe */
- aggr = kzalloc(sizeof(*aggr) + count + 1, GFP_KERNEL);
- if (!aggr)
- return -ENOMEM;
-
- memcpy(aggr->args, buf, count + 1);
-
- aggr->lookups = kzalloc(struct_size(aggr->lookups, table, 1),
- GFP_KERNEL);
- if (!aggr->lookups) {
- res = -ENOMEM;
- goto free_ga;
- }
-
- mutex_lock(&gpio_aggregator_lock);
- id = idr_alloc(&gpio_aggregator_idr, aggr, 0, 0, GFP_KERNEL);
- mutex_unlock(&gpio_aggregator_lock);
-
- if (id < 0) {
- res = id;
- goto free_table;
- }
-
- aggr->lookups->dev_id = kasprintf(GFP_KERNEL, "%s.%d", DRV_NAME, id);
- if (!aggr->lookups->dev_id) {
- res = -ENOMEM;
- goto remove_idr;
- }
-
- res = aggr_parse(aggr);
- if (res)
- goto free_dev_id;
-
- gpiod_add_lookup_table(aggr->lookups);
-
- pdev = platform_device_register_simple(DRV_NAME, id, NULL, 0);
- if (IS_ERR(pdev)) {
- res = PTR_ERR(pdev);
- goto remove_table;
- }
-
- aggr->pdev = pdev;
- return count;
-
-remove_table:
- gpiod_remove_lookup_table(aggr->lookups);
-free_dev_id:
- kfree(aggr->lookups->dev_id);
-remove_idr:
- mutex_lock(&gpio_aggregator_lock);
- idr_remove(&gpio_aggregator_idr, id);
- mutex_unlock(&gpio_aggregator_lock);
-free_table:
- kfree(aggr->lookups);
-free_ga:
- kfree(aggr);
- return res;
-}
-
-static DRIVER_ATTR_WO(new_device);
-
-static void gpio_aggregator_free(struct gpio_aggregator *aggr)
-{
- platform_device_unregister(aggr->pdev);
- gpiod_remove_lookup_table(aggr->lookups);
- kfree(aggr->lookups->dev_id);
- kfree(aggr->lookups);
- kfree(aggr);
-}
-
-static ssize_t delete_device_store(struct device_driver *driver,
- const char *buf, size_t count)
-{
- struct gpio_aggregator *aggr;
- unsigned int id;
- int error;
-
- if (!str_has_prefix(buf, DRV_NAME "."))
- return -EINVAL;
-
- error = kstrtouint(buf + strlen(DRV_NAME "."), 10, &id);
- if (error)
- return error;
-
- mutex_lock(&gpio_aggregator_lock);
- aggr = idr_remove(&gpio_aggregator_idr, id);
- mutex_unlock(&gpio_aggregator_lock);
- if (!aggr)
- return -ENOENT;
-
- gpio_aggregator_free(aggr);
- return count;
-}
-static DRIVER_ATTR_WO(delete_device);
-
-static struct attribute *gpio_aggregator_attrs[] = {
- &driver_attr_new_device.attr,
- &driver_attr_delete_device.attr,
- NULL
-};
-ATTRIBUTE_GROUPS(gpio_aggregator);
-
-static int __exit gpio_aggregator_idr_remove(int id, void *p, void *data)
-{
- gpio_aggregator_free(p);
- return 0;
-}
-
-static void __exit gpio_aggregator_remove_all(void)
-{
- mutex_lock(&gpio_aggregator_lock);
- idr_for_each(&gpio_aggregator_idr, gpio_aggregator_idr_remove, NULL);
- idr_destroy(&gpio_aggregator_idr);
- mutex_unlock(&gpio_aggregator_lock);
-}
-
/*
* GPIO Forwarder
@@ -559,6 +385,170 @@ static struct gpiochip_fwd *gpiochip_fwd_create(struct device *dev,
}
+/*
+ * Sysfs interface
+ */
+static int aggr_parse(struct gpio_aggregator *aggr)
+{
+ char *args = skip_spaces(aggr->args);
+ char *name, *offsets, *p;
+ unsigned int i, n = 0;
+ int error = 0;
+
+ unsigned long *bitmap __free(bitmap) =
+ bitmap_alloc(AGGREGATOR_MAX_GPIOS, GFP_KERNEL);
+ if (!bitmap)
+ return -ENOMEM;
+
+ args = next_arg(args, &name, &p);
+ while (*args) {
+ args = next_arg(args, &offsets, &p);
+
+ p = get_options(offsets, 0, &error);
+ if (error == 0 || *p) {
+ /* Named GPIO line */
+ error = aggr_add_gpio(aggr, name, U16_MAX, &n);
+ if (error)
+ return error;
+
+ name = offsets;
+ continue;
+ }
+
+ /* GPIO chip + offset(s) */
+ error = bitmap_parselist(offsets, bitmap, AGGREGATOR_MAX_GPIOS);
+ if (error) {
+ pr_err("Cannot parse %s: %d\n", offsets, error);
+ return error;
+ }
+
+ for_each_set_bit(i, bitmap, AGGREGATOR_MAX_GPIOS) {
+ error = aggr_add_gpio(aggr, name, i, &n);
+ if (error)
+ return error;
+ }
+
+ args = next_arg(args, &name, &p);
+ }
+
+ if (!n) {
+ pr_err("No GPIOs specified\n");
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+static ssize_t new_device_store(struct device_driver *driver, const char *buf,
+ size_t count)
+{
+ struct gpio_aggregator *aggr;
+ struct platform_device *pdev;
+ int res, id;
+
+ /* kernfs guarantees string termination, so count + 1 is safe */
+ aggr = kzalloc(sizeof(*aggr) + count + 1, GFP_KERNEL);
+ if (!aggr)
+ return -ENOMEM;
+
+ memcpy(aggr->args, buf, count + 1);
+
+ aggr->lookups = kzalloc(struct_size(aggr->lookups, table, 1),
+ GFP_KERNEL);
+ if (!aggr->lookups) {
+ res = -ENOMEM;
+ goto free_ga;
+ }
+
+ mutex_lock(&gpio_aggregator_lock);
+ id = idr_alloc(&gpio_aggregator_idr, aggr, 0, 0, GFP_KERNEL);
+ mutex_unlock(&gpio_aggregator_lock);
+
+ if (id < 0) {
+ res = id;
+ goto free_table;
+ }
+
+ aggr->lookups->dev_id = kasprintf(GFP_KERNEL, "%s.%d", DRV_NAME, id);
+ if (!aggr->lookups->dev_id) {
+ res = -ENOMEM;
+ goto remove_idr;
+ }
+
+ res = aggr_parse(aggr);
+ if (res)
+ goto free_dev_id;
+
+ gpiod_add_lookup_table(aggr->lookups);
+
+ pdev = platform_device_register_simple(DRV_NAME, id, NULL, 0);
+ if (IS_ERR(pdev)) {
+ res = PTR_ERR(pdev);
+ goto remove_table;
+ }
+
+ aggr->pdev = pdev;
+ return count;
+
+remove_table:
+ gpiod_remove_lookup_table(aggr->lookups);
+free_dev_id:
+ kfree(aggr->lookups->dev_id);
+remove_idr:
+ mutex_lock(&gpio_aggregator_lock);
+ idr_remove(&gpio_aggregator_idr, id);
+ mutex_unlock(&gpio_aggregator_lock);
+free_table:
+ kfree(aggr->lookups);
+free_ga:
+ kfree(aggr);
+ return res;
+}
+
+static DRIVER_ATTR_WO(new_device);
+
+static void gpio_aggregator_free(struct gpio_aggregator *aggr)
+{
+ platform_device_unregister(aggr->pdev);
+ gpiod_remove_lookup_table(aggr->lookups);
+ kfree(aggr->lookups->dev_id);
+ kfree(aggr->lookups);
+ kfree(aggr);
+}
+
+static ssize_t delete_device_store(struct device_driver *driver,
+ const char *buf, size_t count)
+{
+ struct gpio_aggregator *aggr;
+ unsigned int id;
+ int error;
+
+ if (!str_has_prefix(buf, DRV_NAME "."))
+ return -EINVAL;
+
+ error = kstrtouint(buf + strlen(DRV_NAME "."), 10, &id);
+ if (error)
+ return error;
+
+ mutex_lock(&gpio_aggregator_lock);
+ aggr = idr_remove(&gpio_aggregator_idr, id);
+ mutex_unlock(&gpio_aggregator_lock);
+ if (!aggr)
+ return -ENOENT;
+
+ gpio_aggregator_free(aggr);
+ return count;
+}
+static DRIVER_ATTR_WO(delete_device);
+
+static struct attribute *gpio_aggregator_attrs[] = {
+ &driver_attr_new_device.attr,
+ &driver_attr_delete_device.attr,
+ NULL
+};
+ATTRIBUTE_GROUPS(gpio_aggregator);
+
+
/*
* GPIO Aggregator platform device
*/
@@ -616,6 +606,20 @@ static struct platform_driver gpio_aggregator_driver = {
},
};
+static int __exit gpio_aggregator_idr_remove(int id, void *p, void *data)
+{
+ gpio_aggregator_free(p);
+ return 0;
+}
+
+static void __exit gpio_aggregator_remove_all(void)
+{
+ mutex_lock(&gpio_aggregator_lock);
+ idr_for_each(&gpio_aggregator_idr, gpio_aggregator_idr_remove, NULL);
+ idr_destroy(&gpio_aggregator_idr);
+ mutex_unlock(&gpio_aggregator_lock);
+}
+
static int __init gpio_aggregator_init(void)
{
return platform_driver_register(&gpio_aggregator_driver);
--
2.45.2
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [PATCH v3 01/13] gpio: aggregator: reorder functions to prepare for configfs introduction
2025-02-16 12:58 ` [PATCH v3 01/13] gpio: aggregator: reorder functions to prepare for configfs introduction Koichiro Den
@ 2025-02-17 13:07 ` Geert Uytterhoeven
2025-02-17 14:34 ` Koichiro Den
0 siblings, 1 reply; 24+ messages in thread
From: Geert Uytterhoeven @ 2025-02-17 13:07 UTC (permalink / raw)
To: Koichiro Den
Cc: linux-gpio, brgl, linus.walleij, maciej.borzecki, linux-kernel
On Sun, 16 Feb 2025 at 13:58, Koichiro Den <koichiro.den@canonical.com> wrote:
> Reorder functions in drivers/gpio/gpio-aggregator.c to prepare for the
> configfs-based interface additions in subsequent commits. Arrange the
> code so that the configfs implementations will appear above the existing
> sysfs-specific code, since the latter will partly depend on the configfs
> interface implementations when it starts to expose the settings to
> configfs.
>
> The order in drivers/gpio/gpio-aggregator.c will be as follows:
>
> * Basic gpio_aggregator/gpio_aggregator_line representations
> * Common utility functions
> * GPIO Forwarder implementations
> * Configfs interface implementations
> * Sysfs interface implementations
> * Platform device implementations
> * Module init/exit implementations
>
> This separate commit ensures a clean diff for the subsequent commits.
>
> No functional change.
>
> Signed-off-by: Koichiro Den <koichiro.den@canonical.com>
My
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
is still valid.
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
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH v3 01/13] gpio: aggregator: reorder functions to prepare for configfs introduction
2025-02-17 13:07 ` Geert Uytterhoeven
@ 2025-02-17 14:34 ` Koichiro Den
0 siblings, 0 replies; 24+ messages in thread
From: Koichiro Den @ 2025-02-17 14:34 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: linux-gpio, brgl, linus.walleij, maciej.borzecki, linux-kernel
On Mon, Feb 17, 2025 at 02:07:53PM GMT, Geert Uytterhoeven wrote:
> On Sun, 16 Feb 2025 at 13:58, Koichiro Den <koichiro.den@canonical.com> wrote:
> > Reorder functions in drivers/gpio/gpio-aggregator.c to prepare for the
> > configfs-based interface additions in subsequent commits. Arrange the
> > code so that the configfs implementations will appear above the existing
> > sysfs-specific code, since the latter will partly depend on the configfs
> > interface implementations when it starts to expose the settings to
> > configfs.
> >
> > The order in drivers/gpio/gpio-aggregator.c will be as follows:
> >
> > * Basic gpio_aggregator/gpio_aggregator_line representations
> > * Common utility functions
> > * GPIO Forwarder implementations
> > * Configfs interface implementations
> > * Sysfs interface implementations
> > * Platform device implementations
> > * Module init/exit implementations
> >
> > This separate commit ensures a clean diff for the subsequent commits.
> >
> > No functional change.
> >
> > Signed-off-by: Koichiro Den <koichiro.den@canonical.com>
>
> My
> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
> is still valid.
Thank you, let me add the tag to the two commits in v4 since they remain
unchanged.
Koichiro
>
> 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
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH v3 02/13] gpio: aggregator: protect driver attr handlers against module unload
2025-02-16 12:58 [PATCH v3 00/13] Introduce configfs-based interface for gpio-aggregator Koichiro Den
2025-02-16 12:58 ` [PATCH v3 01/13] gpio: aggregator: reorder functions to prepare for configfs introduction Koichiro Den
@ 2025-02-16 12:58 ` Koichiro Den
2025-02-16 12:58 ` [PATCH v3 03/13] gpio: pseudo: common helper functions for pseudo gpio devices Koichiro Den
` (11 subsequent siblings)
13 siblings, 0 replies; 24+ messages in thread
From: Koichiro Den @ 2025-02-16 12:58 UTC (permalink / raw)
To: linux-gpio
Cc: brgl, geert+renesas, linus.walleij, maciej.borzecki, linux-kernel
Both new_device_store and delete_device_store touch module global
resources (e.g. gpio_aggregator_lock). To prevent race conditions with
module unload, a reference needs to be held.
Add try_module_get() in these handlers.
For new_device_store, this eliminates what appears to be the most dangerous
scenario: if an id is allocated from gpio_aggregator_idr but
platform_device_register has not yet been called or completed, a concurrent
module unload could fail to unregister/delete the device, leaving behind a
dangling platform device/GPIO forwarder. This can result in various issues.
The following simple reproducer demonstrates these problems:
#!/bin/bash
while :; do
# note: whether 'gpiochip0 0' exists or not does not matter.
echo 'gpiochip0 0' > /sys/bus/platform/drivers/gpio-aggregator/new_device
done &
while :; do
modprobe gpio-aggregator
modprobe -r gpio-aggregator
done &
wait
Starting with the following warning, several kinds of warnings will appear
and the system may become unstable:
------------[ cut here ]------------
list_del corruption, ffff888103e2e980->next is LIST_POISON1 (dead000000000100)
WARNING: CPU: 1 PID: 1327 at lib/list_debug.c:56 __list_del_entry_valid_or_report+0xa3/0x120
[...]
RIP: 0010:__list_del_entry_valid_or_report+0xa3/0x120
[...]
Call Trace:
<TASK>
? __list_del_entry_valid_or_report+0xa3/0x120
? __warn.cold+0x93/0xf2
? __list_del_entry_valid_or_report+0xa3/0x120
? report_bug+0xe6/0x170
? __irq_work_queue_local+0x39/0xe0
? handle_bug+0x58/0x90
? exc_invalid_op+0x13/0x60
? asm_exc_invalid_op+0x16/0x20
? __list_del_entry_valid_or_report+0xa3/0x120
gpiod_remove_lookup_table+0x22/0x60
new_device_store+0x315/0x350 [gpio_aggregator]
kernfs_fop_write_iter+0x137/0x1f0
vfs_write+0x262/0x430
ksys_write+0x60/0xd0
do_syscall_64+0x6c/0x180
entry_SYSCALL_64_after_hwframe+0x76/0x7e
[...]
</TASK>
---[ end trace 0000000000000000 ]---
Fixes: 828546e24280 ("gpio: Add GPIO Aggregator")
Signed-off-by: Koichiro Den <koichiro.den@canonical.com>
---
drivers/gpio/gpio-aggregator.c | 20 +++++++++++++++++---
1 file changed, 17 insertions(+), 3 deletions(-)
diff --git a/drivers/gpio/gpio-aggregator.c b/drivers/gpio/gpio-aggregator.c
index 570cd1ff8cc2..893cd56de867 100644
--- a/drivers/gpio/gpio-aggregator.c
+++ b/drivers/gpio/gpio-aggregator.c
@@ -446,10 +446,15 @@ static ssize_t new_device_store(struct device_driver *driver, const char *buf,
struct platform_device *pdev;
int res, id;
+ if (!try_module_get(THIS_MODULE))
+ return -ENOENT;
+
/* kernfs guarantees string termination, so count + 1 is safe */
aggr = kzalloc(sizeof(*aggr) + count + 1, GFP_KERNEL);
- if (!aggr)
- return -ENOMEM;
+ if (!aggr) {
+ res = -ENOMEM;
+ goto put_module;
+ }
memcpy(aggr->args, buf, count + 1);
@@ -488,6 +493,7 @@ static ssize_t new_device_store(struct device_driver *driver, const char *buf,
}
aggr->pdev = pdev;
+ module_put(THIS_MODULE);
return count;
remove_table:
@@ -502,6 +508,8 @@ static ssize_t new_device_store(struct device_driver *driver, const char *buf,
kfree(aggr->lookups);
free_ga:
kfree(aggr);
+put_module:
+ module_put(THIS_MODULE);
return res;
}
@@ -530,13 +538,19 @@ static ssize_t delete_device_store(struct device_driver *driver,
if (error)
return error;
+ if (!try_module_get(THIS_MODULE))
+ return -ENOENT;
+
mutex_lock(&gpio_aggregator_lock);
aggr = idr_remove(&gpio_aggregator_idr, id);
mutex_unlock(&gpio_aggregator_lock);
- if (!aggr)
+ if (!aggr) {
+ module_put(THIS_MODULE);
return -ENOENT;
+ }
gpio_aggregator_free(aggr);
+ module_put(THIS_MODULE);
return count;
}
static DRIVER_ATTR_WO(delete_device);
--
2.45.2
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH v3 03/13] gpio: pseudo: common helper functions for pseudo gpio devices
2025-02-16 12:58 [PATCH v3 00/13] Introduce configfs-based interface for gpio-aggregator Koichiro Den
2025-02-16 12:58 ` [PATCH v3 01/13] gpio: aggregator: reorder functions to prepare for configfs introduction Koichiro Den
2025-02-16 12:58 ` [PATCH v3 02/13] gpio: aggregator: protect driver attr handlers against module unload Koichiro Den
@ 2025-02-16 12:58 ` Koichiro Den
2025-02-16 12:58 ` [PATCH v3 04/13] gpio: sim: convert to use gpio-pseudo utilities Koichiro Den
` (10 subsequent siblings)
13 siblings, 0 replies; 24+ messages in thread
From: Koichiro Den @ 2025-02-16 12:58 UTC (permalink / raw)
To: linux-gpio
Cc: brgl, geert+renesas, linus.walleij, maciej.borzecki, linux-kernel
Both gpio-sim and gpio-virtuser share a mechanism to instantiate a
platform device and wait synchronously for probe completion.
With gpio-aggregator adopting the same approach in a later commit for
its configfs interface, it's time to factor out the common code.
Add gpio-pseudo.[ch] to house helper functions used by all the pseudo
GPIO device implementations.
No functional change.
Signed-off-by: Koichiro Den <koichiro.den@canonical.com>
---
drivers/gpio/Kconfig | 4 ++
drivers/gpio/Makefile | 1 +
drivers/gpio/gpio-pseudo.c | 86 ++++++++++++++++++++++++++++++++++++++
drivers/gpio/gpio-pseudo.h | 24 +++++++++++
4 files changed, 115 insertions(+)
create mode 100644 drivers/gpio/gpio-pseudo.c
create mode 100644 drivers/gpio/gpio-pseudo.h
diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index 56c1f30ac195..1e2c95e03a95 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -1863,6 +1863,10 @@ config GPIO_MPSSE
endmenu
+# This symbol is selected by some pseudo gpio device implementations
+config GPIO_PSEUDO
+ bool
+
menu "Virtual GPIO drivers"
config GPIO_AGGREGATOR
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index af3ba4d81b58..5eb54147a1ab 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -136,6 +136,7 @@ obj-$(CONFIG_GPIO_PISOSR) += gpio-pisosr.o
obj-$(CONFIG_GPIO_PL061) += gpio-pl061.o
obj-$(CONFIG_GPIO_PMIC_EIC_SPRD) += gpio-pmic-eic-sprd.o
obj-$(CONFIG_GPIO_POLARFIRE_SOC) += gpio-mpfs.o
+obj-$(CONFIG_GPIO_PSEUDO) += gpio-pseudo.o
obj-$(CONFIG_GPIO_PXA) += gpio-pxa.o
obj-$(CONFIG_GPIO_RASPBERRYPI_EXP) += gpio-raspberrypi-exp.o
obj-$(CONFIG_GPIO_RC5T583) += gpio-rc5t583.o
diff --git a/drivers/gpio/gpio-pseudo.c b/drivers/gpio/gpio-pseudo.c
new file mode 100644
index 000000000000..6e3da05440d8
--- /dev/null
+++ b/drivers/gpio/gpio-pseudo.c
@@ -0,0 +1,86 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Helper functions for Pseudo GPIOs
+ *
+ * Copyright 2025 Canonical Ltd.
+ */
+
+#include "gpio-pseudo.h"
+
+static int pseudo_gpio_notifier_call(struct notifier_block *nb,
+ unsigned long action,
+ void *data)
+{
+ struct pseudo_gpio_common *common;
+ struct device *dev = data;
+
+ common = container_of(nb, struct pseudo_gpio_common, bus_notifier);
+ if (!device_match_name(dev, common->name))
+ return NOTIFY_DONE;
+
+ switch (action) {
+ case BUS_NOTIFY_BOUND_DRIVER:
+ common->driver_bound = true;
+ break;
+ case BUS_NOTIFY_DRIVER_NOT_BOUND:
+ common->driver_bound = false;
+ break;
+ default:
+ return NOTIFY_DONE;
+ }
+
+ complete(&common->probe_completion);
+ return NOTIFY_OK;
+}
+
+void pseudo_gpio_init(struct pseudo_gpio_common *common)
+{
+ memset(common, 0, sizeof(*common));
+ init_completion(&common->probe_completion);
+ common->bus_notifier.notifier_call = pseudo_gpio_notifier_call;
+}
+EXPORT_SYMBOL_GPL(pseudo_gpio_init);
+
+int pseudo_gpio_register(struct pseudo_gpio_common *common,
+ struct platform_device_info *pdevinfo)
+{
+ struct platform_device *pdev;
+ char *name;
+
+ name = kasprintf(GFP_KERNEL, "%s.%u", pdevinfo->name, pdevinfo->id);
+ if (!name)
+ return -ENOMEM;
+
+ common->driver_bound = false;
+ common->name = name;
+ reinit_completion(&common->probe_completion);
+ bus_register_notifier(&platform_bus_type, &common->bus_notifier);
+
+ pdev = platform_device_register_full(pdevinfo);
+ if (IS_ERR(pdev)) {
+ bus_unregister_notifier(&platform_bus_type, &common->bus_notifier);
+ kfree(common->name);
+ return PTR_ERR(pdev);
+ }
+
+ wait_for_completion(&common->probe_completion);
+ bus_unregister_notifier(&platform_bus_type, &common->bus_notifier);
+
+ if (!common->driver_bound) {
+ platform_device_unregister(pdev);
+ kfree(common->name);
+ return -ENXIO;
+ }
+
+ common->pdev = pdev;
+ return 0;
+}
+EXPORT_SYMBOL_GPL(pseudo_gpio_register);
+
+void pseudo_gpio_unregister(struct pseudo_gpio_common *common)
+{
+ platform_device_unregister(common->pdev);
+ kfree(common->name);
+ common->pdev = NULL;
+}
+EXPORT_SYMBOL_GPL(pseudo_gpio_unregister);
diff --git a/drivers/gpio/gpio-pseudo.h b/drivers/gpio/gpio-pseudo.h
new file mode 100644
index 000000000000..093112b6cce5
--- /dev/null
+++ b/drivers/gpio/gpio-pseudo.h
@@ -0,0 +1,24 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+#ifndef GPIO_PSEUDO_H
+#define GPIO_PSEUDO_H
+
+#include <linux/completion.h>
+#include <linux/platform_device.h>
+
+struct pseudo_gpio_common {
+ struct platform_device *pdev;
+ const char *name;
+
+ /* Synchronize with probe */
+ struct notifier_block bus_notifier;
+ struct completion probe_completion;
+ bool driver_bound;
+};
+
+void pseudo_gpio_init(struct pseudo_gpio_common *common);
+int pseudo_gpio_register(struct pseudo_gpio_common *common,
+ struct platform_device_info *pdevinfo);
+void pseudo_gpio_unregister(struct pseudo_gpio_common *common);
+
+#endif /* GPIO_PSEUDO_H */
--
2.45.2
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH v3 04/13] gpio: sim: convert to use gpio-pseudo utilities
2025-02-16 12:58 [PATCH v3 00/13] Introduce configfs-based interface for gpio-aggregator Koichiro Den
` (2 preceding siblings ...)
2025-02-16 12:58 ` [PATCH v3 03/13] gpio: pseudo: common helper functions for pseudo gpio devices Koichiro Den
@ 2025-02-16 12:58 ` Koichiro Den
2025-02-16 12:58 ` [PATCH v3 05/13] gpio: virtuser: " Koichiro Den
` (9 subsequent siblings)
13 siblings, 0 replies; 24+ messages in thread
From: Koichiro Den @ 2025-02-16 12:58 UTC (permalink / raw)
To: linux-gpio
Cc: brgl, geert+renesas, linus.walleij, maciej.borzecki, linux-kernel
Update gpio-sim to use the new gpio-pseudo helper functions for
synchronized platform device creation, reducing code duplication.
No functional change.
Signed-off-by: Koichiro Den <koichiro.den@canonical.com>
---
drivers/gpio/Kconfig | 1 +
drivers/gpio/gpio-sim.c | 84 ++++++-----------------------------------
2 files changed, 13 insertions(+), 72 deletions(-)
diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index 1e2c95e03a95..c482e3494bac 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -1913,6 +1913,7 @@ config GPIO_SIM
tristate "GPIO Simulator Module"
select IRQ_SIM
select CONFIGFS_FS
+ select GPIO_PSEUDO
help
This enables the GPIO simulator - a configfs-based GPIO testing
driver.
diff --git a/drivers/gpio/gpio-sim.c b/drivers/gpio/gpio-sim.c
index a086087ada17..45dbf16bee12 100644
--- a/drivers/gpio/gpio-sim.c
+++ b/drivers/gpio/gpio-sim.c
@@ -10,7 +10,6 @@
#include <linux/array_size.h>
#include <linux/bitmap.h>
#include <linux/cleanup.h>
-#include <linux/completion.h>
#include <linux/configfs.h>
#include <linux/device.h>
#include <linux/err.h>
@@ -37,6 +36,8 @@
#include <linux/sysfs.h>
#include <linux/types.h>
+#include "gpio-pseudo.h"
+
#define GPIO_SIM_NGPIO_MAX 1024
#define GPIO_SIM_PROP_MAX 4 /* Max 3 properties + sentinel. */
#define GPIO_SIM_NUM_ATTRS 3 /* value, pull and sentinel */
@@ -541,14 +542,9 @@ static struct platform_driver gpio_sim_driver = {
};
struct gpio_sim_device {
+ struct pseudo_gpio_common common;
struct config_group group;
- /*
- * If pdev is NULL, the device is 'pending' (waiting for configuration).
- * Once the pointer is assigned, the device has been created and the
- * item is 'live'.
- */
- struct platform_device *pdev;
int id;
/*
@@ -562,46 +558,11 @@ struct gpio_sim_device {
*/
struct mutex lock;
- /*
- * This is used to synchronously wait for the driver's probe to complete
- * and notify the user-space about any errors.
- */
- struct notifier_block bus_notifier;
- struct completion probe_completion;
- bool driver_bound;
-
struct gpiod_hog *hogs;
struct list_head bank_list;
};
-/* This is called with dev->lock already taken. */
-static int gpio_sim_bus_notifier_call(struct notifier_block *nb,
- unsigned long action, void *data)
-{
- struct gpio_sim_device *simdev = container_of(nb,
- struct gpio_sim_device,
- bus_notifier);
- struct device *dev = data;
- char devname[32];
-
- snprintf(devname, sizeof(devname), "gpio-sim.%u", simdev->id);
-
- if (!device_match_name(dev, devname))
- return NOTIFY_DONE;
-
- if (action == BUS_NOTIFY_BOUND_DRIVER)
- simdev->driver_bound = true;
- else if (action == BUS_NOTIFY_DRIVER_NOT_BOUND)
- simdev->driver_bound = false;
- else
- return NOTIFY_DONE;
-
- complete(&simdev->probe_completion);
-
- return NOTIFY_OK;
-}
-
static struct gpio_sim_device *to_gpio_sim_device(struct config_item *item)
{
struct config_group *group = to_config_group(item);
@@ -708,7 +669,7 @@ static bool gpio_sim_device_is_live(struct gpio_sim_device *dev)
{
lockdep_assert_held(&dev->lock);
- return !!dev->pdev;
+ return !!dev->common.pdev;
}
static char *gpio_sim_strdup_trimmed(const char *str, size_t count)
@@ -730,7 +691,7 @@ static ssize_t gpio_sim_device_config_dev_name_show(struct config_item *item,
guard(mutex)(&dev->lock);
- pdev = dev->pdev;
+ pdev = dev->common.pdev;
if (pdev)
return sprintf(page, "%s\n", dev_name(&pdev->dev));
@@ -939,7 +900,6 @@ static int gpio_sim_device_activate(struct gpio_sim_device *dev)
{
struct platform_device_info pdevinfo;
struct fwnode_handle *swnode;
- struct platform_device *pdev;
struct gpio_sim_bank *bank;
int ret;
@@ -981,31 +941,13 @@ static int gpio_sim_device_activate(struct gpio_sim_device *dev)
pdevinfo.fwnode = swnode;
pdevinfo.id = dev->id;
- reinit_completion(&dev->probe_completion);
- dev->driver_bound = false;
- bus_register_notifier(&platform_bus_type, &dev->bus_notifier);
-
- pdev = platform_device_register_full(&pdevinfo);
- if (IS_ERR(pdev)) {
- bus_unregister_notifier(&platform_bus_type, &dev->bus_notifier);
- gpio_sim_remove_hogs(dev);
- gpio_sim_remove_swnode_recursive(swnode);
- return PTR_ERR(pdev);
- }
-
- wait_for_completion(&dev->probe_completion);
- bus_unregister_notifier(&platform_bus_type, &dev->bus_notifier);
-
- if (!dev->driver_bound) {
- /* Probe failed, check kernel log. */
- platform_device_unregister(pdev);
+ ret = pseudo_gpio_register(&dev->common, &pdevinfo);
+ if (ret) {
gpio_sim_remove_hogs(dev);
gpio_sim_remove_swnode_recursive(swnode);
- return -ENXIO;
+ return ret;
}
- dev->pdev = pdev;
-
return 0;
}
@@ -1015,11 +957,10 @@ static void gpio_sim_device_deactivate(struct gpio_sim_device *dev)
lockdep_assert_held(&dev->lock);
- swnode = dev_fwnode(&dev->pdev->dev);
- platform_device_unregister(dev->pdev);
+ swnode = dev_fwnode(&dev->common.pdev->dev);
+ pseudo_gpio_unregister(&dev->common);
gpio_sim_remove_hogs(dev);
gpio_sim_remove_swnode_recursive(swnode);
- dev->pdev = NULL;
}
static void
@@ -1117,7 +1058,7 @@ static ssize_t gpio_sim_bank_config_chip_name_show(struct config_item *item,
guard(mutex)(&dev->lock);
if (gpio_sim_device_is_live(dev))
- return device_for_each_child(&dev->pdev->dev, &ctx,
+ return device_for_each_child(&dev->common.pdev->dev, &ctx,
gpio_sim_emit_chip_name);
return sprintf(page, "none\n");
@@ -1558,8 +1499,7 @@ gpio_sim_config_make_device_group(struct config_group *group, const char *name)
mutex_init(&dev->lock);
INIT_LIST_HEAD(&dev->bank_list);
- dev->bus_notifier.notifier_call = gpio_sim_bus_notifier_call;
- init_completion(&dev->probe_completion);
+ pseudo_gpio_init(&dev->common);
return &no_free_ptr(dev)->group;
}
--
2.45.2
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH v3 05/13] gpio: virtuser: convert to use gpio-pseudo utilities
2025-02-16 12:58 [PATCH v3 00/13] Introduce configfs-based interface for gpio-aggregator Koichiro Den
` (3 preceding siblings ...)
2025-02-16 12:58 ` [PATCH v3 04/13] gpio: sim: convert to use gpio-pseudo utilities Koichiro Den
@ 2025-02-16 12:58 ` Koichiro Den
2025-02-16 15:32 ` kernel test robot
2025-02-16 16:25 ` kernel test robot
2025-02-16 12:58 ` [PATCH v3 06/13] gpio: aggregator: " Koichiro Den
` (8 subsequent siblings)
13 siblings, 2 replies; 24+ messages in thread
From: Koichiro Den @ 2025-02-16 12:58 UTC (permalink / raw)
To: linux-gpio
Cc: brgl, geert+renesas, linus.walleij, maciej.borzecki, linux-kernel
Update gpio-virtuser to use the new gpio-pseudo helper functions for
synchronized platform device creation, reducing code duplication.
No functional change.
Signed-off-by: Koichiro Den <koichiro.den@canonical.com>
---
drivers/gpio/Kconfig | 1 +
drivers/gpio/gpio-virtuser.c | 73 +++++-------------------------------
2 files changed, 11 insertions(+), 63 deletions(-)
diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index c482e3494bac..d8fede07149f 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -1942,6 +1942,7 @@ config GPIO_VIRTUSER
select DEBUG_FS
select CONFIGFS_FS
select IRQ_WORK
+ select GPIO_PSEUDO
help
Say Y here to enable the configurable, configfs-based virtual GPIO
consumer testing driver.
diff --git a/drivers/gpio/gpio-virtuser.c b/drivers/gpio/gpio-virtuser.c
index e89f299f2140..a825edc6fbc5 100644
--- a/drivers/gpio/gpio-virtuser.c
+++ b/drivers/gpio/gpio-virtuser.c
@@ -11,7 +11,6 @@
#include <linux/atomic.h>
#include <linux/bitmap.h>
#include <linux/cleanup.h>
-#include <linux/completion.h>
#include <linux/configfs.h>
#include <linux/debugfs.h>
#include <linux/device.h>
@@ -37,6 +36,8 @@
#include <linux/string_helpers.h>
#include <linux/types.h>
+#include "gpio-pseudo.h"
+
#define GPIO_VIRTUSER_NAME_BUF_LEN 32
static DEFINE_IDA(gpio_virtuser_ida);
@@ -973,49 +974,17 @@ static struct platform_driver gpio_virtuser_driver = {
};
struct gpio_virtuser_device {
+ struct pseudo_gpio_common common;
struct config_group group;
- struct platform_device *pdev;
int id;
struct mutex lock;
- struct notifier_block bus_notifier;
- struct completion probe_completion;
- bool driver_bound;
-
struct gpiod_lookup_table *lookup_table;
struct list_head lookup_list;
};
-static int gpio_virtuser_bus_notifier_call(struct notifier_block *nb,
- unsigned long action, void *data)
-{
- struct gpio_virtuser_device *vdev;
- struct device *dev = data;
- char devname[32];
-
- vdev = container_of(nb, struct gpio_virtuser_device, bus_notifier);
- snprintf(devname, sizeof(devname), "gpio-virtuser.%d", vdev->id);
-
- if (!device_match_name(dev, devname))
- return NOTIFY_DONE;
-
- switch (action) {
- case BUS_NOTIFY_BOUND_DRIVER:
- vdev->driver_bound = true;
- break;
- case BUS_NOTIFY_DRIVER_NOT_BOUND:
- vdev->driver_bound = false;
- break;
- default:
- return NOTIFY_DONE;
- }
-
- complete(&vdev->probe_completion);
- return NOTIFY_OK;
-}
-
static struct gpio_virtuser_device *
to_gpio_virtuser_device(struct config_item *item)
{
@@ -1029,7 +998,7 @@ gpio_virtuser_device_is_live(struct gpio_virtuser_device *dev)
{
lockdep_assert_held(&dev->lock);
- return !!dev->pdev;
+ return !!dev->common.pdev;
}
struct gpio_virtuser_lookup {
@@ -1369,7 +1338,7 @@ gpio_virtuser_device_config_dev_name_show(struct config_item *item,
guard(mutex)(&dev->lock);
- pdev = dev->pdev;
+ pdev = dev->common.pdev;
if (pdev)
return sprintf(page, "%s\n", dev_name(&pdev->dev));
@@ -1478,7 +1447,6 @@ gpio_virtuser_device_activate(struct gpio_virtuser_device *dev)
{
struct platform_device_info pdevinfo;
struct fwnode_handle *swnode;
- struct platform_device *pdev;
int ret;
lockdep_assert_held(&dev->lock);
@@ -1499,31 +1467,12 @@ gpio_virtuser_device_activate(struct gpio_virtuser_device *dev)
if (ret)
goto err_remove_swnode;
- reinit_completion(&dev->probe_completion);
- dev->driver_bound = false;
- bus_register_notifier(&platform_bus_type, &dev->bus_notifier);
-
- pdev = platform_device_register_full(&pdevinfo);
- if (IS_ERR(pdev)) {
- ret = PTR_ERR(pdev);
- bus_unregister_notifier(&platform_bus_type, &dev->bus_notifier);
+ ret = pseudo_gpio_register(&dev->common, &pdevinfo);
+ if (ret)
goto err_remove_lookup_table;
- }
-
- wait_for_completion(&dev->probe_completion);
- bus_unregister_notifier(&platform_bus_type, &dev->bus_notifier);
-
- if (!dev->driver_bound) {
- ret = -ENXIO;
- goto err_unregister_pdev;
- }
-
- dev->pdev = pdev;
return 0;
-err_unregister_pdev:
- platform_device_unregister(pdev);
err_remove_lookup_table:
gpio_virtuser_remove_lookup_table(dev);
err_remove_swnode:
@@ -1539,11 +1488,10 @@ gpio_virtuser_device_deactivate(struct gpio_virtuser_device *dev)
lockdep_assert_held(&dev->lock);
- swnode = dev_fwnode(&dev->pdev->dev);
- platform_device_unregister(dev->pdev);
+ swnode = dev_fwnode(&dev->common.pdev->dev);
+ pseudo_gpio_unregister(&dev->common);
gpio_virtuser_remove_lookup_table(dev);
fwnode_remove_software_node(swnode);
- dev->pdev = NULL;
}
static void
@@ -1772,8 +1720,7 @@ gpio_virtuser_config_make_device_group(struct config_group *group,
&gpio_virtuser_device_config_group_type);
mutex_init(&dev->lock);
INIT_LIST_HEAD(&dev->lookup_list);
- dev->bus_notifier.notifier_call = gpio_virtuser_bus_notifier_call;
- init_completion(&dev->probe_completion);
+ pseudo_gpio_init(&dev->common);
return &no_free_ptr(dev)->group;
}
--
2.45.2
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [PATCH v3 05/13] gpio: virtuser: convert to use gpio-pseudo utilities
2025-02-16 12:58 ` [PATCH v3 05/13] gpio: virtuser: " Koichiro Den
@ 2025-02-16 15:32 ` kernel test robot
2025-02-16 16:25 ` kernel test robot
1 sibling, 0 replies; 24+ messages in thread
From: kernel test robot @ 2025-02-16 15:32 UTC (permalink / raw)
To: Koichiro Den, linux-gpio
Cc: oe-kbuild-all, brgl, geert+renesas, linus.walleij,
maciej.borzecki, linux-kernel
Hi Koichiro,
kernel test robot noticed the following build errors:
[auto build test ERROR on brgl/gpio/for-next]
[also build test ERROR on linus/master v6.14-rc2 next-20250214]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Koichiro-Den/gpio-aggregator-reorder-functions-to-prepare-for-configfs-introduction/20250216-210413
base: https://git.kernel.org/pub/scm/linux/kernel/git/brgl/linux.git gpio/for-next
patch link: https://lore.kernel.org/r/20250216125816.14430-6-koichiro.den%40canonical.com
patch subject: [PATCH v3 05/13] gpio: virtuser: convert to use gpio-pseudo utilities
config: csky-randconfig-002-20250216 (https://download.01.org/0day-ci/archive/20250216/202502162345.FT5z7kr9-lkp@intel.com/config)
compiler: csky-linux-gcc (GCC) 14.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250216/202502162345.FT5z7kr9-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202502162345.FT5z7kr9-lkp@intel.com/
All errors (new ones prefixed by >>):
drivers/gpio/gpio-pseudo.c: In function 'pseudo_gpio_register':
>> drivers/gpio/gpio-pseudo.c:62:17: error: implicit declaration of function 'kfree'; did you mean 'kvfree'? [-Wimplicit-function-declaration]
62 | kfree(common->name);
| ^~~~~
| kvfree
vim +62 drivers/gpio/gpio-pseudo.c
ef524a2229b717 Koichiro Den 2025-02-16 43
ef524a2229b717 Koichiro Den 2025-02-16 44 int pseudo_gpio_register(struct pseudo_gpio_common *common,
ef524a2229b717 Koichiro Den 2025-02-16 45 struct platform_device_info *pdevinfo)
ef524a2229b717 Koichiro Den 2025-02-16 46 {
ef524a2229b717 Koichiro Den 2025-02-16 47 struct platform_device *pdev;
ef524a2229b717 Koichiro Den 2025-02-16 48 char *name;
ef524a2229b717 Koichiro Den 2025-02-16 49
ef524a2229b717 Koichiro Den 2025-02-16 50 name = kasprintf(GFP_KERNEL, "%s.%u", pdevinfo->name, pdevinfo->id);
ef524a2229b717 Koichiro Den 2025-02-16 51 if (!name)
ef524a2229b717 Koichiro Den 2025-02-16 52 return -ENOMEM;
ef524a2229b717 Koichiro Den 2025-02-16 53
ef524a2229b717 Koichiro Den 2025-02-16 54 common->driver_bound = false;
ef524a2229b717 Koichiro Den 2025-02-16 55 common->name = name;
ef524a2229b717 Koichiro Den 2025-02-16 56 reinit_completion(&common->probe_completion);
ef524a2229b717 Koichiro Den 2025-02-16 57 bus_register_notifier(&platform_bus_type, &common->bus_notifier);
ef524a2229b717 Koichiro Den 2025-02-16 58
ef524a2229b717 Koichiro Den 2025-02-16 59 pdev = platform_device_register_full(pdevinfo);
ef524a2229b717 Koichiro Den 2025-02-16 60 if (IS_ERR(pdev)) {
ef524a2229b717 Koichiro Den 2025-02-16 61 bus_unregister_notifier(&platform_bus_type, &common->bus_notifier);
ef524a2229b717 Koichiro Den 2025-02-16 @62 kfree(common->name);
ef524a2229b717 Koichiro Den 2025-02-16 63 return PTR_ERR(pdev);
ef524a2229b717 Koichiro Den 2025-02-16 64 }
ef524a2229b717 Koichiro Den 2025-02-16 65
ef524a2229b717 Koichiro Den 2025-02-16 66 wait_for_completion(&common->probe_completion);
ef524a2229b717 Koichiro Den 2025-02-16 67 bus_unregister_notifier(&platform_bus_type, &common->bus_notifier);
ef524a2229b717 Koichiro Den 2025-02-16 68
ef524a2229b717 Koichiro Den 2025-02-16 69 if (!common->driver_bound) {
ef524a2229b717 Koichiro Den 2025-02-16 70 platform_device_unregister(pdev);
ef524a2229b717 Koichiro Den 2025-02-16 71 kfree(common->name);
ef524a2229b717 Koichiro Den 2025-02-16 72 return -ENXIO;
ef524a2229b717 Koichiro Den 2025-02-16 73 }
ef524a2229b717 Koichiro Den 2025-02-16 74
ef524a2229b717 Koichiro Den 2025-02-16 75 common->pdev = pdev;
ef524a2229b717 Koichiro Den 2025-02-16 76 return 0;
ef524a2229b717 Koichiro Den 2025-02-16 77 }
ef524a2229b717 Koichiro Den 2025-02-16 78 EXPORT_SYMBOL_GPL(pseudo_gpio_register);
ef524a2229b717 Koichiro Den 2025-02-16 79
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH v3 05/13] gpio: virtuser: convert to use gpio-pseudo utilities
2025-02-16 12:58 ` [PATCH v3 05/13] gpio: virtuser: " Koichiro Den
2025-02-16 15:32 ` kernel test robot
@ 2025-02-16 16:25 ` kernel test robot
1 sibling, 0 replies; 24+ messages in thread
From: kernel test robot @ 2025-02-16 16:25 UTC (permalink / raw)
To: Koichiro Den, linux-gpio
Cc: llvm, oe-kbuild-all, brgl, geert+renesas, linus.walleij,
maciej.borzecki, linux-kernel
Hi Koichiro,
kernel test robot noticed the following build errors:
[auto build test ERROR on brgl/gpio/for-next]
[also build test ERROR on linus/master v6.14-rc2 next-20250214]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Koichiro-Den/gpio-aggregator-reorder-functions-to-prepare-for-configfs-introduction/20250216-210413
base: https://git.kernel.org/pub/scm/linux/kernel/git/brgl/linux.git gpio/for-next
patch link: https://lore.kernel.org/r/20250216125816.14430-6-koichiro.den%40canonical.com
patch subject: [PATCH v3 05/13] gpio: virtuser: convert to use gpio-pseudo utilities
config: i386-buildonly-randconfig-003-20250216 (https://download.01.org/0day-ci/archive/20250217/202502170056.DR6nbxpY-lkp@intel.com/config)
compiler: clang version 19.1.3 (https://github.com/llvm/llvm-project ab51eccf88f5321e7c60591c5546b254b6afab99)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250217/202502170056.DR6nbxpY-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202502170056.DR6nbxpY-lkp@intel.com/
All errors (new ones prefixed by >>):
>> drivers/gpio/gpio-pseudo.c:62:3: error: call to undeclared function 'kfree'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
62 | kfree(common->name);
| ^
drivers/gpio/gpio-pseudo.c:71:3: error: call to undeclared function 'kfree'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
71 | kfree(common->name);
| ^
drivers/gpio/gpio-pseudo.c:83:2: error: call to undeclared function 'kfree'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
83 | kfree(common->name);
| ^
3 errors generated.
vim +/kfree +62 drivers/gpio/gpio-pseudo.c
ef524a2229b717 Koichiro Den 2025-02-16 43
ef524a2229b717 Koichiro Den 2025-02-16 44 int pseudo_gpio_register(struct pseudo_gpio_common *common,
ef524a2229b717 Koichiro Den 2025-02-16 45 struct platform_device_info *pdevinfo)
ef524a2229b717 Koichiro Den 2025-02-16 46 {
ef524a2229b717 Koichiro Den 2025-02-16 47 struct platform_device *pdev;
ef524a2229b717 Koichiro Den 2025-02-16 48 char *name;
ef524a2229b717 Koichiro Den 2025-02-16 49
ef524a2229b717 Koichiro Den 2025-02-16 50 name = kasprintf(GFP_KERNEL, "%s.%u", pdevinfo->name, pdevinfo->id);
ef524a2229b717 Koichiro Den 2025-02-16 51 if (!name)
ef524a2229b717 Koichiro Den 2025-02-16 52 return -ENOMEM;
ef524a2229b717 Koichiro Den 2025-02-16 53
ef524a2229b717 Koichiro Den 2025-02-16 54 common->driver_bound = false;
ef524a2229b717 Koichiro Den 2025-02-16 55 common->name = name;
ef524a2229b717 Koichiro Den 2025-02-16 56 reinit_completion(&common->probe_completion);
ef524a2229b717 Koichiro Den 2025-02-16 57 bus_register_notifier(&platform_bus_type, &common->bus_notifier);
ef524a2229b717 Koichiro Den 2025-02-16 58
ef524a2229b717 Koichiro Den 2025-02-16 59 pdev = platform_device_register_full(pdevinfo);
ef524a2229b717 Koichiro Den 2025-02-16 60 if (IS_ERR(pdev)) {
ef524a2229b717 Koichiro Den 2025-02-16 61 bus_unregister_notifier(&platform_bus_type, &common->bus_notifier);
ef524a2229b717 Koichiro Den 2025-02-16 @62 kfree(common->name);
ef524a2229b717 Koichiro Den 2025-02-16 63 return PTR_ERR(pdev);
ef524a2229b717 Koichiro Den 2025-02-16 64 }
ef524a2229b717 Koichiro Den 2025-02-16 65
ef524a2229b717 Koichiro Den 2025-02-16 66 wait_for_completion(&common->probe_completion);
ef524a2229b717 Koichiro Den 2025-02-16 67 bus_unregister_notifier(&platform_bus_type, &common->bus_notifier);
ef524a2229b717 Koichiro Den 2025-02-16 68
ef524a2229b717 Koichiro Den 2025-02-16 69 if (!common->driver_bound) {
ef524a2229b717 Koichiro Den 2025-02-16 70 platform_device_unregister(pdev);
ef524a2229b717 Koichiro Den 2025-02-16 71 kfree(common->name);
ef524a2229b717 Koichiro Den 2025-02-16 72 return -ENXIO;
ef524a2229b717 Koichiro Den 2025-02-16 73 }
ef524a2229b717 Koichiro Den 2025-02-16 74
ef524a2229b717 Koichiro Den 2025-02-16 75 common->pdev = pdev;
ef524a2229b717 Koichiro Den 2025-02-16 76 return 0;
ef524a2229b717 Koichiro Den 2025-02-16 77 }
ef524a2229b717 Koichiro Den 2025-02-16 78 EXPORT_SYMBOL_GPL(pseudo_gpio_register);
ef524a2229b717 Koichiro Den 2025-02-16 79
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH v3 06/13] gpio: aggregator: convert to use gpio-pseudo utilities
2025-02-16 12:58 [PATCH v3 00/13] Introduce configfs-based interface for gpio-aggregator Koichiro Den
` (4 preceding siblings ...)
2025-02-16 12:58 ` [PATCH v3 05/13] gpio: virtuser: " Koichiro Den
@ 2025-02-16 12:58 ` Koichiro Den
2025-02-16 12:58 ` [PATCH v3 07/13] gpio: aggregator: add aggr_alloc()/aggr_free() Koichiro Den
` (7 subsequent siblings)
13 siblings, 0 replies; 24+ messages in thread
From: Koichiro Den @ 2025-02-16 12:58 UTC (permalink / raw)
To: linux-gpio
Cc: brgl, geert+renesas, linus.walleij, maciej.borzecki, linux-kernel
Update gpio-aggregator to use the new gpio-pseudo helper functions.
Note that the current sysfs interface for gpio-aggregator does not wait
for probe completion when creating a platform device. This change brings
no immediate benefit but prepares for a later commit introducing
configfs that shares this mechanism.
No functional change.
Signed-off-by: Koichiro Den <koichiro.den@canonical.com>
---
drivers/gpio/Kconfig | 1 +
drivers/gpio/gpio-aggregator.c | 8 +++++---
2 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index d8fede07149f..8b9ffe17426e 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -1871,6 +1871,7 @@ menu "Virtual GPIO drivers"
config GPIO_AGGREGATOR
tristate "GPIO Aggregator"
+ select GPIO_PSEUDO
help
Say yes here to enable the GPIO Aggregator, which provides a way to
aggregate existing GPIO lines into a new virtual GPIO chip.
diff --git a/drivers/gpio/gpio-aggregator.c b/drivers/gpio/gpio-aggregator.c
index 893cd56de867..b24ed963cd9a 100644
--- a/drivers/gpio/gpio-aggregator.c
+++ b/drivers/gpio/gpio-aggregator.c
@@ -27,6 +27,8 @@
#include <linux/gpio/driver.h>
#include <linux/gpio/machine.h>
+#include "gpio-pseudo.h"
+
#define AGGREGATOR_MAX_GPIOS 512
/*
@@ -34,8 +36,8 @@
*/
struct gpio_aggregator {
+ struct pseudo_gpio_common common;
struct gpiod_lookup_table *lookups;
- struct platform_device *pdev;
char args[];
};
@@ -492,7 +494,7 @@ static ssize_t new_device_store(struct device_driver *driver, const char *buf,
goto remove_table;
}
- aggr->pdev = pdev;
+ aggr->common.pdev = pdev;
module_put(THIS_MODULE);
return count;
@@ -517,7 +519,7 @@ static DRIVER_ATTR_WO(new_device);
static void gpio_aggregator_free(struct gpio_aggregator *aggr)
{
- platform_device_unregister(aggr->pdev);
+ platform_device_unregister(aggr->common.pdev);
gpiod_remove_lookup_table(aggr->lookups);
kfree(aggr->lookups->dev_id);
kfree(aggr->lookups);
--
2.45.2
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH v3 07/13] gpio: aggregator: add aggr_alloc()/aggr_free()
2025-02-16 12:58 [PATCH v3 00/13] Introduce configfs-based interface for gpio-aggregator Koichiro Den
` (5 preceding siblings ...)
2025-02-16 12:58 ` [PATCH v3 06/13] gpio: aggregator: " Koichiro Den
@ 2025-02-16 12:58 ` Koichiro Den
2025-02-16 12:58 ` [PATCH v3 08/13] gpio: aggregator: introduce basic configfs interface Koichiro Den
` (6 subsequent siblings)
13 siblings, 0 replies; 24+ messages in thread
From: Koichiro Den @ 2025-02-16 12:58 UTC (permalink / raw)
To: linux-gpio
Cc: brgl, geert+renesas, linus.walleij, maciej.borzecki, linux-kernel
Prepare for the upcoming configfs interface. These functions will be
used by both the existing sysfs interface and the new configfs
interface, reducing code duplication.
No functional change.
Signed-off-by: Koichiro Den <koichiro.den@canonical.com>
---
drivers/gpio/gpio-aggregator.c | 58 +++++++++++++++++++++-------------
1 file changed, 36 insertions(+), 22 deletions(-)
diff --git a/drivers/gpio/gpio-aggregator.c b/drivers/gpio/gpio-aggregator.c
index b24ed963cd9a..6252a686f805 100644
--- a/drivers/gpio/gpio-aggregator.c
+++ b/drivers/gpio/gpio-aggregator.c
@@ -38,12 +38,41 @@
struct gpio_aggregator {
struct pseudo_gpio_common common;
struct gpiod_lookup_table *lookups;
+ int id;
char args[];
};
static DEFINE_MUTEX(gpio_aggregator_lock); /* protects idr */
static DEFINE_IDR(gpio_aggregator_idr);
+static int aggr_alloc(struct gpio_aggregator **aggr, size_t arg_size)
+{
+ struct gpio_aggregator *new __free(kfree) = NULL;
+ int ret;
+
+ new = kzalloc(sizeof(*new) + arg_size, GFP_KERNEL);
+ if (!new)
+ return -ENOMEM;
+
+ mutex_lock(&gpio_aggregator_lock);
+ ret = idr_alloc(&gpio_aggregator_idr, new, 0, 0, GFP_KERNEL);
+ mutex_unlock(&gpio_aggregator_lock);
+ if (ret < 0)
+ return ret;
+
+ new->id = ret;
+ *aggr = no_free_ptr(new);
+ return 0;
+}
+
+static void aggr_free(struct gpio_aggregator *aggr)
+{
+ mutex_lock(&gpio_aggregator_lock);
+ idr_remove(&gpio_aggregator_idr, aggr->id);
+ mutex_unlock(&gpio_aggregator_lock);
+ kfree(aggr);
+}
+
static int aggr_add_gpio(struct gpio_aggregator *aggr, const char *key,
int hwnum, unsigned int *n)
{
@@ -446,17 +475,15 @@ static ssize_t new_device_store(struct device_driver *driver, const char *buf,
{
struct gpio_aggregator *aggr;
struct platform_device *pdev;
- int res, id;
+ int res;
if (!try_module_get(THIS_MODULE))
return -ENOENT;
/* kernfs guarantees string termination, so count + 1 is safe */
- aggr = kzalloc(sizeof(*aggr) + count + 1, GFP_KERNEL);
- if (!aggr) {
- res = -ENOMEM;
+ res = aggr_alloc(&aggr, count + 1);
+ if (res)
goto put_module;
- }
memcpy(aggr->args, buf, count + 1);
@@ -467,19 +494,10 @@ static ssize_t new_device_store(struct device_driver *driver, const char *buf,
goto free_ga;
}
- mutex_lock(&gpio_aggregator_lock);
- id = idr_alloc(&gpio_aggregator_idr, aggr, 0, 0, GFP_KERNEL);
- mutex_unlock(&gpio_aggregator_lock);
-
- if (id < 0) {
- res = id;
- goto free_table;
- }
-
- aggr->lookups->dev_id = kasprintf(GFP_KERNEL, "%s.%d", DRV_NAME, id);
+ aggr->lookups->dev_id = kasprintf(GFP_KERNEL, "%s.%d", DRV_NAME, aggr->id);
if (!aggr->lookups->dev_id) {
res = -ENOMEM;
- goto remove_idr;
+ goto free_table;
}
res = aggr_parse(aggr);
@@ -488,7 +506,7 @@ static ssize_t new_device_store(struct device_driver *driver, const char *buf,
gpiod_add_lookup_table(aggr->lookups);
- pdev = platform_device_register_simple(DRV_NAME, id, NULL, 0);
+ pdev = platform_device_register_simple(DRV_NAME, aggr->id, NULL, 0);
if (IS_ERR(pdev)) {
res = PTR_ERR(pdev);
goto remove_table;
@@ -502,14 +520,10 @@ static ssize_t new_device_store(struct device_driver *driver, const char *buf,
gpiod_remove_lookup_table(aggr->lookups);
free_dev_id:
kfree(aggr->lookups->dev_id);
-remove_idr:
- mutex_lock(&gpio_aggregator_lock);
- idr_remove(&gpio_aggregator_idr, id);
- mutex_unlock(&gpio_aggregator_lock);
free_table:
kfree(aggr->lookups);
free_ga:
- kfree(aggr);
+ aggr_free(aggr);
put_module:
module_put(THIS_MODULE);
return res;
--
2.45.2
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH v3 08/13] gpio: aggregator: introduce basic configfs interface
2025-02-16 12:58 [PATCH v3 00/13] Introduce configfs-based interface for gpio-aggregator Koichiro Den
` (6 preceding siblings ...)
2025-02-16 12:58 ` [PATCH v3 07/13] gpio: aggregator: add aggr_alloc()/aggr_free() Koichiro Den
@ 2025-02-16 12:58 ` Koichiro Den
2025-02-16 12:58 ` [PATCH v3 09/13] gpio: aggregator: add 'name' attribute for custom GPIO line names Koichiro Den
` (5 subsequent siblings)
13 siblings, 0 replies; 24+ messages in thread
From: Koichiro Den @ 2025-02-16 12:58 UTC (permalink / raw)
To: linux-gpio
Cc: brgl, geert+renesas, linus.walleij, maciej.borzecki, linux-kernel
The existing sysfs 'new_device' interface has several limitations:
* No way to determine when GPIO aggregator creation is complete.
* No way to retrieve errors when creating a GPIO aggregator.
* No way to trace a GPIO line of an aggregator back to its
corresponding physical device.
* The 'new_device' echo does not indicate which virtual gpiochip<N>
was created.
* No way to assign names to GPIO lines exported through an aggregator.
Introduce the new configfs interface for gpio-aggregator to address
these limitations. It provides a more streamlined, modern, and
extensible configuration method. For backward compatibility, the
'new_device' interface and its behavior is retained for now.
This commit implements basic functionalities:
/config/gpio-aggregator/<name-of-your-choice>/
/config/gpio-aggregator/<name-of-your-choice>/live
/config/gpio-aggregator/<name-of-your-choice>/dev_name
/config/gpio-aggregator/<name-of-your-choice>/<lineY>/
/config/gpio-aggregator/<name-of-your-choice>/<lineY>/key
/config/gpio-aggregator/<name-of-your-choice>/<lineY>/offset
Basic setup flow is:
1. Create a directory for a GPIO aggregator.
2. Create subdirectories for each line you want to instantiate.
3. In each line directory, configure the key and offset.
The key/offset semantics are as follows:
* If offset is >= 0:
- key specifies the name of the chip this GPIO belongs to
- offset specifies the line offset within that chip.
* If offset is <0:
- key needs to specify the GPIO line name.
4. Return to the aggregator's root directory and write '1' to the live
attribute.
For example, the command in the existing kernel doc:
echo 'e6052000.gpio 19 e6050000.gpio 20-21' > new_device
is equivalent to:
mkdir /sys/kernel/config/gpio-aggregator/<custom-name>
# Change <custom-name> to name of your choice (e.g. "aggr0")
cd /sys/kernel/config/gpio-aggregator/<custom-name>
mkdir line0 line1 line2 # Only "line<Y>" naming allowed.
echo e6052000.gpio > line0/key
echo 19 > line0/offset
echo e6050000.gpio > line1/key
echo 20 > line1/offset
echo e6050000.gpio > line2/key
echo 21 > line2/offset
echo 1 > live
The corresponding gpio_device id can be identified as follows:
cd /sys/kernel/config/gpio-aggregator/<custom-name>
ls -d /sys/devices/platform/`cat dev_name`/gpiochip*
Signed-off-by: Koichiro Den <koichiro.den@canonical.com>
---
drivers/gpio/Kconfig | 1 +
drivers/gpio/gpio-aggregator.c | 540 ++++++++++++++++++++++++++++++++-
2 files changed, 533 insertions(+), 8 deletions(-)
diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index 8b9ffe17426e..591244e6cd4e 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -1871,6 +1871,7 @@ menu "Virtual GPIO drivers"
config GPIO_AGGREGATOR
tristate "GPIO Aggregator"
+ select CONFIGFS_FS
select GPIO_PSEUDO
help
Say yes here to enable the GPIO Aggregator, which provides a way to
diff --git a/drivers/gpio/gpio-aggregator.c b/drivers/gpio/gpio-aggregator.c
index 6252a686f805..ec102453817b 100644
--- a/drivers/gpio/gpio-aggregator.c
+++ b/drivers/gpio/gpio-aggregator.c
@@ -9,10 +9,13 @@
#include <linux/bitmap.h>
#include <linux/bitops.h>
+#include <linux/configfs.h>
#include <linux/ctype.h>
#include <linux/delay.h>
#include <linux/idr.h>
#include <linux/kernel.h>
+#include <linux/list.h>
+#include <linux/lockdep.h>
#include <linux/mod_devicetable.h>
#include <linux/module.h>
#include <linux/mutex.h>
@@ -37,11 +40,35 @@
struct gpio_aggregator {
struct pseudo_gpio_common common;
+ struct config_group group;
struct gpiod_lookup_table *lookups;
+ struct mutex lock;
int id;
+
+ /* List of gpio_aggregator_line. Always added in order */
+ struct list_head list_head;
+
+ /* used by legacy sysfs interface only */
+ bool init_via_sysfs;
char args[];
};
+struct gpio_aggregator_line {
+ struct config_group group;
+ struct gpio_aggregator *parent;
+ struct list_head entry;
+
+ /* Line index within the aggregator device */
+ unsigned int idx;
+
+ /* GPIO chip label or line name */
+ const char *key;
+ /* Can be negative to indicate lookup by line name */
+ int offset;
+
+ enum gpio_lookup_flags flags;
+};
+
static DEFINE_MUTEX(gpio_aggregator_lock); /* protects idr */
static DEFINE_IDR(gpio_aggregator_idr);
@@ -61,6 +88,8 @@ static int aggr_alloc(struct gpio_aggregator **aggr, size_t arg_size)
return ret;
new->id = ret;
+ INIT_LIST_HEAD(&new->list_head);
+ mutex_init(&new->lock);
*aggr = no_free_ptr(new);
return 0;
}
@@ -70,6 +99,7 @@ static void aggr_free(struct gpio_aggregator *aggr)
mutex_lock(&gpio_aggregator_lock);
idr_remove(&gpio_aggregator_idr, aggr->id);
mutex_unlock(&gpio_aggregator_lock);
+ mutex_destroy(&aggr->lock);
kfree(aggr);
}
@@ -92,6 +122,70 @@ static int aggr_add_gpio(struct gpio_aggregator *aggr, const char *key,
return 0;
}
+static bool aggr_is_active(struct gpio_aggregator *aggr)
+{
+ lockdep_assert_held(&aggr->lock);
+
+ return aggr->common.pdev && platform_get_drvdata(aggr->common.pdev);
+}
+
+static size_t aggr_count_lines(struct gpio_aggregator *aggr)
+{
+ lockdep_assert_held(&aggr->lock);
+
+ return list_count_nodes(&aggr->list_head);
+}
+
+static struct gpio_aggregator_line *aggr_line_alloc(
+ struct gpio_aggregator *parent, unsigned int idx, char *key, int offset)
+{
+ struct gpio_aggregator_line *line;
+
+ line = kzalloc(sizeof(*line), GFP_KERNEL);
+ if (!line)
+ return ERR_PTR(-ENOMEM);
+
+ if (key) {
+ line->key = kstrdup(key, GFP_KERNEL);
+ if (!line->key) {
+ kfree(line);
+ return ERR_PTR(-ENOMEM);
+ }
+ }
+
+ line->flags = GPIO_LOOKUP_FLAGS_DEFAULT;
+ line->parent = parent;
+ line->idx = idx;
+ line->offset = offset;
+ INIT_LIST_HEAD(&line->entry);
+
+ return line;
+}
+
+static void aggr_line_add(struct gpio_aggregator *aggr,
+ struct gpio_aggregator_line *line)
+{
+ struct gpio_aggregator_line *tmp;
+
+ lockdep_assert_held(&aggr->lock);
+
+ list_for_each_entry(tmp, &aggr->list_head, entry) {
+ if (tmp->idx > line->idx) {
+ list_add_tail(&line->entry, &tmp->entry);
+ return;
+ }
+ }
+ list_add_tail(&line->entry, &aggr->list_head);
+}
+
+static void aggr_line_del(struct gpio_aggregator *aggr,
+ struct gpio_aggregator_line *line)
+{
+ lockdep_assert_held(&aggr->lock);
+
+ list_del(&line->entry);
+}
+
/*
* GPIO Forwarder
@@ -416,6 +510,400 @@ static struct gpiochip_fwd *gpiochip_fwd_create(struct device *dev,
}
+/*
+ * Configfs interface
+ */
+
+static struct gpio_aggregator *
+to_gpio_aggregator(struct config_item *item)
+{
+ struct config_group *group = to_config_group(item);
+
+ return container_of(group, struct gpio_aggregator, group);
+}
+
+static struct gpio_aggregator_line *
+to_gpio_aggregator_line(struct config_item *item)
+{
+ struct config_group *group = to_config_group(item);
+
+ return container_of(group, struct gpio_aggregator_line, group);
+}
+
+static int aggr_activate(struct gpio_aggregator *aggr)
+{
+ struct platform_device_info pdevinfo;
+ struct gpio_aggregator_line *line;
+ unsigned int n = 0;
+ int ret = 0;
+
+ if (aggr_count_lines(aggr) == 0)
+ return -EINVAL;
+
+ aggr->lookups = kzalloc(struct_size(aggr->lookups, table, 1),
+ GFP_KERNEL);
+ if (!aggr->lookups)
+ return -ENOMEM;
+
+ memset(&pdevinfo, 0, sizeof(pdevinfo));
+ pdevinfo.name = DRV_NAME;
+ pdevinfo.id = aggr->id;
+
+ /* The list is always sorted as new elements are inserted in order. */
+ list_for_each_entry(line, &aggr->list_head, entry) {
+ /*
+ * - Either GPIO chip label or line name must be configured
+ * (i.e. line->key must be non-NULL)
+ * - Line directories must be named with sequential numeric
+ * suffixes starting from 0. (i.e. ./line0, ./line1, ...)
+ */
+ if (!line->key || line->idx != n) {
+ ret = -EINVAL;
+ goto err_remove_lookups;
+ }
+
+ if (line->offset < 0)
+ ret = aggr_add_gpio(aggr, line->key, U16_MAX, &n);
+ else
+ ret = aggr_add_gpio(aggr, line->key, line->offset, &n);
+ if (ret)
+ goto err_remove_lookups;
+ }
+
+ aggr->lookups->dev_id = kasprintf(GFP_KERNEL, "%s.%d", DRV_NAME, aggr->id);
+ if (!aggr->lookups->dev_id) {
+ ret = -ENOMEM;
+ goto err_remove_lookups;
+ }
+
+ gpiod_add_lookup_table(aggr->lookups);
+
+ ret = pseudo_gpio_register(&aggr->common, &pdevinfo);
+ if (ret)
+ goto err_remove_lookup_table;
+
+ return 0;
+
+err_remove_lookup_table:
+ kfree(aggr->lookups->dev_id);
+ gpiod_remove_lookup_table(aggr->lookups);
+err_remove_lookups:
+ kfree(aggr->lookups);
+
+ return ret;
+}
+
+static void aggr_deactivate(struct gpio_aggregator *aggr)
+{
+ pseudo_gpio_unregister(&aggr->common);
+ gpiod_remove_lookup_table(aggr->lookups);
+ kfree(aggr->lookups->dev_id);
+ kfree(aggr->lookups);
+}
+
+static void aggr_lockup_configfs(struct gpio_aggregator *aggr, bool lock)
+{
+ struct configfs_subsystem *subsys = aggr->group.cg_subsys;
+ struct gpio_aggregator_line *line;
+
+ /*
+ * The device only needs to depend on leaf lines. This is
+ * sufficient to lock up all the configfs entries that the
+ * instantiated, alive device depends on.
+ */
+ list_for_each_entry(line, &aggr->list_head, entry) {
+ if (lock)
+ configfs_depend_item_unlocked(
+ subsys, &line->group.cg_item);
+ else
+ configfs_undepend_item_unlocked(
+ &line->group.cg_item);
+ }
+}
+
+static ssize_t
+gpio_aggr_line_key_show(struct config_item *item, char *page)
+{
+ struct gpio_aggregator_line *line = to_gpio_aggregator_line(item);
+ struct gpio_aggregator *aggr = line->parent;
+
+ guard(mutex)(&aggr->lock);
+
+ return sysfs_emit(page, "%s\n", line->key ?: "");
+}
+
+static ssize_t
+gpio_aggr_line_key_store(struct config_item *item, const char *page,
+ size_t count)
+{
+ struct gpio_aggregator_line *line = to_gpio_aggregator_line(item);
+ struct gpio_aggregator *aggr = line->parent;
+
+ char *key __free(kfree) = kstrndup(skip_spaces(page), count,
+ GFP_KERNEL);
+ if (!key)
+ return -ENOMEM;
+
+ strim(key);
+
+ guard(mutex)(&aggr->lock);
+
+ if (aggr_is_active(aggr))
+ return -EBUSY;
+
+ kfree(line->key);
+ line->key = no_free_ptr(key);
+
+ return count;
+}
+CONFIGFS_ATTR(gpio_aggr_line_, key);
+
+static ssize_t
+gpio_aggr_line_offset_show(struct config_item *item, char *page)
+{
+ struct gpio_aggregator_line *line = to_gpio_aggregator_line(item);
+ struct gpio_aggregator *aggr = line->parent;
+ unsigned int offset;
+
+ scoped_guard(mutex, &aggr->lock)
+ offset = line->offset;
+
+ return sysfs_emit(page, "%d\n", offset);
+}
+
+static ssize_t
+gpio_aggr_line_offset_store(struct config_item *item, const char *page,
+ size_t count)
+{
+ struct gpio_aggregator_line *line = to_gpio_aggregator_line(item);
+ struct gpio_aggregator *aggr = line->parent;
+ int offset, ret;
+
+ ret = kstrtoint(page, 0, &offset);
+ if (ret)
+ return ret;
+
+ /*
+ * When offset == -1, 'key' represents a line name to lookup.
+ * When 0 <= offset < 65535, 'key' represents the label of the chip with
+ * the 'offset' value representing the line within that chip.
+ *
+ * GPIOLIB uses the U16_MAX value to indicate lookup by line name so
+ * the greatest offset we can accept is (U16_MAX - 1).
+ */
+ if (offset > (U16_MAX - 1) || offset < -1)
+ return -EINVAL;
+
+ guard(mutex)(&aggr->lock);
+
+ if (aggr_is_active(aggr))
+ return -EBUSY;
+
+ line->offset = offset;
+
+ return count;
+}
+CONFIGFS_ATTR(gpio_aggr_line_, offset);
+
+static struct configfs_attribute *gpio_aggr_line_attrs[] = {
+ &gpio_aggr_line_attr_key,
+ &gpio_aggr_line_attr_offset,
+ NULL
+};
+
+static ssize_t
+gpio_aggr_device_dev_name_show(struct config_item *item, char *page)
+{
+ struct gpio_aggregator *aggr = to_gpio_aggregator(item);
+ struct platform_device *pdev;
+
+ guard(mutex)(&aggr->lock);
+
+ pdev = aggr->common.pdev;
+ if (pdev)
+ return sysfs_emit(page, "%s\n", dev_name(&pdev->dev));
+
+ return sysfs_emit(page, "%s.%d\n", DRV_NAME, aggr->id);
+}
+CONFIGFS_ATTR_RO(gpio_aggr_device_, dev_name);
+
+static ssize_t
+gpio_aggr_device_live_show(struct config_item *item, char *page)
+{
+ struct gpio_aggregator *aggr = to_gpio_aggregator(item);
+ bool active;
+
+ scoped_guard(mutex, &aggr->lock)
+ active = aggr_is_active(aggr);
+
+ return sysfs_emit(page, "%c\n", active ? '1' : '0');
+}
+
+static ssize_t
+gpio_aggr_device_live_store(struct config_item *item, const char *page,
+ size_t count)
+{
+ struct gpio_aggregator *aggr = to_gpio_aggregator(item);
+ int ret = 0;
+ bool live;
+
+ ret = kstrtobool(page, &live);
+ if (ret)
+ return ret;
+
+ if (!try_module_get(THIS_MODULE))
+ return -ENOENT;
+
+ if (live)
+ aggr_lockup_configfs(aggr, true);
+
+ scoped_guard(mutex, &aggr->lock) {
+ if (live == aggr_is_active(aggr))
+ ret = -EPERM;
+ else if (live)
+ ret = aggr_activate(aggr);
+ else
+ aggr_deactivate(aggr);
+ }
+
+ /*
+ * Undepend is required only if device disablement (live == 0)
+ * succeeds or if device enablement (live == 1) fails.
+ */
+ if (live == !!ret)
+ aggr_lockup_configfs(aggr, false);
+
+ module_put(THIS_MODULE);
+
+ return ret ?: count;
+}
+CONFIGFS_ATTR(gpio_aggr_device_, live);
+
+static struct configfs_attribute *gpio_aggr_device_attrs[] = {
+ &gpio_aggr_device_attr_dev_name,
+ &gpio_aggr_device_attr_live,
+ NULL
+};
+
+static void
+gpio_aggr_line_release(struct config_item *item)
+{
+ struct gpio_aggregator_line *line = to_gpio_aggregator_line(item);
+ struct gpio_aggregator *aggr = line->parent;
+
+ guard(mutex)(&aggr->lock);
+
+ aggr_line_del(aggr, line);
+ kfree(line->key);
+ kfree(line);
+}
+
+static struct configfs_item_operations gpio_aggr_line_item_ops = {
+ .release = gpio_aggr_line_release,
+};
+
+static const struct config_item_type gpio_aggr_line_type = {
+ .ct_item_ops = &gpio_aggr_line_item_ops,
+ .ct_attrs = gpio_aggr_line_attrs,
+ .ct_owner = THIS_MODULE,
+};
+
+static void gpio_aggr_device_release(struct config_item *item)
+{
+ struct gpio_aggregator *aggr = to_gpio_aggregator(item);
+
+ guard(mutex)(&aggr->lock);
+
+ /*
+ * If the aggregator is active, this code wouldn't be reached,
+ * so calling aggr_deactivate() is always unnecessary.
+ */
+ aggr_free(aggr);
+}
+
+static struct configfs_item_operations gpio_aggr_device_item_ops = {
+ .release = gpio_aggr_device_release,
+};
+
+static struct config_group *
+gpio_aggr_device_make_group(struct config_group *group, const char *name)
+{
+ struct gpio_aggregator *aggr = to_gpio_aggregator(&group->cg_item);
+ struct gpio_aggregator_line *line;
+ unsigned int idx;
+ int ret, nchar;
+
+ ret = sscanf(name, "line%u%n", &idx, &nchar);
+ if (ret != 1 || nchar != strlen(name))
+ return ERR_PTR(-EINVAL);
+
+ guard(mutex)(&aggr->lock);
+
+ if (aggr_is_active(aggr))
+ return ERR_PTR(-EBUSY);
+
+ list_for_each_entry(line, &aggr->list_head, entry)
+ if (line->idx == idx)
+ return ERR_PTR(-EINVAL);
+
+ line = aggr_line_alloc(aggr, idx, NULL, -1);
+ if (!line)
+ return ERR_PTR(-ENOMEM);
+
+ config_group_init_type_name(&line->group, name, &gpio_aggr_line_type);
+
+ aggr_line_add(aggr, line);
+
+ return &line->group;
+}
+
+static struct configfs_group_operations gpio_aggr_device_group_ops = {
+ .make_group = gpio_aggr_device_make_group,
+};
+
+static const struct config_item_type gpio_aggr_device_type = {
+ .ct_group_ops = &gpio_aggr_device_group_ops,
+ .ct_item_ops = &gpio_aggr_device_item_ops,
+ .ct_attrs = gpio_aggr_device_attrs,
+ .ct_owner = THIS_MODULE,
+};
+
+static struct config_group *
+gpio_aggr_make_group(struct config_group *group, const char *name)
+{
+ struct gpio_aggregator *aggr;
+ int ret;
+
+ /* arg space is unneeded */
+ ret = aggr_alloc(&aggr, 0);
+ if (ret)
+ return ERR_PTR(ret);
+
+ config_group_init_type_name(&aggr->group, name, &gpio_aggr_device_type);
+ pseudo_gpio_init(&aggr->common);
+
+ return &aggr->group;
+}
+
+static struct configfs_group_operations gpio_aggr_group_ops = {
+ .make_group = gpio_aggr_make_group,
+};
+
+static const struct config_item_type gpio_aggr_type = {
+ .ct_group_ops = &gpio_aggr_group_ops,
+ .ct_owner = THIS_MODULE,
+};
+
+static struct configfs_subsystem gpio_aggr_subsys = {
+ .su_group = {
+ .cg_item = {
+ .ci_namebuf = DRV_NAME,
+ .ci_type = &gpio_aggr_type,
+ },
+ },
+};
+
+
/*
* Sysfs interface
*/
@@ -487,6 +975,7 @@ static ssize_t new_device_store(struct device_driver *driver, const char *buf,
memcpy(aggr->args, buf, count + 1);
+ aggr->init_via_sysfs = true;
aggr->lookups = kzalloc(struct_size(aggr->lookups, table, 1),
GFP_KERNEL);
if (!aggr->lookups) {
@@ -533,10 +1022,7 @@ static DRIVER_ATTR_WO(new_device);
static void gpio_aggregator_free(struct gpio_aggregator *aggr)
{
- platform_device_unregister(aggr->common.pdev);
- gpiod_remove_lookup_table(aggr->lookups);
- kfree(aggr->lookups->dev_id);
- kfree(aggr->lookups);
+ aggr_deactivate(aggr);
kfree(aggr);
}
@@ -558,12 +1044,19 @@ static ssize_t delete_device_store(struct device_driver *driver,
return -ENOENT;
mutex_lock(&gpio_aggregator_lock);
- aggr = idr_remove(&gpio_aggregator_idr, id);
- mutex_unlock(&gpio_aggregator_lock);
- if (!aggr) {
+ aggr = idr_find(&gpio_aggregator_idr, id);
+ /*
+ * For simplicity, devices created via configfs cannot be deleted
+ * via sysfs.
+ */
+ if (aggr && aggr->init_via_sysfs)
+ idr_remove(&gpio_aggregator_idr, id);
+ else {
+ mutex_unlock(&gpio_aggregator_lock);
module_put(THIS_MODULE);
return -ENOENT;
}
+ mutex_unlock(&gpio_aggregator_lock);
gpio_aggregator_free(aggr);
module_put(THIS_MODULE);
@@ -638,6 +1131,10 @@ static struct platform_driver gpio_aggregator_driver = {
static int __exit gpio_aggregator_idr_remove(int id, void *p, void *data)
{
+ /*
+ * There should be no aggregator created via configfs, as their
+ * presence would prevent module unloading.
+ */
gpio_aggregator_free(p);
return 0;
}
@@ -652,7 +1149,33 @@ static void __exit gpio_aggregator_remove_all(void)
static int __init gpio_aggregator_init(void)
{
- return platform_driver_register(&gpio_aggregator_driver);
+ int ret = 0;
+
+ config_group_init(&gpio_aggr_subsys.su_group);
+ mutex_init(&gpio_aggr_subsys.su_mutex);
+ ret = configfs_register_subsystem(&gpio_aggr_subsys);
+ if (ret) {
+ pr_err("Failed to register the '%s' configfs subsystem: %d\n",
+ gpio_aggr_subsys.su_group.cg_item.ci_namebuf, ret);
+ mutex_destroy(&gpio_aggr_subsys.su_mutex);
+ return ret;
+ }
+
+ /*
+ * CAVEAT: This must occur after configfs registration. Otherwise,
+ * a race condition could arise: driver attribute groups might be
+ * exposed and accessed by users before configfs registration
+ * completes. new_device_store() does not expect a partially
+ * initialized configfs state.
+ */
+ ret = platform_driver_register(&gpio_aggregator_driver);
+ if (ret) {
+ pr_err("Failed to register the platform driver: %d\n", ret);
+ mutex_destroy(&gpio_aggr_subsys.su_mutex);
+ configfs_unregister_subsystem(&gpio_aggr_subsys);
+ }
+
+ return ret;
}
module_init(gpio_aggregator_init);
@@ -660,6 +1183,7 @@ static void __exit gpio_aggregator_exit(void)
{
gpio_aggregator_remove_all();
platform_driver_unregister(&gpio_aggregator_driver);
+ configfs_unregister_subsystem(&gpio_aggr_subsys);
}
module_exit(gpio_aggregator_exit);
--
2.45.2
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH v3 09/13] gpio: aggregator: add 'name' attribute for custom GPIO line names
2025-02-16 12:58 [PATCH v3 00/13] Introduce configfs-based interface for gpio-aggregator Koichiro Den
` (7 preceding siblings ...)
2025-02-16 12:58 ` [PATCH v3 08/13] gpio: aggregator: introduce basic configfs interface Koichiro Den
@ 2025-02-16 12:58 ` Koichiro Den
2025-02-16 12:58 ` [PATCH v3 10/13] gpio: aggregator: rename 'name' to 'key' in aggr_parse() Koichiro Den
` (4 subsequent siblings)
13 siblings, 0 replies; 24+ messages in thread
From: Koichiro Den @ 2025-02-16 12:58 UTC (permalink / raw)
To: linux-gpio
Cc: brgl, geert+renesas, linus.walleij, maciej.borzecki, linux-kernel
Previously, there was no way to assign names to GPIO lines exported
through the aggregator when the device was created via sysfs.
Allow users to set custom line names via a 'name' attribute and
expose them using swnode.
Signed-off-by: Koichiro Den <koichiro.den@canonical.com>
---
drivers/gpio/gpio-aggregator.c | 84 ++++++++++++++++++++++++++++++++--
1 file changed, 81 insertions(+), 3 deletions(-)
diff --git a/drivers/gpio/gpio-aggregator.c b/drivers/gpio/gpio-aggregator.c
index ec102453817b..692d90246674 100644
--- a/drivers/gpio/gpio-aggregator.c
+++ b/drivers/gpio/gpio-aggregator.c
@@ -61,6 +61,8 @@ struct gpio_aggregator_line {
/* Line index within the aggregator device */
unsigned int idx;
+ /* Custom name for the virtual line */
+ const char *name;
/* GPIO chip label or line name */
const char *key;
/* Can be negative to indicate lookup by line name */
@@ -530,10 +532,40 @@ to_gpio_aggregator_line(struct config_item *item)
return container_of(group, struct gpio_aggregator_line, group);
}
+static struct fwnode_handle *aggr_make_device_swnode(struct gpio_aggregator *aggr)
+{
+ const char **line_names __free(kfree) = NULL;
+ struct property_entry properties[2];
+ struct gpio_aggregator_line *line;
+ size_t num_lines;
+ int n = 0;
+
+ memset(properties, 0, sizeof(properties));
+
+ num_lines = aggr_count_lines(aggr);
+ if (num_lines == 0)
+ return NULL;
+
+ line_names = kcalloc(num_lines, sizeof(*line_names), GFP_KERNEL);
+ if (!line_names)
+ return ERR_PTR(-ENOMEM);
+
+ /* The list is always sorted as new elements are inserted in order. */
+ list_for_each_entry(line, &aggr->list_head, entry)
+ line_names[n++] = line->name ?: "";
+
+ properties[0] = PROPERTY_ENTRY_STRING_ARRAY_LEN(
+ "gpio-line-names",
+ line_names, num_lines);
+
+ return fwnode_create_software_node(properties, NULL);
+}
+
static int aggr_activate(struct gpio_aggregator *aggr)
{
struct platform_device_info pdevinfo;
struct gpio_aggregator_line *line;
+ struct fwnode_handle *swnode;
unsigned int n = 0;
int ret = 0;
@@ -545,9 +577,14 @@ static int aggr_activate(struct gpio_aggregator *aggr)
if (!aggr->lookups)
return -ENOMEM;
+ swnode = aggr_make_device_swnode(aggr);
+ if (IS_ERR(swnode))
+ goto err_remove_lookups;
+
memset(&pdevinfo, 0, sizeof(pdevinfo));
pdevinfo.name = DRV_NAME;
pdevinfo.id = aggr->id;
+ pdevinfo.fwnode = swnode;
/* The list is always sorted as new elements are inserted in order. */
list_for_each_entry(line, &aggr->list_head, entry) {
@@ -559,7 +596,7 @@ static int aggr_activate(struct gpio_aggregator *aggr)
*/
if (!line->key || line->idx != n) {
ret = -EINVAL;
- goto err_remove_lookups;
+ goto err_remove_swnode;
}
if (line->offset < 0)
@@ -567,13 +604,13 @@ static int aggr_activate(struct gpio_aggregator *aggr)
else
ret = aggr_add_gpio(aggr, line->key, line->offset, &n);
if (ret)
- goto err_remove_lookups;
+ goto err_remove_swnode;
}
aggr->lookups->dev_id = kasprintf(GFP_KERNEL, "%s.%d", DRV_NAME, aggr->id);
if (!aggr->lookups->dev_id) {
ret = -ENOMEM;
- goto err_remove_lookups;
+ goto err_remove_swnode;
}
gpiod_add_lookup_table(aggr->lookups);
@@ -587,6 +624,8 @@ static int aggr_activate(struct gpio_aggregator *aggr)
err_remove_lookup_table:
kfree(aggr->lookups->dev_id);
gpiod_remove_lookup_table(aggr->lookups);
+err_remove_swnode:
+ fwnode_remove_software_node(swnode);
err_remove_lookups:
kfree(aggr->lookups);
@@ -658,6 +697,43 @@ gpio_aggr_line_key_store(struct config_item *item, const char *page,
}
CONFIGFS_ATTR(gpio_aggr_line_, key);
+static ssize_t
+gpio_aggr_line_name_show(struct config_item *item, char *page)
+{
+ struct gpio_aggregator_line *line = to_gpio_aggregator_line(item);
+ struct gpio_aggregator *aggr = line->parent;
+
+ guard(mutex)(&aggr->lock);
+
+ return sysfs_emit(page, "%s\n", line->name ?: "");
+}
+
+static ssize_t
+gpio_aggr_line_name_store(struct config_item *item, const char *page,
+ size_t count)
+{
+ struct gpio_aggregator_line *line = to_gpio_aggregator_line(item);
+ struct gpio_aggregator *aggr = line->parent;
+
+ char *name __free(kfree) = kstrndup(skip_spaces(page), count,
+ GFP_KERNEL);
+ if (!name)
+ return -ENOMEM;
+
+ strim(name);
+
+ guard(mutex)(&aggr->lock);
+
+ if (aggr_is_active(aggr))
+ return -EBUSY;
+
+ kfree(line->name);
+ line->name = no_free_ptr(name);
+
+ return count;
+}
+CONFIGFS_ATTR(gpio_aggr_line_, name);
+
static ssize_t
gpio_aggr_line_offset_show(struct config_item *item, char *page)
{
@@ -707,6 +783,7 @@ CONFIGFS_ATTR(gpio_aggr_line_, offset);
static struct configfs_attribute *gpio_aggr_line_attrs[] = {
&gpio_aggr_line_attr_key,
+ &gpio_aggr_line_attr_name,
&gpio_aggr_line_attr_offset,
NULL
};
@@ -795,6 +872,7 @@ gpio_aggr_line_release(struct config_item *item)
aggr_line_del(aggr, line);
kfree(line->key);
+ kfree(line->name);
kfree(line);
}
--
2.45.2
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH v3 10/13] gpio: aggregator: rename 'name' to 'key' in aggr_parse()
2025-02-16 12:58 [PATCH v3 00/13] Introduce configfs-based interface for gpio-aggregator Koichiro Den
` (8 preceding siblings ...)
2025-02-16 12:58 ` [PATCH v3 09/13] gpio: aggregator: add 'name' attribute for custom GPIO line names Koichiro Den
@ 2025-02-16 12:58 ` Koichiro Den
2025-02-17 13:08 ` Geert Uytterhoeven
2025-02-16 12:58 ` [PATCH v3 11/13] gpio: aggregator: expose aggregator created via legacy sysfs to configfs Koichiro Den
` (3 subsequent siblings)
13 siblings, 1 reply; 24+ messages in thread
From: Koichiro Den @ 2025-02-16 12:58 UTC (permalink / raw)
To: linux-gpio
Cc: brgl, geert+renesas, linus.walleij, maciej.borzecki, linux-kernel
Rename the local variable 'name' in aggr_parse() to 'key' because struct
gpio_aggregator_line now uses the 'name' field for the custom line name
and the local variable actually represents a 'key'. This change prepares
for the next but one commit.
No functional change.
Signed-off-by: Koichiro Den <koichiro.den@canonical.com>
---
drivers/gpio/gpio-aggregator.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/gpio/gpio-aggregator.c b/drivers/gpio/gpio-aggregator.c
index 692d90246674..2e993c9a7ce5 100644
--- a/drivers/gpio/gpio-aggregator.c
+++ b/drivers/gpio/gpio-aggregator.c
@@ -988,7 +988,7 @@ static struct configfs_subsystem gpio_aggr_subsys = {
static int aggr_parse(struct gpio_aggregator *aggr)
{
char *args = skip_spaces(aggr->args);
- char *name, *offsets, *p;
+ char *key, *offsets, *p;
unsigned int i, n = 0;
int error = 0;
@@ -997,18 +997,18 @@ static int aggr_parse(struct gpio_aggregator *aggr)
if (!bitmap)
return -ENOMEM;
- args = next_arg(args, &name, &p);
+ args = next_arg(args, &key, &p);
while (*args) {
args = next_arg(args, &offsets, &p);
p = get_options(offsets, 0, &error);
if (error == 0 || *p) {
/* Named GPIO line */
- error = aggr_add_gpio(aggr, name, U16_MAX, &n);
+ error = aggr_add_gpio(aggr, key, U16_MAX, &n);
if (error)
return error;
- name = offsets;
+ key = offsets;
continue;
}
@@ -1020,12 +1020,12 @@ static int aggr_parse(struct gpio_aggregator *aggr)
}
for_each_set_bit(i, bitmap, AGGREGATOR_MAX_GPIOS) {
- error = aggr_add_gpio(aggr, name, i, &n);
+ error = aggr_add_gpio(aggr, key, i, &n);
if (error)
return error;
}
- args = next_arg(args, &name, &p);
+ args = next_arg(args, &key, &p);
}
if (!n) {
--
2.45.2
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [PATCH v3 10/13] gpio: aggregator: rename 'name' to 'key' in aggr_parse()
2025-02-16 12:58 ` [PATCH v3 10/13] gpio: aggregator: rename 'name' to 'key' in aggr_parse() Koichiro Den
@ 2025-02-17 13:08 ` Geert Uytterhoeven
0 siblings, 0 replies; 24+ messages in thread
From: Geert Uytterhoeven @ 2025-02-17 13:08 UTC (permalink / raw)
To: Koichiro Den
Cc: linux-gpio, brgl, linus.walleij, maciej.borzecki, linux-kernel
On Sun, 16 Feb 2025 at 13:58, Koichiro Den <koichiro.den@canonical.com> wrote:
> Rename the local variable 'name' in aggr_parse() to 'key' because struct
> gpio_aggregator_line now uses the 'name' field for the custom line name
> and the local variable actually represents a 'key'. This change prepares
> for the next but one commit.
>
> No functional change.
>
> Signed-off-by: Koichiro Den <koichiro.den@canonical.com>
My
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
is still valid.
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
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH v3 11/13] gpio: aggregator: expose aggregator created via legacy sysfs to configfs
2025-02-16 12:58 [PATCH v3 00/13] Introduce configfs-based interface for gpio-aggregator Koichiro Den
` (9 preceding siblings ...)
2025-02-16 12:58 ` [PATCH v3 10/13] gpio: aggregator: rename 'name' to 'key' in aggr_parse() Koichiro Den
@ 2025-02-16 12:58 ` Koichiro Den
2025-02-16 12:58 ` [PATCH v3 12/13] gpio: aggregator: cancel deferred probe for devices created via configfs Koichiro Den
` (2 subsequent siblings)
13 siblings, 0 replies; 24+ messages in thread
From: Koichiro Den @ 2025-02-16 12:58 UTC (permalink / raw)
To: linux-gpio
Cc: brgl, geert+renesas, linus.walleij, maciej.borzecki, linux-kernel
Expose settings for aggregators created using the sysfs 'new_device'
interface to configfs. Once written to 'new_device', an "_sysfs.<N>" path
appears in the configfs regardless of whether the probe succeeds.
Consequently, users can no longer use that prefix for custom GPIO
aggregator names. The 'live' attribute changes to 1 when the probe
succeeds and the GPIO forwarder is instantiated.
Note that the aggregator device created via sysfs is asynchronous, i.e.
writing into 'new_device' returns without waiting for probe completion,
and the probe may succeed, fail, or eventually succeed via deferred
probe. Thus, the 'live' attribute may change from 0 to 1 asynchronously
without notice. So, editing key/offset/name while it's waiting for
deferred probe is prohibited.
The configfs auto-generation relies on create_default_group(), which
inherently prohibits rmdir(2). To align with the limitation, this commit
also prohibits mkdir(2) for them. When users want to change the number
of lines for an aggregator initialized via 'new_device', they need to
tear down the device using 'delete_device' and reconfigure it from
scratch. This does not break previous behavior; users of legacy sysfs
interface simply gain additional almost read-only configfs exposure.
Still, users can write to the 'live' attribute to toggle the device
unless it's waiting for deferred probe. So once probe succeeds, they can
deactivate it in the same manner as the devices initialized via
configfs.
Signed-off-by: Koichiro Den <koichiro.den@canonical.com>
---
drivers/gpio/gpio-aggregator.c | 117 +++++++++++++++++++++++++++++----
1 file changed, 105 insertions(+), 12 deletions(-)
diff --git a/drivers/gpio/gpio-aggregator.c b/drivers/gpio/gpio-aggregator.c
index 2e993c9a7ce5..8f8793f27211 100644
--- a/drivers/gpio/gpio-aggregator.c
+++ b/drivers/gpio/gpio-aggregator.c
@@ -33,6 +33,7 @@
#include "gpio-pseudo.h"
#define AGGREGATOR_MAX_GPIOS 512
+#define AGGREGATOR_LEGACY_PREFIX "_sysfs"
/*
* GPIO Aggregator sysfs interface
@@ -131,6 +132,14 @@ static bool aggr_is_active(struct gpio_aggregator *aggr)
return aggr->common.pdev && platform_get_drvdata(aggr->common.pdev);
}
+/* Only aggregators created via legacy sysfs can be "activating". */
+static bool aggr_is_activating(struct gpio_aggregator *aggr)
+{
+ lockdep_assert_held(&aggr->lock);
+
+ return aggr->common.pdev && !platform_get_drvdata(aggr->common.pdev);
+}
+
static size_t aggr_count_lines(struct gpio_aggregator *aggr)
{
lockdep_assert_held(&aggr->lock);
@@ -188,6 +197,18 @@ static void aggr_line_del(struct gpio_aggregator *aggr,
list_del(&line->entry);
}
+static void aggr_free_lines(struct gpio_aggregator *aggr)
+{
+ struct gpio_aggregator_line *line, *tmp;
+
+ list_for_each_entry_safe(line, tmp, &aggr->list_head, entry) {
+ configfs_unregister_group(&line->group);
+ aggr_line_del(aggr, line);
+ kfree(line->key);
+ kfree(line);
+ }
+}
+
/*
* GPIO Forwarder
@@ -687,7 +708,7 @@ gpio_aggr_line_key_store(struct config_item *item, const char *page,
guard(mutex)(&aggr->lock);
- if (aggr_is_active(aggr))
+ if (aggr_is_activating(aggr) || aggr_is_active(aggr))
return -EBUSY;
kfree(line->key);
@@ -724,7 +745,7 @@ gpio_aggr_line_name_store(struct config_item *item, const char *page,
guard(mutex)(&aggr->lock);
- if (aggr_is_active(aggr))
+ if (aggr_is_activating(aggr) || aggr_is_active(aggr))
return -EBUSY;
kfree(line->name);
@@ -772,7 +793,7 @@ gpio_aggr_line_offset_store(struct config_item *item, const char *page,
guard(mutex)(&aggr->lock);
- if (aggr_is_active(aggr))
+ if (aggr_is_activating(aggr) || aggr_is_active(aggr))
return -EBUSY;
line->offset = offset;
@@ -831,11 +852,12 @@ gpio_aggr_device_live_store(struct config_item *item, const char *page,
if (!try_module_get(THIS_MODULE))
return -ENOENT;
- if (live)
+ if (live && !aggr->init_via_sysfs)
aggr_lockup_configfs(aggr, true);
scoped_guard(mutex, &aggr->lock) {
- if (live == aggr_is_active(aggr))
+ if (aggr_is_activating(aggr) ||
+ (live == aggr_is_active(aggr)))
ret = -EPERM;
else if (live)
ret = aggr_activate(aggr);
@@ -847,7 +869,7 @@ gpio_aggr_device_live_store(struct config_item *item, const char *page,
* Undepend is required only if device disablement (live == 0)
* succeeds or if device enablement (live == 1) fails.
*/
- if (live == !!ret)
+ if (live == !!ret && !aggr->init_via_sysfs)
aggr_lockup_configfs(aggr, false);
module_put(THIS_MODULE);
@@ -893,7 +915,7 @@ static void gpio_aggr_device_release(struct config_item *item)
guard(mutex)(&aggr->lock);
/*
- * If the aggregator is active, this code wouldn't be reached,
+ * At this point, aggr is neither active nor activating,
* so calling aggr_deactivate() is always unnecessary.
*/
aggr_free(aggr);
@@ -915,6 +937,15 @@ gpio_aggr_device_make_group(struct config_group *group, const char *name)
if (ret != 1 || nchar != strlen(name))
return ERR_PTR(-EINVAL);
+ if (aggr->init_via_sysfs)
+ /*
+ * Aggregators created via legacy sysfs interface are exposed as
+ * default groups, which means rmdir(2) is prohibited for them.
+ * For simplicity, and to avoid confusion, we also prohibit
+ * mkdir(2).
+ */
+ return ERR_PTR(-EPERM);
+
guard(mutex)(&aggr->lock);
if (aggr_is_active(aggr))
@@ -952,6 +983,14 @@ gpio_aggr_make_group(struct config_group *group, const char *name)
struct gpio_aggregator *aggr;
int ret;
+ /*
+ * "_sysfs" prefix is reserved for auto-generated config group
+ * for devices create via legacy sysfs interface.
+ */
+ if (strncmp(name, AGGREGATOR_LEGACY_PREFIX,
+ sizeof(AGGREGATOR_LEGACY_PREFIX)) == 0)
+ return ERR_PTR(-EINVAL);
+
/* arg space is unneeded */
ret = aggr_alloc(&aggr, 0);
if (ret)
@@ -988,6 +1027,8 @@ static struct configfs_subsystem gpio_aggr_subsys = {
static int aggr_parse(struct gpio_aggregator *aggr)
{
char *args = skip_spaces(aggr->args);
+ struct gpio_aggregator_line *line;
+ char name[CONFIGFS_ITEM_NAME_LEN];
char *key, *offsets, *p;
unsigned int i, n = 0;
int error = 0;
@@ -999,14 +1040,29 @@ static int aggr_parse(struct gpio_aggregator *aggr)
args = next_arg(args, &key, &p);
while (*args) {
+ scnprintf(name, sizeof(name), "line%u", n);
+
args = next_arg(args, &offsets, &p);
p = get_options(offsets, 0, &error);
if (error == 0 || *p) {
/* Named GPIO line */
+ line = aggr_line_alloc(aggr, n, key, -1);
+ if (!line) {
+ error = -ENOMEM;
+ goto err;
+ }
+ config_group_init_type_name(&line->group, name,
+ &gpio_aggr_line_type);
+ error = configfs_register_group(&aggr->group,
+ &line->group);
+ if (error)
+ goto err;
+ aggr_line_add(aggr, line);
+
error = aggr_add_gpio(aggr, key, U16_MAX, &n);
if (error)
- return error;
+ goto err;
key = offsets;
continue;
@@ -1020,9 +1076,22 @@ static int aggr_parse(struct gpio_aggregator *aggr)
}
for_each_set_bit(i, bitmap, AGGREGATOR_MAX_GPIOS) {
+ line = aggr_line_alloc(aggr, n, key, i);
+ if (!line) {
+ error = -ENOMEM;
+ goto err;
+ }
+ config_group_init_type_name(&line->group, name,
+ &gpio_aggr_line_type);
+ error = configfs_register_group(&aggr->group,
+ &line->group);
+ if (error)
+ goto err;
+ aggr_line_add(aggr, line);
+
error = aggr_add_gpio(aggr, key, i, &n);
if (error)
- return error;
+ goto err;
}
args = next_arg(args, &key, &p);
@@ -1030,15 +1099,20 @@ static int aggr_parse(struct gpio_aggregator *aggr)
if (!n) {
pr_err("No GPIOs specified\n");
- return -EINVAL;
+ goto err;
}
return 0;
+
+err:
+ aggr_free_lines(aggr);
+ return error;
}
static ssize_t new_device_store(struct device_driver *driver, const char *buf,
size_t count)
{
+ char name[CONFIGFS_ITEM_NAME_LEN];
struct gpio_aggregator *aggr;
struct platform_device *pdev;
int res;
@@ -1067,10 +1141,24 @@ static ssize_t new_device_store(struct device_driver *driver, const char *buf,
goto free_table;
}
- res = aggr_parse(aggr);
+ scnprintf(name, sizeof(name), "%s.%d", AGGREGATOR_LEGACY_PREFIX, aggr->id);
+ config_group_init_type_name(&aggr->group, name, &gpio_aggr_device_type);
+
+ /*
+ * Since the device created by sysfs might be toggled via configfs
+ * 'live' attribute later, this initialization is needed.
+ */
+ pseudo_gpio_init(&aggr->common);
+
+ /* Expose to configfs */
+ res = configfs_register_group(&gpio_aggr_subsys.su_group, &aggr->group);
if (res)
goto free_dev_id;
+ res = aggr_parse(aggr);
+ if (res)
+ goto unregister_group;
+
gpiod_add_lookup_table(aggr->lookups);
pdev = platform_device_register_simple(DRV_NAME, aggr->id, NULL, 0);
@@ -1085,6 +1173,8 @@ static ssize_t new_device_store(struct device_driver *driver, const char *buf,
remove_table:
gpiod_remove_lookup_table(aggr->lookups);
+unregister_group:
+ configfs_unregister_group(&aggr->group);
free_dev_id:
kfree(aggr->lookups->dev_id);
free_table:
@@ -1100,7 +1190,10 @@ static DRIVER_ATTR_WO(new_device);
static void gpio_aggregator_free(struct gpio_aggregator *aggr)
{
- aggr_deactivate(aggr);
+ if (aggr_is_activating(aggr) || aggr_is_active(aggr))
+ aggr_deactivate(aggr);
+ aggr_free_lines(aggr);
+ configfs_unregister_group(&aggr->group);
kfree(aggr);
}
--
2.45.2
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH v3 12/13] gpio: aggregator: cancel deferred probe for devices created via configfs
2025-02-16 12:58 [PATCH v3 00/13] Introduce configfs-based interface for gpio-aggregator Koichiro Den
` (10 preceding siblings ...)
2025-02-16 12:58 ` [PATCH v3 11/13] gpio: aggregator: expose aggregator created via legacy sysfs to configfs Koichiro Den
@ 2025-02-16 12:58 ` Koichiro Den
2025-02-16 12:58 ` [PATCH v3 13/13] Documentation: gpio: document configfs interface for gpio-aggregator Koichiro Den
2025-02-16 15:56 ` [PATCH v3 00/13] Introduce configfs-based " Bartosz Golaszewski
13 siblings, 0 replies; 24+ messages in thread
From: Koichiro Den @ 2025-02-16 12:58 UTC (permalink / raw)
To: linux-gpio
Cc: brgl, geert+renesas, linus.walleij, maciej.borzecki, linux-kernel
For aggregators initialized via configfs, write 1 to 'live' waits for
probe completion and returns an error if the probe fails, unlike the
legacy sysfs interface, which is asynchronous.
Since users control the liveness of the aggregator device and might be
editing configurations while 'live' is 0, deferred probing is both
unnatural and unsafe.
Cancel deferred probe for purely configfs-based aggregators when probe
fails.
Signed-off-by: Koichiro Den <koichiro.den@canonical.com>
---
drivers/gpio/gpio-aggregator.c | 30 +++++++++++++++++++++++++++---
1 file changed, 27 insertions(+), 3 deletions(-)
diff --git a/drivers/gpio/gpio-aggregator.c b/drivers/gpio/gpio-aggregator.c
index 8f8793f27211..f6beb9f41b9a 100644
--- a/drivers/gpio/gpio-aggregator.c
+++ b/drivers/gpio/gpio-aggregator.c
@@ -72,6 +72,10 @@ struct gpio_aggregator_line {
enum gpio_lookup_flags flags;
};
+struct gpio_aggregator_pdev_meta {
+ bool init_via_sysfs;
+};
+
static DEFINE_MUTEX(gpio_aggregator_lock); /* protects idr */
static DEFINE_IDR(gpio_aggregator_idr);
@@ -1112,6 +1116,7 @@ static int aggr_parse(struct gpio_aggregator *aggr)
static ssize_t new_device_store(struct device_driver *driver, const char *buf,
size_t count)
{
+ struct gpio_aggregator_pdev_meta meta = { .init_via_sysfs = true };
char name[CONFIGFS_ITEM_NAME_LEN];
struct gpio_aggregator *aggr;
struct platform_device *pdev;
@@ -1161,7 +1166,7 @@ static ssize_t new_device_store(struct device_driver *driver, const char *buf,
gpiod_add_lookup_table(aggr->lookups);
- pdev = platform_device_register_simple(DRV_NAME, aggr->id, NULL, 0);
+ pdev = platform_device_register_data(NULL, DRV_NAME, aggr->id, &meta, sizeof(meta));
if (IS_ERR(pdev)) {
res = PTR_ERR(pdev);
goto remove_table;
@@ -1242,14 +1247,15 @@ static struct attribute *gpio_aggregator_attrs[] = {
};
ATTRIBUTE_GROUPS(gpio_aggregator);
-
/*
* GPIO Aggregator platform device
*/
static int gpio_aggregator_probe(struct platform_device *pdev)
{
+ struct gpio_aggregator_pdev_meta *meta;
struct device *dev = &pdev->dev;
+ bool init_via_sysfs = false;
struct gpio_desc **descs;
struct gpiochip_fwd *fwd;
unsigned long features;
@@ -1263,10 +1269,28 @@ static int gpio_aggregator_probe(struct platform_device *pdev)
if (!descs)
return -ENOMEM;
+ meta = dev_get_platdata(&pdev->dev);
+ if (meta && meta->init_via_sysfs)
+ init_via_sysfs = true;
+
for (i = 0; i < n; i++) {
descs[i] = devm_gpiod_get_index(dev, NULL, i, GPIOD_ASIS);
- if (IS_ERR(descs[i]))
+ if (IS_ERR(descs[i])) {
+ /*
+ * Deferred probing is not suitable when the aggregator
+ * is created via configfs. They should just retry later
+ * whenever they like. For device creation via sysfs,
+ * error is propagated without overriding for backward
+ * compatibility. .prevent_deferred_probe is kept unset
+ * for other cases.
+ */
+ if (!init_via_sysfs && !dev_of_node(dev) &&
+ descs[i] == ERR_PTR(-EPROBE_DEFER)) {
+ pr_warn("Deferred probe canceled for creation via configfs.\n");
+ return -ENODEV;
+ }
return PTR_ERR(descs[i]);
+ }
}
features = (uintptr_t)device_get_match_data(dev);
--
2.45.2
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH v3 13/13] Documentation: gpio: document configfs interface for gpio-aggregator
2025-02-16 12:58 [PATCH v3 00/13] Introduce configfs-based interface for gpio-aggregator Koichiro Den
` (11 preceding siblings ...)
2025-02-16 12:58 ` [PATCH v3 12/13] gpio: aggregator: cancel deferred probe for devices created via configfs Koichiro Den
@ 2025-02-16 12:58 ` Koichiro Den
2025-02-16 15:56 ` [PATCH v3 00/13] Introduce configfs-based " Bartosz Golaszewski
13 siblings, 0 replies; 24+ messages in thread
From: Koichiro Den @ 2025-02-16 12:58 UTC (permalink / raw)
To: linux-gpio
Cc: brgl, geert+renesas, linus.walleij, maciej.borzecki, linux-kernel
Add documentation for the newly added configfs-based interface for GPIO
aggregator.
Signed-off-by: Koichiro Den <koichiro.den@canonical.com>
---
.../admin-guide/gpio/gpio-aggregator.rst | 107 ++++++++++++++++++
1 file changed, 107 insertions(+)
diff --git a/Documentation/admin-guide/gpio/gpio-aggregator.rst b/Documentation/admin-guide/gpio/gpio-aggregator.rst
index 5cd1e7221756..8374a9df9105 100644
--- a/Documentation/admin-guide/gpio/gpio-aggregator.rst
+++ b/Documentation/admin-guide/gpio/gpio-aggregator.rst
@@ -69,6 +69,113 @@ write-only attribute files in sysfs.
$ echo gpio-aggregator.0 > delete_device
+Aggregating GPIOs using Configfs
+--------------------------------
+
+**Group:** ``/config/gpio-aggregator``
+
+ This is the root directory of the gpio-aggregator configfs tree.
+
+**Group:** ``/config/gpio-aggregator/<example-name>``
+
+ This directory represents a GPIO aggregator device. You can assign any
+ name to ``<example-name>`` (e.g. ``agg0``), except names starting with
+ ``_sysfs`` prefix, which are reserved for auto-generated configfs
+ entries corresponding to devices created via Sysfs.
+
+**Attribute:** ``/config/gpio-aggregator/<example-name>/live``
+
+ The ``live`` attribute allows to trigger the actual creation of the device
+ once it's fully configured. Accepted values are:
+
+ * ``1``, ``yes``, ``true`` : enable the virtual device
+ * ``0``, ``no``, ``false`` : disable the virtual device
+
+**Attribute:** ``/config/gpio-aggregator/<example-name>/dev_name``
+
+ The read-only ``dev_name`` attribute exposes the name of the device as it
+ will appear in the system on the platform bus (e.g. ``gpio-aggregator.0``).
+ This is useful for identifying a character device for the newly created
+ aggregator. If it's ``gpio-aggregator.0``,
+ ``/sys/devices/platform/gpio-aggregator.0/gpiochipX`` path tells you that the
+ GPIO device id is ``X``.
+
+You must create subdirectories for each virtual line you want to
+instantiate, named exactly as ``line0``, ``line1``, ..., ``lineY``, when
+you want to instantiate ``Y+1`` (Y >= 0) lines. Configure all lines before
+activating the device by setting ``live`` to 1.
+
+**Group:** ``/config/gpio-aggregator/<example-name>/<lineY>/``
+
+ This directory represents a GPIO line to include in the aggregator.
+
+**Attribute:** ``/config/gpio-aggregator/<example-name>/<lineY>/key``
+
+**Attribute:** ``/config/gpio-aggregator/<example-name>/<lineY>/offset``
+
+ The default values after creating the ``<lineY>`` directory are:
+
+ * ``key`` : <empty>
+ * ``offset`` : -1
+
+ ``key`` must always be explicitly configured, while ``offset`` depends.
+ Two configuration patterns exist for each ``<lineY>``:
+
+ (a). For lookup by GPIO line name:
+
+ * Set ``key`` to the line name.
+ * Ensure ``offset`` remains -1 (the default).
+
+ (b). For lookup by GPIO chip name and the line offset within the chip:
+
+ * Set ``key`` to the chip name.
+ * Set ``offset`` to the line offset (0 <= ``offset`` < 65535).
+
+**Attribute:** ``/config/gpio-aggregator/<example-name>/<lineY>/name``
+
+ The ``name`` attribute sets a custom name for lineY. If left unset, the
+ line will remain unnamed.
+
+Once the configuration is done, the ``'live'`` attribute must be set to 1
+in order to instantiate the aggregator device. It can be set back to 0 to
+destroy the virtual device. The module will synchronously wait for the new
+aggregator device to be successfully probed and if this doesn't happen, writing
+to ``'live'`` will result in an error. This is a different behaviour from the
+case when you create it using sysfs ``new_device`` interface.
+
+.. note::
+
+ For aggregators created via Sysfs, the configfs entries are
+ auto-generated and appear as ``/config/gpio-aggregator/_sysfs.<N>/``. You
+ cannot add or remove line directories with mkdir(2)/rmdir(2). To modify
+ lines, you must use the "delete_device" interface to tear down the
+ existing device and reconfigure it from scratch. However, you can still
+ toggle the aggregator with the ``live`` attribute and adjust the
+ ``key``, ``offset``, and ``name`` attributes for each line when ``live``
+ is set to 0 by hand (i.e. it's not waiting for deferred probe).
+
+Sample configuration commands
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+.. code-block:: sh
+
+ # Create a directory for an aggregator device
+ $ mkdir /sys/kernel/config/gpio-aggregator/agg0
+
+ # Configure each line
+ $ mkdir /sys/kernel/config/gpio-aggregator/agg0/line0
+ $ echo gpiochip0 > /sys/kernel/config/gpio-aggregator/agg0/line0/key
+ $ echo 6 > /sys/kernel/config/gpio-aggregator/agg0/line0/offset
+ $ echo test0 > /sys/kernel/config/gpio-aggregator/agg0/line0/name
+ $ mkdir /sys/kernel/config/gpio-aggregator/agg0/line1
+ $ echo gpiochip0 > /sys/kernel/config/gpio-aggregator/agg0/line1/key
+ $ echo 7 > /sys/kernel/config/gpio-aggregator/agg0/line1/offset
+ $ echo test1 > /sys/kernel/config/gpio-aggregator/agg0/line1/name
+
+ # Activate the aggregator device
+ $ echo 1 > /sys/kernel/config/gpio-aggregator/agg0/live
+
+
Generic GPIO Driver
-------------------
--
2.45.2
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [PATCH v3 00/13] Introduce configfs-based interface for gpio-aggregator
2025-02-16 12:58 [PATCH v3 00/13] Introduce configfs-based interface for gpio-aggregator Koichiro Den
` (12 preceding siblings ...)
2025-02-16 12:58 ` [PATCH v3 13/13] Documentation: gpio: document configfs interface for gpio-aggregator Koichiro Den
@ 2025-02-16 15:56 ` Bartosz Golaszewski
2025-02-17 1:07 ` Koichiro Den
13 siblings, 1 reply; 24+ messages in thread
From: Bartosz Golaszewski @ 2025-02-16 15:56 UTC (permalink / raw)
To: Koichiro Den
Cc: linux-gpio, geert+renesas, linus.walleij, maciej.borzecki,
linux-kernel
On Sun, Feb 16, 2025 at 1:58 PM Koichiro Den <koichiro.den@canonical.com> wrote:
>
> This patch series introduces a configfs-based interface to gpio-aggregator
> to address limitations in the existing 'new_device' interface.
>
> The existing 'new_device' interface has several limitations:
>
> Issue#1. No way to determine when GPIO aggregator creation is complete.
> Issue#2. No way to retrieve errors when creating a GPIO aggregator.
> Issue#3. No way to trace a GPIO line of an aggregator back to its
> corresponding physical device.
> Issue#4. The 'new_device' echo does not indicate which virtual
> gpiochip<N> was created.
> Issue#5. No way to assign names to GPIO lines exported through an
> aggregator.
>
> Although Issue#1 to #3 could technically be resolved easily without
> configfs, using configfs offers a streamlined, modern, and extensible
> approach, especially since gpio-sim and gpio-virtuser already utilize
> configfs.
>
> This v3 patch series includes 13 patches:
>
> Patch#1-7: Prepare for Patch#8
> * #1: Prepare for the following patches.
> * #2: Fix an issue that was spotted during v3 preparation.
> * #3: Add gpio-pseudo.[ch] to reduce code duplications.
> * #4: Update gpio-sim to use gpio-pseudo.[ch].
> * #5: Update gpio-virtuser to use gpio-pseudo.[ch].
> * #6: Update gpio-aggregator to use gpio-pseudo.[ch].
> * #7: Add aggr_alloc() to reduce code duplication.
Please don't ram this new functionality into an unrelated series.
Split it into the gpio-pseudo code, factoring out common parts and
converting existing drivers, then send the aggregator series saying it
depends on the former. Otherwise it gets way too complex to review.
Bartosz
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH v3 00/13] Introduce configfs-based interface for gpio-aggregator
2025-02-16 15:56 ` [PATCH v3 00/13] Introduce configfs-based " Bartosz Golaszewski
@ 2025-02-17 1:07 ` Koichiro Den
2025-02-17 1:17 ` Koichiro Den
0 siblings, 1 reply; 24+ messages in thread
From: Koichiro Den @ 2025-02-17 1:07 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: linux-gpio, geert+renesas, linus.walleij, maciej.borzecki,
linux-kernel
On Sun, Feb 16, 2025 at 04:56:59PM GMT, Bartosz Golaszewski wrote:
> On Sun, Feb 16, 2025 at 1:58 PM Koichiro Den <koichiro.den@canonical.com> wrote:
> >
> > This patch series introduces a configfs-based interface to gpio-aggregator
> > to address limitations in the existing 'new_device' interface.
> >
> > The existing 'new_device' interface has several limitations:
> >
> > Issue#1. No way to determine when GPIO aggregator creation is complete.
> > Issue#2. No way to retrieve errors when creating a GPIO aggregator.
> > Issue#3. No way to trace a GPIO line of an aggregator back to its
> > corresponding physical device.
> > Issue#4. The 'new_device' echo does not indicate which virtual
> > gpiochip<N> was created.
> > Issue#5. No way to assign names to GPIO lines exported through an
> > aggregator.
> >
> > Although Issue#1 to #3 could technically be resolved easily without
> > configfs, using configfs offers a streamlined, modern, and extensible
> > approach, especially since gpio-sim and gpio-virtuser already utilize
> > configfs.
> >
> > This v3 patch series includes 13 patches:
> >
> > Patch#1-7: Prepare for Patch#8
> > * #1: Prepare for the following patches.
> > * #2: Fix an issue that was spotted during v3 preparation.
> > * #3: Add gpio-pseudo.[ch] to reduce code duplications.
> > * #4: Update gpio-sim to use gpio-pseudo.[ch].
> > * #5: Update gpio-virtuser to use gpio-pseudo.[ch].
> > * #6: Update gpio-aggregator to use gpio-pseudo.[ch].
> > * #7: Add aggr_alloc() to reduce code duplication.
>
> Please don't ram this new functionality into an unrelated series.
> Split it into the gpio-pseudo code, factoring out common parts and
> converting existing drivers, then send the aggregator series saying it
> depends on the former. Otherwise it gets way too complex to review.
Ok, I'll do so.
Thanks,
Koichiro
>
> Bartosz
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v3 00/13] Introduce configfs-based interface for gpio-aggregator
2025-02-17 1:07 ` Koichiro Den
@ 2025-02-17 1:17 ` Koichiro Den
2025-02-17 8:12 ` Bartosz Golaszewski
0 siblings, 1 reply; 24+ messages in thread
From: Koichiro Den @ 2025-02-17 1:17 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: linux-gpio, geert+renesas, linus.walleij, maciej.borzecki,
linux-kernel
On Mon, Feb 17, 2025 at 10:07:03AM GMT, Koichiro Den wrote:
> On Sun, Feb 16, 2025 at 04:56:59PM GMT, Bartosz Golaszewski wrote:
> > On Sun, Feb 16, 2025 at 1:58 PM Koichiro Den <koichiro.den@canonical.com> wrote:
> > >
> > > This patch series introduces a configfs-based interface to gpio-aggregator
> > > to address limitations in the existing 'new_device' interface.
> > >
> > > The existing 'new_device' interface has several limitations:
> > >
> > > Issue#1. No way to determine when GPIO aggregator creation is complete.
> > > Issue#2. No way to retrieve errors when creating a GPIO aggregator.
> > > Issue#3. No way to trace a GPIO line of an aggregator back to its
> > > corresponding physical device.
> > > Issue#4. The 'new_device' echo does not indicate which virtual
> > > gpiochip<N> was created.
> > > Issue#5. No way to assign names to GPIO lines exported through an
> > > aggregator.
> > >
> > > Although Issue#1 to #3 could technically be resolved easily without
> > > configfs, using configfs offers a streamlined, modern, and extensible
> > > approach, especially since gpio-sim and gpio-virtuser already utilize
> > > configfs.
> > >
> > > This v3 patch series includes 13 patches:
> > >
> > > Patch#1-7: Prepare for Patch#8
> > > * #1: Prepare for the following patches.
> > > * #2: Fix an issue that was spotted during v3 preparation.
> > > * #3: Add gpio-pseudo.[ch] to reduce code duplications.
> > > * #4: Update gpio-sim to use gpio-pseudo.[ch].
> > > * #5: Update gpio-virtuser to use gpio-pseudo.[ch].
> > > * #6: Update gpio-aggregator to use gpio-pseudo.[ch].
> > > * #7: Add aggr_alloc() to reduce code duplication.
> >
> > Please don't ram this new functionality into an unrelated series.
> > Split it into the gpio-pseudo code, factoring out common parts and
> > converting existing drivers, then send the aggregator series saying it
> > depends on the former. Otherwise it gets way too complex to review.
>
> Ok, I'll do so.
> Thanks,
Should Patch#2 also be split off into another submission?
Koichiro
>
> Koichiro
>
> >
> > Bartosz
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v3 00/13] Introduce configfs-based interface for gpio-aggregator
2025-02-17 1:17 ` Koichiro Den
@ 2025-02-17 8:12 ` Bartosz Golaszewski
2025-02-17 12:42 ` Koichiro Den
0 siblings, 1 reply; 24+ messages in thread
From: Bartosz Golaszewski @ 2025-02-17 8:12 UTC (permalink / raw)
To: Koichiro Den
Cc: linux-gpio, geert+renesas, linus.walleij, maciej.borzecki,
linux-kernel
On Mon, Feb 17, 2025 at 2:18 AM Koichiro Den <koichiro.den@canonical.com> wrote:
>
> On Mon, Feb 17, 2025 at 10:07:03AM GMT, Koichiro Den wrote:
> > On Sun, Feb 16, 2025 at 04:56:59PM GMT, Bartosz Golaszewski wrote:
> > > On Sun, Feb 16, 2025 at 1:58 PM Koichiro Den <koichiro.den@canonical.com> wrote:
> > > >
> > > > This patch series introduces a configfs-based interface to gpio-aggregator
> > > > to address limitations in the existing 'new_device' interface.
> > > >
> > > > The existing 'new_device' interface has several limitations:
> > > >
> > > > Issue#1. No way to determine when GPIO aggregator creation is complete.
> > > > Issue#2. No way to retrieve errors when creating a GPIO aggregator.
> > > > Issue#3. No way to trace a GPIO line of an aggregator back to its
> > > > corresponding physical device.
> > > > Issue#4. The 'new_device' echo does not indicate which virtual
> > > > gpiochip<N> was created.
> > > > Issue#5. No way to assign names to GPIO lines exported through an
> > > > aggregator.
> > > >
> > > > Although Issue#1 to #3 could technically be resolved easily without
> > > > configfs, using configfs offers a streamlined, modern, and extensible
> > > > approach, especially since gpio-sim and gpio-virtuser already utilize
> > > > configfs.
> > > >
> > > > This v3 patch series includes 13 patches:
> > > >
> > > > Patch#1-7: Prepare for Patch#8
> > > > * #1: Prepare for the following patches.
> > > > * #2: Fix an issue that was spotted during v3 preparation.
> > > > * #3: Add gpio-pseudo.[ch] to reduce code duplications.
> > > > * #4: Update gpio-sim to use gpio-pseudo.[ch].
> > > > * #5: Update gpio-virtuser to use gpio-pseudo.[ch].
> > > > * #6: Update gpio-aggregator to use gpio-pseudo.[ch].
> > > > * #7: Add aggr_alloc() to reduce code duplication.
> > >
> > > Please don't ram this new functionality into an unrelated series.
> > > Split it into the gpio-pseudo code, factoring out common parts and
> > > converting existing drivers, then send the aggregator series saying it
> > > depends on the former. Otherwise it gets way too complex to review.
> >
> > Ok, I'll do so.
> > Thanks,
>
> Should Patch#2 also be split off into another submission?
>
I'd fold it into the aggregator rework but make it come first in the
series so that I can pick it up into fixes easily and send it for
stable.
Bart
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v3 00/13] Introduce configfs-based interface for gpio-aggregator
2025-02-17 8:12 ` Bartosz Golaszewski
@ 2025-02-17 12:42 ` Koichiro Den
0 siblings, 0 replies; 24+ messages in thread
From: Koichiro Den @ 2025-02-17 12:42 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: linux-gpio, geert+renesas, linus.walleij, maciej.borzecki,
linux-kernel
On Mon, Feb 17, 2025 at 09:12:07AM GMT, Bartosz Golaszewski wrote:
> On Mon, Feb 17, 2025 at 2:18 AM Koichiro Den <koichiro.den@canonical.com> wrote:
> >
> > On Mon, Feb 17, 2025 at 10:07:03AM GMT, Koichiro Den wrote:
> > > On Sun, Feb 16, 2025 at 04:56:59PM GMT, Bartosz Golaszewski wrote:
> > > > On Sun, Feb 16, 2025 at 1:58 PM Koichiro Den <koichiro.den@canonical.com> wrote:
> > > > >
> > > > > This patch series introduces a configfs-based interface to gpio-aggregator
> > > > > to address limitations in the existing 'new_device' interface.
> > > > >
> > > > > The existing 'new_device' interface has several limitations:
> > > > >
> > > > > Issue#1. No way to determine when GPIO aggregator creation is complete.
> > > > > Issue#2. No way to retrieve errors when creating a GPIO aggregator.
> > > > > Issue#3. No way to trace a GPIO line of an aggregator back to its
> > > > > corresponding physical device.
> > > > > Issue#4. The 'new_device' echo does not indicate which virtual
> > > > > gpiochip<N> was created.
> > > > > Issue#5. No way to assign names to GPIO lines exported through an
> > > > > aggregator.
> > > > >
> > > > > Although Issue#1 to #3 could technically be resolved easily without
> > > > > configfs, using configfs offers a streamlined, modern, and extensible
> > > > > approach, especially since gpio-sim and gpio-virtuser already utilize
> > > > > configfs.
> > > > >
> > > > > This v3 patch series includes 13 patches:
> > > > >
> > > > > Patch#1-7: Prepare for Patch#8
> > > > > * #1: Prepare for the following patches.
> > > > > * #2: Fix an issue that was spotted during v3 preparation.
> > > > > * #3: Add gpio-pseudo.[ch] to reduce code duplications.
> > > > > * #4: Update gpio-sim to use gpio-pseudo.[ch].
> > > > > * #5: Update gpio-virtuser to use gpio-pseudo.[ch].
> > > > > * #6: Update gpio-aggregator to use gpio-pseudo.[ch].
> > > > > * #7: Add aggr_alloc() to reduce code duplication.
> > > >
> > > > Please don't ram this new functionality into an unrelated series.
> > > > Split it into the gpio-pseudo code, factoring out common parts and
> > > > converting existing drivers, then send the aggregator series saying it
> > > > depends on the former. Otherwise it gets way too complex to review.
> > >
> > > Ok, I'll do so.
> > > Thanks,
> >
> > Should Patch#2 also be split off into another submission?
> >
>
> I'd fold it into the aggregator rework but make it come first in the
> series so that I can pick it up into fixes easily and send it for
> stable.
Ok, thanks for guiding!
Koichiro
>
> Bart
^ permalink raw reply [flat|nested] 24+ messages in thread