Re: [PATCH v2 2/2] gpiolib: match secondary fwnode too in gpio_device_find_by_fwnode()

[Andy Shevchenko <andriy.shevchenko@linux.intel.com>]

On Tue, Feb 24, 2026 at 10:56:16AM +0200, Sakari Ailus wrote:
> On Tue, Feb 24, 2026 at 09:47:57AM +0100, Bartosz Golaszewski wrote:
> > On Mon, Feb 23, 2026 at 11:07 PM Sakari Ailus
> > <sakari.ailus@linux.intel.com> wrote:

...

> > > > > >  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))
> > > > >
> > > > > Could device_match_fwnode() match secondary fwnode as well?
> > > >
> > > > In the previous discussion on this, Andy was against doing that due to
> > > > the concern that it might introduce subtle bugs, which I agree with.
> > >
> > > Could you elaborate or provide an example?

I believe you ask me. Okay, the sophisticated case I have in mind is the
intel_quark_i2c_gpio.c which provides a GPIO device with a list of children.

First of all, it seems broken as it rewrites the secondary link for the
I²C device. (Which makes me think that we need to have a copy of the
[primary] fwnode in the children devices of MFD, but I don't know how
to refcount that properly). The gpiolib-acpi-core.c has a matching function
via ACPI_HANDLE(). So it might be not affected by this.

What I don't know is USB Type-C and USB DWC3 code where it's much more
complicated. And I'm not in a position to state that the change won't
affect those.

> > > The function has some 27 users although few are individual drivers.
> > >
> > > My understanding is that we only have the secondary fwnode for being able
> > > to attach objects from different backend to the same node. The fwnode API
> > > in the meantime generally tries to hide the existence of the secondary
> > > fwnode; a rewrite (which ideally would have happened perhaps a few years
> > > ago?) would probably make the fwnode a linked list instead so we'd lose
> > > that secondary pointer in the process.
> > 
> > It already is a (singly) linked list. Ideally it would be a
> 
> With two entries at most.

There is no technical limitation based on the data type.

> > doubly-linked list moved into struct device with struct fwnode_handle
> > having no concept of primary and secondary nodes.
> 
> I'd think we had that list in struct fwnode_handle, which will still
> represent nodes. But let's see the details when someone gets to implement
> it. :-)

In the case above single or double linked list doesn't solve the issue of
the corrupted (parent) fwnode. We need also to have a siblings list so it
looks more like a tree.

-- 
With Best Regards,
Andy Shevchenko