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
