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