Re: [PATCH v4 03/10] software node: allow referencing firmware nodes

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

On Mon, Nov 03, 2025 at 10:35:23AM +0100, Bartosz Golaszewski wrote:
> 
> At the moment software nodes can only reference other software nodes.
> This is a limitation for devices created, for instance, on the auxiliary
> bus with a dynamic software node attached which cannot reference devices
> the firmware node of which is "real" (as an OF node or otherwise).
> 
> Make it possible for a software node to reference all firmware nodes in
> addition to static software nodes. To that end: add a second pointer to
> struct software_node_ref_args of type struct fwnode_handle. The core
> swnode code will first check the swnode pointer and if it's NULL, it
> will assume the fwnode pointer should be set. Rework the helper macros
> and deprecate the existing ones whose names don't indicate the reference
> type.
> 
> Software node graphs remain the same, as in: the remote endpoints still
> have to be software nodes.

...

> +	/*
> +	 * A software node can reference other software nodes or firmware
> +	 * nodes (which are the abstraction layer sitting on top of them).
> +	 * This is done to ensure we can create references to static software
> +	 * nodes before they're registered with the firmware node framework.
> +	 * At the time the reference is being resolved, we expect the swnodes
> +	 * in question to already have been registered and to be backed by
> +	 * a firmware node. This is why we use the fwnode API below to read the

A nit-pick (since anyway it requires a new version): move 'the' to the next
line to make them more equal in the length.

> +	 * relevant properties and bump the reference count.
> +	 */

...

> -#define SOFTWARE_NODE_REFERENCE(_ref_, ...)			\
> +#define __SOFTWARE_NODE_REF(_ref, ...)				\

No, NAK. The renaming of the parameters is not related to this change _at all_.
Why do you change established style here? Did I miss your answer to my question
in the previous rounds?

>  (const struct software_node_ref_args) {				\
> -	.node = _ref_,						\
> +	.swnode = _Generic(_ref,				\
> +			   const struct software_node *: _ref,	\
> +			   default: NULL),			\
> +	.fwnode = _Generic(_ref,				\
> +			   struct fwnode_handle *: _ref,	\
> +			   default: NULL),			\
>  	.nargs = COUNT_ARGS(__VA_ARGS__),			\
>  	.args = { __VA_ARGS__ },				\
>  }

...

> +#define SOFTWARE_NODE_REF_SWNODE(_ref, ...)			\
> +	__SOFTWARE_NODE_REF(_ref, __VA_ARGS__)
> +
> +#define SOFTWARE_NODE_REF_FWNODE(_ref, ...)			\
> +	__SOFTWARE_NODE_REF(_ref, __VA_ARGS__)
> +
> +/* DEPRECATED, use SOFTWARE_NODE_REF_SWNODE() instead. */
> +#define SOFTWARE_NODE_REFERENCE(_ref, ...)			\
> +	SOFTWARE_NODE_REF_SWNODE(_ref, __VA_ARGS__)

Now, useless.

...


> -#define PROPERTY_ENTRY_REF(_name_, _ref_, ...)				\
> +#define __PROPERTY_ENTRY_REF(_type, _name, _ref, ...)			\
>  (struct property_entry) {						\
> -	.name = _name_,							\
> +	.name = _name,							\
>  	.length = sizeof(struct software_node_ref_args),		\
>  	.type = DEV_PROP_REF,						\
> -	{ .pointer = &SOFTWARE_NODE_REFERENCE(_ref_, ##__VA_ARGS__), },	\
> +	{ .pointer = &_type(_ref, ##__VA_ARGS__), },			\
>  }

Do we need this now? I assume that _Generic() takes case of this.

...

> +#define PROPERTY_ENTRY_REF_SWNODE(_name, _ref, ...)			\
> +	__PROPERTY_ENTRY_REF(SOFTWARE_NODE_REF_SWNODE,			\
> +			     _name, _ref, __VA_ARGS__)
> +
> +#define PROPERTY_ENTRY_REF_FWNODE(_name, _ref, ...)			\
> +	__PROPERTY_ENTRY_REF(SOFTWARE_NODE_REF_FWNODE,			\
> +			    _name, _ref, __VA_ARGS__)
> +
> +/* DEPRECATED, use PROPERTY_ENTRY_REF_SWNODE() instead. */
> +#define PROPERTY_ENTRY_REF(_name, _ref, ...)				\
> +	PROPERTY_ENTRY_REF_SWNODE(_name, _ref, __VA_ARGS__)

Seems like useless churn.

-- 
With Best Regards,
Andy Shevchenko