Re: [PATCH 0/4] platform/x86: x86-android-tablets: use real firmware node references with intel drivers

[Bartosz Golaszewski]

On Fri, 20 Mar 2026 02:49:02 +0100, Dmitry Torokhov
<dmitry.torokhov@xxxxxxxxx> said:
> Hi Bartosz,
>
> On Thu, Mar 19, 2026 at 05:10:53PM +0100, Bartosz Golaszewski wrote:
>>
>> This series proposes a solution in the form of automatic secondary
>> software node assignment (I'm open to better naming ideas). We extend
>> the swnode API with functions allowing to set up a behind-the-scenes bus
>> notifier for a group of named software nodes. It will wait for bus
>> events and when a device is added, it will check its name against the
>> software node's name and - on match - assign the software node as the
>> secondary firmware node of the device's *real* firmware node.
>
> The more I think about the current approaches with strict identity
> matching the less I like them, and the reason is that strict identity
> matching establishes rigid links between consumers and producers of
> GPIOS/swnodes, and puts us into link order hell. For example, I believe
> if andoird tablets drivers were in drivers/android vs
> drivers/platform/... the current scheme would break since the nodes
> would not be registered and GPIO lookups would fail with -ENOENT vs
> -EPROBE_DEFER.
>

Why would they not get registered? They get attached when the target devices
are added in modules this module depends on. They are exported symbols so the
prerequisite modules will get loaded before and the module's init function
will have run by the time the software nodes are referred to by the fwnode
interface at which point they will have been registered with the swnode
framework.

> Given that this series somewhat re-introduces the name matching, I
> wonder if we can not do something like the following (the rough draft):
>

I'm open to better ideas and possibly multiple matching mechanisms but this
just fit in this particular case. I'm not overly attached to name matching. We
may as well use whatever properties ACPI provides to identify the devices and
assign them their swnodes.

> diff --git a/drivers/base/swnode.c b/drivers/base/swnode.c
> index 51320837f3a9..b0e8923a092c 100644
> --- a/drivers/base/swnode.c
> +++ b/drivers/base/swnode.c
> @@ -509,6 +509,7 @@ software_node_get_reference_args(const struct fwnode_handle *fwnode,
>  	struct swnode *swnode = to_swnode(fwnode);
>  	const struct software_node_ref_args *ref_array;
>  	const struct software_node_ref_args *ref;
> +	const struct software_node *ref_swnode;
>  	const struct property_entry *prop;
>  	struct fwnode_handle *refnode;
>  	u32 nargs_prop_val;
> @@ -550,7 +551,10 @@ software_node_get_reference_args(const struct fwnode_handle *fwnode,
>  		refnode = software_node_fwnode(ref->swnode);
>  	else if (ref->fwnode)
>  		refnode = ref->fwnode;
> -	else
> +	else if (ref->swnode_name) {
> +		ref_swnode = software_node_find_by_name(NULL, ref->swnode_name);
> +		refnode = ref_swnode ? software_node_fwnode(ref_swnode) : NULL;
> +	} else
>  		return -EINVAL;
>
>  	if (!refnode)
> diff --git a/include/linux/property.h b/include/linux/property.h
> index e30ef23a9af3..44e96ee47272 100644
> --- a/include/linux/property.h
> +++ b/include/linux/property.h
> @@ -363,6 +363,7 @@ struct software_node;
>  struct software_node_ref_args {
>  	const struct software_node *swnode;
>  	struct fwnode_handle *fwnode;
> +	const char *swnode_name;
>  	unsigned int nargs;
>  	u64 args[NR_FWNODE_REFERENCE_ARGS];
>  };
> @@ -373,6 +374,9 @@ struct software_node_ref_args {
>  			   const struct software_node *: _ref_,	\
>  			   struct software_node *: _ref_,	\
>  			   default: NULL),			\
> +	.swnode_name = _Generic(_ref_,				\
> +				const char *: _ref_,		\
> +				default: NULL),			\
>  	.fwnode = _Generic(_ref_,				\
>  			   struct fwnode_handle *: _ref_,	\
>  			   default: NULL),			\
>
> This will allow consumers specify top-level software node name instead
> of software node structure, and it will get resolved to concrete
> firmware node. GPIOLIB can continue matching on node identity.
>
> WDYT?
>

I think it's bad design and even bigger abuse of the software node concept.
What you're proposing is effectively allowing to use struct software_node as
a misleading string wrapper. You wouldn't use it to pass any properties to
the device you're pointing to - because how if there's no link between them -
you would just store an arbitrary string in a structure that serves
a completely different purpose.

Which is BTW exactly what was done in GPIO and - while there's no denying that
I signed-off on these patches - it goes to show just how misleading this design
is - I was aware of this development and queued the patches but only really
understood what was going on when it was too late and this pattern was
copy-pasted all over the kernel.

Software nodes are just an implementation of firmware nodes and as such should
follow the same principles. If a software node describes a device, it should be
attached to it so that references can be resolved by checking the address of
the underlying firmware node handle and not by string matching. I will die on
that hill. :)

If you want to match string, use GPIO lookup tables. I remember your point about
wanting to use PROPERTY_ENTRY_REF() for consistency and fully support it, but
please do use *references* because otherwise it's just a lookup table with extra
steps.

Just think about it: what if we try to generalize fw_devlink to software nodes?
It would be completely broken for the offending GPIO users because there's no
real link between the software nodes supposedly pointing to the GPIO
controllers and the controllers themselves.

> Also, you need to update the existing documentation in order to be able
> to call the existing documented practice an "abuse" ;)
>

Yes, I should.

Thanks,
Bartosz