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

[Hans de Goede <hdegoede@redhat.com>]

Hi,

On 25-Oct-24 5:32 PM, Andy Shevchenko wrote:
> 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?

Ack done for v2.

Regards,

Hans