On Sat, Feb 28, 2026 at 11:03:35AM +0100, Bartosz Golaszewski wrote:
> 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
[...]
> > > ---
> > > 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/[email protected]
