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

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

[Bartosz Golaszewski]

Problem statement: GPIO software node lookup should rely exclusively on
matching the addresses of the referenced firmware nodes. I tried to
enforce it with commit e5d527be7e69 ("gpio: swnode: don't use the
swnode's name as the key for GPIO lookup") but it broke existing
users who abuse the software node mechanism by creating "dummy" software
nodes named after the device they want to get GPIOs from but never
attaching them to the actual GPIO devices. They rely on the current
behavior of GPIOLIB where it will match the label of the GPIO controller
against the name of the software node and does not require a true link.

x86-android-tablets driver is one of the abusers in that it creates
dummy software nodes for baytrail and cherryview GPIO controllers but
they don't really reference these devices. Before we can reapply
e5d527be7e69 and support matching by fwnode address exclusively, we need
to convert all the users to using actual fwnode references.

It's possible for drivers to reference real firmware nodes from software
nodes via PROPERTY_ENTRY_REF() in general or PROPERTY_ENTRY_GPIO()
specifically but for platform devices binding to real firmware nodes (DT
or ACPI) it's cumbersome as they would need to dynamically look for the
right nodes and create references dynamically with no way of using
static const software nodes.

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.

For now it only supports name matching but I suppose we could extend it
with additional mechanisms if needed. I didn't plan to introduce a
generic API for this initially but the code in cherryview and baytrail
drivers ended up being exactly the same so I decided to factor it out
into common library code.

First patch provides the new interfaces. Next two patches set up
automatic secondary fwnodes for intel baytrail and cherryview drivers.
Finally patch 4/4 replaces the dummy software nodes with ones that will
end up actually being attached to the relevant devices.

GPIOLIB is now able to match a GPIO controller by both its primary as
well as secondary firmware node. While I don't have the relevant
hardware, I tested the series and the new behavior on x86 qemu with
a dummy ACPI table and modified gpio-sim. I assume it will work in real
life but testing is welcome.

Merging strategy: This touches three trees and there are build-time
dependencies. I think it should go through the x86 platform tree with
Ilpo providing an immutable branch for the driver core and pinctrl
trees.

Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
---
Bartosz Golaszewski (4):
      software node: support automatic secondary fwnode assignment
      pinctrl: intel: expose software nodes for baytrail GPIO devices
      pinctrl: intel: expose software nodes for cherryiew GPIO devices
      platform/x86: x86-android-tablets: enable fwnode matching of GPIO chips

 drivers/base/swnode.c                              | 105 +++++++++++++++++++++
 drivers/pinctrl/intel/pinctrl-baytrail.c           |  40 +++++++-
 drivers/pinctrl/intel/pinctrl-cherryview.c         |  47 ++++++++-
 drivers/platform/x86/x86-android-tablets/Kconfig   |   1 +
 drivers/platform/x86/x86-android-tablets/acer.c    |  11 ++-
 drivers/platform/x86/x86-android-tablets/asus.c    |   9 +-
 drivers/platform/x86/x86-android-tablets/core.c    |  47 +--------
 drivers/platform/x86/x86-android-tablets/lenovo.c  |  31 +++---
 drivers/platform/x86/x86-android-tablets/other.c   |  23 ++---
 .../x86/x86-android-tablets/shared-psy-info.c      |   7 +-
 .../x86/x86-android-tablets/x86-android-tablets.h  |   4 -
 include/linux/pinctrl/intel.h                      |  17 ++++
 include/linux/property.h                           |  18 ++++
 13 files changed, 270 insertions(+), 90 deletions(-)
---
base-commit: f50e991d263338028b296f4398c669feca8f0c3b
change-id: 20260319-baytrail-real-swnode-7de7a3870f78

Best regards,
-- 
Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>

[PATCH 1/4] software node: support automatic secondary fwnode assignment

[Bartosz Golaszewski]

Provide a structure and a set of functions allowing to set up automatic
secondary firmware node assignment for platform devices. It uses
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.

Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
---
 drivers/base/swnode.c    | 105 +++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/property.h |  18 ++++++++
 2 files changed, 123 insertions(+)

diff --git a/drivers/base/swnode.c b/drivers/base/swnode.c
index 51320837f3a9f1bf4f65aa161d9b941affc74936..97e3354cdafd94e175d29acb697a0dc61186a9c8 100644
--- a/drivers/base/swnode.c
+++ b/drivers/base/swnode.c
@@ -6,6 +6,7 @@
  * Author: Heikki Krogerus <heikki.krogerus@linux.intel.com>
  */
 
+#include <linux/cleanup.h>
 #include <linux/container_of.h>
 #include <linux/device.h>
 #include <linux/err.h>
@@ -1088,6 +1089,110 @@ int device_create_managed_software_node(struct device *dev,
 }
 EXPORT_SYMBOL_GPL(device_create_managed_software_node);
 
+static struct software_node_auto_secondary *to_auto_sec(struct notifier_block *nb)
+{
+	return container_of(nb, struct software_node_auto_secondary, nb);
+}
+
+static void swnode_set_secondary_fwnode(struct device *dev,
+					const struct software_node *swnode)
+{
+	struct fwnode_handle *fwnode;
+
+	fwnode = software_node_fwnode(swnode);
+	if (WARN(!fwnode, "Software node %s should have been registered before", swnode->name))
+		return;
+
+	set_secondary_fwnode(dev, fwnode);
+}
+
+static int swnode_auto_secondary_notifier(struct notifier_block *nb,
+					  unsigned long action, void *data)
+{
+	struct software_node_auto_secondary *auto_sec = to_auto_sec(nb);
+	const struct software_node * const *swnode;
+	struct device *dev = data;
+
+	switch (action) {
+	case BUS_NOTIFY_ADD_DEVICE:
+		for (swnode = auto_sec->node_group; *swnode; swnode++) {
+			if (strcmp(dev_name(dev), (*swnode)->name))
+				continue;
+
+			swnode_set_secondary_fwnode(dev, *swnode);
+			return NOTIFY_OK;
+		}
+		break;
+	default:
+		break;
+	}
+
+	return NOTIFY_DONE;
+}
+
+/**
+ * software_node_register_auto_secondary() - set up automatic assignment of
+ *                                           secondary firmware nodes
+ * @auto_sec: Context data to use.
+ *
+ * NOTE: All software nodes passed in @auto_sec must be named.
+ *
+ * Returns:
+ * 0 on success, negative error number on failure.
+ *
+ * This registers the software node group passed in @auto_sec and sets up
+ * automatic assignment of them as secondary firmware nodes of real nodes
+ * attached to appropriate devices on the bus specified in @auto_sec. The
+ * software nodes must be named and their names must be the same as the
+ * devices they should reference. The assignment happens when the device is
+ * first added to the bus.
+ */
+int software_node_register_auto_secondary(struct software_node_auto_secondary *auto_sec)
+{
+	const struct software_node * const *swnode;
+	int ret;
+
+	if (!auto_sec->node_group || !auto_sec->bus)
+		return -EINVAL;
+
+	for (swnode = auto_sec->node_group; *swnode; swnode++) {
+		if (!(*swnode)->name)
+			return -EINVAL;
+	}
+
+	ret = software_node_register_node_group(auto_sec->node_group);
+	if (ret)
+		return ret;
+
+	auto_sec->nb.notifier_call = swnode_auto_secondary_notifier;
+	ret = bus_register_notifier(auto_sec->bus, &auto_sec->nb);
+	if (ret)
+		software_node_unregister_node_group(auto_sec->node_group);
+
+	/* Device may have been already added. */
+	for (swnode = auto_sec->node_group; *swnode; swnode++) {
+		struct device *dev __free(put_device) =
+			bus_find_device_by_name(auto_sec->bus, NULL, (*swnode)->name);
+		if (dev)
+			swnode_set_secondary_fwnode(dev, *swnode);
+	}
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(software_node_register_auto_secondary);
+
+/**
+ * software_node_unregister_auto_secondary() - unregister automatic assignment
+ *                                             of secondary firmware nodes
+ * @auto_sec: Address of the context structure used at registration
+ */
+void software_node_unregister_auto_secondary(struct software_node_auto_secondary *auto_sec)
+{
+	bus_unregister_notifier(auto_sec->bus, &auto_sec->nb);
+	software_node_unregister_node_group(auto_sec->node_group);
+}
+EXPORT_SYMBOL_GPL(software_node_unregister_auto_secondary);
+
 void software_node_notify(struct device *dev)
 {
 	struct swnode *swnode;
diff --git a/include/linux/property.h b/include/linux/property.h
index e30ef23a9af3396455d5bb19bb6d41089d81d77f..2a7af60cbb8ecc2ba83819ce92562db42705b82a 100644
--- a/include/linux/property.h
+++ b/include/linux/property.h
@@ -15,10 +15,12 @@
 #include <linux/bits.h>
 #include <linux/cleanup.h>
 #include <linux/fwnode.h>
+#include <linux/notifier.h>
 #include <linux/stddef.h>
 #include <linux/types.h>
 #include <linux/util_macros.h>
 
+struct bus_type;
 struct device;
 
 enum dev_prop_type {
@@ -610,4 +612,20 @@ int device_create_managed_software_node(struct device *dev,
 					const struct property_entry *properties,
 					const struct software_node *parent);
 
+/**
+ * struct software_node_auto_secondary - context data for automatic secondary
+ *                                       fwnode assignment
+ * @nb: Private bus notifier data - don't use
+ * @node_group: NULL-terminated array of software node addresses
+ * @bus: Bus on which to wait for devices
+ */
+struct software_node_auto_secondary {
+	struct notifier_block nb;
+	const struct software_node * const *node_group;
+	const struct bus_type *bus;
+};
+
+int software_node_register_auto_secondary(struct software_node_auto_secondary *auto_sec);
+void software_node_unregister_auto_secondary(struct software_node_auto_secondary *auto_sec);
+
 #endif /* _LINUX_PROPERTY_H_ */

-- 
2.47.3

Re: [PATCH 1/4] software node: support automatic secondary fwnode assignment

[Andy Shevchenko]

On Thu, Mar 19, 2026 at 05:10:54PM +0100, Bartosz Golaszewski wrote:
> Provide a structure and a set of functions allowing to set up automatic
> secondary firmware node assignment for platform devices. It uses
> 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.

...

> +/**
> + * software_node_register_auto_secondary() - set up automatic assignment of
> + *                                           secondary firmware nodes
> + * @auto_sec: Context data to use.
> + *
> + * NOTE: All software nodes passed in @auto_sec must be named.

> + * Returns:

Is it with 's' in other kernel-doc? The official is "Return", the 's' variant
is supported, but not documented.

> + * 0 on success, negative error number on failure.

The Return section must be last in the kernel-doc description. This is documented.

> + * This registers the software node group passed in @auto_sec and sets up
> + * automatic assignment of them as secondary firmware nodes of real nodes
> + * attached to appropriate devices on the bus specified in @auto_sec. The
> + * software nodes must be named and their names must be the same as the
> + * devices they should reference. The assignment happens when the device is
> + * first added to the bus.
> + */

...

> +/**
> + * struct software_node_auto_secondary - context data for automatic secondary
> + *                                       fwnode assignment
> + * @nb: Private bus notifier data - don't use

Mark it with __private then.

> + * @node_group: NULL-terminated array of software node addresses
> + * @bus: Bus on which to wait for devices

If bus is not compiled into kernel, this optionally has to support NULL
in the code (I haven't checked the code, though).

> + */
> +struct software_node_auto_secondary {
> +	struct notifier_block nb;

	struct notifier_block __private nb;

> +	const struct software_node * const *node_group;
> +	const struct bus_type *bus;
> +};

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 1/4] software node: support automatic secondary fwnode assignment

[Bartosz Golaszewski]

On Fri, Mar 20, 2026 at 8:39 AM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> On Thu, Mar 19, 2026 at 05:10:54PM +0100, Bartosz Golaszewski wrote:
> > Provide a structure and a set of functions allowing to set up automatic
> > secondary firmware node assignment for platform devices. It uses
> > 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.
>
> ...
>
> > +/**
> > + * software_node_register_auto_secondary() - set up automatic assignment of
> > + *                                           secondary firmware nodes
> > + * @auto_sec: Context data to use.
> > + *
> > + * NOTE: All software nodes passed in @auto_sec must be named.
>
> > + * Returns:
>
> Is it with 's' in other kernel-doc? The official is "Return", the 's' variant
> is supported, but not documented.
>
> > + * 0 on success, negative error number on failure.
>
> The Return section must be last in the kernel-doc description. This is documented.
>
> > + * This registers the software node group passed in @auto_sec and sets up
> > + * automatic assignment of them as secondary firmware nodes of real nodes
> > + * attached to appropriate devices on the bus specified in @auto_sec. The
> > + * software nodes must be named and their names must be the same as the
> > + * devices they should reference. The assignment happens when the device is
> > + * first added to the bus.
> > + */
>
> ...
>
> > +/**
> > + * struct software_node_auto_secondary - context data for automatic secondary
> > + *                                       fwnode assignment
> > + * @nb: Private bus notifier data - don't use
>
> Mark it with __private then.

Ok for this and the ones above.

>
> > + * @node_group: NULL-terminated array of software node addresses
> > + * @bus: Bus on which to wait for devices
>
> If bus is not compiled into kernel, this optionally has to support NULL
> in the code (I haven't checked the code, though).
>

The bus type object for given bus is typically defined in whatever
compilation unit contains the implementation of that bus. It's not
possible to access its address if it's not enabled.

Bart

[PATCH 2/4] pinctrl: intel: expose software nodes for baytrail GPIO devices

[Bartosz Golaszewski]

Use the new automatic secondary fwnode API to ensure that when the
baytrail pinctrl device is added to the platform bus, the static
software node provided for drivers not able to use ACPI will get
automatically assigned as the secondary fwnode of the primary ACPI node.

Create a new header under linux/pinctrl/ containing intel-specific
symbols and declare the new variables there.

Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
---
 drivers/pinctrl/intel/pinctrl-baytrail.c | 40 +++++++++++++++++++++++++++++++-
 include/linux/pinctrl/intel.h            | 12 ++++++++++
 2 files changed, 51 insertions(+), 1 deletion(-)

diff --git a/drivers/pinctrl/intel/pinctrl-baytrail.c b/drivers/pinctrl/intel/pinctrl-baytrail.c
index 02fdadf2189afb0ff654c80c131f813b59d623d6..bd0ba7c3351c021584127b3679c59d7463cc8a57 100644
--- a/drivers/pinctrl/intel/pinctrl-baytrail.c
+++ b/drivers/pinctrl/intel/pinctrl-baytrail.c
@@ -22,6 +22,7 @@
 #include <linux/seq_file.h>
 #include <linux/string_helpers.h>
 
+#include <linux/pinctrl/intel.h>
 #include <linux/pinctrl/pinctrl.h>
 #include <linux/pinctrl/pinmux.h>
 #include <linux/pinctrl/pinconf.h>
@@ -1723,9 +1724,46 @@ static struct platform_driver byt_gpio_driver = {
 	},
 };
 
+const struct software_node baytrail_gpio_node_00 = {
+	.name = "INT33FC:00",
+};
+EXPORT_SYMBOL_NS_GPL(baytrail_gpio_node_00, "PINCTRL_INTEL");
+
+const struct software_node baytrail_gpio_node_01 = {
+	.name = "INT33FC:01",
+};
+EXPORT_SYMBOL_NS_GPL(baytrail_gpio_node_01, "PINCTRL_INTEL");
+
+const struct software_node baytrail_gpio_node_02 = {
+	.name = "INT33FC:02",
+};
+EXPORT_SYMBOL_NS_GPL(baytrail_gpio_node_02, "PINCTRL_INTEL");
+
+static const struct software_node *baytrail_gpio_node_group[] = {
+	&baytrail_gpio_node_00,
+	&baytrail_gpio_node_01,
+	&baytrail_gpio_node_02,
+	NULL
+};
+
+static struct software_node_auto_secondary byt_auto_secondary = {
+	.node_group = baytrail_gpio_node_group,
+	.bus = &platform_bus_type,
+};
+
 static int __init byt_gpio_init(void)
 {
-	return platform_driver_register(&byt_gpio_driver);
+	int ret;
+
+	ret = software_node_register_auto_secondary(&byt_auto_secondary);
+	if (ret)
+		return ret;
+
+	ret = platform_driver_register(&byt_gpio_driver);
+	if (ret)
+		software_node_unregister_auto_secondary(&byt_auto_secondary);
+
+	return ret;
 }
 subsys_initcall(byt_gpio_init);
 
diff --git a/include/linux/pinctrl/intel.h b/include/linux/pinctrl/intel.h
new file mode 100644
index 0000000000000000000000000000000000000000..d45f090adc1f532f866c98aeca144a4d83fa28f4
--- /dev/null
+++ b/include/linux/pinctrl/intel.h
@@ -0,0 +1,12 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+#ifndef __LINUX_PINCTRL_INTEL_H
+#define __LINUX_PINCTRL_INTEL_H
+
+struct software_node;
+
+extern const struct software_node baytrail_gpio_node_00;
+extern const struct software_node baytrail_gpio_node_01;
+extern const struct software_node baytrail_gpio_node_02;
+
+#endif /* __LINUX_PINCTRL_INTEL_H */

-- 
2.47.3

Re: [PATCH 2/4] pinctrl: intel: expose software nodes for baytrail GPIO devices

[Andy Shevchenko]

On Thu, Mar 19, 2026 at 05:10:55PM +0100, Bartosz Golaszewski wrote:
> Use the new automatic secondary fwnode API to ensure that when the
> baytrail pinctrl device is added to the platform bus, the static
> software node provided for drivers not able to use ACPI will get
> automatically assigned as the secondary fwnode of the primary ACPI node.
> 
> Create a new header under linux/pinctrl/ containing intel-specific
> symbols and declare the new variables there.

As I read the code, this doesn't need to be part of drivers/pinctrl/intel/.
I.o.w. I am unable to see why we need to penetrate the certain pinctrl
driver for this.

...

>  static int __init byt_gpio_init(void)
>  {
> -	return platform_driver_register(&byt_gpio_driver);
> +	int ret;
> +
> +	ret = software_node_register_auto_secondary(&byt_auto_secondary);
> +	if (ret)
> +		return ret;
> +
> +	ret = platform_driver_register(&byt_gpio_driver);
> +	if (ret)
> +		software_node_unregister_auto_secondary(&byt_auto_secondary);
> +
> +	return ret;
>  }

This hack can be done in similar way on how we do ACPI LPSS for those
platforms, i.e. in drivers/acpi/x86/lpss.c. No?

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 2/4] pinctrl: intel: expose software nodes for baytrail GPIO devices

[Andy Shevchenko]

On Fri, Mar 20, 2026 at 12:38:24PM +0200, Andy Shevchenko wrote:
> On Thu, Mar 19, 2026 at 05:10:55PM +0100, Bartosz Golaszewski wrote:
> > Use the new automatic secondary fwnode API to ensure that when the
> > baytrail pinctrl device is added to the platform bus, the static
> > software node provided for drivers not able to use ACPI will get
> > automatically assigned as the secondary fwnode of the primary ACPI node.
> > 
> > Create a new header under linux/pinctrl/ containing intel-specific
> > symbols and declare the new variables there.
> 
> As I read the code, this doesn't need to be part of drivers/pinctrl/intel/.
> I.o.w. I am unable to see why we need to penetrate the certain pinctrl
> driver for this.

...

> >  static int __init byt_gpio_init(void)
> >  {
> > -	return platform_driver_register(&byt_gpio_driver);
> > +	int ret;
> > +
> > +	ret = software_node_register_auto_secondary(&byt_auto_secondary);
> > +	if (ret)
> > +		return ret;
> > +
> > +	ret = platform_driver_register(&byt_gpio_driver);
> > +	if (ret)
> > +		software_node_unregister_auto_secondary(&byt_auto_secondary);
> > +
> > +	return ret;
> >  }
> 
> This hack can be done in similar way on how we do ACPI LPSS for those
> platforms, i.e. in drivers/acpi/x86/lpss.c. No?

Note, that driver is used on above mentioned platforms. Without it they are
basically useless. With that said, requiring that driver is fine and good thing
to do on those platforms (Bay Trail and Cherry View).

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 2/4] pinctrl: intel: expose software nodes for baytrail GPIO devices

[Bartosz Golaszewski]

On Fri, Mar 20, 2026 at 11:39 AM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> On Thu, Mar 19, 2026 at 05:10:55PM +0100, Bartosz Golaszewski wrote:
> > Use the new automatic secondary fwnode API to ensure that when the
> > baytrail pinctrl device is added to the platform bus, the static
> > software node provided for drivers not able to use ACPI will get
> > automatically assigned as the secondary fwnode of the primary ACPI node.
> >
> > Create a new header under linux/pinctrl/ containing intel-specific
> > symbols and declare the new variables there.
>
> As I read the code, this doesn't need to be part of drivers/pinctrl/intel/.
> I.o.w. I am unable to see why we need to penetrate the certain pinctrl
> driver for this.
>

If old board files were an analogy, the kind of information software
nodes carry would live neither in the provider module nor in the
consumer driver. It would be defined in a third place - the board
file. Do we need something like this or should this logic be invoked
from the x86 platform driver that uses these GPIOs but accesses them
via the swnode lookup?

> ...
>
> >  static int __init byt_gpio_init(void)
> >  {
> > -     return platform_driver_register(&byt_gpio_driver);
> > +     int ret;
> > +
> > +     ret = software_node_register_auto_secondary(&byt_auto_secondary);
> > +     if (ret)
> > +             return ret;
> > +
> > +     ret = platform_driver_register(&byt_gpio_driver);
> > +     if (ret)
> > +             software_node_unregister_auto_secondary(&byt_auto_secondary);
> > +
> > +     return ret;
> >  }
>
> This hack can be done in similar way on how we do ACPI LPSS for those
> platforms, i.e. in drivers/acpi/x86/lpss.c. No?
>

Hey, this is not a hack! I'm coming up with a generic solution here. :)

It already is similar in that it uses a notifier. For v2 howevere, I
want to propose a mechanism for having multiple ways of matching real
fwnodes with software nodes.

Bart

Re: [PATCH 2/4] pinctrl: intel: expose software nodes for baytrail GPIO devices

[Andy Shevchenko]

On Mon, Mar 23, 2026 at 04:28:39PM +0100, Bartosz Golaszewski wrote:
> On Fri, Mar 20, 2026 at 11:39 AM Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> >
> > On Thu, Mar 19, 2026 at 05:10:55PM +0100, Bartosz Golaszewski wrote:
> > > Use the new automatic secondary fwnode API to ensure that when the
> > > baytrail pinctrl device is added to the platform bus, the static
> > > software node provided for drivers not able to use ACPI will get
> > > automatically assigned as the secondary fwnode of the primary ACPI node.
> > >
> > > Create a new header under linux/pinctrl/ containing intel-specific
> > > symbols and declare the new variables there.
> >
> > As I read the code, this doesn't need to be part of drivers/pinctrl/intel/.
> > I.o.w. I am unable to see why we need to penetrate the certain pinctrl
> > driver for this.
> 
> If old board files were an analogy, the kind of information software
> nodes carry would live neither in the provider module nor in the
> consumer driver. It would be defined in a third place - the board
> file. Do we need something like this or should this logic be invoked
> from the x86 platform driver that uses these GPIOs but accesses them
> via the swnode lookup?

Yes, fine, and that file is kinda board files (or rather quirks) for the
Bay Trail, Braswell and a few more. Please, use that one instead, let's
keep drivers/pinctrl/intel/ clean from it.

...

> > >  static int __init byt_gpio_init(void)
> > >  {
> > > -     return platform_driver_register(&byt_gpio_driver);
> > > +     int ret;
> > > +
> > > +     ret = software_node_register_auto_secondary(&byt_auto_secondary);
> > > +     if (ret)
> > > +             return ret;
> > > +
> > > +     ret = platform_driver_register(&byt_gpio_driver);
> > > +     if (ret)
> > > +             software_node_unregister_auto_secondary(&byt_auto_secondary);
> > > +
> > > +     return ret;
> > >  }
> >
> > This hack can be done in similar way on how we do ACPI LPSS for those
> > platforms, i.e. in drivers/acpi/x86/lpss.c. No?
> 
> Hey, this is not a hack! I'm coming up with a generic solution here. :)

Sorry, let's call it solution.

> It already is similar in that it uses a notifier. For v2 howevere, I
> want to propose a mechanism for having multiple ways of matching real
> fwnodes with software nodes.

Good.

-- 
With Best Regards,
Andy Shevchenko

[PATCH 3/4] pinctrl: intel: expose software nodes for cherryiew GPIO devices

[Bartosz Golaszewski]

Use the new automatic secondary fwnode API to ensure that when the
cherryview pinctrl device is added to the platform bus, the static
software node provided for drivers not able to use ACPI will get
automatically assigned as the secondary fwnode of the primary ACPI node.

Update the existing intel.h header with new symbols.

Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
---
 drivers/pinctrl/intel/pinctrl-cherryview.c | 47 +++++++++++++++++++++++++++++-
 include/linux/pinctrl/intel.h              |  5 ++++
 2 files changed, 51 insertions(+), 1 deletion(-)

diff --git a/drivers/pinctrl/intel/pinctrl-cherryview.c b/drivers/pinctrl/intel/pinctrl-cherryview.c
index a33eca5eafc44ee3337c4665967682404cb6ece0..45e52a796c8201c844127dbab497b4b3002930dc 100644
--- a/drivers/pinctrl/intel/pinctrl-cherryview.c
+++ b/drivers/pinctrl/intel/pinctrl-cherryview.c
@@ -21,6 +21,7 @@
 #include <linux/types.h>
 
 #include <linux/pinctrl/consumer.h>
+#include <linux/pinctrl/intel.h>
 #include <linux/pinctrl/pinconf-generic.h>
 #include <linux/pinctrl/pinconf.h>
 #include <linux/pinctrl/pinctrl.h>
@@ -1769,15 +1770,59 @@ static struct platform_driver chv_pinctrl_driver = {
 	},
 };
 
+const struct software_node cherryview_gpio_node_00 = {
+	.name = "INT33FF:00",
+};
+EXPORT_SYMBOL_NS_GPL(cherryview_gpio_node_00, "PINCTRL_INTEL");
+
+const struct software_node cherryview_gpio_node_01 = {
+	.name = "INT33FF:01",
+};
+EXPORT_SYMBOL_NS_GPL(cherryview_gpio_node_01, "PINCTRL_INTEL");
+
+const struct software_node cherryview_gpio_node_02 = {
+	.name = "INT33FF:02",
+};
+EXPORT_SYMBOL_NS_GPL(cherryview_gpio_node_02, "PINCTRL_INTEL");
+
+const struct software_node cherryview_gpio_node_03 = {
+	.name = "INT33FF:03",
+};
+EXPORT_SYMBOL_NS_GPL(cherryview_gpio_node_03, "PINCTRL_INTEL");
+
+static const struct software_node *chv_gpio_node_group[] = {
+	&cherryview_gpio_node_00,
+	&cherryview_gpio_node_01,
+	&cherryview_gpio_node_02,
+	&cherryview_gpio_node_03,
+	NULL
+};
+
+static struct software_node_auto_secondary chv_auto_secondary = {
+	.node_group = chv_gpio_node_group,
+	.bus = &platform_bus_type,
+};
+
 static int __init chv_pinctrl_init(void)
 {
-	return platform_driver_register(&chv_pinctrl_driver);
+	int ret;
+
+	ret = software_node_register_auto_secondary(&chv_auto_secondary);
+	if (ret)
+		return ret;
+
+	ret = platform_driver_register(&chv_pinctrl_driver);
+	if (ret)
+		software_node_unregister_auto_secondary(&chv_auto_secondary);
+
+	return ret;
 }
 subsys_initcall(chv_pinctrl_init);
 
 static void __exit chv_pinctrl_exit(void)
 {
 	platform_driver_unregister(&chv_pinctrl_driver);
+	software_node_unregister_auto_secondary(&chv_auto_secondary);
 }
 module_exit(chv_pinctrl_exit);
 
diff --git a/include/linux/pinctrl/intel.h b/include/linux/pinctrl/intel.h
index d45f090adc1f532f866c98aeca144a4d83fa28f4..bd3e6ee8b15ebc492275a14ae6d53827b701f783 100644
--- a/include/linux/pinctrl/intel.h
+++ b/include/linux/pinctrl/intel.h
@@ -9,4 +9,9 @@ extern const struct software_node baytrail_gpio_node_00;
 extern const struct software_node baytrail_gpio_node_01;
 extern const struct software_node baytrail_gpio_node_02;
 
+extern const struct software_node cherryview_gpio_node_00;
+extern const struct software_node cherryview_gpio_node_01;
+extern const struct software_node cherryview_gpio_node_02;
+extern const struct software_node cherryview_gpio_node_03;
+
 #endif /* __LINUX_PINCTRL_INTEL_H */

-- 
2.47.3

[PATCH 4/4] platform/x86: x86-android-tablets: enable fwnode matching of GPIO chips

[Bartosz Golaszewski]

With cherryview and baytrail drivers now exposing software nodes that
will actually be set as secondary firmware nodes of their primary ACPI
nodes and with GPIOLIB supporting fwnode matching for secondary fwnodes
as well, we can finally replace the dummy software nodes defined locally
in the x86-android-tablets driver and not attached to anything with ones
for which true fwnode GPIO lookup will work correctly and not require
the fallback to name matching.

Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
---
 drivers/platform/x86/x86-android-tablets/Kconfig   |  1 +
 drivers/platform/x86/x86-android-tablets/acer.c    | 11 ++---
 drivers/platform/x86/x86-android-tablets/asus.c    |  9 +++--
 drivers/platform/x86/x86-android-tablets/core.c    | 47 +---------------------
 drivers/platform/x86/x86-android-tablets/lenovo.c  | 31 +++++++-------
 drivers/platform/x86/x86-android-tablets/other.c   | 23 ++++++-----
 .../x86/x86-android-tablets/shared-psy-info.c      |  7 ++--
 .../x86/x86-android-tablets/x86-android-tablets.h  |  4 --
 8 files changed, 45 insertions(+), 88 deletions(-)

diff --git a/drivers/platform/x86/x86-android-tablets/Kconfig b/drivers/platform/x86/x86-android-tablets/Kconfig
index 193da15ee01ca5943581d800d5d2f82f39aee196..18425f787e933927add436c3354460fad166f092 100644
--- a/drivers/platform/x86/x86-android-tablets/Kconfig
+++ b/drivers/platform/x86/x86-android-tablets/Kconfig
@@ -8,6 +8,7 @@ config X86_ANDROID_TABLETS
 	depends on I2C && SPI && SERIAL_DEV_BUS
 	depends on GPIOLIB && PMIC_OPREGION
 	depends on ACPI && EFI && PCI
+	depends on PINCTRL_BAYTRAIL && PINCTRL_CHERRYVIEW
 	select NEW_LEDS
 	select LEDS_CLASS
 	select POWER_SUPPLY
diff --git a/drivers/platform/x86/x86-android-tablets/acer.c b/drivers/platform/x86/x86-android-tablets/acer.c
index d48c70ffd9925033ec4ab937a7e55622e3f7a374..037648a45fa78679e5e6df132433c8e6fa4e991f 100644
--- a/drivers/platform/x86/x86-android-tablets/acer.c
+++ b/drivers/platform/x86/x86-android-tablets/acer.c
@@ -10,6 +10,7 @@
 
 #include <linux/gpio/machine.h>
 #include <linux/gpio/property.h>
+#include <linux/pinctrl/intel.h>
 #include <linux/platform_device.h>
 #include <linux/property.h>
 
@@ -31,7 +32,7 @@ static const struct software_node acer_a1_840_bq24190_node = {
 static const struct property_entry acer_a1_840_touchscreen_props[] = {
 	PROPERTY_ENTRY_U32("touchscreen-size-x", 800),
 	PROPERTY_ENTRY_U32("touchscreen-size-y", 1280),
-	PROPERTY_ENTRY_GPIO("reset-gpios", &baytrail_gpiochip_nodes[1], 26, GPIO_ACTIVE_LOW),
+	PROPERTY_ENTRY_GPIO("reset-gpios", &baytrail_gpio_node_01, 26, GPIO_ACTIVE_LOW),
 	{ }
 };
 
@@ -91,8 +92,8 @@ static const struct x86_i2c_client_info acer_a1_840_i2c_clients[] __initconst =
 };
 
 static const struct property_entry acer_a1_840_int3496_props[] __initconst = {
-	PROPERTY_ENTRY_GPIO("mux-gpios", &baytrail_gpiochip_nodes[2], 1, GPIO_ACTIVE_HIGH),
-	PROPERTY_ENTRY_GPIO("id-gpios", &baytrail_gpiochip_nodes[2], 18, GPIO_ACTIVE_HIGH),
+	PROPERTY_ENTRY_GPIO("mux-gpios", &baytrail_gpio_node_02, 1, GPIO_ACTIVE_HIGH),
+	PROPERTY_ENTRY_GPIO("id-gpios", &baytrail_gpio_node_02, 18, GPIO_ACTIVE_HIGH),
 	{ }
 };
 
@@ -109,7 +110,7 @@ static const struct platform_device_info acer_a1_840_pdevs[] __initconst = {
 static const struct property_entry acer_a1_840_fg_props[] = {
 	PROPERTY_ENTRY_REF("monitored-battery", &generic_lipo_4v2_battery_node),
 	PROPERTY_ENTRY_STRING_ARRAY_LEN("supplied-from", bq24190_psy, 1),
-	PROPERTY_ENTRY_GPIO("charged-gpios", &baytrail_gpiochip_nodes[2], 10, GPIO_ACTIVE_HIGH),
+	PROPERTY_ENTRY_GPIO("charged-gpios", &baytrail_gpio_node_02, 10, GPIO_ACTIVE_HIGH),
 	{ }
 };
 
@@ -193,7 +194,7 @@ static const struct software_node acer_b1_750_bma250e_node = {
 };
 
 static const struct property_entry acer_b1_750_novatek_props[] = {
-	PROPERTY_ENTRY_GPIO("reset-gpios", &baytrail_gpiochip_nodes[1], 26, GPIO_ACTIVE_LOW),
+	PROPERTY_ENTRY_GPIO("reset-gpios", &baytrail_gpio_node_01, 26, GPIO_ACTIVE_LOW),
 	{ }
 };
 
diff --git a/drivers/platform/x86/x86-android-tablets/asus.c b/drivers/platform/x86/x86-android-tablets/asus.c
index 7d29c7654d214029720f444a17326851629c0342..42adf2a82bd4749f8b4c1d3ce9f733949292fd1b 100644
--- a/drivers/platform/x86/x86-android-tablets/asus.c
+++ b/drivers/platform/x86/x86-android-tablets/asus.c
@@ -12,13 +12,14 @@
 #include <linux/gpio/property.h>
 #include <linux/input-event-codes.h>
 #include <linux/platform_device.h>
+#include <linux/pinctrl/intel.h>
 
 #include "shared-psy-info.h"
 #include "x86-android-tablets.h"
 
 /* Asus ME176C and TF103C tablets shared data */
 static const struct property_entry asus_me176c_tf103c_int3496_props[] __initconst = {
-	PROPERTY_ENTRY_GPIO("id-gpios", &baytrail_gpiochip_nodes[2], 22, GPIO_ACTIVE_HIGH),
+	PROPERTY_ENTRY_GPIO("id-gpios", &baytrail_gpio_node_02, 22, GPIO_ACTIVE_HIGH),
 	{ }
 };
 
@@ -39,7 +40,7 @@ static const struct property_entry asus_me176c_tf103c_lid_props[] = {
 	PROPERTY_ENTRY_U32("linux,input-type", EV_SW),
 	PROPERTY_ENTRY_U32("linux,code", SW_LID),
 	PROPERTY_ENTRY_STRING("label", "lid_sw"),
-	PROPERTY_ENTRY_GPIO("gpios", &baytrail_gpiochip_nodes[2], 12, GPIO_ACTIVE_LOW),
+	PROPERTY_ENTRY_GPIO("gpios", &baytrail_gpio_node_02, 12, GPIO_ACTIVE_LOW),
 	PROPERTY_ENTRY_U32("debounce-interval", 50),
 	PROPERTY_ENTRY_BOOL("wakeup-source"),
 	{ }
@@ -97,8 +98,8 @@ static const struct software_node asus_me176c_ug3105_node = {
 };
 
 static const struct property_entry asus_me176c_touchscreen_props[] = {
-	PROPERTY_ENTRY_GPIO("reset-gpios", &baytrail_gpiochip_nodes[0], 60, GPIO_ACTIVE_HIGH),
-	PROPERTY_ENTRY_GPIO("irq-gpios", &baytrail_gpiochip_nodes[2], 28, GPIO_ACTIVE_HIGH),
+	PROPERTY_ENTRY_GPIO("reset-gpios", &baytrail_gpio_node_00, 60, GPIO_ACTIVE_HIGH),
+	PROPERTY_ENTRY_GPIO("irq-gpios", &baytrail_gpio_node_02, 28, GPIO_ACTIVE_HIGH),
 	{ }
 };
 
diff --git a/drivers/platform/x86/x86-android-tablets/core.c b/drivers/platform/x86/x86-android-tablets/core.c
index 021009e9085bec3db9c4daa1f6235600210a6099..7e8c0eb9792cac493268e6ecc3b3a0ecf8dccc4e 100644
--- a/drivers/platform/x86/x86-android-tablets/core.c
+++ b/drivers/platform/x86/x86-android-tablets/core.c
@@ -154,7 +154,6 @@ static struct platform_device **pdevs;
 static struct serdev_device **serdevs;
 static const struct software_node **gpio_button_swnodes;
 static const struct software_node **swnode_group;
-static const struct software_node **gpiochip_node_group;
 static void (*exit_handler)(void);
 
 static __init struct i2c_adapter *
@@ -332,34 +331,6 @@ static __init int x86_instantiate_serdev(const struct x86_dev_info *dev_info, in
 	return ret;
 }
 
-const struct software_node baytrail_gpiochip_nodes[] = {
-	{ .name = "INT33FC:00" },
-	{ .name = "INT33FC:01" },
-	{ .name = "INT33FC:02" },
-};
-
-static const struct software_node *baytrail_gpiochip_node_group[] = {
-	&baytrail_gpiochip_nodes[0],
-	&baytrail_gpiochip_nodes[1],
-	&baytrail_gpiochip_nodes[2],
-	NULL
-};
-
-const struct software_node cherryview_gpiochip_nodes[] = {
-	{ .name = "INT33FF:00" },
-	{ .name = "INT33FF:01" },
-	{ .name = "INT33FF:02" },
-	{ .name = "INT33FF:03" },
-};
-
-static const struct software_node *cherryview_gpiochip_node_group[] = {
-	&cherryview_gpiochip_nodes[0],
-	&cherryview_gpiochip_nodes[1],
-	&cherryview_gpiochip_nodes[2],
-	&cherryview_gpiochip_nodes[3],
-	NULL
-};
-
 static void x86_android_tablet_remove(struct platform_device *pdev)
 {
 	int i;
@@ -391,7 +362,6 @@ static void x86_android_tablet_remove(struct platform_device *pdev)
 
 	software_node_unregister_node_group(gpio_button_swnodes);
 	software_node_unregister_node_group(swnode_group);
-	software_node_unregister_node_group(gpiochip_node_group);
 }
 
 static __init int x86_android_tablet_probe(struct platform_device *pdev)
@@ -415,22 +385,6 @@ static __init int x86_android_tablet_probe(struct platform_device *pdev)
 	for (i = 0; dev_info->modules && dev_info->modules[i]; i++)
 		request_module(dev_info->modules[i]);
 
-	switch (dev_info->gpiochip_type) {
-	case X86_GPIOCHIP_BAYTRAIL:
-		gpiochip_node_group = baytrail_gpiochip_node_group;
-		break;
-	case X86_GPIOCHIP_CHERRYVIEW:
-		gpiochip_node_group = cherryview_gpiochip_node_group;
-		break;
-	case X86_GPIOCHIP_UNSPECIFIED:
-		gpiochip_node_group = NULL;
-		break;
-	}
-
-	ret = software_node_register_node_group(gpiochip_node_group);
-	if (ret)
-		return ret;
-
 	ret = software_node_register_node_group(dev_info->swnode_group);
 	if (ret) {
 		x86_android_tablet_remove(pdev);
@@ -563,3 +517,4 @@ module_exit(x86_android_tablet_exit);
 MODULE_AUTHOR("Hans de Goede <hansg@kernel.org>");
 MODULE_DESCRIPTION("X86 Android tablets DSDT fixups driver");
 MODULE_LICENSE("GPL");
+MODULE_IMPORT_NS("PINCTRL_INTEL");
diff --git a/drivers/platform/x86/x86-android-tablets/lenovo.c b/drivers/platform/x86/x86-android-tablets/lenovo.c
index 8d825e0b4661c0be879160ea963ac167112c41ea..bd4e0ca2c1ef35ab9e6b1bf9dbb91936e2815b84 100644
--- a/drivers/platform/x86/x86-android-tablets/lenovo.c
+++ b/drivers/platform/x86/x86-android-tablets/lenovo.c
@@ -18,6 +18,7 @@
 #include <linux/mfd/arizona/registers.h>
 #include <linux/mfd/intel_soc_pmic.h>
 #include <linux/pinctrl/consumer.h>
+#include <linux/pinctrl/intel.h>
 #include <linux/pinctrl/machine.h>
 #include <linux/platform_data/lp855x.h>
 #include <linux/platform_device.h>
@@ -72,8 +73,8 @@ static const struct software_node crystalcove_gpiochip_node = {
 /* Lenovo Yoga Book X90F / X90L's Android factory image has everything hardcoded */
 
 static const struct property_entry lenovo_yb1_x90_goodix_props[] = {
-	PROPERTY_ENTRY_GPIO("reset-gpios", &cherryview_gpiochip_nodes[1], 53, GPIO_ACTIVE_HIGH),
-	PROPERTY_ENTRY_GPIO("irq-gpios", &cherryview_gpiochip_nodes[1], 56, GPIO_ACTIVE_HIGH),
+	PROPERTY_ENTRY_GPIO("reset-gpios", &cherryview_gpio_node_01, 53, GPIO_ACTIVE_HIGH),
+	PROPERTY_ENTRY_GPIO("irq-gpios", &cherryview_gpio_node_01, 56, GPIO_ACTIVE_HIGH),
 	{ }
 };
 
@@ -84,7 +85,7 @@ static const struct software_node lenovo_yb1_x90_goodix_node = {
 static const struct property_entry lenovo_yb1_x90_wacom_props[] = {
 	PROPERTY_ENTRY_U32("hid-descr-addr", 0x0001),
 	PROPERTY_ENTRY_U32("post-reset-deassert-delay-ms", 150),
-	PROPERTY_ENTRY_GPIO("reset-gpios", &cherryview_gpiochip_nodes[0], 82, GPIO_ACTIVE_LOW),
+	PROPERTY_ENTRY_GPIO("reset-gpios", &cherryview_gpio_node_00, 82, GPIO_ACTIVE_LOW),
 	{ }
 };
 
@@ -106,7 +107,7 @@ static const struct property_entry lenovo_yb1_x90_hideep_ts_props[] = {
 	PROPERTY_ENTRY_U32("touchscreen-size-y", 1920),
 	PROPERTY_ENTRY_U32("touchscreen-max-pressure", 16384),
 	PROPERTY_ENTRY_BOOL("hideep,force-native-protocol"),
-	PROPERTY_ENTRY_GPIO("reset-gpios", &cherryview_gpiochip_nodes[0], 7, GPIO_ACTIVE_LOW),
+	PROPERTY_ENTRY_GPIO("reset-gpios", &cherryview_gpio_node_00, 7, GPIO_ACTIVE_LOW),
 	{ }
 };
 
@@ -221,7 +222,7 @@ static const struct property_entry lenovo_yb1_x90_lid_props[] = {
 	PROPERTY_ENTRY_U32("linux,input-type", EV_SW),
 	PROPERTY_ENTRY_U32("linux,code", SW_LID),
 	PROPERTY_ENTRY_STRING("label", "lid_sw"),
-	PROPERTY_ENTRY_GPIO("gpios", &cherryview_gpiochip_nodes[2], 19, GPIO_ACTIVE_LOW),
+	PROPERTY_ENTRY_GPIO("gpios", &cherryview_gpio_node_02, 19, GPIO_ACTIVE_LOW),
 	PROPERTY_ENTRY_U32("debounce-interval", 50),
 	PROPERTY_ENTRY_BOOL("wakeup-source"),
 	{ }
@@ -305,7 +306,7 @@ static const struct property_entry lenovo_yoga_tab2_830_1050_lid_props[] = {
 	PROPERTY_ENTRY_U32("linux,input-type", EV_SW),
 	PROPERTY_ENTRY_U32("linux,code", SW_LID),
 	PROPERTY_ENTRY_STRING("label", "lid_sw"),
-	PROPERTY_ENTRY_GPIO("gpios", &baytrail_gpiochip_nodes[2], 26, GPIO_ACTIVE_LOW),
+	PROPERTY_ENTRY_GPIO("gpios", &baytrail_gpio_node_02, 26, GPIO_ACTIVE_LOW),
 	PROPERTY_ENTRY_U32("debounce-interval", 50),
 	PROPERTY_ENTRY_BOOL("wakeup-source"),
 	{ }
@@ -400,8 +401,8 @@ static struct x86_i2c_client_info lenovo_yoga_tab2_830_1050_i2c_clients[] __init
 };
 
 static const struct property_entry lenovo_yoga_tab2_830_1050_int3496_props[] __initconst = {
-	PROPERTY_ENTRY_GPIO("mux-gpios", &baytrail_gpiochip_nodes[2], 1, GPIO_ACTIVE_LOW),
-	PROPERTY_ENTRY_GPIO("id-gpios", &baytrail_gpiochip_nodes[2], 24, GPIO_ACTIVE_HIGH),
+	PROPERTY_ENTRY_GPIO("mux-gpios", &baytrail_gpio_node_02, 1, GPIO_ACTIVE_LOW),
+	PROPERTY_ENTRY_GPIO("id-gpios", &baytrail_gpio_node_02, 24, GPIO_ACTIVE_HIGH),
 	{ }
 };
 
@@ -420,7 +421,7 @@ static const struct property_entry lenovo_yoga_tab2_830_1050_wm1502_props[] = {
 	PROPERTY_ENTRY_GPIO("reset-gpios",
 			    &crystalcove_gpiochip_node, 3, GPIO_ACTIVE_HIGH),
 	PROPERTY_ENTRY_GPIO("wlf,ldoena-gpios",
-			    &baytrail_gpiochip_nodes[1], 23, GPIO_ACTIVE_HIGH),
+			    &baytrail_gpio_node_01, 23, GPIO_ACTIVE_HIGH),
 	PROPERTY_ENTRY_GPIO("wlf,spkvdd-ena-gpios",
 			    &arizona_gpiochip_node, 2, GPIO_ACTIVE_HIGH),
 	PROPERTY_ENTRY_GPIO("wlf,micd-pol-gpios",
@@ -759,8 +760,8 @@ static const struct x86_i2c_client_info lenovo_yoga_tab2_1380_i2c_clients[] __in
 };
 
 static const struct property_entry lenovo_yoga_tab2_1380_fc_props[] __initconst = {
-	PROPERTY_ENTRY_GPIO("uart3_txd-gpios", &baytrail_gpiochip_nodes[0], 57, GPIO_ACTIVE_HIGH),
-	PROPERTY_ENTRY_GPIO("uart3_rxd-gpios", &baytrail_gpiochip_nodes[0], 61, GPIO_ACTIVE_HIGH),
+	PROPERTY_ENTRY_GPIO("uart3_txd-gpios", &baytrail_gpio_node_00, 57, GPIO_ACTIVE_HIGH),
+	PROPERTY_ENTRY_GPIO("uart3_rxd-gpios", &baytrail_gpio_node_00, 61, GPIO_ACTIVE_HIGH),
 	{ }
 };
 
@@ -850,7 +851,7 @@ static const struct property_entry lenovo_yt3_hideep_ts_props[] = {
 	PROPERTY_ENTRY_U32("touchscreen-size-x", 1600),
 	PROPERTY_ENTRY_U32("touchscreen-size-y", 2560),
 	PROPERTY_ENTRY_U32("touchscreen-max-pressure", 255),
-	PROPERTY_ENTRY_GPIO("reset-gpios", &cherryview_gpiochip_nodes[0], 7, GPIO_ACTIVE_LOW),
+	PROPERTY_ENTRY_GPIO("reset-gpios", &cherryview_gpio_node_00, 7, GPIO_ACTIVE_LOW),
 	{ }
 };
 
@@ -987,10 +988,10 @@ static struct arizona_pdata lenovo_yt3_wm5102_pdata = {
 
 static const struct property_entry lenovo_yt3_wm1502_props[] = {
 	PROPERTY_ENTRY_GPIO("wlf,spkvdd-ena-gpios",
-			    &cherryview_gpiochip_nodes[0], 75, GPIO_ACTIVE_HIGH),
+			    &cherryview_gpio_node_00, 75, GPIO_ACTIVE_HIGH),
 	PROPERTY_ENTRY_GPIO("wlf,ldoena-gpios",
-			    &cherryview_gpiochip_nodes[0], 81, GPIO_ACTIVE_HIGH),
-	PROPERTY_ENTRY_GPIO("reset-gpios", &cherryview_gpiochip_nodes[0], 82, GPIO_ACTIVE_HIGH),
+			    &cherryview_gpio_node_00, 81, GPIO_ACTIVE_HIGH),
+	PROPERTY_ENTRY_GPIO("reset-gpios", &cherryview_gpio_node_00, 82, GPIO_ACTIVE_HIGH),
 	PROPERTY_ENTRY_GPIO("wlf,micd-pol-gpios", &arizona_gpiochip_node, 2, GPIO_ACTIVE_HIGH),
 	{ }
 };
diff --git a/drivers/platform/x86/x86-android-tablets/other.c b/drivers/platform/x86/x86-android-tablets/other.c
index 7532af2d72d1d9de807a1f371425249ac5ff86da..b61930ef51570be402df04c5c2a646217908a8fc 100644
--- a/drivers/platform/x86/x86-android-tablets/other.c
+++ b/drivers/platform/x86/x86-android-tablets/other.c
@@ -14,6 +14,7 @@
 #include <linux/input-event-codes.h>
 #include <linux/leds.h>
 #include <linux/pci.h>
+#include <linux/pinctrl/intel.h>
 #include <linux/platform_device.h>
 #include <linux/pwm.h>
 
@@ -35,7 +36,7 @@ static const struct software_node advantech_mica_071_gpio_keys_node = {
 static const struct property_entry advantech_mica_071_prog1_key_props[] = {
 	PROPERTY_ENTRY_U32("linux,code", KEY_PROG1),
 	PROPERTY_ENTRY_STRING("label", "prog1_key"),
-	PROPERTY_ENTRY_GPIO("gpios", &baytrail_gpiochip_nodes[0], 2, GPIO_ACTIVE_LOW),
+	PROPERTY_ENTRY_GPIO("gpios", &baytrail_gpio_node_00, 2, GPIO_ACTIVE_LOW),
 	PROPERTY_ENTRY_U32("debounce-interval", 50),
 	{ }
 };
@@ -156,7 +157,7 @@ static const struct software_node cyberbook_t116_gpio_keys_node = {
 static const struct property_entry cyberbook_t116_prog1_key_props[] = {
 	PROPERTY_ENTRY_U32("linux,code", KEY_PROG1),
 	PROPERTY_ENTRY_STRING("label", "prog1_key"),
-	PROPERTY_ENTRY_GPIO("gpios", &cherryview_gpiochip_nodes[0], 30, GPIO_ACTIVE_LOW),
+	PROPERTY_ENTRY_GPIO("gpios", &cherryview_gpio_node_00, 30, GPIO_ACTIVE_LOW),
 	PROPERTY_ENTRY_U32("debounce-interval", 50),
 	{ }
 };
@@ -169,7 +170,7 @@ static const struct software_node cyberbook_t116_prog1_key_node = {
 static const struct property_entry cyberbook_t116_prog2_key_props[] = {
 	PROPERTY_ENTRY_U32("linux,code", KEY_PROG2),
 	PROPERTY_ENTRY_STRING("label", "prog2_key"),
-	PROPERTY_ENTRY_GPIO("gpios", &cherryview_gpiochip_nodes[3], 48, GPIO_ACTIVE_LOW),
+	PROPERTY_ENTRY_GPIO("gpios", &cherryview_gpio_node_03, 48, GPIO_ACTIVE_LOW),
 	PROPERTY_ENTRY_U32("debounce-interval", 50),
 	{ }
 };
@@ -244,8 +245,8 @@ static const struct software_node medion_lifetab_s10346_accel_node = {
 static const struct property_entry medion_lifetab_s10346_touchscreen_props[] = {
 	PROPERTY_ENTRY_BOOL("touchscreen-inverted-x"),
 	PROPERTY_ENTRY_BOOL("touchscreen-swapped-x-y"),
-	PROPERTY_ENTRY_GPIO("reset-gpios", &baytrail_gpiochip_nodes[1], 26, GPIO_ACTIVE_HIGH),
-	PROPERTY_ENTRY_GPIO("irq-gpios", &baytrail_gpiochip_nodes[2], 3, GPIO_ACTIVE_HIGH),
+	PROPERTY_ENTRY_GPIO("reset-gpios", &baytrail_gpio_node_01, 26, GPIO_ACTIVE_HIGH),
+	PROPERTY_ENTRY_GPIO("irq-gpios", &baytrail_gpio_node_02, 3, GPIO_ACTIVE_HIGH),
 	{ }
 };
 
@@ -378,7 +379,7 @@ static const struct software_node nextbook_ares8a_accel_node = {
 static const struct property_entry nextbook_ares8a_ft5416_props[] = {
 	PROPERTY_ENTRY_U32("touchscreen-size-x", 800),
 	PROPERTY_ENTRY_U32("touchscreen-size-y", 1280),
-	PROPERTY_ENTRY_GPIO("reset-gpios", &cherryview_gpiochip_nodes[1], 25, GPIO_ACTIVE_LOW),
+	PROPERTY_ENTRY_GPIO("reset-gpios", &cherryview_gpio_node_01, 25, GPIO_ACTIVE_LOW),
 	{ }
 };
 
@@ -435,7 +436,7 @@ static const struct software_node peaq_c1010_gpio_keys_node = {
 static const struct property_entry peaq_c1010_dolby_key_props[] = {
 	PROPERTY_ENTRY_U32("linux,code", KEY_SOUND),
 	PROPERTY_ENTRY_STRING("label", "dolby_key"),
-	PROPERTY_ENTRY_GPIO("gpios", &baytrail_gpiochip_nodes[0], 3, GPIO_ACTIVE_LOW),
+	PROPERTY_ENTRY_GPIO("gpios", &baytrail_gpio_node_00, 3, GPIO_ACTIVE_LOW),
 	PROPERTY_ENTRY_U32("debounce-interval", 50),
 	{ }
 };
@@ -481,8 +482,8 @@ static const struct property_entry whitelabel_tm800a550l_goodix_props[] = {
 	PROPERTY_ENTRY_STRING("firmware-name", "gt912-tm800a550l.fw"),
 	PROPERTY_ENTRY_STRING("goodix,config-name", "gt912-tm800a550l.cfg"),
 	PROPERTY_ENTRY_U32("goodix,main-clk", 54),
-	PROPERTY_ENTRY_GPIO("reset-gpios", &baytrail_gpiochip_nodes[1], 26, GPIO_ACTIVE_HIGH),
-	PROPERTY_ENTRY_GPIO("irq-gpios", &baytrail_gpiochip_nodes[2], 3, GPIO_ACTIVE_HIGH),
+	PROPERTY_ENTRY_GPIO("reset-gpios", &baytrail_gpio_node_01, 26, GPIO_ACTIVE_HIGH),
+	PROPERTY_ENTRY_GPIO("irq-gpios", &baytrail_gpio_node_02, 3, GPIO_ACTIVE_HIGH),
 	{ }
 };
 
@@ -531,7 +532,7 @@ const struct x86_dev_info whitelabel_tm800a550l_info __initconst = {
 static const struct property_entry vexia_edu_atla10_5v_touchscreen_props[] = {
 	PROPERTY_ENTRY_U32("hid-descr-addr", 0x0000),
 	PROPERTY_ENTRY_U32("post-reset-deassert-delay-ms", 120),
-	PROPERTY_ENTRY_GPIO("reset-gpios", &baytrail_gpiochip_nodes[1], 26, GPIO_ACTIVE_LOW),
+	PROPERTY_ENTRY_GPIO("reset-gpios", &baytrail_gpio_node_01, 26, GPIO_ACTIVE_LOW),
 	{ }
 };
 
@@ -605,7 +606,7 @@ static const struct software_node vexia_edu_atla10_9v_accel_node = {
 static const struct property_entry vexia_edu_atla10_9v_touchscreen_props[] = {
 	PROPERTY_ENTRY_U32("hid-descr-addr", 0x0000),
 	PROPERTY_ENTRY_U32("post-reset-deassert-delay-ms", 120),
-	PROPERTY_ENTRY_GPIO("reset-gpios", &baytrail_gpiochip_nodes[0], 60, GPIO_ACTIVE_LOW),
+	PROPERTY_ENTRY_GPIO("reset-gpios", &baytrail_gpio_node_00, 60, GPIO_ACTIVE_LOW),
 	{ }
 };
 
diff --git a/drivers/platform/x86/x86-android-tablets/shared-psy-info.c b/drivers/platform/x86/x86-android-tablets/shared-psy-info.c
index 29fc466f76fe7a99f7cb853e38f62593d7464138..f214152a9b0455089d40b977bf1116098afac764 100644
--- a/drivers/platform/x86/x86-android-tablets/shared-psy-info.c
+++ b/drivers/platform/x86/x86-android-tablets/shared-psy-info.c
@@ -10,6 +10,7 @@
 
 #include <linux/gpio/machine.h>
 #include <linux/gpio/property.h>
+#include <linux/pinctrl/intel.h>
 #include <linux/platform_device.h>
 #include <linux/power/bq24190_charger.h>
 #include <linux/property.h>
@@ -169,9 +170,9 @@ const char * const bq24190_modules[] __initconst = {
 };
 
 static const struct property_entry int3496_reference_props[] __initconst = {
-	PROPERTY_ENTRY_GPIO("vbus-gpios", &baytrail_gpiochip_nodes[1], 15, GPIO_ACTIVE_HIGH),
-	PROPERTY_ENTRY_GPIO("mux-gpios", &baytrail_gpiochip_nodes[2], 1, GPIO_ACTIVE_HIGH),
-	PROPERTY_ENTRY_GPIO("id-gpios", &baytrail_gpiochip_nodes[2], 18, GPIO_ACTIVE_HIGH),
+	PROPERTY_ENTRY_GPIO("vbus-gpios", &baytrail_gpio_node_01, 15, GPIO_ACTIVE_HIGH),
+	PROPERTY_ENTRY_GPIO("mux-gpios", &baytrail_gpio_node_02, 1, GPIO_ACTIVE_HIGH),
+	PROPERTY_ENTRY_GPIO("id-gpios", &baytrail_gpio_node_02, 18, GPIO_ACTIVE_HIGH),
 	{ }
 };
 
diff --git a/drivers/platform/x86/x86-android-tablets/x86-android-tablets.h b/drivers/platform/x86/x86-android-tablets/x86-android-tablets.h
index 2498390958ad44e89b1c929ff46397e3f9e0c74b..cc61e49f93890e3cde16ea549d2a4f9fd11a41ac 100644
--- a/drivers/platform/x86/x86-android-tablets/x86-android-tablets.h
+++ b/drivers/platform/x86/x86-android-tablets/x86-android-tablets.h
@@ -103,10 +103,6 @@ int x86_android_tablet_get_gpiod(const char *chip, int pin, const char *con_id,
 				 struct gpio_desc **desc);
 int x86_acpi_irq_helper_get(const struct x86_acpi_irq_data *data);
 
-/* Software nodes representing GPIO chips used by various tablets */
-extern const struct software_node baytrail_gpiochip_nodes[];
-extern const struct software_node cherryview_gpiochip_nodes[];
-
 /*
  * Extern declarations of x86_dev_info structs so there can be a single
  * MODULE_DEVICE_TABLE(dmi, ...), while splitting the board descriptions.

-- 
2.47.3

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

[Dmitry Torokhov]

Hi Bartosz,

On Thu, Mar 19, 2026 at 05:10:53PM +0100, Bartosz Golaszewski wrote:
> Problem statement: GPIO software node lookup should rely exclusively on
> matching the addresses of the referenced firmware nodes. I tried to
> enforce it with commit e5d527be7e69 ("gpio: swnode: don't use the
> swnode's name as the key for GPIO lookup") but it broke existing
> users who abuse the software node mechanism by creating "dummy" software
> nodes named after the device they want to get GPIOs from but never
> attaching them to the actual GPIO devices. They rely on the current
> behavior of GPIOLIB where it will match the label of the GPIO controller
> against the name of the software node and does not require a true link.
> 
> x86-android-tablets driver is one of the abusers in that it creates
> dummy software nodes for baytrail and cherryview GPIO controllers but
> they don't really reference these devices. Before we can reapply
> e5d527be7e69 and support matching by fwnode address exclusively, we need
> to convert all the users to using actual fwnode references.
> 
> It's possible for drivers to reference real firmware nodes from software
> nodes via PROPERTY_ENTRY_REF() in general or PROPERTY_ENTRY_GPIO()
> specifically but for platform devices binding to real firmware nodes (DT
> or ACPI) it's cumbersome as they would need to dynamically look for the
> right nodes and create references dynamically with no way of using
> static const software nodes.
> 
> 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.

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

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?

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

Thanks.

-- 
Dmitry

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@gmail.com> 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

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

[Dmitry Torokhov]

On Fri, Mar 20, 2026 at 01:33:06PM -0700, Bartosz Golaszewski wrote:
> On Fri, 20 Mar 2026 02:49:02 +0100, Dmitry Torokhov
> <dmitry.torokhov@gmail.com> 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.

I mentioned link order, which implies no modules are involved. When code
is built into the kernel initialization follows link order, which is
typically alphabetical. To ensure the order you require you either need
to move targets inside makefiles or change some drivers from
module_init() to <some other>_initcall(). This is known as "link order
hell" that was very common before deferred probing was introduced.

> 
> > 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.

What ACPI has to do with this? Oftentimes we are dealing with non x86
systems.

> 
> > 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.

I think you completely misunderstood the proposal. We are not using
software node as a string wrapper, we give an opportunity to use
software node name to resolve to the real software node at the time we
try to resolve the reference. The software node is still expected to be
bound to the target device (unlike the original approach that has a
dangling software node which name expected to match gpiochip label).

I think this actually a very good approach: it severs the tight coupling
while still maintains the spirit of firmware nodes being attached to
devices. The only difference we are using object's name and not its
address as starting point. Similarly how you use name derived from
device name to locate and assign secondary node in this patch series.

> 
> 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

Yes, and the patch above solves this.

> 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. :)

I would like to understand why. I outlined the problems with this
approach (too tight coupling, needing to export nodes, include
additional headers, and deal with the link order issues, along with
potential changes to the system initialization/probe order). What are
the drawbacks of name matching as long as we do not create
dangling/shadow nodes?

You are saying that you want resolve strictly by address, but have you
looked at how OF references are resolved? Hint: phandle is not a raw
address. It's actually a 32 bit integer! Look at ACPI. It also does not
simply have a pointer to another ACPI node there, the data structure is
much more complex.

So why are you trying to make software nodes different from all the
other firmware nodes?

> 
> 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.

There is, you just misunderstood the proposal I'm afraid.

Thanks.

-- 
Dmitry

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

[Bartosz Golaszewski]

On Sat, 21 Mar 2026 08:01:30 +0100, Dmitry Torokhov
<dmitry.torokhov@gmail.com> said:
> On Fri, Mar 20, 2026 at 01:33:06PM -0700, Bartosz Golaszewski wrote:
>> On Fri, 20 Mar 2026 02:49:02 +0100, Dmitry Torokhov
>> <dmitry.torokhov@gmail.com> 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.
>
> I mentioned link order, which implies no modules are involved. When code
> is built into the kernel initialization follows link order, which is
> typically alphabetical. To ensure the order you require you either need
> to move targets inside makefiles or change some drivers from
> module_init() to <some other>_initcall(). This is known as "link order
> hell" that was very common before deferred probing was introduced.
>

Maybe we should return -EPROBE_DEFER in GPIO swnode lookup when
fwnode_property_get_reference_args() returns -ENOENT because if there's a
software node that's not yet been registered as a firmware node, it's not much
different from there being a firmware node not bound to a device yet - which
is grounds for probe deferral.

>>
>> > 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.
>
> What ACPI has to do with this? Oftentimes we are dealing with non x86
> systems.
>

What I meant is: name matching is only one method of assigning software nodes
to devices. Here I went with waiting for devices but with DT we may as well
look up a node by compatible depending on the driver. For ACPI we may probably
use some mechanism that matches the devices to the software node in a more
specific way. That's just hypothetical, I don't know if there even is one.

IOW: device-name-to-software-node name matching is only one of possible methods.

>>
>> > 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.
>
> I think you completely misunderstood the proposal. We are not using

Ok, I didn't fully get that part, I was OoO on Friday and should have probably
not rushed a late evening answer. :)

> software node as a string wrapper, we give an opportunity to use
> software node name to resolve to the real software node at the time we
> try to resolve the reference. The software node is still expected to be
> bound to the target device (unlike the original approach that has a
> dangling software node which name expected to match gpiochip label).
>
> I think this actually a very good approach: it severs the tight coupling
> while still maintains the spirit of firmware nodes being attached to
> devices. The only difference we are using object's name and not its
> address as starting point. Similarly how you use name derived from
> device name to locate and assign secondary node in this patch series.
>

I still don't like it. It forces us to use names for the remote software nodes,
which are after all C structures that we could identify by addresses alone.
Even this series could be reworked to drop the names from the GPIO software
nodes.

software_node_find_by_name() only goes through the list of registered software
nodes so we'd still end up returning -ENOENT for nodes which have not been
registered yet and wouldn't be able to tell this situation apart from a case
where there's no such software node at all.

The consumer driver still needs to know the remote device by an arbitrary
string.

I think you're exaggarating the possible problems with link order. I believe
that issue could be fixed by returning EPROBE_DEFER when a software node is
not yet known by an fwnode handle but we know it is there so it's just a matter
of it getting registered.

That being said, I would like to hear from driver core maintainers in general
and from software node maintainers in particular.

>>
>> 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
>
> Yes, and the patch above solves this.
>

I would say it just pushes the string matching deeper.

>> 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. :)
>
> I would like to understand why. I outlined the problems with this
> approach (too tight coupling, needing to export nodes, include
> additional headers, and deal with the link order issues, along with
> potential changes to the system initialization/probe order). What are
> the drawbacks of name matching as long as we do not create
> dangling/shadow nodes?
>
> You are saying that you want resolve strictly by address, but have you
> looked at how OF references are resolved? Hint: phandle is not a raw
> address. It's actually a 32 bit integer! Look at ACPI. It also does not
> simply have a pointer to another ACPI node there, the data structure is
> much more complex.
>

That's dictated by the binary format of devicetree where:

	prop = <&foo>;

Is translated to:

	foo {
		phandle = <0x123>;
	};

	prop = <0x123>;

And I suppose this could be translated into addresses of respective struct
device_node objects already at the tree unflattening stage - just like is
done for parent-child relationships.

However, this is very much hidden from users and someone who deals with DT
sources always sees a proper reference to a node (by path, label or otherwise).

Here we're talking about referring to firmware nodes by name which does happen
but is typically reserved for some special cases and well-known nodes.

Bartosz

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

[Dmitry Torokhov]

On Mon, Mar 23, 2026 at 04:40:11AM -0700, Bartosz Golaszewski wrote:
> On Sat, 21 Mar 2026 08:01:30 +0100, Dmitry Torokhov
> <dmitry.torokhov@gmail.com> said:
> > On Fri, Mar 20, 2026 at 01:33:06PM -0700, Bartosz Golaszewski wrote:
> >> On Fri, 20 Mar 2026 02:49:02 +0100, Dmitry Torokhov
> >> <dmitry.torokhov@gmail.com> 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.
> >
> > I mentioned link order, which implies no modules are involved. When code
> > is built into the kernel initialization follows link order, which is
> > typically alphabetical. To ensure the order you require you either need
> > to move targets inside makefiles or change some drivers from
> > module_init() to <some other>_initcall(). This is known as "link order
> > hell" that was very common before deferred probing was introduced.
> >
> 
> Maybe we should return -EPROBE_DEFER in GPIO swnode lookup when
> fwnode_property_get_reference_args() returns -ENOENT because if there's a
> software node that's not yet been registered as a firmware node, it's not much
> different from there being a firmware node not bound to a device yet - which
> is grounds for probe deferral.

Yes, I think this is a good idea.

> 
> >>
> >> > 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.
> >
> > What ACPI has to do with this? Oftentimes we are dealing with non x86
> > systems.
> >
> 
> What I meant is: name matching is only one method of assigning software nodes
> to devices. Here I went with waiting for devices but with DT we may as well
> look up a node by compatible depending on the driver. For ACPI we may probably
> use some mechanism that matches the devices to the software node in a more
> specific way. That's just hypothetical, I don't know if there even is one.
> 
> IOW: device-name-to-software-node name matching is only one of possible methods.

I see.

> 
> >>
> >> > 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.
> >
> > I think you completely misunderstood the proposal. We are not using
> 
> Ok, I didn't fully get that part, I was OoO on Friday and should have probably
> not rushed a late evening answer. :)
> 
> > software node as a string wrapper, we give an opportunity to use
> > software node name to resolve to the real software node at the time we
> > try to resolve the reference. The software node is still expected to be
> > bound to the target device (unlike the original approach that has a
> > dangling software node which name expected to match gpiochip label).
> >
> > I think this actually a very good approach: it severs the tight coupling
> > while still maintains the spirit of firmware nodes being attached to
> > devices. The only difference we are using object's name and not its
> > address as starting point. Similarly how you use name derived from
> > device name to locate and assign secondary node in this patch series.
> >
> 
> I still don't like it. It forces us to use names for the remote software nodes,
> which are after all C structures that we could identify by addresses alone.
> Even this series could be reworked to drop the names from the GPIO software
> nodes.

The whole device property business is string matching, because that is
what device properties are. If we want strict type matching we should
continue using per-driver platform data, it provides type safety. But I
think the preference is to actually use properties for configuring
devices.

> 
> software_node_find_by_name() only goes through the list of registered software
> nodes so we'd still end up returning -ENOENT for nodes which have not been
> registered yet and wouldn't be able to tell this situation apart from a case
> where there's no such software node at all.

Yes, we'd need to convert this to -EPROBE_DEFER similarly to what you
proposed above.

> 
> The consumer driver still needs to know the remote device by an arbitrary
> string.

Yes, but I think this is acceptable because of increased flexibility.

> 
> I think you're exaggarating the possible problems with link order. I believe
> that issue could be fixed by returning EPROBE_DEFER when a software node is
> not yet known by an fwnode handle but we know it is there so it's just a matter
> of it getting registered.

You need to have the module loaded and initialized so that the address
can be resolved. This changes link order and may produce unwanted
changes to the device init order. 

> 
> That being said, I would like to hear from driver core maintainers in general
> and from software node maintainers in particular.

I think I'll send out a forma patch and we can discuss further.

Thanks.

-- 
Dmitry