Hi Kai-Heng Feng, On 5/8/24 6:42 AM, Kai-Heng Feng wrote: > [+Cc AceLan] > > Hi Hans, > > On Wed, Apr 24, 2024 at 5:58 PM Hans de Goede <hdegoede@xxxxxxxxxx> wrote: >> >> Hi Kai-Heng Feng, >> >> On 4/24/24 10:04 AM, Kai-Heng Feng wrote: >>> Hi Hans, >>> >>> 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> Great thank you for testing. Luck has it that the user for who's Dell AOI I started working on this also just reported back that the driver works for them :) So I'm going to send out the patch series for this now with the following diff squashed in vs what I send you: diff --git a/drivers/platform/x86/dell/dell-uart-backlight.c b/drivers/platform/x86/dell/dell-uart-backlight.c index da4a640c0d88..3882bb7d6c71 100644 --- a/drivers/platform/x86/dell/dell-uart-backlight.c +++ b/drivers/platform/x86/dell/dell-uart-backlight.c @@ -352,7 +352,7 @@ static int dell_uart_bl_pdev_probe(struct platform_device *pdev) struct device *ctrl_dev; int ret; - ctrl_dev = get_serdev_controller("DELL0501", "0", 0, "serial0"); + ctrl_dev = get_serdev_controller("DELL0501", NULL, 0, "serial0"); if (IS_ERR(ctrl_dev)) return PTR_ERR(ctrl_dev); Regards, Hans
