Re: [PATCH 1/2] platform/x86: x86-android-tablets: Add support for getting i2c_adapter by PCI parent devname()

[Andy Shevchenko <andy@kernel.org>]

On Fri, Oct 25, 2024 at 11:44:34AM +0200, Hans de Goede wrote:
> On the Vexia EDU ATLA 10 tablet, which ships with Android + a custom Linux
> (guadalinex) using the custom Android kernel the I2C controllers are not
> enumerated as ACPI devices as they typically are.
> 
> Instead they are enumerated as PCI devices which do not have ACPI firmware
> nodes associated with them, so getting the i2c_adapter by the ACPI path of
> its firmware node does not work.
> 
> Add support for getting the i2c_adapter by the devname() of its PCI parent
> instead.

...

>  #include <linux/acpi.h>
> +#include <linux/device.h>

> +#include <linux/device/bus.h>

The above is enough. but if you want go with this, I would swap them:

#include <linux/device/bus.h>
#include <linux/device.h>

>  #include <linux/dmi.h>
>  #include <linux/gpio/consumer.h>
>  #include <linux/gpio/machine.h>
>  #include <linux/irq.h>
>  #include <linux/module.h>
> +#include <linux/pci.h>
>  #include <linux/platform_device.h>
>  #include <linux/serdev.h>
>  #include <linux/string.h>

...

> +static __init int match_parent(struct device *dev, const void *data)
> +{
> +	return dev->parent == data;
> +}

To me it sounds like a candidate to be in drivers/base/core.c and bus.h.

...

> -	status = acpi_get_handle(NULL, client_info->adapter_path, &handle);
> -	if (ACPI_FAILURE(status)) {
> -		pr_err("Error could not get %s handle\n", client_info->adapter_path);
> -		return -ENODEV;
> +	if (dev_info->use_pci_devname) {
> +		struct device *pdev, *adap_dev;
> +
> +		pdev = bus_find_device_by_name(&pci_bus_type, NULL, client_info->adapter_path);
> +		if (!pdev) {
> +			pr_err("Error could not find %s PCI device\n", client_info->adapter_path);
> +			return -ENODEV;
> +		}
> +
> +		adap_dev = bus_find_device(&i2c_bus_type, NULL, pdev, match_parent);
> +		if (adap_dev) {
> +			adap = i2c_verify_adapter(adap_dev);
> +			if (!adap)
> +				put_device(adap_dev);
> +		}
> +
> +		put_device(pdev);
> +	} else {
> +		acpi_handle handle;
> +		acpi_status status;
> +
> +		status = acpi_get_handle(NULL, client_info->adapter_path, &handle);
> +		if (ACPI_FAILURE(status)) {
> +			pr_err("Error could not get %s handle\n", client_info->adapter_path);
> +			return -ENODEV;
> +		}
> +
> +		adap = i2c_acpi_find_adapter_by_handle(handle);

Can we rather have two patches:
1) create a helper out of the existing code;
2) added the new approach also using a separate helper?

>  	}

-- 
With Best Regards,
Andy Shevchenko