Re: [PATCH net-next 2/6] software node: allow named software node to be created

[Vladimir Oltean <olteanv@gmail.com>]

Hi Sakari,

On Tue, Jul 19, 2022 at 08:50:27AM +0000, Sakari Ailus wrote:
> Basically what your patch is doing is adding a helper function that creates
> an fwnode with a given name. This functionality was there previously through
> software_node_register_nodes(), with node allocation responsibility residing
> on the caller. It's used e.g. here:
> drivers/media/pci/intel/ipu3/cio2-bridge.c .
> 
> The larger question is perhaps when can you safely remove software nodes.
> And which of these two APIs would be preferred. I haven't checked how many
> users each has. There's no refcounting nor locking for software nodes, so
> once made visible to the rest of the kernel, they're always expected to be
> there, unchanged, or at least it needs to be known when they can be removed.

Just for my clarity, are you saying that this printf selftest is
violating the software nodes' expectation to always be there unchanged
and never be removed?

static void __init fwnode_pointer(void)
{
	const struct software_node softnodes[] = {
		{ .name = "first", },
		{ .name = "second", .parent = &softnodes[0], },
		{ .name = "third", .parent = &softnodes[1], },
		{ NULL /* Guardian */ }
	};
	const char * const full_name = "first/second/third";
	const char * const full_name_second = "first/second";
	const char * const second_name = "second";
	const char * const third_name = "third";
	int rval;

	rval = software_node_register_nodes(softnodes);
	if (rval) {
		pr_warn("cannot register softnodes; rval %d\n", rval);
		return;
	}

	test(full_name_second, "%pfw", software_node_fwnode(&softnodes[1]));
	test(full_name, "%pfw", software_node_fwnode(&softnodes[2]));
	test(full_name, "%pfwf", software_node_fwnode(&softnodes[2]));
	test(second_name, "%pfwP", software_node_fwnode(&softnodes[1]));
	test(third_name, "%pfwP", software_node_fwnode(&softnodes[2]));

	software_node_unregister_nodes(softnodes);
}

The use case in this patch set is essentially equivalent to what printf
does: exposing the software nodes to the rest of the kernel and to user
space is probably not necessary, it's just that we need to call a
function that parses their structure (essentially an equivalent to
calling "test" above). Could you indicate whether there is a better
alternative of doing this?