mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH v3 0/2] platform/x86: x86-android-tablets: use real firmware node references with intel drivers
@ 2026-04-27 12:19 Bartosz Golaszewski
  2026-04-27 12:19 ` [PATCH v3 1/2] ACPI: bus: export the acpi_bus_type symbol Bartosz Golaszewski
  2026-04-27 12:19 ` [PATCH v3 2/2] platform/x86: x86-android-tablets: enable fwnode matching of GPIO chips Bartosz Golaszewski
  0 siblings, 2 replies; 7+ messages in thread
From: Bartosz Golaszewski @ 2026-04-27 12:19 UTC (permalink / raw)
  To: Andy Shevchenko, Daniel Scally, Heikki Krogerus, Sakari Ailus,
	Greg Kroah-Hartman, Rafael J. Wysocki, Danilo Krummrich,
	Mika Westerberg, Andy Shevchenko, Linus Walleij, Hans de Goede,
	Ilpo Järvinen, Dmitry Torokhov, Len Brown
  Cc: linux-acpi, driver-core, linux-kernel, linux-gpio,
	platform-driver-x86, brgl, Bartosz Golaszewski

First let me repeat the 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.

==

Now for the new approach:

Now with the recent patches I sent or plan to send out, it turns out
that almost all existing users of software node lookup can easily have
the software node's representing the GPIO controllers attached to their
target devices as their primary firmware node. The remaining complex
cases are: pcengines-apuv2 and geode-common but in the first case we
have control over the creation of the GPIO device - we can assign it a
firmware node - and in the second case, we deal with an MFD driver so
can export the cell's software node in a header.

The only remaining use-case where we need to reference a GPIO controller
described in ACPI from a software node is the x86-android-tablets
driver. In this case, I think it no longer makes sense to propose a new
generic interface. Instead, I'd like to propose to keep it as a
driver-specific quirk.

This series proposes the following changes:
1. Export the acpi_bus_type symbol. It's already available in the
acpi_bus.h header but it's not available to loadable modules.
2. Implement support for dynamic assignment of secondary firmware nodes
in the x86-android-tablets driver. More details on how it's approached
are in the relevant commit message.

The dependencies in driver core and GPIOLIB are already upstream.

Merging strategy: I'd like to propose queing patches 1-3 independently
through their respective trees: driver core, GPIO and ACPI. The final
patch would make its way upstream after v7.1-rc1. I would probably
extend it to convert the driver to using more devres interfaces too at
this point.

Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
---
Changes in v3:
- Drop dependencies that got upstream for v7.1
- Rebase on top of v7.1-rc1
- Link to v2: https://patch.msgid.link/20260402-baytrail-real-swnode-v2-0-6f5054a4cc07@oss.qualcomm.com

Changes in v2:
- Completely new approach
- Keep the cherryview and baytrail secondary fwnode handling as a
  driver-specific quirk for x86 tablets and don't introduce any new API
- Change the return value when resolving a reference to an unregistered
  software node and defer probe in GPIOLIB in that situation
- Export acpi_bus_type for use in x86-android-tablets
- Link to v1: https://patch.msgid.link/20260319-baytrail-real-swnode-v1-0-75f2264ae49f@oss.qualcomm.com

---
Bartosz Golaszewski (2):
      ACPI: bus: export the acpi_bus_type symbol
      platform/x86: x86-android-tablets: enable fwnode matching of GPIO chips

 drivers/acpi/bus.c                              |   1 +
 drivers/platform/x86/x86-android-tablets/core.c | 127 +++++++++++++++++++++++-
 2 files changed, 125 insertions(+), 3 deletions(-)
---
base-commit: 085f14acda1c46ab815605ecf2341afe5a691eb9
change-id: 20260319-baytrail-real-swnode-7de7a3870f78

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


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH v3 1/2] ACPI: bus: export the acpi_bus_type symbol
  2026-04-27 12:19 [PATCH v3 0/2] platform/x86: x86-android-tablets: use real firmware node references with intel drivers Bartosz Golaszewski
@ 2026-04-27 12:19 ` Bartosz Golaszewski
  2026-04-27 19:27   ` Rafael J. Wysocki
  2026-04-27 12:19 ` [PATCH v3 2/2] platform/x86: x86-android-tablets: enable fwnode matching of GPIO chips Bartosz Golaszewski
  1 sibling, 1 reply; 7+ messages in thread
From: Bartosz Golaszewski @ 2026-04-27 12:19 UTC (permalink / raw)
  To: Andy Shevchenko, Daniel Scally, Heikki Krogerus, Sakari Ailus,
	Greg Kroah-Hartman, Rafael J. Wysocki, Danilo Krummrich,
	Mika Westerberg, Andy Shevchenko, Linus Walleij, Hans de Goede,
	Ilpo Järvinen, Dmitry Torokhov, Len Brown
  Cc: linux-acpi, driver-core, linux-kernel, linux-gpio,
	platform-driver-x86, brgl, Bartosz Golaszewski

In order to allow loadable modules to access the ACPI bus type struct,
export the relevant symbol.

Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
---
 drivers/acpi/bus.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
index 2ec095e2009e40b0645f654fb051429e5679dc7c..d4da76607905473de2e25e50352450c10f598784 100644
--- a/drivers/acpi/bus.c
+++ b/drivers/acpi/bus.c
@@ -1174,6 +1174,7 @@ const struct bus_type acpi_bus_type = {
 	.remove		= acpi_device_remove,
 	.uevent		= acpi_device_uevent,
 };
+EXPORT_SYMBOL_GPL(acpi_bus_type);
 
 int acpi_bus_for_each_dev(int (*fn)(struct device *, void *), void *data)
 {

-- 
2.47.3


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH v3 2/2] platform/x86: x86-android-tablets: enable fwnode matching of GPIO chips
  2026-04-27 12:19 [PATCH v3 0/2] platform/x86: x86-android-tablets: use real firmware node references with intel drivers Bartosz Golaszewski
  2026-04-27 12:19 ` [PATCH v3 1/2] ACPI: bus: export the acpi_bus_type symbol Bartosz Golaszewski
@ 2026-04-27 12:19 ` Bartosz Golaszewski
  2026-04-27 19:32   ` Rafael J. Wysocki
  2026-04-28  9:16   ` Hans de Goede
  1 sibling, 2 replies; 7+ messages in thread
From: Bartosz Golaszewski @ 2026-04-27 12:19 UTC (permalink / raw)
  To: Andy Shevchenko, Daniel Scally, Heikki Krogerus, Sakari Ailus,
	Greg Kroah-Hartman, Rafael J. Wysocki, Danilo Krummrich,
	Mika Westerberg, Andy Shevchenko, Linus Walleij, Hans de Goede,
	Ilpo Järvinen, Dmitry Torokhov, Len Brown
  Cc: linux-acpi, driver-core, linux-kernel, linux-gpio,
	platform-driver-x86, brgl, Bartosz Golaszewski

In order to allow GPIOLIB to match cherryview and baytrail GPIO
controllers by their firmware nodes instead of their names, we need to
attach the - currently "dangling" - existing software nodes to their
target devices dynamically.

We deal with devices described in ACPI so set up a bus notifier waiting
for the ADD events. We know the name of the device we're waiting for so
match against it and - on match - assign the appropriate software node
as the secondary firmware node of the underlying ACPI node. In case the
event was emitted earlier than this driver's probe: also make sure the
device was not added before.

Scheduling fine-grained devres actions allows for proper teardown and
unsetting of the secondary firmware nodes.

Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
---
 drivers/platform/x86/x86-android-tablets/core.c | 127 +++++++++++++++++++++++-
 1 file changed, 124 insertions(+), 3 deletions(-)

diff --git a/drivers/platform/x86/x86-android-tablets/core.c b/drivers/platform/x86/x86-android-tablets/core.c
index 021009e9085bec3db9c4daa1f6235600210a6099..9e6e8f272dfe16cda421b569802045c3d94fc0ab 100644
--- a/drivers/platform/x86/x86-android-tablets/core.c
+++ b/drivers/platform/x86/x86-android-tablets/core.c
@@ -13,10 +13,12 @@
 #include <linux/acpi.h>
 #include <linux/device.h>
 #include <linux/dmi.h>
+#include <linux/fwnode.h>
 #include <linux/gpio/consumer.h>
 #include <linux/gpio/machine.h>
 #include <linux/irq.h>
 #include <linux/module.h>
+#include <linux/notifier.h>
 #include <linux/pci.h>
 #include <linux/platform_device.h>
 #include <linux/serdev.h>
@@ -360,6 +362,124 @@ static const struct software_node *cherryview_gpiochip_node_group[] = {
 	NULL
 };
 
+struct auto_secondary_data {
+	struct notifier_block nb;
+	struct device *parent;
+};
+
+static void auto_secondary_unset(void *data)
+{
+	struct fwnode_handle *fwnode = data;
+
+	fwnode->secondary = NULL;
+}
+
+static int acpi_set_secondary_fwnode(struct device *parent, struct device *dev,
+				     const struct software_node *const swnode)
+{
+	struct acpi_device *device = to_acpi_device(dev);
+	struct fwnode_handle *fwnode;
+	int ret;
+
+	fwnode = software_node_fwnode(swnode);
+	if (WARN_ON(!fwnode))
+		return -ENOENT;
+
+	fwnode->secondary = ERR_PTR(-ENODEV);
+	device->fwnode.secondary = fwnode;
+
+	ret = devm_add_action_or_reset(parent, auto_secondary_unset, &device->fwnode);
+	if (ret)
+		dev_err(parent, "Failed to schedule the unset action for secondary fwnode\n");
+
+	return ret;
+}
+
+static int acpi_auto_secondary_notifier(struct notifier_block *nb,
+					unsigned long action, void *data)
+{
+	struct auto_secondary_data *auto_sec = container_of(nb, struct auto_secondary_data, nb);
+	const struct software_node *const *swnode;
+	struct device *dev = data;
+	int ret;
+
+	switch (action) {
+	case BUS_NOTIFY_ADD_DEVICE:
+		for (swnode = gpiochip_node_group; *swnode; swnode++) {
+			if (strcmp((*swnode)->name, dev_name(dev)) == 0) {
+				ret = acpi_set_secondary_fwnode(auto_sec->parent, dev, *swnode);
+				return ret ? NOTIFY_BAD : NOTIFY_OK;
+			}
+		}
+		break;
+	default:
+		break;
+	}
+
+	return NOTIFY_DONE;
+}
+
+static void auto_secondary_unregister_node_group(void *data)
+{
+	const struct software_node **nodes = data;
+
+	software_node_unregister_node_group(nodes);
+}
+
+static void auto_secondary_unregister_notifier(void *data)
+{
+	struct notifier_block *nb = data;
+
+	bus_unregister_notifier(&acpi_bus_type, nb);
+}
+
+static int auto_secondary_fwnode_init(struct device *parent)
+{
+	const struct software_node *const *swnode;
+	struct auto_secondary_data *data;
+	int ret;
+
+	ret = software_node_register_node_group(gpiochip_node_group);
+	if (ret)
+		return ret;
+
+	ret = devm_add_action_or_reset(parent,
+				       auto_secondary_unregister_node_group,
+				       gpiochip_node_group);
+	if (ret)
+		return ret;
+
+	data = devm_kzalloc(parent, sizeof(*data), GFP_KERNEL);
+	if (!data)
+		return -ENOMEM;
+
+	data->nb.notifier_call = acpi_auto_secondary_notifier;
+	data->parent = parent;
+
+	ret = bus_register_notifier(&acpi_bus_type, &data->nb);
+	if (ret)
+		return ret;
+
+	ret = devm_add_action_or_reset(parent,
+				       auto_secondary_unregister_notifier,
+				       &data->nb);
+	if (ret)
+		return ret;
+
+	/* Device may have been already added. */
+	for (swnode = gpiochip_node_group; *swnode; swnode++) {
+		struct device *dev __free(put_device) =
+			bus_find_device_by_name(&acpi_bus_type, NULL, (*swnode)->name);
+		if (dev) {
+			ret = acpi_set_secondary_fwnode(parent, dev, *swnode);
+			if (ret)
+				return ret;
+		}
+	}
+
+	return 0;
+}
+
 static void x86_android_tablet_remove(struct platform_device *pdev)
 {
 	int i;
@@ -391,7 +511,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)
@@ -427,9 +546,11 @@ static __init int x86_android_tablet_probe(struct platform_device *pdev)
 		break;
 	}
 
-	ret = software_node_register_node_group(gpiochip_node_group);
-	if (ret)
+	ret = auto_secondary_fwnode_init(&pdev->dev);
+	if (ret) {
+		x86_android_tablet_remove(pdev);
 		return ret;
+	}
 
 	ret = software_node_register_node_group(dev_info->swnode_group);
 	if (ret) {

-- 
2.47.3


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v3 1/2] ACPI: bus: export the acpi_bus_type symbol
  2026-04-27 12:19 ` [PATCH v3 1/2] ACPI: bus: export the acpi_bus_type symbol Bartosz Golaszewski
@ 2026-04-27 19:27   ` Rafael J. Wysocki
  0 siblings, 0 replies; 7+ messages in thread
From: Rafael J. Wysocki @ 2026-04-27 19:27 UTC (permalink / raw)
  To: Bartosz Golaszewski
  Cc: Andy Shevchenko, Daniel Scally, Heikki Krogerus, Sakari Ailus,
	Greg Kroah-Hartman, Rafael J. Wysocki, Danilo Krummrich,
	Mika Westerberg, Andy Shevchenko, Linus Walleij, Hans de Goede,
	Ilpo Järvinen, Dmitry Torokhov, Len Brown, linux-acpi,
	driver-core, linux-kernel, linux-gpio, platform-driver-x86, brgl

On Mon, Apr 27, 2026 at 2:19 PM Bartosz Golaszewski
<bartosz.golaszewski@oss.qualcomm.com> wrote:
>
> In order to allow loadable modules to access the ACPI bus type struct,
> export the relevant symbol.
>
> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
> ---
>  drivers/acpi/bus.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
> index 2ec095e2009e40b0645f654fb051429e5679dc7c..d4da76607905473de2e25e50352450c10f598784 100644
> --- a/drivers/acpi/bus.c
> +++ b/drivers/acpi/bus.c
> @@ -1174,6 +1174,7 @@ const struct bus_type acpi_bus_type = {
>         .remove         = acpi_device_remove,
>         .uevent         = acpi_device_uevent,
>  };
> +EXPORT_SYMBOL_GPL(acpi_bus_type);

Not really (and I've already said that I think).

Please find a different way.

>  int acpi_bus_for_each_dev(int (*fn)(struct device *, void *), void *data)
>  {
>
> --

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v3 2/2] platform/x86: x86-android-tablets: enable fwnode matching of GPIO chips
  2026-04-27 12:19 ` [PATCH v3 2/2] platform/x86: x86-android-tablets: enable fwnode matching of GPIO chips Bartosz Golaszewski
@ 2026-04-27 19:32   ` Rafael J. Wysocki
  2026-04-28  9:16   ` Hans de Goede
  1 sibling, 0 replies; 7+ messages in thread
From: Rafael J. Wysocki @ 2026-04-27 19:32 UTC (permalink / raw)
  To: Bartosz Golaszewski
  Cc: Andy Shevchenko, Daniel Scally, Heikki Krogerus, Sakari Ailus,
	Greg Kroah-Hartman, Rafael J. Wysocki, Danilo Krummrich,
	Mika Westerberg, Andy Shevchenko, Linus Walleij, Hans de Goede,
	Ilpo Järvinen, Dmitry Torokhov, Len Brown, linux-acpi,
	driver-core, linux-kernel, linux-gpio, platform-driver-x86, brgl

On Mon, Apr 27, 2026 at 2:19 PM Bartosz Golaszewski
<bartosz.golaszewski@oss.qualcomm.com> wrote:
>
> In order to allow GPIOLIB to match cherryview and baytrail GPIO
> controllers by their firmware nodes instead of their names, we need to
> attach the - currently "dangling" - existing software nodes to their
> target devices dynamically.
>
> We deal with devices described in ACPI so set up a bus notifier waiting
> for the ADD events. We know the name of the device we're waiting for so
> match against it and - on match - assign the appropriate software node
> as the secondary firmware node of the underlying ACPI node. In case the
> event was emitted earlier than this driver's probe: also make sure the
> device was not added before.
>
> Scheduling fine-grained devres actions allows for proper teardown and
> unsetting of the secondary firmware nodes.
>
> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
> ---
>  drivers/platform/x86/x86-android-tablets/core.c | 127 +++++++++++++++++++++++-
>  1 file changed, 124 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/platform/x86/x86-android-tablets/core.c b/drivers/platform/x86/x86-android-tablets/core.c
> index 021009e9085bec3db9c4daa1f6235600210a6099..9e6e8f272dfe16cda421b569802045c3d94fc0ab 100644
> --- a/drivers/platform/x86/x86-android-tablets/core.c
> +++ b/drivers/platform/x86/x86-android-tablets/core.c
> @@ -13,10 +13,12 @@
>  #include <linux/acpi.h>
>  #include <linux/device.h>
>  #include <linux/dmi.h>
> +#include <linux/fwnode.h>
>  #include <linux/gpio/consumer.h>
>  #include <linux/gpio/machine.h>
>  #include <linux/irq.h>
>  #include <linux/module.h>
> +#include <linux/notifier.h>
>  #include <linux/pci.h>
>  #include <linux/platform_device.h>
>  #include <linux/serdev.h>
> @@ -360,6 +362,124 @@ static const struct software_node *cherryview_gpiochip_node_group[] = {
>         NULL
>  };
>
> +struct auto_secondary_data {
> +       struct notifier_block nb;
> +       struct device *parent;
> +};
> +
> +static void auto_secondary_unset(void *data)
> +{
> +       struct fwnode_handle *fwnode = data;
> +
> +       fwnode->secondary = NULL;
> +}
> +
> +static int acpi_set_secondary_fwnode(struct device *parent, struct device *dev,
> +                                    const struct software_node *const swnode)
> +{
> +       struct acpi_device *device = to_acpi_device(dev);
> +       struct fwnode_handle *fwnode;
> +       int ret;
> +
> +       fwnode = software_node_fwnode(swnode);
> +       if (WARN_ON(!fwnode))
> +               return -ENOENT;
> +
> +       fwnode->secondary = ERR_PTR(-ENODEV);
> +       device->fwnode.secondary = fwnode;
> +
> +       ret = devm_add_action_or_reset(parent, auto_secondary_unset, &device->fwnode);
> +       if (ret)
> +               dev_err(parent, "Failed to schedule the unset action for secondary fwnode\n");
> +
> +       return ret;
> +}
> +
> +static int acpi_auto_secondary_notifier(struct notifier_block *nb,
> +                                       unsigned long action, void *data)
> +{
> +       struct auto_secondary_data *auto_sec = container_of(nb, struct auto_secondary_data, nb);
> +       const struct software_node *const *swnode;
> +       struct device *dev = data;
> +       int ret;
> +
> +       switch (action) {
> +       case BUS_NOTIFY_ADD_DEVICE:
> +               for (swnode = gpiochip_node_group; *swnode; swnode++) {
> +                       if (strcmp((*swnode)->name, dev_name(dev)) == 0) {
> +                               ret = acpi_set_secondary_fwnode(auto_sec->parent, dev, *swnode);
> +                               return ret ? NOTIFY_BAD : NOTIFY_OK;
> +                       }
> +               }
> +               break;
> +       default:
> +               break;
> +       }
> +
> +       return NOTIFY_DONE;
> +}
> +
> +static void auto_secondary_unregister_node_group(void *data)
> +{
> +       const struct software_node **nodes = data;
> +
> +       software_node_unregister_node_group(nodes);
> +}
> +
> +static void auto_secondary_unregister_notifier(void *data)
> +{
> +       struct notifier_block *nb = data;
> +
> +       bus_unregister_notifier(&acpi_bus_type, nb);

Please introduce acpi_bus_unregister_notifier() and use it here instead.

> +}
> +
> +static int auto_secondary_fwnode_init(struct device *parent)
> +{
> +       const struct software_node *const *swnode;
> +       struct auto_secondary_data *data;
> +       int ret;
> +
> +       ret = software_node_register_node_group(gpiochip_node_group);
> +       if (ret)
> +               return ret;
> +
> +       ret = devm_add_action_or_reset(parent,
> +                                      auto_secondary_unregister_node_group,
> +                                      gpiochip_node_group);
> +       if (ret)
> +               return ret;
> +
> +       data = devm_kzalloc(parent, sizeof(*data), GFP_KERNEL);
> +       if (!data)
> +               return -ENOMEM;
> +
> +       data->nb.notifier_call = acpi_auto_secondary_notifier;
> +       data->parent = parent;
> +
> +       ret = bus_register_notifier(&acpi_bus_type, &data->nb);

Please introduce acpi_bus_register_notifier() and use it here instead.

> +       if (ret)
> +               return ret;
> +
> +       ret = devm_add_action_or_reset(parent,
> +                                      auto_secondary_unregister_notifier,
> +                                      &data->nb);
> +       if (ret)
> +               return ret;
> +
> +       /* Device may have been already added. */
> +       for (swnode = gpiochip_node_group; *swnode; swnode++) {
> +               struct device *dev __free(put_device) =
> +                       bus_find_device_by_name(&acpi_bus_type, NULL, (*swnode)->name);

Please introduce acpi_bus_find_device_by_name() and use it here instead.

> +               if (dev) {
> +                       ret = acpi_set_secondary_fwnode(parent, dev, *swnode);
> +                       if (ret)
> +                               return ret;
> +               }
> +       }
> +
> +       return 0;
> +}
> +
>  static void x86_android_tablet_remove(struct platform_device *pdev)
>  {
>         int i;
> @@ -391,7 +511,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)
> @@ -427,9 +546,11 @@ static __init int x86_android_tablet_probe(struct platform_device *pdev)
>                 break;
>         }
>
> -       ret = software_node_register_node_group(gpiochip_node_group);
> -       if (ret)
> +       ret = auto_secondary_fwnode_init(&pdev->dev);
> +       if (ret) {
> +               x86_android_tablet_remove(pdev);
>                 return ret;
> +       }
>
>         ret = software_node_register_node_group(dev_info->swnode_group);
>         if (ret) {
>
> --
> 2.47.3
>
>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v3 2/2] platform/x86: x86-android-tablets: enable fwnode matching of GPIO chips
  2026-04-27 12:19 ` [PATCH v3 2/2] platform/x86: x86-android-tablets: enable fwnode matching of GPIO chips Bartosz Golaszewski
  2026-04-27 19:32   ` Rafael J. Wysocki
@ 2026-04-28  9:16   ` Hans de Goede
  2026-04-28  9:19     ` Bartosz Golaszewski
  1 sibling, 1 reply; 7+ messages in thread
From: Hans de Goede @ 2026-04-28  9:16 UTC (permalink / raw)
  To: Bartosz Golaszewski, Andy Shevchenko, Daniel Scally,
	Heikki Krogerus, Sakari Ailus, Greg Kroah-Hartman,
	Rafael J. Wysocki, Danilo Krummrich, Mika Westerberg,
	Andy Shevchenko, Linus Walleij, Ilpo Järvinen,
	Dmitry Torokhov, Len Brown
  Cc: linux-acpi, driver-core, linux-kernel, linux-gpio,
	platform-driver-x86, brgl

Hi Bartosz,

On 27-Apr-26 14:19, Bartosz Golaszewski wrote:
> In order to allow GPIOLIB to match cherryview and baytrail GPIO
> controllers by their firmware nodes instead of their names, we need to
> attach the - currently "dangling" - existing software nodes to their
> target devices dynamically.
> 
> We deal with devices described in ACPI so set up a bus notifier waiting
> for the ADD events. We know the name of the device we're waiting for so
> match against it and - on match - assign the appropriate software node
> as the secondary firmware node of the underlying ACPI node. In case the
> event was emitted earlier than this driver's probe: also make sure the
> device was not added before.
> 
> Scheduling fine-grained devres actions allows for proper teardown and
> unsetting of the secondary firmware nodes.

Thank you for your work on this.

The x86-android-tablets.ko kernel module uses platform_create_bundle()
so its probe() cannot return -EPROBE_DEFER. IOW it expects all the GPIO
pins which it needs to already be there when it loads (which so far in
practice holds, since these x86 GPIO controllers are always builtin
for various reasons).

This means that there is no need all the notifier stuff. Only adding
an acpi_bus_find_device_by_name() helper as suggested by Rafael and
then finding the GPIO controllers and attaching the swnodes is
necessary.

And if the acpi_bus_find_device_by_name() fails it is ok to fail
the probe() just like it currently fails when gpiod_get() returns
-EPROBE_DEFER (or fails for other reasons).

This should nicely simplify this patch.

Regards,

Hans





> 
> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
> ---
>  drivers/platform/x86/x86-android-tablets/core.c | 127 +++++++++++++++++++++++-
>  1 file changed, 124 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/platform/x86/x86-android-tablets/core.c b/drivers/platform/x86/x86-android-tablets/core.c
> index 021009e9085bec3db9c4daa1f6235600210a6099..9e6e8f272dfe16cda421b569802045c3d94fc0ab 100644
> --- a/drivers/platform/x86/x86-android-tablets/core.c
> +++ b/drivers/platform/x86/x86-android-tablets/core.c
> @@ -13,10 +13,12 @@
>  #include <linux/acpi.h>
>  #include <linux/device.h>
>  #include <linux/dmi.h>
> +#include <linux/fwnode.h>
>  #include <linux/gpio/consumer.h>
>  #include <linux/gpio/machine.h>
>  #include <linux/irq.h>
>  #include <linux/module.h>
> +#include <linux/notifier.h>
>  #include <linux/pci.h>
>  #include <linux/platform_device.h>
>  #include <linux/serdev.h>
> @@ -360,6 +362,124 @@ static const struct software_node *cherryview_gpiochip_node_group[] = {
>  	NULL
>  };
>  
> +struct auto_secondary_data {
> +	struct notifier_block nb;
> +	struct device *parent;
> +};
> +
> +static void auto_secondary_unset(void *data)
> +{
> +	struct fwnode_handle *fwnode = data;
> +
> +	fwnode->secondary = NULL;
> +}
> +
> +static int acpi_set_secondary_fwnode(struct device *parent, struct device *dev,
> +				     const struct software_node *const swnode)
> +{
> +	struct acpi_device *device = to_acpi_device(dev);
> +	struct fwnode_handle *fwnode;
> +	int ret;
> +
> +	fwnode = software_node_fwnode(swnode);
> +	if (WARN_ON(!fwnode))
> +		return -ENOENT;
> +
> +	fwnode->secondary = ERR_PTR(-ENODEV);
> +	device->fwnode.secondary = fwnode;
> +
> +	ret = devm_add_action_or_reset(parent, auto_secondary_unset, &device->fwnode);
> +	if (ret)
> +		dev_err(parent, "Failed to schedule the unset action for secondary fwnode\n");
> +
> +	return ret;
> +}
> +
> +static int acpi_auto_secondary_notifier(struct notifier_block *nb,
> +					unsigned long action, void *data)
> +{
> +	struct auto_secondary_data *auto_sec = container_of(nb, struct auto_secondary_data, nb);
> +	const struct software_node *const *swnode;
> +	struct device *dev = data;
> +	int ret;
> +
> +	switch (action) {
> +	case BUS_NOTIFY_ADD_DEVICE:
> +		for (swnode = gpiochip_node_group; *swnode; swnode++) {
> +			if (strcmp((*swnode)->name, dev_name(dev)) == 0) {
> +				ret = acpi_set_secondary_fwnode(auto_sec->parent, dev, *swnode);
> +				return ret ? NOTIFY_BAD : NOTIFY_OK;
> +			}
> +		}
> +		break;
> +	default:
> +		break;
> +	}
> +
> +	return NOTIFY_DONE;
> +}
> +
> +static void auto_secondary_unregister_node_group(void *data)
> +{
> +	const struct software_node **nodes = data;
> +
> +	software_node_unregister_node_group(nodes);
> +}
> +
> +static void auto_secondary_unregister_notifier(void *data)
> +{
> +	struct notifier_block *nb = data;
> +
> +	bus_unregister_notifier(&acpi_bus_type, nb);
> +}
> +
> +static int auto_secondary_fwnode_init(struct device *parent)
> +{
> +	const struct software_node *const *swnode;
> +	struct auto_secondary_data *data;
> +	int ret;
> +
> +	ret = software_node_register_node_group(gpiochip_node_group);
> +	if (ret)
> +		return ret;
> +
> +	ret = devm_add_action_or_reset(parent,
> +				       auto_secondary_unregister_node_group,
> +				       gpiochip_node_group);
> +	if (ret)
> +		return ret;
> +
> +	data = devm_kzalloc(parent, sizeof(*data), GFP_KERNEL);
> +	if (!data)
> +		return -ENOMEM;
> +
> +	data->nb.notifier_call = acpi_auto_secondary_notifier;
> +	data->parent = parent;
> +
> +	ret = bus_register_notifier(&acpi_bus_type, &data->nb);
> +	if (ret)
> +		return ret;
> +
> +	ret = devm_add_action_or_reset(parent,
> +				       auto_secondary_unregister_notifier,
> +				       &data->nb);
> +	if (ret)
> +		return ret;
> +
> +	/* Device may have been already added. */
> +	for (swnode = gpiochip_node_group; *swnode; swnode++) {
> +		struct device *dev __free(put_device) =
> +			bus_find_device_by_name(&acpi_bus_type, NULL, (*swnode)->name);
> +		if (dev) {
> +			ret = acpi_set_secondary_fwnode(parent, dev, *swnode);
> +			if (ret)
> +				return ret;
> +		}
> +	}
> +
> +	return 0;
> +}
> +
>  static void x86_android_tablet_remove(struct platform_device *pdev)
>  {
>  	int i;
> @@ -391,7 +511,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)
> @@ -427,9 +546,11 @@ static __init int x86_android_tablet_probe(struct platform_device *pdev)
>  		break;
>  	}
>  
> -	ret = software_node_register_node_group(gpiochip_node_group);
> -	if (ret)
> +	ret = auto_secondary_fwnode_init(&pdev->dev);
> +	if (ret) {
> +		x86_android_tablet_remove(pdev);
>  		return ret;
> +	}
>  
>  	ret = software_node_register_node_group(dev_info->swnode_group);
>  	if (ret) {
> 


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v3 2/2] platform/x86: x86-android-tablets: enable fwnode matching of GPIO chips
  2026-04-28  9:16   ` Hans de Goede
@ 2026-04-28  9:19     ` Bartosz Golaszewski
  0 siblings, 0 replies; 7+ messages in thread
From: Bartosz Golaszewski @ 2026-04-28  9:19 UTC (permalink / raw)
  To: Hans de Goede
  Cc: Bartosz Golaszewski, Andy Shevchenko, Daniel Scally,
	Heikki Krogerus, Sakari Ailus, Greg Kroah-Hartman,
	Rafael J. Wysocki, Danilo Krummrich, Mika Westerberg,
	Andy Shevchenko, Linus Walleij, Ilpo Järvinen,
	Dmitry Torokhov, Len Brown, linux-acpi, driver-core,
	linux-kernel, linux-gpio, platform-driver-x86

On Tue, Apr 28, 2026 at 11:16 AM Hans de Goede <hansg@kernel.org> wrote:
>
> Hi Bartosz,
>
> On 27-Apr-26 14:19, Bartosz Golaszewski wrote:
> > In order to allow GPIOLIB to match cherryview and baytrail GPIO
> > controllers by their firmware nodes instead of their names, we need to
> > attach the - currently "dangling" - existing software nodes to their
> > target devices dynamically.
> >
> > We deal with devices described in ACPI so set up a bus notifier waiting
> > for the ADD events. We know the name of the device we're waiting for so
> > match against it and - on match - assign the appropriate software node
> > as the secondary firmware node of the underlying ACPI node. In case the
> > event was emitted earlier than this driver's probe: also make sure the
> > device was not added before.
> >
> > Scheduling fine-grained devres actions allows for proper teardown and
> > unsetting of the secondary firmware nodes.
>
> Thank you for your work on this.
>
> The x86-android-tablets.ko kernel module uses platform_create_bundle()
> so its probe() cannot return -EPROBE_DEFER. IOW it expects all the GPIO
> pins which it needs to already be there when it loads (which so far in
> practice holds, since these x86 GPIO controllers are always builtin
> for various reasons).
>
> This means that there is no need all the notifier stuff. Only adding
> an acpi_bus_find_device_by_name() helper as suggested by Rafael and
> then finding the GPIO controllers and attaching the swnodes is
> necessary.
>
> And if the acpi_bus_find_device_by_name() fails it is ok to fail
> the probe() just like it currently fails when gpiod_get() returns
> -EPROBE_DEFER (or fails for other reasons).
>
> This should nicely simplify this patch.
>

Thanks Hans, I was not aware of this. Given that this is the only
driver needing this quirk, it will indeed make things much simpler.
I'll have a v4 by the end of this week.

Bart

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2026-04-28  9:19 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-04-27 12:19 [PATCH v3 0/2] platform/x86: x86-android-tablets: use real firmware node references with intel drivers Bartosz Golaszewski
2026-04-27 12:19 ` [PATCH v3 1/2] ACPI: bus: export the acpi_bus_type symbol Bartosz Golaszewski
2026-04-27 19:27   ` Rafael J. Wysocki
2026-04-27 12:19 ` [PATCH v3 2/2] platform/x86: x86-android-tablets: enable fwnode matching of GPIO chips Bartosz Golaszewski
2026-04-27 19:32   ` Rafael J. Wysocki
2026-04-28  9:16   ` Hans de Goede
2026-04-28  9:19     ` Bartosz Golaszewski

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®