* [PATCH RFC 1/9] gpio: wcd934x: mark the GPIO controller as sleeping
2025-09-24 14:51 [PATCH RFC 0/9] gpio: improve support for shared GPIOs Bartosz Golaszewski
@ 2025-09-24 14:51 ` Bartosz Golaszewski
2025-09-24 14:51 ` [PATCH RFC 2/9] string: provide strends() Bartosz Golaszewski
` (12 subsequent siblings)
13 siblings, 0 replies; 43+ messages in thread
From: Bartosz Golaszewski @ 2025-09-24 14:51 UTC (permalink / raw)
To: Kees Cook, Mika Westerberg, Dmitry Torokhov, Andrew Morton,
Linus Walleij, Manivannan Sadhasivam, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Saravana Kannan,
Greg Kroah-Hartman, Andy Shevchenko, Bartosz Golaszewski,
Catalin Marinas, Will Deacon, Srinivas Kandagatla, Liam Girdwood,
Mark Brown, Jaroslav Kysela, Takashi Iwai
Cc: linux-hardening, linux-kernel, linux-gpio, linux-arm-kernel,
linux-sound, linux-arm-msm, Bartosz Golaszewski
From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
The slimbus regmap passed to the GPIO driver down from MFD does not use
fast_io. This means a mutex is used for locking and thus this GPIO chip
must not sleep. Change the can_sleep switch in struct gpio_chip to true.
Fixes: 59c324683400 ("gpio: wcd934x: Add support to wcd934x gpio controller")
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
---
drivers/gpio/gpio-wcd934x.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpio/gpio-wcd934x.c b/drivers/gpio/gpio-wcd934x.c
index 4af504c23e6ff5ad2bad1d70a0c42e2fd91f0c7a..572b85e773700ea0740d3e4efc06d3538f7e368e 100644
--- a/drivers/gpio/gpio-wcd934x.c
+++ b/drivers/gpio/gpio-wcd934x.c
@@ -103,7 +103,7 @@ static int wcd_gpio_probe(struct platform_device *pdev)
chip->base = -1;
chip->ngpio = WCD934X_NPINS;
chip->label = dev_name(dev);
- chip->can_sleep = false;
+ chip->can_sleep = true;
return devm_gpiochip_add_data(dev, chip, data);
}
--
2.48.1
^ permalink raw reply related [flat|nested] 43+ messages in thread* [PATCH RFC 2/9] string: provide strends()
2025-09-24 14:51 [PATCH RFC 0/9] gpio: improve support for shared GPIOs Bartosz Golaszewski
2025-09-24 14:51 ` [PATCH RFC 1/9] gpio: wcd934x: mark the GPIO controller as sleeping Bartosz Golaszewski
@ 2025-09-24 14:51 ` Bartosz Golaszewski
2025-09-24 14:51 ` [PATCH RFC 3/9] gpiolib: define GPIOD_FLAG_SHARED Bartosz Golaszewski
` (11 subsequent siblings)
13 siblings, 0 replies; 43+ messages in thread
From: Bartosz Golaszewski @ 2025-09-24 14:51 UTC (permalink / raw)
To: Kees Cook, Mika Westerberg, Dmitry Torokhov, Andrew Morton,
Linus Walleij, Manivannan Sadhasivam, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Saravana Kannan,
Greg Kroah-Hartman, Andy Shevchenko, Bartosz Golaszewski,
Catalin Marinas, Will Deacon, Srinivas Kandagatla, Liam Girdwood,
Mark Brown, Jaroslav Kysela, Takashi Iwai
Cc: linux-hardening, linux-kernel, linux-gpio, linux-arm-kernel,
linux-sound, linux-arm-msm, Bartosz Golaszewski
From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Implement a function for checking if a string ends with a different
string and add its kunit test cases.
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
---
include/linux/string.h | 2 ++
lib/string.c | 19 +++++++++++++++++++
lib/tests/string_kunit.c | 13 +++++++++++++
3 files changed, 34 insertions(+)
diff --git a/include/linux/string.h b/include/linux/string.h
index fdd3442c6bcbd786e177b6e87358e1065a0ffafc..16149404fc3aed535c094b9550f7bd7c9b44a0c9 100644
--- a/include/linux/string.h
+++ b/include/linux/string.h
@@ -562,4 +562,6 @@ static inline bool strstarts(const char *str, const char *prefix)
return strncmp(str, prefix, strlen(prefix)) == 0;
}
+bool strends(const char *str, const char *suffix);
+
#endif /* _LINUX_STRING_H_ */
diff --git a/lib/string.c b/lib/string.c
index b632c71df1a5061256f556c40a30587cf45dce18..ac18313d9c52b38132f0bfdd952cdec6b1354d76 100644
--- a/lib/string.c
+++ b/lib/string.c
@@ -880,3 +880,22 @@ void *memchr_inv(const void *start, int c, size_t bytes)
return check_bytes8(start, value, bytes % 8);
}
EXPORT_SYMBOL(memchr_inv);
+
+/**
+ * strends - Check if a string ends with another string.
+ * @str - NULL-terminated string to check against @suffix
+ * @suffix - NULL-terminated string defining the suffix to look for in @str
+ *
+ * Returns:
+ * True if @str ends with @suffix. False in all other cases.
+ */
+bool strends(const char *str, const char *suffix)
+{
+ unsigned int str_len = strlen(str), suffix_len = strlen(suffix);
+
+ if (str_len < suffix_len)
+ return false;
+
+ return !(strcmp(str + str_len - suffix_len, suffix));
+}
+EXPORT_SYMBOL(strends);
diff --git a/lib/tests/string_kunit.c b/lib/tests/string_kunit.c
index 0ed7448a26d3aa0fe9e2a6a894d4c49c2c0b86e0..f9a8e557ba7734c9848d58ff986407d8000f52ee 100644
--- a/lib/tests/string_kunit.c
+++ b/lib/tests/string_kunit.c
@@ -602,6 +602,18 @@ static void string_test_memtostr(struct kunit *test)
KUNIT_EXPECT_EQ(test, dest[7], '\0');
}
+static void string_test_strends(struct kunit *test)
+{
+ KUNIT_EXPECT_TRUE(test, strends("foo-bar", "bar"));
+ KUNIT_EXPECT_TRUE(test, strends("foo-bar", "-bar"));
+ KUNIT_EXPECT_TRUE(test, strends("foobar", "foobar"));
+ KUNIT_EXPECT_TRUE(test, strends("foobar", ""));
+ KUNIT_EXPECT_FALSE(test, strends("bar", "foobar"));
+ KUNIT_EXPECT_FALSE(test, strends("", "foo"));
+ KUNIT_EXPECT_FALSE(test, strends("foobar", "ba"));
+ KUNIT_EXPECT_TRUE(test, strends("", ""));
+}
+
static struct kunit_case string_test_cases[] = {
KUNIT_CASE(string_test_memset16),
KUNIT_CASE(string_test_memset32),
@@ -623,6 +635,7 @@ static struct kunit_case string_test_cases[] = {
KUNIT_CASE(string_test_strlcat),
KUNIT_CASE(string_test_strtomem),
KUNIT_CASE(string_test_memtostr),
+ KUNIT_CASE(string_test_strends),
{}
};
--
2.48.1
^ permalink raw reply related [flat|nested] 43+ messages in thread* [PATCH RFC 3/9] gpiolib: define GPIOD_FLAG_SHARED
2025-09-24 14:51 [PATCH RFC 0/9] gpio: improve support for shared GPIOs Bartosz Golaszewski
2025-09-24 14:51 ` [PATCH RFC 1/9] gpio: wcd934x: mark the GPIO controller as sleeping Bartosz Golaszewski
2025-09-24 14:51 ` [PATCH RFC 2/9] string: provide strends() Bartosz Golaszewski
@ 2025-09-24 14:51 ` Bartosz Golaszewski
2025-09-24 14:51 ` [PATCH RFC 4/9] gpiolib: implement low-level, shared GPIO support Bartosz Golaszewski
` (10 subsequent siblings)
13 siblings, 0 replies; 43+ messages in thread
From: Bartosz Golaszewski @ 2025-09-24 14:51 UTC (permalink / raw)
To: Kees Cook, Mika Westerberg, Dmitry Torokhov, Andrew Morton,
Linus Walleij, Manivannan Sadhasivam, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Saravana Kannan,
Greg Kroah-Hartman, Andy Shevchenko, Bartosz Golaszewski,
Catalin Marinas, Will Deacon, Srinivas Kandagatla, Liam Girdwood,
Mark Brown, Jaroslav Kysela, Takashi Iwai
Cc: linux-hardening, linux-kernel, linux-gpio, linux-arm-kernel,
linux-sound, linux-arm-msm, Bartosz Golaszewski
From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Define a new GPIO descriptor flag for marking pins that are shared by
multiple consumer. This flag will be used in several places so we need
to do it in advance and separately from other changes.
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
---
drivers/gpio/gpiolib.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/gpio/gpiolib.h b/drivers/gpio/gpiolib.h
index 2a003a7311e7ac5beeaa1c947d921149ad991acf..09ac92ed0853f2ff0013d1b16a5dd082c9fd162e 100644
--- a/drivers/gpio/gpiolib.h
+++ b/drivers/gpio/gpiolib.h
@@ -204,6 +204,7 @@ struct gpio_desc {
#define GPIOD_FLAG_EDGE_FALLING 17 /* GPIO CDEV detects falling edge events */
#define GPIOD_FLAG_EVENT_CLOCK_REALTIME 18 /* GPIO CDEV reports REALTIME timestamps in events */
#define GPIOD_FLAG_EVENT_CLOCK_HTE 19 /* GPIO CDEV reports hardware timestamps in events */
+#define GPIOD_FLAG_SHARED 20 /* GPIO is shared by multiple consumers */
/* Connection label */
struct gpio_desc_label __rcu *label;
--
2.48.1
^ permalink raw reply related [flat|nested] 43+ messages in thread* [PATCH RFC 4/9] gpiolib: implement low-level, shared GPIO support
2025-09-24 14:51 [PATCH RFC 0/9] gpio: improve support for shared GPIOs Bartosz Golaszewski
` (2 preceding siblings ...)
2025-09-24 14:51 ` [PATCH RFC 3/9] gpiolib: define GPIOD_FLAG_SHARED Bartosz Golaszewski
@ 2025-09-24 14:51 ` Bartosz Golaszewski
2025-09-24 14:51 ` [PATCH RFC 5/9] gpio: shared-proxy: implement the shared GPIO proxy driver Bartosz Golaszewski
` (9 subsequent siblings)
13 siblings, 0 replies; 43+ messages in thread
From: Bartosz Golaszewski @ 2025-09-24 14:51 UTC (permalink / raw)
To: Kees Cook, Mika Westerberg, Dmitry Torokhov, Andrew Morton,
Linus Walleij, Manivannan Sadhasivam, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Saravana Kannan,
Greg Kroah-Hartman, Andy Shevchenko, Bartosz Golaszewski,
Catalin Marinas, Will Deacon, Srinivas Kandagatla, Liam Girdwood,
Mark Brown, Jaroslav Kysela, Takashi Iwai
Cc: linux-hardening, linux-kernel, linux-gpio, linux-arm-kernel,
linux-sound, linux-arm-msm, Bartosz Golaszewski
From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
This module scans the device tree (for now only OF nodes are supported
but care is taken to make other fwnode implementations easy to
integrate) and determines which GPIO lines are shared by multiple users.
It stores that information in memory. When the GPIO chip exposing shared
lines is registered, the shared GPIO descriptors it exposes are marked
as shared and virtual "proxy" devices that mediate access to the shared
lines are created. When a consumer of a shared GPIO looks it up, its
fwnode lookup is redirected to a just-in-time machine lookup that points
to this proxy device.
This code can be compiled out on platforms which don't use shared GPIOs.
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
---
drivers/gpio/Kconfig | 8 +
drivers/gpio/Makefile | 1 +
drivers/gpio/gpiolib-shared.c | 481 ++++++++++++++++++++++++++++++++++++++++++
drivers/gpio/gpiolib-shared.h | 71 +++++++
4 files changed, 561 insertions(+)
diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index 7ee3afbc2b05da4d7470e609a0311ecc38727b00..679a7385a9776eef96a86ca4f429ee83ac939362 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -6,6 +6,9 @@
config GPIOLIB_LEGACY
def_bool y
+config HAVE_SHARED_GPIOS
+ bool
+
menuconfig GPIOLIB
bool "GPIO Support"
help
@@ -42,6 +45,11 @@ config GPIOLIB_IRQCHIP
select IRQ_DOMAIN
bool
+config GPIO_SHARED
+ def_bool y
+ depends on HAVE_SHARED_GPIOS || COMPILE_TEST
+ select AUXILIARY_BUS
+
config OF_GPIO_MM_GPIOCHIP
bool
help
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index ec296fa14bfdb360c27b4578826354762f01f074..f702f7e27e5b4017e7eab3019dae4ec912d534f8 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -12,6 +12,7 @@ obj-$(CONFIG_GPIO_SYSFS) += gpiolib-sysfs.o
obj-$(CONFIG_GPIO_ACPI) += gpiolib-acpi.o
gpiolib-acpi-y := gpiolib-acpi-core.o gpiolib-acpi-quirks.o
obj-$(CONFIG_GPIOLIB) += gpiolib-swnode.o
+obj-$(CONFIG_GPIO_SHARED) += gpiolib-shared.o
# Device drivers. Generally keep list sorted alphabetically
obj-$(CONFIG_GPIO_REGMAP) += gpio-regmap.o
diff --git a/drivers/gpio/gpiolib-shared.c b/drivers/gpio/gpiolib-shared.c
new file mode 100644
index 0000000000000000000000000000000000000000..97c16a73bb207bdf1728adda87161c1522415a67
--- /dev/null
+++ b/drivers/gpio/gpiolib-shared.c
@@ -0,0 +1,481 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (C) 2025 Linaro Ltd.
+ */
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <linux/auxiliary_bus.h>
+#include <linux/cleanup.h>
+#include <linux/device.h>
+#include <linux/fwnode.h>
+#include <linux/gpio/consumer.h>
+#include <linux/gpio/machine.h>
+#include <linux/idr.h>
+#include <linux/kref.h>
+#include <linux/list.h>
+#include <linux/lockdep.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/of.h>
+#include <linux/overflow.h>
+#include <linux/printk.h>
+#include <linux/property.h>
+#include <linux/slab.h>
+#include <linux/string.h>
+
+#include "gpiolib.h"
+#include "gpiolib-shared.h"
+
+/* Represents a single reference to a GPIO pin. */
+struct gpio_shared_ref {
+ struct list_head list;
+ /* Firmware node associated with this GPIO's consumer. */
+ struct fwnode_handle *fwnode;
+ /* GPIO flags this consumer uses for the request. */
+ enum gpiod_flags flags;
+ char *con_id;
+ int dev_id;
+ struct auxiliary_device adev;
+ struct gpiod_lookup_table *lookup;
+};
+
+/* Represents a single GPIO pin. */
+struct gpio_shared_entry {
+ struct list_head list;
+ /* Firmware node associated with the GPIO controller. */
+ struct fwnode_handle *fwnode;
+ /* Hardware offset of the GPIO within its chip. */
+ unsigned int offset;
+ /* Index in the property value array. */
+ size_t index;
+ struct gpio_shared_desc *shared_desc;
+ struct kref ref;
+ struct list_head refs;
+};
+
+static LIST_HEAD(gpio_shared_list);
+static DEFINE_MUTEX(gpio_shared_lock);
+static DEFINE_IDA(gpio_shared_ida);
+
+static struct gpio_shared_entry *
+gpio_shared_find_entry(struct fwnode_handle *controller_node,
+ unsigned int offset)
+{
+ struct gpio_shared_entry *entry;
+
+ list_for_each_entry(entry, &gpio_shared_list, list) {
+ if (entry->fwnode == controller_node && entry->offset == offset)
+ return entry;
+ }
+
+ return NULL;
+}
+
+#if IS_ENABLED(CONFIG_OF)
+static int gpio_shared_of_traverse(struct device_node *curr)
+{
+ static const char *const suffixes[] = { "-gpios", "-gpio", NULL };
+
+ struct gpio_shared_entry *entry;
+ size_t con_id_len, suffix_len;
+ struct fwnode_handle *fwnode;
+ struct gpio_shared_ref *ref;
+ struct of_phandle_args args;
+ const char *const *suffix;
+ struct property *prop;
+ unsigned int offset;
+ int ret, count, i;
+
+ for_each_property_of_node(curr, prop) {
+ /*
+ * The standard name for a GPIO property is "foo-gpios"
+ * or "foo-gpio". Some bindings also use "gpios" or "gpio".
+ * There are some legacy device-trees which have a different
+ * naming convention and for which we have rename quirks in
+ * place in gpiolib-of.c. I don't think any of them require
+ * support for shared GPIOs so for now let's just ignore
+ * them. We can always just export the quirk list and
+ * iterate over it here.
+ */
+ if (!strends(prop->name, "-gpios") &&
+ !strends(prop->name, "-gpio") &&
+ strcmp(prop->name, "gpios") != 0 &&
+ strcmp(prop->name, "gpio") != 0)
+ continue;
+
+ count = of_count_phandle_with_args(curr, prop->name,
+ "#gpio-cells");
+ if (count <= 0)
+ continue;
+
+ for (i = 0; i < count; i++) {
+ struct device_node *np __free(device_node) = NULL;
+
+ ret = of_parse_phandle_with_args(curr, prop->name,
+ "#gpio-cells", i,
+ &args);
+ if (ret)
+ continue;
+
+ np = args.np;
+
+ if (!of_property_present(np, "gpio-controller"))
+ continue;
+
+ /*
+ * We support 1, 2 and 3 cell GPIO bindings in the
+ * kernel currently. There's only one old MIPS dts that
+ * has a one-cell binding but there's no associated
+ * consumer so it may as well be an error. There don't
+ * seem to be any 3-cell users of non-exclusive GPIOs,
+ * so we can skip this as well. Let's occupy ourselves
+ * with the predominant 2-cell binding with the first
+ * cell indicating the hardware offset of the GPIO and
+ * the second defining the GPIO flags of the request.
+ */
+ if (args.args_count != 2)
+ continue;
+
+ fwnode = of_fwnode_handle(args.np);
+ offset = args.args[0];
+
+ entry = gpio_shared_find_entry(fwnode, offset);
+ if (!entry) {
+ entry = kzalloc(sizeof(*entry), GFP_KERNEL);
+ if (!entry)
+ return -ENOMEM;
+
+ entry->fwnode = fwnode_handle_get(fwnode);
+ entry->offset = offset;
+ entry->index = count;
+ INIT_LIST_HEAD(&entry->refs);
+
+ list_add_tail(&entry->list, &gpio_shared_list);
+ }
+
+ ref = kzalloc(sizeof(*ref), GFP_KERNEL);
+ if (!ref)
+ return -ENOMEM;
+
+ ref->fwnode = fwnode_handle_get(of_fwnode_handle(curr));
+ ref->flags = args.args[1];
+
+ for (suffix = suffixes; *suffix; suffix++) {
+ if (!strends(prop->name, *suffix))
+ continue;
+
+ ref->con_id = kstrdup(prop->name, GFP_KERNEL);
+ if (!ref->con_id)
+ return -ENOMEM;
+
+ ref->dev_id = ida_alloc(&gpio_shared_ida, GFP_KERNEL);
+ if (ref->dev_id < 0) {
+ kfree(ref->con_id);
+ return -ENOMEM;
+ }
+
+ con_id_len = strlen(ref->con_id);
+ suffix_len = strlen(*suffix);
+
+ ref->con_id[con_id_len - suffix_len] = '\0';
+ }
+
+ if (!list_empty(&entry->refs))
+ pr_debug("GPIO %u at %s is shared by multiple firmware nodes\n",
+ entry->offset, fwnode_get_name(entry->fwnode));
+
+ list_add_tail(&ref->list, &entry->refs);
+ }
+ }
+
+ for_each_child_of_node_scoped(curr, child) {
+ ret = gpio_shared_of_traverse(child);
+ if (ret)
+ return ret;
+ }
+
+ return 0;
+}
+
+static int gpio_shared_of_scan(void)
+{
+ return gpio_shared_of_traverse(of_root);
+}
+#else
+static int gpio_shared_of_scan(void)
+{
+ return 0;
+}
+#endif /* CONFIG_OF */
+
+static int gpio_shared_make_adev(struct gpio_device *gdev,
+ struct gpio_shared_ref *ref)
+{
+ struct auxiliary_device *adev = &ref->adev;
+ int ret;
+
+ lockdep_assert_held(&gpio_shared_lock);
+
+ memset(adev, 0, sizeof(*adev));
+
+ adev->id = ref->dev_id;
+ adev->name = "proxy";
+ adev->dev.parent = gdev->dev.parent;
+ /* No need to dev->release() anything. */
+
+ ret = auxiliary_device_init(adev);
+ if (ret)
+ return ret;
+
+ ret = auxiliary_device_add(adev);
+ if (ret) {
+ auxiliary_device_uninit(adev);
+ return ret;
+ }
+
+ pr_debug("Created an auxiliary GPIO proxy %s for GPIO device %s\n",
+ dev_name(&adev->dev), gpio_device_get_label(gdev));
+
+ return 0;
+}
+
+int gpio_shared_add_proxy_lookup(struct device *consumer, unsigned long lflags)
+{
+ const char *dev_id = dev_name(consumer);
+ struct gpio_shared_entry *entry;
+ struct gpio_shared_ref *ref;
+
+ struct gpiod_lookup_table *lookup __free(kfree) =
+ kzalloc(struct_size(lookup, table, 2), GFP_KERNEL);
+ if (!lookup)
+ return -ENOMEM;
+
+ guard(mutex)(&gpio_shared_lock);
+
+ list_for_each_entry(entry, &gpio_shared_list, list) {
+ list_for_each_entry(ref, &entry->refs, list) {
+ if (!device_match_fwnode(consumer, ref->fwnode))
+ continue;
+
+ /* We've already done that on a previous request. */
+ if (ref->lookup)
+ return 0;
+
+ char *key __free(kfree) =
+ kasprintf(GFP_KERNEL,
+ KBUILD_MODNAME ".proxy.%u",
+ ref->adev.id);
+ if (!key)
+ return -ENOMEM;
+
+ pr_debug("Adding machine lookup entry for a shared GPIO for consumer %s, with key '%s' and con_id '%s'\n",
+ dev_id, key, ref->con_id);
+
+ lookup->dev_id = dev_id;
+ lookup->table[0] = GPIO_LOOKUP(no_free_ptr(key), 0,
+ ref->con_id, lflags);
+
+ gpiod_add_lookup_table(no_free_ptr(lookup));
+
+ return 0;
+ }
+ }
+
+ /* We warn here because this can only happen if the programmer borked. */
+ WARN_ON(1);
+ return -ENOENT;
+}
+
+static void gpio_shared_remove_adev(struct auxiliary_device *adev)
+{
+ lockdep_assert_held(&gpio_shared_lock);
+
+ auxiliary_device_uninit(adev);
+ auxiliary_device_delete(adev);
+}
+
+int gpio_device_setup_shared(struct gpio_device *gdev)
+{
+ struct gpio_shared_entry *entry;
+ struct gpio_shared_ref *ref;
+ unsigned long *flags;
+ int ret;
+
+ guard(mutex)(&gpio_shared_lock);
+
+ list_for_each_entry(entry, &gpio_shared_list, list) {
+ if (!device_match_fwnode(&gdev->dev, entry->fwnode))
+ continue;
+
+ if (list_count_nodes(&entry->refs) <= 1)
+ continue;
+
+ flags = &gdev->descs[entry->offset].flags;
+
+ set_bit(GPIOD_FLAG_SHARED, flags);
+ /*
+ * Shared GPIOs are not requested via the normal path. Make
+ * them inaccessible to anyone even before we register the
+ * chip.
+ */
+ set_bit(GPIOD_FLAG_REQUESTED, flags);
+
+ pr_debug("GPIO %u owned by %s is shared by multiple consumers\n",
+ entry->offset, gpio_device_get_label(gdev));
+
+ list_for_each_entry(ref, &entry->refs, list) {
+ pr_debug("Setting up a shared GPIO entry for %s\n",
+ fwnode_get_name(ref->fwnode));
+
+ ret = gpio_shared_make_adev(gdev, ref);
+ if (ret)
+ return ret;
+ }
+ }
+
+ return 0;
+}
+
+void gpio_device_teardown_shared(struct gpio_device *gdev)
+{
+ struct gpio_shared_entry *entry;
+ struct gpio_shared_ref *ref;
+
+ guard(mutex)(&gpio_shared_lock);
+
+ list_for_each_entry(entry, &gpio_shared_list, list) {
+ if (!device_match_fwnode(&gdev->dev, entry->fwnode))
+ continue;
+
+ list_for_each_entry(ref, &entry->refs, list) {
+ gpiod_remove_lookup_table(ref->lookup);
+ kfree(ref->lookup->table[0].key);
+ kfree(ref->lookup);
+ ref->lookup = NULL;
+ gpio_shared_remove_adev(&ref->adev);
+ }
+ }
+}
+
+static void gpio_shared_release(struct kref *kref)
+{
+ struct gpio_shared_entry *entry =
+ container_of(kref, struct gpio_shared_entry, ref);
+ struct gpio_shared_desc *shared_desc = entry->shared_desc;
+
+ guard(mutex)(&gpio_shared_lock);
+
+ gpio_device_put(shared_desc->desc->gdev);
+ if (shared_desc->can_sleep)
+ mutex_destroy(&shared_desc->mutex);
+ kfree(shared_desc);
+ entry->shared_desc = NULL;
+}
+
+static void gpiod_shared_put(void *data)
+{
+ struct gpio_shared_entry *entry = data;
+
+ lockdep_assert_not_held(&gpio_shared_lock);
+
+ kref_put(&entry->ref, gpio_shared_release);
+}
+
+struct gpio_shared_desc *devm_gpiod_shared_get(struct device *dev)
+{
+ struct auxiliary_device *adev = to_auxiliary_dev(dev);
+ struct gpio_shared_desc *shared_desc;
+ struct gpio_shared_entry *entry;
+ struct gpio_shared_ref *ref;
+ struct gpio_device *gdev;
+ int ret;
+
+ scoped_guard(mutex, &gpio_shared_lock) {
+ list_for_each_entry(entry, &gpio_shared_list, list) {
+ list_for_each_entry(ref, &entry->refs, list) {
+ if (adev != &ref->adev)
+ continue;
+
+ if (entry->shared_desc) {
+ kref_get(&entry->ref);
+ shared_desc = entry->shared_desc;
+ break;
+ }
+
+ shared_desc = kzalloc(sizeof(*shared_desc), GFP_KERNEL);
+ if (!shared_desc)
+ return ERR_PTR(-ENOMEM);
+
+ gdev = gpio_device_find_by_fwnode(entry->fwnode);
+ if (!gdev) {
+ kfree(shared_desc);
+ return ERR_PTR(-EPROBE_DEFER);
+ }
+
+ shared_desc->desc = &gdev->descs[entry->offset];
+ shared_desc->can_sleep = gpiod_cansleep(shared_desc->desc);
+ if (shared_desc->can_sleep)
+ mutex_init(&shared_desc->mutex);
+ else
+ spin_lock_init(&shared_desc->spinlock);
+
+ kref_init(&entry->ref);
+ entry->shared_desc = shared_desc;
+
+ pr_debug("Device %s acquired a reference to the shared GPIO %u owned by %s\n",
+ dev_name(dev), desc_to_gpio(shared_desc->desc),
+ gpio_device_get_label(gdev));
+ break;
+ }
+ }
+ }
+
+ ret = devm_add_action_or_reset(dev, gpiod_shared_put, entry);
+ if (ret)
+ return ERR_PTR(ret);
+
+ return shared_desc;
+}
+EXPORT_SYMBOL_GPL(devm_gpiod_shared_get);
+
+/*
+ * This is only called if gpio_shared_init() fails so it's in fact __init and
+ * not __exit.
+ */
+static void __init gpio_shared_teardown(void)
+{
+ struct gpio_shared_entry *entry, *epos;
+ struct gpio_shared_ref *ref, *rpos;
+
+ list_for_each_entry_safe(entry, epos, &gpio_shared_list, list) {
+ list_for_each_entry_safe(ref, rpos, &entry->refs, list) {
+ list_del(&ref->list);
+ kfree(ref->con_id);
+ ida_free(&gpio_shared_ida, ref->dev_id);
+ fwnode_handle_put(ref->fwnode);
+ kfree(ref);
+ }
+
+ list_del(&entry->list);
+ fwnode_handle_put(entry->fwnode);
+ kfree(entry);
+ }
+}
+
+static int __init gpio_shared_init(void)
+{
+ int ret;
+
+ /* Right now, we only support OF-based systems. */
+ ret = gpio_shared_of_scan();
+ if (ret) {
+ gpio_shared_teardown();
+ pr_err("Failed to scan OF nodes for shared GPIOs: %d\n", ret);
+ return ret;
+ }
+
+ pr_debug("Finished scanning firmware nodes for shared GPIOs\n");
+ return 0;
+}
+postcore_initcall(gpio_shared_init);
diff --git a/drivers/gpio/gpiolib-shared.h b/drivers/gpio/gpiolib-shared.h
new file mode 100644
index 0000000000000000000000000000000000000000..667dbdff3585066b7cbe2ebe476725fe7d683d84
--- /dev/null
+++ b/drivers/gpio/gpiolib-shared.h
@@ -0,0 +1,71 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+#ifndef __LINUX_GPIO_SHARED_H
+#define __LINUX_GPIO_SHARED_H
+
+#include <linux/cleanup.h>
+#include <linux/lockdep.h>
+#include <linux/mutex.h>
+#include <linux/spinlock.h>
+
+struct gpio_device;
+struct gpio_desc;
+struct device;
+
+#if IS_ENABLED(CONFIG_GPIO_SHARED)
+
+int gpio_device_setup_shared(struct gpio_device *gdev);
+void gpio_device_teardown_shared(struct gpio_device *gdev);
+int gpio_shared_add_proxy_lookup(struct device *consumer, unsigned long lflags);
+
+#else
+
+static inline int gpio_device_setup_shared(struct gpio_device *gdev)
+{
+ return 0;
+}
+
+static inline void gpio_device_teardown_shared(struct gpio_device *gdev) { }
+
+static inline int gpio_shared_add_proxy_lookup(struct device *consumer,
+ unsigned long lflags)
+{
+ return 0;
+}
+
+#endif /* CONFIG_GPIO_SHARED */
+
+struct gpio_shared_desc {
+ struct gpio_desc *desc;
+ bool can_sleep;
+ unsigned long cfg;
+ unsigned int usecnt;
+ unsigned int highcnt;
+ union {
+ struct mutex mutex;
+ spinlock_t spinlock;
+ };
+};
+
+struct gpio_shared_desc *devm_gpiod_shared_get(struct device *dev);
+
+DEFINE_LOCK_GUARD_1(gpio_shared_desc_lock, struct gpio_shared_desc,
+ if (_T->lock->can_sleep)
+ mutex_lock(&_T->lock->mutex);
+ else
+ spin_lock_irqsave(&_T->lock->spinlock, _T->flags),
+ if (_T->lock->can_sleep)
+ mutex_unlock(&_T->lock->mutex);
+ else
+ spin_unlock_irqrestore(&_T->lock->spinlock, _T->flags),
+ unsigned long flags)
+
+static inline void gpio_shared_lockdep_assert(struct gpio_shared_desc *shared_desc)
+{
+ if (shared_desc->can_sleep)
+ lockdep_assert_held(&shared_desc->mutex);
+ else
+ lockdep_assert_held(&shared_desc->spinlock);
+}
+
+#endif /* __LINUX_GPIO_SHARED_H */
--
2.48.1
^ permalink raw reply related [flat|nested] 43+ messages in thread* [PATCH RFC 5/9] gpio: shared-proxy: implement the shared GPIO proxy driver
2025-09-24 14:51 [PATCH RFC 0/9] gpio: improve support for shared GPIOs Bartosz Golaszewski
` (3 preceding siblings ...)
2025-09-24 14:51 ` [PATCH RFC 4/9] gpiolib: implement low-level, shared GPIO support Bartosz Golaszewski
@ 2025-09-24 14:51 ` Bartosz Golaszewski
2025-09-24 14:51 ` [PATCH RFC 6/9] gpiolib: support shared GPIOs in core subsystem code Bartosz Golaszewski
` (8 subsequent siblings)
13 siblings, 0 replies; 43+ messages in thread
From: Bartosz Golaszewski @ 2025-09-24 14:51 UTC (permalink / raw)
To: Kees Cook, Mika Westerberg, Dmitry Torokhov, Andrew Morton,
Linus Walleij, Manivannan Sadhasivam, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Saravana Kannan,
Greg Kroah-Hartman, Andy Shevchenko, Bartosz Golaszewski,
Catalin Marinas, Will Deacon, Srinivas Kandagatla, Liam Girdwood,
Mark Brown, Jaroslav Kysela, Takashi Iwai
Cc: linux-hardening, linux-kernel, linux-gpio, linux-arm-kernel,
linux-sound, linux-arm-msm, Bartosz Golaszewski
From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Add a virtual GPIO proxy driver which arbitrates access to a single
shared GPIO by multiple users. It works together with the core shared
GPIO support from GPIOLIB and functions by acquiring a reference to a
shared GPIO descriptor exposed by gpiolib-shared and making sure that
the state of the GPIO stays consistent.
In general: if there's only one user at the moment: allow it to do
anything as if this was a normal GPIO (in essence: just propagate calls
to the underlying real hardware driver). If there are more users: don't
allow to change the direction set by the initial user, allow to change
configuration options but warn about possible conflicts and finally:
treat the output-high value as a reference counted, logical "GPIO
enabled" setting, meaning: the GPIO value is set to high when the first
user requests it to be high and back to low once the last user stops
"voting" for high.
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
---
drivers/gpio/Kconfig | 9 ++
drivers/gpio/Makefile | 1 +
drivers/gpio/gpio-shared-proxy.c | 328 +++++++++++++++++++++++++++++++++++++++
3 files changed, 338 insertions(+)
diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index 679a7385a9776eef96a86ca4f429ee83ac939362..de8e4febf344fc19a633cd7efe8412fe12166fb8 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -2025,6 +2025,15 @@ config GPIO_SIM
This enables the GPIO simulator - a configfs-based GPIO testing
driver.
+config GPIO_SHARED_PROXY
+ tristate "Proxy driver for non-exclusive GPIOs"
+ default m
+ depends on GPIO_SHARED || COMPILE_TEST
+ select AUXILIARY_BUS
+ help
+ This enables the GPIO shared proxy driver - an abstraction layer
+ for GPIO pins that are shared by multiple devices.
+
endmenu
menu "GPIO Debugging utilities"
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index f702f7e27e5b4017e7eab3019dae4ec912d534f8..d0020bc70b84f6fb9949165070c21725a60f47e2 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -160,6 +160,7 @@ obj-$(CONFIG_ARCH_SA1100) += gpio-sa1100.o
obj-$(CONFIG_GPIO_SAMA5D2_PIOBU) += gpio-sama5d2-piobu.o
obj-$(CONFIG_GPIO_SCH311X) += gpio-sch311x.o
obj-$(CONFIG_GPIO_SCH) += gpio-sch.o
+obj-$(CONFIG_GPIO_SHARED_PROXY) += gpio-shared-proxy.o
obj-$(CONFIG_GPIO_SIFIVE) += gpio-sifive.o
obj-$(CONFIG_GPIO_SIM) += gpio-sim.o
obj-$(CONFIG_GPIO_SIOX) += gpio-siox.o
diff --git a/drivers/gpio/gpio-shared-proxy.c b/drivers/gpio/gpio-shared-proxy.c
new file mode 100644
index 0000000000000000000000000000000000000000..804038d33a40e45c8577f278faea79145a91a5bd
--- /dev/null
+++ b/drivers/gpio/gpio-shared-proxy.c
@@ -0,0 +1,328 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (C) 2025 Linaro Ltd.
+ */
+
+#include <linux/auxiliary_bus.h>
+#include <linux/cleanup.h>
+#include <linux/device.h>
+#include <linux/gpio/consumer.h>
+#include <linux/gpio/driver.h>
+#include <linux/module.h>
+
+#include "gpiolib-shared.h"
+
+struct gpio_shared_proxy_data {
+ struct gpio_chip gc;
+ struct gpio_shared_desc *shared_desc;
+ struct device *dev;
+ bool voted_high;
+};
+
+static int
+gpio_shared_proxy_set_unlocked(struct gpio_shared_proxy_data *proxy,
+ int (*set_func)(struct gpio_desc *desc, int value),
+ int value)
+{
+ struct gpio_shared_desc *shared_desc = proxy->shared_desc;
+ struct gpio_desc *desc = shared_desc->desc;
+ int ret = 0;
+
+ gpio_shared_lockdep_assert(shared_desc);
+
+ if (value) {
+ /* User wants to set value to high. */
+ if (proxy->voted_high)
+ /* Already voted for high, nothing to do. */
+ goto out;
+
+ /* Haven't voted for high yet. */
+ if (!shared_desc->highcnt) {
+ /*
+ * Current value is low, need to actually set value
+ * to high.
+ */
+ ret = set_func(desc, 1);
+ if (ret)
+ goto out;
+ }
+
+ shared_desc->highcnt++;
+ proxy->voted_high = true;
+
+ goto out;
+ }
+
+ /* Desired value is low. */
+ if (!proxy->voted_high)
+ /* We didn't vote for high, nothing to do. */
+ goto out;
+
+ /* We previously voted for high. */
+ if (shared_desc->highcnt == 1) {
+ /* This is the last remaining vote for high, set value to low. */
+ ret = set_func(desc, 0);
+ if (ret)
+ goto out;
+ }
+
+ shared_desc->highcnt--;
+ proxy->voted_high = false;
+
+out:
+ if (shared_desc->highcnt)
+ dev_dbg(proxy->dev,
+ "Voted for value '%s', effective value is 'high', number of votes for 'high': %u\n",
+ value ? "high" : "low", shared_desc->highcnt);
+ else
+ dev_dbg(proxy->dev, "Voted for value 'low', effective value is 'low'\n");
+
+ return ret;
+}
+
+static int gpio_shared_proxy_request(struct gpio_chip *gc, unsigned int offset)
+{
+ struct gpio_shared_proxy_data *proxy = gpiochip_get_data(gc);
+ struct gpio_shared_desc *shared_desc = proxy->shared_desc;
+
+ guard(gpio_shared_desc_lock)(shared_desc);
+
+ proxy->shared_desc->usecnt++;
+
+ dev_dbg(proxy->dev, "Shared GPIO requested, number of users: %u\n",
+ proxy->shared_desc->usecnt);
+
+ return 0;
+}
+
+static void gpio_shared_proxy_free(struct gpio_chip *gc, unsigned int offset)
+{
+ struct gpio_shared_proxy_data *proxy = gpiochip_get_data(gc);
+ struct gpio_shared_desc *shared_desc = proxy->shared_desc;
+
+ guard(gpio_shared_desc_lock)(shared_desc);
+
+ proxy->shared_desc->usecnt--;
+
+ dev_dbg(proxy->dev, "Shared GPIO freed, number of users: %u\n",
+ proxy->shared_desc->usecnt);
+}
+
+static int gpio_shared_proxy_set_config(struct gpio_chip *gc,
+ unsigned int offset, unsigned long cfg)
+{
+ struct gpio_shared_proxy_data *proxy = gpiochip_get_data(gc);
+ struct gpio_shared_desc *shared_desc = proxy->shared_desc;
+ struct gpio_desc *desc = shared_desc->desc;
+ int ret;
+
+ guard(gpio_shared_desc_lock)(shared_desc);
+
+ if (shared_desc->usecnt > 1) {
+ if (shared_desc->cfg != cfg) {
+ dev_dbg(proxy->dev,
+ "Shared GPIO's configuration already set, accepting changes but users may conflict!!\n");
+ } else {
+ dev_dbg(proxy->dev, "Equal config requested, nothing to do\n");
+ return 0;
+ }
+ }
+
+ ret = gpiod_set_config(desc, cfg);
+ if (ret && ret != -ENOTSUPP)
+ return ret;
+
+ shared_desc->cfg = cfg;
+ return 0;
+}
+
+static int gpio_shared_proxy_direction_input(struct gpio_chip *gc,
+ unsigned int offset)
+{
+ struct gpio_shared_proxy_data *proxy = gpiochip_get_data(gc);
+ struct gpio_shared_desc *shared_desc = proxy->shared_desc;
+ struct gpio_desc *desc = shared_desc->desc;
+ int dir;
+
+ guard(gpio_shared_desc_lock)(shared_desc);
+
+ if (shared_desc->usecnt == 1) {
+ dev_dbg(proxy->dev,
+ "Only one user of this shared GPIO, allowing to set direction to input\n");
+
+ return gpiod_direction_input(desc);
+ }
+
+ dir = gpiod_get_direction(desc);
+ if (dir < 0)
+ return dir;
+
+ if (dir == GPIO_LINE_DIRECTION_OUT) {
+ dev_dbg(proxy->dev,
+ "Shared GPIO's direction already set to output, refusing to change\n");
+ return -EPERM;
+ }
+
+ return 0;
+}
+
+static int gpio_shared_proxy_direction_output(struct gpio_chip *gc,
+ unsigned int offset, int value)
+{
+ struct gpio_shared_proxy_data *proxy = gpiochip_get_data(gc);
+ struct gpio_shared_desc *shared_desc = proxy->shared_desc;
+ struct gpio_desc *desc = shared_desc->desc;
+ int ret, dir;
+
+ guard(gpio_shared_desc_lock)(shared_desc);
+
+ if (shared_desc->usecnt == 1) {
+ dev_dbg(proxy->dev,
+ "Only one user of this shared GPIO, allowing to set direction to output\n");
+
+ ret = gpiod_direction_output(desc, value);
+ if (ret)
+ return ret;
+
+ if (value) {
+ proxy->voted_high = true;
+ shared_desc->highcnt = 1;
+ } else {
+ proxy->voted_high = false;
+ shared_desc->highcnt = 0;
+ }
+
+ return 0;
+ }
+
+ dir = gpiod_get_direction(desc);
+ if (dir < 0)
+ return dir;
+
+ if (dir == GPIO_LINE_DIRECTION_IN) {
+ dev_dbg(proxy->dev,
+ "Shared GPIO's direction already set to input, refusing to change\n");
+ return -EPERM;
+ }
+
+ return gpio_shared_proxy_set_unlocked(proxy, gpiod_direction_output, value);
+}
+
+static int gpio_shared_proxy_get(struct gpio_chip *gc, unsigned int offset)
+{
+ struct gpio_shared_proxy_data *proxy = gpiochip_get_data(gc);
+
+ return gpiod_get_value(proxy->shared_desc->desc);
+}
+
+static int gpio_shared_proxy_get_cansleep(struct gpio_chip *gc,
+ unsigned int offset)
+{
+ struct gpio_shared_proxy_data *proxy = gpiochip_get_data(gc);
+
+ return gpiod_get_value_cansleep(proxy->shared_desc->desc);
+}
+
+static int gpio_shared_proxy_do_set(struct gpio_shared_proxy_data *proxy,
+ int (*set_func)(struct gpio_desc *desc, int value),
+ int value)
+{
+ guard(gpio_shared_desc_lock)(proxy->shared_desc);
+
+ return gpio_shared_proxy_set_unlocked(proxy, set_func, value);
+}
+
+static int gpio_shared_proxy_set(struct gpio_chip *gc, unsigned int offset,
+ int value)
+{
+ struct gpio_shared_proxy_data *proxy = gpiochip_get_data(gc);
+
+ return gpio_shared_proxy_do_set(proxy, gpiod_set_value, value);
+}
+
+static int gpio_shared_proxy_set_cansleep(struct gpio_chip *gc,
+ unsigned int offset, int value)
+{
+ struct gpio_shared_proxy_data *proxy = gpiochip_get_data(gc);
+
+ return gpio_shared_proxy_do_set(proxy, gpiod_set_value_cansleep, value);
+}
+
+static int gpio_shared_proxy_get_direction(struct gpio_chip *gc,
+ unsigned int offset)
+{
+ struct gpio_shared_proxy_data *proxy = gpiochip_get_data(gc);
+
+ return gpiod_get_direction(proxy->shared_desc->desc);
+}
+
+static int gpio_shared_proxy_to_irq(struct gpio_chip *gc, unsigned int offset)
+{
+ struct gpio_shared_proxy_data *proxy = gpiochip_get_data(gc);
+
+ return gpiod_to_irq(proxy->shared_desc->desc);
+}
+
+static int gpio_shared_proxy_probe(struct auxiliary_device *adev,
+ const struct auxiliary_device_id *id)
+{
+ struct gpio_shared_proxy_data *proxy;
+ struct gpio_shared_desc *shared_desc;
+ struct device *dev = &adev->dev;
+ struct gpio_chip *gc;
+
+ shared_desc = devm_gpiod_shared_get(dev);
+ if (IS_ERR(shared_desc))
+ return PTR_ERR(shared_desc);
+
+ proxy = devm_kzalloc(dev, sizeof(*proxy), GFP_KERNEL);
+ if (!proxy)
+ return -ENOMEM;
+
+ proxy->shared_desc = shared_desc;
+ proxy->dev = dev;
+
+ gc = &proxy->gc;
+ gc->base = -1;
+ gc->ngpio = 1;
+ gc->label = dev_name(dev);
+ gc->parent = dev;
+ gc->owner = THIS_MODULE;
+ gc->can_sleep = shared_desc->can_sleep;
+
+ gc->request = gpio_shared_proxy_request;
+ gc->free = gpio_shared_proxy_free;
+ gc->set_config = gpio_shared_proxy_set_config;
+ gc->direction_input = gpio_shared_proxy_direction_input;
+ gc->direction_output = gpio_shared_proxy_direction_output;
+ if (gc->can_sleep) {
+ gc->set = gpio_shared_proxy_set_cansleep;
+ gc->get = gpio_shared_proxy_get_cansleep;
+ } else {
+ gc->set = gpio_shared_proxy_set;
+ gc->get = gpio_shared_proxy_get;
+ }
+ gc->get_direction = gpio_shared_proxy_get_direction;
+ gc->to_irq = gpio_shared_proxy_to_irq;
+
+ return devm_gpiochip_add_data(dev, &proxy->gc, proxy);
+}
+
+static const struct auxiliary_device_id gpio_shared_proxy_id_table[] = {
+ { .name = "gpiolib_shared.proxy" },
+ {},
+};
+MODULE_DEVICE_TABLE(auxiliary, gpio_shared_proxy_id_table);
+
+static struct auxiliary_driver gpio_shared_proxy_driver = {
+ .driver = {
+ .name = "gpio-shared-proxy",
+ },
+ .probe = gpio_shared_proxy_probe,
+ .id_table = gpio_shared_proxy_id_table,
+};
+module_auxiliary_driver(gpio_shared_proxy_driver);
+
+MODULE_AUTHOR("Bartosz Golaszewski <bartosz.golaszewski@linaro.org>");
+MODULE_DESCRIPTION("Shared GPIO mux driver.");
+MODULE_LICENSE("GPL");
--
2.48.1
^ permalink raw reply related [flat|nested] 43+ messages in thread* [PATCH RFC 6/9] gpiolib: support shared GPIOs in core subsystem code
2025-09-24 14:51 [PATCH RFC 0/9] gpio: improve support for shared GPIOs Bartosz Golaszewski
` (4 preceding siblings ...)
2025-09-24 14:51 ` [PATCH RFC 5/9] gpio: shared-proxy: implement the shared GPIO proxy driver Bartosz Golaszewski
@ 2025-09-24 14:51 ` Bartosz Golaszewski
2025-09-24 14:51 ` [PATCH RFC 7/9] arm64: select HAVE_SHARED_GPIOS for ARCH_QCOM Bartosz Golaszewski
` (7 subsequent siblings)
13 siblings, 0 replies; 43+ messages in thread
From: Bartosz Golaszewski @ 2025-09-24 14:51 UTC (permalink / raw)
To: Kees Cook, Mika Westerberg, Dmitry Torokhov, Andrew Morton,
Linus Walleij, Manivannan Sadhasivam, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Saravana Kannan,
Greg Kroah-Hartman, Andy Shevchenko, Bartosz Golaszewski,
Catalin Marinas, Will Deacon, Srinivas Kandagatla, Liam Girdwood,
Mark Brown, Jaroslav Kysela, Takashi Iwai
Cc: linux-hardening, linux-kernel, linux-gpio, linux-arm-kernel,
linux-sound, linux-arm-msm, Bartosz Golaszewski
From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
As the final step in adding official support for shared GPIOs, enable
the previously added elements in core GPIO subsystem code. Set-up shared
GPIOs when adding a GPIO chip, tear it down on removal and check if a
GPIO descriptor looked up during the firmware-node stage is shared and
fall-back to machine lookup in this case.
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
---
drivers/gpio/gpiolib.c | 50 +++++++++++++++++++++++++++++++++++++++++---------
1 file changed, 41 insertions(+), 9 deletions(-)
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 9952e412da505c12ceddfefd9cfd778aad3bd5e6..1e4c991797126d0d91233bbe7c3ae3831bf77d6c 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -37,6 +37,7 @@
#include "gpiolib-acpi.h"
#include "gpiolib-cdev.h"
#include "gpiolib-of.h"
+#include "gpiolib-shared.h"
#include "gpiolib-swnode.h"
#include "gpiolib-sysfs.h"
#include "gpiolib.h"
@@ -1200,6 +1201,10 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data,
if (ret)
goto err_remove_irqchip_mask;
+ ret = gpio_device_setup_shared(gdev);
+ if (ret)
+ goto err_remove_irqchip;
+
/*
* By first adding the chardev, and then adding the device,
* we get a device node entry in sysfs under
@@ -1211,10 +1216,13 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data,
if (gpiolib_initialized) {
ret = gpiochip_setup_dev(gdev);
if (ret)
- goto err_remove_irqchip;
+ goto err_teardown_shared;
}
+
return 0;
+err_teardown_shared:
+ gpio_device_teardown_shared(gdev);
err_remove_irqchip:
gpiochip_irqchip_remove(gc);
err_remove_irqchip_mask:
@@ -1283,6 +1291,7 @@ void gpiochip_remove(struct gpio_chip *gc)
/* Numb the device, cancelling all outstanding operations */
rcu_assign_pointer(gdev->chip, NULL);
synchronize_srcu(&gdev->srcu);
+ gpio_device_teardown_shared(gdev);
gpiochip_irqchip_remove(gc);
acpi_gpiochip_remove(gc);
of_gpiochip_remove(gc);
@@ -4652,11 +4661,29 @@ struct gpio_desc *gpiod_find_and_request(struct device *consumer,
scoped_guard(srcu, &gpio_devices_srcu) {
desc = gpiod_fwnode_lookup(fwnode, consumer, con_id, idx,
&flags, &lookupflags);
+ if (!IS_ERR_OR_NULL(desc) &&
+ test_bit(GPIOD_FLAG_SHARED, &desc->flags)) {
+ /*
+ * We're dealing with a GPIO shared by multiple
+ * consumers. This is the moment to add the machine
+ * lookup table for the proxy device as previously
+ * we only knew the consumer's fwnode.
+ */
+ ret = gpio_shared_add_proxy_lookup(consumer, lookupflags);
+ if (ret)
+ return ERR_PTR(ret);
+
+ /* Trigger platform lookup for shared GPIO proxy. */
+ desc = ERR_PTR(-ENOENT);
+ /* Trigger it even for fwnode-only gpiod_get(). */
+ platform_lookup_allowed = true;
+ }
+
if (gpiod_not_found(desc) && platform_lookup_allowed) {
/*
* Either we are not using DT or ACPI, or their lookup
- * did not return a result. In that case, use platform
- * lookup as a fallback.
+ * did not return a result or this is a shared GPIO. In
+ * that case, use platform lookup as a fallback.
*/
dev_dbg(consumer,
"using lookup tables for GPIO lookup\n");
@@ -4679,14 +4706,19 @@ struct gpio_desc *gpiod_find_and_request(struct device *consumer,
return ERR_PTR(ret);
/*
- * This happens when there are several consumers for
- * the same GPIO line: we just return here without
- * further initialization. It is a bit of a hack.
- * This is necessary to support fixed regulators.
+ * This happens when there are several consumers for the same
+ * GPIO line: we just return here without further
+ * initialization. It's a hack introduced long ago to support
+ * fixed regulators. We now have a better solution with
+ * automated scanning where affected platforms just need to
+ * select the provided Kconfig option.
*
- * FIXME: Make this more sane and safe.
+ * FIXME: Remove the GPIOD_FLAGS_BIT_NONEXCLUSIVE flag after
+ * making sure all platforms use the new mechanism.
*/
- dev_info(consumer, "nonexclusive access to GPIO for %s\n", name);
+ dev_info(consumer,
+ "nonexclusive access to GPIO for %s, consider updating your code to using gpio-shared-proxy\n",
+ name);
return desc;
}
--
2.48.1
^ permalink raw reply related [flat|nested] 43+ messages in thread* [PATCH RFC 7/9] arm64: select HAVE_SHARED_GPIOS for ARCH_QCOM
2025-09-24 14:51 [PATCH RFC 0/9] gpio: improve support for shared GPIOs Bartosz Golaszewski
` (5 preceding siblings ...)
2025-09-24 14:51 ` [PATCH RFC 6/9] gpiolib: support shared GPIOs in core subsystem code Bartosz Golaszewski
@ 2025-09-24 14:51 ` Bartosz Golaszewski
2025-09-24 14:51 ` [PATCH RFC 8/9] ASoC: wsa881x: drop GPIOD_FLAGS_BIT_NONEXCLUSIVE flag from GPIO lookup Bartosz Golaszewski
` (6 subsequent siblings)
13 siblings, 0 replies; 43+ messages in thread
From: Bartosz Golaszewski @ 2025-09-24 14:51 UTC (permalink / raw)
To: Kees Cook, Mika Westerberg, Dmitry Torokhov, Andrew Morton,
Linus Walleij, Manivannan Sadhasivam, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Saravana Kannan,
Greg Kroah-Hartman, Andy Shevchenko, Bartosz Golaszewski,
Catalin Marinas, Will Deacon, Srinivas Kandagatla, Liam Girdwood,
Mark Brown, Jaroslav Kysela, Takashi Iwai
Cc: linux-hardening, linux-kernel, linux-gpio, linux-arm-kernel,
linux-sound, linux-arm-msm, Bartosz Golaszewski
From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Some qualcomm platforms use shared GPIOs. Enable support for them by
selecting the Kconfig switch provided by GPIOLIB.
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
---
arch/arm64/Kconfig.platforms | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/arm64/Kconfig.platforms b/arch/arm64/Kconfig.platforms
index 13173795c43d4f28e2d47acc700f80a165d44671..3dbff0261f0add0516d8cb3fd0f29e277af94f20 100644
--- a/arch/arm64/Kconfig.platforms
+++ b/arch/arm64/Kconfig.platforms
@@ -316,6 +316,7 @@ config ARCH_QCOM
select GPIOLIB
select PINCTRL
select HAVE_PWRCTRL if PCI
+ select HAVE_SHARED_GPIOS
help
This enables support for the ARMv8 based Qualcomm chipsets.
--
2.48.1
^ permalink raw reply related [flat|nested] 43+ messages in thread* [PATCH RFC 8/9] ASoC: wsa881x: drop GPIOD_FLAGS_BIT_NONEXCLUSIVE flag from GPIO lookup
2025-09-24 14:51 [PATCH RFC 0/9] gpio: improve support for shared GPIOs Bartosz Golaszewski
` (6 preceding siblings ...)
2025-09-24 14:51 ` [PATCH RFC 7/9] arm64: select HAVE_SHARED_GPIOS for ARCH_QCOM Bartosz Golaszewski
@ 2025-09-24 14:51 ` Bartosz Golaszewski
2025-09-24 14:51 ` [PATCH RFC 9/9] ASoC: wsa883x: " Bartosz Golaszewski
` (5 subsequent siblings)
13 siblings, 0 replies; 43+ messages in thread
From: Bartosz Golaszewski @ 2025-09-24 14:51 UTC (permalink / raw)
To: Kees Cook, Mika Westerberg, Dmitry Torokhov, Andrew Morton,
Linus Walleij, Manivannan Sadhasivam, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Saravana Kannan,
Greg Kroah-Hartman, Andy Shevchenko, Bartosz Golaszewski,
Catalin Marinas, Will Deacon, Srinivas Kandagatla, Liam Girdwood,
Mark Brown, Jaroslav Kysela, Takashi Iwai
Cc: linux-hardening, linux-kernel, linux-gpio, linux-arm-kernel,
linux-sound, linux-arm-msm, Bartosz Golaszewski
From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
This driver is only used on Qualcomm platforms which now select
HAVE_SHARED_GPIOS so this flag can be dropped.
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
---
sound/soc/codecs/wsa881x.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/sound/soc/codecs/wsa881x.c b/sound/soc/codecs/wsa881x.c
index 636e59abc3772fc0b333873a329b65f4213c3ef3..92a1e3bb8371e178571a6c1ed6f1185fe6c2e757 100644
--- a/sound/soc/codecs/wsa881x.c
+++ b/sound/soc/codecs/wsa881x.c
@@ -1112,8 +1112,7 @@ static int wsa881x_probe(struct sdw_slave *pdev,
if (!wsa881x)
return -ENOMEM;
- wsa881x->sd_n = devm_gpiod_get_optional(dev, "powerdown",
- GPIOD_FLAGS_BIT_NONEXCLUSIVE);
+ wsa881x->sd_n = devm_gpiod_get_optional(dev, "powerdown", 0);
if (IS_ERR(wsa881x->sd_n))
return dev_err_probe(dev, PTR_ERR(wsa881x->sd_n),
"Shutdown Control GPIO not found\n");
--
2.48.1
^ permalink raw reply related [flat|nested] 43+ messages in thread* [PATCH RFC 9/9] ASoC: wsa883x: drop GPIOD_FLAGS_BIT_NONEXCLUSIVE flag from GPIO lookup
2025-09-24 14:51 [PATCH RFC 0/9] gpio: improve support for shared GPIOs Bartosz Golaszewski
` (7 preceding siblings ...)
2025-09-24 14:51 ` [PATCH RFC 8/9] ASoC: wsa881x: drop GPIOD_FLAGS_BIT_NONEXCLUSIVE flag from GPIO lookup Bartosz Golaszewski
@ 2025-09-24 14:51 ` Bartosz Golaszewski
2025-09-24 18:25 ` [PATCH RFC 0/9] gpio: improve support for shared GPIOs Dmitry Torokhov
` (4 subsequent siblings)
13 siblings, 0 replies; 43+ messages in thread
From: Bartosz Golaszewski @ 2025-09-24 14:51 UTC (permalink / raw)
To: Kees Cook, Mika Westerberg, Dmitry Torokhov, Andrew Morton,
Linus Walleij, Manivannan Sadhasivam, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Saravana Kannan,
Greg Kroah-Hartman, Andy Shevchenko, Bartosz Golaszewski,
Catalin Marinas, Will Deacon, Srinivas Kandagatla, Liam Girdwood,
Mark Brown, Jaroslav Kysela, Takashi Iwai
Cc: linux-hardening, linux-kernel, linux-gpio, linux-arm-kernel,
linux-sound, linux-arm-msm, Bartosz Golaszewski
From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
This driver is only used on Qualcomm platforms which now select
HAVE_SHARED_GPIOS so this flag can be dropped.
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
---
sound/soc/codecs/wsa883x.c | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/sound/soc/codecs/wsa883x.c b/sound/soc/codecs/wsa883x.c
index ca4520ade79aa2c9377449e29cdd33e9e8dd28c5..ba60b4de6cab7740cd8aa70c89f6e03e1dc2dd12 100644
--- a/sound/soc/codecs/wsa883x.c
+++ b/sound/soc/codecs/wsa883x.c
@@ -1572,13 +1572,10 @@ static int wsa883x_get_reset(struct device *dev, struct wsa883x_priv *wsa883x)
if (IS_ERR(wsa883x->sd_reset))
return dev_err_probe(dev, PTR_ERR(wsa883x->sd_reset),
"Failed to get reset\n");
- /*
- * if sd_reset: NULL, so use the backwards compatible way for powerdown-gpios,
- * which does not handle sharing GPIO properly.
- */
+
+ /* if sd_reset: NULL, so use the backwards compatible way for powerdown-gpios */
if (!wsa883x->sd_reset) {
wsa883x->sd_n = devm_gpiod_get_optional(dev, "powerdown",
- GPIOD_FLAGS_BIT_NONEXCLUSIVE |
GPIOD_OUT_HIGH);
if (IS_ERR(wsa883x->sd_n))
return dev_err_probe(dev, PTR_ERR(wsa883x->sd_n),
--
2.48.1
^ permalink raw reply related [flat|nested] 43+ messages in thread* Re: [PATCH RFC 0/9] gpio: improve support for shared GPIOs
2025-09-24 14:51 [PATCH RFC 0/9] gpio: improve support for shared GPIOs Bartosz Golaszewski
` (8 preceding siblings ...)
2025-09-24 14:51 ` [PATCH RFC 9/9] ASoC: wsa883x: " Bartosz Golaszewski
@ 2025-09-24 18:25 ` Dmitry Torokhov
2025-09-24 18:58 ` Bartosz Golaszewski
2025-10-06 15:43 ` Manivannan Sadhasivam
2025-10-01 8:49 ` Linus Walleij
` (3 subsequent siblings)
13 siblings, 2 replies; 43+ messages in thread
From: Dmitry Torokhov @ 2025-09-24 18:25 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: Kees Cook, Mika Westerberg, Andrew Morton, Linus Walleij,
Manivannan Sadhasivam, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Saravana Kannan, Greg Kroah-Hartman,
Andy Shevchenko, Catalin Marinas, Will Deacon,
Srinivas Kandagatla, Liam Girdwood, Mark Brown, Jaroslav Kysela,
Takashi Iwai, linux-hardening, linux-kernel, linux-gpio,
linux-arm-kernel, linux-sound, linux-arm-msm, Bartosz Golaszewski
Hi Bartosz,
On Wed, Sep 24, 2025 at 04:51:28PM +0200, Bartosz Golaszewski wrote:
> Here's a functional RFC for improving the handling of shared GPIOs in
> linux.
>
> Problem statement: GPIOs are implemented as a strictly exclusive
> resource in the kernel but there are lots of platforms on which single
> pin is shared by multiple devices which don't communicate so need some
> way of properly sharing access to a GPIO. What we have now is the
> GPIOD_FLAGS_BIT_NONEXCLUSIVE flag which was introduced as a hack and
> doesn't do any locking or arbitration of access - it literally just hand
> the same GPIO descriptor to all interested users.
>
> The proposed solution is composed of three major parts: the high-level,
> shared GPIO proxy driver that arbitrates access to the shared pin and
> exposes a regular GPIO chip interface to consumers, a low-level shared
> GPIOLIB module that scans firmware nodes and creates auxiliary devices
> that attach to the proxy driver and finally a set of core GPIOLIB
> changes that plug the former into the GPIO lookup path.
>
> The changes are implemented in a way that allows to seamlessly compile
> out any code related to sharing GPIOs for systems that don't need it.
>
> The practical use-case for this are the powerdown GPIOs shared by
> speakers on Qualcomm db845c platform, however I have also extensively
> tested it using gpio-virtuser on arm64 qemu with various DT
> configurations.
How is this different from the existing gpio-backed regulator/supply?
IMO GPIOs are naturally exclusive-use resources (in cases when you need
to control them, not simply read their state), and when there is a need
to share them there are more appropriate abstractions that are built on
top of GPIOs...
Thanks.
--
Dmitry
^ permalink raw reply [flat|nested] 43+ messages in thread* Re: [PATCH RFC 0/9] gpio: improve support for shared GPIOs
2025-09-24 18:25 ` [PATCH RFC 0/9] gpio: improve support for shared GPIOs Dmitry Torokhov
@ 2025-09-24 18:58 ` Bartosz Golaszewski
2025-10-06 15:43 ` Manivannan Sadhasivam
1 sibling, 0 replies; 43+ messages in thread
From: Bartosz Golaszewski @ 2025-09-24 18:58 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Kees Cook, Mika Westerberg, Andrew Morton, Linus Walleij,
Manivannan Sadhasivam, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Saravana Kannan, Greg Kroah-Hartman,
Andy Shevchenko, Catalin Marinas, Will Deacon,
Srinivas Kandagatla, Liam Girdwood, Mark Brown, Jaroslav Kysela,
Takashi Iwai, linux-hardening, linux-kernel, linux-gpio,
linux-arm-kernel, linux-sound, linux-arm-msm, Bartosz Golaszewski
On Wed, Sep 24, 2025 at 8:25 PM Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
>
> Hi Bartosz,
>
> On Wed, Sep 24, 2025 at 04:51:28PM +0200, Bartosz Golaszewski wrote:
> > Here's a functional RFC for improving the handling of shared GPIOs in
> > linux.
> >
[snip]
> >
> > The practical use-case for this are the powerdown GPIOs shared by
> > speakers on Qualcomm db845c platform, however I have also extensively
> > tested it using gpio-virtuser on arm64 qemu with various DT
> > configurations.
>
> How is this different from the existing gpio-backed regulator/supply?
> IMO GPIOs are naturally exclusive-use resources (in cases when you need
> to control them, not simply read their state), and when there is a need
> to share them there are more appropriate abstractions that are built on
> top of GPIOs...
>
I think you have never been on the receiving end of Krzysztof's wrath
when trying to model a simple shared pin as a nonexistent reset
provider or a fixed regulator in device-tree. :)
Unless you mean some other abstractions I am missing.
Bartosz
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH RFC 0/9] gpio: improve support for shared GPIOs
2025-09-24 18:25 ` [PATCH RFC 0/9] gpio: improve support for shared GPIOs Dmitry Torokhov
2025-09-24 18:58 ` Bartosz Golaszewski
@ 2025-10-06 15:43 ` Manivannan Sadhasivam
2025-10-06 16:10 ` Bartosz Golaszewski
1 sibling, 1 reply; 43+ messages in thread
From: Manivannan Sadhasivam @ 2025-10-06 15:43 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Bartosz Golaszewski, Kees Cook, Mika Westerberg, Andrew Morton,
Linus Walleij, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Saravana Kannan, Greg Kroah-Hartman, Andy Shevchenko,
Catalin Marinas, Will Deacon, Srinivas Kandagatla, Liam Girdwood,
Mark Brown, Jaroslav Kysela, Takashi Iwai, linux-hardening,
linux-kernel, linux-gpio, linux-arm-kernel, linux-sound,
linux-arm-msm, Bartosz Golaszewski
On Wed, Sep 24, 2025 at 11:25:12AM -0700, Dmitry Torokhov wrote:
> Hi Bartosz,
>
> On Wed, Sep 24, 2025 at 04:51:28PM +0200, Bartosz Golaszewski wrote:
> > Here's a functional RFC for improving the handling of shared GPIOs in
> > linux.
> >
> > Problem statement: GPIOs are implemented as a strictly exclusive
> > resource in the kernel but there are lots of platforms on which single
> > pin is shared by multiple devices which don't communicate so need some
> > way of properly sharing access to a GPIO. What we have now is the
> > GPIOD_FLAGS_BIT_NONEXCLUSIVE flag which was introduced as a hack and
> > doesn't do any locking or arbitration of access - it literally just hand
> > the same GPIO descriptor to all interested users.
> >
> > The proposed solution is composed of three major parts: the high-level,
> > shared GPIO proxy driver that arbitrates access to the shared pin and
> > exposes a regular GPIO chip interface to consumers, a low-level shared
> > GPIOLIB module that scans firmware nodes and creates auxiliary devices
> > that attach to the proxy driver and finally a set of core GPIOLIB
> > changes that plug the former into the GPIO lookup path.
> >
> > The changes are implemented in a way that allows to seamlessly compile
> > out any code related to sharing GPIOs for systems that don't need it.
> >
> > The practical use-case for this are the powerdown GPIOs shared by
> > speakers on Qualcomm db845c platform, however I have also extensively
> > tested it using gpio-virtuser on arm64 qemu with various DT
> > configurations.
>
> How is this different from the existing gpio-backed regulator/supply?
> IMO GPIOs are naturally exclusive-use resources (in cases when you need
> to control them, not simply read their state), and when there is a need
> to share them there are more appropriate abstractions that are built on
> top of GPIOs...
>
Not always... For something like shared reset line, consumers request the line
as GPIO and expect gpiolib to do resource manangement.
- Mani
--
மணிவண்ணன் சதாசிவம்
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH RFC 0/9] gpio: improve support for shared GPIOs
2025-10-06 15:43 ` Manivannan Sadhasivam
@ 2025-10-06 16:10 ` Bartosz Golaszewski
2025-10-06 21:52 ` Srinivas Kandagatla
2025-10-06 22:09 ` Manivannan Sadhasivam
0 siblings, 2 replies; 43+ messages in thread
From: Bartosz Golaszewski @ 2025-10-06 16:10 UTC (permalink / raw)
To: Manivannan Sadhasivam
Cc: Dmitry Torokhov, Kees Cook, Mika Westerberg, Andrew Morton,
Linus Walleij, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Saravana Kannan, Greg Kroah-Hartman, Andy Shevchenko,
Catalin Marinas, Will Deacon, Srinivas Kandagatla, Liam Girdwood,
Mark Brown, Jaroslav Kysela, Takashi Iwai, linux-hardening,
linux-kernel, linux-gpio, linux-arm-kernel, linux-sound,
linux-arm-msm, Bartosz Golaszewski
On Mon, Oct 6, 2025 at 5:43 PM Manivannan Sadhasivam <mani@kernel.org> wrote:
>
> On Wed, Sep 24, 2025 at 11:25:12AM -0700, Dmitry Torokhov wrote:
> > Hi Bartosz,
> >
> > >
> > > The practical use-case for this are the powerdown GPIOs shared by
> > > speakers on Qualcomm db845c platform, however I have also extensively
> > > tested it using gpio-virtuser on arm64 qemu with various DT
> > > configurations.
> >
> > How is this different from the existing gpio-backed regulator/supply?
> > IMO GPIOs are naturally exclusive-use resources (in cases when you need
> > to control them, not simply read their state), and when there is a need
> > to share them there are more appropriate abstractions that are built on
> > top of GPIOs...
> >
>
> Not always... For something like shared reset line, consumers request the line
> as GPIO and expect gpiolib to do resource manangement.
>
They could use the reset API and it would implicitly create a virtual
device that requests the reset GPIO and controls its enable count.
Except that some devices also do a specific reset sequence with delays
etc. That would require some additional logic in reset-gpio.
Bart
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH RFC 0/9] gpio: improve support for shared GPIOs
2025-10-06 16:10 ` Bartosz Golaszewski
@ 2025-10-06 21:52 ` Srinivas Kandagatla
2025-10-06 22:09 ` Manivannan Sadhasivam
1 sibling, 0 replies; 43+ messages in thread
From: Srinivas Kandagatla @ 2025-10-06 21:52 UTC (permalink / raw)
To: Bartosz Golaszewski, Manivannan Sadhasivam
Cc: Dmitry Torokhov, Kees Cook, Mika Westerberg, Andrew Morton,
Linus Walleij, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Saravana Kannan, Greg Kroah-Hartman, Andy Shevchenko,
Catalin Marinas, Will Deacon, Srinivas Kandagatla, Liam Girdwood,
Mark Brown, Jaroslav Kysela, Takashi Iwai, linux-hardening,
linux-kernel, linux-gpio, linux-arm-kernel, linux-sound,
linux-arm-msm, Bartosz Golaszewski
On 10/6/25 5:10 PM, Bartosz Golaszewski wrote:
> On Mon, Oct 6, 2025 at 5:43 PM Manivannan Sadhasivam <mani@kernel.org> wrote:
>>
>> On Wed, Sep 24, 2025 at 11:25:12AM -0700, Dmitry Torokhov wrote:
>>> Hi Bartosz,
>>>
>>>>
>>>> The practical use-case for this are the powerdown GPIOs shared by
>>>> speakers on Qualcomm db845c platform, however I have also extensively
>>>> tested it using gpio-virtuser on arm64 qemu with various DT
>>>> configurations.
>>>
>>> How is this different from the existing gpio-backed regulator/supply?
>>> IMO GPIOs are naturally exclusive-use resources (in cases when you need
>>> to control them, not simply read their state), and when there is a need
>>> to share them there are more appropriate abstractions that are built on
>>> top of GPIOs...
>>>
>>
>> Not always... For something like shared reset line, consumers request the line
>> as GPIO and expect gpiolib to do resource manangement.
>>
>
> They could use the reset API and it would implicitly create a virtual
> device that requests the reset GPIO and controls its enable count.
> Except that some devices also do a specific reset sequence with delays
> etc. That would require some additional logic in reset-gpio.
That should be a platform specific reset controller driver.
>
> Bart
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH RFC 0/9] gpio: improve support for shared GPIOs
2025-10-06 16:10 ` Bartosz Golaszewski
2025-10-06 21:52 ` Srinivas Kandagatla
@ 2025-10-06 22:09 ` Manivannan Sadhasivam
2025-10-07 12:29 ` Bartosz Golaszewski
1 sibling, 1 reply; 43+ messages in thread
From: Manivannan Sadhasivam @ 2025-10-06 22:09 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: Dmitry Torokhov, Kees Cook, Mika Westerberg, Andrew Morton,
Linus Walleij, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Saravana Kannan, Greg Kroah-Hartman, Andy Shevchenko,
Catalin Marinas, Will Deacon, Srinivas Kandagatla, Liam Girdwood,
Mark Brown, Jaroslav Kysela, Takashi Iwai, linux-hardening,
linux-kernel, linux-gpio, linux-arm-kernel, linux-sound,
linux-arm-msm, Bartosz Golaszewski
On Mon, Oct 06, 2025 at 06:10:59PM +0200, Bartosz Golaszewski wrote:
> On Mon, Oct 6, 2025 at 5:43 PM Manivannan Sadhasivam <mani@kernel.org> wrote:
> >
> > On Wed, Sep 24, 2025 at 11:25:12AM -0700, Dmitry Torokhov wrote:
> > > Hi Bartosz,
> > >
> > > >
> > > > The practical use-case for this are the powerdown GPIOs shared by
> > > > speakers on Qualcomm db845c platform, however I have also extensively
> > > > tested it using gpio-virtuser on arm64 qemu with various DT
> > > > configurations.
> > >
> > > How is this different from the existing gpio-backed regulator/supply?
> > > IMO GPIOs are naturally exclusive-use resources (in cases when you need
> > > to control them, not simply read their state), and when there is a need
> > > to share them there are more appropriate abstractions that are built on
> > > top of GPIOs...
> > >
> >
> > Not always... For something like shared reset line, consumers request the line
> > as GPIO and expect gpiolib to do resource manangement.
> >
>
> They could use the reset API and it would implicitly create a virtual
> device that requests the reset GPIO and controls its enable count.
> Except that some devices also do a specific reset sequence with delays
> etc. That would require some additional logic in reset-gpio.
>
I was referring to the PCIe PERST# line, for which the 'reset-gpios' property
already exist in the schema. Now, you want me to model this simple GPIO as a
fake reset controller and use it the PCIe Bridge nodes through 'resets'
property?
- Mani
--
மணிவண்ணன் சதாசிவம்
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH RFC 0/9] gpio: improve support for shared GPIOs
2025-10-06 22:09 ` Manivannan Sadhasivam
@ 2025-10-07 12:29 ` Bartosz Golaszewski
2025-10-21 1:53 ` Manivannan Sadhasivam
0 siblings, 1 reply; 43+ messages in thread
From: Bartosz Golaszewski @ 2025-10-07 12:29 UTC (permalink / raw)
To: Manivannan Sadhasivam
Cc: Dmitry Torokhov, Kees Cook, Mika Westerberg, Andrew Morton,
Linus Walleij, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Saravana Kannan, Greg Kroah-Hartman, Andy Shevchenko,
Catalin Marinas, Will Deacon, Srinivas Kandagatla, Liam Girdwood,
Mark Brown, Jaroslav Kysela, Takashi Iwai, linux-hardening,
linux-kernel, linux-gpio, linux-arm-kernel, linux-sound,
linux-arm-msm, Bartosz Golaszewski
On Tue, Oct 7, 2025 at 12:09 AM Manivannan Sadhasivam <mani@kernel.org> wrote:
>
> > >
> > > Not always... For something like shared reset line, consumers request the line
> > > as GPIO and expect gpiolib to do resource manangement.
> > >
> >
> > They could use the reset API and it would implicitly create a virtual
> > device that requests the reset GPIO and controls its enable count.
> > Except that some devices also do a specific reset sequence with delays
> > etc. That would require some additional logic in reset-gpio.
> >
>
> I was referring to the PCIe PERST# line, for which the 'reset-gpios' property
> already exist in the schema. Now, you want me to model this simple GPIO as a
> fake reset controller and use it the PCIe Bridge nodes through 'resets'
> property?
>
No, not at all. It's just that a shared `reset-gpios` property is
pretty common and Krzysztof implemented the reset-gpio driver[1] to
address it. Drivers that request a reset control via the OF interface
will notice that there's no `resets` property but if there's a
`reset-gpios`, the reset core will create a virtual device binding to
the reset-gpio driver which requests the GPIO in question (once!) and
registers with the reset subsystem providing shared reset control to
users. Basically the abstraction Srini mentioned minus any reset
sequence.
That only happens if the driver uses the reset API. If you go with the
GPIOLIB then none of this matters. I definitely don't want to change
the existing DT sources either but I want to find out if the code in
this series is suitable (with some modifications) for supporting the
PERST# line or if the logic behind it is more complex and possibly
requires separate, more fine-grained handling.
Bartosz
[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/reset/reset-gpio.c
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH RFC 0/9] gpio: improve support for shared GPIOs
2025-10-07 12:29 ` Bartosz Golaszewski
@ 2025-10-21 1:53 ` Manivannan Sadhasivam
2025-10-21 12:06 ` Bartosz Golaszewski
0 siblings, 1 reply; 43+ messages in thread
From: Manivannan Sadhasivam @ 2025-10-21 1:53 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: Dmitry Torokhov, Kees Cook, Mika Westerberg, Andrew Morton,
Linus Walleij, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Saravana Kannan, Greg Kroah-Hartman, Andy Shevchenko,
Catalin Marinas, Will Deacon, Srinivas Kandagatla, Liam Girdwood,
Mark Brown, Jaroslav Kysela, Takashi Iwai, linux-hardening,
linux-kernel, linux-gpio, linux-arm-kernel, linux-sound,
linux-arm-msm, Bartosz Golaszewski
On Tue, Oct 07, 2025 at 02:29:01PM +0200, Bartosz Golaszewski wrote:
> On Tue, Oct 7, 2025 at 12:09 AM Manivannan Sadhasivam <mani@kernel.org> wrote:
> >
> > > >
> > > > Not always... For something like shared reset line, consumers request the line
> > > > as GPIO and expect gpiolib to do resource manangement.
> > > >
> > >
> > > They could use the reset API and it would implicitly create a virtual
> > > device that requests the reset GPIO and controls its enable count.
> > > Except that some devices also do a specific reset sequence with delays
> > > etc. That would require some additional logic in reset-gpio.
> > >
> >
> > I was referring to the PCIe PERST# line, for which the 'reset-gpios' property
> > already exist in the schema. Now, you want me to model this simple GPIO as a
> > fake reset controller and use it the PCIe Bridge nodes through 'resets'
> > property?
> >
>
> No, not at all. It's just that a shared `reset-gpios` property is
> pretty common and Krzysztof implemented the reset-gpio driver[1] to
> address it. Drivers that request a reset control via the OF interface
> will notice that there's no `resets` property but if there's a
> `reset-gpios`, the reset core will create a virtual device binding to
> the reset-gpio driver which requests the GPIO in question (once!) and
> registers with the reset subsystem providing shared reset control to
> users. Basically the abstraction Srini mentioned minus any reset
> sequence.
>
> That only happens if the driver uses the reset API. If you go with the
> GPIOLIB then none of this matters. I definitely don't want to change
> the existing DT sources either but I want to find out if the code in
> this series is suitable (with some modifications) for supporting the
> PERST# line or if the logic behind it is more complex and possibly
> requires separate, more fine-grained handling.
>
All PCI controllers relied on '{reset/perst}-gpios' property for handling the
PERST# signal. Now if we change it to a reset line, then the drivers have to
first detect it as a reset line and use the reset APIs, if not fallback to gpiod
APIs (for DT backwards compatibility), which will add unncessary churn IMO.
But if there is no way the GPIO subsystem is going to support shared GPIOs, then
we have to live with it.
- Mani
--
மணிவண்ணன் சதாசிவம்
^ permalink raw reply [flat|nested] 43+ messages in thread* Re: [PATCH RFC 0/9] gpio: improve support for shared GPIOs
2025-10-21 1:53 ` Manivannan Sadhasivam
@ 2025-10-21 12:06 ` Bartosz Golaszewski
2025-10-21 12:19 ` Manivannan Sadhasivam
2025-10-21 15:56 ` [PATCH RFC 0/9] gpio: improve support for shared GPIOs Andy Shevchenko
0 siblings, 2 replies; 43+ messages in thread
From: Bartosz Golaszewski @ 2025-10-21 12:06 UTC (permalink / raw)
To: Manivannan Sadhasivam
Cc: Dmitry Torokhov, Kees Cook, Mika Westerberg, Andrew Morton,
Linus Walleij, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Saravana Kannan, Greg Kroah-Hartman, Andy Shevchenko,
Catalin Marinas, Will Deacon, Srinivas Kandagatla, Liam Girdwood,
Mark Brown, Jaroslav Kysela, Takashi Iwai, linux-hardening,
linux-kernel, linux-gpio, linux-arm-kernel, linux-sound,
linux-arm-msm, Bartosz Golaszewski
On Tue, Oct 21, 2025 at 3:53 AM Manivannan Sadhasivam <mani@kernel.org> wrote:
> >
> > That only happens if the driver uses the reset API. If you go with the
> > GPIOLIB then none of this matters. I definitely don't want to change
> > the existing DT sources either but I want to find out if the code in
> > this series is suitable (with some modifications) for supporting the
> > PERST# line or if the logic behind it is more complex and possibly
> > requires separate, more fine-grained handling.
> >
>
> All PCI controllers relied on '{reset/perst}-gpios' property for handling the
> PERST# signal. Now if we change it to a reset line, then the drivers have to
> first detect it as a reset line and use the reset APIs, if not fallback to gpiod
> APIs (for DT backwards compatibility), which will add unncessary churn IMO.
>
Ok so some platforms define perst-gpios while others use reset-gpios,
I see now. Yeah, it's better to go with explicit GPIOs then.
> But if there is no way the GPIO subsystem is going to support shared GPIOs, then
> we have to live with it.
>
Well, there is going to be. We already de-facto have it but it doesn't
work very well and is fragile (I'm talking about the non-exclusive
flag). I very much intend to bring this upstream.
My question wrt PCI PERST# was whether this is useful for it because
IIRC all endpoints sharing the signal will assert it (or rather their
pwrctl drivers will) and then only deassert it once all endpoints are
powered up. This would translate to the pwrctl driver doing the
following for each endpoint:
perst = gpiod_get(dev, "perst");
gpiod_set_value_cansleep(perst, 1);
Do the power up.
gpiod_set_value_cansleep(perst, 0);
And with the implementation this series proposes it would mean that
the perst signal will go high after the first endpoint pwrctl driver
sets it to high and only go down once the last driver sets it to low.
The only thing I'm not sure about is the synchronization between the
endpoints - how do we wait for all of them to be powered-up before
calling the last gpiod_set_value()?
Bartosz
^ permalink raw reply [flat|nested] 43+ messages in thread* Re: [PATCH RFC 0/9] gpio: improve support for shared GPIOs
2025-10-21 12:06 ` Bartosz Golaszewski
@ 2025-10-21 12:19 ` Manivannan Sadhasivam
2025-10-21 12:22 ` Bartosz Golaszewski
2025-10-21 15:56 ` [PATCH RFC 0/9] gpio: improve support for shared GPIOs Andy Shevchenko
1 sibling, 1 reply; 43+ messages in thread
From: Manivannan Sadhasivam @ 2025-10-21 12:19 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: Dmitry Torokhov, Kees Cook, Mika Westerberg, Andrew Morton,
Linus Walleij, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Saravana Kannan, Greg Kroah-Hartman, Andy Shevchenko,
Catalin Marinas, Will Deacon, Srinivas Kandagatla, Liam Girdwood,
Mark Brown, Jaroslav Kysela, Takashi Iwai, linux-hardening,
linux-kernel, linux-gpio, linux-arm-kernel, linux-sound,
linux-arm-msm, Bartosz Golaszewski
On Tue, Oct 21, 2025 at 02:06:30PM +0200, Bartosz Golaszewski wrote:
> On Tue, Oct 21, 2025 at 3:53 AM Manivannan Sadhasivam <mani@kernel.org> wrote:
> > >
> > > That only happens if the driver uses the reset API. If you go with the
> > > GPIOLIB then none of this matters. I definitely don't want to change
> > > the existing DT sources either but I want to find out if the code in
> > > this series is suitable (with some modifications) for supporting the
> > > PERST# line or if the logic behind it is more complex and possibly
> > > requires separate, more fine-grained handling.
> > >
> >
> > All PCI controllers relied on '{reset/perst}-gpios' property for handling the
> > PERST# signal. Now if we change it to a reset line, then the drivers have to
> > first detect it as a reset line and use the reset APIs, if not fallback to gpiod
> > APIs (for DT backwards compatibility), which will add unncessary churn IMO.
> >
>
> Ok so some platforms define perst-gpios while others use reset-gpios,
> I see now. Yeah, it's better to go with explicit GPIOs then.
>
> > But if there is no way the GPIO subsystem is going to support shared GPIOs, then
> > we have to live with it.
> >
>
> Well, there is going to be. We already de-facto have it but it doesn't
> work very well and is fragile (I'm talking about the non-exclusive
> flag). I very much intend to bring this upstream.
>
> My question wrt PCI PERST# was whether this is useful for it because
> IIRC all endpoints sharing the signal will assert it (or rather their
> pwrctl drivers will) and then only deassert it once all endpoints are
> powered up. This would translate to the pwrctl driver doing the
> following for each endpoint:
>
> perst = gpiod_get(dev, "perst");
> gpiod_set_value_cansleep(perst, 1);
>
> Do the power up.
>
> gpiod_set_value_cansleep(perst, 0);
>
> And with the implementation this series proposes it would mean that
> the perst signal will go high after the first endpoint pwrctl driver
> sets it to high and only go down once the last driver sets it to low.
> The only thing I'm not sure about is the synchronization between the
> endpoints - how do we wait for all of them to be powered-up before
> calling the last gpiod_set_value()?
>
That will be handled by the pwrctrl core. Not today, but in the coming days.
- Mani
--
மணிவண்ணன் சதாசிவம்
^ permalink raw reply [flat|nested] 43+ messages in thread* Re: [PATCH RFC 0/9] gpio: improve support for shared GPIOs
2025-10-21 12:19 ` Manivannan Sadhasivam
@ 2025-10-21 12:22 ` Bartosz Golaszewski
2025-10-21 12:53 ` Manivannan Sadhasivam
0 siblings, 1 reply; 43+ messages in thread
From: Bartosz Golaszewski @ 2025-10-21 12:22 UTC (permalink / raw)
To: Manivannan Sadhasivam
Cc: Dmitry Torokhov, Kees Cook, Mika Westerberg, Andrew Morton,
Linus Walleij, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Saravana Kannan, Greg Kroah-Hartman, Andy Shevchenko,
Catalin Marinas, Will Deacon, Srinivas Kandagatla, Liam Girdwood,
Mark Brown, Jaroslav Kysela, Takashi Iwai, linux-hardening,
linux-kernel, linux-gpio, linux-arm-kernel, linux-sound,
linux-arm-msm, Bartosz Golaszewski
On Tue, Oct 21, 2025 at 2:20 PM Manivannan Sadhasivam <mani@kernel.org> wrote:
>
> >
> > And with the implementation this series proposes it would mean that
> > the perst signal will go high after the first endpoint pwrctl driver
> > sets it to high and only go down once the last driver sets it to low.
> > The only thing I'm not sure about is the synchronization between the
> > endpoints - how do we wait for all of them to be powered-up before
> > calling the last gpiod_set_value()?
> >
>
> That will be handled by the pwrctrl core. Not today, but in the coming days.
>
But is this the right approach or are you doing it this way *because*
there's no support for enable-counted GPIOs as of yet?
Bart
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH RFC 0/9] gpio: improve support for shared GPIOs
2025-10-21 12:22 ` Bartosz Golaszewski
@ 2025-10-21 12:53 ` Manivannan Sadhasivam
2025-10-21 14:02 ` Bartosz Golaszewski
[not found] ` <CAGb2v65acHoO5025ZN7DhX0xVQf6JyHmUK3CB9UhnmTDDHq6vg@mail.gmail.com>
0 siblings, 2 replies; 43+ messages in thread
From: Manivannan Sadhasivam @ 2025-10-21 12:53 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: Dmitry Torokhov, Kees Cook, Mika Westerberg, Andrew Morton,
Linus Walleij, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Saravana Kannan, Greg Kroah-Hartman, Andy Shevchenko,
Catalin Marinas, Will Deacon, Srinivas Kandagatla, Liam Girdwood,
Mark Brown, Jaroslav Kysela, Takashi Iwai, linux-hardening,
linux-kernel, linux-gpio, linux-arm-kernel, linux-sound,
linux-arm-msm, Bartosz Golaszewski
On Tue, Oct 21, 2025 at 02:22:46PM +0200, Bartosz Golaszewski wrote:
> On Tue, Oct 21, 2025 at 2:20 PM Manivannan Sadhasivam <mani@kernel.org> wrote:
> >
> > >
> > > And with the implementation this series proposes it would mean that
> > > the perst signal will go high after the first endpoint pwrctl driver
> > > sets it to high and only go down once the last driver sets it to low.
> > > The only thing I'm not sure about is the synchronization between the
> > > endpoints - how do we wait for all of them to be powered-up before
> > > calling the last gpiod_set_value()?
> > >
> >
> > That will be handled by the pwrctrl core. Not today, but in the coming days.
> >
>
> But is this the right approach or are you doing it this way *because*
> there's no support for enable-counted GPIOs as of yet?
>
This is the right approach since as of today, pwrctrl core scans the bus, tries
to probe the pwrctrl driver (if one exists for the device to be scanned), powers
it ON, and deasserts the PERST#. If the device is a PCI bridge/switch, then the
devices underneath the downstream bus will only be powered ON after the further
rescan of the downstream bus. But the pwrctrl drivers for those devices might
get loaded at any time (even after the bus rescan).
This causes several issues with the PCI core as this behavior sort of emulates
the PCI hot-plug (devices showing up at random times after bus scan). If the
upstream PCI bridge/switch is not hot-plug capable, then the devices that were
showing up later will fail to enumerate due to lack of resources. The failure
is due to PCI core limiting the resources for non hot-plug PCI bridges as it
doesn't expect the devices to show up later in the downstream port.
One way to fix this issue is by making sure all the pwrctrl capable devices
underneath a PCI bridge getting probed, powered ON, and finally deasserting the
PERST# for each one of them. If the PERST# happens to be shared, it will be
deasserted once at the last. And this order has to be ensured by the pwrctrl
core irrespective of the shared PERST#.
- Mani
--
மணிவண்ணன் சதாசிவம்
^ permalink raw reply [flat|nested] 43+ messages in thread* Re: [PATCH RFC 0/9] gpio: improve support for shared GPIOs
2025-10-21 12:53 ` Manivannan Sadhasivam
@ 2025-10-21 14:02 ` Bartosz Golaszewski
[not found] ` <CAGb2v65acHoO5025ZN7DhX0xVQf6JyHmUK3CB9UhnmTDDHq6vg@mail.gmail.com>
1 sibling, 0 replies; 43+ messages in thread
From: Bartosz Golaszewski @ 2025-10-21 14:02 UTC (permalink / raw)
To: Manivannan Sadhasivam
Cc: Dmitry Torokhov, Kees Cook, Mika Westerberg, Andrew Morton,
Linus Walleij, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Saravana Kannan, Greg Kroah-Hartman, Andy Shevchenko,
Catalin Marinas, Will Deacon, Srinivas Kandagatla, Liam Girdwood,
Mark Brown, Jaroslav Kysela, Takashi Iwai, linux-hardening,
linux-kernel, linux-gpio, linux-arm-kernel, linux-sound,
linux-arm-msm, Bartosz Golaszewski
On Tue, Oct 21, 2025 at 2:53 PM Manivannan Sadhasivam <mani@kernel.org> wrote:
>
> On Tue, Oct 21, 2025 at 02:22:46PM +0200, Bartosz Golaszewski wrote:
> > On Tue, Oct 21, 2025 at 2:20 PM Manivannan Sadhasivam <mani@kernel.org> wrote:
> > >
> > > >
> > > > And with the implementation this series proposes it would mean that
> > > > the perst signal will go high after the first endpoint pwrctl driver
> > > > sets it to high and only go down once the last driver sets it to low.
> > > > The only thing I'm not sure about is the synchronization between the
> > > > endpoints - how do we wait for all of them to be powered-up before
> > > > calling the last gpiod_set_value()?
> > > >
> > >
> > > That will be handled by the pwrctrl core. Not today, but in the coming days.
> > >
> >
> > But is this the right approach or are you doing it this way *because*
> > there's no support for enable-counted GPIOs as of yet?
> >
>
> This is the right approach since as of today, pwrctrl core scans the bus, tries
> to probe the pwrctrl driver (if one exists for the device to be scanned), powers
> it ON, and deasserts the PERST#. If the device is a PCI bridge/switch, then the
> devices underneath the downstream bus will only be powered ON after the further
> rescan of the downstream bus. But the pwrctrl drivers for those devices might
> get loaded at any time (even after the bus rescan).
>
> This causes several issues with the PCI core as this behavior sort of emulates
> the PCI hot-plug (devices showing up at random times after bus scan). If the
> upstream PCI bridge/switch is not hot-plug capable, then the devices that were
> showing up later will fail to enumerate due to lack of resources. The failure
> is due to PCI core limiting the resources for non hot-plug PCI bridges as it
> doesn't expect the devices to show up later in the downstream port.
>
> One way to fix this issue is by making sure all the pwrctrl capable devices
> underneath a PCI bridge getting probed, powered ON, and finally deasserting the
> PERST# for each one of them. If the PERST# happens to be shared, it will be
> deasserted once at the last. And this order has to be ensured by the pwrctrl
> core irrespective of the shared PERST#.
>
Ok, makes sense. In that case this series probably doesn't affect your
work or PCI in general.
Bartosz
^ permalink raw reply [flat|nested] 43+ messages in thread[parent not found: <CAGb2v65acHoO5025ZN7DhX0xVQf6JyHmUK3CB9UhnmTDDHq6vg@mail.gmail.com>]
* Re: PCIe link training and pwrctrl sequence
[not found] ` <CAGb2v65acHoO5025ZN7DhX0xVQf6JyHmUK3CB9UhnmTDDHq6vg@mail.gmail.com>
@ 2025-11-05 9:31 ` Manivannan Sadhasivam
0 siblings, 0 replies; 43+ messages in thread
From: Manivannan Sadhasivam @ 2025-11-05 9:31 UTC (permalink / raw)
To: Chen-Yu Tsai
Cc: Bartosz Golaszewski, Bjorn Helgaas, linux-kernel,
linux-arm-kernel, PCI, open list:THERMAL
On Wed, Oct 22, 2025 at 12:39:41AM +0800, Chen-Yu Tsai wrote:
> (recipient list trimmed down and added PCI & pwrctrl maintainers and lists)
>
> On Tue, Oct 21, 2025 at 8:54 PM Manivannan Sadhasivam <mani@kernel.org> wrote:
> >
> > On Tue, Oct 21, 2025 at 02:22:46PM +0200, Bartosz Golaszewski wrote:
> > > On Tue, Oct 21, 2025 at 2:20 PM Manivannan Sadhasivam <mani@kernel.org> wrote:
> > > >
> > > > >
> > > > > And with the implementation this series proposes it would mean that
> > > > > the perst signal will go high after the first endpoint pwrctl driver
> > > > > sets it to high and only go down once the last driver sets it to low.
> > > > > The only thing I'm not sure about is the synchronization between the
> > > > > endpoints - how do we wait for all of them to be powered-up before
> > > > > calling the last gpiod_set_value()?
> > > > >
> > > >
> > > > That will be handled by the pwrctrl core. Not today, but in the coming days.
> > > >
> > >
> > > But is this the right approach or are you doing it this way *because*
> > > there's no support for enable-counted GPIOs as of yet?
> > >
> >
> > This is the right approach since as of today, pwrctrl core scans the bus, tries
> > to probe the pwrctrl driver (if one exists for the device to be scanned), powers
> > it ON, and deasserts the PERST#. If the device is a PCI bridge/switch, then the
> > devices underneath the downstream bus will only be powered ON after the further
> > rescan of the downstream bus. But the pwrctrl drivers for those devices might
> > get loaded at any time (even after the bus rescan).
> >
> > This causes several issues with the PCI core as this behavior sort of emulates
> > the PCI hot-plug (devices showing up at random times after bus scan). If the
> > upstream PCI bridge/switch is not hot-plug capable, then the devices that were
> > showing up later will fail to enumerate due to lack of resources. The failure
> > is due to PCI core limiting the resources for non hot-plug PCI bridges as it
> > doesn't expect the devices to show up later in the downstream port.
>
> Side note:
>
> Today I was looking into how the PCI core does slot pwrctrl, and it doesn't
> really work for some of the PCI controller drivers.
>
> The pwrctrl stuff happens after the driver adds the host bus bridge.
> However drivers are doing link training before that. If the power is
> not on, link training will fail, and the driver errors out. It never
> has a chance to get to pwrctrl.
>
> I wonder if some bits should be split out so they could be interleaved with
> link management on the host side. AFAICT only dwc and qcom will rescan the
> bus when an interrupt says the link is up. Other controllers might not have
> such an interrupt notification. I was looking at the MediaTek gen3 driver
> specifically.
>
This is a known issue. With the initial design of the pwrctrl framework, we
thought that the pwrctrl devices should be created without controller
intervention. But it is proving to be wrong as some controllers expect the
devices to show up before the PHY initialization, as yours.
We are working on a series (almost complete, just needs cleanup) that moves the
pwrctrl creation to a new exported API and allows the controller drivers to call
the API from whereever they want based on the requirement. This series also
fixes the above mentioned hotplug/PERST# issues by scanning all the PCIe nodes
in one shot and creating pwrctrl devices and making use all of them gets probed
(just the pwrctrl drivers, not PCIe client drivers) before deasserting PERST#
and then scanning the bus. This would require all the pwrctrl drivers to be
loaded before probing the controller driver (otherwise, probe deferral will
happen), but that's a valid dependency.
This will allow the PCI core to find all the devices during the initial bus
scan and will fix the resource allocation issue.
- Mani
--
மணிவண்ணன் சதாசிவம்
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH RFC 0/9] gpio: improve support for shared GPIOs
2025-10-21 12:06 ` Bartosz Golaszewski
2025-10-21 12:19 ` Manivannan Sadhasivam
@ 2025-10-21 15:56 ` Andy Shevchenko
1 sibling, 0 replies; 43+ messages in thread
From: Andy Shevchenko @ 2025-10-21 15:56 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: Manivannan Sadhasivam, Dmitry Torokhov, Kees Cook,
Mika Westerberg, Andrew Morton, Linus Walleij, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Saravana Kannan,
Greg Kroah-Hartman, Andy Shevchenko, Catalin Marinas, Will Deacon,
Srinivas Kandagatla, Liam Girdwood, Mark Brown, Jaroslav Kysela,
Takashi Iwai, linux-hardening, linux-kernel, linux-gpio,
linux-arm-kernel, linux-sound, linux-arm-msm, Bartosz Golaszewski
On Tue, Oct 21, 2025 at 02:06:30PM +0200, Bartosz Golaszewski wrote:
> On Tue, Oct 21, 2025 at 3:53 AM Manivannan Sadhasivam <mani@kernel.org> wrote:
...
> > > That only happens if the driver uses the reset API. If you go with the
> > > GPIOLIB then none of this matters. I definitely don't want to change
> > > the existing DT sources either but I want to find out if the code in
> > > this series is suitable (with some modifications) for supporting the
> > > PERST# line or if the logic behind it is more complex and possibly
> > > requires separate, more fine-grained handling.
> >
> > All PCI controllers relied on '{reset/perst}-gpios' property for handling the
> > PERST# signal. Now if we change it to a reset line, then the drivers have to
> > first detect it as a reset line and use the reset APIs, if not fallback to gpiod
> > APIs (for DT backwards compatibility), which will add unncessary churn IMO.
Can't the reset check be added into a common place once for all the PCI controllers?
Also one can make a helper that does all necessary checks and if required falls back
to the GPIO.
int pcie_controller_get_perst(... dev, const chat *con_id /* fallback */);
(name is from top of my mind without following any patterns).
> Ok so some platforms define perst-gpios while others use reset-gpios,
> I see now. Yeah, it's better to go with explicit GPIOs then.
>
> > But if there is no way the GPIO subsystem is going to support shared GPIOs, then
> > we have to live with it.
>
> Well, there is going to be. We already de-facto have it but it doesn't
> work very well and is fragile (I'm talking about the non-exclusive
> flag). I very much intend to bring this upstream.
>
> My question wrt PCI PERST# was whether this is useful for it because
> IIRC all endpoints sharing the signal will assert it (or rather their
> pwrctl drivers will) and then only deassert it once all endpoints are
> powered up. This would translate to the pwrctl driver doing the
> following for each endpoint:
>
> perst = gpiod_get(dev, "perst");
> gpiod_set_value_cansleep(perst, 1);
>
> Do the power up.
>
> gpiod_set_value_cansleep(perst, 0);
>
> And with the implementation this series proposes it would mean that
> the perst signal will go high after the first endpoint pwrctl driver
> sets it to high and only go down once the last driver sets it to low.
> The only thing I'm not sure about is the synchronization between the
> endpoints - how do we wait for all of them to be powered-up before
> calling the last gpiod_set_value()?
The refcount inside GPIO descriptor? Then gpiod_set_value() will do the magic.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH RFC 0/9] gpio: improve support for shared GPIOs
2025-09-24 14:51 [PATCH RFC 0/9] gpio: improve support for shared GPIOs Bartosz Golaszewski
` (9 preceding siblings ...)
2025-09-24 18:25 ` [PATCH RFC 0/9] gpio: improve support for shared GPIOs Dmitry Torokhov
@ 2025-10-01 8:49 ` Linus Walleij
2025-10-01 10:53 ` Linus Walleij
2025-10-04 13:31 ` Srinivas Kandagatla
` (2 subsequent siblings)
13 siblings, 1 reply; 43+ messages in thread
From: Linus Walleij @ 2025-10-01 8:49 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: Kees Cook, Mika Westerberg, Dmitry Torokhov, Andrew Morton,
Manivannan Sadhasivam, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Saravana Kannan, Greg Kroah-Hartman,
Andy Shevchenko, Catalin Marinas, Will Deacon,
Srinivas Kandagatla, Liam Girdwood, Mark Brown, Jaroslav Kysela,
Takashi Iwai, linux-hardening, linux-kernel, linux-gpio,
linux-arm-kernel, linux-sound, linux-arm-msm, Bartosz Golaszewski
Hi Bartosz,
I see the big picture of this plan!
One quick comment:
On Wed, Sep 24, 2025 at 4:51 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
> I'm Cc'ing some people that may help with reviewing/be interested in
> this: OF maintainers (because the main target are OF systems initially),
> Mark Brown because most users of GPIOD_FLAGS_BIT_NONEXCLUSIVE live
> in audio or regulator drivers and one of the goals of this series is
> dropping the hand-crafted GPIO enable counting via struct
> regulator_enable_gpio in regulator core),
...and that is what I thought as well, so:
> arm64: select HAVE_SHARED_GPIOS for ARCH_QCOM
why would we be selecting this per-subarch?
What will happen is that CONFIG_REGULATOR will select
it and since everyone and their dog is using regulator, what
will happen is that every system will have this enabled,
and every GPIO access on every system will be proxied
and then this better be fast.
Two things come to mind, and I bet you have thought of
them already:
1. Footprint: all systems using regulators will now have
to compile in all this code as well.
2. Performance, I didn't quite get it if every GPIO on the
system will be proxied through a layer of indirection
if you select HAVE_SHARED_GPIOS
but that would not be good, since some users are in
fastpath such as IRQ handlers, and the old way of
sharing GPIOs would just affect pins that are actually
shared.
I don't know of a good generic solution for (2) to be honest,
last resort would be something like runtime patching of
calls when a GPIO becomes shared and that is really
advanced but maybe necessary to get a performant and
generic solution.
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 43+ messages in thread* Re: [PATCH RFC 0/9] gpio: improve support for shared GPIOs
2025-10-01 8:49 ` Linus Walleij
@ 2025-10-01 10:53 ` Linus Walleij
2025-10-01 13:04 ` Bartosz Golaszewski
0 siblings, 1 reply; 43+ messages in thread
From: Linus Walleij @ 2025-10-01 10:53 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: Kees Cook, Mika Westerberg, Dmitry Torokhov, Andrew Morton,
Manivannan Sadhasivam, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Saravana Kannan, Greg Kroah-Hartman,
Andy Shevchenko, Catalin Marinas, Will Deacon,
Srinivas Kandagatla, Liam Girdwood, Mark Brown, Jaroslav Kysela,
Takashi Iwai, linux-hardening, linux-kernel, linux-gpio,
linux-arm-kernel, linux-sound, linux-arm-msm, Bartosz Golaszewski
Replying to self so Bartosz don't have to tell me off...
On Wed, Oct 1, 2025 at 10:49 AM Linus Walleij <linus.walleij@linaro.org> wrote:
> and every GPIO access on every system will be proxied
> and then this better be fast.
What about I read the code before I talk :/
Inspecting patch 4/9 it is clear that only GPIOs that actually
need to be proxied are proxied.
> Two things come to mind, and I bet you have thought of
> them already:
>
> 1. Footprint: all systems using regulators will now have
> to compile in all this code as well.
This still holds. It could be a concern if it's a lot of code.
> 2. Performance, I didn't quite get it if every GPIO on the
> system will be proxied through a layer of indirection
> if you select HAVE_SHARED_GPIOS
> but that would not be good, since some users are in
> fastpath such as IRQ handlers, and the old way of
> sharing GPIOs would just affect pins that are actually
> shared.
It is clear from patch 4/9 that this only affects GPIOs
that are actually shared, and those tend to not be
performance-critical so this concern is moot.
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH RFC 0/9] gpio: improve support for shared GPIOs
2025-10-01 10:53 ` Linus Walleij
@ 2025-10-01 13:04 ` Bartosz Golaszewski
0 siblings, 0 replies; 43+ messages in thread
From: Bartosz Golaszewski @ 2025-10-01 13:04 UTC (permalink / raw)
To: Linus Walleij
Cc: Kees Cook, Mika Westerberg, Dmitry Torokhov, Andrew Morton,
Manivannan Sadhasivam, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Saravana Kannan, Greg Kroah-Hartman,
Andy Shevchenko, Catalin Marinas, Will Deacon,
Srinivas Kandagatla, Liam Girdwood, Mark Brown, Jaroslav Kysela,
Takashi Iwai, linux-hardening, linux-kernel, linux-gpio,
linux-arm-kernel, linux-sound, linux-arm-msm, Bartosz Golaszewski,
Bartosz Golaszewski
On Wed, 1 Oct 2025 12:53:07 +0200, Linus Walleij
<linus.walleij@linaro.org> said:
> Replying to self so Bartosz don't have to tell me off...
>
> On Wed, Oct 1, 2025 at 10:49 AM Linus Walleij <linus.walleij@linaro.org> wrote:
>
>> and every GPIO access on every system will be proxied
>> and then this better be fast.
>
> What about I read the code before I talk :/
>
> Inspecting patch 4/9 it is clear that only GPIOs that actually
> need to be proxied are proxied.
>
>> Two things come to mind, and I bet you have thought of
>> them already:
>>
>> 1. Footprint: all systems using regulators will now have
>> to compile in all this code as well.
>
> This still holds. It could be a concern if it's a lot of code.
It depends on how we implement this. If we just rip out the enable counting
from regulator core entirely, then it would be transparent from the
regulator's point of view and each platform could still select the new option
as required.
However there's the issue of regulator consumers who need to know when
something changes on a regulator and to that end subscribe to the regulator
notifer. Regulator core knows then it actually changes the GPIO so it emits
the event. There are several ways to approach it but the best one seems to
be: allow to subscribe for a per-descriptor event notifier (implementation
details may include: only actually creating the notifier for shared GPIOs),
and be notified about an actual change in value and then propagate it to
regulator users. This would still be transparent and allow us to select
HAVE_SHARED_GPIOS on a per-arch basis.
Bartosz
>
>> 2. Performance, I didn't quite get it if every GPIO on the
>> system will be proxied through a layer of indirection
>> if you select HAVE_SHARED_GPIOS
>> but that would not be good, since some users are in
>> fastpath such as IRQ handlers, and the old way of
>> sharing GPIOs would just affect pins that are actually
>> shared.
>
> It is clear from patch 4/9 that this only affects GPIOs
> that are actually shared, and those tend to not be
> performance-critical so this concern is moot.
>
> Yours,
> Linus Walleij
>
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH RFC 0/9] gpio: improve support for shared GPIOs
2025-09-24 14:51 [PATCH RFC 0/9] gpio: improve support for shared GPIOs Bartosz Golaszewski
` (10 preceding siblings ...)
2025-10-01 8:49 ` Linus Walleij
@ 2025-10-04 13:31 ` Srinivas Kandagatla
2025-10-06 11:55 ` Bartosz Golaszewski
2025-10-06 12:19 ` Mark Brown
2025-10-09 10:12 ` (subset) " Bartosz Golaszewski
2025-10-17 17:26 ` Bartosz Golaszewski
13 siblings, 2 replies; 43+ messages in thread
From: Srinivas Kandagatla @ 2025-10-04 13:31 UTC (permalink / raw)
To: Bartosz Golaszewski, Kees Cook, Mika Westerberg, Dmitry Torokhov,
Andrew Morton, Linus Walleij, Manivannan Sadhasivam, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Saravana Kannan,
Greg Kroah-Hartman, Andy Shevchenko, Catalin Marinas, Will Deacon,
Srinivas Kandagatla, Liam Girdwood, Mark Brown, Jaroslav Kysela,
Takashi Iwai
Cc: linux-hardening, linux-kernel, linux-gpio, linux-arm-kernel,
linux-sound, linux-arm-msm, Bartosz Golaszewski
On 9/24/25 3:51 PM, Bartosz Golaszewski wrote:
> Here's a functional RFC for improving the handling of shared GPIOs in
> linux.
>
> Problem statement: GPIOs are implemented as a strictly exclusive
> resource in the kernel but there are lots of platforms on which single
> pin is shared by multiple devices which don't communicate so need some
> way of properly sharing access to a GPIO. What we have now is the
> GPIOD_FLAGS_BIT_NONEXCLUSIVE flag which was introduced as a hack and
> doesn't do any locking or arbitration of access - it literally just hand
> the same GPIO descriptor to all interested users.
Isn't the main issue here is about not using a correct framework around
to the gpios that driver uses. ex: the codec usecase that you are
refering in this is using gpio to reset the line, instead of using a
proper gpio-reset control. same with some of the gpio-muxes. the problem
is fixed once such direct users of gpio are move their correct frameworks.
Am not sure adding a abstraction with-in gpio framework is right
solution, But I do agree that NONEXCLUSIVE flags should disappear and
users that are using this should be moved to correct frameworks where
they belong.
--srini
>
> The proposed solution is composed of three major parts: the high-level,
> shared GPIO proxy driver that arbitrates access to the shared pin and
> exposes a regular GPIO chip interface to consumers, a low-level shared
> GPIOLIB module that scans firmware nodes and creates auxiliary devices
> that attach to the proxy driver and finally a set of core GPIOLIB
> changes that plug the former into the GPIO lookup path.
>
> The changes are implemented in a way that allows to seamlessly compile
> out any code related to sharing GPIOs for systems that don't need it.
>
> The practical use-case for this are the powerdown GPIOs shared by
> speakers on Qualcomm db845c platform, however I have also extensively
> tested it using gpio-virtuser on arm64 qemu with various DT
> configurations.
>
> I'm Cc'ing some people that may help with reviewing/be interested in
> this: OF maintainers (because the main target are OF systems initially),
> Mark Brown because most users of GPIOD_FLAGS_BIT_NONEXCLUSIVE live
> in audio or regulator drivers and one of the goals of this series is
> dropping the hand-crafted GPIO enable counting via struct
> regulator_enable_gpio in regulator core), Andy and Mika because I'd like
> to also cover ACPI (even though I don't know about any ACPI platform that
> would need this at the moment, I think it makes sense to make the
> solution complete), Dmitry (same thing but for software nodes), Mani
> (because you have a somewhat related use-case for the PERST# signal and
> I'd like to hear your input on whether this is something you can use or
> maybe it needs a separate, implicit gpio-perst driver similar to what
> Krzysztof did for reset-gpios) and Greg (because I mentioned this to you
> last week in person and I also use the auxiliary bus for the proxy
> devices).
>
> First patch in the series is a bugfix targetting stable, I'm surprised
> nobody noticed the lockdep splat yet. The second adds a library function
> I use in a later patch. All remaining patches implement or use the
> shared GPIO support.
>
> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> ---
> Bartosz Golaszewski (9):
> gpio: wcd934x: mark the GPIO controller as sleeping
> string: provide strends()
> gpiolib: define GPIOD_FLAG_SHARED
> gpiolib: implement low-level, shared GPIO support
> gpio: shared-proxy: implement the shared GPIO proxy driver
> gpiolib: support shared GPIOs in core subsystem code
> arm64: select HAVE_SHARED_GPIOS for ARCH_QCOM
> ASoC: wsa881x: drop GPIOD_FLAGS_BIT_NONEXCLUSIVE flag from GPIO lookup
> ASoC: wsa883x: drop GPIOD_FLAGS_BIT_NONEXCLUSIVE flag from GPIO lookup
>
> arch/arm64/Kconfig.platforms | 1 +
> drivers/gpio/Kconfig | 17 ++
> drivers/gpio/Makefile | 2 +
> drivers/gpio/gpio-shared-proxy.c | 328 ++++++++++++++++++++++++++
> drivers/gpio/gpio-wcd934x.c | 2 +-
> drivers/gpio/gpiolib-shared.c | 481 +++++++++++++++++++++++++++++++++++++++
> drivers/gpio/gpiolib-shared.h | 71 ++++++
> drivers/gpio/gpiolib.c | 50 +++-
> drivers/gpio/gpiolib.h | 1 +
> include/linux/string.h | 2 +
> lib/string.c | 19 ++
> lib/tests/string_kunit.c | 13 ++
> sound/soc/codecs/wsa881x.c | 3 +-
> sound/soc/codecs/wsa883x.c | 7 +-
> 14 files changed, 980 insertions(+), 17 deletions(-)
> ---
> base-commit: b46f7370d4a0f0b55f05b854e73b2a90dff41e1b
> change-id: 20250908-gpio-shared-67ec352884b6
>
> Best regards,
^ permalink raw reply [flat|nested] 43+ messages in thread* Re: [PATCH RFC 0/9] gpio: improve support for shared GPIOs
2025-10-04 13:31 ` Srinivas Kandagatla
@ 2025-10-06 11:55 ` Bartosz Golaszewski
2025-10-06 21:55 ` Srinivas Kandagatla
2025-10-06 12:19 ` Mark Brown
1 sibling, 1 reply; 43+ messages in thread
From: Bartosz Golaszewski @ 2025-10-06 11:55 UTC (permalink / raw)
To: Srinivas Kandagatla
Cc: Kees Cook, Mika Westerberg, Dmitry Torokhov, Andrew Morton,
Linus Walleij, Manivannan Sadhasivam, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Saravana Kannan,
Greg Kroah-Hartman, Andy Shevchenko, Catalin Marinas, Will Deacon,
Liam Girdwood, Mark Brown, Jaroslav Kysela, Takashi Iwai,
linux-hardening, linux-kernel, linux-gpio, linux-arm-kernel,
linux-sound, linux-arm-msm, Bartosz Golaszewski
On Sat, Oct 4, 2025 at 3:32 PM Srinivas Kandagatla <srini@kernel.org> wrote:
> On 9/24/25 3:51 PM, Bartosz Golaszewski wrote:
> > Here's a functional RFC for improving the handling of shared GPIOs in
> > linux.
> >
> > Problem statement: GPIOs are implemented as a strictly exclusive
> > resource in the kernel but there are lots of platforms on which single
> > pin is shared by multiple devices which don't communicate so need some
> > way of properly sharing access to a GPIO. What we have now is the
> > GPIOD_FLAGS_BIT_NONEXCLUSIVE flag which was introduced as a hack and
> > doesn't do any locking or arbitration of access - it literally just hand
> > the same GPIO descriptor to all interested users.
>
> Isn't the main issue here is about not using a correct framework around
> to the gpios that driver uses. ex: the codec usecase that you are
> refering in this is using gpio to reset the line, instead of using a
> proper gpio-reset control. same with some of the gpio-muxes. the problem
> is fixed once such direct users of gpio are move their correct frameworks.
>
If they were called "reset-gpios" then we could (and should) use
Krzysztof's reset-gpio driver here, but we have many cases where
that's not the case and the names (and implied functions) are
arbitrary. In the case addressed in this series, the GPIOs are called
"powerdown". The second big user of nonexclusive GPIOs are fixed
regulators where the line isn't called "reset" either. There are also
various other uses sprinkled all over the kernel for which no good
abstraction exists or can even be designed in a generic way.
> Am not sure adding a abstraction with-in gpio framework is right
> solution, But I do agree that NONEXCLUSIVE flags should disappear and
> users that are using this should be moved to correct frameworks where
> they belong.
>
I'm open to suggestions but DT maintainers have not been particularly
fond of creating "virtual" devices to accommodate driver
implementations.
Bartosz
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH RFC 0/9] gpio: improve support for shared GPIOs
2025-10-06 11:55 ` Bartosz Golaszewski
@ 2025-10-06 21:55 ` Srinivas Kandagatla
2025-10-07 13:13 ` Bartosz Golaszewski
0 siblings, 1 reply; 43+ messages in thread
From: Srinivas Kandagatla @ 2025-10-06 21:55 UTC (permalink / raw)
To: Bartosz Golaszewski, Srinivas Kandagatla
Cc: Kees Cook, Mika Westerberg, Dmitry Torokhov, Andrew Morton,
Linus Walleij, Manivannan Sadhasivam, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Saravana Kannan,
Greg Kroah-Hartman, Andy Shevchenko, Catalin Marinas, Will Deacon,
Liam Girdwood, Mark Brown, Jaroslav Kysela, Takashi Iwai,
linux-hardening, linux-kernel, linux-gpio, linux-arm-kernel,
linux-sound, linux-arm-msm, Bartosz Golaszewski
On 10/6/25 12:55 PM, Bartosz Golaszewski wrote:
> On Sat, Oct 4, 2025 at 3:32 PM Srinivas Kandagatla <srini@kernel.org> wrote:
>> On 9/24/25 3:51 PM, Bartosz Golaszewski wrote:
>>> Here's a functional RFC for improving the handling of shared GPIOs in
>>> linux.
>>>
>>> Problem statement: GPIOs are implemented as a strictly exclusive
>>> resource in the kernel but there are lots of platforms on which single
>>> pin is shared by multiple devices which don't communicate so need some
>>> way of properly sharing access to a GPIO. What we have now is the
>>> GPIOD_FLAGS_BIT_NONEXCLUSIVE flag which was introduced as a hack and
>>> doesn't do any locking or arbitration of access - it literally just hand
>>> the same GPIO descriptor to all interested users.
>>
>> Isn't the main issue here is about not using a correct framework around
>> to the gpios that driver uses. ex: the codec usecase that you are
>> refering in this is using gpio to reset the line, instead of using a
>> proper gpio-reset control. same with some of the gpio-muxes. the problem
>> is fixed once such direct users of gpio are move their correct frameworks.
>>
>
> If they were called "reset-gpios" then we could (and should) use
> Krzysztof's reset-gpio driver here, but we have many cases where
> that's not the case and the names (and implied functions) are
Yes, these codec drivers are due to be moved to use reset-gpios.
--srini
> arbitrary. In the case addressed in this series, the GPIOs are called
> "powerdown". The second big user of nonexclusive GPIOs are fixed
> regulators where the line isn't called "reset" either. There are also
> various other uses sprinkled all over the kernel for which no good
> abstraction exists or can even be designed in a generic way.
>
>> Am not sure adding a abstraction with-in gpio framework is right
>> solution, But I do agree that NONEXCLUSIVE flags should disappear and
>> users that are using this should be moved to correct frameworks where
>> they belong.
>>
>
> I'm open to suggestions but DT maintainers have not been particularly
> fond of creating "virtual" devices to accommodate driver
> implementations.
>
> Bartosz
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH RFC 0/9] gpio: improve support for shared GPIOs
2025-10-06 21:55 ` Srinivas Kandagatla
@ 2025-10-07 13:13 ` Bartosz Golaszewski
2025-10-07 13:18 ` Mark Brown
0 siblings, 1 reply; 43+ messages in thread
From: Bartosz Golaszewski @ 2025-10-07 13:13 UTC (permalink / raw)
To: Srinivas Kandagatla
Cc: Kees Cook, Mika Westerberg, Dmitry Torokhov, Andrew Morton,
Linus Walleij, Manivannan Sadhasivam, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Saravana Kannan,
Greg Kroah-Hartman, Andy Shevchenko, Catalin Marinas, Will Deacon,
Liam Girdwood, Mark Brown, Jaroslav Kysela, Takashi Iwai,
linux-hardening, linux-kernel, linux-gpio, linux-arm-kernel,
linux-sound, linux-arm-msm, Bartosz Golaszewski
On Mon, Oct 6, 2025 at 11:55 PM Srinivas Kandagatla <srini@kernel.org> wrote:
>
> >
> > If they were called "reset-gpios" then we could (and should) use
> > Krzysztof's reset-gpio driver here, but we have many cases where
> > that's not the case and the names (and implied functions) are
>
> Yes, these codec drivers are due to be moved to use reset-gpios.
>
You will still need to keep support for the current "powerdown-gpios"
property in existing device tree sources so that doesn't change
anything. And what about shared pins other than reset? 'dc-gpios' for
display, other 'powerdown' instances, 'enable-gpios', all kinds of
uncommon names like: `dlg,cs`, `wlf,ldo2ena`, `speaker-enable`,
`maxim,enable`? It's not likely we will create a separate abstraction
for each of them. What I'm proposing is a sane, catch-all mechanism
for shared GPIOs that right now use a rather wonky hack. Also: the
drivers have no business knowing that they request GPIOs that can be
shared. This is why I want to hide it inside GPIOLIB.
Bartosz
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH RFC 0/9] gpio: improve support for shared GPIOs
2025-10-07 13:13 ` Bartosz Golaszewski
@ 2025-10-07 13:18 ` Mark Brown
2025-10-07 13:21 ` Bartosz Golaszewski
0 siblings, 1 reply; 43+ messages in thread
From: Mark Brown @ 2025-10-07 13:18 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: Srinivas Kandagatla, Kees Cook, Mika Westerberg, Dmitry Torokhov,
Andrew Morton, Linus Walleij, Manivannan Sadhasivam, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Saravana Kannan,
Greg Kroah-Hartman, Andy Shevchenko, Catalin Marinas, Will Deacon,
Liam Girdwood, Jaroslav Kysela, Takashi Iwai, linux-hardening,
linux-kernel, linux-gpio, linux-arm-kernel, linux-sound,
linux-arm-msm, Bartosz Golaszewski
[-- Attachment #1: Type: text/plain, Size: 579 bytes --]
On Tue, Oct 07, 2025 at 03:13:29PM +0200, Bartosz Golaszewski wrote:
> On Mon, Oct 6, 2025 at 11:55 PM Srinivas Kandagatla <srini@kernel.org> wrote:
> > Yes, these codec drivers are due to be moved to use reset-gpios.
...
> anything. And what about shared pins other than reset? 'dc-gpios' for
> display, other 'powerdown' instances, 'enable-gpios', all kinds of
> uncommon names like: `dlg,cs`, `wlf,ldo2ena`, `speaker-enable`,
> `maxim,enable`? It's not likely we will create a separate abstraction
Many of which, crucially, don't actually reset the device.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH RFC 0/9] gpio: improve support for shared GPIOs
2025-10-07 13:18 ` Mark Brown
@ 2025-10-07 13:21 ` Bartosz Golaszewski
0 siblings, 0 replies; 43+ messages in thread
From: Bartosz Golaszewski @ 2025-10-07 13:21 UTC (permalink / raw)
To: Mark Brown
Cc: Srinivas Kandagatla, Kees Cook, Mika Westerberg, Dmitry Torokhov,
Andrew Morton, Linus Walleij, Manivannan Sadhasivam, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Saravana Kannan,
Greg Kroah-Hartman, Andy Shevchenko, Catalin Marinas, Will Deacon,
Liam Girdwood, Jaroslav Kysela, Takashi Iwai, linux-hardening,
linux-kernel, linux-gpio, linux-arm-kernel, linux-sound,
linux-arm-msm, Bartosz Golaszewski
On Tue, Oct 7, 2025 at 3:18 PM Mark Brown <broonie@kernel.org> wrote:
>
> On Tue, Oct 07, 2025 at 03:13:29PM +0200, Bartosz Golaszewski wrote:
> > On Mon, Oct 6, 2025 at 11:55 PM Srinivas Kandagatla <srini@kernel.org> wrote:
>
> > > Yes, these codec drivers are due to be moved to use reset-gpios.
>
> ...
>
> > anything. And what about shared pins other than reset? 'dc-gpios' for
> > display, other 'powerdown' instances, 'enable-gpios', all kinds of
> > uncommon names like: `dlg,cs`, `wlf,ldo2ena`, `speaker-enable`,
> > `maxim,enable`? It's not likely we will create a separate abstraction
>
> Many of which, crucially, don't actually reset the device.
That's my point. While the function of a `reset-gpios` is quite clear,
these are not.
Bartosz
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH RFC 0/9] gpio: improve support for shared GPIOs
2025-10-04 13:31 ` Srinivas Kandagatla
2025-10-06 11:55 ` Bartosz Golaszewski
@ 2025-10-06 12:19 ` Mark Brown
2025-10-07 12:32 ` Srinivas Kandagatla
1 sibling, 1 reply; 43+ messages in thread
From: Mark Brown @ 2025-10-06 12:19 UTC (permalink / raw)
To: Srinivas Kandagatla
Cc: Bartosz Golaszewski, Kees Cook, Mika Westerberg, Dmitry Torokhov,
Andrew Morton, Linus Walleij, Manivannan Sadhasivam, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Saravana Kannan,
Greg Kroah-Hartman, Andy Shevchenko, Catalin Marinas, Will Deacon,
Liam Girdwood, Jaroslav Kysela, Takashi Iwai, linux-hardening,
linux-kernel, linux-gpio, linux-arm-kernel, linux-sound,
linux-arm-msm, Bartosz Golaszewski
[-- Attachment #1: Type: text/plain, Size: 507 bytes --]
On Sat, Oct 04, 2025 at 02:31:16PM +0100, Srinivas Kandagatla wrote:
> Isn't the main issue here is about not using a correct framework around
> to the gpios that driver uses. ex: the codec usecase that you are
> refering in this is using gpio to reset the line, instead of using a
> proper gpio-reset control. same with some of the gpio-muxes. the problem
> is fixed once such direct users of gpio are move their correct frameworks.
It's common to have GPIO controls for other things, mutes for example.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH RFC 0/9] gpio: improve support for shared GPIOs
2025-10-06 12:19 ` Mark Brown
@ 2025-10-07 12:32 ` Srinivas Kandagatla
0 siblings, 0 replies; 43+ messages in thread
From: Srinivas Kandagatla @ 2025-10-07 12:32 UTC (permalink / raw)
To: Mark Brown, Srinivas Kandagatla
Cc: Bartosz Golaszewski, Kees Cook, Mika Westerberg, Dmitry Torokhov,
Andrew Morton, Linus Walleij, Manivannan Sadhasivam, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Saravana Kannan,
Greg Kroah-Hartman, Andy Shevchenko, Catalin Marinas, Will Deacon,
Liam Girdwood, Jaroslav Kysela, Takashi Iwai, linux-hardening,
linux-kernel, linux-gpio, linux-arm-kernel, linux-sound,
linux-arm-msm, Bartosz Golaszewski
On 10/6/25 1:19 PM, Mark Brown wrote:
> On Sat, Oct 04, 2025 at 02:31:16PM +0100, Srinivas Kandagatla wrote:
>
>> Isn't the main issue here is about not using a correct framework around
>> to the gpios that driver uses. ex: the codec usecase that you are
>> refering in this is using gpio to reset the line, instead of using a
>> proper gpio-reset control. same with some of the gpio-muxes. the problem
>> is fixed once such direct users of gpio are move their correct frameworks.
>
> It's common to have GPIO controls for other things, mutes for example.
Ofcourse, but this aggregation logic which is specific to the use-case
should not live in gpio subsystem which makes the whole solution fragile
and abused.
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: (subset) [PATCH RFC 0/9] gpio: improve support for shared GPIOs
2025-09-24 14:51 [PATCH RFC 0/9] gpio: improve support for shared GPIOs Bartosz Golaszewski
` (11 preceding siblings ...)
2025-10-04 13:31 ` Srinivas Kandagatla
@ 2025-10-09 10:12 ` Bartosz Golaszewski
2025-10-17 17:26 ` Bartosz Golaszewski
13 siblings, 0 replies; 43+ messages in thread
From: Bartosz Golaszewski @ 2025-10-09 10:12 UTC (permalink / raw)
To: Kees Cook, Mika Westerberg, Dmitry Torokhov, Andrew Morton,
Linus Walleij, Manivannan Sadhasivam, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Saravana Kannan,
Greg Kroah-Hartman, Andy Shevchenko, Catalin Marinas, Will Deacon,
Srinivas Kandagatla, Liam Girdwood, Mark Brown, Jaroslav Kysela,
Takashi Iwai, Bartosz Golaszewski
Cc: Bartosz Golaszewski, linux-hardening, linux-kernel, linux-gpio,
linux-arm-kernel, linux-sound, linux-arm-msm
From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
On Wed, 24 Sep 2025 16:51:28 +0200, Bartosz Golaszewski wrote:
> Here's a functional RFC for improving the handling of shared GPIOs in
> linux.
>
> Problem statement: GPIOs are implemented as a strictly exclusive
> resource in the kernel but there are lots of platforms on which single
> pin is shared by multiple devices which don't communicate so need some
> way of properly sharing access to a GPIO. What we have now is the
> GPIOD_FLAGS_BIT_NONEXCLUSIVE flag which was introduced as a hack and
> doesn't do any locking or arbitration of access - it literally just hand
> the same GPIO descriptor to all interested users.
>
> [...]
I'm picking this one up for fixes as it addresses a locking bug.
[1/9] gpio: wcd934x: mark the GPIO controller as sleeping
https://git.kernel.org/brgl/linux/c/83d314fac266a3d9de61e4a4490c4f2eafc86b05
Best regards,
--
Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
^ permalink raw reply [flat|nested] 43+ messages in thread* Re: [PATCH RFC 0/9] gpio: improve support for shared GPIOs
2025-09-24 14:51 [PATCH RFC 0/9] gpio: improve support for shared GPIOs Bartosz Golaszewski
` (12 preceding siblings ...)
2025-10-09 10:12 ` (subset) " Bartosz Golaszewski
@ 2025-10-17 17:26 ` Bartosz Golaszewski
2025-10-17 17:32 ` Mark Brown
13 siblings, 1 reply; 43+ messages in thread
From: Bartosz Golaszewski @ 2025-10-17 17:26 UTC (permalink / raw)
To: Bartosz Golaszewski, Mark Brown
Cc: linux-hardening, linux-kernel, linux-gpio, linux-arm-kernel,
linux-sound, linux-arm-msm, Bartosz Golaszewski,
Srinivas Kandagatla, Kees Cook, Mika Westerberg, Conor Dooley,
Jaroslav Kysela, Catalin Marinas, Krzysztof Kozlowski,
Takashi Iwai, Rob Herring, Greg Kroah-Hartman, Andy Shevchenko,
Saravana Kannan, Will Deacon, Liam Girdwood, Andrew Morton,
Manivannan Sadhasivam, Linus Walleij, Dmitry Torokhov
On Wed, Sep 24, 2025 at 4:51 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
>
> Here's a functional RFC for improving the handling of shared GPIOs in
> linux.
>
> Problem statement: GPIOs are implemented as a strictly exclusive
> resource in the kernel but there are lots of platforms on which single
> pin is shared by multiple devices which don't communicate so need some
> way of properly sharing access to a GPIO. What we have now is the
> GPIOD_FLAGS_BIT_NONEXCLUSIVE flag which was introduced as a hack and
> doesn't do any locking or arbitration of access - it literally just hand
> the same GPIO descriptor to all interested users.
>
> The proposed solution is composed of three major parts: the high-level,
> shared GPIO proxy driver that arbitrates access to the shared pin and
> exposes a regular GPIO chip interface to consumers, a low-level shared
> GPIOLIB module that scans firmware nodes and creates auxiliary devices
> that attach to the proxy driver and finally a set of core GPIOLIB
> changes that plug the former into the GPIO lookup path.
>
> The changes are implemented in a way that allows to seamlessly compile
> out any code related to sharing GPIOs for systems that don't need it.
>
> The practical use-case for this are the powerdown GPIOs shared by
> speakers on Qualcomm db845c platform, however I have also extensively
> tested it using gpio-virtuser on arm64 qemu with various DT
> configurations.
>
> I'm Cc'ing some people that may help with reviewing/be interested in
> this: OF maintainers (because the main target are OF systems initially),
> Mark Brown because most users of GPIOD_FLAGS_BIT_NONEXCLUSIVE live
> in audio or regulator drivers and one of the goals of this series is
> dropping the hand-crafted GPIO enable counting via struct
> regulator_enable_gpio in regulator core), Andy and Mika because I'd like
> to also cover ACPI (even though I don't know about any ACPI platform that
> would need this at the moment, I think it makes sense to make the
> solution complete), Dmitry (same thing but for software nodes), Mani
> (because you have a somewhat related use-case for the PERST# signal and
> I'd like to hear your input on whether this is something you can use or
> maybe it needs a separate, implicit gpio-perst driver similar to what
> Krzysztof did for reset-gpios) and Greg (because I mentioned this to you
> last week in person and I also use the auxiliary bus for the proxy
> devices).
>
> First patch in the series is a bugfix targetting stable, I'm surprised
> nobody noticed the lockdep splat yet. The second adds a library function
> I use in a later patch. All remaining patches implement or use the
> shared GPIO support.
>
Mark,
I was working on extending this series with patches that make the
regulator subsystem aware of shared GPIOs managed by GPIOLIB. I admit
that after our previous discussions I was under the impression that
the regulator core not only manages the enable count of the underlying
non-exclusive GPIOs but also emits the relevant
REGULATOR_EVENT_ENABLE/DISABLE notifications for GPIO-driven
regulators when the physical pin actually gets enabled/disabled
respectively.
Upon a closer inspection it turns out that this is not the case - the
ENABLE/DISABLE events are emitted when the *logical* regulator is
enabled/disabled even if this does not involve a change in the state
of the shared pin.
Example: the tlv320aic3x.c codec driver may use a fixed regulator that
uses a shared GPIO pin. The regulator in question may get disabled but
its GPIO might not change state as it's still enabled by a different
regulator. The driver will still see the disable event and put the
codec in reset.
I'm not saying this behavior is incorrect, I'm just mentioning it as I
thought that there's a need for some GPIO descriptor notifier that
allows to signal the actual change in value to users but it doesn't
seem to be the case if we just want to maintain the current behavior.
The only change we need to support the existing NONEXCLUSIVE flag and
the new shared GPIOs at the same time - until we convert all users -
is providing gpiod_is_shared() and just making regulator core skip the
descriptor comparison and referencing via struct regulator_enable_gpio
if it sees that a GPIO descriptor is shared.
I will post a v2 early next week.
Bartosz
^ permalink raw reply [flat|nested] 43+ messages in thread* Re: [PATCH RFC 0/9] gpio: improve support for shared GPIOs
2025-10-17 17:26 ` Bartosz Golaszewski
@ 2025-10-17 17:32 ` Mark Brown
2025-10-20 9:41 ` Bartosz Golaszewski
0 siblings, 1 reply; 43+ messages in thread
From: Mark Brown @ 2025-10-17 17:32 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: linux-hardening, linux-kernel, linux-gpio, linux-arm-kernel,
linux-sound, linux-arm-msm, Bartosz Golaszewski,
Srinivas Kandagatla, Kees Cook, Mika Westerberg, Conor Dooley,
Jaroslav Kysela, Catalin Marinas, Krzysztof Kozlowski,
Takashi Iwai, Rob Herring, Greg Kroah-Hartman, Andy Shevchenko,
Saravana Kannan, Will Deacon, Liam Girdwood, Andrew Morton,
Manivannan Sadhasivam, Linus Walleij, Dmitry Torokhov
[-- Attachment #1: Type: text/plain, Size: 381 bytes --]
On Fri, Oct 17, 2025 at 07:26:51PM +0200, Bartosz Golaszewski wrote:
> Upon a closer inspection it turns out that this is not the case - the
> ENABLE/DISABLE events are emitted when the *logical* regulator is
> enabled/disabled even if this does not involve a change in the state
> of the shared pin.
It really should be the actual physical state change that triggers the
event.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH RFC 0/9] gpio: improve support for shared GPIOs
2025-10-17 17:32 ` Mark Brown
@ 2025-10-20 9:41 ` Bartosz Golaszewski
2025-10-21 14:08 ` Mark Brown
0 siblings, 1 reply; 43+ messages in thread
From: Bartosz Golaszewski @ 2025-10-20 9:41 UTC (permalink / raw)
To: Mark Brown
Cc: linux-hardening, linux-kernel, linux-gpio, linux-arm-kernel,
linux-sound, linux-arm-msm, Bartosz Golaszewski,
Srinivas Kandagatla, Kees Cook, Mika Westerberg, Conor Dooley,
Jaroslav Kysela, Catalin Marinas, Krzysztof Kozlowski,
Takashi Iwai, Rob Herring, Greg Kroah-Hartman, Andy Shevchenko,
Saravana Kannan, Will Deacon, Liam Girdwood, Andrew Morton,
Manivannan Sadhasivam, Linus Walleij, Dmitry Torokhov
On Fri, Oct 17, 2025 at 7:32 PM Mark Brown <broonie@kernel.org> wrote:
>
> On Fri, Oct 17, 2025 at 07:26:51PM +0200, Bartosz Golaszewski wrote:
>
> > Upon a closer inspection it turns out that this is not the case - the
> > ENABLE/DISABLE events are emitted when the *logical* regulator is
> > enabled/disabled even if this does not involve a change in the state
> > of the shared pin.
>
> It really should be the actual physical state change that triggers the
> event.
I guess so, but this would require some non-trivial rework of the
regulator core. We'd need some list of deferred notifications stored
in memory for when the physical state actually changes.
Bartosz
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH RFC 0/9] gpio: improve support for shared GPIOs
2025-10-20 9:41 ` Bartosz Golaszewski
@ 2025-10-21 14:08 ` Mark Brown
2025-10-21 14:42 ` Bartosz Golaszewski
0 siblings, 1 reply; 43+ messages in thread
From: Mark Brown @ 2025-10-21 14:08 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: linux-hardening, linux-kernel, linux-gpio, linux-arm-kernel,
linux-sound, linux-arm-msm, Bartosz Golaszewski,
Srinivas Kandagatla, Kees Cook, Mika Westerberg, Conor Dooley,
Jaroslav Kysela, Catalin Marinas, Krzysztof Kozlowski,
Takashi Iwai, Rob Herring, Greg Kroah-Hartman, Andy Shevchenko,
Saravana Kannan, Will Deacon, Liam Girdwood, Andrew Morton,
Manivannan Sadhasivam, Linus Walleij, Dmitry Torokhov
[-- Attachment #1: Type: text/plain, Size: 652 bytes --]
On Mon, Oct 20, 2025 at 11:41:52AM +0200, Bartosz Golaszewski wrote:
> On Fri, Oct 17, 2025 at 7:32 PM Mark Brown <broonie@kernel.org> wrote:
> > It really should be the actual physical state change that triggers the
> > event.
> I guess so, but this would require some non-trivial rework of the
> regulator core. We'd need some list of deferred notifications stored
> in memory for when the physical state actually changes.
I'm not sure I see the need for deferred notifications? We'd need to go
round all the users whenever a physical change to the GPIO happens but
it's not clear what we'd need to store beyond the list of users?
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH RFC 0/9] gpio: improve support for shared GPIOs
2025-10-21 14:08 ` Mark Brown
@ 2025-10-21 14:42 ` Bartosz Golaszewski
2025-10-21 15:02 ` Mark Brown
0 siblings, 1 reply; 43+ messages in thread
From: Bartosz Golaszewski @ 2025-10-21 14:42 UTC (permalink / raw)
To: Mark Brown
Cc: linux-hardening, linux-kernel, linux-gpio, linux-arm-kernel,
linux-sound, linux-arm-msm, Bartosz Golaszewski,
Srinivas Kandagatla, Kees Cook, Mika Westerberg, Conor Dooley,
Jaroslav Kysela, Catalin Marinas, Krzysztof Kozlowski,
Takashi Iwai, Rob Herring, Greg Kroah-Hartman, Andy Shevchenko,
Saravana Kannan, Will Deacon, Liam Girdwood, Andrew Morton,
Manivannan Sadhasivam, Linus Walleij, Dmitry Torokhov
On Tue, Oct 21, 2025 at 4:08 PM Mark Brown <broonie@kernel.org> wrote:
>
> On Mon, Oct 20, 2025 at 11:41:52AM +0200, Bartosz Golaszewski wrote:
> > On Fri, Oct 17, 2025 at 7:32 PM Mark Brown <broonie@kernel.org> wrote:
>
> > > It really should be the actual physical state change that triggers the
> > > event.
>
> > I guess so, but this would require some non-trivial rework of the
> > regulator core. We'd need some list of deferred notifications stored
> > in memory for when the physical state actually changes.
>
> I'm not sure I see the need for deferred notifications? We'd need to go
> round all the users whenever a physical change to the GPIO happens but
> it's not clear what we'd need to store beyond the list of users?
In my mind I was thinking that we only need to send the notifications
to users who already enabled/disabled the regulator too but you're
right, it seems like a loop over the relevant pins should be enough.
In any case: this is outside the scope of this series but I'll see if
it's easy enough to add separately.
Bartosz
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH RFC 0/9] gpio: improve support for shared GPIOs
2025-10-21 14:42 ` Bartosz Golaszewski
@ 2025-10-21 15:02 ` Mark Brown
0 siblings, 0 replies; 43+ messages in thread
From: Mark Brown @ 2025-10-21 15:02 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: linux-hardening, linux-kernel, linux-gpio, linux-arm-kernel,
linux-sound, linux-arm-msm, Bartosz Golaszewski,
Srinivas Kandagatla, Kees Cook, Mika Westerberg, Conor Dooley,
Jaroslav Kysela, Catalin Marinas, Krzysztof Kozlowski,
Takashi Iwai, Rob Herring, Greg Kroah-Hartman, Andy Shevchenko,
Saravana Kannan, Will Deacon, Liam Girdwood, Andrew Morton,
Manivannan Sadhasivam, Linus Walleij, Dmitry Torokhov
[-- Attachment #1: Type: text/plain, Size: 897 bytes --]
On Tue, Oct 21, 2025 at 04:42:11PM +0200, Bartosz Golaszewski wrote:
> On Tue, Oct 21, 2025 at 4:08 PM Mark Brown <broonie@kernel.org> wrote:
> > I'm not sure I see the need for deferred notifications? We'd need to go
> > round all the users whenever a physical change to the GPIO happens but
> > it's not clear what we'd need to store beyond the list of users?
> In my mind I was thinking that we only need to send the notifications
> to users who already enabled/disabled the regulator too but you're
> right, it seems like a loop over the relevant pins should be enough.
Ah, great - yeah, I'd expect notifications either way just to be on the
safe side.
> In any case: this is outside the scope of this series but I'll see if
> it's easy enough to add separately.
Yes, definitely outside the scope of the series. If you do something
that'd be great but no pressure.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 43+ messages in thread