Re: [PATCH 3/9] software node: allow referencing firmware nodes

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

On Mon, Oct 20, 2025 at 10:06:43AM +0200, Bartosz Golaszewski wrote:
> On Sat, Oct 18, 2025 at 7:34 PM Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> > On Mon, Oct 06, 2025 at 03:00:18PM +0200, Bartosz Golaszewski wrote:

...

> > > +enum software_node_ref_type {
> > > +     /* References a software node. */
> > > +     SOFTWARE_NODE_REF_SWNODE = 0,
> >
> > I don't see why we need an explicit value here.
> 
> It was to make it clear, this is the default value and it's the one
> used in older code with the legacy macros. I can drop it, it's no big
> deal.

Usually when we assign a default value(s) in enum, it should be justified.
The common mistake here (not this case) is to use autoincrement feature
with some of the values explicitly defined for the enums that reflect the
HW bits / states, which obviously makes code fragile and easy to break.

> > > +     /* References a firmware node. */
> > > +     SOFTWARE_NODE_REF_FWNODE,
> > > +};

...

> > >  /**
> > >   * struct software_node_ref_args - Reference property with additional arguments
> > > - * @node: Reference to a software node
> > > + * @swnode: Reference to a software node
> > > + * @fwnode: Alternative reference to a firmware node handle
> > >   * @nargs: Number of elements in @args array
> > >   * @args: Integer arguments
> > >   */
> > >  struct software_node_ref_args {
> > > -     const struct software_node *node;
> > > +     enum software_node_ref_type type;
> > > +     union {
> > > +             const struct software_node *swnode;
> > > +             struct fwnode_handle *fwnode;
> > > +     };
> >
> > Can't we always have an fwnode reference?
> 
> Unfortunately no. A const struct software_node is not yet a full
> fwnode, it's just a template that becomes an actual firmware node when
> it's registered with the swnode framework. However in order to allow
> creating a graph of software nodes before we register them, we need a
> way to reference those templates and then look them up internally in
> swnode code.

Strange that you need this way. The IPU3 bridge driver (that creates a graph of
fwnodes at run-time for being consumed by the respective parts of v4l2
framework) IIRC has no such issue. Why your case is different?

> > >       unsigned int nargs;
> > >       u64 args[NR_FWNODE_REFERENCE_ARGS];
> > >  };

-- 
With Best Regards,
Andy Shevchenko