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

[Danilo Krummrich]

On Mon Feb 23, 2026 at 8:55 PM CET, Rafael J. Wysocki wrote:
> On Mon, Feb 23, 2026 at 8:45 PM Danilo Krummrich <dakr@xxxxxxxxxx> wrote:
>>
>> On Mon Feb 23, 2026 at 4:40 PM CET, Bartosz Golaszewski 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))
>> > +             return 1;
>> > +
>> > +     return fwnode_is_primary(node) && node->secondary == fwnode;
>> >  }
>>
>> Rafael, I understand [1] as you agree with my point, but object to introduce
>> device_match_fwnode_ext() (or whatever name we would pick eventually :)
>> regardless because only the GPIO code would need it as by now.
>
> This is a preference, not a strong objection, but yes.
>
>> IIUC, I wonder if exposing fwnode_is_primary() instead is a good trade.
>
> Well, there is the secondary pointer in struct fwnode_handle, so it is
> kind of exported anyway and it could be documented as "a secondary
> fwnode_handle supplying additional properties or an error pointer", so
> exposing this static inline doesn't change much IMV.
>
> What's your specific concern about exposing it?

No concern with either approach from my side, I was just curious. :)

Maybe it makes sense to add a comment to gpio_chip_match_by_fwnode() hinting to
move this into common code once there's another occurance of this pattern.

But either way, this is

Reviewed-by: Danilo Krummrich <dakr@xxxxxxxxxx>

>> [1] https://lore.kernel.org/driver-core/CAJZ5v0jUCtKTW-g-C0pKu0DQqOkyfSz=upXwbtYeV_=rMBUMyg@xxxxxxxxxxxxxx/