mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] Revert "i2c: designware: defer probe if child GpioInt controllers are not bound"
@ 2026-08-14 10:07 Hardik Prakash
  2026-08-14 10:49 ` Thorsten Leemhuis
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Hardik Prakash @ 2026-08-14 10:07 UTC (permalink / raw)
  To: linux-i2c
  Cc: linux-gpio, wsa, andriy.shevchenko, mario.limonciello, brgl,
	basavaraj.natikar, linusw, nathan, chaitanya.kumar.borah,
	andi.shyti, linux, regressions, linux-kernel, Hardik Prakash

This reverts commit 0a4bb2abc3e56d7be6e69b050c88ba52c87e22bf.

The reverted commit causes a regression on ThinkPad T14s Gen 4 (AMD):
the touchpad's I2C controller fails with lost arbitration errors,
because it delays i2c-designware's probe by roughly 500ms, which
shifts the touchpad's first HID descriptor fetch into a window where
the platform's embedded controller is still acting as a secondary I2C
bus master. Debug tracing confirms the GpioInt dependency check itself
behaves correctly (it defers appropriately and confirms the GPIO
controller is bound); the arbitration failure happens roughly a
second after the check passes, when i2c_hid_acpi's own probe attempts
its first transaction.

The original fix is still needed for the Lenovo Yoga 7 14AGP11
touchscreen race the commit addressed, but a corrected version will
be resubmitted once the EC bus-mastering interaction is understood
and handled properly, rather than reintroducing a different
regression on more widely-used ThinkPad hardware in the meantime.

Reported-by: Thorsten Leemhuis <linux@leemhuis.info>
Closes: https://lore.kernel.org/all/b4a4eadb-282f-464c-843a-19d415a34d0c@leemhuis.info/
Signed-off-by: Hardik Prakash <hardikprakash.official@gmail.com>
---
 drivers/i2c/busses/i2c-designware-platdrv.c | 80 ---------------------
 1 file changed, 80 deletions(-)

diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c
index c8a203fff4d1..6d6e81242f74 100644
--- a/drivers/i2c/busses/i2c-designware-platdrv.c
+++ b/drivers/i2c/busses/i2c-designware-platdrv.c
@@ -8,14 +8,12 @@
  * 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>
@@ -132,80 +130,6 @@ 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);
@@ -214,10 +138,6 @@ 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: 3d6d817622b0a9721e3cc404df3469171582be13
-- 
2.55.0


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

* Re: [PATCH] Revert "i2c: designware: defer probe if child GpioInt controllers are not bound"
  2026-08-14 10:07 [PATCH] Revert "i2c: designware: defer probe if child GpioInt controllers are not bound" Hardik Prakash
@ 2026-08-14 10:49 ` Thorsten Leemhuis
  2026-08-14 14:55 ` Mario Limonciello
  2026-08-14 21:19 ` Andi Shyti
  2 siblings, 0 replies; 4+ messages in thread
From: Thorsten Leemhuis @ 2026-08-14 10:49 UTC (permalink / raw)
  To: andi.shyti, linux-i2c
  Cc: linux-gpio, wsa, Hardik Prakash, andriy.shevchenko,
	mario.limonciello, brgl, basavaraj.natikar, linusw, nathan,
	chaitanya.kumar.borah, regressions, linux-kernel

On 8/14/26 12:07, Hardik Prakash wrote:
> This reverts commit 0a4bb2abc3e56d7be6e69b050c88ba52c87e22bf.

@Andi Shyti, would be great if you could pick this up and send it to
Linus for 7.2 (aka soon) or ask him to apply this from here. But from a
quick search on lore it looks like you are busy, on vacation, or
something. No worries, but therefore I might ask Linus to pull this
directly tomorrow to ensure this makes it into 7.2, as mentioned
yesterday already here:
https://lore.kernel.org/all/9095d7c8-92fe-4163-98f8-a53bbcefd6cb@leemhuis.info/

> The reverted commit causes a regression on ThinkPad T14s Gen 4 (AMD):
> the touchpad's I2C controller fails with lost arbitration errors,
> because it delays i2c-designware's probe by roughly 500ms, which
> shifts the touchpad's first HID descriptor fetch into a window where
> the platform's embedded controller is still acting as a secondary I2C
> bus master. Debug tracing confirms the GpioInt dependency check itself
> behaves correctly (it defers appropriately and confirms the GPIO
> controller is bound); the arbitration failure happens roughly a
> second after the check passes, when i2c_hid_acpi's own probe attempts
> its first transaction.
> 
> The original fix is still needed for the Lenovo Yoga 7 14AGP11
> touchscreen race the commit addressed, but a corrected version will
> be resubmitted once the EC bus-mastering interaction is understood
> and handled properly, rather than reintroducing a different
> regression on more widely-used ThinkPad hardware in the meantime.
> 
> Reported-by: Thorsten Leemhuis <linux@leemhuis.info>

Tested-by: Thorsten Leemhuis <linux@leemhuis.info>

Ciao, Thorsten

> Closes: https://lore.kernel.org/all/b4a4eadb-282f-464c-843a-19d415a34d0c@leemhuis.info/
> Signed-off-by: Hardik Prakash <hardikprakash.official@gmail.com>
> ---
>  drivers/i2c/busses/i2c-designware-platdrv.c | 80 ---------------------
>  1 file changed, 80 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c
> index c8a203fff4d1..6d6e81242f74 100644
> --- a/drivers/i2c/busses/i2c-designware-platdrv.c
> +++ b/drivers/i2c/busses/i2c-designware-platdrv.c
> @@ -8,14 +8,12 @@
>   * 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>
> @@ -132,80 +130,6 @@ 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);
> @@ -214,10 +138,6 @@ 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: 3d6d817622b0a9721e3cc404df3469171582be13


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

* Re: [PATCH] Revert "i2c: designware: defer probe if child GpioInt controllers are not bound"
  2026-08-14 10:07 [PATCH] Revert "i2c: designware: defer probe if child GpioInt controllers are not bound" Hardik Prakash
  2026-08-14 10:49 ` Thorsten Leemhuis
@ 2026-08-14 14:55 ` Mario Limonciello
  2026-08-14 21:19 ` Andi Shyti
  2 siblings, 0 replies; 4+ messages in thread
From: Mario Limonciello @ 2026-08-14 14:55 UTC (permalink / raw)
  To: Hardik Prakash, linux-i2c
  Cc: linux-gpio, wsa, andriy.shevchenko, brgl, basavaraj.natikar,
	linusw, nathan, chaitanya.kumar.borah, andi.shyti, linux,
	regressions, linux-kernel



On 8/14/26 05:07, Hardik Prakash wrote:
> This reverts commit 0a4bb2abc3e56d7be6e69b050c88ba52c87e22bf.
> 
> The reverted commit causes a regression on ThinkPad T14s Gen 4 (AMD):
> the touchpad's I2C controller fails with lost arbitration errors,
> because it delays i2c-designware's probe by roughly 500ms, which
> shifts the touchpad's first HID descriptor fetch into a window where
> the platform's embedded controller is still acting as a secondary I2C
> bus master. Debug tracing confirms the GpioInt dependency check itself
> behaves correctly (it defers appropriately and confirms the GPIO
> controller is bound); the arbitration failure happens roughly a
> second after the check passes, when i2c_hid_acpi's own probe attempts
> its first transaction.
> 
> The original fix is still needed for the Lenovo Yoga 7 14AGP11
> touchscreen race the commit addressed, but a corrected version will
> be resubmitted once the EC bus-mastering interaction is understood
> and handled properly, rather than reintroducing a different
> regression on more widely-used ThinkPad hardware in the meantime.
> 
> Reported-by: Thorsten Leemhuis <linux@leemhuis.info>
> Closes: https://lore.kernel.org/all/b4a4eadb-282f-464c-843a-19d415a34d0c@leemhuis.info/
> Signed-off-by: Hardik Prakash <hardikprakash.official@gmail.com>
Reviewed-by: Mario Limonciello (AMD) <superm1@kernel.org>> ---
>   drivers/i2c/busses/i2c-designware-platdrv.c | 80 ---------------------
>   1 file changed, 80 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c
> index c8a203fff4d1..6d6e81242f74 100644
> --- a/drivers/i2c/busses/i2c-designware-platdrv.c
> +++ b/drivers/i2c/busses/i2c-designware-platdrv.c
> @@ -8,14 +8,12 @@
>    * 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>
> @@ -132,80 +130,6 @@ 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);
> @@ -214,10 +138,6 @@ 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: 3d6d817622b0a9721e3cc404df3469171582be13


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

* Re: [PATCH] Revert "i2c: designware: defer probe if child GpioInt controllers are not bound"
  2026-08-14 10:07 [PATCH] Revert "i2c: designware: defer probe if child GpioInt controllers are not bound" Hardik Prakash
  2026-08-14 10:49 ` Thorsten Leemhuis
  2026-08-14 14:55 ` Mario Limonciello
@ 2026-08-14 21:19 ` Andi Shyti
  2 siblings, 0 replies; 4+ messages in thread
From: Andi Shyti @ 2026-08-14 21:19 UTC (permalink / raw)
  To: Hardik Prakash
  Cc: linux-i2c, linux-gpio, wsa, andriy.shevchenko, mario.limonciello,
	brgl, basavaraj.natikar, linusw, nathan, chaitanya.kumar.borah,
	linux, regressions, linux-kernel

Hi Hardik,

On Fri, Aug 14, 2026 at 03:37:19PM +0530, Hardik Prakash wrote:
> This reverts commit 0a4bb2abc3e56d7be6e69b050c88ba52c87e22bf.
> 
> The reverted commit causes a regression on ThinkPad T14s Gen 4 (AMD):
> the touchpad's I2C controller fails with lost arbitration errors,
> because it delays i2c-designware's probe by roughly 500ms, which
> shifts the touchpad's first HID descriptor fetch into a window where
> the platform's embedded controller is still acting as a secondary I2C
> bus master. Debug tracing confirms the GpioInt dependency check itself
> behaves correctly (it defers appropriately and confirms the GPIO
> controller is bound); the arbitration failure happens roughly a
> second after the check passes, when i2c_hid_acpi's own probe attempts
> its first transaction.
> 
> The original fix is still needed for the Lenovo Yoga 7 14AGP11
> touchscreen race the commit addressed, but a corrected version will
> be resubmitted once the EC bus-mastering interaction is understood
> and handled properly, rather than reintroducing a different
> regression on more widely-used ThinkPad hardware in the meantime.
> 
> Reported-by: Thorsten Leemhuis <linux@leemhuis.info>
> Closes: https://lore.kernel.org/all/b4a4eadb-282f-464c-843a-19d415a34d0c@leemhuis.info/
> Signed-off-by: Hardik Prakash <hardikprakash.official@gmail.com>

merged to i2c/i2c-2.

Thanks,
Andi

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

end of thread, other threads:[~2026-08-14 21:19 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-08-14 10:07 [PATCH] Revert "i2c: designware: defer probe if child GpioInt controllers are not bound" Hardik Prakash
2026-08-14 10:49 ` Thorsten Leemhuis
2026-08-14 14:55 ` Mario Limonciello
2026-08-14 21:19 ` Andi Shyti

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®