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

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

[Hans de Goede]

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(-)

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,
-- 
2.47.0

[PATCH 2/2] platform/x86: x86-android-tablets: Add support for Vexia EDU ATLA 10 tablet

[Hans de Goede]

Add support for the Vexia EDU ATLA 10 tablet, Android 4.2/4.4 + Guadalinex
Ubuntu tablet distributed to schools in the Spanish Andalucía region.

Besides the usual broken DSDT issues this tablet is special because all
its LPSS island peripherals are enumerated as PCI devices rather then as
ACPI devices as they typically are.

At the same time there are disabled (_STA=0) ACPI devices for
the peripherals and child ACPI devices for e.g. attached I2C/SDIO devices
are children of these disabled ACPI devices and thus will not be used
by Linux since the parent is disabled.

So besides the usual manual i2c-client instantiation for accel/touchscreen
this tablet also requires manual i2c-client instantiation for the codec
and for the PMIC.

Also it seems the mainboard was designed for Windows not Android, so
it has an I2C attached embedded controller instead of allowing direct
access to the charger + fuel-gauge chips as is usual with Android boards.

Normally when there is an embedded controller, there also is ACPI battery
support, but since this shipped with Android that is missing and Linux
needs to have a power_supply class driver talking directly to the EC.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 .../platform/x86/x86-android-tablets/Kconfig  |   1 +
 .../platform/x86/x86-android-tablets/dmi.c    |  10 ++
 .../platform/x86/x86-android-tablets/other.c  | 165 ++++++++++++++++++
 .../x86-android-tablets/x86-android-tablets.h |   1 +
 4 files changed, 177 insertions(+)

diff --git a/drivers/platform/x86/x86-android-tablets/Kconfig b/drivers/platform/x86/x86-android-tablets/Kconfig
index 88d9e8f2ff24..2267d8362020 100644
--- a/drivers/platform/x86/x86-android-tablets/Kconfig
+++ b/drivers/platform/x86/x86-android-tablets/Kconfig
@@ -6,6 +6,7 @@
 config X86_ANDROID_TABLETS
 	tristate "X86 Android tablet support"
 	depends on I2C && SPI && SERIAL_DEV_BUS && ACPI && EFI && GPIOLIB && PMIC_OPREGION
+	depends on PCI
 	select NEW_LEDS
 	select LEDS_CLASS
 	help
diff --git a/drivers/platform/x86/x86-android-tablets/dmi.c b/drivers/platform/x86/x86-android-tablets/dmi.c
index 17f6da96aa01..f1e8428c98d8 100644
--- a/drivers/platform/x86/x86-android-tablets/dmi.c
+++ b/drivers/platform/x86/x86-android-tablets/dmi.c
@@ -189,6 +189,16 @@ const struct dmi_system_id x86_android_tablet_ids[] __initconst = {
 		},
 		.driver_data = (void *)&whitelabel_tm800a550l_info,
 	},
+	{
+		/* Vexia Edu Atla 10 tablet */
+		.matches = {
+			DMI_MATCH(DMI_BOARD_VENDOR, "AMI Corporation"),
+			DMI_MATCH(DMI_BOARD_NAME, "Aptio CRB"),
+			/* Above strings are too generic, also match on BIOS date */
+			DMI_MATCH(DMI_BIOS_DATE, "08/25/2014"),
+		},
+		.driver_data = (void *)&vexia_edu_atla10_info,
+	},
 	{
 		/* Xiaomi Mi Pad 2 */
 		.matches = {
diff --git a/drivers/platform/x86/x86-android-tablets/other.c b/drivers/platform/x86/x86-android-tablets/other.c
index 7db8aa58b907..01f1e4826426 100644
--- a/drivers/platform/x86/x86-android-tablets/other.c
+++ b/drivers/platform/x86/x86-android-tablets/other.c
@@ -9,9 +9,12 @@
  */
 
 #include <linux/acpi.h>
+#include <linux/device.h>
+#include <linux/device/bus.h>
 #include <linux/gpio/machine.h>
 #include <linux/input.h>
 #include <linux/leds.h>
+#include <linux/pci.h>
 #include <linux/platform_device.h>
 #include <linux/pwm.h>
 
@@ -597,6 +600,168 @@ const struct x86_dev_info whitelabel_tm800a550l_info __initconst = {
 	.gpiod_lookup_tables = whitelabel_tm800a550l_gpios,
 };
 
+/*
+ * Vexia EDU ATLA 10 tablet, Android 4.2 / 4.4 + Guadalinex Ubuntu tablet
+ * distributed to schools in the Spanish Andalucía region.
+ */
+const char * const crystal_cove_pwrsrc_psy[] = { "crystal_cove_pwrsrc" };
+
+static const struct property_entry vexia_edu_atla10_ulpmc_props[] = {
+	PROPERTY_ENTRY_STRING_ARRAY("supplied-from", crystal_cove_pwrsrc_psy),
+	{ }
+};
+
+const struct software_node vexia_edu_atla10_ulpmc_node = {
+	.properties = vexia_edu_atla10_ulpmc_props,
+};
+
+static const char * const vexia_edu_atla10_accel_mount_matrix[] = {
+	"0", "-1", "0",
+	"1", "0", "0",
+	"0", "0", "1"
+};
+
+static const struct property_entry vexia_edu_atla10_accel_props[] = {
+	PROPERTY_ENTRY_STRING_ARRAY("mount-matrix", vexia_edu_atla10_accel_mount_matrix),
+	{ }
+};
+
+static const struct software_node vexia_edu_atla10_accel_node = {
+	.properties = vexia_edu_atla10_accel_props,
+};
+
+static const struct property_entry vexia_edu_atla10_touchscreen_props[] = {
+	PROPERTY_ENTRY_U32("hid-descr-addr", 0x0000),
+	PROPERTY_ENTRY_U32("post-reset-deassert-delay-ms", 120),
+	{ }
+};
+
+static const struct software_node vexia_edu_atla10_touchscreen_node = {
+	.properties = vexia_edu_atla10_touchscreen_props,
+};
+
+static const struct property_entry vexia_edu_atla10_pmic_props[] = {
+	PROPERTY_ENTRY_BOOL("linux,register-pwrsrc-power_supply"),
+	{ }
+};
+
+static const struct software_node vexia_edu_atla10_pmic_node = {
+	.properties = vexia_edu_atla10_pmic_props,
+};
+
+static const struct x86_i2c_client_info vexia_edu_atla10_i2c_clients[] __initconst = {
+	{	/* I2C attached embedded controller, used to access fuel-gauge */
+		.board_info = {
+			.type = "vexia_atla10_ec",
+			.addr = 0x76,
+			.dev_name = "ulpmc",
+			.swnode = &vexia_edu_atla10_ulpmc_node,
+		},
+		.adapter_path = "0000:00:18.1",
+	},
+	{	/* RT5642 audio codec */
+		.board_info = {
+			.type = "rt5640",
+			.addr = 0x1c,
+			.dev_name = "rt5640",
+		},
+		.adapter_path = "0000:00:18.2",
+		.irq_data = {
+			.type = X86_ACPI_IRQ_TYPE_GPIOINT,
+			.chip = "INT33FC:02",
+			.index = 4,
+			.trigger = ACPI_EDGE_SENSITIVE,
+			.polarity = ACPI_ACTIVE_HIGH,
+			.con_id = "rt5640_irq",
+		},
+	},
+	{
+		/* kxtj21009 accelerometer */
+		.board_info = {
+			.type = "kxtj21009",
+			.addr = 0x0f,
+			.dev_name = "kxtj21009",
+			.swnode = &vexia_edu_atla10_accel_node,
+		},
+		.adapter_path = "0000:00:18.5",
+	}, {
+		/* FT5416DQ9 touchscreen controller */
+		.board_info = {
+			.type = "hid-over-i2c",
+			.addr = 0x38,
+			.dev_name = "FTSC1000",
+			.swnode = &vexia_edu_atla10_touchscreen_node,
+		},
+		.adapter_path = "0000:00:18.6",
+		.irq_data = {
+			.type = X86_ACPI_IRQ_TYPE_APIC,
+			.index = 0x45,
+			.trigger = ACPI_LEVEL_SENSITIVE,
+			.polarity = ACPI_ACTIVE_HIGH,
+		},
+	}, {
+		/* Crystal Cove PMIC */
+		.board_info = {
+			.type = "intel-crystal-cove",
+			.addr = 0x6e,
+			.dev_name = "intel-crystal-cove",
+			.swnode = &vexia_edu_atla10_pmic_node,
+		},
+		.adapter_path = "0000:00:18.7",
+		.irq_data = {
+			.type = X86_ACPI_IRQ_TYPE_APIC,
+			.index = 0x43,
+			.trigger = ACPI_LEVEL_SENSITIVE,
+			.polarity = ACPI_ACTIVE_HIGH,
+		},
+	}
+};
+
+static struct gpiod_lookup_table vexia_edu_atla10_ft5416_gpios = {
+	.dev_id = "i2c-FTSC1000",
+	.table = {
+		GPIO_LOOKUP("INT33FC:00", 60, "reset", GPIO_ACTIVE_LOW),
+		{ }
+	},
+};
+
+static struct gpiod_lookup_table * const vexia_edu_atla10_gpios[] = {
+	&vexia_edu_atla10_ft5416_gpios,
+	NULL
+};
+
+static int __init vexia_edu_atla10_init(struct device *dev)
+{
+	struct device *pdev;
+	int ret;
+
+	/* Enable the Wifi module by setting the wifi_enable pin to 1 */
+	ret = x86_android_tablet_get_gpiod("INT33FC:02", 20, "wifi_enable",
+					   false, GPIOD_OUT_HIGH, NULL);
+	if (ret)
+		return ret;
+
+	/* Reprobe the SDIO controller to enumerate the now enabled Wifi module */
+	pdev = bus_find_device_by_name(&pci_bus_type, NULL, "0000:00:11.0");
+	if (!pdev)
+		return -EPROBE_DEFER;
+
+	ret = device_reprobe(pdev);
+	if (ret)
+		dev_warn(pdev, "Reprobing 0000:00:11.0 error: %d\n", ret);
+
+	put_device(pdev);
+	return 0;
+}
+
+const struct x86_dev_info vexia_edu_atla10_info __initconst = {
+	.i2c_client_info = vexia_edu_atla10_i2c_clients,
+	.i2c_client_count = ARRAY_SIZE(vexia_edu_atla10_i2c_clients),
+	.gpiod_lookup_tables = vexia_edu_atla10_gpios,
+	.init = vexia_edu_atla10_init,
+	.use_pci_devname = true,
+};
+
 /*
  * The firmware node for ktd2026 on Xaomi pad2. It composed of a RGB LED node
  * with three subnodes for each color (B/G/R). The RGB LED node is named
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 d26a4792eb0e..0fc7e8cff672 100644
--- a/drivers/platform/x86/x86-android-tablets/x86-android-tablets.h
+++ b/drivers/platform/x86/x86-android-tablets/x86-android-tablets.h
@@ -120,6 +120,7 @@ extern const struct x86_dev_info nextbook_ares8_info;
 extern const struct x86_dev_info nextbook_ares8a_info;
 extern const struct x86_dev_info peaq_c1010_info;
 extern const struct x86_dev_info whitelabel_tm800a550l_info;
+extern const struct x86_dev_info vexia_edu_atla10_info;
 extern const struct x86_dev_info xiaomi_mipad2_info;
 extern const struct dmi_system_id x86_android_tablet_ids[];
 
-- 
2.47.0

Re: [PATCH 2/2] platform/x86: x86-android-tablets: Add support for Vexia EDU ATLA 10 tablet

[Andy Shevchenko]

On Fri, Oct 25, 2024 at 11:44:35AM +0200, Hans de Goede wrote:
> Add support for the Vexia EDU ATLA 10 tablet, Android 4.2/4.4 + Guadalinex
> Ubuntu tablet distributed to schools in the Spanish Andalucía region.
> 
> Besides the usual broken DSDT issues this tablet is special because all
> its LPSS island peripherals are enumerated as PCI devices rather then as
> ACPI devices as they typically are.

Just wondering if you investigated the possibility to switch this via
(most likely hidden or absent) BIOS options? On the prototypes we have
a switch in the firmware on how to enumerate LPSS, so it's possible
to get them enumerated as ACPI ones.

> At the same time there are disabled (_STA=0) ACPI devices for
> the peripherals and child ACPI devices for e.g. attached I2C/SDIO devices
> are children of these disabled ACPI devices and thus will not be used
> by Linux since the parent is disabled.
> 
> So besides the usual manual i2c-client instantiation for accel/touchscreen
> this tablet also requires manual i2c-client instantiation for the codec
> and for the PMIC.
> 
> Also it seems the mainboard was designed for Windows not Android, so
> it has an I2C attached embedded controller instead of allowing direct
> access to the charger + fuel-gauge chips as is usual with Android boards.
> 
> Normally when there is an embedded controller, there also is ACPI battery
> support, but since this shipped with Android that is missing and Linux
> needs to have a power_supply class driver talking directly to the EC.

...

>  	depends on I2C && SPI && SERIAL_DEV_BUS && ACPI && EFI && GPIOLIB && PMIC_OPREGION
> +	depends on PCI

Maybe it's time to rearrange this a bit?

	depends on I2C && SPI && SERIAL_DEV_BUS
	depends on GPIOLIB && PMIC_OPREGION
	depends on ACPI && EFI && PCI

?

...

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

device.h is enough.

...

> +static int __init vexia_edu_atla10_init(struct device *dev)
> +{
> +	struct device *pdev;
> +	int ret;
> +
> +	/* Enable the Wifi module by setting the wifi_enable pin to 1 */
> +	ret = x86_android_tablet_get_gpiod("INT33FC:02", 20, "wifi_enable",
> +					   false, GPIOD_OUT_HIGH, NULL);
> +	if (ret)
> +		return ret;
> +
> +	/* Reprobe the SDIO controller to enumerate the now enabled Wifi module */
> +	pdev = bus_find_device_by_name(&pci_bus_type, NULL, "0000:00:11.0");
> +	if (!pdev)
> +		return -EPROBE_DEFER;

Why? Can't the pci_get_domain_bus_and_slot() give you the same result more
effectively?

> +	ret = device_reprobe(pdev);
> +	if (ret)
> +		dev_warn(pdev, "Reprobing 0000:00:11.0 error: %d\n", ret);
> +
> +	put_device(pdev);
> +	return 0;
> +}

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 2/2] platform/x86: x86-android-tablets: Add support for Vexia EDU ATLA 10 tablet

[Hans de Goede]

Hi Andy,

On 25-Oct-24 7:47 PM, Andy Shevchenko wrote:
> On Fri, Oct 25, 2024 at 11:44:35AM +0200, Hans de Goede wrote:
>> Add support for the Vexia EDU ATLA 10 tablet, Android 4.2/4.4 + Guadalinex
>> Ubuntu tablet distributed to schools in the Spanish Andalucía region.
>>
>> Besides the usual broken DSDT issues this tablet is special because all
>> its LPSS island peripherals are enumerated as PCI devices rather then as
>> ACPI devices as they typically are.
> 
> Just wondering if you investigated the possibility to switch this via
> (most likely hidden or absent) BIOS options? On the prototypes we have
> a switch in the firmware on how to enumerate LPSS, so it's possible
> to get them enumerated as ACPI ones.

I checked (before sending out these patches) and the option to switch
LPSS devices to be ACPI enumerated is there (it as an unlocked BIOS with
all options) but it does not work (it does not do anything at all).

> 
>> At the same time there are disabled (_STA=0) ACPI devices for
>> the peripherals and child ACPI devices for e.g. attached I2C/SDIO devices
>> are children of these disabled ACPI devices and thus will not be used
>> by Linux since the parent is disabled.
>>
>> So besides the usual manual i2c-client instantiation for accel/touchscreen
>> this tablet also requires manual i2c-client instantiation for the codec
>> and for the PMIC.
>>
>> Also it seems the mainboard was designed for Windows not Android, so
>> it has an I2C attached embedded controller instead of allowing direct
>> access to the charger + fuel-gauge chips as is usual with Android boards.
>>
>> Normally when there is an embedded controller, there also is ACPI battery
>> support, but since this shipped with Android that is missing and Linux
>> needs to have a power_supply class driver talking directly to the EC.
> 
> ...
> 
>>  	depends on I2C && SPI && SERIAL_DEV_BUS && ACPI && EFI && GPIOLIB && PMIC_OPREGION
>> +	depends on PCI
> 
> Maybe it's time to rearrange this a bit?
> 
> 	depends on I2C && SPI && SERIAL_DEV_BUS
> 	depends on GPIOLIB && PMIC_OPREGION
> 	depends on ACPI && EFI && PCI
> 
> ?


Ack, fixed for v2; and I've also moved this to the patch to get
the i2c adapter by PCI parent since that code also needs the PCI
core.

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

Ack, fixed for v2.

> 
> ...
> 
>> +static int __init vexia_edu_atla10_init(struct device *dev)
>> +{
>> +	struct device *pdev;
>> +	int ret;
>> +
>> +	/* Enable the Wifi module by setting the wifi_enable pin to 1 */
>> +	ret = x86_android_tablet_get_gpiod("INT33FC:02", 20, "wifi_enable",
>> +					   false, GPIOD_OUT_HIGH, NULL);
>> +	if (ret)
>> +		return ret;
>> +
>> +	/* Reprobe the SDIO controller to enumerate the now enabled Wifi module */
>> +	pdev = bus_find_device_by_name(&pci_bus_type, NULL, "0000:00:11.0");
>> +	if (!pdev)
>> +		return -EPROBE_DEFER;
> 
> Why? Can't the pci_get_domain_bus_and_slot() give you the same result more
> effectively?

Yes I think that that can work. I'll switch to that and run some quick tests for v2.

Regards,

Hans




> 
>> +	ret = device_reprobe(pdev);
>> +	if (ret)
>> +		dev_warn(pdev, "Reprobing 0000:00:11.0 error: %d\n", ret);
>> +
>> +	put_device(pdev);
>> +	return 0;
>> +}
> 

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

[Andy Shevchenko]

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

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

[Hans de Goede]

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