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

From: Hans de Goede <hdegoede@redhat.com>
To: Andy Shevchenko <andy@kernel.org>
Cc: "Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>,
	platform-driver-x86@vger.kernel.org,
	"Tony Lindgren" <tony@atomide.com>
Subject: Re: [PATCH 2/4] platform/x86: Add new get_serdev_controller() helper
Date: Fri, 16 Feb 2024 23:36:25 +0100	[thread overview]
Message-ID: <774d159d-0822-4205-b214-95ffab03a988@redhat.com> (raw)
In-Reply-To: <Zc_Sf73kfss-c2TD@smile.fi.intel.com>

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