Re: [PATCH 2/5] software node: add fw_devlink support

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

+Cc: Herve, to just ask the situation with one series that adds nice helper.

On Mon, Jun 29, 2026 at 12:52:07PM +0200, Bartosz Golaszewski wrote:
> Software nodes can be used to describe supplier-consumer relationships
> between devices they represent using reference property entries. Unlike
> for OF-nodes, driver core cannot yet use these references to create a
> probe order that avoids needless probe deferrals on missing providers.
> 
> Implement software_node_add_links() modelled on of_fwnode_add_links().
> For every DEV_PROP_REF property we resolve each referenced supplier and
> create an fwnode link from the node to it. The driver core later promotes
> these to device links and defers the consumer until the suppliers are
> ready.
> 
> There's no allowlist like the one DT needs - devicetree phandles appear
> in plenty of non-supplier contexts, but a software node only carries a
> reference property when its author explicitly points at another node, so
> we treat every reference as an intentional supplier dependency and link
> all of them. Graph "remote-endpoint" references are skipped for now: they
> go 2-ways between endpoint nodes and would create graph cycles without
> the port-parent lifting DT does via get_con_dev(). References to
> suppliers that aren't registered yet and self-references are ignored.
> 
> fw_devlink resolves the supplier device through fwnode->dev but the core
> only records the owning device on the primary fwnode. When the software
> node is a device's secondary fwnode, mirror the device pointer onto it in
> software_node_notify() so the consumer can actually find the supplier
> instead of deferring forever.
> 
> While at it: purge the fwnode links in software_node_release() now that
> software nodes can own them.

...

> +static int software_node_add_links(struct fwnode_handle *fwnode)
> +{
> +	const struct software_node_ref_args *ref, *ref_array;
> +	struct swnode *swnode = to_swnode(fwnode);
> +	const struct property_entry *prop;
> +	struct fwnode_handle *refnode;
> +	unsigned int count, i;

'i' is local to the loop.

> +	if (!swnode || !swnode->node->properties)
> +		return 0;
> +
> +	/*
> +	 * Unlike Device Tree, where phandles appear in many non-supplier
> +	 * contexts and a curated allowlist is required, a software node only
> +	 * carries a DEV_PROP_REF property when the author explicitly describes
> +	 * a reference to another node. Every such reference is therefore an
> +	 * intentional supplier dependency, so we create fwnode links for all
> +	 * of them.
> +	 */
> +	for (prop = swnode->node->properties; prop->name; prop++) {
> +		if (prop->type != DEV_PROP_REF || prop->is_inline)
> +			continue;
> +
> +		/*
> +		 * TODO: Graph "remote-endpoint" references go both ways
> +		 * between endpoint child nodes and would create endpoint
> +		 * cycles. Let's leave it out for now until we have potential
> +		 * users.
> +		 */
> +		if (!strcmp(prop->name, "remote-endpoint"))
> +			continue;
> +
> +		ref_array = prop->pointer;
> +		count = prop->length / sizeof(*ref_array);

Seems we are going to have more of a such, perhaps make a helper for counting
(which will do that division beneath).

> +		for (i = 0; i < count; i++) {

		for (unsigned int i = 0; i < count; i++) {

> +

Redundant blank line?

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

...

> +	/*
> +	 * When the software node is the device's secondary firmware node, the
> +	 * core only records the owning device on the primary fwnode (see
> +	 * device_add()). fw_devlink resolves a supplier device through
> +	 * fwnode->dev, so without this a consumer referencing the software
> +	 * node could never find the supplier device and would defer forever.
> +	 * Make fwnode.dev point to its owner in that case.

The below seems more balanced in terms of line lengths.

	 * When the software node is the device's secondary firmware node,
	 * the core only records the owning device on the primary fwnode
	 * (see device_add()). fw_devlink resolves a supplier device through
	 * fwnode->dev, so without this a consumer referencing the software
	 * node could never find the supplier device and would defer forever.
	 * Make fwnode.dev point to its owner in that case.

> +	 */
> +	if (dev_fwnode(dev) != &swnode->fwnode && !swnode->fwnode.dev)
> +		swnode->fwnode.dev = dev;

Doesn't Herve's patch(es) add some helpers for this?
https://lore.kernel.org/lkml/20260511155930.34604-2-herve.codina@bootlin.com/

Ah, still not applied :-( Perhaps it can be applied as just part that brings
fw_devlink_set_device()?

> +	/*
> +	 * Drop the device pointer mirrored onto a secondary software node in
> +	 * software_node_notify(). For a primary software node the core owns
> +	 * fwnode->dev and clears it in device_del().
> +	 */
> +	if (dev_fwnode(dev) != &swnode->fwnode && swnode->fwnode.dev == dev)
> +		swnode->fwnode.dev = NULL;

-- 
With Best Regards,
Andy Shevchenko