From a05543d6b05ba998fdbb4b383319ae5121bb7407 Mon Sep 17 00:00:00 2001 From: Bartosz Golaszewski Date: Wed, 10 Dec 2025 06:50:26 +0100 Subject: gpio: it87: balance superio enter/exit calls in error path We always call superio_enter() in it87_gpio_direction_out() but only call superio_exit() if the call to it87_gpio_set() succeeds. Move the label to balance the calls in error path as well. Fixes: ef877a159072 ("gpio: it87: use new line value setter callbacks") Reported-by: Daniel Gibson Closes: https://lore.kernel.org/all/bd0a00e3-9b8c-43e8-8772-e67b91f4c71e@gibson.sh/ Link: https://lore.kernel.org/r/20251210055026.23146-1-bartosz.golaszewski@oss.qualcomm.com Signed-off-by: Bartosz Golaszewski --- drivers/gpio/gpio-it87.c | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/drivers/gpio/gpio-it87.c b/drivers/gpio/gpio-it87.c index 5d677bcfccf2..2ad3c239367b 100644 --- a/drivers/gpio/gpio-it87.c +++ b/drivers/gpio/gpio-it87.c @@ -12,6 +12,7 @@ #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt +#include #include #include #include @@ -241,23 +242,17 @@ static int it87_gpio_direction_out(struct gpio_chip *chip, mask = 1 << (gpio_num % 8); group = (gpio_num / 8); - spin_lock(&it87_gpio->lock); + guard(spinlock)(&it87_gpio->lock); rc = superio_enter(); if (rc) - goto exit; + return rc; /* set the output enable bit */ superio_set_mask(mask, group + it87_gpio->output_base); rc = it87_gpio_set(chip, gpio_num, val); - if (rc) - goto exit; - superio_exit(); - -exit: - spin_unlock(&it87_gpio->lock); return rc; } -- cgit v1.2.3 From 9700b0fccf386b671e6f8401ddea6c5669cd0914 Mon Sep 17 00:00:00 2001 From: Bartosz Golaszewski Date: Mon, 22 Dec 2025 11:01:26 +0100 Subject: gpiolib: allow multiple lookup tables per consumer The GPIO machine lookup mechanism dates back to old ARM board files where lookup tables would all be defined in a single place. Since then, lookup tables have also been used to address various DT and ACPI quirks like missing GPIOs and - as of recently - setting up shared GPIO proxy devices. The lookup itself stops when we find the first matching table for a consumer and - if it doesn't contain the right entry - the lookup fails. Since the tables can now be defined in multiple places and we can't know how many there are, effectively limiting a consumer to only a single lookup table is no longer viable. Rework the code to always look through all tables matching the consumer. Link: https://lore.kernel.org/r/20251222-gpio-shared-reset-gpio-proxy-v1-1-8d4bba7d8c14@oss.qualcomm.com Signed-off-by: Bartosz Golaszewski --- drivers/gpio/gpiolib.c | 91 +++++++++++++++++++++++++++++++------------------- 1 file changed, 56 insertions(+), 35 deletions(-) diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c index 91e0c384f34a..6d583b3b07bb 100644 --- a/drivers/gpio/gpiolib.c +++ b/drivers/gpio/gpiolib.c @@ -4508,45 +4508,41 @@ void gpiod_remove_hogs(struct gpiod_hog *hogs) } EXPORT_SYMBOL_GPL(gpiod_remove_hogs); -static struct gpiod_lookup_table *gpiod_find_lookup_table(struct device *dev) +static bool gpiod_match_lookup_table(struct device *dev, + const struct gpiod_lookup_table *table) { const char *dev_id = dev ? dev_name(dev) : NULL; - struct gpiod_lookup_table *table; - list_for_each_entry(table, &gpio_lookup_list, list) { - if (table->dev_id && dev_id) { - /* - * Valid strings on both ends, must be identical to have - * a match - */ - if (!strcmp(table->dev_id, dev_id)) - return table; - } else { - /* - * One of the pointers is NULL, so both must be to have - * a match - */ - if (dev_id == table->dev_id) - return table; - } + lockdep_assert_held(&gpio_lookup_lock); + + if (table->dev_id && dev_id) { + /* + * Valid strings on both ends, must be identical to have + * a match + */ + if (!strcmp(table->dev_id, dev_id)) + return true; + } else { + /* + * One of the pointers is NULL, so both must be to have + * a match + */ + if (dev_id == table->dev_id) + return true; } - return NULL; + return false; } -static struct gpio_desc *gpiod_find(struct device *dev, const char *con_id, - unsigned int idx, unsigned long *flags) +static struct gpio_desc *gpio_desc_table_match(struct device *dev, const char *con_id, + unsigned int idx, unsigned long *flags, + struct gpiod_lookup_table *table) { - struct gpio_desc *desc = ERR_PTR(-ENOENT); - struct gpiod_lookup_table *table; + struct gpio_desc *desc; struct gpiod_lookup *p; struct gpio_chip *gc; - guard(mutex)(&gpio_lookup_lock); - - table = gpiod_find_lookup_table(dev); - if (!table) - return desc; + lockdep_assert_held(&gpio_lookup_lock); for (p = &table->table[0]; p->key; p++) { /* idx must always match exactly */ @@ -4600,6 +4596,29 @@ static struct gpio_desc *gpiod_find(struct device *dev, const char *con_id, return desc; } + return NULL; +} + +static struct gpio_desc *gpiod_find(struct device *dev, const char *con_id, + unsigned int idx, unsigned long *flags) +{ + struct gpio_desc *desc = ERR_PTR(-ENOENT); + struct gpiod_lookup_table *table; + + guard(mutex)(&gpio_lookup_lock); + + list_for_each_entry(table, &gpio_lookup_list, list) { + if (!gpiod_match_lookup_table(dev, table)) + continue; + + desc = gpio_desc_table_match(dev, con_id, idx, flags, table); + if (!desc) + continue; + + /* On IS_ERR() or match. */ + return desc; + } + return desc; } @@ -4610,14 +4629,16 @@ static int platform_gpio_count(struct device *dev, const char *con_id) unsigned int count = 0; scoped_guard(mutex, &gpio_lookup_lock) { - table = gpiod_find_lookup_table(dev); - if (!table) - return -ENOENT; + list_for_each_entry(table, &gpio_lookup_list, list) { + if (!gpiod_match_lookup_table(dev, table)) + continue; - for (p = &table->table[0]; p->key; p++) { - if ((con_id && p->con_id && !strcmp(con_id, p->con_id)) || - (!con_id && !p->con_id)) - count++; + for (p = &table->table[0]; p->key; p++) { + if ((con_id && p->con_id && + !strcmp(con_id, p->con_id)) || + (!con_id && !p->con_id)) + count++; + } } } -- cgit v1.2.3 From cb0451e33be047fff7137f58d9996370e11fb344 Mon Sep 17 00:00:00 2001 From: Bartosz Golaszewski Date: Mon, 22 Dec 2025 11:01:27 +0100 Subject: gpio: shared: verify con_id when adding proxy lookup When matching the firmware node with the potential consumer, we currently omit the con_id string. This can lead to false positives in the unlikely case of the consumer having been assigned more than one shared GPIO. Check the connector ID before proceeding. Fixes: a060b8c511ab ("gpiolib: implement low-level, shared GPIO support") Link: https://lore.kernel.org/r/20251222-gpio-shared-reset-gpio-proxy-v1-2-8d4bba7d8c14@oss.qualcomm.com Signed-off-by: Bartosz Golaszewski --- drivers/gpio/gpiolib-shared.c | 7 ++++++- drivers/gpio/gpiolib-shared.h | 4 +++- drivers/gpio/gpiolib.c | 3 ++- 3 files changed, 11 insertions(+), 3 deletions(-) diff --git a/drivers/gpio/gpiolib-shared.c b/drivers/gpio/gpiolib-shared.c index ba4b718d40a0..f589109590c7 100644 --- a/drivers/gpio/gpiolib-shared.c +++ b/drivers/gpio/gpiolib-shared.c @@ -365,7 +365,8 @@ static bool gpio_shared_dev_is_reset_gpio(struct device *consumer, } #endif /* CONFIG_RESET_GPIO */ -int gpio_shared_add_proxy_lookup(struct device *consumer, unsigned long lflags) +int gpio_shared_add_proxy_lookup(struct device *consumer, const char *con_id, + unsigned long lflags) { const char *dev_id = dev_name(consumer); struct gpio_shared_entry *entry; @@ -384,6 +385,10 @@ int gpio_shared_add_proxy_lookup(struct device *consumer, unsigned long lflags) guard(mutex)(&ref->lock); + if ((!con_id && ref->con_id) || (con_id && !ref->con_id) || + (con_id && ref->con_id && strcmp(con_id, ref->con_id) != 0)) + continue; + /* We've already done that on a previous request. */ if (ref->lookup) return 0; diff --git a/drivers/gpio/gpiolib-shared.h b/drivers/gpio/gpiolib-shared.h index 667dbdff3585..40568ef7364c 100644 --- a/drivers/gpio/gpiolib-shared.h +++ b/drivers/gpio/gpiolib-shared.h @@ -16,7 +16,8 @@ struct device; 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); +int gpio_shared_add_proxy_lookup(struct device *consumer, const char *con_id, + unsigned long lflags); #else @@ -28,6 +29,7 @@ static inline int gpio_device_setup_shared(struct gpio_device *gdev) static inline void gpio_device_teardown_shared(struct gpio_device *gdev) { } static inline int gpio_shared_add_proxy_lookup(struct device *consumer, + const char *con_id, unsigned long lflags) { return 0; diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c index 6d583b3b07bb..9ccfb7af67cc 100644 --- a/drivers/gpio/gpiolib.c +++ b/drivers/gpio/gpiolib.c @@ -4717,7 +4717,8 @@ struct gpio_desc *gpiod_find_and_request(struct device *consumer, * lookup table for the proxy device as previously * we only knew the consumer's fwnode. */ - ret = gpio_shared_add_proxy_lookup(consumer, lookupflags); + ret = gpio_shared_add_proxy_lookup(consumer, con_id, + lookupflags); if (ret) return ERR_PTR(ret); -- cgit v1.2.3 From 49416483a953662aa53c6d9bef651757d4a95ba5 Mon Sep 17 00:00:00 2001 From: Bartosz Golaszewski Date: Mon, 22 Dec 2025 11:01:28 +0100 Subject: gpio: shared: allow sharing a reset-gpios pin between reset-gpio and gpiolib We currently support sharing GPIOs between multiple devices whose drivers use either the GPIOLIB API *OR* the reset control API but not both at the same time. There's an unlikely corner-case where a reset-gpios pin can be shared by one driver using the GPIOLIB API and a second using the reset API. This will currently not work as the reset-gpio consumers are not described in firmware at the time of scanning so the shared GPIO just chooses one of the proxies created for the consumers when the reset-gpio driver performs the lookup. This can of course conflict in the case described above. In order to fix it: if we deal with the "reset-gpios" pin that's shared acconding to the firmware node setup, create a proxy for each described consumer as well as another one for the potential reset-gpio device. To that end: rework the matching to take this into account. Fixes: 7b78b26757e0 ("gpio: shared: handle the reset-gpios corner case") Link: https://lore.kernel.org/r/20251222-gpio-shared-reset-gpio-proxy-v1-3-8d4bba7d8c14@oss.qualcomm.com Signed-off-by: Bartosz Golaszewski --- drivers/gpio/gpiolib-shared.c | 182 ++++++++++++++++++++++++++++++------------ 1 file changed, 129 insertions(+), 53 deletions(-) diff --git a/drivers/gpio/gpiolib-shared.c b/drivers/gpio/gpiolib-shared.c index f589109590c7..baf7e07a3bb8 100644 --- a/drivers/gpio/gpiolib-shared.c +++ b/drivers/gpio/gpiolib-shared.c @@ -76,6 +76,56 @@ gpio_shared_find_entry(struct fwnode_handle *controller_node, return NULL; } +static struct gpio_shared_ref *gpio_shared_make_ref(struct fwnode_handle *fwnode, + const char *con_id, + enum gpiod_flags flags) +{ + char *con_id_cpy __free(kfree) = NULL; + + struct gpio_shared_ref *ref __free(kfree) = kzalloc(sizeof(*ref), GFP_KERNEL); + if (!ref) + return NULL; + + if (con_id) { + con_id_cpy = kstrdup(con_id, GFP_KERNEL); + if (!con_id_cpy) + return NULL; + } + + ref->dev_id = ida_alloc(&gpio_shared_ida, GFP_KERNEL); + if (ref->dev_id < 0) + return NULL; + + ref->flags = flags; + ref->con_id = no_free_ptr(con_id_cpy); + ref->fwnode = fwnode; + mutex_init(&ref->lock); + + return no_free_ptr(ref); +} + +static int gpio_shared_setup_reset_proxy(struct gpio_shared_entry *entry, + enum gpiod_flags flags) +{ + struct gpio_shared_ref *ref; + + list_for_each_entry(ref, &entry->refs, list) { + if (!ref->fwnode && ref->con_id && strcmp(ref->con_id, "reset") == 0) + return 0; + } + + ref = gpio_shared_make_ref(NULL, "reset", flags); + if (!ref) + return -ENOMEM; + + list_add_tail(&ref->list, &entry->refs); + + pr_debug("Created a secondary shared GPIO reference for potential reset-gpio device for GPIO %u at %s\n", + entry->offset, fwnode_get_name(entry->fwnode)); + + return 0; +} + /* Handle all special nodes that we should ignore. */ static bool gpio_shared_of_node_ignore(struct device_node *node) { @@ -106,6 +156,7 @@ static int gpio_shared_of_traverse(struct device_node *curr) size_t con_id_len, suffix_len; struct fwnode_handle *fwnode; struct of_phandle_args args; + struct gpio_shared_ref *ref; struct property *prop; unsigned int offset; const char *suffix; @@ -138,6 +189,7 @@ static int gpio_shared_of_traverse(struct device_node *curr) for (i = 0; i < count; i++) { struct device_node *np __free(device_node) = NULL; + char *con_id __free(kfree) = NULL; ret = of_parse_phandle_with_args(curr, prop->name, "#gpio-cells", i, @@ -182,15 +234,6 @@ static int gpio_shared_of_traverse(struct device_node *curr) list_add_tail(&entry->list, &gpio_shared_list); } - struct gpio_shared_ref *ref __free(kfree) = - kzalloc(sizeof(*ref), GFP_KERNEL); - if (!ref) - return -ENOMEM; - - ref->fwnode = fwnode_handle_get(of_fwnode_handle(curr)); - ref->flags = args.args[1]; - mutex_init(&ref->lock); - if (strends(prop->name, "gpios")) suffix = "-gpios"; else if (strends(prop->name, "gpio")) @@ -202,27 +245,32 @@ static int gpio_shared_of_traverse(struct device_node *curr) /* We only set con_id if there's actually one. */ if (strcmp(prop->name, "gpios") && strcmp(prop->name, "gpio")) { - ref->con_id = kstrdup(prop->name, GFP_KERNEL); - if (!ref->con_id) + con_id = kstrdup(prop->name, GFP_KERNEL); + if (!con_id) return -ENOMEM; - con_id_len = strlen(ref->con_id); + con_id_len = strlen(con_id); suffix_len = strlen(suffix); - ref->con_id[con_id_len - suffix_len] = '\0'; + con_id[con_id_len - suffix_len] = '\0'; } - ref->dev_id = ida_alloc(&gpio_shared_ida, GFP_KERNEL); - if (ref->dev_id < 0) { - kfree(ref->con_id); + ref = gpio_shared_make_ref(fwnode_handle_get(of_fwnode_handle(curr)), + con_id, args.args[1]); + if (!ref) return -ENOMEM; - } 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(&no_free_ptr(ref)->list, &entry->refs); + list_add_tail(&ref->list, &entry->refs); + + if (strcmp(prop->name, "reset-gpios") == 0) { + ret = gpio_shared_setup_reset_proxy(entry, args.args[1]); + if (ret) + return ret; + } } } @@ -306,20 +354,16 @@ static bool gpio_shared_dev_is_reset_gpio(struct device *consumer, struct fwnode_handle *reset_fwnode = dev_fwnode(consumer); struct fwnode_reference_args ref_args, aux_args; struct device *parent = consumer->parent; + struct gpio_shared_ref *real_ref; bool match; int ret; + lockdep_assert_held(&ref->lock); + /* The reset-gpio device must have a parent AND a firmware node. */ if (!parent || !reset_fwnode) return false; - /* - * FIXME: use device_is_compatible() once the reset-gpio drivers gains - * a compatible string which it currently does not have. - */ - if (!strstarts(dev_name(consumer), "reset.gpio.")) - return false; - /* * Parent of the reset-gpio auxiliary device is the GPIO chip whose * fwnode we stored in the entry structure. @@ -328,33 +372,56 @@ static bool gpio_shared_dev_is_reset_gpio(struct device *consumer, return false; /* - * The device associated with the shared reference's firmware node is - * the consumer of the reset control exposed by the reset-gpio device. - * It must have a "reset-gpios" property that's referencing the entry's - * firmware node. - * - * The reference args must agree between the real consumer and the - * auxiliary reset-gpio device. + * Now we need to find the actual pin we want to assign to this + * reset-gpio device. To that end: iterate over the list of references + * of this entry and see if there's one, whose reset-gpios property's + * arguments match the ones from this consumer's node. */ - ret = fwnode_property_get_reference_args(ref->fwnode, "reset-gpios", - NULL, 2, 0, &ref_args); - if (ret) - return false; + list_for_each_entry(real_ref, &entry->refs, list) { + if (!real_ref->fwnode) + continue; + + /* + * The device associated with the shared reference's firmware + * node is the consumer of the reset control exposed by the + * reset-gpio device. It must have a "reset-gpios" property + * that's referencing the entry's firmware node. + * + * The reference args must agree between the real consumer and + * the auxiliary reset-gpio device. + */ + ret = fwnode_property_get_reference_args(real_ref->fwnode, + "reset-gpios", + NULL, 2, 0, &ref_args); + if (ret) + continue; + + ret = fwnode_property_get_reference_args(reset_fwnode, "reset-gpios", + NULL, 2, 0, &aux_args); + if (ret) { + fwnode_handle_put(ref_args.fwnode); + continue; + } + + match = ((ref_args.fwnode == entry->fwnode) && + (aux_args.fwnode == entry->fwnode) && + (ref_args.args[0] == aux_args.args[0])); - ret = fwnode_property_get_reference_args(reset_fwnode, "reset-gpios", - NULL, 2, 0, &aux_args); - if (ret) { fwnode_handle_put(ref_args.fwnode); - return false; - } + fwnode_handle_put(aux_args.fwnode); + + if (!match) + continue; - match = ((ref_args.fwnode == entry->fwnode) && - (aux_args.fwnode == entry->fwnode) && - (ref_args.args[0] == aux_args.args[0])); + /* + * Reuse the fwnode of the real device, next time we'll use it + * in the normal path. + */ + ref->fwnode = fwnode_handle_get(real_ref->fwnode); + return true; + } - fwnode_handle_put(ref_args.fwnode); - fwnode_handle_put(aux_args.fwnode); - return match; + return false; } #else static bool gpio_shared_dev_is_reset_gpio(struct device *consumer, @@ -379,12 +446,20 @@ int gpio_shared_add_proxy_lookup(struct device *consumer, const char *con_id, list_for_each_entry(entry, &gpio_shared_list, list) { list_for_each_entry(ref, &entry->refs, list) { - if (!device_match_fwnode(consumer, ref->fwnode) && - !gpio_shared_dev_is_reset_gpio(consumer, entry, ref)) - continue; - guard(mutex)(&ref->lock); + /* + * FIXME: use device_is_compatible() once the reset-gpio + * drivers gains a compatible string which it currently + * does not have. + */ + if (!ref->fwnode && strstarts(dev_name(consumer), "reset.gpio.")) { + if (!gpio_shared_dev_is_reset_gpio(consumer, entry, ref)) + continue; + } else if (!device_match_fwnode(consumer, ref->fwnode)) { + continue; + } + if ((!con_id && ref->con_id) || (con_id && !ref->con_id) || (con_id && ref->con_id && strcmp(con_id, ref->con_id) != 0)) continue; @@ -471,8 +546,9 @@ int gpio_device_setup_shared(struct gpio_device *gdev) 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)); + pr_debug("Setting up a shared GPIO entry for %s (con_id: '%s')\n", + fwnode_get_name(ref->fwnode) ?: "(no fwnode)", + ref->con_id ?: "(none)"); ret = gpio_shared_make_adev(gdev, entry, ref); if (ret) -- cgit v1.2.3 From a7ac22d53d0990152b108c3f4fe30df45fcb0181 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Narewski?= Date: Wed, 24 Dec 2025 09:26:40 +0100 Subject: gpiolib: fix race condition for gdev->srcu MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If two drivers were calling gpiochip_add_data_with_key(), one may be traversing the srcu-protected list in gpio_name_to_desc(), meanwhile other has just added its gdev in gpiodev_add_to_list_unlocked(). This creates a non-mutexed and non-protected timeframe, when one instance is dereferencing and using &gdev->srcu, before the other has initialized it, resulting in crash: [ 4.935481] Unable to handle kernel paging request at virtual address ffff800272bcc000 [ 4.943396] Mem abort info: [ 4.943400] ESR = 0x0000000096000005 [ 4.943403] EC = 0x25: DABT (current EL), IL = 32 bits [ 4.943407] SET = 0, FnV = 0 [ 4.943410] EA = 0, S1PTW = 0 [ 4.943413] FSC = 0x05: level 1 translation fault [ 4.943416] Data abort info: [ 4.943418] ISV = 0, ISS = 0x00000005, ISS2 = 0x00000000 [ 4.946220] CM = 0, WnR = 0, TnD = 0, TagAccess = 0 [ 4.955261] GCS = 0, Overlay = 0, DirtyBit = 0, Xs = 0 [ 4.955268] swapper pgtable: 4k pages, 48-bit VAs, pgdp=0000000038e6c000 [ 4.961449] [ffff800272bcc000] pgd=0000000000000000 [ 4.969203] , p4d=1000000039739003 [ 4.979730] , pud=0000000000000000 [ 4.980210] phandle (CPU): 0x0000005e, phandle (BE): 0x5e000000 for node "reset" [ 4.991736] Internal error: Oops: 0000000096000005 [#1] PREEMPT SMP ... [ 5.121359] pc : __srcu_read_lock+0x44/0x98 [ 5.131091] lr : gpio_name_to_desc+0x60/0x1a0 [ 5.153671] sp : ffff8000833bb430 [ 5.298440] [ 5.298443] Call trace: [ 5.298445] __srcu_read_lock+0x44/0x98 [ 5.309484] gpio_name_to_desc+0x60/0x1a0 [ 5.320692] gpiochip_add_data_with_key+0x488/0xf00 5.946419] ---[ end trace 0000000000000000 ]--- Move initialization code for gdev fields before it is added to gpio_devices, with adjacent initialization code. Adjust goto statements to reflect modified order of operations Fixes: 47d8b4c1d868 ("gpio: add SRCU infrastructure to struct gpio_device") Reviewed-by: Jakub Lewalski Signed-off-by: Paweł Narewski [Bartosz: fixed a build issue, removed stray newline] Link: https://lore.kernel.org/r/20251224082641.10769-1-bartosz.golaszewski@oss.qualcomm.com Signed-off-by: Bartosz Golaszewski --- drivers/gpio/gpiolib.c | 40 ++++++++++++++++++++-------------------- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c index 9ccfb7af67cc..c06152b16dbc 100644 --- a/drivers/gpio/gpiolib.c +++ b/drivers/gpio/gpiolib.c @@ -1105,6 +1105,18 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data, gdev->ngpio = gc->ngpio; gdev->can_sleep = gc->can_sleep; + rwlock_init(&gdev->line_state_lock); + RAW_INIT_NOTIFIER_HEAD(&gdev->line_state_notifier); + BLOCKING_INIT_NOTIFIER_HEAD(&gdev->device_notifier); + + ret = init_srcu_struct(&gdev->srcu); + if (ret) + goto err_free_label; + + ret = init_srcu_struct(&gdev->desc_srcu); + if (ret) + goto err_cleanup_gdev_srcu; + scoped_guard(mutex, &gpio_devices_lock) { /* * TODO: this allocates a Linux GPIO number base in the global @@ -1119,7 +1131,7 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data, if (base < 0) { ret = base; base = 0; - goto err_free_label; + goto err_cleanup_desc_srcu; } /* @@ -1139,22 +1151,10 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data, ret = gpiodev_add_to_list_unlocked(gdev); if (ret) { gpiochip_err(gc, "GPIO integer space overlap, cannot add chip\n"); - goto err_free_label; + goto err_cleanup_desc_srcu; } } - rwlock_init(&gdev->line_state_lock); - RAW_INIT_NOTIFIER_HEAD(&gdev->line_state_notifier); - BLOCKING_INIT_NOTIFIER_HEAD(&gdev->device_notifier); - - ret = init_srcu_struct(&gdev->srcu); - if (ret) - goto err_remove_from_list; - - ret = init_srcu_struct(&gdev->desc_srcu); - if (ret) - goto err_cleanup_gdev_srcu; - #ifdef CONFIG_PINCTRL INIT_LIST_HEAD(&gdev->pin_ranges); #endif @@ -1164,11 +1164,11 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data, ret = gpiochip_set_names(gc); if (ret) - goto err_cleanup_desc_srcu; + goto err_remove_from_list; ret = gpiochip_init_valid_mask(gc); if (ret) - goto err_cleanup_desc_srcu; + goto err_remove_from_list; for (desc_index = 0; desc_index < gc->ngpio; desc_index++) { struct gpio_desc *desc = &gdev->descs[desc_index]; @@ -1248,10 +1248,6 @@ err_remove_of_chip: of_gpiochip_remove(gc); err_free_valid_mask: gpiochip_free_valid_mask(gc); -err_cleanup_desc_srcu: - cleanup_srcu_struct(&gdev->desc_srcu); -err_cleanup_gdev_srcu: - cleanup_srcu_struct(&gdev->srcu); err_remove_from_list: scoped_guard(mutex, &gpio_devices_lock) list_del_rcu(&gdev->list); @@ -1261,6 +1257,10 @@ err_remove_from_list: gpio_device_put(gdev); goto err_print_message; } +err_cleanup_desc_srcu: + cleanup_srcu_struct(&gdev->desc_srcu); +err_cleanup_gdev_srcu: + cleanup_srcu_struct(&gdev->srcu); err_free_label: kfree_const(gdev->label); err_free_descs: -- cgit v1.2.3 From 014a17deb41201449f76df2b20c857a9c3294a7c Mon Sep 17 00:00:00 2001 From: Ernest Van Hoecke Date: Wed, 17 Dec 2025 16:30:25 +0100 Subject: gpio: pca953x: handle short interrupt pulses on PCAL devices MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit GPIO drivers with latch input support may miss short pulses on input pins even when input latching is enabled. The generic interrupt logic in the pca953x driver reports interrupts by comparing the current input value against the previously sampled one and only signals an event when a level change is observed between two reads. For short pulses, the first edge is captured when the input register is read, but if the signal returns to its previous level before the read, the second edge is not observed. As a result, successive pulses can produce identical input values at read time and no level change is detected, causing interrupts to be missed. Below timing diagram shows this situation where the top signal is the input pin level and the bottom signal indicates the latched value. ─────┐ ┌──*───────────────┐ ┌──*─────────────────┐ ┌──*─── │ │ . │ │ . │ │ . │ │ │ │ │ │ │ │ │ └──*──┘ │ └──*──┘ │ └──*──┘ │ Input │ │ │ │ │ │ ▼ │ ▼ │ ▼ │ IRQ │ IRQ │ IRQ │ . . . ─────┐ .┌──────────────┐ .┌────────────────┐ .┌── │ │ │ │ │ │ │ │ │ │ │ │ └────────*┘ └────────*┘ └────────*┘ Latched │ │ │ ▼ ▼ ▼ READ 0 READ 0 READ 0 NO CHANGE NO CHANGE PCAL variants provide an interrupt status register that records which pins triggered an interrupt, but the status and input registers cannot be read atomically. The interrupt status is only cleared when the input port is read, and the input value must also be read to determine the triggering edge. If another interrupt occurs on a different line after the status register has been read but before the input register is sampled, that event will not be reflected in the earlier status snapshot, so relying solely on the interrupt status register is also insufficient. Support for input latching and interrupt status handling was previously added by [1], but the interrupt status-based logic was reverted by [2] due to these issues. This patch addresses the original problem by combining both sources of information. Events indicated by the interrupt status register are merged with events detected through the existing level-change logic. As a result: * short pulses, whose second edges are invisible, are detected via the interrupt status register, and * interrupts that occur between the status and input reads are still caught by the generic level-change logic. This significantly improves robustness on devices that signal interrupts as short pulses, while avoiding the issues that led to the earlier reversion. In practice, even if only the first edge of a pulse is observable, the interrupt is reliably detected. This fixes missed interrupts from an Ilitek touch controller with its interrupt line connected to a PCAL6416A, where active-low pulses are approximately 200 us long. [1] commit 44896beae605 ("gpio: pca953x: add PCAL9535 interrupt support for Galileo Gen2") [2] commit d6179f6c6204 ("gpio: pca953x: Improve interrupt support") Fixes: d6179f6c6204 ("gpio: pca953x: Improve interrupt support") Signed-off-by: Ernest Van Hoecke Reviewed-by: Andy Shevchenko Link: https://lore.kernel.org/r/20251217153050.142057-1-ernestvanhoecke@gmail.com Signed-off-by: Bartosz Golaszewski --- drivers/gpio/gpio-pca953x.c | 25 ++++++++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/drivers/gpio/gpio-pca953x.c b/drivers/gpio/gpio-pca953x.c index 0a3916cc2772..8727ae54bc57 100644 --- a/drivers/gpio/gpio-pca953x.c +++ b/drivers/gpio/gpio-pca953x.c @@ -943,14 +943,35 @@ static bool pca953x_irq_pending(struct pca953x_chip *chip, unsigned long *pendin DECLARE_BITMAP(old_stat, MAX_LINE); DECLARE_BITMAP(cur_stat, MAX_LINE); DECLARE_BITMAP(new_stat, MAX_LINE); + DECLARE_BITMAP(int_stat, MAX_LINE); DECLARE_BITMAP(trigger, MAX_LINE); DECLARE_BITMAP(edges, MAX_LINE); int ret; + if (chip->driver_data & PCA_PCAL) { + /* Read INT_STAT before it is cleared by the input-port read. */ + ret = pca953x_read_regs(chip, PCAL953X_INT_STAT, int_stat); + if (ret) + return false; + } + ret = pca953x_read_regs(chip, chip->regs->input, cur_stat); if (ret) return false; + if (chip->driver_data & PCA_PCAL) { + /* Detect short pulses via INT_STAT. */ + bitmap_and(trigger, int_stat, chip->irq_mask, gc->ngpio); + + /* Apply filter for rising/falling edge selection. */ + bitmap_replace(new_stat, chip->irq_trig_fall, chip->irq_trig_raise, + cur_stat, gc->ngpio); + + bitmap_and(int_stat, new_stat, trigger, gc->ngpio); + } else { + bitmap_zero(int_stat, gc->ngpio); + } + /* Remove output pins from the equation */ pca953x_read_regs(chip, chip->regs->direction, reg_direction); @@ -964,7 +985,8 @@ static bool pca953x_irq_pending(struct pca953x_chip *chip, unsigned long *pendin if (bitmap_empty(chip->irq_trig_level_high, gc->ngpio) && bitmap_empty(chip->irq_trig_level_low, gc->ngpio)) { - if (bitmap_empty(trigger, gc->ngpio)) + if (bitmap_empty(trigger, gc->ngpio) && + bitmap_empty(int_stat, gc->ngpio)) return false; } @@ -972,6 +994,7 @@ static bool pca953x_irq_pending(struct pca953x_chip *chip, unsigned long *pendin bitmap_and(old_stat, chip->irq_trig_raise, new_stat, gc->ngpio); bitmap_or(edges, old_stat, cur_stat, gc->ngpio); bitmap_and(pending, edges, trigger, gc->ngpio); + bitmap_or(pending, pending, int_stat, gc->ngpio); bitmap_and(cur_stat, new_stat, chip->irq_trig_level_high, gc->ngpio); bitmap_and(cur_stat, cur_stat, chip->irq_mask, gc->ngpio); -- cgit v1.2.3 From 1e876e5a0875e71e34148c9feb2eedd3bf6b2b43 Mon Sep 17 00:00:00 2001 From: Abdun Nihaal Date: Fri, 26 Dec 2025 11:34:10 +0530 Subject: gpio: mpsse: fix reference leak in gpio_mpsse_probe() error paths The reference obtained by calling usb_get_dev() is not released in the gpio_mpsse_probe() error paths. Fix that by using device managed helper functions. Also remove the usb_put_dev() call in the disconnect function since now it will be released automatically. Cc: stable@vger.kernel.org Fixes: c46a74ff05c0 ("gpio: add support for FTDI's MPSSE as GPIO") Signed-off-by: Abdun Nihaal Link: https://lore.kernel.org/r/20251226060414.20785-1-nihaal@cse.iitm.ac.in Signed-off-by: Bartosz Golaszewski --- drivers/gpio/gpio-mpsse.c | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/drivers/gpio/gpio-mpsse.c b/drivers/gpio/gpio-mpsse.c index ace652ba4df1..12191aeb6566 100644 --- a/drivers/gpio/gpio-mpsse.c +++ b/drivers/gpio/gpio-mpsse.c @@ -548,6 +548,13 @@ static void gpio_mpsse_ida_remove(void *data) ida_free(&gpio_mpsse_ida, priv->id); } +static void gpio_mpsse_usb_put_dev(void *data) +{ + struct mpsse_priv *priv = data; + + usb_put_dev(priv->udev); +} + static int mpsse_init_valid_mask(struct gpio_chip *chip, unsigned long *valid_mask, unsigned int ngpios) @@ -592,6 +599,10 @@ static int gpio_mpsse_probe(struct usb_interface *interface, INIT_LIST_HEAD(&priv->workers); priv->udev = usb_get_dev(interface_to_usbdev(interface)); + err = devm_add_action_or_reset(dev, gpio_mpsse_usb_put_dev, priv); + if (err) + return err; + priv->intf = interface; priv->intf_id = interface->cur_altsetting->desc.bInterfaceNumber; @@ -713,7 +724,6 @@ static void gpio_mpsse_disconnect(struct usb_interface *intf) priv->intf = NULL; usb_set_intfdata(intf, NULL); - usb_put_dev(priv->udev); } static struct usb_driver gpio_mpsse_driver = { -- cgit v1.2.3 From 20cf2aed89ac6d78a0122e31c875228e15247194 Mon Sep 17 00:00:00 2001 From: Bartosz Golaszewski Date: Tue, 6 Jan 2026 10:00:11 +0100 Subject: gpio: rockchip: mark the GPIO controller as sleeping The GPIO controller is configured as non-sleeping but it uses generic pinctrl helpers which use a mutex for synchronization. This can cause the following lockdep splat with shared GPIOs enabled on boards which have multiple devices using the same GPIO: BUG: sleeping function called from invalid context at kernel/locking/mutex.c:591 in_atomic(): 1, irqs_disabled(): 1, non_block: 0, pid: 12, name: kworker/u16:0 preempt_count: 1, expected: 0 RCU nest depth: 0, expected: 0 6 locks held by kworker/u16:0/12: #0: ffff0001f0018d48 ((wq_completion)events_unbound#2){+.+.}-{0:0}, at: process_one_work+0x18c/0x604 #1: ffff8000842dbdf0 (deferred_probe_work){+.+.}-{0:0}, at: process_one_work+0x1b4/0x604 #2: ffff0001f18498f8 (&dev->mutex){....}-{4:4}, at: __device_attach+0x38/0x1b0 #3: ffff0001f75f1e90 (&gdev->srcu){.+.?}-{0:0}, at: gpiod_direction_output_raw_commit+0x0/0x360 #4: ffff0001f46e3db8 (&shared_desc->spinlock){....}-{3:3}, at: gpio_shared_proxy_direction_output+0xd0/0x144 [gpio_shared_proxy] #5: ffff0001f180ee90 (&gdev->srcu){.+.?}-{0:0}, at: gpiod_direction_output_raw_commit+0x0/0x360 irq event stamp: 81450 hardirqs last enabled at (81449): [] _raw_spin_unlock_irqrestore+0x74/0x78 hardirqs last disabled at (81450): [] _raw_spin_lock_irqsave+0x84/0x88 softirqs last enabled at (79616): [] __alloc_skb+0x17c/0x1e8 softirqs last disabled at (79614): [] __alloc_skb+0x17c/0x1e8 CPU: 2 UID: 0 PID: 12 Comm: kworker/u16:0 Not tainted 6.19.0-rc4-next-20260105+ #11975 PREEMPT Hardware name: Hardkernel ODROID-M1 (DT) Workqueue: events_unbound deferred_probe_work_func Call trace: show_stack+0x18/0x24 (C) dump_stack_lvl+0x90/0xd0 dump_stack+0x18/0x24 __might_resched+0x144/0x248 __might_sleep+0x48/0x98 __mutex_lock+0x5c/0x894 mutex_lock_nested+0x24/0x30 pinctrl_get_device_gpio_range+0x44/0x128 pinctrl_gpio_direction+0x3c/0xe0 pinctrl_gpio_direction_output+0x14/0x20 rockchip_gpio_direction_output+0xb8/0x19c gpiochip_direction_output+0x38/0x94 gpiod_direction_output_raw_commit+0x1d8/0x360 gpiod_direction_output_nonotify+0x7c/0x230 gpiod_direction_output+0x34/0xf8 gpio_shared_proxy_direction_output+0xec/0x144 [gpio_shared_proxy] gpiochip_direction_output+0x38/0x94 gpiod_direction_output_raw_commit+0x1d8/0x360 gpiod_direction_output_nonotify+0x7c/0x230 gpiod_configure_flags+0xbc/0x480 gpiod_find_and_request+0x1a0/0x574 gpiod_get_index+0x58/0x84 devm_gpiod_get_index+0x20/0xb4 devm_gpiod_get_optional+0x18/0x30 rockchip_pcie_probe+0x98/0x380 platform_probe+0x5c/0xac really_probe+0xbc/0x298 Fixes: 936ee2675eee ("gpio/rockchip: add driver for rockchip gpio") Cc: stable@vger.kernel.org Reported-by: Marek Szyprowski Closes: https://lore.kernel.org/all/d035fc29-3b03-4cd6-b8ec-001f93540bc6@samsung.com/ Acked-by: Heiko Stuebner Link: https://lore.kernel.org/r/20260106090011.21603-1-bartosz.golaszewski@oss.qualcomm.com Signed-off-by: Bartosz Golaszewski --- drivers/gpio/gpio-rockchip.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/gpio/gpio-rockchip.c b/drivers/gpio/gpio-rockchip.c index 47174eb3ba76..bae2061f15fc 100644 --- a/drivers/gpio/gpio-rockchip.c +++ b/drivers/gpio/gpio-rockchip.c @@ -593,6 +593,7 @@ static int rockchip_gpiolib_register(struct rockchip_pin_bank *bank) gc->ngpio = bank->nr_pins; gc->label = bank->name; gc->parent = bank->dev; + gc->can_sleep = true; ret = gpiochip_add_data(gc, bank); if (ret) { -- cgit v1.2.3 From 0fe50631791bd3504dc7f32af6421bd4041f14aa Mon Sep 17 00:00:00 2001 From: Bartosz Golaszewski Date: Tue, 6 Jan 2026 10:34:21 +0100 Subject: gpio: shared: assign the correct firmware node for reset-gpio use-case When we defer probe due to unlucky timing of adding the lookup table, we assign the matching firmware node to the shared reference for the future probing. However, the fwnode we assign is wrong so fix it and assign the one associated with the reset-gpio device. Fixes: 49416483a953 ("gpio: shared: allow sharing a reset-gpios pin between reset-gpio and gpiolib") Reported-by: Marek Szyprowski Closes: https://lore.kernel.org/all/00107523-7737-4b92-a785-14ce4e93b8cb@samsung.com/ Tested-by: Mark Brown Link: https://lore.kernel.org/r/20260106-gpio-shared-fixes-v2-1-c7091d2f7581@oss.qualcomm.com Signed-off-by: Bartosz Golaszewski --- drivers/gpio/gpiolib-shared.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpio/gpiolib-shared.c b/drivers/gpio/gpiolib-shared.c index baf7e07a3bb8..a68af06a6cc4 100644 --- a/drivers/gpio/gpiolib-shared.c +++ b/drivers/gpio/gpiolib-shared.c @@ -417,7 +417,7 @@ static bool gpio_shared_dev_is_reset_gpio(struct device *consumer, * Reuse the fwnode of the real device, next time we'll use it * in the normal path. */ - ref->fwnode = fwnode_handle_get(real_ref->fwnode); + ref->fwnode = fwnode_handle_get(reset_fwnode); return true; } -- cgit v1.2.3 From 476e44d06fc107f8cd99695d8e4f1c792dfc3379 Mon Sep 17 00:00:00 2001 From: Bartosz Golaszewski Date: Tue, 6 Jan 2026 10:34:22 +0100 Subject: gpio: shared: fix a race condition When matching the reset-gpio reference with the actual firmware node consuming the GPIO, we also need to lock the structure associated with the latter as it can change while we're doing it. Due to triggering lockdep false-positives, we need to use a per-reference lockdep class but accidentally, this also allows us to remove the previous lockdep workaround for cleaner code. Fixes: 49416483a953 ("gpio: shared: allow sharing a reset-gpios pin between reset-gpio and gpiolib") Reported-by: Marek Szyprowski Closes: https://lore.kernel.org/all/00107523-7737-4b92-a785-14ce4e93b8cb@samsung.com/ Tested-by: Mark Brown Link: https://lore.kernel.org/r/20260106-gpio-shared-fixes-v2-2-c7091d2f7581@oss.qualcomm.com Signed-off-by: Bartosz Golaszewski --- drivers/gpio/gpiolib-shared.c | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/drivers/gpio/gpiolib-shared.c b/drivers/gpio/gpiolib-shared.c index a68af06a6cc4..4c57b0928760 100644 --- a/drivers/gpio/gpiolib-shared.c +++ b/drivers/gpio/gpiolib-shared.c @@ -38,6 +38,7 @@ struct gpio_shared_ref { int dev_id; /* Protects the auxiliary device struct and the lookup table. */ struct mutex lock; + struct lock_class_key lock_key; struct auxiliary_device adev; struct gpiod_lookup_table *lookup; }; @@ -99,7 +100,8 @@ static struct gpio_shared_ref *gpio_shared_make_ref(struct fwnode_handle *fwnode ref->flags = flags; ref->con_id = no_free_ptr(con_id_cpy); ref->fwnode = fwnode; - mutex_init(&ref->lock); + lockdep_register_key(&ref->lock_key); + mutex_init_with_key(&ref->lock, &ref->lock_key); return no_free_ptr(ref); } @@ -378,6 +380,11 @@ static bool gpio_shared_dev_is_reset_gpio(struct device *consumer, * arguments match the ones from this consumer's node. */ list_for_each_entry(real_ref, &entry->refs, list) { + if (real_ref == ref) + continue; + + guard(mutex)(&real_ref->lock); + if (!real_ref->fwnode) continue; @@ -568,15 +575,6 @@ void gpio_device_teardown_shared(struct gpio_device *gdev) if (!device_match_fwnode(&gdev->dev, entry->fwnode)) continue; - /* - * For some reason if we call synchronize_srcu() in GPIO core, - * descent here and take this mutex and then recursively call - * synchronize_srcu() again from gpiochip_remove() (which is - * totally fine) called after gpio_shared_remove_adev(), - * lockdep prints a false positive deadlock splat. Disable - * lockdep here. - */ - lockdep_off(); list_for_each_entry(ref, &entry->refs, list) { guard(mutex)(&ref->lock); @@ -589,7 +587,6 @@ void gpio_device_teardown_shared(struct gpio_device *gdev) gpio_shared_remove_adev(&ref->adev); } - lockdep_on(); } } @@ -685,6 +682,7 @@ static void gpio_shared_drop_ref(struct gpio_shared_ref *ref) { list_del(&ref->list); mutex_destroy(&ref->lock); + lockdep_unregister_key(&ref->lock_key); kfree(ref->con_id); ida_free(&gpio_shared_ida, ref->dev_id); fwnode_handle_put(ref->fwnode); -- cgit v1.2.3 From a80208072df8f4ceb53cd905c1f4362f84ce397f Mon Sep 17 00:00:00 2001 From: Bartosz Golaszewski Date: Tue, 6 Jan 2026 10:34:23 +0100 Subject: gpio: shared: don't allocate the lookup table until we really need it We allocate memory for the GPIO lookup table at the top of gpio_shared_add_proxy_lookup() but we don't use it until the very end. Depending on the timing, we may return earlier. Move the allocation towards the end. Fixes: a060b8c511ab ("gpiolib: implement low-level, shared GPIO support") Tested-by: Mark Brown Link: https://lore.kernel.org/r/20260106-gpio-shared-fixes-v2-3-c7091d2f7581@oss.qualcomm.com Signed-off-by: Bartosz Golaszewski --- drivers/gpio/gpiolib-shared.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/drivers/gpio/gpiolib-shared.c b/drivers/gpio/gpiolib-shared.c index 4c57b0928760..076d8642675c 100644 --- a/drivers/gpio/gpiolib-shared.c +++ b/drivers/gpio/gpiolib-shared.c @@ -443,14 +443,10 @@ int gpio_shared_add_proxy_lookup(struct device *consumer, const char *con_id, unsigned long lflags) { const char *dev_id = dev_name(consumer); + struct gpiod_lookup_table *lookup; 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; - list_for_each_entry(entry, &gpio_shared_list, list) { list_for_each_entry(ref, &entry->refs, list) { guard(mutex)(&ref->lock); @@ -482,6 +478,10 @@ int gpio_shared_add_proxy_lookup(struct device *consumer, const char *con_id, if (!key) return -ENOMEM; + lookup = kzalloc(struct_size(lookup, table, 2), GFP_KERNEL); + if (!lookup) + 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 ?: "none"); @@ -489,7 +489,7 @@ int gpio_shared_add_proxy_lookup(struct device *consumer, const char *con_id, lookup->table[0] = GPIO_LOOKUP(no_free_ptr(key), 0, ref->con_id, lflags); - ref->lookup = no_free_ptr(lookup); + ref->lookup = lookup; gpiod_add_lookup_table(ref->lookup); return 0; -- cgit v1.2.3 From 36f597bba049928004a050c132ab787ba0eba524 Mon Sep 17 00:00:00 2001 From: Bartosz Golaszewski Date: Thu, 8 Jan 2026 11:23:14 +0100 Subject: gpiolib: fix lookup table matching If on any iteration in gpiod_find(), gpio_desc_table_match() returns NULL (which is normal and expected), we never reinitialize desc back to ERR_PTR(-ENOENT) and if we don't find a match later on, we will return NULL causing a NULL-pointer dereference in users not expecting it. Don't initialize desc, but return ERR_PTR(-ENOENT) explicitly at the end of the function. Fixes: 9700b0fccf38 ("gpiolib: allow multiple lookup tables per consumer") Reported-by: Marek Szyprowski Closes: https://lore.kernel.org/all/00107523-7737-4b92-a785-14ce4e93b8cb@samsung.com/ Tested-by: Marek Szyprowski Link: https://lore.kernel.org/r/20260108102314.18816-1-bartosz.golaszewski@oss.qualcomm.com Signed-off-by: Bartosz Golaszewski --- drivers/gpio/gpiolib.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c index c06152b16dbc..dcf427d3cf43 100644 --- a/drivers/gpio/gpiolib.c +++ b/drivers/gpio/gpiolib.c @@ -4602,8 +4602,8 @@ static struct gpio_desc *gpio_desc_table_match(struct device *dev, const char *c static struct gpio_desc *gpiod_find(struct device *dev, const char *con_id, unsigned int idx, unsigned long *flags) { - struct gpio_desc *desc = ERR_PTR(-ENOENT); struct gpiod_lookup_table *table; + struct gpio_desc *desc; guard(mutex)(&gpio_lookup_lock); @@ -4619,7 +4619,7 @@ static struct gpio_desc *gpiod_find(struct device *dev, const char *con_id, return desc; } - return desc; + return ERR_PTR(-ENOENT); } static int platform_gpio_count(struct device *dev, const char *con_id) -- cgit v1.2.3 From d578b31856cec31315f27b3ba97b212e4c6989b3 Mon Sep 17 00:00:00 2001 From: Bartosz Golaszewski Date: Thu, 8 Jan 2026 14:39:19 +0100 Subject: gpio: shared: fix a false-positive sharing detection with reset-gpios After scanning the devicetree, we remove all entries that have only one reference, while creating GPIO shared proxies for the remaining, shared entries. However: for the reset-gpio corner-case, we will have two references for a "reset-gpios" pin that's not really shared. In this case one will come from the actual consumer fwnode and the other from the potential auxiliary reset-gpio device. This causes the GPIO core to create unnecessary GPIO shared proxy devices for pins that are not really shared. Add a function that can detect this situation and remove entries that have exactly two references but one of them is a reset-gpio. Fixes: 7b78b26757e0 ("gpio: shared: handle the reset-gpios corner case") Link: https://lore.kernel.org/r/20260108-gpio-shared-false-positive-v1-1-5dbf8d1b2f7d@oss.qualcomm.com Signed-off-by: Bartosz Golaszewski --- drivers/gpio/gpiolib-shared.c | 34 ++++++++++++++++++++++++++++++++-- 1 file changed, 32 insertions(+), 2 deletions(-) diff --git a/drivers/gpio/gpiolib-shared.c b/drivers/gpio/gpiolib-shared.c index 076d8642675c..17343fdc9758 100644 --- a/drivers/gpio/gpiolib-shared.c +++ b/drivers/gpio/gpiolib-shared.c @@ -41,6 +41,7 @@ struct gpio_shared_ref { struct lock_class_key lock_key; struct auxiliary_device adev; struct gpiod_lookup_table *lookup; + bool is_reset_gpio; }; /* Represents a single GPIO pin. */ @@ -112,7 +113,8 @@ static int gpio_shared_setup_reset_proxy(struct gpio_shared_entry *entry, struct gpio_shared_ref *ref; list_for_each_entry(ref, &entry->refs, list) { - if (!ref->fwnode && ref->con_id && strcmp(ref->con_id, "reset") == 0) + if (ref->is_reset_gpio) + /* Already set-up. */ return 0; } @@ -120,6 +122,8 @@ static int gpio_shared_setup_reset_proxy(struct gpio_shared_entry *entry, if (!ref) return -ENOMEM; + ref->is_reset_gpio = true; + list_add_tail(&ref->list, &entry->refs); pr_debug("Created a secondary shared GPIO reference for potential reset-gpio device for GPIO %u at %s\n", @@ -714,12 +718,38 @@ static void __init gpio_shared_teardown(void) } } +static bool gpio_shared_entry_is_really_shared(struct gpio_shared_entry *entry) +{ + size_t num_nodes = list_count_nodes(&entry->refs); + struct gpio_shared_ref *ref; + + if (num_nodes <= 1) + return false; + + if (num_nodes > 2) + return true; + + /* Exactly two references: */ + list_for_each_entry(ref, &entry->refs, list) { + /* + * Corner-case: the second reference comes from the potential + * reset-gpio instance. However, this pin is not really shared + * as it would have three references in this case. Avoid + * creating unnecessary proxies. + */ + if (ref->is_reset_gpio) + return false; + } + + return true; +} + static void gpio_shared_free_exclusive(void) { struct gpio_shared_entry *entry, *epos; list_for_each_entry_safe(entry, epos, &gpio_shared_list, list) { - if (list_count_nodes(&entry->refs) > 1) + if (gpio_shared_entry_is_really_shared(entry)) continue; gpio_shared_drop_ref(list_first_entry(&entry->refs, -- cgit v1.2.3