| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-Id: <20251125-gpiolib-shared-reset-gpio-fix-v2-1-4eb6fa41f1dd@linaro.org>
Date: Tue, 25 Nov 2025 11:19:09 +0100
From: Bartosz Golaszewski <brgl@...ev.pl>
To: Val Packett <val@...kett.cool>, Krzysztof Kozlowski <krzk@...nel.org>,
Philipp Zabel <p.zabel@...gutronix.de>,
Linus Walleij <linus.walleij@...aro.org>,
Bartosz Golaszewski <brgl@...nel.org>
Cc: linux-gpio@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-arm-msm@...r.kernel.org,
Bartosz Golaszewski <bartosz.golaszewski@...aro.org>
Subject: [PATCH v2] gpio: shared: handle the reset-gpios corner case
From: Bartosz Golaszewski <bartosz.golaszewski@...aro.org>
There's an unexpected interaction between the reset-gpio driver and the
shared GPIO support. The reset-gpio device is an auxiliary device that's
created dynamically and fulfills a similar role to the gpio-shared-proxy
driver but is limited in scope to just supporting the "reset-gpios"
property.
The shared GPIO core code does not take into account that the machine
lookup entry we create when scanning the device-tree must connect the
reset-gpio device - that is the actual consumer of the GPIO and not the
consumer defined on the device tree, which in turn consumes the shared
reset control exposed by the reset-gpio device - to the GPIO controller.
We also must not skip the gpio-shared-proxy driver as it's possible that
a shared GPIO may be used by one consumer as a reset-gpios going through
the reset-gpio device and another that uses GPIOLIB.
We need to make it a special case handled in gpiolib-shared.c. Add a new
function - gpio_shared_dev_is_reset_gpio() - whose role it is to verify
if a non-matching consumer of a shared pin is a reset-gpio device and
make sure it's the right one for this pin. To that end make sure that
its parent is the GPIO controller in question and that the fwnode we
identified as sharing the pin references that controller via the
"reset-gpios" property.
Only include that code if the reset-gpio driver is enabled.
Fixes: a060b8c511ab ("gpiolib: implement low-level, shared GPIO support")
Reported-by: Val Packett <val@...kett.cool>
Closes: https://lore.kernel.org/all/3b5d9df5-934d-4591-8827-6c9573a6f7ba@packett.cool/
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@...aro.org>
---
Changes in v2:
- Make the reset-gpio checks stricter to cover use-cases with multiple
reset-gpio devices using pins from the same controller
- Link to v1: https://lore.kernel.org/r/20251121-gpiolib-shared-reset-gpio-fix-v1-1-cf0128fe4b47@linaro.org
---
drivers/gpio/gpiolib-shared.c | 81 ++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 80 insertions(+), 1 deletion(-)
diff --git a/drivers/gpio/gpiolib-shared.c b/drivers/gpio/gpiolib-shared.c
index 3803b5c938f9933dab01c6d777c349ed3b42ce9b..cc8646f563d2fa7b1972c94c734b19c4cb01244c 100644
--- a/drivers/gpio/gpiolib-shared.c
+++ b/drivers/gpio/gpiolib-shared.c
@@ -253,6 +253,84 @@ static int gpio_shared_make_adev(struct gpio_device *gdev,
return 0;
}
+#if IS_ENABLED(CONFIG_RESET_GPIO)
+/*
+ * Special case: reset-gpio is an auxiliary device that's created dynamically
+ * and put in between the GPIO controller and consumers of shared GPIOs
+ * referred to by the "reset-gpios" property.
+ *
+ * If the supposed consumer of a shared GPIO didn't match any of the mappings
+ * we created when scanning the firmware nodes, it's still possible that it's
+ * the reset-gpio device which didn't exist at the time of the scan.
+ *
+ * This function verifies it an return true if it's the case.
+ */
+static bool gpio_shared_dev_is_reset_gpio(struct device *consumer,
+ struct gpio_shared_entry *entry,
+ struct gpio_shared_ref *ref)
+{
+ struct fwnode_handle *reset_fwnode = dev_fwnode(consumer);
+ struct fwnode_reference_args ref_args, aux_args;
+ struct device *parent = consumer->parent;
+ bool match;
+ int ret;
+
+ /* 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.
+ */
+ if (!device_match_fwnode(parent, entry->fwnode))
+ 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.
+ */
+ ret = fwnode_property_get_reference_args(ref->fwnode, "reset-gpios",
+ NULL, 2, 0, &ref_args);
+ if (ret)
+ return false;
+
+ 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;
+ }
+
+ match = ((ref_args.fwnode == entry->fwnode) &&
+ (aux_args.fwnode == entry->fwnode) &&
+ (ref_args.args[0] == aux_args.args[0]));
+
+ fwnode_handle_put(ref_args.fwnode);
+ fwnode_handle_put(aux_args.fwnode);
+ return match;
+}
+#else
+static bool gpio_shared_dev_is_reset_gpio(struct device *consumer,
+ struct gpio_shared_entry *entry,
+ struct gpio_shared_ref *ref)
+{
+ return false;
+}
+#endif /* CONFIG_RESET_GPIO */
+
int gpio_shared_add_proxy_lookup(struct device *consumer, unsigned long lflags)
{
const char *dev_id = dev_name(consumer);
@@ -268,7 +346,8 @@ int gpio_shared_add_proxy_lookup(struct device *consumer, unsigned long lflags)
list_for_each_entry(entry, &gpio_shared_list, list) {
list_for_each_entry(ref, &entry->refs, list) {
- if (!device_match_fwnode(consumer, ref->fwnode))
+ if (!device_match_fwnode(consumer, ref->fwnode) &&
+ !gpio_shared_dev_is_reset_gpio(consumer, entry, ref))
continue;
/* We've already done that on a previous request. */
---
base-commit: 71dfa12e37df94917955574484b122abaf0e7bf5
change-id: 20251121-gpiolib-shared-reset-gpio-fix-c8e161b9f6cb
Best regards,
--
Bartosz Golaszewski <bartosz.golaszewski@...aro.org>
Powered by blists - more mailing lists