Re: [PATCH v2 3/4] software node: pin referenced swnodes while resolving them

[Andy Shevchenko]

On Fri, Nov 06, 2026 at 12:38:37PM +0800, Peng Fan (OSS) wrote:

> software_node_get_reference_args() and software_node_add_links() resolve
> DEV_PROP_REF entries by calling software_node_fwnode(), which returns a
> borrowed pointer. The referenced swnode can be unregistered and released
> between the lookup and the fwnode reference or link operation.
> 
> Use software_node_to_swnode_get() for software node references and keep
> the reference until the argument or link has been set up. For direct
> fwnode references in software_node_add_links(), take a reference with
> fwnode_handle_get() and release it after fwnode_link_add() completes.

...

> -	if (ref->swnode)
> -		refnode = software_node_fwnode(ref->swnode);
> -	else if (ref->fwnode)
> -		refnode = ref->fwnode;
> -	else
> -		return -EINVAL;
> +	if (ref->swnode) {
> +		struct swnode *remote = software_node_to_swnode_get(ref->swnode);
>  
> -	if (!refnode)
> -		return -ENOTCONN;
> +		if (!remote)
> +			return -ENOTCONN;

It's always better to maintain when assignment is coupled with a validation
check.

		struct swnode *remote;

		remote = software_node_to_swnode_get(ref->swnode);
		if (!remote)
			return -ENOTCONN;

> +		refnode = &remote->fwnode;
> +	} else if (ref->fwnode) {
> +		refnode = fwnode_handle_get(ref->fwnode);
> +	} else {
> +		return -EINVAL;
> +	}

...

> -			if (ref->swnode)
> -				refnode = software_node_fwnode(ref->swnode);
> -			else if (ref->fwnode)
> -				refnode = ref->fwnode;
> -			else
> +			if (ref->swnode) {
> +				struct swnode *remote;

> +				remote = software_node_to_swnode_get(ref->swnode);
> +				if (!remote)
> +					continue;

Seems like a dup code, perhaps a helper?

> +				refnode = &remote->fwnode;
> +			} else if (ref->fwnode) {
> +				refnode = fwnode_handle_get(ref->fwnode);
> +			} else {
>  				continue;
> +			}
>  
>  			/* Supplier not registered yet, or self-reference. */
> -			if (!refnode || refnode == &swnode->fwnode)

> +			if (refnode == &swnode->fwnode) {
> +				fwnode_handle_put(refnode);
>  				continue;
> +			}
>  
>  			fwnode_link_add(&swnode->fwnode, refnode, 0);
> +			fwnode_handle_put(refnode);

I don't see a context, but it looks like it continues in both cases.
If so, the above may be refactored for that.

-- 
With Best Regards,
Andy Shevchenko