i2c designware change broke touchpad of a thinkpad (was: i2c: designware: defer probe if child GpioInt controllers are not bound)
Thorsten Leemhuis <[email protected]> Fri, 7 Aug 2026 16:07:23 +0200
| Newsgroups | org.kernel.vger.linux-i2c,dev.linux.lists.regressions,org.kernel.vger.linux-gpio |
|---|---|
| Message-ID | <[email protected]> |
On 7/28/26 10:30, Andi Shyti wrote:
> On Sat, Jul 18, 2026 at 11:13:31AM +0530, Hardik Prakash wrote:
>> I2C controllers may have child devices with GpioInt resources that
>> depend on GPIO controllers being 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
>> fail with lost arbitration errors:
>>
>> 0.285952 amd_gpio_probe: registering gpiochip <- GPIO chip visible
>> 0.287121 amd_gpio_probe: requesting parent IRQ <- probe still running
>> 0.301454 AMDI0010:02 dw_i2c_plat_probe: start <- races here
>> 2.348157 lost arbitration
>>
>> Add a dependency check that walks ACPI child devices and defers probe
>> until any referenced GPIO controller is bound.
>>
>> Fixes: 3812a9e84265 ("pinctrl-amd: enable IRQ for WACF2200 touchscreen on Lenovo Yoga 7 14AGP11")
>> Suggested-by: Mario Limonciello <[email protected]>
>> Suggested-by: Andy Shevchenko <[email protected]>
> [...]
> merged to i2c/i2c-fixes.
>
> Thanks,
> Andi
This became 0a4bb2abc3e56d ("i2c: designware: defer probe if child
GpioInt controllers are not bound") and causes a regression for me: the
Touchpad in my ThinkPad T14s Gen 4 (AMD) stopped working. From the
kernel log:
i2c_designware AMDI0010:01: i2c_dw_handle_tx_abort: lost arbitration
i2c_designware AMDI0010:01: controller timed out
i2c_hid_acpi i2c-SYNA8018:00: failed to fetch HID descriptor: -110
i2c_hid_acpi i2c-SYNA8018:00: Failed to fetch the HID Descriptor
Reverting this change in current mainline/master (f9a2394a23482) fixed
things for me. Full dmesg of current master and master+revert can be
found here:
https://www.leemhuis.info/files/misc/dmesg-master
https://www.leemhuis.info/files/misc/dmesg-master-plus_revert
Ciao, Thorsten
#regzbot introduced: 0a4bb2abc3e56d
#regzbot title i2c: designware: ThinkPad touchpad broke
>> ---
>> v12 -> v13:
>> - v12 was sent as an incomplete diff (generated against v11 rather than
>> against current master due to a git format-patch invocation mistake on
>> my end), not a real standalone patch (Andy Shevchenko)
>> - Not carrying forward Bartosz Golaszewski's v12 Acked-by, per Andy's
>> request
>> - Add blank line before guard(device)(gpio_dev) for readability (Andy
>> Shevchenko)
>> - No other functional code changes since v12
>>
>> v11 -> v12:
>> - Move gdev declaration to point of use with gpio_device_find_by_fwnode()
>> call (Andy Shevchenko)
>> - Extract acpi_get_handle() return into acpi_status variable rather than
>> inline in ACPI_FAILURE() (Andy Shevchenko)
>> - Use guard(device)(...) instead of scoped_guard(device, ...) {} for a
>> single following statement (Andy Shevchenko)
>> - Use LIST_HEAD(res_list) instead of separate declaration + INIT_LIST_HEAD()
>> (Andy Shevchenko)
>> - Check acpi_dev_get_resources() error before freeing the resource list,
>> restoring the check-then-free ordering that had regressed in v11
>> (Andy Shevchenko)
>> - Split ACPI_COMPANION(dev) assignment onto its own line rather than at
>> declaration (Andy Shevchenko)
>>
>> v10 -> v11:
>> - Replaced custom gpio_controller_ref list and dedup logic with
>> gpio_device_find_by_fwnode() (Suggested-by: Andy Shevchenko)
>> - Moved resource-skip explanation from commit message into a code comment
>> - Fixed device_is_bound() to check gpio_device_to_device(gdev)->parent
>> rather than the gpio_device's own internal class device, which is
>> registered on the synthetic "gpio" bus and never has a driver bound to it
>>
>> v9 -> v10:
>> - Use acpi_gpio_get_irq_resource() instead of open-coding GPIO resource
>> type checks, eliminating duplication with gpiolib-acpi (Andy Shevchenko)
>> - Simplify string_source guard to a single string_length check, confirmed
>> sufficient via ACPICA parser in drivers/acpi/acpica/rsutils.c
>> (Andy Shevchenko)
>> - acpi_dev_get_resources() on single line, free only on success path
>> (Andy Shevchenko)
>> - Reversed xmas tree ordering for variable declarations (Andy Shevchenko)
>> - Removed useless int ret = 0 assignment (Andy Shevchenko)
>> - acpi_dev_for_each_child() on single line (Andy Shevchenko)
>> - Use guard(device)(gpio_dev) pattern to avoid uninitialized variable
>> warning and deduplicate the !gpio_dev check (Andy Shevchenko)
>>
>> v8 -> v9:
>> - Sent as a 2-patch series: revert of the broken v8 patch, plus a
>> corrected resubmission
>> - Use acpi_gpio_get_irq_resource() instead of open-coding GPIO resource
>> type checks, eliminating duplication with gpiolib-acpi (Andy Shevchenko)
>> - Remove gpio_dep_ctx wrapper struct, pass list_head * directly (Andy Shevchenko)
>> - Add const to gpio_controller_ref.path (Andy Shevchenko)
>> - Add NULL check for resource_source.string_ptr to fix a crash on hardware
>> where GPIO resources have no named controller (Reported-by: Nathan
>> Chancellor, Reported-by: Chaitanya Kumar Borah)
>> - Use acpi_dev_get_resources() return value properly (Andy Shevchenko)
>> - Fix all error paths to call free_gpio_controller_list() (Andy Shevchenko)
>> - Change guard to #if defined(CONFIG_ACPI) && defined(CONFIG_GPIOLIB)
>> - Remove erroneous put_device() calls -- acpi_get_first_physical_node()
>> returns a borrowed pointer with no refcount increment
>> - Use LIST_HEAD() macro, split adev declaration and assignment (Andy Shevchenko)
>> - scoped_guard single statement without braces (Andy Shevchenko)
>> - Remove misused Reported-by/Closes tags (Andy Shevchenko)
>>
>> v7 -> v8:
>> - Add revert of the earlier pinctrl-amd DMI quirk (patch 1 of the original
>> v1-v7 series), confirmed unnecessary once probe ordering is fixed
>> (Mario Limonciello)
>> - Collect Acked-by (Bartosz Golaszewski) and Reviewed-by (Mario Limonciello)
>>
>> v6 -> v7:
>> - Fix unused 'ret' variable in check_child_gpioint() (kernel test robot)
>> - Replace BugLink: with Link: (Bartosz Golaszewski)
>> - Add Reported-by and Closes tags for kernel test robot warning
>>
>> v5 -> v6:
>> - Replace DMI-specific deferral with a generic GpioInt dependency check
>> walking ACPI child devices (Suggested-by: Mario Limonciello)
>>
>> v4 -> v5:
>> - Add blank line before #include <linux/acpi.h> (Bartosz Golaszewski)
>> - Use scoped_guard(device, gpio_dev) (Bartosz Golaszewski)
>>
>> v3 -> v4:
>> - Rebase onto Linus Walleij's tree (the original patch 1 already there)
>> - Use --base so series is correctly 1/1 (Andy Shevchenko)
>>
>> v2 -> v3:
>> - Fix variable declaration style (Andy Shevchenko)
>> - Add BugLink tag (Andy Shevchenko)
>> - CC AMD engineers (Andy Shevchenko)
>>
>> v1 -> v2:
>> - Replace custom HID/UID lookup with acpi_dev_get_first_match_dev()
>> - Use device_is_bound() under device_lock() with explanatory comments
>> - Add Assisted-by tags per coding-assistants.rst
>>
>> Note: this patch was originally part of a 2-patch series (v1-v7) whose
>> first patch was a DMI-specific pinctrl-amd IRQ quirk; that patch was
>> merged separately into Linus Walleij's tree at v1 and later reverted at
>> v8 once this patch made the manual IRQ restoration unnecessary.
>>
>> drivers/i2c/busses/i2c-designware-platdrv.c | 80 +++++++++++++++++++++
>> 1 file changed, 80 insertions(+)
>>
>> diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c
>> index 6d6e81242f74..c8a203fff4d1 100644
>> --- a/drivers/i2c/busses/i2c-designware-platdrv.c
>> +++ b/drivers/i2c/busses/i2c-designware-platdrv.c
>> @@ -8,12 +8,14 @@
>> * 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>
>> #include <linux/dmi.h>
>> #include <linux/err.h>
>> #include <linux/errno.h>
>> +#include <linux/gpio/driver.h>
>> #include <linux/i2c.h>
>> #include <linux/interrupt.h>
>> #include <linux/io.h>
>> @@ -130,6 +132,80 @@ static int i2c_dw_probe_lock_support(struct dw_i2c_dev *dev)
>> return 0;
>> }
>>
>> +#if defined(CONFIG_ACPI) && defined(CONFIG_GPIOLIB)
>> +/*
>> + * Check whether an ACPI GpioInt resource's referenced GPIO controller
>> + * has finished probing. Resources with no named controller (resource
>> + * source string) are skipped, since they can't be resolved to a
>> + * struct device.
>> + */
>> +static int check_gpioint_resource(struct acpi_resource *ares, void *data)
>> +{
>> + struct acpi_resource_gpio *agpio;
>> + struct acpi_device *gpio_adev;
>> + struct device *gpio_dev;
>> + acpi_handle handle;
>> + acpi_status status;
>> +
>> + if (!acpi_gpio_get_irq_resource(ares, &agpio))
>> + return 1; /* not a GpioInt resource, skip */
>> +
>> + if (!agpio->resource_source.string_length)
>> + return 1; /* no named controller, skip */
>> +
>> + status = acpi_get_handle(NULL, agpio->resource_source.string_ptr, &handle);
>> + if (ACPI_FAILURE(status))
>> + return 1;
>> +
>> + gpio_adev = acpi_fetch_acpi_dev(handle);
>> + if (!gpio_adev)
>> + return 1;
>> +
>> + struct gpio_device *gdev __free(gpio_device_put) =
>> + gpio_device_find_by_fwnode(acpi_fwnode_handle(gpio_adev));
>> + if (!gdev)
>> + return -EPROBE_DEFER; /* controller not registered yet: abort walk */
>> +
>> + gpio_dev = gpio_device_to_device(gdev)->parent;
>> +
>> + guard(device)(gpio_dev);
>> + if (!device_is_bound(gpio_dev))
>> + return -EPROBE_DEFER; /* controller not bound yet: abort walk */
>> +
>> + return 1; /* bound, skip adding to resource list, continue walk */
>> +}
>> +
>> +static int check_child_gpioint(struct acpi_device *adev, void *data)
>> +{
>> + LIST_HEAD(res_list);
>> + int ret;
>> +
>> + ret = acpi_dev_get_resources(adev, &res_list, check_gpioint_resource, NULL);
>> + if (ret < 0)
>> + return ret;
>> +
>> + acpi_dev_free_resource_list(&res_list);
>> +
>> + return 0;
>> +}
>> +
>> +static int i2c_dw_check_gpio_dependencies(struct device *dev)
>> +{
>> + struct acpi_device *adev;
>> +
>> + adev = ACPI_COMPANION(dev);
>> + if (!adev)
>> + return 0;
>> +
>> + return acpi_dev_for_each_child(adev, check_child_gpioint, NULL);
>> +}
>> +#else
>> +static int i2c_dw_check_gpio_dependencies(struct device *dev)
>> +{
>> + return 0;
>> +}
>> +#endif /* CONFIG_ACPI && CONFIG_GPIOLIB */
>> +
>> static int dw_i2c_plat_probe(struct platform_device *pdev)
>> {
>> u32 flags = (uintptr_t)device_get_match_data(&pdev->dev);
>> @@ -138,6 +214,10 @@ static int dw_i2c_plat_probe(struct platform_device *pdev)
>> struct dw_i2c_dev *dev;
>> int irq, ret;
>>
>> + ret = i2c_dw_check_gpio_dependencies(device);
>> + if (ret)
>> + return ret;
>> +
>> irq = platform_get_irq_optional(pdev, 0);
>> if (irq == -ENXIO)
>> flags |= ACCESS_POLLING;
>>
>> base-commit: 58717b2a1365d06c8c64b72aa948541b53fe31eb
>> --
>> 2.54.0
>>
>