| Message ID | 20241025094435.71718-1-hdegoede@redhat.com (mailing list archive) |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | [1/2] platform/x86: x86-android-tablets: Add support for getting i2c_adapter by PCI parent devname() | expand |
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? > }
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
diff --git a/drivers/platform/x86/x86-android-tablets/core.c b/drivers/platform/x86/x86-android-tablets/core.c index ef572b90e06b..599737d84242 100644 --- a/drivers/platform/x86/x86-android-tablets/core.c +++ b/drivers/platform/x86/x86-android-tablets/core.c @@ -11,11 +11,14 @@ #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt #include <linux/acpi.h> +#include <linux/device.h> +#include <linux/device/bus.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> @@ -155,26 +158,52 @@ static struct gpiod_lookup_table * const *gpiod_lookup_tables; static const struct software_node *bat_swnode; static void (*exit_handler)(void); +static __init int match_parent(struct device *dev, const void *data) +{ + return dev->parent == data; +} + static __init int x86_instantiate_i2c_client(const struct x86_dev_info *dev_info, int idx) { const struct x86_i2c_client_info *client_info = &dev_info->i2c_client_info[idx]; struct i2c_board_info board_info = client_info->board_info; - struct i2c_adapter *adap; - acpi_handle handle; - acpi_status status; + struct i2c_adapter *adap = NULL; board_info.irq = x86_acpi_irq_helper_get(&client_info->irq_data); if (board_info.irq < 0) return board_info.irq; - 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); } - adap = i2c_acpi_find_adapter_by_handle(handle); if (!adap) { pr_err("error could not get %s adapter\n", client_info->adapter_path); return -ENODEV; diff --git a/drivers/platform/x86/x86-android-tablets/x86-android-tablets.h b/drivers/platform/x86/x86-android-tablets/x86-android-tablets.h index 5517e438c7b6..d26a4792eb0e 100644 --- a/drivers/platform/x86/x86-android-tablets/x86-android-tablets.h +++ b/drivers/platform/x86/x86-android-tablets/x86-android-tablets.h @@ -91,6 +91,7 @@ struct x86_dev_info { int gpio_button_count; int (*init)(struct device *dev); void (*exit)(void); + bool use_pci_devname; }; int x86_android_tablet_get_gpiod(const char *chip, int pin, const char *con_id,
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. Signed-off-by: Hans de Goede <hdegoede@redhat.com> --- .../platform/x86/x86-android-tablets/core.c | 45 +++++++++++++++---- .../x86-android-tablets/x86-android-tablets.h | 1 + 2 files changed, 38 insertions(+), 8 deletions(-)