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

[Andy Shevchenko <andriy.shevchenko@intel.com>]

On Tue, May 12, 2026 at 01:01:39PM +0530, Hardik Prakash wrote:
> The existing dw_i2c_amd_gpio_defer_dmi quirk for Lenovo 83TD checks

Existing? Is this message and the whole stuff is somehow AI-assisted?

> gpio_dev->driver to determine if the GPIO controller is ready, but
> this pointer is set before pinctrl-amd's probe completes, causing
> i2c_designware to probe AMDI0010:02 before the GPIO IRQ quirk runs.
> 
> Switch to device_is_bound() under device_lock() to correctly defer
> until GPIO probe has fully completed. Also replace the string-based
> device lookup with ACPI HID/UID matching for robustness, and add
> DMI_BOARD_NAME to the DMI match to reduce false positives.

> Fixes: the lost arbitration on AMDI0010:02 at boot on Lenovo 83TD.

This is incorrect way of providing a Fixes tag.

...

> +struct dw_i2c_hid_uid {
> +	const char *hid;
> +	u64 uid;
> +};

Useless as duplicates the existing strictures. See also below.

...

> +static int dw_i2c_match_hid_uid(struct device *dev, const void *data)
> +{
> +	struct acpi_device *adev = ACPI_COMPANION(dev);
> +	const struct dw_i2c_hid_uid *id = data;
> +
> +	if (!adev)
> +		return 0;
> +
> +	return acpi_dev_hid_uid_match(adev, id->hid, id->uid);
> +}
> +
> +static struct device *dw_i2c_find_platform_hid_uid(const char *hid, u64 uid)
> +{
> +	struct dw_i2c_hid_uid data = {
> +		.hid = hid,
> +		.uid = uid,
> +	};
> +
> +	return bus_find_device(&platform_bus_type, NULL, &data, dw_i2c_match_hid_uid);
> +}
> +
> +static bool dw_i2c_needs_amd_gpio_dep(struct device *device)
> +{
> +	struct acpi_device *adev = ACPI_COMPANION(device);
> +
> +	if (!dmi_check_system(dw_i2c_amd_gpio_defer_dmi))
> +		return false;
> +	if (!adev)
> +		return false;
> +
> +	return acpi_dev_hid_uid_match(adev, "AMDI0010", 2);
> +}

The whole flow is just a repetition of acpi_dev_get_first_match_dev().

...

> +static int dw_i2c_defer_for_amd_gpio(struct device *device)
> +{
> +	struct device *gpio_dev;
> +
> +	if (!dw_i2c_needs_amd_gpio_dep(device))
> +		return 0;
> +
> +	gpio_dev = dw_i2c_find_platform_hid_uid("AMDI0030", 0);
> +	if (!gpio_dev)
> +		return -EPROBE_DEFER;
> +
> +	device_lock(gpio_dev);
> +	if (!device_is_bound(gpio_dev)) {
> +		device_unlock(gpio_dev);
> +		put_device(gpio_dev);
> +		return -EPROBE_DEFER;
> +	}
> +	device_unlock(gpio_dev);

Interesting dance. Needs a comment explaining what's going on here and why this
deferral probe won't be a problem in other scenarios.

> +	if (!device_link_add(device, gpio_dev, DL_FLAG_AUTOREMOVE_CONSUMER))

This needs a good comment explaining what's going on.

> +		dev_warn(device, "failed to add device link to AMDI0030:00\n");

Why :00? Is it guaranteed that this is going to be with :00? Note, this suffix
is instance number in Linux and strictly speaking might differ even from boot
to boot on the same machine.

> +	put_device(gpio_dev);
> +	return 0;
> +}

-- 
With Best Regards,
Andy Shevchenko