| Message ID | 20240218151533.5720-1-hdegoede@redhat.com |
|---|---|
| State | New |
| Headers | show |
Hi Hans, On Sun, Feb 18, 2024 at 4:15 PM Hans de Goede <hdegoede@redhat.com> 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. OK > 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). > > If you have a different idea how to handle this I'm certainly open > to suggestions. I agree with the approach, thanks!
[+Cc AceLan] Hi Hans, On Wed, Apr 24, 2024 at 5:58 PM Hans de Goede <hdegoede@redhat.com> 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@redhat.com> 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@canonical.com> Kai-Heng > > Regards, > > Hans > > > > > > > >> If you have a different idea how to handle this I'm certainly open > >> to suggestions. > >> > >> Regards, > >> > >> Hans > >> > >> 1) All In One a monitor with a PC builtin > >> > >> > >> p.s. > >> > >> I also tried this approach, but that did not work: > >> > >> This was an attempt to create both a pdev from acpi_default_enumeration() > >> by making the PNP scan handler attach() method return 0 rather then 1; > >> and get a pnp_device created for the UART driver as well by > >> making acpi_is_pnp_device() return true. > >> > >> This approach does not work due to the following code in pnpacpi_add_device(): > >> > >> /* Skip devices that are already bound */ > >> if (device->physical_node_count) > >> return 0; > >> > >> diff --git a/drivers/acpi/acpi_pnp.c b/drivers/acpi/acpi_pnp.c > >> index 01abf26764b0..847c08deea7b 100644 > >> --- a/drivers/acpi/acpi_pnp.c > >> +++ b/drivers/acpi/acpi_pnp.c > >> @@ -353,10 +353,17 @@ static bool acpi_pnp_match(const char *idstr, const struct acpi_device_id **matc > >> * given ACPI device object, the PNP scan handler will not attach to that > >> * object, because there is a proper non-PNP driver in the kernel for the > >> * device represented by it. > >> + * > >> + * The DELL0501 ACPI HID represents an UART (CID is set to PNP0501) with > >> + * a backlight-controller attached. There is no separate ACPI device with > >> + * an UartSerialBusV2() resource to model the backlight-controller. > >> + * This setup requires instantiating both a pnp_device for the UART as well > >> + * as a platform_device for the backlight-controller driver to bind too. > >> */ > >> static const struct acpi_device_id acpi_nonpnp_device_ids[] = { > >> {"INTC1080"}, > >> {"INTC1081"}, > >> + {"DELL0501"}, > >> {""}, > >> }; > >> > >> @@ -376,13 +383,16 @@ static struct acpi_scan_handler acpi_pnp_handler = { > >> * For CMOS RTC devices, the PNP ACPI scan handler does not work, because > >> * there is a CMOS RTC ACPI scan handler installed already, so we need to > >> * check those devices and enumerate them to the PNP bus directly. > >> + * For DELL0501 devices the PNP ACPI scan handler is skipped to create > >> + * a platform_device, see the acpi_nonpnp_device_ids[] comment. > >> */ > >> -static int is_cmos_rtc_device(struct acpi_device *adev) > >> +static int is_special_pnp_device(struct acpi_device *adev) > >> { > >> static const struct acpi_device_id ids[] = { > >> { "PNP0B00" }, > >> { "PNP0B01" }, > >> { "PNP0B02" }, > >> + { "DELL0501" }, > >> {""}, > >> }; > >> return !acpi_match_device_ids(adev, ids); > >> @@ -390,7 +400,7 @@ static int is_cmos_rtc_device(struct acpi_device *adev) > >> > >> bool acpi_is_pnp_device(struct acpi_device *adev) > >> { > >> - return adev->handler == &acpi_pnp_handler || is_cmos_rtc_device(adev); > >> + return adev->handler == &acpi_pnp_handler || is_special_pnp_device(adev); > >> } > >> EXPORT_SYMBOL_GPL(acpi_is_pnp_device); > >> > >> > >> Hans de Goede (2): > >> ACPI: x86: Move acpi_quirk_skip_serdev_enumeration() out of > >> CONFIG_X86_ANDROID_TABLETS > >> ACPI: x86: Add DELL0501 handling to > >> acpi_quirk_skip_serdev_enumeration() > >> > >> drivers/acpi/x86/utils.c | 38 ++++++++++++++++++++++++++++++++++---- > >> include/acpi/acpi_bus.h | 22 +++++++++++----------- > >> 2 files changed, 45 insertions(+), 15 deletions(-) > >> > >> -- > >> 2.43.0 > >> > >
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@redhat.com> 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@redhat.com> 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@canonical.com> Do we have tables with _UID set to 0? If so, we would need more complex approach.
On Wed, May 8, 2024 at 5:44 PM Andy Shevchenko <andy@kernel.org> 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@redhat.com> 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@redhat.com> 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@canonical.com> > > Do we have tables with _UID set to 0? > If so, we would need more complex approach. Yes, some tables have _UID set to 0 and some have other _UID values. Kai-Heng > > -- > With Best Regards, > Andy Shevchenko > >
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@redhat.com> 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@redhat.com> 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@canonical.com> > > 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
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@redhat.com> 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@redhat.com> 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@canonical.com> 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
diff --git a/drivers/acpi/acpi_pnp.c b/drivers/acpi/acpi_pnp.c index 01abf26764b0..847c08deea7b 100644 --- a/drivers/acpi/acpi_pnp.c +++ b/drivers/acpi/acpi_pnp.c @@ -353,10 +353,17 @@ static bool acpi_pnp_match(const char *idstr, const struct acpi_device_id **matc * given ACPI device object, the PNP scan handler will not attach to that * object, because there is a proper non-PNP driver in the kernel for the * device represented by it. + * + * The DELL0501 ACPI HID represents an UART (CID is set to PNP0501) with + * a backlight-controller attached. There is no separate ACPI device with + * an UartSerialBusV2() resource to model the backlight-controller. + * This setup requires instantiating both a pnp_device for the UART as well + * as a platform_device for the backlight-controller driver to bind too. */ static const struct acpi_device_id acpi_nonpnp_device_ids[] = { {"INTC1080"}, {"INTC1081"}, + {"DELL0501"}, {""}, }; @@ -376,13 +383,16 @@ static struct acpi_scan_handler acpi_pnp_handler = { * For CMOS RTC devices, the PNP ACPI scan handler does not work, because * there is a CMOS RTC ACPI scan handler installed already, so we need to * check those devices and enumerate them to the PNP bus directly. + * For DELL0501 devices the PNP ACPI scan handler is skipped to create + * a platform_device, see the acpi_nonpnp_device_ids[] comment. */ -static int is_cmos_rtc_device(struct acpi_device *adev) +static int is_special_pnp_device(struct acpi_device *adev) { static const struct acpi_device_id ids[] = { { "PNP0B00" }, { "PNP0B01" }, { "PNP0B02" }, + { "DELL0501" }, {""}, }; return !acpi_match_device_ids(adev, ids); @@ -390,7 +400,7 @@ static int is_cmos_rtc_device(struct acpi_device *adev) bool acpi_is_pnp_device(struct acpi_device *adev) { - return adev->handler == &acpi_pnp_handler || is_cmos_rtc_device(adev); + return adev->handler == &acpi_pnp_handler || is_special_pnp_device(adev); } EXPORT_SYMBOL_GPL(acpi_is_pnp_device);
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). If you have a different idea how to handle this I'm certainly open to suggestions. Regards, Hans 1) All In One a monitor with a PC builtin p.s. I also tried this approach, but that did not work: This was an attempt to create both a pdev from acpi_default_enumeration() by making the PNP scan handler attach() method return 0 rather then 1; and get a pnp_device created for the UART driver as well by making acpi_is_pnp_device() return true. This approach does not work due to the following code in pnpacpi_add_device(): /* Skip devices that are already bound */ if (device->physical_node_count) return 0; Hans de Goede (2): ACPI: x86: Move acpi_quirk_skip_serdev_enumeration() out of CONFIG_X86_ANDROID_TABLETS ACPI: x86: Add DELL0501 handling to acpi_quirk_skip_serdev_enumeration() drivers/acpi/x86/utils.c | 38 ++++++++++++++++++++++++++++++++++---- include/acpi/acpi_bus.h | 22 +++++++++++----------- 2 files changed, 45 insertions(+), 15 deletions(-)