On Fri, Feb 27, 2026 at 10:36 PM Marek Szyprowski <[email protected]> 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 <[email protected]> > > 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/[email protected] > > - Pass struct gpio_chip * only. > > > > v2: https://lore.kernel.org/all/[email protected] > > - No changes. > > > > v1: https://lore.kernel.org/all/[email protected] > > > > 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
