[PATCH 0/4] platform/x86: x86-android-tablets: 2 regression fixes

[PATCH 0/4] platform/x86: x86-android-tablets: 2 regression fixes

[Hans de Goede]

Hi All,

This series fixes 2 recent regressions in the x86-android-tablets code

Patch 1: Fixes the Goodix touchscreen in the Lenovo YogaBook1 X90
         no longer working in 6.7 and 6.8
Patch 2: Is a prep patch for manual serdev instantation failing on 6.8
Patch 3: Fixes manual serdev instantation failing on 6.8
Patch 4: Is just a rename of a single variable

Patches 1 - 3 are actual regression fixes and patch 4 although not
technically a bugfix is trivial. So to keep things simple I plan
to merge this entire series through the fixes branch.

Regards,

Hans


Hans de Goede (4):
  platform/x86: x86-android-tablets: Fix keyboard touchscreen on Lenovo
    Yogabook1 X90
  platform/x86: Add new get_serdev_controller() helper
  platform/x86: x86-android-tablets: Fix serdev instantiation no longer
    working
  platform/x86: x86-android-tablets: Fix acer_b1_750_goodix_gpios name

 drivers/platform/x86/serdev_helpers.h         | 77 +++++++++++++++++++
 .../platform/x86/x86-android-tablets/core.c   | 38 +++------
 .../platform/x86/x86-android-tablets/lenovo.c |  1 +
 .../platform/x86/x86-android-tablets/other.c  |  4 +-
 .../x86-android-tablets/x86-android-tablets.h |  1 +
 5 files changed, 93 insertions(+), 28 deletions(-)
 create mode 100644 drivers/platform/x86/serdev_helpers.h

-- 
2.43.0

[PATCH 1/4] platform/x86: x86-android-tablets: Fix keyboard touchscreen on Lenovo Yogabook1 X90

[Hans de Goede]

After commit 4014ae236b1d ("platform/x86: x86-android-tablets: Stop using
gpiolib private APIs") the touchscreen in the keyboard half of
the Lenovo Yogabook1 X90 stopped working with the following error:

 Goodix-TS i2c-goodix_ts: error -EBUSY: Failed to get irq GPIO

The problem is that when getting the IRQ for instantiated i2c_client-s
from a GPIO (rather then using an IRQ directly from the IOAPIC),
x86_acpi_irq_helper_get() now properly requests the GPIO, which disallows
other drivers from requesting it. Normally this is a good thing, but
the goodix touchscreen also uses the IRQ as an output during reset
to select which of its 2 possible I2C addresses should be used.

Add a new free_gpio flag to struct x86_acpi_irq_data to deal with this
and release the GPIO after getting the IRQ in this special case.

Fixes: 4014ae236b1d ("platform/x86: x86-android-tablets: Stop using gpiolib private APIs")
Cc: stable@vger.kernel.org
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/platform/x86/x86-android-tablets/core.c                | 3 +++
 drivers/platform/x86/x86-android-tablets/lenovo.c              | 1 +
 drivers/platform/x86/x86-android-tablets/x86-android-tablets.h | 1 +
 3 files changed, 5 insertions(+)

diff --git a/drivers/platform/x86/x86-android-tablets/core.c b/drivers/platform/x86/x86-android-tablets/core.c
index f8221a15575b..f6547c9d7584 100644
--- a/drivers/platform/x86/x86-android-tablets/core.c
+++ b/drivers/platform/x86/x86-android-tablets/core.c
@@ -113,6 +113,9 @@ int x86_acpi_irq_helper_get(const struct x86_acpi_irq_data *data)
 		if (irq_type != IRQ_TYPE_NONE && irq_type != irq_get_trigger_type(irq))
 			irq_set_irq_type(irq, irq_type);
 
+		if (data->free_gpio)
+			devm_gpiod_put(&x86_android_tablet_device->dev, gpiod);
+
 		return irq;
 	case X86_ACPI_IRQ_TYPE_PMIC:
 		status = acpi_get_handle(NULL, data->chip, &handle);
diff --git a/drivers/platform/x86/x86-android-tablets/lenovo.c b/drivers/platform/x86/x86-android-tablets/lenovo.c
index f1c66a61bfc5..c297391955ad 100644
--- a/drivers/platform/x86/x86-android-tablets/lenovo.c
+++ b/drivers/platform/x86/x86-android-tablets/lenovo.c
@@ -116,6 +116,7 @@ static const struct x86_i2c_client_info lenovo_yb1_x90_i2c_clients[] __initconst
 			.trigger = ACPI_EDGE_SENSITIVE,
 			.polarity = ACPI_ACTIVE_LOW,
 			.con_id = "goodix_ts_irq",
+			.free_gpio = true,
 		},
 	}, {
 		/* Wacom Digitizer in keyboard half */
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 49fed9410adb..468993edfeee 100644
--- a/drivers/platform/x86/x86-android-tablets/x86-android-tablets.h
+++ b/drivers/platform/x86/x86-android-tablets/x86-android-tablets.h
@@ -39,6 +39,7 @@ struct x86_acpi_irq_data {
 	int index;
 	int trigger;  /* ACPI_EDGE_SENSITIVE / ACPI_LEVEL_SENSITIVE */
 	int polarity; /* ACPI_ACTIVE_HIGH / ACPI_ACTIVE_LOW / ACPI_ACTIVE_BOTH */
+	bool free_gpio; /* Release GPIO after getting IRQ (for TYPE_GPIOINT) */
 	const char *con_id;
 };
 
-- 
2.43.0

[PATCH 2/4] platform/x86: Add new get_serdev_controller() helper

[Hans de Goede]

In some cases UART attached devices which require an in kernel driver,
e.g. UART attached Bluetooth HCIs are described in the ACPI tables
by an ACPI device with a broken or missing UartSerialBusV2() resource.

This causes the kernel to create a /dev/ttyS# char-device for the UART
instead of creating an in kernel serdev-controller + serdev-device pair
for the in kernel driver.

The quirk handling in acpi_quirk_skip_serdev_enumeration() makes the kernel
create a serdev-controller device for these UARTs instead of a /dev/ttyS#.

Instantiating the actual serdev-device to bind to is up to pdx86 code,
so far this was handled by the x86-android-tablets code. But since
commit b286f4e87e32 ("serial: core: Move tty and serdev to be children of
serial core port device") the serdev-controller device has moved in the
device hierarchy from (e.g.) /sys/devices/pci0000:00/8086228A:00/serial0 to
/sys/devices/pci0000:00/8086228A:00/8086228A:00:0/8086228A:00:0.0/serial0 .

This makes this a bit trickier to do and another driver is in the works
which will also need this functionality.

Add a new helper to get the serdev-controller device, so that the new
code for this can be shared.

Fixes: b286f4e87e32 ("serial: core: Move tty and serdev to be children of serial core port device")
Cc: Tony Lindgren <tony@atomide.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/platform/x86/serdev_helpers.h | 77 +++++++++++++++++++++++++++
 1 file changed, 77 insertions(+)
 create mode 100644 drivers/platform/x86/serdev_helpers.h

diff --git a/drivers/platform/x86/serdev_helpers.h b/drivers/platform/x86/serdev_helpers.h
new file mode 100644
index 000000000000..825979f6736b
--- /dev/null
+++ b/drivers/platform/x86/serdev_helpers.h
@@ -0,0 +1,77 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ * In some cases UART attached devices which require an in kernel driver,
+ * e.g. UART attached Bluetooth HCIs are described in the ACPI tables
+ * by an ACPI device with a broken or missing UartSerialBusV2() resource.
+ *
+ * This causes the kernel to create a /dev/ttyS# char-device for the UART
+ * instead of creating an in kernel serdev-controller + serdev-device pair
+ * for the in kernel driver.
+ *
+ * The quirk handling in acpi_quirk_skip_serdev_enumeration() makes the kernel
+ * create a serdev-controller device for these UARTs instead of a /dev/ttyS#.
+ *
+ * Instantiating the actual serdev-device to bind to is up to pdx86 code,
+ * this header provides a helper for getting the serdev-controller device.
+ */
+#include <linux/acpi.h>
+#include <linux/device.h>
+#include <linux/printk.h>
+
+static inline struct device *
+get_serdev_controller(const char *serial_ctrl_hid,
+		      const char *serial_ctrl_uid,
+		      int serial_ctrl_port,
+		      const char *serdev_ctrl_name)
+{
+	struct device *ctrl_dev, *child;
+	struct acpi_device *ctrl_adev;
+	char name[32];
+	int i;
+
+	ctrl_adev = acpi_dev_get_first_match_dev(serial_ctrl_hid, serial_ctrl_uid, -1);
+	if (!ctrl_adev) {
+		pr_err("error could not get %s/%s serial-ctrl adev\n",
+		       serial_ctrl_hid, serial_ctrl_uid);
+		return ERR_PTR(-ENODEV);
+	}
+
+	/* get_first_physical_node() returns a weak ref */
+	ctrl_dev = get_device(acpi_get_first_physical_node(ctrl_adev));
+	if (!ctrl_dev) {
+		pr_err("error could not get %s/%s serial-ctrl physical node\n",
+		       serial_ctrl_hid, serial_ctrl_uid);
+		ctrl_dev = ERR_PTR(-ENODEV);
+		goto put_ctrl_adev;
+	}
+
+	/* Walk host -> uart-ctrl -> port -> serdev-ctrl */
+	for (i = 0; i < 3; i++) {
+		switch (i) {
+		case 0:
+			snprintf(name, sizeof(name), "%s:0", dev_name(ctrl_dev));
+			break;
+		case 1:
+			snprintf(name, sizeof(name), "%s.%d",
+				 dev_name(ctrl_dev), serial_ctrl_port);
+			break;
+		case 2:
+			strscpy(name, serdev_ctrl_name, sizeof(name));
+			break;
+		}
+
+		child = device_find_child_by_name(ctrl_dev, name);
+		put_device(ctrl_dev);
+		if (!child) {
+			pr_err("error could not find '%s' device\n", name);
+			ctrl_dev = ERR_PTR(-ENODEV);
+			goto put_ctrl_adev;
+		}
+
+		ctrl_dev = child;
+	}
+
+put_ctrl_adev:
+	acpi_dev_put(ctrl_adev);
+	return ctrl_dev;
+}
-- 
2.43.0

[PATCH 3/4] platform/x86: x86-android-tablets: Fix serdev instantiation no longer working

[Hans de Goede]

After commit b286f4e87e32 ("serial: core: Move tty and serdev to be
children of serial core port device") x86_instantiate_serdev() no longer
works due to the serdev-controller-device moving in the device hierarchy
from (e.g.) /sys/devices/pci0000:00/8086228A:00/serial0 to
/sys/devices/pci0000:00/8086228A:00/8086228A:00:0/8086228A:00:0.0/serial0

Use the new get_serdev_controller() helper function to fix this.

Fixes: b286f4e87e32 ("serial: core: Move tty and serdev to be children of serial core port device")
Cc: Tony Lindgren <tony@atomide.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 .../platform/x86/x86-android-tablets/core.c   | 35 +++++--------------
 1 file changed, 9 insertions(+), 26 deletions(-)

diff --git a/drivers/platform/x86/x86-android-tablets/core.c b/drivers/platform/x86/x86-android-tablets/core.c
index f6547c9d7584..a3415f1c0b5f 100644
--- a/drivers/platform/x86/x86-android-tablets/core.c
+++ b/drivers/platform/x86/x86-android-tablets/core.c
@@ -21,6 +21,7 @@
 #include <linux/string.h>
 
 #include "x86-android-tablets.h"
+#include "../serdev_helpers.h"
 
 static struct platform_device *x86_android_tablet_device;
 
@@ -232,38 +233,20 @@ static __init int x86_instantiate_spi_dev(const struct x86_dev_info *dev_info, i
 
 static __init int x86_instantiate_serdev(const struct x86_serdev_info *info, int idx)
 {
-	struct acpi_device *ctrl_adev, *serdev_adev;
+	struct acpi_device *serdev_adev;
 	struct serdev_device *serdev;
 	struct device *ctrl_dev;
 	int ret = -ENODEV;
 
-	ctrl_adev = acpi_dev_get_first_match_dev(info->ctrl_hid, info->ctrl_uid, -1);
-	if (!ctrl_adev) {
-		pr_err("error could not get %s/%s ctrl adev\n",
-		       info->ctrl_hid, info->ctrl_uid);
-		return -ENODEV;
-	}
+	ctrl_dev = get_serdev_controller(info->ctrl_hid, info->ctrl_uid, 0,
+					 info->ctrl_devname);
+	if (IS_ERR(ctrl_dev))
+		return PTR_ERR(ctrl_dev);
 
 	serdev_adev = acpi_dev_get_first_match_dev(info->serdev_hid, NULL, -1);
 	if (!serdev_adev) {
 		pr_err("error could not get %s serdev adev\n", info->serdev_hid);
-		goto put_ctrl_adev;
-	}
-
-	/* get_first_physical_node() returns a weak ref, no need to put() it */
-	ctrl_dev = acpi_get_first_physical_node(ctrl_adev);
-	if (!ctrl_dev)	{
-		pr_err("error could not get %s/%s ctrl physical dev\n",
-		       info->ctrl_hid, info->ctrl_uid);
-		goto put_serdev_adev;
-	}
-
-	/* ctrl_dev now points to the controller's parent, get the controller */
-	ctrl_dev = device_find_child_by_name(ctrl_dev, info->ctrl_devname);
-	if (!ctrl_dev) {
-		pr_err("error could not get %s/%s %s ctrl dev\n",
-		       info->ctrl_hid, info->ctrl_uid, info->ctrl_devname);
-		goto put_serdev_adev;
+		goto put_ctrl_dev;
 	}
 
 	serdev = serdev_device_alloc(to_serdev_controller(ctrl_dev));
@@ -286,8 +269,8 @@ static __init int x86_instantiate_serdev(const struct x86_serdev_info *info, int
 
 put_serdev_adev:
 	acpi_dev_put(serdev_adev);
-put_ctrl_adev:
-	acpi_dev_put(ctrl_adev);
+put_ctrl_dev:
+	put_device(ctrl_dev);
 	return ret;
 }
 
-- 
2.43.0

[PATCH 4/4] platform/x86: x86-android-tablets: Fix acer_b1_750_goodix_gpios name

[Hans de Goede]

The Acer B1 750 tablet used a Novatek NVT-ts touchscreen,
not a Goodix touchscreen.

Rename acer_b1_750_goodix_gpios to acer_b1_750_nvt_ts_gpios
to correctly reflect this.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/platform/x86/x86-android-tablets/other.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/platform/x86/x86-android-tablets/other.c b/drivers/platform/x86/x86-android-tablets/other.c
index bc6bbf7ec6ea..278402dcb808 100644
--- a/drivers/platform/x86/x86-android-tablets/other.c
+++ b/drivers/platform/x86/x86-android-tablets/other.c
@@ -68,7 +68,7 @@ static const struct x86_i2c_client_info acer_b1_750_i2c_clients[] __initconst =
 	},
 };
 
-static struct gpiod_lookup_table acer_b1_750_goodix_gpios = {
+static struct gpiod_lookup_table acer_b1_750_nvt_ts_gpios = {
 	.dev_id = "i2c-NVT-ts",
 	.table = {
 		GPIO_LOOKUP("INT33FC:01", 26, "reset", GPIO_ACTIVE_LOW),
@@ -77,7 +77,7 @@ static struct gpiod_lookup_table acer_b1_750_goodix_gpios = {
 };
 
 static struct gpiod_lookup_table * const acer_b1_750_gpios[] = {
-	&acer_b1_750_goodix_gpios,
+	&acer_b1_750_nvt_ts_gpios,
 	&int3496_reference_gpios,
 	NULL
 };
-- 
2.43.0

Re: [PATCH 2/4] platform/x86: Add new get_serdev_controller() helper

[Andy Shevchenko]

On Fri, Feb 16, 2024 at 09:17:19PM +0100, Hans de Goede wrote:
> In some cases UART attached devices which require an in kernel driver,
> e.g. UART attached Bluetooth HCIs are described in the ACPI tables
> by an ACPI device with a broken or missing UartSerialBusV2() resource.
> 
> This causes the kernel to create a /dev/ttyS# char-device for the UART
> instead of creating an in kernel serdev-controller + serdev-device pair
> for the in kernel driver.
> 
> The quirk handling in acpi_quirk_skip_serdev_enumeration() makes the kernel
> create a serdev-controller device for these UARTs instead of a /dev/ttyS#.
> 
> Instantiating the actual serdev-device to bind to is up to pdx86 code,
> so far this was handled by the x86-android-tablets code. But since
> commit b286f4e87e32 ("serial: core: Move tty and serdev to be children of
> serial core port device") the serdev-controller device has moved in the
> device hierarchy from (e.g.) /sys/devices/pci0000:00/8086228A:00/serial0 to
> /sys/devices/pci0000:00/8086228A:00/8086228A:00:0/8086228A:00:0.0/serial0 .
> 
> This makes this a bit trickier to do and another driver is in the works
> which will also need this functionality.
> 
> Add a new helper to get the serdev-controller device, so that the new
> code for this can be shared.

The above doesn't explain why the new code is h-file.

...

> +#include <linux/acpi.h>

+ err.h

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

+ sprintf.h
+ string.h

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 3/4] platform/x86: x86-android-tablets: Fix serdev instantiation no longer working

[Andy Shevchenko]

On Fri, Feb 16, 2024 at 09:17:20PM +0100, Hans de Goede wrote:
> After commit b286f4e87e32 ("serial: core: Move tty and serdev to be
> children of serial core port device") x86_instantiate_serdev() no longer
> works due to the serdev-controller-device moving in the device hierarchy
> from (e.g.) /sys/devices/pci0000:00/8086228A:00/serial0 to
> /sys/devices/pci0000:00/8086228A:00/8086228A:00:0/8086228A:00:0.0/serial0
> 
> Use the new get_serdev_controller() helper function to fix this.

...

>  #include "x86-android-tablets.h"

> +#include "../serdev_helpers.h"

Seems Q more to the previous patch, why is this in the upper folder? Are we
expecting more users?

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 2/4] platform/x86: Add new get_serdev_controller() helper

[Hans de Goede]

Hi Andy,

On 2/16/24 22:24, Andy Shevchenko wrote:
> On Fri, Feb 16, 2024 at 09:17:19PM +0100, Hans de Goede wrote:
>> In some cases UART attached devices which require an in kernel driver,
>> e.g. UART attached Bluetooth HCIs are described in the ACPI tables
>> by an ACPI device with a broken or missing UartSerialBusV2() resource.
>>
>> This causes the kernel to create a /dev/ttyS# char-device for the UART
>> instead of creating an in kernel serdev-controller + serdev-device pair
>> for the in kernel driver.
>>
>> The quirk handling in acpi_quirk_skip_serdev_enumeration() makes the kernel
>> create a serdev-controller device for these UARTs instead of a /dev/ttyS#.
>>
>> Instantiating the actual serdev-device to bind to is up to pdx86 code,
>> so far this was handled by the x86-android-tablets code. But since
>> commit b286f4e87e32 ("serial: core: Move tty and serdev to be children of
>> serial core port device") the serdev-controller device has moved in the
>> device hierarchy from (e.g.) /sys/devices/pci0000:00/8086228A:00/serial0 to
>> /sys/devices/pci0000:00/8086228A:00/8086228A:00:0/8086228A:00:0.0/serial0 .
>>
>> This makes this a bit trickier to do and another driver is in the works
>> which will also need this functionality.
>>
>> Add a new helper to get the serdev-controller device, so that the new
>> code for this can be shared.
> 
> The above doesn't explain why the new code is h-file.

It is in a h file because as metioned: "another driver is in the works"
which will also need this.

And the code is large/complicated enough that I don't want to copy
and paste it. Yet small enough that it would be silly to put it
in its own .ko file.

Regards,

Hans

p.s.

About the other driver. I recently learned that some Dell AIOs (1) use
a backlight controller board connected to an UART. Canonical even
submitted a driver for this in 2017, but never followed-up on getting 
it merged:
https://lkml.org/lkml/2017/10/26/78

This UART has a DELL0501 HID with CID set to PNP0501 so that the UART is
still handled by 8250_pnp.c. Unfortunately there is no separate ACPI device
with an UartSerialBusV2() resource to model the backlight-controller.

My patch series for this will use acpi_quirk_skip_serdev_enumeration()
to still create a serdev for this for a backlight driver to bind to
instead of creating a /dev/ttyS0.

Like other cases where the UartSerialBusV2() resource is missing or broken
this will only create the serdev-controller device and the serdev-device
itself will need to be instantiated by a pdx86 driver. This driver will
use this new helper to create the serdev-device (client) itself.

1) All In One a monitor with a PC builtin

Re: [PATCH 2/4] platform/x86: Add new get_serdev_controller() helper

[Andy Shevchenko]

On Sat, Feb 17, 2024 at 12:36 AM Hans de Goede <hdegoede@redhat.com> wrote:
> On 2/16/24 22:24, Andy Shevchenko wrote:
> > On Fri, Feb 16, 2024 at 09:17:19PM +0100, Hans de Goede wrote:

...

> > The above doesn't explain why the new code is h-file.
>
> It is in a h file because as metioned: "another driver is in the works"
> which will also need this.

Implied, but quite unclear. Can you rephrase?

> And the code is large/complicated enough that I don't want to copy
> and paste it. Yet small enough that it would be silly to put it
> in its own .ko file.

We have even smaller code in the separate module. So I don't consider
this as a strong argument.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 2/4] platform/x86: Add new get_serdev_controller() helper

[Hans de Goede]

Hi Andy,

On 2/17/24 19:09, Andy Shevchenko wrote:
> On Sat, Feb 17, 2024 at 12:36 AM Hans de Goede <hdegoede@redhat.com> wrote:
>> On 2/16/24 22:24, Andy Shevchenko wrote:
>>> On Fri, Feb 16, 2024 at 09:17:19PM +0100, Hans de Goede wrote:
> 
> ...
> 
>>> The above doesn't explain why the new code is h-file.
>>
>> It is in a h file because as metioned: "another driver is in the works"
>> which will also need this.
> 
> Implied, but quite unclear. Can you rephrase?

Ack will do and I'll also add the missing headers which
you pointed out.

Regards,

Hans

Re: [PATCH 0/4] platform/x86: x86-android-tablets: 2 regression fixes

[Hans de Goede]

Hi,

On 2/16/24 21:17, Hans de Goede wrote:
> Hi All,
> 
> This series fixes 2 recent regressions in the x86-android-tablets code
> 
> Patch 1: Fixes the Goodix touchscreen in the Lenovo YogaBook1 X90
>          no longer working in 6.7 and 6.8
> Patch 2: Is a prep patch for manual serdev instantation failing on 6.8
> Patch 3: Fixes manual serdev instantation failing on 6.8
> Patch 4: Is just a rename of a single variable
> 
> Patches 1 - 3 are actual regression fixes and patch 4 although not
> technically a bugfix is trivial. So to keep things simple I plan
> to merge this entire series through the fixes branch.

I've added these to my review-hans (soon to be fixes) branch now.

Regards,

Hans




> Hans de Goede (4):
>   platform/x86: x86-android-tablets: Fix keyboard touchscreen on Lenovo
>     Yogabook1 X90
>   platform/x86: Add new get_serdev_controller() helper
>   platform/x86: x86-android-tablets: Fix serdev instantiation no longer
>     working
>   platform/x86: x86-android-tablets: Fix acer_b1_750_goodix_gpios name
> 
>  drivers/platform/x86/serdev_helpers.h         | 77 +++++++++++++++++++
>  .../platform/x86/x86-android-tablets/core.c   | 38 +++------
>  .../platform/x86/x86-android-tablets/lenovo.c |  1 +
>  .../platform/x86/x86-android-tablets/other.c  |  4 +-
>  .../x86-android-tablets/x86-android-tablets.h |  1 +
>  5 files changed, 93 insertions(+), 28 deletions(-)
>  create mode 100644 drivers/platform/x86/serdev_helpers.h
> 

Re: [PATCH 2/4] platform/x86: Add new get_serdev_controller() helper

[Tony Lindgren]

* Hans de Goede <hdegoede@redhat.com> [240216 20:17]:
> +static inline struct device *
> +get_serdev_controller(const char *serial_ctrl_hid,
> +		      const char *serial_ctrl_uid,
> +		      int serial_ctrl_port,
> +		      const char *serdev_ctrl_name)
> +{
> +	struct device *ctrl_dev, *child;
> +	struct acpi_device *ctrl_adev;
> +	char name[32];
> +	int i;
> +
> +	ctrl_adev = acpi_dev_get_first_match_dev(serial_ctrl_hid, serial_ctrl_uid, -1);
> +	if (!ctrl_adev) {
> +		pr_err("error could not get %s/%s serial-ctrl adev\n",
> +		       serial_ctrl_hid, serial_ctrl_uid);
> +		return ERR_PTR(-ENODEV);
> +	}

Maybe split get_serdev_controller() into two functions, an acpi specific
function, and a serdev specific function? And I also think these should
not be in the header file, maybe splitting will also help with that :)

> +	/* get_first_physical_node() returns a weak ref */
> +	ctrl_dev = get_device(acpi_get_first_physical_node(ctrl_adev));
> +	if (!ctrl_dev) {
> +		pr_err("error could not get %s/%s serial-ctrl physical node\n",
> +		       serial_ctrl_hid, serial_ctrl_uid);
> +		ctrl_dev = ERR_PTR(-ENODEV);
> +		goto put_ctrl_adev;
> +	}
> +
> +	/* Walk host -> uart-ctrl -> port -> serdev-ctrl */
> +	for (i = 0; i < 3; i++) {
> +		switch (i) {
> +		case 0:
> +			snprintf(name, sizeof(name), "%s:0", dev_name(ctrl_dev));

Note that in theory it's possible we will encounter a device that has
multiple serial core controller instances as noted by Jiri earlier.

And each controller may have one or more ports. For the multiport test
case, you can use qemu with options like below FYI:

-chardev null,id=s1 -chardev null,id=s2 \
-device pci-serial-2x,chardev1=s1,chardev2=s2

Regards,

Tony