| Message ID | 20210122193600.1415639-1-saravanak@google.com |
|---|---|
| State | Accepted |
| Commit | 4731210c09f5977300f439b6c56ba220c65b2348 |
| Headers | show |
| Series | [v5] gpiolib: Bind gpio_device to a driver to enable fw_devlink=on by default | expand |
On Fri, Jan 22, 2021 at 8:36 PM Saravana Kannan <saravanak@google.com> wrote: > There are multiple instances of GPIO device tree nodes of the form: > > foo { > compatible = "acme,foo"; > ... > > gpio0: gpio0@xxxxxxxx { > compatible = "acme,bar"; > ... > gpio-controller; > }; > > gpio1: gpio1@xxxxxxxx { > compatible = "acme,bar"; > ... > gpio-controller; > }; > > ... > } > > bazz { > my-gpios = <&gpio0 ...>; > } > > Case 1: The driver for "foo" populates struct device for these gpio* > nodes and then probes them using a driver that binds with "acme,bar". > This driver for "acme,bar" then registers the gpio* nodes with gpiolib. > This lines up with how DT nodes with the "compatible" property are > typically converted to struct devices and then registered with driver > core to probe them. This also allows the gpio* devices to hook into all > the driver core capabilities like runtime PM, probe deferral, > suspend/resume ordering, device links, etc. > > Case 2: The driver for "foo" doesn't populate struct devices for these > gpio* nodes before registering them with gpiolib. Instead it just loops > through its child nodes and directly registers the gpio* nodes with > gpiolib. > > Drivers that follow case 2 cause problems with fw_devlink=on. This is > because fw_devlink will prevent bazz from probing until there's a struct > device that has gpio0 as its fwnode (because bazz lists gpio0 as a GPIO > supplier). Once the struct device is available, fw_devlink will create a > device link with gpio0 device as the supplier and bazz device as the > consumer. After this point, since the gpio0 device will never bind to a > driver, the device link will prevent bazz device from ever probing. > > Finding and refactoring all the instances of drivers that follow case 2 > will cause a lot of code churn and it is not something that can be done > in one shot. In some instances it might not even be possible to refactor > them cleanly. Examples of such instances are [1] [2]. > > This patch works around this problem and avoids all the code churn by > simply setting the fwnode of the gpio_device and creating a stub driver > to bind to the gpio_device. This allows all the consumers to continue > probing when the driver follows case 2. > > [1] - https://lore.kernel.org/lkml/20201014191235.7f71fcb4@xhacker.debian/ > [2] - https://lore.kernel.org/lkml/e28e1f38d87c12a3c714a6573beba6e1@kernel.org/ > Cc: Marc Zyngier <maz@kernel.org> > Cc: Jisheng Zhang <Jisheng.Zhang@synaptics.com> > Cc: Kever Yang <kever.yang@rock-chips.com> > Fixes: e590474768f1 ("driver core: Set fw_devlink=on by default") > Signed-off-by: Saravana Kannan <saravanak@google.com> This v5 version is a beauty! Reviewed-by: Linus Walleij <linus.walleij@linaro.org> Yours, Linus Walleij
On Sat, Jan 23, 2021 at 2:52 PM Linus Walleij <linus.walleij@linaro.org> wrote: > > On Fri, Jan 22, 2021 at 8:36 PM Saravana Kannan <saravanak@google.com> wrote: > > > There are multiple instances of GPIO device tree nodes of the form: > > > > foo { > > compatible = "acme,foo"; > > ... > > > > gpio0: gpio0@xxxxxxxx { > > compatible = "acme,bar"; > > ... > > gpio-controller; > > }; > > > > gpio1: gpio1@xxxxxxxx { > > compatible = "acme,bar"; > > ... > > gpio-controller; > > }; > > > > ... > > } > > > > bazz { > > my-gpios = <&gpio0 ...>; > > } > > > > Case 1: The driver for "foo" populates struct device for these gpio* > > nodes and then probes them using a driver that binds with "acme,bar". > > This driver for "acme,bar" then registers the gpio* nodes with gpiolib. > > This lines up with how DT nodes with the "compatible" property are > > typically converted to struct devices and then registered with driver > > core to probe them. This also allows the gpio* devices to hook into all > > the driver core capabilities like runtime PM, probe deferral, > > suspend/resume ordering, device links, etc. > > > > Case 2: The driver for "foo" doesn't populate struct devices for these > > gpio* nodes before registering them with gpiolib. Instead it just loops > > through its child nodes and directly registers the gpio* nodes with > > gpiolib. > > > > Drivers that follow case 2 cause problems with fw_devlink=on. This is > > because fw_devlink will prevent bazz from probing until there's a struct > > device that has gpio0 as its fwnode (because bazz lists gpio0 as a GPIO > > supplier). Once the struct device is available, fw_devlink will create a > > device link with gpio0 device as the supplier and bazz device as the > > consumer. After this point, since the gpio0 device will never bind to a > > driver, the device link will prevent bazz device from ever probing. > > > > Finding and refactoring all the instances of drivers that follow case 2 > > will cause a lot of code churn and it is not something that can be done > > in one shot. In some instances it might not even be possible to refactor > > them cleanly. Examples of such instances are [1] [2]. > > > > This patch works around this problem and avoids all the code churn by > > simply setting the fwnode of the gpio_device and creating a stub driver > > to bind to the gpio_device. This allows all the consumers to continue > > probing when the driver follows case 2. > > > > [1] - https://lore.kernel.org/lkml/20201014191235.7f71fcb4@xhacker.debian/ > > [2] - https://lore.kernel.org/lkml/e28e1f38d87c12a3c714a6573beba6e1@kernel.org/ > > Cc: Marc Zyngier <maz@kernel.org> > > Cc: Jisheng Zhang <Jisheng.Zhang@synaptics.com> > > Cc: Kever Yang <kever.yang@rock-chips.com> > > Fixes: e590474768f1 ("driver core: Set fw_devlink=on by default") > > Signed-off-by: Saravana Kannan <saravanak@google.com> > > This v5 version is a beauty! Lol, thanks. > Reviewed-by: Linus Walleij <linus.walleij@linaro.org> Considering the "Fixes" is only in driver-core-next, should this go through driver-core? -Saravana
On Sun, Jan 24, 2021 at 3:54 AM Saravana Kannan <saravanak@google.com> wrote: > Considering the "Fixes" is only in driver-core-next, should this go > through driver-core? I think Bartosz should pick it up as a GPIO fix for the -rc:s because it touches code that he is managing. Yours, Linus Walleij
On Sun, Jan 24, 2021 at 11:48:28PM +0100, Linus Walleij wrote: > On Sun, Jan 24, 2021 at 3:54 AM Saravana Kannan <saravanak@google.com> wrote: > > > Considering the "Fixes" is only in driver-core-next, should this go > > through driver-core? > > I think Bartosz should pick it up as a GPIO fix for the -rc:s > because it touches code that he is managing. Ok, that's fine with me, but if not, I can easily take it as well, just let me know. thanks, greg k-h
On Tue, Jan 26, 2021 at 1:40 AM Andy Shevchenko <andy.shevchenko@gmail.com> wrote: > > > > On Friday, January 22, 2021, Saravana Kannan <saravanak@google.com> wrote: >> >> There are multiple instances of GPIO device tree nodes of the form: >> >> foo { >> compatible = "acme,foo"; >> ... >> >> gpio0: gpio0@xxxxxxxx { >> compatible = "acme,bar"; >> ... >> gpio-controller; >> }; >> >> gpio1: gpio1@xxxxxxxx { >> compatible = "acme,bar"; >> ... >> gpio-controller; >> }; >> >> ... >> } >> >> bazz { >> my-gpios = <&gpio0 ...>; >> } >> >> Case 1: The driver for "foo" populates struct device for these gpio* >> nodes and then probes them using a driver that binds with "acme,bar". >> This driver for "acme,bar" then registers the gpio* nodes with gpiolib. >> This lines up with how DT nodes with the "compatible" property are >> typically converted to struct devices and then registered with driver >> core to probe them. This also allows the gpio* devices to hook into all >> the driver core capabilities like runtime PM, probe deferral, >> suspend/resume ordering, device links, etc. >> >> Case 2: The driver for "foo" doesn't populate struct devices for these >> gpio* nodes before registering them with gpiolib. Instead it just loops >> through its child nodes and directly registers the gpio* nodes with >> gpiolib. >> >> Drivers that follow case 2 cause problems with fw_devlink=on. This is >> because fw_devlink will prevent bazz from probing until there's a struct >> device that has gpio0 as its fwnode (because bazz lists gpio0 as a GPIO >> supplier). Once the struct device is available, fw_devlink will create a >> device link with gpio0 device as the supplier and bazz device as the >> consumer. After this point, since the gpio0 device will never bind to a >> driver, the device link will prevent bazz device from ever probing. >> >> Finding and refactoring all the instances of drivers that follow case 2 >> will cause a lot of code churn and it is not something that can be done >> in one shot. In some instances it might not even be possible to refactor >> them cleanly. Examples of such instances are [1] [2]. >> >> This patch works around this problem and avoids all the code churn by >> simply setting the fwnode of the gpio_device and creating a stub driver >> to bind to the gpio_device. This allows all the consumers to continue >> probing when the driver follows case 2. >> > > Do we need to unregister it at __exit initcall? > What side effects would be of the stub driver presence on the GPIO bus? Any traverse on it will work as before? I checked. There is no __exit initcall. -Saravana
On Tue, Jan 26, 2021 at 8:17 PM Saravana Kannan <saravanak@google.com> wrote: > On Tue, Jan 26, 2021 at 1:40 AM Andy Shevchenko > <andy.shevchenko@gmail.com> wrote: > > On Friday, January 22, 2021, Saravana Kannan <saravanak@google.com> wrote: ... > >> Case 1: The driver for "foo" populates struct device for these gpio* the struct > >> nodes and then probes them using a driver that binds with "acme,bar". > >> This driver for "acme,bar" then registers the gpio* nodes with gpiolib. > >> This lines up with how DT nodes with the "compatible" property are > >> typically converted to struct devices and then registered with driver > >> core to probe them. This also allows the gpio* devices to hook into all > >> the driver core capabilities like runtime PM, probe deferral, > >> suspend/resume ordering, device links, etc. > >> > >> Case 2: The driver for "foo" doesn't populate struct devices for these > >> gpio* nodes before registering them with gpiolib. Instead it just loops > >> through its child nodes and directly registers the gpio* nodes with > >> gpiolib. > >> > >> Drivers that follow case 2 cause problems with fw_devlink=on. This is > >> because fw_devlink will prevent bazz from probing until there's a struct prevent the bazz > >> device that has gpio0 as its fwnode (because bazz lists gpio0 as a GPIO > >> supplier). Once the struct device is available, fw_devlink will create a > >> device link with gpio0 device as the supplier and bazz device as the > >> consumer. After this point, since the gpio0 device will never bind to a > >> driver, the device link will prevent bazz device from ever probing. > >> > >> Finding and refactoring all the instances of drivers that follow case 2 > >> will cause a lot of code churn and it is not something that can be done > >> in one shot. In some instances it might not even be possible to refactor > >> them cleanly. Examples of such instances are [1] [2]. > >> > >> This patch works around this problem and avoids all the code churn by > >> simply setting the fwnode of the gpio_device and creating a stub driver > >> to bind to the gpio_device. This allows all the consumers to continue > >> probing when the driver follows case 2. ... > > Do we need to unregister it at __exit initcall? > > What side effects would be of the stub driver presence on the GPIO bus? Any traverse on it will work as before? > > I checked. There is no __exit initcall. You might have checked further out of curiosity, but yeah, I used the attribute name while the initcall name is __exitcall(). -- With Best Regards, Andy Shevchenko
On Fri, Jan 22, 2021 at 11:35:59AM -0800, Saravana Kannan wrote: > There are multiple instances of GPIO device tree nodes of the form: > > foo { > compatible = "acme,foo"; > ... > > gpio0: gpio0@xxxxxxxx { > compatible = "acme,bar"; > ... > gpio-controller; > }; > > gpio1: gpio1@xxxxxxxx { > compatible = "acme,bar"; > ... > gpio-controller; > }; > > ... > } > > bazz { > my-gpios = <&gpio0 ...>; > } > > Case 1: The driver for "foo" populates struct device for these gpio* > nodes and then probes them using a driver that binds with "acme,bar". > This driver for "acme,bar" then registers the gpio* nodes with gpiolib. > This lines up with how DT nodes with the "compatible" property are > typically converted to struct devices and then registered with driver > core to probe them. This also allows the gpio* devices to hook into all > the driver core capabilities like runtime PM, probe deferral, > suspend/resume ordering, device links, etc. > > Case 2: The driver for "foo" doesn't populate struct devices for these > gpio* nodes before registering them with gpiolib. Instead it just loops > through its child nodes and directly registers the gpio* nodes with > gpiolib. > > Drivers that follow case 2 cause problems with fw_devlink=on. This is > because fw_devlink will prevent bazz from probing until there's a struct > device that has gpio0 as its fwnode (because bazz lists gpio0 as a GPIO > supplier). Once the struct device is available, fw_devlink will create a > device link with gpio0 device as the supplier and bazz device as the > consumer. After this point, since the gpio0 device will never bind to a > driver, the device link will prevent bazz device from ever probing. > > Finding and refactoring all the instances of drivers that follow case 2 > will cause a lot of code churn and it is not something that can be done > in one shot. In some instances it might not even be possible to refactor > them cleanly. Examples of such instances are [1] [2]. > > This patch works around this problem and avoids all the code churn by > simply setting the fwnode of the gpio_device and creating a stub driver > to bind to the gpio_device. This allows all the consumers to continue > probing when the driver follows case 2. > > [1] - https://lore.kernel.org/lkml/20201014191235.7f71fcb4@xhacker.debian/ > [2] - https://lore.kernel.org/lkml/e28e1f38d87c12a3c714a6573beba6e1@kernel.org/ > Cc: Marc Zyngier <maz@kernel.org> > Cc: Jisheng Zhang <Jisheng.Zhang@synaptics.com> > Cc: Kever Yang <kever.yang@rock-chips.com> > Fixes: e590474768f1 ("driver core: Set fw_devlink=on by default") As this commit is in my driver-core git tree, can I just take this in the same tree? Can I get an ack from the maintainer for this? thanks, greg k-h
On Wed, Jan 27, 2021 at 3:22 PM Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote: > > On Fri, Jan 22, 2021 at 11:35:59AM -0800, Saravana Kannan wrote: > > There are multiple instances of GPIO device tree nodes of the form: > > > > foo { > > compatible = "acme,foo"; > > ... > > > > gpio0: gpio0@xxxxxxxx { > > compatible = "acme,bar"; > > ... > > gpio-controller; > > }; > > > > gpio1: gpio1@xxxxxxxx { > > compatible = "acme,bar"; > > ... > > gpio-controller; > > }; > > > > ... > > } > > > > bazz { > > my-gpios = <&gpio0 ...>; > > } > > > > Case 1: The driver for "foo" populates struct device for these gpio* > > nodes and then probes them using a driver that binds with "acme,bar". > > This driver for "acme,bar" then registers the gpio* nodes with gpiolib. > > This lines up with how DT nodes with the "compatible" property are > > typically converted to struct devices and then registered with driver > > core to probe them. This also allows the gpio* devices to hook into all > > the driver core capabilities like runtime PM, probe deferral, > > suspend/resume ordering, device links, etc. > > > > Case 2: The driver for "foo" doesn't populate struct devices for these > > gpio* nodes before registering them with gpiolib. Instead it just loops > > through its child nodes and directly registers the gpio* nodes with > > gpiolib. > > > > Drivers that follow case 2 cause problems with fw_devlink=on. This is > > because fw_devlink will prevent bazz from probing until there's a struct > > device that has gpio0 as its fwnode (because bazz lists gpio0 as a GPIO > > supplier). Once the struct device is available, fw_devlink will create a > > device link with gpio0 device as the supplier and bazz device as the > > consumer. After this point, since the gpio0 device will never bind to a > > driver, the device link will prevent bazz device from ever probing. > > > > Finding and refactoring all the instances of drivers that follow case 2 > > will cause a lot of code churn and it is not something that can be done > > in one shot. In some instances it might not even be possible to refactor > > them cleanly. Examples of such instances are [1] [2]. > > > > This patch works around this problem and avoids all the code churn by > > simply setting the fwnode of the gpio_device and creating a stub driver > > to bind to the gpio_device. This allows all the consumers to continue > > probing when the driver follows case 2. > > > > [1] - https://lore.kernel.org/lkml/20201014191235.7f71fcb4@xhacker.debian/ > > [2] - https://lore.kernel.org/lkml/e28e1f38d87c12a3c714a6573beba6e1@kernel.org/ > > Cc: Marc Zyngier <maz@kernel.org> > > Cc: Jisheng Zhang <Jisheng.Zhang@synaptics.com> > > Cc: Kever Yang <kever.yang@rock-chips.com> > > Fixes: e590474768f1 ("driver core: Set fw_devlink=on by default") > > As this commit is in my driver-core git tree, can I just take this in > the same tree? Can I get an ack from the maintainer for this? > > thanks, > > greg k-h Go ahead. Acked-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
On Wed, Jan 27, 2021 at 03:31:17PM +0100, Bartosz Golaszewski wrote: > On Wed, Jan 27, 2021 at 3:22 PM Greg Kroah-Hartman > <gregkh@linuxfoundation.org> wrote: > > > > On Fri, Jan 22, 2021 at 11:35:59AM -0800, Saravana Kannan wrote: > > > There are multiple instances of GPIO device tree nodes of the form: > > > > > > foo { > > > compatible = "acme,foo"; > > > ... > > > > > > gpio0: gpio0@xxxxxxxx { > > > compatible = "acme,bar"; > > > ... > > > gpio-controller; > > > }; > > > > > > gpio1: gpio1@xxxxxxxx { > > > compatible = "acme,bar"; > > > ... > > > gpio-controller; > > > }; > > > > > > ... > > > } > > > > > > bazz { > > > my-gpios = <&gpio0 ...>; > > > } > > > > > > Case 1: The driver for "foo" populates struct device for these gpio* > > > nodes and then probes them using a driver that binds with "acme,bar". > > > This driver for "acme,bar" then registers the gpio* nodes with gpiolib. > > > This lines up with how DT nodes with the "compatible" property are > > > typically converted to struct devices and then registered with driver > > > core to probe them. This also allows the gpio* devices to hook into all > > > the driver core capabilities like runtime PM, probe deferral, > > > suspend/resume ordering, device links, etc. > > > > > > Case 2: The driver for "foo" doesn't populate struct devices for these > > > gpio* nodes before registering them with gpiolib. Instead it just loops > > > through its child nodes and directly registers the gpio* nodes with > > > gpiolib. > > > > > > Drivers that follow case 2 cause problems with fw_devlink=on. This is > > > because fw_devlink will prevent bazz from probing until there's a struct > > > device that has gpio0 as its fwnode (because bazz lists gpio0 as a GPIO > > > supplier). Once the struct device is available, fw_devlink will create a > > > device link with gpio0 device as the supplier and bazz device as the > > > consumer. After this point, since the gpio0 device will never bind to a > > > driver, the device link will prevent bazz device from ever probing. > > > > > > Finding and refactoring all the instances of drivers that follow case 2 > > > will cause a lot of code churn and it is not something that can be done > > > in one shot. In some instances it might not even be possible to refactor > > > them cleanly. Examples of such instances are [1] [2]. > > > > > > This patch works around this problem and avoids all the code churn by > > > simply setting the fwnode of the gpio_device and creating a stub driver > > > to bind to the gpio_device. This allows all the consumers to continue > > > probing when the driver follows case 2. > > > > > > [1] - https://lore.kernel.org/lkml/20201014191235.7f71fcb4@xhacker.debian/ > > > [2] - https://lore.kernel.org/lkml/e28e1f38d87c12a3c714a6573beba6e1@kernel.org/ > > > Cc: Marc Zyngier <maz@kernel.org> > > > Cc: Jisheng Zhang <Jisheng.Zhang@synaptics.com> > > > Cc: Kever Yang <kever.yang@rock-chips.com> > > > Fixes: e590474768f1 ("driver core: Set fw_devlink=on by default") > > > > As this commit is in my driver-core git tree, can I just take this in > > the same tree? Can I get an ack from the maintainer for this? > > > > thanks, > > > > greg k-h > > Go ahead. > > Acked-by: Bartosz Golaszewski <bgolaszewski@baylibre.com> Thanks, now queued up. greg k-h
22.01.2021 22:35, Saravana Kannan пишет: > There are multiple instances of GPIO device tree nodes of the form: > > foo { > compatible = "acme,foo"; > ... > > gpio0: gpio0@xxxxxxxx { > compatible = "acme,bar"; > ... > gpio-controller; > }; > > gpio1: gpio1@xxxxxxxx { > compatible = "acme,bar"; > ... > gpio-controller; > }; > > ... > } > > bazz { > my-gpios = <&gpio0 ...>; > } > > Case 1: The driver for "foo" populates struct device for these gpio* > nodes and then probes them using a driver that binds with "acme,bar". > This driver for "acme,bar" then registers the gpio* nodes with gpiolib. > This lines up with how DT nodes with the "compatible" property are > typically converted to struct devices and then registered with driver > core to probe them. This also allows the gpio* devices to hook into all > the driver core capabilities like runtime PM, probe deferral, > suspend/resume ordering, device links, etc. > > Case 2: The driver for "foo" doesn't populate struct devices for these > gpio* nodes before registering them with gpiolib. Instead it just loops > through its child nodes and directly registers the gpio* nodes with > gpiolib. > > Drivers that follow case 2 cause problems with fw_devlink=on. This is > because fw_devlink will prevent bazz from probing until there's a struct > device that has gpio0 as its fwnode (because bazz lists gpio0 as a GPIO > supplier). Once the struct device is available, fw_devlink will create a > device link with gpio0 device as the supplier and bazz device as the > consumer. After this point, since the gpio0 device will never bind to a > driver, the device link will prevent bazz device from ever probing. > > Finding and refactoring all the instances of drivers that follow case 2 > will cause a lot of code churn and it is not something that can be done > in one shot. In some instances it might not even be possible to refactor > them cleanly. Examples of such instances are [1] [2]. > > This patch works around this problem and avoids all the code churn by > simply setting the fwnode of the gpio_device and creating a stub driver > to bind to the gpio_device. This allows all the consumers to continue > probing when the driver follows case 2. > > [1] - https://lore.kernel.org/lkml/20201014191235.7f71fcb4@xhacker.debian/ > [2] - https://lore.kernel.org/lkml/e28e1f38d87c12a3c714a6573beba6e1@kernel.org/ > Cc: Marc Zyngier <maz@kernel.org> > Cc: Jisheng Zhang <Jisheng.Zhang@synaptics.com> > Cc: Kever Yang <kever.yang@rock-chips.com> > Fixes: e590474768f1 ("driver core: Set fw_devlink=on by default") > Signed-off-by: Saravana Kannan <saravanak@google.com> > --- > v1 -> v2: > - Fixed up compilation errors that were introduced accidentally > - Fixed a missing put_device() > > v2 -> v3: > - Changed chip_warn() to pr_warn() > - Changed some variable names > > v3 -> v4: > - Dropped the warning since it's not always valid > - This simplifies the code a lot > - Added comments and fixed up commit text > > v4 -> v5: > - Fixed the code to not mess up non-DT cases. > - Moved code into gpiolib-of.c > > drivers/gpio/gpiolib-of.c | 11 +++++++++++ > drivers/gpio/gpiolib-of.h | 5 +++++ > drivers/gpio/gpiolib.c | 38 +++++++++++++++++++++++++++++++------- > 3 files changed, 47 insertions(+), 7 deletions(-) > > diff --git a/drivers/gpio/gpiolib-of.c b/drivers/gpio/gpiolib-of.c > index b4a71119a4b0..baf0153b7bca 100644 > --- a/drivers/gpio/gpiolib-of.c > +++ b/drivers/gpio/gpiolib-of.c > @@ -1039,3 +1039,14 @@ void of_gpiochip_remove(struct gpio_chip *chip) > { > of_node_put(chip->of_node); > } > + > +void of_gpio_dev_init(struct gpio_chip *gc, struct gpio_device *gdev) > +{ > + /* If the gpiochip has an assigned OF node this takes precedence */ > + if (gc->of_node) > + gdev->dev.of_node = gc->of_node; > + else > + gc->of_node = gdev->dev.of_node; > + if (gdev->dev.of_node) > + gdev->dev.fwnode = of_fwnode_handle(gdev->dev.of_node); > +} > diff --git a/drivers/gpio/gpiolib-of.h b/drivers/gpio/gpiolib-of.h > index ed26664f1537..8af2bc899aab 100644 > --- a/drivers/gpio/gpiolib-of.h > +++ b/drivers/gpio/gpiolib-of.h > @@ -15,6 +15,7 @@ int of_gpiochip_add(struct gpio_chip *gc); > void of_gpiochip_remove(struct gpio_chip *gc); > int of_gpio_get_count(struct device *dev, const char *con_id); > bool of_gpio_need_valid_mask(const struct gpio_chip *gc); > +void of_gpio_dev_init(struct gpio_chip *gc, struct gpio_device *gdev); > #else > static inline struct gpio_desc *of_find_gpio(struct device *dev, > const char *con_id, > @@ -33,6 +34,10 @@ static inline bool of_gpio_need_valid_mask(const struct gpio_chip *gc) > { > return false; > } > +static inline void of_gpio_dev_init(struct gpio_chip *gc, > + struct gpio_device *gdev) > +{ > +} > #endif /* CONFIG_OF_GPIO */ > > extern struct notifier_block gpio_of_notifier; > diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c > index b02cc2abd3b6..70fb15ae5d36 100644 > --- a/drivers/gpio/gpiolib.c > +++ b/drivers/gpio/gpiolib.c > @@ -590,13 +590,7 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data, > gdev->dev.of_node = gc->parent->of_node; > } > > -#ifdef CONFIG_OF_GPIO > - /* If the gpiochip has an assigned OF node this takes precedence */ > - if (gc->of_node) > - gdev->dev.of_node = gc->of_node; > - else > - gc->of_node = gdev->dev.of_node; > -#endif > + of_gpio_dev_init(gc, gdev); > > gdev->id = ida_alloc(&gpio_ida, GFP_KERNEL); > if (gdev->id < 0) { > @@ -4202,6 +4196,29 @@ void gpiod_put_array(struct gpio_descs *descs) > } > EXPORT_SYMBOL_GPL(gpiod_put_array); > > +static int gpio_stub_drv_probe(struct device *dev) > +{ > + /* > + * The DT node of some GPIO chips have a "compatible" property, but > + * never have a struct device added and probed by a driver to register > + * the GPIO chip with gpiolib. In such cases, fw_devlink=on will cause > + * the consumers of the GPIO chip to get probe deferred forever because > + * they will be waiting for a device associated with the GPIO chip > + * firmware node to get added and bound to a driver. > + * > + * To allow these consumers to probe, we associate the struct > + * gpio_device of the GPIO chip with the firmware node and then simply > + * bind it to this stub driver. > + */ > + return 0; > +} > + > +static struct device_driver gpio_stub_drv = { > + .name = "gpio_stub_drv", > + .bus = &gpio_bus_type, > + .probe = gpio_stub_drv_probe, > +}; > + > static int __init gpiolib_dev_init(void) > { > int ret; > @@ -4213,9 +4230,16 @@ static int __init gpiolib_dev_init(void) > return ret; > } > > + if (driver_register(&gpio_stub_drv) < 0) { > + pr_err("gpiolib: could not register GPIO stub driver\n"); > + bus_unregister(&gpio_bus_type); > + return ret; > + } > + > ret = alloc_chrdev_region(&gpio_devt, 0, GPIO_DEV_MAX, GPIOCHIP_NAME); > if (ret < 0) { > pr_err("gpiolib: failed to allocate char dev region\n"); > + driver_unregister(&gpio_stub_drv); > bus_unregister(&gpio_bus_type); > return ret; > } > Hi, This patch causes these new errors on NVIDIA Tegra30 Nexus 7 using recent linux-next: gpio-1022 (cpu-pwr-req-hog): hogged as input max77620-pinctrl max77620-pinctrl: pin gpio4 already requested by max77620-pinctrl; cannot claim for gpiochip1 max77620-pinctrl max77620-pinctrl: pin-4 (gpiochip1) status -22 max77620-pinctrl max77620-pinctrl: could not request pin 4 (gpio4) from group gpio4 on device max77620-pinctrl gpio_stub_drv gpiochip1: Error applying setting, reverse things back gpio_stub_drv: probe of gpiochip1 failed with error -22 Please fix, thanks in advance.
On Sat, Jan 30, 2021 at 9:39 AM Dmitry Osipenko <digetx@gmail.com> wrote: > > 22.01.2021 22:35, Saravana Kannan пишет: > > There are multiple instances of GPIO device tree nodes of the form: > > > > foo { > > compatible = "acme,foo"; > > ... > > > > gpio0: gpio0@xxxxxxxx { > > compatible = "acme,bar"; > > ... > > gpio-controller; > > }; > > > > gpio1: gpio1@xxxxxxxx { > > compatible = "acme,bar"; > > ... > > gpio-controller; > > }; > > > > ... > > } > > > > bazz { > > my-gpios = <&gpio0 ...>; > > } > > > > Case 1: The driver for "foo" populates struct device for these gpio* > > nodes and then probes them using a driver that binds with "acme,bar". > > This driver for "acme,bar" then registers the gpio* nodes with gpiolib. > > This lines up with how DT nodes with the "compatible" property are > > typically converted to struct devices and then registered with driver > > core to probe them. This also allows the gpio* devices to hook into all > > the driver core capabilities like runtime PM, probe deferral, > > suspend/resume ordering, device links, etc. > > > > Case 2: The driver for "foo" doesn't populate struct devices for these > > gpio* nodes before registering them with gpiolib. Instead it just loops > > through its child nodes and directly registers the gpio* nodes with > > gpiolib. > > > > Drivers that follow case 2 cause problems with fw_devlink=on. This is > > because fw_devlink will prevent bazz from probing until there's a struct > > device that has gpio0 as its fwnode (because bazz lists gpio0 as a GPIO > > supplier). Once the struct device is available, fw_devlink will create a > > device link with gpio0 device as the supplier and bazz device as the > > consumer. After this point, since the gpio0 device will never bind to a > > driver, the device link will prevent bazz device from ever probing. > > > > Finding and refactoring all the instances of drivers that follow case 2 > > will cause a lot of code churn and it is not something that can be done > > in one shot. In some instances it might not even be possible to refactor > > them cleanly. Examples of such instances are [1] [2]. > > > > This patch works around this problem and avoids all the code churn by > > simply setting the fwnode of the gpio_device and creating a stub driver > > to bind to the gpio_device. This allows all the consumers to continue > > probing when the driver follows case 2. > > > > [1] - https://lore.kernel.org/lkml/20201014191235.7f71fcb4@xhacker.debian/ > > [2] - https://lore.kernel.org/lkml/e28e1f38d87c12a3c714a6573beba6e1@kernel.org/ > > Cc: Marc Zyngier <maz@kernel.org> > > Cc: Jisheng Zhang <Jisheng.Zhang@synaptics.com> > > Cc: Kever Yang <kever.yang@rock-chips.com> > > Fixes: e590474768f1 ("driver core: Set fw_devlink=on by default") > > Signed-off-by: Saravana Kannan <saravanak@google.com> > > --- > > v1 -> v2: > > - Fixed up compilation errors that were introduced accidentally > > - Fixed a missing put_device() > > > > v2 -> v3: > > - Changed chip_warn() to pr_warn() > > - Changed some variable names > > > > v3 -> v4: > > - Dropped the warning since it's not always valid > > - This simplifies the code a lot > > - Added comments and fixed up commit text > > > > v4 -> v5: > > - Fixed the code to not mess up non-DT cases. > > - Moved code into gpiolib-of.c > > > > drivers/gpio/gpiolib-of.c | 11 +++++++++++ > > drivers/gpio/gpiolib-of.h | 5 +++++ > > drivers/gpio/gpiolib.c | 38 +++++++++++++++++++++++++++++++------- > > 3 files changed, 47 insertions(+), 7 deletions(-) > > > > diff --git a/drivers/gpio/gpiolib-of.c b/drivers/gpio/gpiolib-of.c > > index b4a71119a4b0..baf0153b7bca 100644 > > --- a/drivers/gpio/gpiolib-of.c > > +++ b/drivers/gpio/gpiolib-of.c > > @@ -1039,3 +1039,14 @@ void of_gpiochip_remove(struct gpio_chip *chip) > > { > > of_node_put(chip->of_node); > > } > > + > > +void of_gpio_dev_init(struct gpio_chip *gc, struct gpio_device *gdev) > > +{ > > + /* If the gpiochip has an assigned OF node this takes precedence */ > > + if (gc->of_node) > > + gdev->dev.of_node = gc->of_node; > > + else > > + gc->of_node = gdev->dev.of_node; > > + if (gdev->dev.of_node) > > + gdev->dev.fwnode = of_fwnode_handle(gdev->dev.of_node); > > +} > > diff --git a/drivers/gpio/gpiolib-of.h b/drivers/gpio/gpiolib-of.h > > index ed26664f1537..8af2bc899aab 100644 > > --- a/drivers/gpio/gpiolib-of.h > > +++ b/drivers/gpio/gpiolib-of.h > > @@ -15,6 +15,7 @@ int of_gpiochip_add(struct gpio_chip *gc); > > void of_gpiochip_remove(struct gpio_chip *gc); > > int of_gpio_get_count(struct device *dev, const char *con_id); > > bool of_gpio_need_valid_mask(const struct gpio_chip *gc); > > +void of_gpio_dev_init(struct gpio_chip *gc, struct gpio_device *gdev); > > #else > > static inline struct gpio_desc *of_find_gpio(struct device *dev, > > const char *con_id, > > @@ -33,6 +34,10 @@ static inline bool of_gpio_need_valid_mask(const struct gpio_chip *gc) > > { > > return false; > > } > > +static inline void of_gpio_dev_init(struct gpio_chip *gc, > > + struct gpio_device *gdev) > > +{ > > +} > > #endif /* CONFIG_OF_GPIO */ > > > > extern struct notifier_block gpio_of_notifier; > > diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c > > index b02cc2abd3b6..70fb15ae5d36 100644 > > --- a/drivers/gpio/gpiolib.c > > +++ b/drivers/gpio/gpiolib.c > > @@ -590,13 +590,7 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data, > > gdev->dev.of_node = gc->parent->of_node; > > } > > > > -#ifdef CONFIG_OF_GPIO > > - /* If the gpiochip has an assigned OF node this takes precedence */ > > - if (gc->of_node) > > - gdev->dev.of_node = gc->of_node; > > - else > > - gc->of_node = gdev->dev.of_node; > > -#endif > > + of_gpio_dev_init(gc, gdev); > > > > gdev->id = ida_alloc(&gpio_ida, GFP_KERNEL); > > if (gdev->id < 0) { > > @@ -4202,6 +4196,29 @@ void gpiod_put_array(struct gpio_descs *descs) > > } > > EXPORT_SYMBOL_GPL(gpiod_put_array); > > > > +static int gpio_stub_drv_probe(struct device *dev) > > +{ > > + /* > > + * The DT node of some GPIO chips have a "compatible" property, but > > + * never have a struct device added and probed by a driver to register > > + * the GPIO chip with gpiolib. In such cases, fw_devlink=on will cause > > + * the consumers of the GPIO chip to get probe deferred forever because > > + * they will be waiting for a device associated with the GPIO chip > > + * firmware node to get added and bound to a driver. > > + * > > + * To allow these consumers to probe, we associate the struct > > + * gpio_device of the GPIO chip with the firmware node and then simply > > + * bind it to this stub driver. > > + */ > > + return 0; > > +} > > + > > +static struct device_driver gpio_stub_drv = { > > + .name = "gpio_stub_drv", > > + .bus = &gpio_bus_type, > > + .probe = gpio_stub_drv_probe, > > +}; > > + > > static int __init gpiolib_dev_init(void) > > { > > int ret; > > @@ -4213,9 +4230,16 @@ static int __init gpiolib_dev_init(void) > > return ret; > > } > > > > + if (driver_register(&gpio_stub_drv) < 0) { > > + pr_err("gpiolib: could not register GPIO stub driver\n"); > > + bus_unregister(&gpio_bus_type); > > + return ret; > > + } > > + > > ret = alloc_chrdev_region(&gpio_devt, 0, GPIO_DEV_MAX, GPIOCHIP_NAME); > > if (ret < 0) { > > pr_err("gpiolib: failed to allocate char dev region\n"); > > + driver_unregister(&gpio_stub_drv); > > bus_unregister(&gpio_bus_type); > > return ret; > > } > > > > Hi, > > This patch causes these new errors on NVIDIA Tegra30 Nexus 7 using recent linux-next: > > gpio-1022 (cpu-pwr-req-hog): hogged as input > max77620-pinctrl max77620-pinctrl: pin gpio4 already requested by max77620-pinctrl; cannot claim for gpiochip1 > max77620-pinctrl max77620-pinctrl: pin-4 (gpiochip1) status -22 > max77620-pinctrl max77620-pinctrl: could not request pin 4 (gpio4) from group gpio4 on device max77620-pinctrl > gpio_stub_drv gpiochip1: Error applying setting, reverse things back > gpio_stub_drv: probe of gpiochip1 failed with error -22 > > Please fix, thanks in advance. I have a partial guess on why this is happening. So can you try this patch? Thanks, Saravana --- a/drivers/gpio/gpiolib.c +++ b/drivers/gpio/gpiolib.c @@ -4213,6 +4213,8 @@ static int gpio_stub_drv_probe(struct device *dev) * gpio_device of the GPIO chip with the firmware node and then simply * bind it to this stub driver. */ + if (dev->fwnode && dev->fwnode->dev != dev) + return -EBUSY; return 0; }
01.02.2021 00:28, Saravana Kannan пишет: >> This patch causes these new errors on NVIDIA Tegra30 Nexus 7 using recent linux-next: >> >> gpio-1022 (cpu-pwr-req-hog): hogged as input >> max77620-pinctrl max77620-pinctrl: pin gpio4 already requested by max77620-pinctrl; cannot claim for gpiochip1 >> max77620-pinctrl max77620-pinctrl: pin-4 (gpiochip1) status -22 >> max77620-pinctrl max77620-pinctrl: could not request pin 4 (gpio4) from group gpio4 on device max77620-pinctrl >> gpio_stub_drv gpiochip1: Error applying setting, reverse things back >> gpio_stub_drv: probe of gpiochip1 failed with error -22 >> >> Please fix, thanks in advance. > I have a partial guess on why this is happening. So can you try this patch? > > Thanks, > Saravana > > --- a/drivers/gpio/gpiolib.c > +++ b/drivers/gpio/gpiolib.c > @@ -4213,6 +4213,8 @@ static int gpio_stub_drv_probe(struct device *dev) > * gpio_device of the GPIO chip with the firmware node and then simply > * bind it to this stub driver. > */ > + if (dev->fwnode && dev->fwnode->dev != dev) > + return -EBUSY; > return 0; > } This change doesn't help, exactly the same errors are still there.
On Mon, Feb 1, 2021 at 8:49 AM Dmitry Osipenko <digetx@gmail.com> wrote: > > 01.02.2021 00:28, Saravana Kannan пишет: > >> This patch causes these new errors on NVIDIA Tegra30 Nexus 7 using recent linux-next: > >> > >> gpio-1022 (cpu-pwr-req-hog): hogged as input > >> max77620-pinctrl max77620-pinctrl: pin gpio4 already requested by max77620-pinctrl; cannot claim for gpiochip1 > >> max77620-pinctrl max77620-pinctrl: pin-4 (gpiochip1) status -22 > >> max77620-pinctrl max77620-pinctrl: could not request pin 4 (gpio4) from group gpio4 on device max77620-pinctrl > >> gpio_stub_drv gpiochip1: Error applying setting, reverse things back > >> gpio_stub_drv: probe of gpiochip1 failed with error -22 > >> > >> Please fix, thanks in advance. > > I have a partial guess on why this is happening. So can you try this patch? > > > > Thanks, > > Saravana > > > > --- a/drivers/gpio/gpiolib.c > > +++ b/drivers/gpio/gpiolib.c > > @@ -4213,6 +4213,8 @@ static int gpio_stub_drv_probe(struct device *dev) > > * gpio_device of the GPIO chip with the firmware node and then simply > > * bind it to this stub driver. > > */ > > + if (dev->fwnode && dev->fwnode->dev != dev) > > + return -EBUSY; > > return 0; > > } > > This change doesn't help, exactly the same errors are still there. Sorry, I see what's happening. Try this instead. If it works, I'll send out a proper patch. Thanks, Saravana diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c index 8e0564c50840..f3d0ffe8a930 100644 --- a/drivers/gpio/gpiolib.c +++ b/drivers/gpio/gpiolib.c @@ -56,8 +56,10 @@ static DEFINE_IDA(gpio_ida); static dev_t gpio_devt; #define GPIO_DEV_MAX 256 /* 256 GPIO chip devices supported */ +static int gpio_bus_match(struct device *dev, struct device_driver *drv); static struct bus_type gpio_bus_type = { .name = "gpio", + .match = gpio_bus_match, }; /* @@ -4199,6 +4201,14 @@ void gpiod_put_array(struct gpio_descs *descs) } EXPORT_SYMBOL_GPL(gpiod_put_array); + +static int gpio_bus_match(struct device *dev, struct device_driver *drv) +{ + if (dev->fwnode && dev->fwnode->dev != dev) + return 0; + return 1; +} + static int gpio_stub_drv_probe(struct device *dev) { /*
01.02.2021 23:15, Saravana Kannan пишет: > On Mon, Feb 1, 2021 at 8:49 AM Dmitry Osipenko <digetx@gmail.com> wrote: >> >> 01.02.2021 00:28, Saravana Kannan пишет: >>>> This patch causes these new errors on NVIDIA Tegra30 Nexus 7 using recent linux-next: >>>> >>>> gpio-1022 (cpu-pwr-req-hog): hogged as input >>>> max77620-pinctrl max77620-pinctrl: pin gpio4 already requested by max77620-pinctrl; cannot claim for gpiochip1 >>>> max77620-pinctrl max77620-pinctrl: pin-4 (gpiochip1) status -22 >>>> max77620-pinctrl max77620-pinctrl: could not request pin 4 (gpio4) from group gpio4 on device max77620-pinctrl >>>> gpio_stub_drv gpiochip1: Error applying setting, reverse things back >>>> gpio_stub_drv: probe of gpiochip1 failed with error -22 >>>> >>>> Please fix, thanks in advance. >>> I have a partial guess on why this is happening. So can you try this patch? >>> >>> Thanks, >>> Saravana >>> >>> --- a/drivers/gpio/gpiolib.c >>> +++ b/drivers/gpio/gpiolib.c >>> @@ -4213,6 +4213,8 @@ static int gpio_stub_drv_probe(struct device *dev) >>> * gpio_device of the GPIO chip with the firmware node and then simply >>> * bind it to this stub driver. >>> */ >>> + if (dev->fwnode && dev->fwnode->dev != dev) >>> + return -EBUSY; >>> return 0; >>> } >> >> This change doesn't help, exactly the same errors are still there. > > Sorry, I see what's happening. Try this instead. If it works, I'll > send out a proper patch. > > Thanks, > Saravana > > diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c > index 8e0564c50840..f3d0ffe8a930 100644 > --- a/drivers/gpio/gpiolib.c > +++ b/drivers/gpio/gpiolib.c > @@ -56,8 +56,10 @@ > static DEFINE_IDA(gpio_ida); > static dev_t gpio_devt; > #define GPIO_DEV_MAX 256 /* 256 GPIO chip devices supported */ > +static int gpio_bus_match(struct device *dev, struct device_driver *drv); > static struct bus_type gpio_bus_type = { > .name = "gpio", > + .match = gpio_bus_match, > }; > > /* > @@ -4199,6 +4201,14 @@ void gpiod_put_array(struct gpio_descs *descs) > } > EXPORT_SYMBOL_GPL(gpiod_put_array); > > + > +static int gpio_bus_match(struct device *dev, struct device_driver *drv) > +{ > + if (dev->fwnode && dev->fwnode->dev != dev) > + return 0; > + return 1; > +} > + > static int gpio_stub_drv_probe(struct device *dev) > { > /* > This works, thank you! Tested-by: Dmitry Osipenko <digetx@gmail.com>
diff --git a/drivers/gpio/gpiolib-of.c b/drivers/gpio/gpiolib-of.c index b4a71119a4b0..baf0153b7bca 100644 --- a/drivers/gpio/gpiolib-of.c +++ b/drivers/gpio/gpiolib-of.c @@ -1039,3 +1039,14 @@ void of_gpiochip_remove(struct gpio_chip *chip) { of_node_put(chip->of_node); } + +void of_gpio_dev_init(struct gpio_chip *gc, struct gpio_device *gdev) +{ + /* If the gpiochip has an assigned OF node this takes precedence */ + if (gc->of_node) + gdev->dev.of_node = gc->of_node; + else + gc->of_node = gdev->dev.of_node; + if (gdev->dev.of_node) + gdev->dev.fwnode = of_fwnode_handle(gdev->dev.of_node); +} diff --git a/drivers/gpio/gpiolib-of.h b/drivers/gpio/gpiolib-of.h index ed26664f1537..8af2bc899aab 100644 --- a/drivers/gpio/gpiolib-of.h +++ b/drivers/gpio/gpiolib-of.h @@ -15,6 +15,7 @@ int of_gpiochip_add(struct gpio_chip *gc); void of_gpiochip_remove(struct gpio_chip *gc); int of_gpio_get_count(struct device *dev, const char *con_id); bool of_gpio_need_valid_mask(const struct gpio_chip *gc); +void of_gpio_dev_init(struct gpio_chip *gc, struct gpio_device *gdev); #else static inline struct gpio_desc *of_find_gpio(struct device *dev, const char *con_id, @@ -33,6 +34,10 @@ static inline bool of_gpio_need_valid_mask(const struct gpio_chip *gc) { return false; } +static inline void of_gpio_dev_init(struct gpio_chip *gc, + struct gpio_device *gdev) +{ +} #endif /* CONFIG_OF_GPIO */ extern struct notifier_block gpio_of_notifier; diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c index b02cc2abd3b6..70fb15ae5d36 100644 --- a/drivers/gpio/gpiolib.c +++ b/drivers/gpio/gpiolib.c @@ -590,13 +590,7 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data, gdev->dev.of_node = gc->parent->of_node; } -#ifdef CONFIG_OF_GPIO - /* If the gpiochip has an assigned OF node this takes precedence */ - if (gc->of_node) - gdev->dev.of_node = gc->of_node; - else - gc->of_node = gdev->dev.of_node; -#endif + of_gpio_dev_init(gc, gdev); gdev->id = ida_alloc(&gpio_ida, GFP_KERNEL); if (gdev->id < 0) { @@ -4202,6 +4196,29 @@ void gpiod_put_array(struct gpio_descs *descs) } EXPORT_SYMBOL_GPL(gpiod_put_array); +static int gpio_stub_drv_probe(struct device *dev) +{ + /* + * The DT node of some GPIO chips have a "compatible" property, but + * never have a struct device added and probed by a driver to register + * the GPIO chip with gpiolib. In such cases, fw_devlink=on will cause + * the consumers of the GPIO chip to get probe deferred forever because + * they will be waiting for a device associated with the GPIO chip + * firmware node to get added and bound to a driver. + * + * To allow these consumers to probe, we associate the struct + * gpio_device of the GPIO chip with the firmware node and then simply + * bind it to this stub driver. + */ + return 0; +} + +static struct device_driver gpio_stub_drv = { + .name = "gpio_stub_drv", + .bus = &gpio_bus_type, + .probe = gpio_stub_drv_probe, +}; + static int __init gpiolib_dev_init(void) { int ret; @@ -4213,9 +4230,16 @@ static int __init gpiolib_dev_init(void) return ret; } + if (driver_register(&gpio_stub_drv) < 0) { + pr_err("gpiolib: could not register GPIO stub driver\n"); + bus_unregister(&gpio_bus_type); + return ret; + } + ret = alloc_chrdev_region(&gpio_devt, 0, GPIO_DEV_MAX, GPIOCHIP_NAME); if (ret < 0) { pr_err("gpiolib: failed to allocate char dev region\n"); + driver_unregister(&gpio_stub_drv); bus_unregister(&gpio_bus_type); return ret; }
There are multiple instances of GPIO device tree nodes of the form: foo { compatible = "acme,foo"; ... gpio0: gpio0@xxxxxxxx { compatible = "acme,bar"; ... gpio-controller; }; gpio1: gpio1@xxxxxxxx { compatible = "acme,bar"; ... gpio-controller; }; ... } bazz { my-gpios = <&gpio0 ...>; } Case 1: The driver for "foo" populates struct device for these gpio* nodes and then probes them using a driver that binds with "acme,bar". This driver for "acme,bar" then registers the gpio* nodes with gpiolib. This lines up with how DT nodes with the "compatible" property are typically converted to struct devices and then registered with driver core to probe them. This also allows the gpio* devices to hook into all the driver core capabilities like runtime PM, probe deferral, suspend/resume ordering, device links, etc. Case 2: The driver for "foo" doesn't populate struct devices for these gpio* nodes before registering them with gpiolib. Instead it just loops through its child nodes and directly registers the gpio* nodes with gpiolib. Drivers that follow case 2 cause problems with fw_devlink=on. This is because fw_devlink will prevent bazz from probing until there's a struct device that has gpio0 as its fwnode (because bazz lists gpio0 as a GPIO supplier). Once the struct device is available, fw_devlink will create a device link with gpio0 device as the supplier and bazz device as the consumer. After this point, since the gpio0 device will never bind to a driver, the device link will prevent bazz device from ever probing. Finding and refactoring all the instances of drivers that follow case 2 will cause a lot of code churn and it is not something that can be done in one shot. In some instances it might not even be possible to refactor them cleanly. Examples of such instances are [1] [2]. This patch works around this problem and avoids all the code churn by simply setting the fwnode of the gpio_device and creating a stub driver to bind to the gpio_device. This allows all the consumers to continue probing when the driver follows case 2. [1] - https://lore.kernel.org/lkml/20201014191235.7f71fcb4@xhacker.debian/ [2] - https://lore.kernel.org/lkml/e28e1f38d87c12a3c714a6573beba6e1@kernel.org/ Cc: Marc Zyngier <maz@kernel.org> Cc: Jisheng Zhang <Jisheng.Zhang@synaptics.com> Cc: Kever Yang <kever.yang@rock-chips.com> Fixes: e590474768f1 ("driver core: Set fw_devlink=on by default") Signed-off-by: Saravana Kannan <saravanak@google.com> --- v1 -> v2: - Fixed up compilation errors that were introduced accidentally - Fixed a missing put_device() v2 -> v3: - Changed chip_warn() to pr_warn() - Changed some variable names v3 -> v4: - Dropped the warning since it's not always valid - This simplifies the code a lot - Added comments and fixed up commit text v4 -> v5: - Fixed the code to not mess up non-DT cases. - Moved code into gpiolib-of.c drivers/gpio/gpiolib-of.c | 11 +++++++++++ drivers/gpio/gpiolib-of.h | 5 +++++ drivers/gpio/gpiolib.c | 38 +++++++++++++++++++++++++++++++------- 3 files changed, 47 insertions(+), 7 deletions(-)