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

[Rafael J. Wysocki]

On Thu, Feb 26, 2026 at 10:55 AM Bartosz Golaszewski <brgl@xxxxxxxxxx> wrote:
>
> On Wed, Feb 25, 2026 at 1:44 PM Rafael J. Wysocki <rafael@xxxxxxxxxx> wrote:
> > >
> > > 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.
> >
> > Or even the device_match_fwnode() check can be folded into the last line:
> >
> > 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_OR_NULL(fwnode))
> > +               return 0;
> > +
> > +       return node == fwnode || (node && node->secondary == fwnode);
> >  }
>
> device_match_fwnode() already contains the NULL check for fwnode.

Yes, it does, but if device_match_fwnode() returns false, you don't
know the exact reason: fwnode may be NULL or it may be non-NULL, but
different from the device's one.  You can't generally assume that
fwnode is not NULL in that case.

> I'm sending a v4 with the IS_ERR() check for secondary dropped I hope this
> is the final one.

This one is fine with me so long as NULL is never passed as fwnode to
this function.