Re: [PATCH v3] gpiolib: match secondary fwnode too in gpio_device_find_by_fwnode()

[Rafael J. Wysocki]

On Wed, Feb 25, 2026 at 11:12 AM Bartosz Golaszewski
<bartosz.golaszewski@xxxxxxxxxxxxxxxx> wrote:
>
> In GPIOLIB, during fwnode lookup, after having resolved the consumer's
> reference to a specific fwnode, we only match it against the primary
> node of the controllers. Let's extend that to also the secondary node by
> reworking gpio_chip_match_by_fwnode()
>
> Suggested-by: Dmitry Torokhov <dmitry.torokhov@xxxxxxxxx>
> Reviewed-by: Danilo Krummrich <dakr@xxxxxxxxxx>
> Reviewed-by: Rafael J. Wysocki (Intel) <rafael@xxxxxxxxxx>
> Reviewed-by: Linus Walleij <linusw@xxxxxxxxxx>
> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@xxxxxxxxxxxxxxxx>
> ---
> Link: https://lore.kernel.org/all/aZUIFiOYt6GOlDQx@xxxxxxxxxx/
> ---
> Changes in v3:
> - Remove the controversial patch 1/2 in favor of hand-coding its
>   functionality in patch 2/2
> - Link to v2: https://patch.msgid.link/20260223-device-match-secondary-fwnode-v2-0-966c00c9eeeb@xxxxxxxxxxxxxxxx
>
> Changes in v2:
> - Fold the code initially put into driver code into GPIOLIB as advised
>   by Rafael
> - Rework the logic as suggested by Andy
> - To that end: make fwnode_is_primary() public
> - Link to v1: https://patch.msgid.link/20260219-device-match-secondary-fwnode-v1-0-a64e8d4754bc@xxxxxxxxxxxxxxxx
> ---
>  drivers/gpio/gpiolib.c | 12 +++++++++++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
> index c52200eaaaff82b12f22dd1ee8459bdd8ec10d81..0182603de368f2125baf174fcf5f1e893917c154 100644
> --- a/drivers/gpio/gpiolib.c
> +++ b/drivers/gpio/gpiolib.c
> @@ -11,6 +11,7 @@
>  #include <linux/errno.h>
>  #include <linux/file.h>
>  #include <linux/fs.h>
> +#include <linux/fwnode.h>
>  #include <linux/idr.h>
>  #include <linux/interrupt.h>
>  #include <linux/irq.h>
> @@ -1395,7 +1396,16 @@ EXPORT_SYMBOL_GPL(gpio_device_find_by_label);
>
>  static int gpio_chip_match_by_fwnode(struct gpio_chip *gc, const void *fwnode)
>  {
> -       return device_match_fwnode(&gc->gpiodev->dev, fwnode);
> +       struct device *dev = &gc->gpiodev->dev;
> +       struct fwnode_handle *node = dev_fwnode(dev);
> +
> +       if (IS_ERR(fwnode))
> +               return 0;
> +
> +       if (device_match_fwnode(dev, fwnode))
> +               return 1;
> +
> +       return node && !IS_ERR(node->secondary) && node->secondary == fwnode;

The second check is redundant because fwnode cannot be an error
pointer (it has been checked against that already above) and so if
node->secondary == fwnode, then node->secondary is not an error
pointer.

I'm not sure if fwnode can be NULL here, but if it can, it should be
checked against NULL.  Alternatively, node->secondary can be checked
against NULL and compared to fwnode.

So, if fwnode != NULL cannot be guaranteed,

        return fwnode && node && node->secondary == fwnode;

or

        return node && node->secondary && node->secondary == fwnode;

The overhead of the former may be a bit lower because it avoids
dereferencing node when fwnode is NULL, but the compiler should be
able to optimize this anyway.

>  }
>
>  /**
>
> ---