[PATCH v4 0/6] gpio: Refactor and add selftest

[PATCH v4 0/6] gpio: Refactor and add selftest

[Tzung-Bi Shih]

The series is separated from v3 to lessen the burden on the subsequent
revocable work per suggestion in
https://lore.kernel.org/all/CAMRc=MfQumD1ULx7yU4W2sx=35wyQf7-v4tSf44OqEu3JDBUAg@mail.gmail.com/.

The series is based on v7.0-rc1 and applies after
https://lore.kernel.org/all/20260205092840.2574840-1-tzungbi@kernel.org.

---
v4:
- Separate the first 6 patches from v3.

v3: https://lore.kernel.org/all/20260213092958.864411-1-tzungbi@kernel.org

Tzung-Bi Shih (6):
  gpio: Access `gpio_bus_type` in gpiochip_setup_dev()
  gpio: Remove redundant check for struct gpio_chip
  gpio: sysfs: Remove redundant check for struct gpio_chip
  gpio: Ensure struct gpio_chip for gpiochip_setup_dev()
  gpio: cdev: Don't check struct gpio_chip in gpio_chrdev_open()
  selftests: gpio: Add gpio-cdev-uaf tests

 drivers/gpio/gpiolib-cdev.c                   |  20 +-
 drivers/gpio/gpiolib-cdev.h                   |   2 +-
 drivers/gpio/gpiolib-sysfs.c                  |  32 +-
 drivers/gpio/gpiolib-sysfs.h                  |   8 +-
 drivers/gpio/gpiolib.c                        |  46 +--
 tools/testing/selftests/gpio/Makefile         |   5 +-
 tools/testing/selftests/gpio/gpio-cdev-uaf.c  | 292 ++++++++++++++++++
 tools/testing/selftests/gpio/gpio-cdev-uaf.sh |  63 ++++
 8 files changed, 402 insertions(+), 66 deletions(-)
 create mode 100644 tools/testing/selftests/gpio/gpio-cdev-uaf.c
 create mode 100755 tools/testing/selftests/gpio/gpio-cdev-uaf.sh

-- 
2.51.0

[PATCH v4 1/6] gpio: Access `gpio_bus_type` in gpiochip_setup_dev()

[Tzung-Bi Shih]

To make the intent clear, access `gpio_bus_type` only when it's ready in
gpiochip_setup_dev().

Reviewed-by: Linus Walleij <linusw@kernel.org>
Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
---
v4:
- Add R-b tag.

v3: https://lore.kernel.org/all/20260213092958.864411-2-tzungbi@kernel.org
- No changes.

v2: https://lore.kernel.org/all/20260203061059.975605-2-tzungbi@kernel.org
- No changes.

v1: https://lore.kernel.org/all/20260116081036.352286-7-tzungbi@kernel.org

 drivers/gpio/gpiolib.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index f77d5121a8a8..0a73561060dd 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -901,6 +901,8 @@ static int gpiochip_setup_dev(struct gpio_device *gdev)
 	struct fwnode_handle *fwnode = dev_fwnode(&gdev->dev);
 	int ret;
 
+	gdev->dev.bus = &gpio_bus_type;
+
 	/*
 	 * If fwnode doesn't belong to another device, it's safe to clear its
 	 * initialized flag.
@@ -1082,7 +1084,6 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data,
 	 * then make sure they get free():ed there.
 	 */
 	gdev->dev.type = &gpio_dev_type;
-	gdev->dev.bus = &gpio_bus_type;
 	gdev->dev.parent = gc->parent;
 	device_set_node(&gdev->dev, gpiochip_choose_fwnode(gc));
 
@@ -1220,8 +1221,8 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data,
 	 * we get a device node entry in sysfs under
 	 * /sys/bus/gpio/devices/gpiochipN/dev that can be used for
 	 * coldplug of device nodes and other udev business.
-	 * We can do this only if gpiolib has been initialized.
-	 * Otherwise, defer until later.
+	 * We can do this only if gpiolib has been initialized
+	 * (i.e., `gpio_bus_type` is ready).  Otherwise, defer until later.
 	 */
 	if (gpiolib_initialized) {
 		ret = gpiochip_setup_dev(gdev);
-- 
2.51.0

[PATCH v4 2/6] gpio: Remove redundant check for struct gpio_chip

[Tzung-Bi Shih]

gpiolib_dbg_show() is only called by gpiolib_seq_show() which has
ensured the struct gpio_chip.  Remove the redundant check in
gpiolib_dbg_show().

Reviewed-by: Linus Walleij <linusw@kernel.org>
Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
---
v4:
- Add R-b tag.

v3: https://lore.kernel.org/all/20260213092958.864411-3-tzungbi@kernel.org
- No changes.

v2: https://lore.kernel.org/all/20260203061059.975605-3-tzungbi@kernel.org
- No changes.

v1: https://lore.kernel.org/all/20260116081036.352286-8-tzungbi@kernel.org

 drivers/gpio/gpiolib.c | 13 ++-----------
 1 file changed, 2 insertions(+), 11 deletions(-)

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 0a73561060dd..08a3697dbd5a 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -5312,23 +5312,14 @@ core_initcall(gpiolib_dev_init);
 
 #ifdef CONFIG_DEBUG_FS
 
-static void gpiolib_dbg_show(struct seq_file *s, struct gpio_device *gdev)
+static void gpiolib_dbg_show(struct seq_file *s, struct gpio_chip *gc)
 {
 	bool active_low, is_irq, is_out;
 	struct gpio_desc *desc;
 	unsigned int gpio = 0;
-	struct gpio_chip *gc;
 	unsigned long flags;
 	int value;
 
-	guard(srcu)(&gdev->srcu);
-
-	gc = srcu_dereference(gdev->chip, &gdev->srcu);
-	if (!gc) {
-		seq_puts(s, "Underlying GPIO chip is gone\n");
-		return;
-	}
-
 	for_each_gpio_desc(gc, desc) {
 		guard(srcu)(&desc->gdev->desc_srcu);
 		flags = READ_ONCE(desc->flags);
@@ -5441,7 +5432,7 @@ static int gpiolib_seq_show(struct seq_file *s, void *v)
 	if (gc->dbg_show)
 		gc->dbg_show(s, gc);
 	else
-		gpiolib_dbg_show(s, gdev);
+		gpiolib_dbg_show(s, gc);
 
 	return 0;
 }
-- 
2.51.0

[PATCH v4 3/6] gpio: sysfs: Remove redundant check for struct gpio_chip

[Tzung-Bi Shih]

gpiochip_sysfs_unregister() is only called by gpiochip_remove() where
the struct gpio_chip is ensured.

Remove the redundant check.

Reviewed-by: Linus Walleij <linusw@kernel.org>
Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
---
v4:
- Add R-b tag.
- To be consistent, rename `chip` -> `gc`.

v3: https://lore.kernel.org/all/20260213092958.864411-4-tzungbi@kernel.org
- Pass struct gpio_chip * only.

v2: https://lore.kernel.org/all/20260203061059.975605-4-tzungbi@kernel.org
- No changes.

v1: https://lore.kernel.org/all/20260116081036.352286-9-tzungbi@kernel.org

 drivers/gpio/gpiolib-sysfs.c | 11 +++--------
 drivers/gpio/gpiolib-sysfs.h |  4 ++--
 drivers/gpio/gpiolib.c       |  2 +-
 3 files changed, 6 insertions(+), 11 deletions(-)

diff --git a/drivers/gpio/gpiolib-sysfs.c b/drivers/gpio/gpiolib-sysfs.c
index 270e8060e761..1c25a7dd3db4 100644
--- a/drivers/gpio/gpiolib-sysfs.c
+++ b/drivers/gpio/gpiolib-sysfs.c
@@ -1053,11 +1053,11 @@ int gpiochip_sysfs_register(struct gpio_device *gdev)
 	return 0;
 }
 
-void gpiochip_sysfs_unregister(struct gpio_device *gdev)
+void gpiochip_sysfs_unregister(struct gpio_chip *gc)
 {
+	struct gpio_device *gdev = gc->gpiodev;
 	struct gpiodev_data *data;
 	struct gpio_desc *desc;
-	struct gpio_chip *chip;
 
 	guard(mutex)(&sysfs_lock);
 
@@ -1065,13 +1065,8 @@ void gpiochip_sysfs_unregister(struct gpio_device *gdev)
 	if (!data)
 		return;
 
-	guard(srcu)(&gdev->srcu);
-	chip = srcu_dereference(gdev->chip, &gdev->srcu);
-	if (!chip)
-		return;
-
 	/* unregister gpiod class devices owned by sysfs */
-	for_each_gpio_desc_with_flag(chip, desc, GPIOD_FLAG_SYSFS) {
+	for_each_gpio_desc_with_flag(gc, desc, GPIOD_FLAG_SYSFS) {
 		gpiod_unexport_unlocked(desc);
 		gpiod_free(desc);
 	}
diff --git a/drivers/gpio/gpiolib-sysfs.h b/drivers/gpio/gpiolib-sysfs.h
index b794b396d6a5..fd5db5384681 100644
--- a/drivers/gpio/gpiolib-sysfs.h
+++ b/drivers/gpio/gpiolib-sysfs.h
@@ -8,7 +8,7 @@ struct gpio_device;
 #ifdef CONFIG_GPIO_SYSFS
 
 int gpiochip_sysfs_register(struct gpio_device *gdev);
-void gpiochip_sysfs_unregister(struct gpio_device *gdev);
+void gpiochip_sysfs_unregister(struct gpio_chip *gc);
 
 #else
 
@@ -17,7 +17,7 @@ static inline int gpiochip_sysfs_register(struct gpio_device *gdev)
 	return 0;
 }
 
-static inline void gpiochip_sysfs_unregister(struct gpio_device *gdev)
+static inline void gpiochip_sysfs_unregister(struct gpio_chip *gc)
 {
 }
 
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 08a3697dbd5a..d5070c538ba5 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -1285,7 +1285,7 @@ void gpiochip_remove(struct gpio_chip *gc)
 	struct gpio_device *gdev = gc->gpiodev;
 
 	/* FIXME: should the legacy sysfs handling be moved to gpio_device? */
-	gpiochip_sysfs_unregister(gdev);
+	gpiochip_sysfs_unregister(gc);
 	gpiochip_free_hogs(gc);
 	gpiochip_free_remaining_irqs(gc);
 
-- 
2.51.0

[PATCH v4 4/6] gpio: Ensure struct gpio_chip for gpiochip_setup_dev()

[Tzung-Bi Shih]

Ensure struct gpio_chip for gpiochip_setup_dev().  This eliminates a few
checks for struct gpio_chip.

Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
---
v4:
- To be consistent, rename `chip` -> `gc`.
- Add lockdep checks.

v3: https://lore.kernel.org/all/20260213092958.864411-5-tzungbi@kernel.org
- Pass struct gpio_chip * only.

v2: https://lore.kernel.org/all/20260203061059.975605-5-tzungbi@kernel.org
- No changes.

v1: https://lore.kernel.org/all/20260116081036.352286-10-tzungbi@kernel.org

 drivers/gpio/gpiolib-cdev.c  | 14 ++++----------
 drivers/gpio/gpiolib-cdev.h  |  2 +-
 drivers/gpio/gpiolib-sysfs.c | 21 ++++++++-------------
 drivers/gpio/gpiolib-sysfs.h |  4 ++--
 drivers/gpio/gpiolib.c       | 24 +++++++++++++++++-------
 5 files changed, 32 insertions(+), 33 deletions(-)

diff --git a/drivers/gpio/gpiolib-cdev.c b/drivers/gpio/gpiolib-cdev.c
index 73ae77f0f213..a154b04e9316 100644
--- a/drivers/gpio/gpiolib-cdev.c
+++ b/drivers/gpio/gpiolib-cdev.c
@@ -2782,11 +2782,13 @@ static const struct file_operations gpio_fileops = {
 #endif
 };
 
-int gpiolib_cdev_register(struct gpio_device *gdev, dev_t devt)
+int gpiolib_cdev_register(struct gpio_chip *gc, dev_t devt)
 {
-	struct gpio_chip *gc;
+	struct gpio_device *gdev = gc->gpiodev;
 	int ret;
 
+	lockdep_assert_held(&gdev->srcu);
+
 	cdev_init(&gdev->chrdev, &gpio_fileops);
 	gdev->chrdev.owner = THIS_MODULE;
 	gdev->dev.devt = MKDEV(MAJOR(devt), gdev->id);
@@ -2802,14 +2804,6 @@ int gpiolib_cdev_register(struct gpio_device *gdev, dev_t devt)
 		return ret;
 	}
 
-	guard(srcu)(&gdev->srcu);
-	gc = srcu_dereference(gdev->chip, &gdev->srcu);
-	if (!gc) {
-		cdev_device_del(&gdev->chrdev, &gdev->dev);
-		destroy_workqueue(gdev->line_state_wq);
-		return -ENODEV;
-	}
-
 	gpiochip_dbg(gc, "added GPIO chardev (%d:%d)\n", MAJOR(devt), gdev->id);
 
 	return 0;
diff --git a/drivers/gpio/gpiolib-cdev.h b/drivers/gpio/gpiolib-cdev.h
index b42644cbffb8..4a9cb3335d99 100644
--- a/drivers/gpio/gpiolib-cdev.h
+++ b/drivers/gpio/gpiolib-cdev.h
@@ -7,7 +7,7 @@
 
 struct gpio_device;
 
-int gpiolib_cdev_register(struct gpio_device *gdev, dev_t devt);
+int gpiolib_cdev_register(struct gpio_chip *gc, dev_t devt);
 void gpiolib_cdev_unregister(struct gpio_device *gdev);
 
 #endif /* GPIOLIB_CDEV_H */
diff --git a/drivers/gpio/gpiolib-sysfs.c b/drivers/gpio/gpiolib-sysfs.c
index 1c25a7dd3db4..748a3eb1bf35 100644
--- a/drivers/gpio/gpiolib-sysfs.c
+++ b/drivers/gpio/gpiolib-sysfs.c
@@ -983,13 +983,15 @@ void gpiod_unexport(struct gpio_desc *desc)
 }
 EXPORT_SYMBOL_GPL(gpiod_unexport);
 
-int gpiochip_sysfs_register(struct gpio_device *gdev)
+int gpiochip_sysfs_register(struct gpio_chip *gc)
 {
+	struct gpio_device *gdev = gc->gpiodev;
 	struct gpiodev_data *data;
-	struct gpio_chip *chip;
 	struct device *parent;
 	int err;
 
+	lockdep_assert_held(&gdev->srcu);
+
 	/*
 	 * Many systems add gpio chips for SOC support very early,
 	 * before driver model support is available.  In those cases we
@@ -999,18 +1001,12 @@ int gpiochip_sysfs_register(struct gpio_device *gdev)
 	if (!class_is_registered(&gpio_class))
 		return 0;
 
-	guard(srcu)(&gdev->srcu);
-
-	chip = srcu_dereference(gdev->chip, &gdev->srcu);
-	if (!chip)
-		return -ENODEV;
-
 	/*
 	 * For sysfs backward compatibility we need to preserve this
 	 * preferred parenting to the gpio_chip parent field, if set.
 	 */
-	if (chip->parent)
-		parent = chip->parent;
+	if (gc->parent)
+		parent = gc->parent;
 	else
 		parent = &gdev->dev;
 
@@ -1029,7 +1025,7 @@ int gpiochip_sysfs_register(struct gpio_device *gdev)
 						    MKDEV(0, 0), data,
 						    gpiochip_groups,
 						    GPIOCHIP_NAME "%d",
-						    chip->base);
+						    gc->base);
 	if (IS_ERR(data->cdev_base)) {
 		err = PTR_ERR(data->cdev_base);
 		kfree(data);
@@ -1085,10 +1081,9 @@ void gpiochip_sysfs_unregister(struct gpio_chip *gc)
  */
 static int gpiofind_sysfs_register(struct gpio_chip *gc, const void *data)
 {
-	struct gpio_device *gdev = gc->gpiodev;
 	int ret;
 
-	ret = gpiochip_sysfs_register(gdev);
+	ret = gpiochip_sysfs_register(gc);
 	if (ret)
 		gpiochip_err(gc, "failed to register the sysfs entry: %d\n", ret);
 
diff --git a/drivers/gpio/gpiolib-sysfs.h b/drivers/gpio/gpiolib-sysfs.h
index fd5db5384681..d0998de043a2 100644
--- a/drivers/gpio/gpiolib-sysfs.h
+++ b/drivers/gpio/gpiolib-sysfs.h
@@ -7,12 +7,12 @@ struct gpio_device;
 
 #ifdef CONFIG_GPIO_SYSFS
 
-int gpiochip_sysfs_register(struct gpio_device *gdev);
+int gpiochip_sysfs_register(struct gpio_chip *gc);
 void gpiochip_sysfs_unregister(struct gpio_chip *gc);
 
 #else
 
-static inline int gpiochip_sysfs_register(struct gpio_device *gdev)
+static inline int gpiochip_sysfs_register(struct gpio_chip *gc)
 {
 	return 0;
 }
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index d5070c538ba5..44635e9a29c3 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -881,14 +881,14 @@ static const struct device_type gpio_dev_type = {
 };
 
 #ifdef CONFIG_GPIO_CDEV
-#define gcdev_register(gdev, devt)	gpiolib_cdev_register((gdev), (devt))
+#define gcdev_register(gc, devt)	gpiolib_cdev_register((gc), (devt))
 #define gcdev_unregister(gdev)		gpiolib_cdev_unregister((gdev))
 #else
 /*
  * gpiolib_cdev_register() indirectly calls device_add(), which is still
  * required even when cdev is not selected.
  */
-#define gcdev_register(gdev, devt)	device_add(&(gdev)->dev)
+#define gcdev_register(gc, devt)	device_add(&(gc)->gpiodev->dev)
 #define gcdev_unregister(gdev)		device_del(&(gdev)->dev)
 #endif
 
@@ -896,8 +896,9 @@ static const struct device_type gpio_dev_type = {
  * An initial reference count has been held in gpiochip_add_data_with_key().
  * The caller should drop the reference via gpio_device_put() on errors.
  */
-static int gpiochip_setup_dev(struct gpio_device *gdev)
+static int gpiochip_setup_dev(struct gpio_chip *gc)
 {
+	struct gpio_device *gdev = gc->gpiodev;
 	struct fwnode_handle *fwnode = dev_fwnode(&gdev->dev);
 	int ret;
 
@@ -910,11 +911,11 @@ static int gpiochip_setup_dev(struct gpio_device *gdev)
 	if (fwnode && !fwnode->dev)
 		fwnode_dev_initialized(fwnode, false);
 
-	ret = gcdev_register(gdev, gpio_devt);
+	ret = gcdev_register(gc, gpio_devt);
 	if (ret)
 		return ret;
 
-	ret = gpiochip_sysfs_register(gdev);
+	ret = gpiochip_sysfs_register(gc);
 	if (ret)
 		goto err_remove_device;
 
@@ -961,13 +962,22 @@ static void machine_gpiochip_add(struct gpio_chip *gc)
 static void gpiochip_setup_devs(void)
 {
 	struct gpio_device *gdev;
+	struct gpio_chip *gc;
 	int ret;
 
 	guard(srcu)(&gpio_devices_srcu);
 
 	list_for_each_entry_srcu(gdev, &gpio_devices, list,
 				 srcu_read_lock_held(&gpio_devices_srcu)) {
-		ret = gpiochip_setup_dev(gdev);
+		guard(srcu)(&gdev->srcu);
+
+		gc = srcu_dereference(gdev->chip, &gdev->srcu);
+		if (!gc) {
+			dev_err(&gdev->dev, "Underlying GPIO chip is gone\n");
+			continue;
+		}
+
+		ret = gpiochip_setup_dev(gc);
 		if (ret) {
 			gpio_device_put(gdev);
 			dev_err(&gdev->dev,
@@ -1225,7 +1235,7 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data,
 	 * (i.e., `gpio_bus_type` is ready).  Otherwise, defer until later.
 	 */
 	if (gpiolib_initialized) {
-		ret = gpiochip_setup_dev(gdev);
+		ret = gpiochip_setup_dev(gc);
 		if (ret)
 			goto err_teardown_shared;
 	}
-- 
2.51.0

[PATCH v4 5/6] gpio: cdev: Don't check struct gpio_chip in gpio_chrdev_open()

[Tzung-Bi Shih]

It's harmless even if: chrdev_open() and cdev_device_del() run at the
same time, and gpio_chrdev_open() gets called after the underlying GPIO
chip has gone.  The subsequent file operations check the availability
of struct gpio_chip anyway.

Don't check struct gpio_chip in gpio_chrdev_open().

Reviewed-by: Linus Walleij <linusw@kernel.org>
Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
---
v4:
- Add R-b tag.

v3: https://lore.kernel.org/all/20260213092958.864411-6-tzungbi@kernel.org
- No changes.

v2: https://lore.kernel.org/all/20260203061059.975605-6-tzungbi@kernel.org
- No changes.

v1: https://lore.kernel.org/all/20260116081036.352286-11-tzungbi@kernel.org

 drivers/gpio/gpiolib-cdev.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/drivers/gpio/gpiolib-cdev.c b/drivers/gpio/gpiolib-cdev.c
index a154b04e9316..475ad185f915 100644
--- a/drivers/gpio/gpiolib-cdev.c
+++ b/drivers/gpio/gpiolib-cdev.c
@@ -2689,12 +2689,6 @@ static int gpio_chrdev_open(struct inode *inode, struct file *file)
 	struct gpio_chardev_data *cdev;
 	int ret = -ENOMEM;
 
-	guard(srcu)(&gdev->srcu);
-
-	/* Fail on open if the backing gpiochip is gone */
-	if (!rcu_access_pointer(gdev->chip))
-		return -ENODEV;
-
 	cdev = kzalloc(sizeof(*cdev), GFP_KERNEL);
 	if (!cdev)
 		return -ENOMEM;
-- 
2.51.0

[PATCH v4 6/6] selftests: gpio: Add gpio-cdev-uaf tests

[Tzung-Bi Shih]

Add tests for gpiolib-cdev to make sure accessing to dangling resources
via the opening file descriptor won't crash the system after the
underlying resource providers have gone.

Reviewed-by: Linus Walleij <linusw@kernel.org>
Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
---
v4:
- Add R-b tag.

v3: https://lore.kernel.org/all/20260213092958.864411-7-tzungbi@kernel.org
- No changes.

v2: https://lore.kernel.org/all/20260203061059.975605-7-tzungbi@kernel.org
- Remove fdinfo test which is redundant.

v1: https://lore.kernel.org/all/20260116081036.352286-12-tzungbi@kernel.org

 tools/testing/selftests/gpio/Makefile         |   5 +-
 tools/testing/selftests/gpio/gpio-cdev-uaf.c  | 292 ++++++++++++++++++
 tools/testing/selftests/gpio/gpio-cdev-uaf.sh |  63 ++++
 3 files changed, 358 insertions(+), 2 deletions(-)
 create mode 100644 tools/testing/selftests/gpio/gpio-cdev-uaf.c
 create mode 100755 tools/testing/selftests/gpio/gpio-cdev-uaf.sh

diff --git a/tools/testing/selftests/gpio/Makefile b/tools/testing/selftests/gpio/Makefile
index 7bfe315f7001..741ab21e1260 100644
--- a/tools/testing/selftests/gpio/Makefile
+++ b/tools/testing/selftests/gpio/Makefile
@@ -1,8 +1,9 @@
 # SPDX-License-Identifier: GPL-2.0
 
-TEST_PROGS := gpio-mockup.sh gpio-sim.sh gpio-aggregator.sh
+TEST_PROGS := gpio-mockup.sh gpio-sim.sh gpio-aggregator.sh gpio-cdev-uaf.sh
 TEST_FILES := gpio-mockup-sysfs.sh
-TEST_GEN_PROGS_EXTENDED := gpio-mockup-cdev gpio-chip-info gpio-line-name
+TEST_GEN_PROGS_EXTENDED := gpio-mockup-cdev gpio-chip-info gpio-line-name \
+			   gpio-cdev-uaf
 CFLAGS += -O2 -g -Wall $(KHDR_INCLUDES)
 
 include ../lib.mk
diff --git a/tools/testing/selftests/gpio/gpio-cdev-uaf.c b/tools/testing/selftests/gpio/gpio-cdev-uaf.c
new file mode 100644
index 000000000000..765d3cc4f0ef
--- /dev/null
+++ b/tools/testing/selftests/gpio/gpio-cdev-uaf.c
@@ -0,0 +1,292 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * GPIO character device helper for UAF tests.
+ *
+ * Copyright 2026 Google LLC
+ */
+
+#include <errno.h>
+#include <fcntl.h>
+#include <linux/gpio.h>
+#include <poll.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <sys/ioctl.h>
+#include <sys/stat.h>
+#include <sys/types.h>
+#include <unistd.h>
+
+#define CONFIGFS_DIR "/sys/kernel/config/gpio-sim"
+#define PROCFS_DIR "/proc"
+
+static void print_usage(void)
+{
+	printf("usage:\n");
+	printf("  gpio-cdev-uaf [chip|handle|event|req] [poll|read|ioctl]\n");
+}
+
+static int _create_chip(const char *name, int create)
+{
+	char path[64];
+
+	snprintf(path, sizeof(path), CONFIGFS_DIR "/%s", name);
+
+	if (create)
+		return mkdir(path, 0755);
+	else
+		return rmdir(path);
+}
+
+static int create_chip(const char *name)
+{
+	return _create_chip(name, 1);
+}
+
+static void remove_chip(const char *name)
+{
+	_create_chip(name, 0);
+}
+
+static int _create_bank(const char *chip_name, const char *name, int create)
+{
+	char path[64];
+
+	snprintf(path, sizeof(path), CONFIGFS_DIR "/%s/%s", chip_name, name);
+
+	if (create)
+		return mkdir(path, 0755);
+	else
+		return rmdir(path);
+}
+
+static int create_bank(const char *chip_name, const char *name)
+{
+	return _create_bank(chip_name, name, 1);
+}
+
+static void remove_bank(const char *chip_name, const char *name)
+{
+	_create_bank(chip_name, name, 0);
+}
+
+static int _enable_chip(const char *name, int enable)
+{
+	char path[64];
+	int fd, ret;
+
+	snprintf(path, sizeof(path), CONFIGFS_DIR "/%s/live", name);
+
+	fd = open(path, O_WRONLY);
+	if (fd == -1)
+		return fd;
+
+	if (enable)
+		ret = write(fd, "1", 1);
+	else
+		ret = write(fd, "0", 1);
+
+	close(fd);
+	return ret == 1 ? 0 : -1;
+}
+
+static int enable_chip(const char *name)
+{
+	return _enable_chip(name, 1);
+}
+
+static void disable_chip(const char *name)
+{
+	_enable_chip(name, 0);
+}
+
+static int open_chip(const char *chip_name, const char *bank_name)
+{
+	char path[64], dev_name[32];
+	int ret, fd;
+
+	ret = create_chip(chip_name);
+	if (ret) {
+		fprintf(stderr, "failed to create chip\n");
+		return ret;
+	}
+
+	ret = create_bank(chip_name, bank_name);
+	if (ret) {
+		fprintf(stderr, "failed to create bank\n");
+		goto err_remove_chip;
+	}
+
+	ret = enable_chip(chip_name);
+	if (ret) {
+		fprintf(stderr, "failed to enable chip\n");
+		goto err_remove_bank;
+	}
+
+	snprintf(path, sizeof(path), CONFIGFS_DIR "/%s/%s/chip_name",
+		 chip_name, bank_name);
+
+	fd = open(path, O_RDONLY);
+	if (fd == -1) {
+		ret = fd;
+		fprintf(stderr, "failed to open %s\n", path);
+		goto err_disable_chip;
+	}
+
+	ret = read(fd, dev_name, sizeof(dev_name) - 1);
+	close(fd);
+	if (ret == -1) {
+		fprintf(stderr, "failed to read %s\n", path);
+		goto err_disable_chip;
+	}
+	dev_name[ret] = '\0';
+	if (ret && dev_name[ret - 1] == '\n')
+		dev_name[ret - 1] = '\0';
+
+	snprintf(path, sizeof(path), "/dev/%s", dev_name);
+
+	fd = open(path, O_RDWR);
+	if (fd == -1) {
+		ret = fd;
+		fprintf(stderr, "failed to open %s\n", path);
+		goto err_disable_chip;
+	}
+
+	return fd;
+err_disable_chip:
+	disable_chip(chip_name);
+err_remove_bank:
+	remove_bank(chip_name, bank_name);
+err_remove_chip:
+	remove_chip(chip_name);
+	return ret;
+}
+
+static void close_chip(const char *chip_name, const char *bank_name)
+{
+	disable_chip(chip_name);
+	remove_bank(chip_name, bank_name);
+	remove_chip(chip_name);
+}
+
+static int test_poll(int fd)
+{
+	struct pollfd pfds;
+
+	pfds.fd = fd;
+	pfds.events = POLLIN;
+	pfds.revents = 0;
+
+	if (poll(&pfds, 1, 0) == -1)
+		return -1;
+
+	return (pfds.revents & ~(POLLHUP | POLLERR)) ? -1 : 0;
+}
+
+static int test_read(int fd)
+{
+	char data;
+
+	if (read(fd, &data, 1) == -1 && errno == ENODEV)
+		return 0;
+	return -1;
+}
+
+static int test_ioctl(int fd)
+{
+	if (ioctl(fd, 0, NULL) == -1 && errno == ENODEV)
+		return 0;
+	return -1;
+}
+
+int main(int argc, char **argv)
+{
+	int cfd, fd, ret;
+	int (*test_func)(int);
+
+	if (argc != 3) {
+		print_usage();
+		return EXIT_FAILURE;
+	}
+
+	if (strcmp(argv[1], "chip") == 0 ||
+	    strcmp(argv[1], "event") == 0 ||
+	    strcmp(argv[1], "req") == 0) {
+		if (strcmp(argv[2], "poll") &&
+		    strcmp(argv[2], "read") &&
+		    strcmp(argv[2], "ioctl")) {
+			fprintf(stderr, "unknown command: %s\n", argv[2]);
+			return EXIT_FAILURE;
+		}
+	} else if (strcmp(argv[1], "handle") == 0) {
+		if (strcmp(argv[2], "ioctl")) {
+			fprintf(stderr, "unknown command: %s\n", argv[2]);
+			return EXIT_FAILURE;
+		}
+	} else {
+		fprintf(stderr, "unknown command: %s\n", argv[1]);
+		return EXIT_FAILURE;
+	}
+
+	if (strcmp(argv[2], "poll") == 0)
+		test_func = test_poll;
+	else if (strcmp(argv[2], "read") == 0)
+		test_func = test_read;
+	else	/* strcmp(argv[2], "ioctl") == 0 */
+		test_func = test_ioctl;
+
+	cfd = open_chip("chip", "bank");
+	if (cfd == -1) {
+		fprintf(stderr, "failed to open chip\n");
+		return EXIT_FAILURE;
+	}
+
+	/* Step 1: Hold a FD to the test target. */
+	if (strcmp(argv[1], "chip") == 0) {
+		fd = cfd;
+	} else if (strcmp(argv[1], "handle") == 0) {
+		struct gpiohandle_request req = {0};
+
+		req.lines = 1;
+		if (ioctl(cfd, GPIO_GET_LINEHANDLE_IOCTL, &req) == -1) {
+			fprintf(stderr, "failed to get handle FD\n");
+			goto err_close_chip;
+		}
+
+		close(cfd);
+		fd = req.fd;
+	} else if (strcmp(argv[1], "event") == 0) {
+		struct gpioevent_request req = {0};
+
+		if (ioctl(cfd, GPIO_GET_LINEEVENT_IOCTL, &req) == -1) {
+			fprintf(stderr, "failed to get event FD\n");
+			goto err_close_chip;
+		}
+
+		close(cfd);
+		fd = req.fd;
+	} else {	/* strcmp(argv[1], "req") == 0 */
+		struct gpio_v2_line_request req = {0};
+
+		req.num_lines = 1;
+		if (ioctl(cfd, GPIO_V2_GET_LINE_IOCTL, &req) == -1) {
+			fprintf(stderr, "failed to get req FD\n");
+			goto err_close_chip;
+		}
+
+		close(cfd);
+		fd = req.fd;
+	}
+
+	/* Step 2: Free the chip. */
+	close_chip("chip", "bank");
+
+	/* Step 3: Access the dangling FD to trigger UAF. */
+	ret = test_func(fd);
+	close(fd);
+	return ret ? EXIT_FAILURE : EXIT_SUCCESS;
+err_close_chip:
+	close(cfd);
+	close_chip("chip", "bank");
+	return EXIT_FAILURE;
+}
diff --git a/tools/testing/selftests/gpio/gpio-cdev-uaf.sh b/tools/testing/selftests/gpio/gpio-cdev-uaf.sh
new file mode 100755
index 000000000000..6e47533019cf
--- /dev/null
+++ b/tools/testing/selftests/gpio/gpio-cdev-uaf.sh
@@ -0,0 +1,63 @@
+#!/bin/sh
+# SPDX-License-Identifier: GPL-2.0
+# Copyright 2026 Google LLC
+
+BASE_DIR=`dirname $0`
+MODULE="gpio-cdev-uaf"
+
+fail() {
+	echo "$*" >&2
+	echo "GPIO $MODULE test FAIL"
+	exit 1
+}
+
+skip() {
+	echo "$*" >&2
+	echo "GPIO $MODULE test SKIP"
+	exit 4
+}
+
+# Load the gpio-sim module. This will pull in configfs if needed too.
+modprobe gpio-sim || skip "unable to load the gpio-sim module"
+# Make sure configfs is mounted at /sys/kernel/config. Wait a bit if needed.
+for _ in `seq 5`; do
+	mountpoint -q /sys/kernel/config && break
+	mount -t configfs none /sys/kernel/config
+	sleep 0.1
+done
+mountpoint -q /sys/kernel/config || \
+	skip "configfs not mounted at /sys/kernel/config"
+
+echo "1. GPIO"
+
+echo "1.1. poll"
+$BASE_DIR/gpio-cdev-uaf chip poll || fail "failed to test chip poll"
+echo "1.2. read"
+$BASE_DIR/gpio-cdev-uaf chip read || fail "failed to test chip read"
+echo "1.3. ioctl"
+$BASE_DIR/gpio-cdev-uaf chip ioctl || fail "failed to test chip ioctl"
+
+echo "2. linehandle"
+
+echo "2.1. ioctl"
+$BASE_DIR/gpio-cdev-uaf handle ioctl || fail "failed to test handle ioctl"
+
+echo "3. lineevent"
+
+echo "3.1. read"
+$BASE_DIR/gpio-cdev-uaf event read || fail "failed to test event read"
+echo "3.2. poll"
+$BASE_DIR/gpio-cdev-uaf event poll || fail "failed to test event poll"
+echo "3.3. ioctl"
+$BASE_DIR/gpio-cdev-uaf event ioctl || fail "failed to test event ioctl"
+
+echo "4. linereq"
+
+echo "4.1. read"
+$BASE_DIR/gpio-cdev-uaf req read || fail "failed to test req read"
+echo "4.2. poll"
+$BASE_DIR/gpio-cdev-uaf req poll || fail "failed to test req poll"
+echo "4.3. ioctl"
+$BASE_DIR/gpio-cdev-uaf req ioctl || fail "failed to test req ioctl"
+
+echo "GPIO $MODULE test PASS"
-- 
2.51.0

Re: [PATCH v4 0/6] gpio: Refactor and add selftest

[Bartosz Golaszewski]

On Mon, 23 Feb 2026 07:17:20 +0100, Tzung-Bi Shih <tzungbi@kernel.org> said:
> The series is separated from v3 to lessen the burden on the subsequent
> revocable work per suggestion in
> https://lore.kernel.org/all/CAMRc=MfQumD1ULx7yU4W2sx=35wyQf7-v4tSf44OqEu3JDBUAg@mail.gmail.com/.
>
> The series is based on v7.0-rc1 and applies after
> https://lore.kernel.org/all/20260205092840.2574840-1-tzungbi@kernel.org.
>
> ---
> v4:
> - Separate the first 6 patches from v3.
>
> v3: https://lore.kernel.org/all/20260213092958.864411-1-tzungbi@kernel.org
>
> Tzung-Bi Shih (6):
>   gpio: Access `gpio_bus_type` in gpiochip_setup_dev()
>   gpio: Remove redundant check for struct gpio_chip
>   gpio: sysfs: Remove redundant check for struct gpio_chip
>   gpio: Ensure struct gpio_chip for gpiochip_setup_dev()
>   gpio: cdev: Don't check struct gpio_chip in gpio_chrdev_open()
>   selftests: gpio: Add gpio-cdev-uaf tests
>
>  drivers/gpio/gpiolib-cdev.c                   |  20 +-
>  drivers/gpio/gpiolib-cdev.h                   |   2 +-
>  drivers/gpio/gpiolib-sysfs.c                  |  32 +-
>  drivers/gpio/gpiolib-sysfs.h                  |   8 +-
>  drivers/gpio/gpiolib.c                        |  46 +--
>  tools/testing/selftests/gpio/Makefile         |   5 +-
>  tools/testing/selftests/gpio/gpio-cdev-uaf.c  | 292 ++++++++++++++++++
>  tools/testing/selftests/gpio/gpio-cdev-uaf.sh |  63 ++++
>  8 files changed, 402 insertions(+), 66 deletions(-)
>  create mode 100644 tools/testing/selftests/gpio/gpio-cdev-uaf.c
>  create mode 100755 tools/testing/selftests/gpio/gpio-cdev-uaf.sh
>
> --
> 2.51.0
>
>

Hi Tzung-Bi!

This no longer applies on top of current linux-next. Could you please rabase
and resend?

Thanks!
Bart

Re: [PATCH v4 0/6] gpio: Refactor and add selftest

[Tzung-Bi Shih]

On Wed, Feb 25, 2026 at 02:26:19AM -0800, Bartosz Golaszewski wrote:
> On Mon, 23 Feb 2026 07:17:20 +0100, Tzung-Bi Shih <tzungbi@kernel.org> said:
> > The series is separated from v3 to lessen the burden on the subsequent
> > revocable work per suggestion in
> > https://lore.kernel.org/all/CAMRc=MfQumD1ULx7yU4W2sx=35wyQf7-v4tSf44OqEu3JDBUAg@mail.gmail.com/.
> >
> > The series is based on v7.0-rc1 and applies after
> > https://lore.kernel.org/all/20260205092840.2574840-1-tzungbi@kernel.org.
[...]
> 
> This no longer applies on top of current linux-next. Could you please rabase
> and resend?

Did you try to apply the series after applying "[PATCH v3] gpio: Fix resource
leaks on errors in gpiochip_add_data_with_key()" (link above)?

Tried on next-20260225; the 1+6 patches apply.

Re: [PATCH v4 0/6] gpio: Refactor and add selftest

[Bartosz Golaszewski]

On Mon, 23 Feb 2026 14:17:20 +0800, Tzung-Bi Shih wrote:
> The series is separated from v3 to lessen the burden on the subsequent
> revocable work per suggestion in
> https://lore.kernel.org/all/CAMRc=MfQumD1ULx7yU4W2sx=35wyQf7-v4tSf44OqEu3JDBUAg@mail.gmail.com/.
> 
> The series is based on v7.0-rc1 and applies after
> https://lore.kernel.org/all/20260205092840.2574840-1-tzungbi@kernel.org.
> 
> [...]

Applied, thanks!

[1/6] gpio: Access `gpio_bus_type` in gpiochip_setup_dev()
      https://git.kernel.org/brgl/c/cc11f4ef666fbca02c8a2f11d0184d57e6b75579
[2/6] gpio: Remove redundant check for struct gpio_chip
      https://git.kernel.org/brgl/c/049f71131734c47a6aaca2472273aef2cd17a6d8
[3/6] gpio: sysfs: Remove redundant check for struct gpio_chip
      https://git.kernel.org/brgl/c/395b8e555dfcbab6b28f360e39bd048b2f3e362b
[4/6] gpio: Ensure struct gpio_chip for gpiochip_setup_dev()
      https://git.kernel.org/brgl/c/cf674f1a0c9893ee1acef832679562007a94250a
[5/6] gpio: cdev: Don't check struct gpio_chip in gpio_chrdev_open()
      https://git.kernel.org/brgl/c/ee68f18d1f0d943d93070eb8cb05598b8b7f0922
[6/6] selftests: gpio: Add gpio-cdev-uaf tests
      https://git.kernel.org/brgl/c/c7f92042d3f3d4f084794f5314fa10366084179c

Best regards,
-- 
Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>

Re: [PATCH v4 0/6] gpio: Refactor and add selftest

[Bartosz Golaszewski]

On Thu, Feb 26, 2026 at 1:44 PM Tzung-Bi Shih <tzungbi@kernel.org> wrote:
>
> On Wed, Feb 25, 2026 at 02:26:19AM -0800, Bartosz Golaszewski wrote:
> > On Mon, 23 Feb 2026 07:17:20 +0100, Tzung-Bi Shih <tzungbi@kernel.org> said:
> > > The series is separated from v3 to lessen the burden on the subsequent
> > > revocable work per suggestion in
> > > https://lore.kernel.org/all/CAMRc=MfQumD1ULx7yU4W2sx=35wyQf7-v4tSf44OqEu3JDBUAg@mail.gmail.com/.
> > >
> > > The series is based on v7.0-rc1 and applies after
> > > https://lore.kernel.org/all/20260205092840.2574840-1-tzungbi@kernel.org.
> [...]
> >
> > This no longer applies on top of current linux-next. Could you please rabase
> > and resend?
>
> Did you try to apply the series after applying "[PATCH v3] gpio: Fix resource
> leaks on errors in gpiochip_add_data_with_key()" (link above)?
>
> Tried on next-20260225; the 1+6 patches apply.

Sorry for the noise, I should have reread the cover letter.

Bart

Re: [PATCH v4 0/6] gpio: Refactor and add selftest

[Linus Walleij]

On Mon, Feb 23, 2026 at 7:17 AM Tzung-Bi Shih <tzungbi@kernel.org> wrote:

> The series is separated from v3 to lessen the burden on the subsequent
> revocable work per suggestion in
> https://lore.kernel.org/all/CAMRc=MfQumD1ULx7yU4W2sx=35wyQf7-v4tSf44OqEu3JDBUAg@mail.gmail.com/.

No matter if revocable goes anywhere or not, this is excellent
maintenance work, thanks Tzung-Bi!

Yours,
Linus Walleij

Re: [PATCH v4 4/6] gpio: Ensure struct gpio_chip for gpiochip_setup_dev()

[Marek Szyprowski]

On 23.02.2026 07:17, Tzung-Bi Shih wrote:
> Ensure struct gpio_chip for gpiochip_setup_dev().  This eliminates a few
> checks for struct gpio_chip.
>
> Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>

This patch landed in today's linux-next as commit cf674f1a0c98 ("gpio: 
Ensure struct gpio_chip for gpiochip_setup_dev()"). In my tests I found 
that it triggers a warning on every test board I have, so I suspect that 
something is missing in the code. Here is an example of such warning:

------------[ cut here ]------------
WARNING: drivers/gpio/gpiolib-cdev.c:2735 at 
gpiolib_cdev_register+0x114/0x140, CPU#1: swapper/0/1
Modules linked in:
CPU: 1 UID: 0 PID: 1 Comm: swapper/0 Not tainted 
7.0.0-rc1-next-20260227-00065-g6af4b9cfeded #12259 PREEMPT
Hardware name: Samsung Exynos (Flattened Device Tree)
Call trace:
  unwind_backtrace from show_stack+0x10/0x14
  show_stack from dump_stack_lvl+0x68/0x88
  dump_stack_lvl from __warn+0x94/0x210
  __warn from warn_slowpath_fmt+0x1b0/0x1bc
  warn_slowpath_fmt from gpiolib_cdev_register+0x114/0x140
  gpiolib_cdev_register from gpiochip_setup_dev+0x4c/0xd0
  gpiochip_setup_dev from gpiochip_add_data_with_key+0x960/0xad4
  gpiochip_add_data_with_key from devm_gpiochip_add_data_with_key+0x20/0x5c
  devm_gpiochip_add_data_with_key from samsung_pinctrl_probe+0x8fc/0xbe8
  samsung_pinctrl_probe from platform_probe+0x5c/0x98
  platform_probe from really_probe+0xe0/0x424
  really_probe from __driver_probe_device+0x9c/0x1f4
  __driver_probe_device from driver_probe_device+0x30/0xc0
  driver_probe_device from __device_attach_driver+0xbc/0x180
  __device_attach_driver from bus_for_each_drv+0x84/0xdc
  bus_for_each_drv from __device_attach+0xb0/0x214
  __device_attach from device_initial_probe+0x3c/0x48
  device_initial_probe from bus_probe_device+0x24/0x80
  bus_probe_device from device_add+0x5c0/0x810
  device_add from of_platform_device_create_pdata+0xac/0x104
  of_platform_device_create_pdata from of_platform_bus_create+0x210/0x534
  of_platform_bus_create from of_platform_bus_create+0x27c/0x534
  of_platform_bus_create from of_platform_populate+0x90/0x150
  of_platform_populate from of_platform_default_populate_init+0xd0/0xe0
  of_platform_default_populate_init from do_one_initcall+0x70/0x3bc
  do_one_initcall from kernel_init_freeable+0x1c0/0x248
  kernel_init_freeable from kernel_init+0x18/0x12c
  kernel_init from ret_from_fork+0x14/0x28
Exception stack(0xf082dfb0 to 0xf082dff8)
dfa0:                                     00000000 00000000 00000000 
00000000
dfc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 
00000000
dfe0: 00000000 00000000 00000000 00000000 00000013 00000000
irq event stamp: 55167
hardirqs last  enabled at (55387): [<c01c3578>] __up_console_sem+0x50/0x60
hardirqs last disabled at (55398): [<c01c3564>] __up_console_sem+0x3c/0x60
softirqs last  enabled at (55352): [<c013c8fc>] handle_softirqs+0x32c/0x5c0
softirqs last disabled at (55419): [<c013cd3c>] __irq_exit_rcu+0x144/0x1f0
---[ end trace 0000000000000000 ]---


> ---
> v4:
> - To be consistent, rename `chip` -> `gc`.
> - Add lockdep checks.
>
> v3: https://lore.kernel.org/all/20260213092958.864411-5-tzungbi@kernel.org
> - Pass struct gpio_chip * only.
>
> v2: https://lore.kernel.org/all/20260203061059.975605-5-tzungbi@kernel.org
> - No changes.
>
> v1: https://lore.kernel.org/all/20260116081036.352286-10-tzungbi@kernel.org
>
>   drivers/gpio/gpiolib-cdev.c  | 14 ++++----------
>   drivers/gpio/gpiolib-cdev.h  |  2 +-
>   drivers/gpio/gpiolib-sysfs.c | 21 ++++++++-------------
>   drivers/gpio/gpiolib-sysfs.h |  4 ++--
>   drivers/gpio/gpiolib.c       | 24 +++++++++++++++++-------
>   5 files changed, 32 insertions(+), 33 deletions(-)
>
> diff --git a/drivers/gpio/gpiolib-cdev.c b/drivers/gpio/gpiolib-cdev.c
> index 73ae77f0f213..a154b04e9316 100644
> --- a/drivers/gpio/gpiolib-cdev.c
> +++ b/drivers/gpio/gpiolib-cdev.c
> @@ -2782,11 +2782,13 @@ static const struct file_operations gpio_fileops = {
>   #endif
>   };
>   
> -int gpiolib_cdev_register(struct gpio_device *gdev, dev_t devt)
> +int gpiolib_cdev_register(struct gpio_chip *gc, dev_t devt)
>   {
> -	struct gpio_chip *gc;
> +	struct gpio_device *gdev = gc->gpiodev;
>   	int ret;
>   
> +	lockdep_assert_held(&gdev->srcu);
> +
>   	cdev_init(&gdev->chrdev, &gpio_fileops);
>   	gdev->chrdev.owner = THIS_MODULE;
>   	gdev->dev.devt = MKDEV(MAJOR(devt), gdev->id);
> @@ -2802,14 +2804,6 @@ int gpiolib_cdev_register(struct gpio_device *gdev, dev_t devt)
>   		return ret;
>   	}
>   
> -	guard(srcu)(&gdev->srcu);
> -	gc = srcu_dereference(gdev->chip, &gdev->srcu);
> -	if (!gc) {
> -		cdev_device_del(&gdev->chrdev, &gdev->dev);
> -		destroy_workqueue(gdev->line_state_wq);
> -		return -ENODEV;
> -	}
> -
>   	gpiochip_dbg(gc, "added GPIO chardev (%d:%d)\n", MAJOR(devt), gdev->id);
>   
>   	return 0;
> diff --git a/drivers/gpio/gpiolib-cdev.h b/drivers/gpio/gpiolib-cdev.h
> index b42644cbffb8..4a9cb3335d99 100644
> --- a/drivers/gpio/gpiolib-cdev.h
> +++ b/drivers/gpio/gpiolib-cdev.h
> @@ -7,7 +7,7 @@
>   
>   struct gpio_device;
>   
> -int gpiolib_cdev_register(struct gpio_device *gdev, dev_t devt);
> +int gpiolib_cdev_register(struct gpio_chip *gc, dev_t devt);
>   void gpiolib_cdev_unregister(struct gpio_device *gdev);
>   
>   #endif /* GPIOLIB_CDEV_H */
> diff --git a/drivers/gpio/gpiolib-sysfs.c b/drivers/gpio/gpiolib-sysfs.c
> index 1c25a7dd3db4..748a3eb1bf35 100644
> --- a/drivers/gpio/gpiolib-sysfs.c
> +++ b/drivers/gpio/gpiolib-sysfs.c
> @@ -983,13 +983,15 @@ void gpiod_unexport(struct gpio_desc *desc)
>   }
>   EXPORT_SYMBOL_GPL(gpiod_unexport);
>   
> -int gpiochip_sysfs_register(struct gpio_device *gdev)
> +int gpiochip_sysfs_register(struct gpio_chip *gc)
>   {
> +	struct gpio_device *gdev = gc->gpiodev;
>   	struct gpiodev_data *data;
> -	struct gpio_chip *chip;
>   	struct device *parent;
>   	int err;
>   
> +	lockdep_assert_held(&gdev->srcu);
> +
>   	/*
>   	 * Many systems add gpio chips for SOC support very early,
>   	 * before driver model support is available.  In those cases we
> @@ -999,18 +1001,12 @@ int gpiochip_sysfs_register(struct gpio_device *gdev)
>   	if (!class_is_registered(&gpio_class))
>   		return 0;
>   
> -	guard(srcu)(&gdev->srcu);
> -
> -	chip = srcu_dereference(gdev->chip, &gdev->srcu);
> -	if (!chip)
> -		return -ENODEV;
> -
>   	/*
>   	 * For sysfs backward compatibility we need to preserve this
>   	 * preferred parenting to the gpio_chip parent field, if set.
>   	 */
> -	if (chip->parent)
> -		parent = chip->parent;
> +	if (gc->parent)
> +		parent = gc->parent;
>   	else
>   		parent = &gdev->dev;
>   
> @@ -1029,7 +1025,7 @@ int gpiochip_sysfs_register(struct gpio_device *gdev)
>   						    MKDEV(0, 0), data,
>   						    gpiochip_groups,
>   						    GPIOCHIP_NAME "%d",
> -						    chip->base);
> +						    gc->base);
>   	if (IS_ERR(data->cdev_base)) {
>   		err = PTR_ERR(data->cdev_base);
>   		kfree(data);
> @@ -1085,10 +1081,9 @@ void gpiochip_sysfs_unregister(struct gpio_chip *gc)
>    */
>   static int gpiofind_sysfs_register(struct gpio_chip *gc, const void *data)
>   {
> -	struct gpio_device *gdev = gc->gpiodev;
>   	int ret;
>   
> -	ret = gpiochip_sysfs_register(gdev);
> +	ret = gpiochip_sysfs_register(gc);
>   	if (ret)
>   		gpiochip_err(gc, "failed to register the sysfs entry: %d\n", ret);
>   
> diff --git a/drivers/gpio/gpiolib-sysfs.h b/drivers/gpio/gpiolib-sysfs.h
> index fd5db5384681..d0998de043a2 100644
> --- a/drivers/gpio/gpiolib-sysfs.h
> +++ b/drivers/gpio/gpiolib-sysfs.h
> @@ -7,12 +7,12 @@ struct gpio_device;
>   
>   #ifdef CONFIG_GPIO_SYSFS
>   
> -int gpiochip_sysfs_register(struct gpio_device *gdev);
> +int gpiochip_sysfs_register(struct gpio_chip *gc);
>   void gpiochip_sysfs_unregister(struct gpio_chip *gc);
>   
>   #else
>   
> -static inline int gpiochip_sysfs_register(struct gpio_device *gdev)
> +static inline int gpiochip_sysfs_register(struct gpio_chip *gc)
>   {
>   	return 0;
>   }
> diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
> index d5070c538ba5..44635e9a29c3 100644
> --- a/drivers/gpio/gpiolib.c
> +++ b/drivers/gpio/gpiolib.c
> @@ -881,14 +881,14 @@ static const struct device_type gpio_dev_type = {
>   };
>   
>   #ifdef CONFIG_GPIO_CDEV
> -#define gcdev_register(gdev, devt)	gpiolib_cdev_register((gdev), (devt))
> +#define gcdev_register(gc, devt)	gpiolib_cdev_register((gc), (devt))
>   #define gcdev_unregister(gdev)		gpiolib_cdev_unregister((gdev))
>   #else
>   /*
>    * gpiolib_cdev_register() indirectly calls device_add(), which is still
>    * required even when cdev is not selected.
>    */
> -#define gcdev_register(gdev, devt)	device_add(&(gdev)->dev)
> +#define gcdev_register(gc, devt)	device_add(&(gc)->gpiodev->dev)
>   #define gcdev_unregister(gdev)		device_del(&(gdev)->dev)
>   #endif
>   
> @@ -896,8 +896,9 @@ static const struct device_type gpio_dev_type = {
>    * An initial reference count has been held in gpiochip_add_data_with_key().
>    * The caller should drop the reference via gpio_device_put() on errors.
>    */
> -static int gpiochip_setup_dev(struct gpio_device *gdev)
> +static int gpiochip_setup_dev(struct gpio_chip *gc)
>   {
> +	struct gpio_device *gdev = gc->gpiodev;
>   	struct fwnode_handle *fwnode = dev_fwnode(&gdev->dev);
>   	int ret;
>   
> @@ -910,11 +911,11 @@ static int gpiochip_setup_dev(struct gpio_device *gdev)
>   	if (fwnode && !fwnode->dev)
>   		fwnode_dev_initialized(fwnode, false);
>   
> -	ret = gcdev_register(gdev, gpio_devt);
> +	ret = gcdev_register(gc, gpio_devt);
>   	if (ret)
>   		return ret;
>   
> -	ret = gpiochip_sysfs_register(gdev);
> +	ret = gpiochip_sysfs_register(gc);
>   	if (ret)
>   		goto err_remove_device;
>   
> @@ -961,13 +962,22 @@ static void machine_gpiochip_add(struct gpio_chip *gc)
>   static void gpiochip_setup_devs(void)
>   {
>   	struct gpio_device *gdev;
> +	struct gpio_chip *gc;
>   	int ret;
>   
>   	guard(srcu)(&gpio_devices_srcu);
>   
>   	list_for_each_entry_srcu(gdev, &gpio_devices, list,
>   				 srcu_read_lock_held(&gpio_devices_srcu)) {
> -		ret = gpiochip_setup_dev(gdev);
> +		guard(srcu)(&gdev->srcu);
> +
> +		gc = srcu_dereference(gdev->chip, &gdev->srcu);
> +		if (!gc) {
> +			dev_err(&gdev->dev, "Underlying GPIO chip is gone\n");
> +			continue;
> +		}
> +
> +		ret = gpiochip_setup_dev(gc);
>   		if (ret) {
>   			gpio_device_put(gdev);
>   			dev_err(&gdev->dev,
> @@ -1225,7 +1235,7 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data,
>   	 * (i.e., `gpio_bus_type` is ready).  Otherwise, defer until later.
>   	 */
>   	if (gpiolib_initialized) {
> -		ret = gpiochip_setup_dev(gdev);
> +		ret = gpiochip_setup_dev(gc);
>   		if (ret)
>   			goto err_teardown_shared;
>   	}

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland

Re: [PATCH v4 4/6] gpio: Ensure struct gpio_chip for gpiochip_setup_dev()

[Bartosz Golaszewski]

On Fri, Feb 27, 2026 at 10:36 PM Marek Szyprowski
<m.szyprowski@samsung.com> wrote:
>
> On 23.02.2026 07:17, Tzung-Bi Shih wrote:
> > Ensure struct gpio_chip for gpiochip_setup_dev().  This eliminates a few
> > checks for struct gpio_chip.
> >
> > Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
>
> This patch landed in today's linux-next as commit cf674f1a0c98 ("gpio:
> Ensure struct gpio_chip for gpiochip_setup_dev()"). In my tests I found
> that it triggers a warning on every test board I have, so I suspect that
> something is missing in the code. Here is an example of such warning:
>
> ------------[ cut here ]------------
> WARNING: drivers/gpio/gpiolib-cdev.c:2735 at
> gpiolib_cdev_register+0x114/0x140, CPU#1: swapper/0/1
> Modules linked in:
> CPU: 1 UID: 0 PID: 1 Comm: swapper/0 Not tainted
> 7.0.0-rc1-next-20260227-00065-g6af4b9cfeded #12259 PREEMPT
> Hardware name: Samsung Exynos (Flattened Device Tree)
> Call trace:
>   unwind_backtrace from show_stack+0x10/0x14
>   show_stack from dump_stack_lvl+0x68/0x88
>   dump_stack_lvl from __warn+0x94/0x210
>   __warn from warn_slowpath_fmt+0x1b0/0x1bc
>   warn_slowpath_fmt from gpiolib_cdev_register+0x114/0x140
>   gpiolib_cdev_register from gpiochip_setup_dev+0x4c/0xd0
>   gpiochip_setup_dev from gpiochip_add_data_with_key+0x960/0xad4
>   gpiochip_add_data_with_key from devm_gpiochip_add_data_with_key+0x20/0x5c
>   devm_gpiochip_add_data_with_key from samsung_pinctrl_probe+0x8fc/0xbe8
>   samsung_pinctrl_probe from platform_probe+0x5c/0x98
>   platform_probe from really_probe+0xe0/0x424
>   really_probe from __driver_probe_device+0x9c/0x1f4
>   __driver_probe_device from driver_probe_device+0x30/0xc0
>   driver_probe_device from __device_attach_driver+0xbc/0x180
>   __device_attach_driver from bus_for_each_drv+0x84/0xdc
>   bus_for_each_drv from __device_attach+0xb0/0x214
>   __device_attach from device_initial_probe+0x3c/0x48
>   device_initial_probe from bus_probe_device+0x24/0x80
>   bus_probe_device from device_add+0x5c0/0x810
>   device_add from of_platform_device_create_pdata+0xac/0x104
>   of_platform_device_create_pdata from of_platform_bus_create+0x210/0x534
>   of_platform_bus_create from of_platform_bus_create+0x27c/0x534
>   of_platform_bus_create from of_platform_populate+0x90/0x150
>   of_platform_populate from of_platform_default_populate_init+0xd0/0xe0
>   of_platform_default_populate_init from do_one_initcall+0x70/0x3bc
>   do_one_initcall from kernel_init_freeable+0x1c0/0x248
>   kernel_init_freeable from kernel_init+0x18/0x12c
>   kernel_init from ret_from_fork+0x14/0x28
> Exception stack(0xf082dfb0 to 0xf082dff8)
> dfa0:                                     00000000 00000000 00000000
> 00000000
> dfc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000
> 00000000
> dfe0: 00000000 00000000 00000000 00000000 00000013 00000000
> irq event stamp: 55167
> hardirqs last  enabled at (55387): [<c01c3578>] __up_console_sem+0x50/0x60
> hardirqs last disabled at (55398): [<c01c3564>] __up_console_sem+0x3c/0x60
> softirqs last  enabled at (55352): [<c013c8fc>] handle_softirqs+0x32c/0x5c0
> softirqs last disabled at (55419): [<c013cd3c>] __irq_exit_rcu+0x144/0x1f0
> ---[ end trace 0000000000000000 ]---
>
>
> > ---
> > v4:
> > - To be consistent, rename `chip` -> `gc`.
> > - Add lockdep checks.
> >
> > v3: https://lore.kernel.org/all/20260213092958.864411-5-tzungbi@kernel.org
> > - Pass struct gpio_chip * only.
> >
> > v2: https://lore.kernel.org/all/20260203061059.975605-5-tzungbi@kernel.org
> > - No changes.
> >
> > v1: https://lore.kernel.org/all/20260116081036.352286-10-tzungbi@kernel.org
> >
> >   drivers/gpio/gpiolib-cdev.c  | 14 ++++----------
> >   drivers/gpio/gpiolib-cdev.h  |  2 +-
> >   drivers/gpio/gpiolib-sysfs.c | 21 ++++++++-------------
> >   drivers/gpio/gpiolib-sysfs.h |  4 ++--
> >   drivers/gpio/gpiolib.c       | 24 +++++++++++++++++-------
> >   5 files changed, 32 insertions(+), 33 deletions(-)
> >
> > diff --git a/drivers/gpio/gpiolib-cdev.c b/drivers/gpio/gpiolib-cdev.c
> > index 73ae77f0f213..a154b04e9316 100644
> > --- a/drivers/gpio/gpiolib-cdev.c
> > +++ b/drivers/gpio/gpiolib-cdev.c
> > @@ -2782,11 +2782,13 @@ static const struct file_operations gpio_fileops = {
> >   #endif
> >   };
> >
> > -int gpiolib_cdev_register(struct gpio_device *gdev, dev_t devt)
> > +int gpiolib_cdev_register(struct gpio_chip *gc, dev_t devt)
> >   {
> > -     struct gpio_chip *gc;
> > +     struct gpio_device *gdev = gc->gpiodev;
> >       int ret;
> >
> > +     lockdep_assert_held(&gdev->srcu);
> > +
> >       cdev_init(&gdev->chrdev, &gpio_fileops);
> >       gdev->chrdev.owner = THIS_MODULE;
> >       gdev->dev.devt = MKDEV(MAJOR(devt), gdev->id);
> > @@ -2802,14 +2804,6 @@ int gpiolib_cdev_register(struct gpio_device *gdev, dev_t devt)
> >               return ret;
> >       }
> >
> > -     guard(srcu)(&gdev->srcu);
> > -     gc = srcu_dereference(gdev->chip, &gdev->srcu);
> > -     if (!gc) {
> > -             cdev_device_del(&gdev->chrdev, &gdev->dev);
> > -             destroy_workqueue(gdev->line_state_wq);
> > -             return -ENODEV;
> > -     }
> > -
> >       gpiochip_dbg(gc, "added GPIO chardev (%d:%d)\n", MAJOR(devt), gdev->id);
> >
> >       return 0;
> > diff --git a/drivers/gpio/gpiolib-cdev.h b/drivers/gpio/gpiolib-cdev.h
> > index b42644cbffb8..4a9cb3335d99 100644
> > --- a/drivers/gpio/gpiolib-cdev.h
> > +++ b/drivers/gpio/gpiolib-cdev.h
> > @@ -7,7 +7,7 @@
> >
> >   struct gpio_device;
> >
> > -int gpiolib_cdev_register(struct gpio_device *gdev, dev_t devt);
> > +int gpiolib_cdev_register(struct gpio_chip *gc, dev_t devt);
> >   void gpiolib_cdev_unregister(struct gpio_device *gdev);
> >
> >   #endif /* GPIOLIB_CDEV_H */
> > diff --git a/drivers/gpio/gpiolib-sysfs.c b/drivers/gpio/gpiolib-sysfs.c
> > index 1c25a7dd3db4..748a3eb1bf35 100644
> > --- a/drivers/gpio/gpiolib-sysfs.c
> > +++ b/drivers/gpio/gpiolib-sysfs.c
> > @@ -983,13 +983,15 @@ void gpiod_unexport(struct gpio_desc *desc)
> >   }
> >   EXPORT_SYMBOL_GPL(gpiod_unexport);
> >
> > -int gpiochip_sysfs_register(struct gpio_device *gdev)
> > +int gpiochip_sysfs_register(struct gpio_chip *gc)
> >   {
> > +     struct gpio_device *gdev = gc->gpiodev;
> >       struct gpiodev_data *data;
> > -     struct gpio_chip *chip;
> >       struct device *parent;
> >       int err;
> >
> > +     lockdep_assert_held(&gdev->srcu);
> > +
> >       /*
> >        * Many systems add gpio chips for SOC support very early,
> >        * before driver model support is available.  In those cases we
> > @@ -999,18 +1001,12 @@ int gpiochip_sysfs_register(struct gpio_device *gdev)
> >       if (!class_is_registered(&gpio_class))
> >               return 0;
> >
> > -     guard(srcu)(&gdev->srcu);
> > -
> > -     chip = srcu_dereference(gdev->chip, &gdev->srcu);
> > -     if (!chip)
> > -             return -ENODEV;
> > -
> >       /*
> >        * For sysfs backward compatibility we need to preserve this
> >        * preferred parenting to the gpio_chip parent field, if set.
> >        */
> > -     if (chip->parent)
> > -             parent = chip->parent;
> > +     if (gc->parent)
> > +             parent = gc->parent;
> >       else
> >               parent = &gdev->dev;
> >
> > @@ -1029,7 +1025,7 @@ int gpiochip_sysfs_register(struct gpio_device *gdev)
> >                                                   MKDEV(0, 0), data,
> >                                                   gpiochip_groups,
> >                                                   GPIOCHIP_NAME "%d",
> > -                                                 chip->base);
> > +                                                 gc->base);
> >       if (IS_ERR(data->cdev_base)) {
> >               err = PTR_ERR(data->cdev_base);
> >               kfree(data);
> > @@ -1085,10 +1081,9 @@ void gpiochip_sysfs_unregister(struct gpio_chip *gc)
> >    */
> >   static int gpiofind_sysfs_register(struct gpio_chip *gc, const void *data)
> >   {
> > -     struct gpio_device *gdev = gc->gpiodev;
> >       int ret;
> >
> > -     ret = gpiochip_sysfs_register(gdev);
> > +     ret = gpiochip_sysfs_register(gc);
> >       if (ret)
> >               gpiochip_err(gc, "failed to register the sysfs entry: %d\n", ret);
> >
> > diff --git a/drivers/gpio/gpiolib-sysfs.h b/drivers/gpio/gpiolib-sysfs.h
> > index fd5db5384681..d0998de043a2 100644
> > --- a/drivers/gpio/gpiolib-sysfs.h
> > +++ b/drivers/gpio/gpiolib-sysfs.h
> > @@ -7,12 +7,12 @@ struct gpio_device;
> >
> >   #ifdef CONFIG_GPIO_SYSFS
> >
> > -int gpiochip_sysfs_register(struct gpio_device *gdev);
> > +int gpiochip_sysfs_register(struct gpio_chip *gc);
> >   void gpiochip_sysfs_unregister(struct gpio_chip *gc);
> >
> >   #else
> >
> > -static inline int gpiochip_sysfs_register(struct gpio_device *gdev)
> > +static inline int gpiochip_sysfs_register(struct gpio_chip *gc)
> >   {
> >       return 0;
> >   }
> > diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
> > index d5070c538ba5..44635e9a29c3 100644
> > --- a/drivers/gpio/gpiolib.c
> > +++ b/drivers/gpio/gpiolib.c
> > @@ -881,14 +881,14 @@ static const struct device_type gpio_dev_type = {
> >   };
> >
> >   #ifdef CONFIG_GPIO_CDEV
> > -#define gcdev_register(gdev, devt)   gpiolib_cdev_register((gdev), (devt))
> > +#define gcdev_register(gc, devt)     gpiolib_cdev_register((gc), (devt))
> >   #define gcdev_unregister(gdev)              gpiolib_cdev_unregister((gdev))
> >   #else
> >   /*
> >    * gpiolib_cdev_register() indirectly calls device_add(), which is still
> >    * required even when cdev is not selected.
> >    */
> > -#define gcdev_register(gdev, devt)   device_add(&(gdev)->dev)
> > +#define gcdev_register(gc, devt)     device_add(&(gc)->gpiodev->dev)
> >   #define gcdev_unregister(gdev)              device_del(&(gdev)->dev)
> >   #endif
> >
> > @@ -896,8 +896,9 @@ static const struct device_type gpio_dev_type = {
> >    * An initial reference count has been held in gpiochip_add_data_with_key().
> >    * The caller should drop the reference via gpio_device_put() on errors.
> >    */
> > -static int gpiochip_setup_dev(struct gpio_device *gdev)
> > +static int gpiochip_setup_dev(struct gpio_chip *gc)
> >   {
> > +     struct gpio_device *gdev = gc->gpiodev;
> >       struct fwnode_handle *fwnode = dev_fwnode(&gdev->dev);
> >       int ret;
> >
> > @@ -910,11 +911,11 @@ static int gpiochip_setup_dev(struct gpio_device *gdev)
> >       if (fwnode && !fwnode->dev)
> >               fwnode_dev_initialized(fwnode, false);
> >
> > -     ret = gcdev_register(gdev, gpio_devt);
> > +     ret = gcdev_register(gc, gpio_devt);
> >       if (ret)
> >               return ret;
> >
> > -     ret = gpiochip_sysfs_register(gdev);
> > +     ret = gpiochip_sysfs_register(gc);
> >       if (ret)
> >               goto err_remove_device;
> >
> > @@ -961,13 +962,22 @@ static void machine_gpiochip_add(struct gpio_chip *gc)
> >   static void gpiochip_setup_devs(void)
> >   {
> >       struct gpio_device *gdev;
> > +     struct gpio_chip *gc;
> >       int ret;
> >
> >       guard(srcu)(&gpio_devices_srcu);
> >
> >       list_for_each_entry_srcu(gdev, &gpio_devices, list,
> >                                srcu_read_lock_held(&gpio_devices_srcu)) {
> > -             ret = gpiochip_setup_dev(gdev);
> > +             guard(srcu)(&gdev->srcu);
> > +
> > +             gc = srcu_dereference(gdev->chip, &gdev->srcu);
> > +             if (!gc) {
> > +                     dev_err(&gdev->dev, "Underlying GPIO chip is gone\n");
> > +                     continue;
> > +             }
> > +
> > +             ret = gpiochip_setup_dev(gc);
> >               if (ret) {
> >                       gpio_device_put(gdev);
> >                       dev_err(&gdev->dev,
> > @@ -1225,7 +1235,7 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data,
> >        * (i.e., `gpio_bus_type` is ready).  Otherwise, defer until later.
> >        */
> >       if (gpiolib_initialized) {
> > -             ret = gpiochip_setup_dev(gdev);
> > +             ret = gpiochip_setup_dev(gc);
> >               if (ret)
> >                       goto err_teardown_shared;
> >       }
>

gpiolib_cdev_register() is only called from
gpiochip_add_data_with_key(). I don't think we need the lockdep check
in the former?

Tzung-Bi: Can you take a look at it and confirm?

Bartosz

Re: [PATCH v4 4/6] gpio: Ensure struct gpio_chip for gpiochip_setup_dev()

[Tzung-Bi Shih]

On Sat, Feb 28, 2026 at 11:03:35AM +0100, Bartosz Golaszewski wrote:
> On Fri, Feb 27, 2026 at 10:36 PM Marek Szyprowski
> <m.szyprowski@samsung.com> wrote:
> >
> > On 23.02.2026 07:17, Tzung-Bi Shih wrote:
> > > Ensure struct gpio_chip for gpiochip_setup_dev().  This eliminates a few
> > > checks for struct gpio_chip.
> > >
> > > Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
> >
> > This patch landed in today's linux-next as commit cf674f1a0c98 ("gpio:
> > Ensure struct gpio_chip for gpiochip_setup_dev()"). In my tests I found
> > that it triggers a warning on every test board I have, so I suspect that
> > something is missing in the code. Here is an example of such warning:
> >
> > ------------[ cut here ]------------
> > WARNING: drivers/gpio/gpiolib-cdev.c:2735 at
> > gpiolib_cdev_register+0x114/0x140, CPU#1: swapper/0/1
> > Modules linked in:
> > CPU: 1 UID: 0 PID: 1 Comm: swapper/0 Not tainted
> > 7.0.0-rc1-next-20260227-00065-g6af4b9cfeded #12259 PREEMPT
> > Hardware name: Samsung Exynos (Flattened Device Tree)
> > Call trace:
> >   unwind_backtrace from show_stack+0x10/0x14
> >   show_stack from dump_stack_lvl+0x68/0x88
> >   dump_stack_lvl from __warn+0x94/0x210
> >   __warn from warn_slowpath_fmt+0x1b0/0x1bc
> >   warn_slowpath_fmt from gpiolib_cdev_register+0x114/0x140
> >   gpiolib_cdev_register from gpiochip_setup_dev+0x4c/0xd0
> >   gpiochip_setup_dev from gpiochip_add_data_with_key+0x960/0xad4
> >   gpiochip_add_data_with_key from devm_gpiochip_add_data_with_key+0x20/0x5c
> >   devm_gpiochip_add_data_with_key from samsung_pinctrl_probe+0x8fc/0xbe8
[...]
> > > ---
> > > v4:
> > > - To be consistent, rename `chip` -> `gc`.
> > > - Add lockdep checks.
> > >
[...]
> > > -int gpiolib_cdev_register(struct gpio_device *gdev, dev_t devt)
> > > +int gpiolib_cdev_register(struct gpio_chip *gc, dev_t devt)
> > >   {
> > > -     struct gpio_chip *gc;
> > > +     struct gpio_device *gdev = gc->gpiodev;
> > >       int ret;
> > >
> > > +     lockdep_assert_held(&gdev->srcu);
> > > +
> > >       cdev_init(&gdev->chrdev, &gpio_fileops);
> > >       gdev->chrdev.owner = THIS_MODULE;
> > >       gdev->dev.devt = MKDEV(MAJOR(devt), gdev->id);
> > > @@ -2802,14 +2804,6 @@ int gpiolib_cdev_register(struct gpio_device *gdev, dev_t devt)
> > >               return ret;
> > >       }
> > >
> > > -     guard(srcu)(&gdev->srcu);
> > > -     gc = srcu_dereference(gdev->chip, &gdev->srcu);
> > > -     if (!gc) {
> > > -             cdev_device_del(&gdev->chrdev, &gdev->dev);
> > > -             destroy_workqueue(gdev->line_state_wq);
> > > -             return -ENODEV;
> > > -     }
> > > -
> > >       gpiochip_dbg(gc, "added GPIO chardev (%d:%d)\n", MAJOR(devt), gdev->id);
> > >
> > >       return 0;
[...]
> > > diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
> > > index d5070c538ba5..44635e9a29c3 100644
> > > --- a/drivers/gpio/gpiolib.c
> > > +++ b/drivers/gpio/gpiolib.c
> > > @@ -881,14 +881,14 @@ static const struct device_type gpio_dev_type = {
> > >   };
> > >
> > >   #ifdef CONFIG_GPIO_CDEV
> > > -#define gcdev_register(gdev, devt)   gpiolib_cdev_register((gdev), (devt))
> > > +#define gcdev_register(gc, devt)     gpiolib_cdev_register((gc), (devt))
[...]
> > > -static int gpiochip_setup_dev(struct gpio_device *gdev)
> > > +static int gpiochip_setup_dev(struct gpio_chip *gc)
> > >   {
> > > +     struct gpio_device *gdev = gc->gpiodev;
> > >       struct fwnode_handle *fwnode = dev_fwnode(&gdev->dev);
> > >       int ret;
> > >
> > > @@ -910,11 +911,11 @@ static int gpiochip_setup_dev(struct gpio_device *gdev)
> > >       if (fwnode && !fwnode->dev)
> > >               fwnode_dev_initialized(fwnode, false);
> > >
> > > -     ret = gcdev_register(gdev, gpio_devt);
> > > +     ret = gcdev_register(gc, gpio_devt);
> > >       if (ret)
> > >               return ret;
> > >
> > > -     ret = gpiochip_sysfs_register(gdev);
> > > +     ret = gpiochip_sysfs_register(gc);
> > >       if (ret)
> > >               goto err_remove_device;
> > >
> > > @@ -961,13 +962,22 @@ static void machine_gpiochip_add(struct gpio_chip *gc)
> > >   static void gpiochip_setup_devs(void)
> > >   {
> > >       struct gpio_device *gdev;
> > > +     struct gpio_chip *gc;
> > >       int ret;
> > >
> > >       guard(srcu)(&gpio_devices_srcu);
> > >
> > >       list_for_each_entry_srcu(gdev, &gpio_devices, list,
> > >                                srcu_read_lock_held(&gpio_devices_srcu)) {
> > > -             ret = gpiochip_setup_dev(gdev);
> > > +             guard(srcu)(&gdev->srcu);
> > > +
> > > +             gc = srcu_dereference(gdev->chip, &gdev->srcu);
> > > +             if (!gc) {
> > > +                     dev_err(&gdev->dev, "Underlying GPIO chip is gone\n");
> > > +                     continue;
> > > +             }
> > > +
> > > +             ret = gpiochip_setup_dev(gc);
> > >               if (ret) {
> > >                       gpio_device_put(gdev);
> > >                       dev_err(&gdev->dev,
> > > @@ -1225,7 +1235,7 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data,
> > >        * (i.e., `gpio_bus_type` is ready).  Otherwise, defer until later.
> > >        */
> > >       if (gpiolib_initialized) {
> > > -             ret = gpiochip_setup_dev(gdev);
> > > +             ret = gpiochip_setup_dev(gc);
> > >               if (ret)
> > >                       goto err_teardown_shared;
> > >       }
> >
> 
> gpiolib_cdev_register() is only called from
> gpiochip_add_data_with_key(). I don't think we need the lockdep check
> in the former?

It the calling path is:

  gpiochip_setup_devs()
  -> gpiochip_setup_dev()
  -> ...

The lockdep check should work.

If the calling path is:

  gpiochip_add_data_with_key()
  -> gpiochip_setup_dev()
  -> gcdev_register()
  -> gpiolib_cdev_register()

The SRCU won't be held as `gc` is ensured, and the lockdep check emits
the warning.  gpiochip_sysfs_register() shares the similar concern.

This is easily to reproduce as most cases should fall into the latter calling
path.  I overlooked the case because I always tested with revocable rework
series which eliminates the lockdep checks...

Given that both gpiolib_cdev_register() and gpiochip_sysfs_register() are
only called from gpiolib but no external users, maybe a simple fix is
removing the lockdep checks?

  gpiolib_cdev_register()
  -> (called from) gcdev_register()
    -> (called from) gpiochip_setup_dev()

  gpiochip_sysfs_register()
  -> (called from) gpiochip_setup_dev()
  or
  -> (called from) gpiofind_sysfs_register()
    -> (called from) gpiolib_sysfs_init()

Proposed a fix in:
https://lore.kernel.org/all/20260228131430.102388-1-tzungbi@kernel.org

Re: [PATCH v4 1/6] gpio: Access `gpio_bus_type` in gpiochip_setup_dev()

[Geert Uytterhoeven]

Hi Tzung-Bi,

On Mon, 23 Feb 2026 at 07:17, Tzung-Bi Shih <tzungbi@kernel.org> wrote:
> To make the intent clear, access `gpio_bus_type` only when it's ready in
> gpiochip_setup_dev().
>
> Reviewed-by: Linus Walleij <linusw@kernel.org>
> Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>

Thanks for your patch, which is now commit cc11f4ef666fbca0 ("gpio:
Access `gpio_bus_type` in gpiochip_setup_dev()") in gpio/gpio/for-next.

> --- a/drivers/gpio/gpiolib.c
> +++ b/drivers/gpio/gpiolib.c
> @@ -901,6 +901,8 @@ static int gpiochip_setup_dev(struct gpio_device *gdev)
>         struct fwnode_handle *fwnode = dev_fwnode(&gdev->dev);
>         int ret;
>
> +       gdev->dev.bus = &gpio_bus_type;
> +
>         /*
>          * If fwnode doesn't belong to another device, it's safe to clear its
>          * initialized flag.
> @@ -1082,7 +1084,6 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data,
>          * then make sure they get free():ed there.
>          */
>         gdev->dev.type = &gpio_dev_type;
> -       gdev->dev.bus = &gpio_bus_type;
>         gdev->dev.parent = gc->parent;
>         device_set_node(&gdev->dev, gpiochip_choose_fwnode(gc));
>

Postponing this assignment does have an impact on early
messages. E.g. on RBTX4927:

    -gpio gpiochip0: Static allocation of GPIO base is deprecated, use
dynamic allocation.
    + gpiochip0: Static allocation of GPIO base is deprecated, use
dynamic allocation.

Or with CONFIG_DEBUG_GPIO=y, e.g. on BeagleBone black:

    -gpio gpiochip0: (gpio-0-31): created GPIO range 0->7 ==>
44e10800.pinmux PIN 0->7
    -gpio gpiochip0: (gpio-0-31): created GPIO range 8->11 ==>
44e10800.pinmux PIN 90->93
    -gpio gpiochip0: (gpio-0-31): created GPIO range 12->27 ==>
44e10800.pinmux PIN 12->27
    -gpio gpiochip0: (gpio-0-31): created GPIO range 28->31 ==>
44e10800.pinmux PIN 30->33
    + gpiochip0: (gpio-0-31): created GPIO range 0->7 ==>
44e10800.pinmux PIN 0->7
    + gpiochip0: (gpio-0-31): created GPIO range 8->11 ==>
44e10800.pinmux PIN 90->93
    + gpiochip0: (gpio-0-31): created GPIO range 12->27 ==>
44e10800.pinmux PIN 12->27
    + gpiochip0: (gpio-0-31): created GPIO range 28->31 ==>
44e10800.pinmux PIN 30->33
     [...]

Note the spaces at the beginning of the printed lines.
Reverting the commit re-adds the "gpio" prefix.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH v4 1/6] gpio: Access `gpio_bus_type` in gpiochip_setup_dev()

[Bartosz Golaszewski]

On Wed, 11 Mar 2026 12:44:31 +0100, Geert Uytterhoeven
<geert@linux-m68k.org> said:
> Hi Tzung-Bi,
>
> On Mon, 23 Feb 2026 at 07:17, Tzung-Bi Shih <tzungbi@kernel.org> wrote:
>> To make the intent clear, access `gpio_bus_type` only when it's ready in
>> gpiochip_setup_dev().
>>
>> Reviewed-by: Linus Walleij <linusw@kernel.org>
>> Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
>
> Thanks for your patch, which is now commit cc11f4ef666fbca0 ("gpio:
> Access `gpio_bus_type` in gpiochip_setup_dev()") in gpio/gpio/for-next.
>
>> --- a/drivers/gpio/gpiolib.c
>> +++ b/drivers/gpio/gpiolib.c
>> @@ -901,6 +901,8 @@ static int gpiochip_setup_dev(struct gpio_device *gdev)
>>         struct fwnode_handle *fwnode = dev_fwnode(&gdev->dev);
>>         int ret;
>>
>> +       gdev->dev.bus = &gpio_bus_type;
>> +
>>         /*
>>          * If fwnode doesn't belong to another device, it's safe to clear its
>>          * initialized flag.
>> @@ -1082,7 +1084,6 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data,
>>          * then make sure they get free():ed there.
>>          */
>>         gdev->dev.type = &gpio_dev_type;
>> -       gdev->dev.bus = &gpio_bus_type;
>>         gdev->dev.parent = gc->parent;
>>         device_set_node(&gdev->dev, gpiochip_choose_fwnode(gc));
>>
>
> Postponing this assignment does have an impact on early
> messages. E.g. on RBTX4927:
>
>     -gpio gpiochip0: Static allocation of GPIO base is deprecated, use
> dynamic allocation.
>     + gpiochip0: Static allocation of GPIO base is deprecated, use
> dynamic allocation.
>
> Or with CONFIG_DEBUG_GPIO=y, e.g. on BeagleBone black:
>
>     -gpio gpiochip0: (gpio-0-31): created GPIO range 0->7 ==>
> 44e10800.pinmux PIN 0->7
>     -gpio gpiochip0: (gpio-0-31): created GPIO range 8->11 ==>
> 44e10800.pinmux PIN 90->93
>     -gpio gpiochip0: (gpio-0-31): created GPIO range 12->27 ==>
> 44e10800.pinmux PIN 12->27
>     -gpio gpiochip0: (gpio-0-31): created GPIO range 28->31 ==>
> 44e10800.pinmux PIN 30->33
>     + gpiochip0: (gpio-0-31): created GPIO range 0->7 ==>
> 44e10800.pinmux PIN 0->7
>     + gpiochip0: (gpio-0-31): created GPIO range 8->11 ==>
> 44e10800.pinmux PIN 90->93
>     + gpiochip0: (gpio-0-31): created GPIO range 12->27 ==>
> 44e10800.pinmux PIN 12->27
>     + gpiochip0: (gpio-0-31): created GPIO range 28->31 ==>
> 44e10800.pinmux PIN 30->33
>      [...]
>
> Note the spaces at the beginning of the printed lines.
> Reverting the commit re-adds the "gpio" prefix.
>

As per the comment in gpiochip_add_data_with_key(): we may end up with
a functional chip before gpiochip_setup_dev() is called and so before we
assign the bus type.

dev_printk() helpers only read the name field of the bus type so it should
be safe, I don't see anyone else accessing it before we register it.

I think it makes sense to revert this commit. Tzung-Bi: what do you think?

Bart

Re: [PATCH v4 1/6] gpio: Access `gpio_bus_type` in gpiochip_setup_dev()

[Tzung-Bi Shih]

On Wed, Mar 11, 2026 at 07:36:51AM -0700, Bartosz Golaszewski wrote:
> On Wed, 11 Mar 2026 12:44:31 +0100, Geert Uytterhoeven
> <geert@linux-m68k.org> said:
> > Hi Tzung-Bi,
> >
> > On Mon, 23 Feb 2026 at 07:17, Tzung-Bi Shih <tzungbi@kernel.org> wrote:
> >> To make the intent clear, access `gpio_bus_type` only when it's ready in
> >> gpiochip_setup_dev().
> >>
> >> Reviewed-by: Linus Walleij <linusw@kernel.org>
> >> Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
> >
> > Thanks for your patch, which is now commit cc11f4ef666fbca0 ("gpio:
> > Access `gpio_bus_type` in gpiochip_setup_dev()") in gpio/gpio/for-next.
> >
> >> --- a/drivers/gpio/gpiolib.c
> >> +++ b/drivers/gpio/gpiolib.c
> >> @@ -901,6 +901,8 @@ static int gpiochip_setup_dev(struct gpio_device *gdev)
> >>         struct fwnode_handle *fwnode = dev_fwnode(&gdev->dev);
> >>         int ret;
> >>
> >> +       gdev->dev.bus = &gpio_bus_type;
> >> +
> >>         /*
> >>          * If fwnode doesn't belong to another device, it's safe to clear its
> >>          * initialized flag.
> >> @@ -1082,7 +1084,6 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data,
> >>          * then make sure they get free():ed there.
> >>          */
> >>         gdev->dev.type = &gpio_dev_type;
> >> -       gdev->dev.bus = &gpio_bus_type;
> >>         gdev->dev.parent = gc->parent;
> >>         device_set_node(&gdev->dev, gpiochip_choose_fwnode(gc));
> >>
> >
> > Postponing this assignment does have an impact on early
> > messages. E.g. on RBTX4927:
> >
> >     -gpio gpiochip0: Static allocation of GPIO base is deprecated, use
> > dynamic allocation.
> >     + gpiochip0: Static allocation of GPIO base is deprecated, use
> > dynamic allocation.
> >
> > Or with CONFIG_DEBUG_GPIO=y, e.g. on BeagleBone black:
> >
> >     -gpio gpiochip0: (gpio-0-31): created GPIO range 0->7 ==>
> > 44e10800.pinmux PIN 0->7
> >     -gpio gpiochip0: (gpio-0-31): created GPIO range 8->11 ==>
> > 44e10800.pinmux PIN 90->93
> >     -gpio gpiochip0: (gpio-0-31): created GPIO range 12->27 ==>
> > 44e10800.pinmux PIN 12->27
> >     -gpio gpiochip0: (gpio-0-31): created GPIO range 28->31 ==>
> > 44e10800.pinmux PIN 30->33
> >     + gpiochip0: (gpio-0-31): created GPIO range 0->7 ==>
> > 44e10800.pinmux PIN 0->7
> >     + gpiochip0: (gpio-0-31): created GPIO range 8->11 ==>
> > 44e10800.pinmux PIN 90->93
> >     + gpiochip0: (gpio-0-31): created GPIO range 12->27 ==>
> > 44e10800.pinmux PIN 12->27
> >     + gpiochip0: (gpio-0-31): created GPIO range 28->31 ==>
> > 44e10800.pinmux PIN 30->33
> >      [...]
> >
> > Note the spaces at the beginning of the printed lines.
> > Reverting the commit re-adds the "gpio" prefix.

Thanks for catching this.

> >
> 
> As per the comment in gpiochip_add_data_with_key(): we may end up with
> a functional chip before gpiochip_setup_dev() is called and so before we
> assign the bus type.
> 
> dev_printk() helpers only read the name field of the bus type so it should
> be safe, I don't see anyone else accessing it before we register it.
> 
> I think it makes sense to revert this commit. Tzung-Bi: what do you think?

That makes sense.  I agree, let's revert it.