Re: [PATCH v5 0/1] i2c: designware: fix probe ordering for AMD GPIO on Lenovo Yoga 7 14AGP11

[Mario Limonciello <mario.limonciello@amd.com>]

[-- Attachment #1: Type: text/plain, Size: 4404 bytes --]



On 5/19/26 14:49, Hardik Prakash wrote:
> On Tue, May 19, 2026 at 20:18, Mario Limonciello wrote:
>> I like this idea. I guess something like this:
>> [gpiolib-acpi-core.c patch]
> 
> Tested patch 1 + gpiolib-acpi deferral, without patch 2. Arbitration
> errors persist:
> 
>    patch 1 + gpiolib-acpi deferral:   arbitration errors, WACF2200 does not probe
>    patch 1 + patch 2 (v5):           clean boot, touchscreen fully functional
> 
> I think the reason is that i2c-designware does not call acpi_get_gpiod()
> during its probe. The GpioInt resource is on the WACF2200 touchscreen
> device (TPNL), not on the I2C controller itself. So the deferral in
> acpi_get_gpiod() never triggers for AMDI0010:02 -- nothing in that probe
> path requests a GPIO descriptor.
> 
> The race is between amd_gpio_probe() completing and dw_i2c_plat_probe()
> starting for AMDI0010:02. Without something that explicitly checks
> whether the GPIO controller is fully bound before the I2C controller
> probes, the race remains.
> 

In the same linke of thinking - how about something like this instead 
(AI generated and attached).

Basically walk through the resources at probe time and make sure they're 
all bound.

> Thanks,
> Hardik
> 
> On Wed, 20 May 2026 at 00:37, Hardik Prakash
> <hardikprakash.official@gmail.com> wrote:
>>
>> On Tue, May 19, 2026 at 19:58, Mario Limonciello wrote:
>>> You add a debug statement to amd_gpio_irq_enable() too right? Can you
>>> please share your debugging messages patch and full log?
>>
>> I did not add debug to amd_gpio_irq_enable() - the statements were
>> only in amd_gpio_probe() and dw_i2c_plat_probe(). I can add one there
>> if useful, but given Bart's suggestion below I'll hold off unless
>> needed.
>>
>>> What is the boot time impact to adding device_is_bound() check?
>>
>> I haven't measured this explicitly. The deferral only fires on DMI-
>> matched hardware (Lenovo 83TD), so on other machines dw_i2c_defer_for_
>> amd_gpio() returns 0 immediately with no overhead.
>>
>> On Tue, May 19, 2026 at 20:18, Mario Limonciello wrote:
>>> I like this idea.
>>
>> I'll test this patch, and let you know how it goes.
>>
>> Thanks,
>> Hardik
>>
>> On Tue, 19 May 2026 at 20:18, Mario Limonciello
>> <mario.limonciello@amd.com> wrote:
>>>
>>>
>>>
>>> On 5/19/26 09:39, Bartosz Golaszewski wrote:
>>>> On Tue, May 19, 2026 at 4:28 PM Mario Limonciello
>>>> <mario.limonciello@amd.com> wrote:
>>>>>
>>>>>>
>>>>>> gpiochip_add_data() at 0.285952 makes the GPIO chip visible to the
>>>>>> system before amd_gpio_probe() has finished. AMDI0010:02 starts probing
>>>>>> at 0.301454 while amd_gpio_probe() is still completing. This is why
>>>>>> device_is_bound() works and initcall promotion does not -- it waits for
>>>>>> probe completion, not just gpiochip registration.
>>>>>
>>>>> What is the boot time impact to adding device_is_bound() check?
>>>>>
>>>>> Bartosz, thoughts?
>>>>>
>>>>
>>>> My thoughts are that ACPI could use some fw_devlink. :) It's not a new
>>>> problem, we've fixed it for OF systems.
>>>>
>>>> Could we modify gpiolib-acpi.c to return -EPROBE_DEFER if the parent
>>>> device of the GPIO chip is not bound yet instead? When resolving the
>>>> reference to the remote GPIO controller we have an address of the
>>>> struct acpi_device. I suppose there's a platform device that is its
>>>> child and then the logical GPIO controller device, is that correct?
>>>> Can we check if the platform device in question is bound at the
>>>> subsystem level?
>>>>
>>>> Bart
>>>
>>> I like this idea.  I guess something like this:
>>>
>>> diff --git a/drivers/gpio/gpiolib-acpi-core.c
>>> b/drivers/gpio/gpiolib-acpi-core.c
>>> index eb8a40cfb7a98..e3511398b1f84 100644
>>> --- a/drivers/gpio/gpiolib-acpi-core.c
>>> +++ b/drivers/gpio/gpiolib-acpi-core.c
>>> @@ -142,6 +142,13 @@ static struct gpio_desc *acpi_get_gpiod(char *path,
>>> unsigned int pin)
>>>           if (!gdev)
>>>                   return ERR_PTR(-EPROBE_DEFER);
>>>
>>> +       if (gdev->dev.parent) {
>>> +               scoped_guard(device, gdev->dev.parent) {
>>> +                       if (!device_is_bound(gdev->dev.parent))
>>> +                               return ERR_PTR(-EPROBE_DEFER);
>>> +               }
>>> +       }
>>> +
>>>           /*
>>>            * FIXME: keep track of the reference to the GPIO device somehow
>>>            * instead of putting it here.
>>>

[-- Attachment #2: 0001-i2c-designware-Defer-probe-if-child-GpioInt-controll.patch --]
[-- Type: text/x-patch, Size: 5992 bytes --]

From 87778c8ed29f1cdab505d6a74505213e1e8bef21 Mon Sep 17 00:00:00 2001
From: Mario Limonciello <mario.limonciello@amd.com>
Date: Tue, 19 May 2026 15:00:31 -0500
Subject: [TEST PATCH] i2c: designware: Defer probe if child GpioInt controllers are
 not bound

I2C controllers may have child devices with GpioInt resources that
depend on GPIO controllers to be fully initialized. If the I2C
controller probes and enumerates children before the referenced GPIO
controller has completed probe, GPIO interrupts may not be properly
configured, leading to device failures.

On Lenovo Yoga 7 14AGP11, the WACF2200 touchscreen (child of
AMDI0010:02) has a GpioInt resource pointing to GPIO 157 on the
pinctrl-amd controller (AMDI0030:00). When i2c-designware probes
AMDI0010:02 before pinctrl-amd finishes initializing, I2C transactions
occur before the GPIO IRQ quirk in amd_gpio_probe() has run, causing:

  i2c_designware AMDI0010:02: i2c_dw_handle_tx_abort: lost arbitration

Add a generic dependency check in i2c-designware that walks ACPI child
devices, identifies any GpioInt resources, resolves the referenced GPIO
controllers, and defers probe if those controllers are not yet bound.

This ensures GPIO controllers complete initialization (including IRQ
setup and quirks) before I2C child enumeration begins, fixing the race
without device-specific quirks or DMI matching.

Link: https://bugzilla.kernel.org/show_bug.cgi?id=221494
Not-Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
Assisted-by: Claude:claude-sonnet-4-6
---
 drivers/i2c/busses/i2c-designware-platdrv.c | 153 ++++++++++++++++++++
 1 file changed, 153 insertions(+)

diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c
index 3351c4a9ef118..856bf679406b9 100644
--- a/drivers/i2c/busses/i2c-designware-platdrv.c
+++ b/drivers/i2c/busses/i2c-designware-platdrv.c
@@ -8,6 +8,7 @@
  * Copyright (C) 2007 MontaVista Software Inc.
  * Copyright (C) 2009 Provigent Ltd.
  */
+#include <linux/acpi.h>
 #include <linux/clk-provider.h>
 #include <linux/clk.h>
 #include <linux/delay.h>
@@ -130,6 +131,150 @@ static int i2c_dw_probe_lock_support(struct dw_i2c_dev *dev)
 	return 0;
 }
 
+#ifdef CONFIG_ACPI
+struct gpio_dep_ctx {
+	struct list_head gpio_controllers;
+	int ret;
+};
+
+struct gpio_controller_ref {
+	struct list_head node;
+	char *path;
+};
+
+static int check_gpioint_resource(struct acpi_resource *ares, void *data)
+{
+	struct gpio_dep_ctx *ctx = data;
+	struct acpi_resource_gpio *agpio;
+	struct gpio_controller_ref *ref, *tmp;
+	bool found = false;
+
+	if (ares->type != ACPI_RESOURCE_TYPE_GPIO)
+		return 1;
+
+	agpio = &ares->data.gpio;
+	if (agpio->connection_type != ACPI_RESOURCE_GPIO_TYPE_INT)
+		return 1;
+
+	/* Check if we've already tracked this GPIO controller */
+	list_for_each_entry(tmp, &ctx->gpio_controllers, node) {
+		if (!strcmp(tmp->path, agpio->resource_source.string_ptr)) {
+			found = true;
+			break;
+		}
+	}
+
+	if (!found) {
+		ref = kzalloc_obj(*ref, GFP_KERNEL);
+		if (!ref) {
+			ctx->ret = -ENOMEM;
+			return 0;
+		}
+
+		ref->path = kstrdup(agpio->resource_source.string_ptr, GFP_KERNEL);
+		if (!ref->path) {
+			kfree(ref);
+			ctx->ret = -ENOMEM;
+			return 0;
+		}
+
+		list_add_tail(&ref->node, &ctx->gpio_controllers);
+	}
+
+	return 1;
+}
+
+static int check_child_gpioint(struct acpi_device *adev, void *data)
+{
+	struct gpio_dep_ctx *ctx = data;
+	struct list_head res_list;
+	int ret;
+
+	INIT_LIST_HEAD(&res_list);
+
+	ret = acpi_dev_get_resources(adev, &res_list, check_gpioint_resource, ctx);
+	acpi_dev_free_resource_list(&res_list);
+
+	if (ctx->ret < 0)
+		return ctx->ret;
+
+	return 0;
+}
+
+static int i2c_dw_check_gpio_dependencies(struct device *dev)
+{
+	struct acpi_device *adev = ACPI_COMPANION(dev);
+	struct gpio_dep_ctx ctx = { .ret = 0 };
+	struct gpio_controller_ref *ref, *tmp;
+	int ret = 0;
+
+	if (!adev)
+		return 0;
+
+	INIT_LIST_HEAD(&ctx.gpio_controllers);
+
+	/* Walk all child devices and collect GpioInt controller references */
+	ret = acpi_dev_for_each_child(adev, check_child_gpioint, &ctx);
+	if (ret < 0 || ctx.ret < 0) {
+		ret = ctx.ret ?: ret;
+		goto cleanup;
+	}
+
+	/* For each GPIO controller, check if its parent device is bound */
+	list_for_each_entry(ref, &ctx.gpio_controllers, node) {
+		acpi_handle handle;
+		acpi_status status;
+		struct acpi_device *gpio_adev;
+		struct device *gpio_dev;
+
+		status = acpi_get_handle(NULL, ref->path, &handle);
+		if (ACPI_FAILURE(status))
+			continue;
+
+		gpio_adev = acpi_fetch_acpi_dev(handle);
+		if (!gpio_adev)
+			continue;
+
+		gpio_dev = acpi_get_first_physical_node(gpio_adev);
+		acpi_dev_put(gpio_adev);
+
+		if (!gpio_dev) {
+			ret = -EPROBE_DEFER;
+			goto cleanup;
+		}
+
+		/*
+		 * Check if the GPIO controller's device is bound. If not,
+		 * defer probe to ensure GPIO initialization (including IRQ
+		 * setup and quirks) is complete before we enumerate I2C
+		 * child devices.
+		 */
+		scoped_guard(device, gpio_dev) {
+			if (!device_is_bound(gpio_dev)) {
+				ret = -EPROBE_DEFER;
+				goto cleanup;
+			}
+		}
+
+		put_device(gpio_dev);
+	}
+
+cleanup:
+	list_for_each_entry_safe(ref, tmp, &ctx.gpio_controllers, node) {
+		list_del(&ref->node);
+		kfree(ref->path);
+		kfree(ref);
+	}
+
+	return ret;
+}
+#else
+static int i2c_dw_check_gpio_dependencies(struct device *dev)
+{
+	return 0;
+}
+#endif /* CONFIG_ACPI */
+
 static int dw_i2c_plat_probe(struct platform_device *pdev)
 {
 	u32 flags = (uintptr_t)device_get_match_data(&pdev->dev);
@@ -138,6 +283,14 @@ static int dw_i2c_plat_probe(struct platform_device *pdev)
 	struct dw_i2c_dev *dev;
 	int irq, ret;
 
+	/*
+	 * Check if any child devices have GpioInt resources, and if so,
+	 * defer probe until those GPIO controllers are fully bound.
+	 */
+	ret = i2c_dw_check_gpio_dependencies(device);
+	if (ret)
+		return ret;
+
 	irq = platform_get_irq_optional(pdev, 0);
 	if (irq == -ENXIO)
 		flags |= ACCESS_POLLING;
-- 
2.43.0