Hi Andy, On 5/8/24 11:43 AM, Andy Shevchenko wrote: > On Wed, May 08, 2024 at 12:42:05PM +0800, Kai-Heng Feng wrote: >> [+Cc AceLan] >> On Wed, Apr 24, 2024 at 5:58 PM Hans de Goede <hdegoede@xxxxxxxxxx> wrote: >>> On 4/24/24 10:04 AM, Kai-Heng Feng wrote: >>>> On Sun, Feb 18, 2024 at 11:15 PM Hans de Goede <hdegoede@xxxxxxxxxx> wrote: >>>>> >>>>> Hi Rafael, >>>>> >>>>> 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: >>>>> 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. >>>>> >>>>> The RFC patch 2/2 in this series uses 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 the consumer (the backlight driver). >>>>> >>>>> Unlike existing other cases which use DMI modaliases to load on a specific >>>>> board to work around brokeness of that board's specific ACPI tables, the >>>>> intend here is to have a single driver for all Dell AIOs using the DELL0501 >>>>> HID for their UART, without needing to maintain a list of DMI matches. >>>>> >>>>> This means that the dell-uart-backlight driver will need something to bind >>>>> to. The original driver from 2017 used an acpi_driver for this matching on >>>>> and binding to the DELL0501 acpi_device. >>>>> >>>>> AFAIK you are trying to get rid of having drivers bind directly to >>>>> acpi_device-s so I assume that you don't want me to introduce a new one. >>>>> So to get a device to bind to without introducing a new acpi_driver >>>>> patch 2/2 if this series creates a platform_device for this. >>>>> >>>>> The creation of this platform_device is why this is marked as RFC, >>>>> if you are ok with this solution I guess you can merge this series >>>>> already as is. With the caveat that the matching dell-uart-backlight >>>>> driver is still under development (its progressing nicely and the >>>>> serdev-device instantation + binding a serdev driver to it already >>>>> works). >>>> >>>> I was about to work on this and found you're already working on it. >>>> >>>> Please add me to Cc list when the driver is ready to be tested, thanks! >>> >>> I hope you have access to actual hw with such a backlight device ? >>> >>> The driver actually has been ready for testing for quite a while now, >>> but the person who reported this backlight controller not being >>> supported to me has been testing this on a AIO of a friend of theirs >>> and this has been going pretty slow. >>> >>> So if you can test the driver (attached) then that would be great :) >>> >>> I even wrote an emulator to test it locally and that works, so >>> assuming I got the protocol right from the original posting of >>> the driver for this years ago then things should work. >>> >>> Note this depends on the kernel also having the patches from this >>> RFC (which Rafael has already merged) applied. >> >> There are newer AIO have UID other than 0, like "SIOBUAR2". >> >> Once change the "0" to NULL in 'get_serdev_controller("DELL0501", "0", >> 0, "serial0");', everything works perfectly. >> >> With that change, >> Tested-by: Kai-Heng Feng <kai.heng.feng@xxxxxxxxxxxxx> > > Do we have tables with _UID set to 0? > If so, we would need more complex approach. passing NULL as uid argument to get_serdev_controller() makes it skip the UID check altogether and there is no reaosn to expect the special "DELL0501" HID to be attached to more then 1 uart, so this should be fine. Regards, Hans
