On Fri, Apr 24, 2026 at 02:34:29PM +0200, Bartosz Golaszewski wrote:
> On Fri, Apr 24, 2026 at 11:01 AM Andy Shevchenko
> <[email protected]> wrote:
> > On Thu, Apr 23, 2026 at 02:12:02PM +0200, Bartosz Golaszewski wrote:
...
> > > + device_remove_software_node(dev);
> >
> > > of_node_put(pa->pdev.dev.of_node);
> >
> > Can we rather replace of_* get/put with the fwnode_* get/put and make a
> > conditional here?
>
> I thought about it but I'm not sure why we bump the refcount of OF
> nodes but not of the firmware nodes supplied in struct
> platform_device_info. Maybe there was a reason for it. That would
> simplify things.
I could with 99% assurance state that the reason behind this as simple as
ACPI doesn't need that. It's no-op there and no-one at that time thought
of software nodes. More, I have some (a few years old) patch locally that
does this conversion, but never had time to look into it carefully for
any missed corner cases (as part of that work, the preparation was done
for AMBA devices in the ca5a75df36dd ("amba: bus: balance firmware node
reference counting") which is more than a couple of years in upstream).
> > if (is_software_node(...))
> > device_remove...
> > else
> > fwnode_handle_put().
> >
> > (or something like this)
> >
> > And IIRC the above pattern has been already seen somewhere else. But I can't
> > point to it, just some weak memories of seeing that already.
>
> That needs to account for two software nodes, I think it's fine to
> call the two in sequence unconditionally (like I do this this patch)
> and just dump the refcount on registration as necessary.
Sure.
--
With Best Regards,
Andy Shevchenko
