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

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

[Hans de Goede]

Hi All,

Here is v2 of my patch series to add support for the Vexia EDU ATLA 10
tablet.

Changes in v2:
- Add get_i2c_adap_by_handle() and get_i2c_adap_by_pci_parent()
  helpers as suggested by Andy

Regards,

Hans


Hans de Goede (3):
  platform/x86: x86-android-tablets: Add get_i2c_adap_by_handle() helper
  platform/x86: x86-android-tablets: Add support for getting i2c_adapter
    by PCI parent devname()
  platform/x86: x86-android-tablets: Add support for Vexia EDU ATLA 10
    tablet

 .../platform/x86/x86-android-tablets/Kconfig  |   1 +
 .../platform/x86/x86-android-tablets/core.c   |  60 ++++++-
 .../platform/x86/x86-android-tablets/dmi.c    |  20 +++
 .../platform/x86/x86-android-tablets/other.c  | 165 ++++++++++++++++++
 .../x86-android-tablets/x86-android-tablets.h |   2 +
 5 files changed, 239 insertions(+), 9 deletions(-)

-- 
2.47.0

[PATCH v2 1/3] platform/x86: x86-android-tablets: Add get_i2c_adap_by_handle() helper

[Hans de Goede]

Add get_i2c_adap_by_handle() helper function, this is a preparation patch
for adding support for getting i2c_adapter-s by PCI parent devname().

Suggested-by: Andy Shevchenko <andy@kernel.org>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
Changes in v2:
- New patch in v2 of this series
---
 .../platform/x86/x86-android-tablets/core.c   | 25 ++++++++++++-------
 1 file changed, 16 insertions(+), 9 deletions(-)

diff --git a/drivers/platform/x86/x86-android-tablets/core.c b/drivers/platform/x86/x86-android-tablets/core.c
index ef572b90e06b..4154395c60bb 100644
--- a/drivers/platform/x86/x86-android-tablets/core.c
+++ b/drivers/platform/x86/x86-android-tablets/core.c
@@ -155,26 +155,33 @@ static struct gpiod_lookup_table * const *gpiod_lookup_tables;
 static const struct software_node *bat_swnode;
 static void (*exit_handler)(void);
 
+static struct i2c_adapter *
+get_i2c_adap_by_handle(const struct x86_i2c_client_info *client_info)
+{
+	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 NULL;
+	}
+
+	return i2c_acpi_find_adapter_by_handle(handle);
+}
+
 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;
 
 	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;
-	}
-
-	adap = i2c_acpi_find_adapter_by_handle(handle);
+	adap = get_i2c_adap_by_handle(client_info);
 	if (!adap) {
 		pr_err("error could not get %s adapter\n", client_info->adapter_path);
 		return -ENODEV;
-- 
2.47.0

[PATCH v2 2/3] 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>
---
Changes in v2:
- Move "depends on PCI" Kconfig change to this patch
- Put the code to get the adapter in a get_i2c_adap_by_pci_parent() helper
---
 .../platform/x86/x86-android-tablets/Kconfig  |  4 +-
 .../platform/x86/x86-android-tablets/core.c   | 39 ++++++++++++++++++-
 .../x86-android-tablets/x86-android-tablets.h |  1 +
 3 files changed, 41 insertions(+), 3 deletions(-)

diff --git a/drivers/platform/x86/x86-android-tablets/Kconfig b/drivers/platform/x86/x86-android-tablets/Kconfig
index 88d9e8f2ff24..a67bddc43007 100644
--- a/drivers/platform/x86/x86-android-tablets/Kconfig
+++ b/drivers/platform/x86/x86-android-tablets/Kconfig
@@ -5,7 +5,9 @@
 
 config X86_ANDROID_TABLETS
 	tristate "X86 Android tablet support"
-	depends on I2C && SPI && SERIAL_DEV_BUS && ACPI && EFI && GPIOLIB && PMIC_OPREGION
+	depends on I2C && SPI && SERIAL_DEV_BUS
+	depends on GPIOLIB && PMIC_OPREGION
+	depends on ACPI && EFI && PCI
 	select NEW_LEDS
 	select LEDS_CLASS
 	help
diff --git a/drivers/platform/x86/x86-android-tablets/core.c b/drivers/platform/x86/x86-android-tablets/core.c
index 4154395c60bb..5241b260c0e8 100644
--- a/drivers/platform/x86/x86-android-tablets/core.c
+++ b/drivers/platform/x86/x86-android-tablets/core.c
@@ -11,11 +11,13 @@
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
 
 #include <linux/acpi.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>
@@ -170,18 +172,51 @@ get_i2c_adap_by_handle(const struct x86_i2c_client_info *client_info)
 	return i2c_acpi_find_adapter_by_handle(handle);
 }
 
+static __init int match_parent(struct device *dev, const void *data)
+{
+	return dev->parent == data;
+}
+
+static struct i2c_adapter *
+get_i2c_adap_by_pci_parent(const struct x86_i2c_client_info *client_info)
+{
+	struct i2c_adapter *adap = NULL;
+	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 NULL;
+	}
+
+	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);
+
+	return adap;
+}
+
 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;
+	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;
 
-	adap = get_i2c_adap_by_handle(client_info);
+	if (dev_info->use_pci_devname)
+		adap = get_i2c_adap_by_pci_parent(client_info);
+	else
+		adap = get_i2c_adap_by_handle(client_info);
+
 	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 v2 3/3] 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>
---
Changes in v2:
- Use pci_get_domain_bus_and_slot()
- Move "depends on PCI" to patch adding get_i2c_adap_by_pci_parent() helper
---
 .../platform/x86/x86-android-tablets/dmi.c    |  10 ++
 .../platform/x86/x86-android-tablets/other.c  | 163 ++++++++++++++++++
 .../x86-android-tablets/x86-android-tablets.h |   1 +
 3 files changed, 174 insertions(+)

diff --git a/drivers/platform/x86/x86-android-tablets/dmi.c b/drivers/platform/x86/x86-android-tablets/dmi.c
index 17f6da96aa01..3e5fa3b6e2fd 100644
--- a/drivers/platform/x86/x86-android-tablets/dmi.c
+++ b/drivers/platform/x86/x86-android-tablets/dmi.c
@@ -179,6 +179,16 @@ const struct dmi_system_id x86_android_tablet_ids[] __initconst = {
 		},
 		.driver_data = (void *)&peaq_c1010_info,
 	},
+	{
+		/* Vexia Edu Atla 10 tablet 9V version */
+		.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,
+	},
 	{
 		/* Whitelabel (sold as various brands) TM800A550L */
 		.matches = {
diff --git a/drivers/platform/x86/x86-android-tablets/other.c b/drivers/platform/x86/x86-android-tablets/other.c
index 7db8aa58b907..3ed808ac5b6b 100644
--- a/drivers/platform/x86/x86-android-tablets/other.c
+++ b/drivers/platform/x86/x86-android-tablets/other.c
@@ -12,6 +12,7 @@
 #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 +598,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_soc_pmic_crc",
+			.addr = 0x6e,
+			.dev_name = "intel_soc_pmic_crc",
+			.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 pci_dev *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 = pci_get_domain_bus_and_slot(0, 0, PCI_DEVFN(0x11, 0));
+	if (!pdev)
+		return -EPROBE_DEFER;
+
+	ret = device_reprobe(&pdev->dev);
+	if (ret)
+		pci_warn(pdev, "Reprobing error: %d\n", ret);
+
+	pci_dev_put(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 v2 1/3] platform/x86: x86-android-tablets: Add get_i2c_adap_by_handle() helper

[Andy Shevchenko]

On Mon, Nov 4, 2024 at 10:09 PM Hans de Goede <hdegoede@redhat.com> wrote:
>
> Add get_i2c_adap_by_handle() helper function, this is a preparation patch
> for adding support for getting i2c_adapter-s by PCI parent devname().

Reviewed-by: Andy Shevchenko <andy@kernel.org>

-- 
With Best Regards,
Andy Shevchenko

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

[Andy Shevchenko]

On Mon, Nov 4, 2024 at 10:09 PM Hans de Goede <hdegoede@redhat.com> 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.

Reviewed-by: Andy Shevchenko <andy@kernel.org>

...

>         struct i2c_board_info board_info = client_info->board_info;
> -       struct i2c_adapter *adap;
> +       struct i2c_adapter *adap = NULL;

Unneeded change.

-- 
With Best Regards,
Andy Shevchenko

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

[Andy Shevchenko]

On Mon, Nov 4, 2024 at 10:09 PM Hans de Goede <hdegoede@redhat.com> 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.
>
> 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.

Reviewed-by: Andy Shevchenko <andy@kernel.org>

...

> +               .adapter_path = "0000:00:18.1",

> +               .adapter_path = "0000:00:18.2",

> +               .adapter_path = "0000:00:18.5",

> +               .adapter_path = "0000:00:18.6",

> +               .adapter_path = "0000:00:18.7",

...

> +       .use_pci_devname = true,

Just realized that you may do something like this to the above and
also use pci_get_...() instead of iterating over the I2C type of
devices. But you can do it later.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 1/3] platform/x86: x86-android-tablets: Add get_i2c_adap_by_handle() helper

[Ilpo Järvinen]

[-- Attachment #1: Type: text/plain, Size: 2595 bytes --]

On Mon, 4 Nov 2024, Hans de Goede wrote:

> Add get_i2c_adap_by_handle() helper function, this is a preparation patch
> for adding support for getting i2c_adapter-s by PCI parent devname().
> 
> Suggested-by: Andy Shevchenko <andy@kernel.org>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
> Changes in v2:
> - New patch in v2 of this series
> ---
>  .../platform/x86/x86-android-tablets/core.c   | 25 ++++++++++++-------
>  1 file changed, 16 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/platform/x86/x86-android-tablets/core.c b/drivers/platform/x86/x86-android-tablets/core.c
> index ef572b90e06b..4154395c60bb 100644
> --- a/drivers/platform/x86/x86-android-tablets/core.c
> +++ b/drivers/platform/x86/x86-android-tablets/core.c
> @@ -155,26 +155,33 @@ static struct gpiod_lookup_table * const *gpiod_lookup_tables;
>  static const struct software_node *bat_swnode;
>  static void (*exit_handler)(void);
>  
> +static struct i2c_adapter *
> +get_i2c_adap_by_handle(const struct x86_i2c_client_info *client_info)
> +{
> +	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 NULL;
> +	}
> +
> +	return i2c_acpi_find_adapter_by_handle(handle);
> +}
> +
>  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;
>  
>  	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;
> -	}
> -
> -	adap = i2c_acpi_find_adapter_by_handle(handle);
> +	adap = get_i2c_adap_by_handle(client_info);
>  	if (!adap) {
>  		pr_err("error could not get %s adapter\n", client_info->adapter_path);
>  		return -ENODEV;

Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>

Not a big deal, but you might want to consider if printing both error 
messages is fine or if the error printing should be somehow modified when 
that other print moves into the inner function.

-- 
 i.

Re: [PATCH v2 1/3] platform/x86: x86-android-tablets: Add get_i2c_adap_by_handle() helper

[Hans de Goede]

Hi Ilpo,

On 5-Nov-24 10:37 AM, Ilpo Järvinen wrote:
> On Mon, 4 Nov 2024, Hans de Goede wrote:
> 
>> Add get_i2c_adap_by_handle() helper function, this is a preparation patch
>> for adding support for getting i2c_adapter-s by PCI parent devname().
>>
>> Suggested-by: Andy Shevchenko <andy@kernel.org>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>> Changes in v2:
>> - New patch in v2 of this series
>> ---
>>  .../platform/x86/x86-android-tablets/core.c   | 25 ++++++++++++-------
>>  1 file changed, 16 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/platform/x86/x86-android-tablets/core.c b/drivers/platform/x86/x86-android-tablets/core.c
>> index ef572b90e06b..4154395c60bb 100644
>> --- a/drivers/platform/x86/x86-android-tablets/core.c
>> +++ b/drivers/platform/x86/x86-android-tablets/core.c
>> @@ -155,26 +155,33 @@ static struct gpiod_lookup_table * const *gpiod_lookup_tables;
>>  static const struct software_node *bat_swnode;
>>  static void (*exit_handler)(void);
>>  
>> +static struct i2c_adapter *
>> +get_i2c_adap_by_handle(const struct x86_i2c_client_info *client_info)
>> +{
>> +	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 NULL;
>> +	}
>> +
>> +	return i2c_acpi_find_adapter_by_handle(handle);
>> +}
>> +
>>  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;
>>  
>>  	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;
>> -	}
>> -
>> -	adap = i2c_acpi_find_adapter_by_handle(handle);
>> +	adap = get_i2c_adap_by_handle(client_info);
>>  	if (!adap) {
>>  		pr_err("error could not get %s adapter\n", client_info->adapter_path);
>>  		return -ENODEV;
> 
> Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> 
> Not a big deal, but you might want to consider if printing both error 
> messages is fine or if the error printing should be somehow modified when 
> that other print moves into the inner function.

I already considered this and since this should never happen
printing both messages if it does happen is fine IMHO.

Basically the idea is if the acpi_get_handle() fails print both
messages, if the i2c_acpi_find_adapter_by_handle() fails only
print the latter message. This way one can tell one scenario
from the other while keeping the logic simple.

The same applies to the get_i2c_adap_by_pci_parent() helper
added in patch 2.

Regards,

Hans

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

[Hans de Goede]

Hi,

On 5-Nov-24 9:22 AM, Andy Shevchenko wrote:
> On Mon, Nov 4, 2024 at 10:09 PM Hans de Goede <hdegoede@redhat.com> 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.
> 
> Reviewed-by: Andy Shevchenko <andy@kernel.org>
> 
> ...
> 
>>         struct i2c_board_info board_info = client_info->board_info;
>> -       struct i2c_adapter *adap;
>> +       struct i2c_adapter *adap = NULL;
> 
> Unneeded change.

Ack I noticed this myself and I thought I had dropped this change,
but apparently I did not.

Ilpo can you fix this up while merging or do you want a v3 series?

Regards,

Hans

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

[Ilpo Järvinen]

[-- Attachment #1: Type: text/plain, Size: 1315 bytes --]

On Tue, 5 Nov 2024, Hans de Goede wrote:

> Hi,
> 
> On 5-Nov-24 9:22 AM, Andy Shevchenko wrote:
> > On Mon, Nov 4, 2024 at 10:09 PM Hans de Goede <hdegoede@redhat.com> 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.
> > 
> > Reviewed-by: Andy Shevchenko <andy@kernel.org>
> > 
> > ...
> > 
> >>         struct i2c_board_info board_info = client_info->board_info;
> >> -       struct i2c_adapter *adap;
> >> +       struct i2c_adapter *adap = NULL;
> > 
> > Unneeded change.
> 
> Ack I noticed this myself and I thought I had dropped this change,
> but apparently I did not.
> 
> Ilpo can you fix this up while merging or do you want a v3 series?

Thanks both.

I removed this change while applying. I also made minor adjustments to the 
braces in the patch 3 for stylish consistency.

-- 
 i.

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

[Hans de Goede]

Hi,

On 6-Nov-24 2:25 PM, Ilpo Järvinen wrote:
> On Tue, 5 Nov 2024, Hans de Goede wrote:
> 
>> Hi,
>>
>> On 5-Nov-24 9:22 AM, Andy Shevchenko wrote:
>>> On Mon, Nov 4, 2024 at 10:09 PM Hans de Goede <hdegoede@redhat.com> 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.
>>>
>>> Reviewed-by: Andy Shevchenko <andy@kernel.org>
>>>
>>> ...
>>>
>>>>         struct i2c_board_info board_info = client_info->board_info;
>>>> -       struct i2c_adapter *adap;
>>>> +       struct i2c_adapter *adap = NULL;
>>>
>>> Unneeded change.
>>
>> Ack I noticed this myself and I thought I had dropped this change,
>> but apparently I did not.
>>
>> Ilpo can you fix this up while merging or do you want a v3 series?
> 
> Thanks both.
> 
> I removed this change while applying. I also made minor adjustments to the 
> braces in the patch 3 for stylish consistency.

Great, Thank you for merging these.

Regards,

Hans