Re: [PATCH] Revert "i2c: designware: defer probe if child GpioInt controllers are not bound"

[Nathan Chancellor <nathan@kernel.org>]

On Wed, Jun 10, 2026 at 02:07:01PM +0530, Hardik Prakash wrote:
> This reverts commit ef76a3a28c79b628890431aa344af633e892035b.
> 
> The patch causes boot regressions on multiple machines. A NULL pointer
> dereference occurs when agpio->resource_source.string_ptr is NULL (i.e.
> when string_length is 0), and a probe deferral loop causes CPU starvation
> leading to kernel panic on Intel CI machines.
> 
> The patch needs a proper rewrite addressing these issues before resubmission.
> 
> Reported-by: Nathan Chancellor <nathan@kernel.org>
> Reported-by: Borah, Chaitanya Kumar <chaitanya.kumar.borah@intel.com>
> Signed-off-by: Hardik Prakash <hardikprakash.official@gmail.com>

Closes: https://lore.kernel.org/20260602185339.GA404948@ax162/
Tested-by: Nathan Chancellor <nathan@kernel.org>

> ---
>  drivers/i2c/busses/i2c-designware-platdrv.c | 156 --------------------
>  1 file changed, 156 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c
> index 1c01b0460385..3351c4a9ef11 100644
> --- a/drivers/i2c/busses/i2c-designware-platdrv.c
> +++ b/drivers/i2c/busses/i2c-designware-platdrv.c
> @@ -8,8 +8,6 @@
>   * 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>
> @@ -132,152 +130,6 @@ 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(sizeof(*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;
> -
> -	INIT_LIST_HEAD(&res_list);
> -
> -	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;
> -		bool bound;
> -
> -		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) {
> -			bound = device_is_bound(gpio_dev);
> -		}
> -		if (!bound) {
> -			put_device(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);
> @@ -286,14 +138,6 @@ 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;
> 
> base-commit: 3f786abd23951f3f600a62fef42469d9200d5f52
> prerequisite-patch-id: 22fa9ba20fa28cf94185281704c51feef7abc701
> -- 
> 2.54.0
> 

-- 
Cheers,
Nathan