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.
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
>>
>
From 5717af4e556f34d2a96bc33596b1c6866e3b09bc Mon Sep 17 00:00:00 2001
From: Hans de Goede <hdegoede@xxxxxxxxxx>
Date: Sun, 18 Feb 2024 16:16:14 +0100
Subject: [PATCH 1/3] platform/x86: Add new Dell UART backlight driver
Dell All In One (AIO) models released after 2017 use a backlight controller
board connected to an UART.
In DSDT this uart port will be defined as:
Name (_HID, "DELL0501")
Name (_CID, EisaId ("PNP0501")
Instead of having a separate ACPI device with an UartSerialBusV2() resource
to model the backlight-controller, which would be the standard way to do
this.
The acpi_quirk_skip_serdev_enumeration() has special handling for this
and it will make the serial port code create a serdev controller device
for the UART instead of a /dev/ttyS0 char-dev. It will also create
a dell-uart-backlight driver platform device for this driver to bind too.
This new kernel module contains 2 drivers for this:
1. A simple platform driver which creates the actual serdev device
(with the serdev controller device as parent)
2. A serdev driver for the created serdev device which exports
the backlight functionality uses a standard backlight class device.
Co-developed-by: AceLan Kao <acelan.kao@xxxxxxxxxxxxx>
Signed-off-by: AceLan Kao <acelan.kao@xxxxxxxxxxxxx>
Signed-off-by: Hans de Goede <hdegoede@xxxxxxxxxx>
---
drivers/platform/x86/dell/Kconfig | 15 +
drivers/platform/x86/dell/Makefile | 1 +
.../platform/x86/dell/dell-uart-backlight.c | 409 ++++++++++++++++++
3 files changed, 425 insertions(+)
create mode 100644 drivers/platform/x86/dell/dell-uart-backlight.c
diff --git a/drivers/platform/x86/dell/Kconfig b/drivers/platform/x86/dell/Kconfig
index bd9f445974cc..195a8bf532cc 100644
--- a/drivers/platform/x86/dell/Kconfig
+++ b/drivers/platform/x86/dell/Kconfig
@@ -145,6 +145,21 @@ config DELL_SMO8800
To compile this driver as a module, choose M here: the module will
be called dell-smo8800.
+config DELL_UART_BACKLIGHT
+ tristate "Dell AIO UART Backlight driver"
+ depends on ACPI
+ depends on BACKLIGHT_CLASS_DEVICE
+ depends on SERIAL_DEV_BUS
+ help
+ Say Y here if you want to support Dell AIO UART backlight interface.
+ The Dell AIO machines released after 2017 come with a UART interface
+ to communicate with the backlight scalar board. This driver creates
+ a standard backlight interface and talks to the scalar board through
+ UART to adjust the AIO screen brightness.
+
+ To compile this driver as a module, choose M here: the module will
+ be called dell_uart_backlight.
+
config DELL_WMI
tristate "Dell WMI notifications"
default m
diff --git a/drivers/platform/x86/dell/Makefile b/drivers/platform/x86/dell/Makefile
index 1b8942426622..8176a257d9c3 100644
--- a/drivers/platform/x86/dell/Makefile
+++ b/drivers/platform/x86/dell/Makefile
@@ -14,6 +14,7 @@ dell-smbios-objs := dell-smbios-base.o
dell-smbios-$(CONFIG_DELL_SMBIOS_WMI) += dell-smbios-wmi.o
dell-smbios-$(CONFIG_DELL_SMBIOS_SMM) += dell-smbios-smm.o
obj-$(CONFIG_DELL_SMO8800) += dell-smo8800.o
+obj-$(CONFIG_DELL_UART_BACKLIGHT) += dell-uart-backlight.o
obj-$(CONFIG_DELL_WMI) += dell-wmi.o
dell-wmi-objs := dell-wmi-base.o
dell-wmi-$(CONFIG_DELL_WMI_PRIVACY) += dell-wmi-privacy.o
diff --git a/drivers/platform/x86/dell/dell-uart-backlight.c b/drivers/platform/x86/dell/dell-uart-backlight.c
new file mode 100644
index 000000000000..d43f5990d93f
--- /dev/null
+++ b/drivers/platform/x86/dell/dell-uart-backlight.c
@@ -0,0 +1,409 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ * Dell AIO Serial Backlight Driver
+ *
+ * Copyright (C) 2024 Hans de Goede <hansg@xxxxxxxxxx>
+ * Copyright (C) 2017 AceLan Kao <acelan.kao@xxxxxxxxxxxxx>
+ */
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <linux/acpi.h>
+#include <linux/backlight.h>
+#include <linux/delay.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/platform_device.h>
+#include <linux/serdev.h>
+#include <linux/wait.h>
+#include "../serdev_helpers.h"
+
+/* The backlight controller must respond within 1 second */
+#define DELL_BL_TIMEOUT msecs_to_jiffies(1000)
+#define DELL_BL_MIN_RESP_SIZE 3
+
+struct dell_uart_backlight {
+ struct mutex mutex;
+ wait_queue_head_t wait_queue;
+ struct device *dev;
+ struct backlight_device *bl;
+ u8 *resp;
+ u8 resp_idx;
+ u8 resp_len;
+ u8 resp_max_len;
+ u8 pending_cmd;
+ int status;
+ int power;
+};
+
+/* Checksum: SUM(Length and Cmd and Data) xor 0xFF */
+static u8 dell_uart_checksum(u8 *buf, int len)
+{
+ u8 val = 0;
+
+ while (len-- > 0)
+ val += buf[len];
+
+ return val ^ 0xff;
+}
+
+static int dell_uart_bl_command(struct dell_uart_backlight *dell_bl,
+ const u8 *cmd, int cmd_len,
+ u8 *resp, int resp_max_len)
+{
+ int ret;
+
+ ret = mutex_lock_killable(&dell_bl->mutex);
+ if (ret)
+ return ret;
+
+ dell_bl->status = 0;
+ dell_bl->resp = resp;
+ dell_bl->resp_idx = 0;
+ dell_bl->resp_max_len = resp_max_len;
+ dell_bl->pending_cmd = cmd[1];
+
+ /* The TTY buffer should be big enough to take the entire cmd in one go */
+ ret = serdev_device_write_buf(to_serdev_device(dell_bl->dev), cmd, cmd_len);
+ if (ret != cmd_len) {
+ dev_err(dell_bl->dev, "Error writing command: %d\n", ret);
+ ret = (ret < 0) ? ret : -EIO;
+ goto out;
+ }
+
+ ret = wait_event_timeout(dell_bl->wait_queue, dell_bl->status, DELL_BL_TIMEOUT);
+ if (ret == 0) {
+ dev_err(dell_bl->dev, "Timed out waiting for response.\n");
+ dell_bl->status = -ETIMEDOUT;
+ }
+
+ if (dell_bl->status == 1)
+ ret = 0;
+ else
+ ret = dell_bl->status;
+
+out:
+ mutex_unlock(&dell_bl->mutex);
+ return ret;
+}
+
+static int dell_uart_set_brightness(struct dell_uart_backlight *dell_bl, int brightness)
+{
+ /*
+ * Set Brightness level: Application uses this command to set brightness.
+ * Command: 0x8A 0x0B <brightness-level> Checksum (Length:4 Type:0x0A Cmd:0x0B)
+ * <brightness-level> ranges from 0~100.
+ * Return data: 0x03 0x0B 0xF1 (Length:3 Cmd:0x0B Checksum:0xF1)
+ */
+ u8 set_brightness[] = { 0x8A, 0x0B, 0x00, 0x00 };
+ u8 resp[3];
+
+ set_brightness[2] = brightness;
+ set_brightness[3] = dell_uart_checksum(set_brightness, 3);
+
+ return dell_uart_bl_command(dell_bl, set_brightness, ARRAY_SIZE(set_brightness),
+ resp, ARRAY_SIZE(resp));
+}
+
+static int dell_uart_get_brightness(struct dell_uart_backlight *dell_bl)
+{
+ /*
+ * Get Brightness level: Application uses this command to get brightness.
+ * Command: 0x6A 0x0C 0x89 (Length:3 Type:0x0A Cmd:0x0C Checksum:0x89)
+ * Return data: 0x04 0x0C Data Checksum
+ * (Length:4 Cmd:0x0C Data:<brightness level>
+ * Checksum: SUM(Length and Cmd and Data) xor 0xFF)
+ * <brightness level> ranges from 0~100.
+ */
+ const u8 get_brightness[] = { 0x6A, 0x0C, 0x89 };
+ u8 resp[4];
+ int ret;
+
+ ret = dell_uart_bl_command(dell_bl, get_brightness, ARRAY_SIZE(get_brightness),
+ resp, ARRAY_SIZE(resp));
+ if (ret)
+ return ret;
+
+ if (resp[0] != 4) {
+ dev_err(dell_bl->dev, "Unexpected get brightness response length: %d\n", resp[0]);
+ return -EIO;
+ }
+
+ if (resp[2] > 100) {
+ dev_err(dell_bl->dev, "Unexpected get brightness response: %d\n", resp[2]);
+ return -EIO;
+ }
+
+ return resp[2];
+}
+
+static int dell_uart_set_bl_power(struct dell_uart_backlight *dell_bl, int power)
+{
+ /*
+ * Screen ON/OFF Control: Application uses this command to control screen ON or OFF.
+ * Command: 0x8A 0x0E Data Checksum (Length:4 Type:0x0A Cmd:0x0E) where
+ * Data=0 to turn OFF the screen.
+ * Data=1 to turn ON the screen.
+ * Other value of Data is reserved and invalid.
+ * Return data: 0x03 0x0E 0xEE (Length:3 Cmd:0x0E Checksum:0xEE)
+ */
+ u8 set_power[] = { 0x8A, 0x0E, 0x00, 0x00 };
+ u8 resp[3];
+ int ret;
+
+ set_power[2] = (power == FB_BLANK_UNBLANK) ? 1 : 0;
+ set_power[3] = dell_uart_checksum(set_power, 3);
+
+ ret = dell_uart_bl_command(dell_bl, set_power, ARRAY_SIZE(set_power),
+ resp, ARRAY_SIZE(resp));
+ if (ret)
+ return ret;
+
+ dell_bl->power = power;
+ return 0;
+}
+
+/*
+ * There is no command to get backlight power status,
+ * so we set the backlight power to "on" while initializing,
+ * and then track and report its status by power variable
+ */
+static int dell_uart_get_bl_power(struct dell_uart_backlight *dell_bl)
+{
+ return dell_bl->power;
+}
+
+static int dell_uart_update_status(struct backlight_device *bd)
+{
+ struct dell_uart_backlight *dell_bl = bl_get_data(bd);
+ int ret;
+
+ ret = dell_uart_set_brightness(dell_bl, bd->props.brightness);
+ if (ret)
+ return ret;
+
+ if (bd->props.power != dell_uart_get_bl_power(dell_bl))
+ ret = dell_uart_set_bl_power(dell_bl, bd->props.power);
+
+ return ret;
+}
+
+static int dell_uart_get_brightness_op(struct backlight_device *bd)
+{
+ return dell_uart_get_brightness(bl_get_data(bd));
+}
+
+static const struct backlight_ops dell_uart_backlight_ops = {
+ .update_status = dell_uart_update_status,
+ .get_brightness = dell_uart_get_brightness_op,
+};
+
+static size_t dell_uart_bl_receive(struct serdev_device *serdev, const u8 *data, size_t len)
+{
+ struct dell_uart_backlight *dell_bl = serdev_device_get_drvdata(serdev);
+ size_t i;
+ u8 csum;
+
+ dev_dbg(dell_bl->dev, "Recv: %*ph\n", (int)len, data);
+
+ /* Throw away unexpected bytes / remainder of response after an error */
+ if (dell_bl->status) {
+ dev_warn(dell_bl->dev, "Bytes received out of band, dropping them.\n");
+ return len;
+ }
+
+ for (i = 0; i < len; i++) {
+ dell_bl->resp[dell_bl->resp_idx] = data[i];
+
+ switch (dell_bl->resp_idx) {
+ case 0: /* Length byte */
+ dell_bl->resp_len = dell_bl->resp[0];
+ if (dell_bl->resp_len < DELL_BL_MIN_RESP_SIZE) {
+ dev_err(dell_bl->dev, "Response length too small %d < %d\n",
+ dell_bl->resp_len, DELL_BL_MIN_RESP_SIZE);
+ dell_bl->status = -EIO;
+ goto wakeup;
+ } else if (dell_bl->resp_len > dell_bl->resp_max_len) {
+ dev_err(dell_bl->dev, "Response length too big %d > %d\n",
+ dell_bl->resp_len, dell_bl->resp_max_len);
+ dell_bl->status = -EIO;
+ goto wakeup;
+ }
+ break;
+ case 1: /* CMD byte */
+ if (dell_bl->resp[1] != dell_bl->pending_cmd) {
+ dev_err(dell_bl->dev, "Response cmd 0x%02x != pending 0x%02x\n",
+ dell_bl->resp[1], dell_bl->pending_cmd);
+ dell_bl->status = -EIO;
+ goto wakeup;
+ }
+ break;
+ }
+
+ dell_bl->resp_idx++;
+ if (dell_bl->resp_idx < dell_bl->resp_len)
+ continue;
+
+ csum = dell_uart_checksum(dell_bl->resp, dell_bl->resp_len - 1);
+ if (dell_bl->resp[dell_bl->resp_len - 1] != csum) {
+ dev_err(dell_bl->dev, "Checksum mismatch got 0x%02x expected 0x%02x\n",
+ dell_bl->resp[dell_bl->resp_len - 1], csum);
+ dell_bl->status = -EIO;
+ goto wakeup;
+ }
+
+ dell_bl->status = 1; /* Success */
+ goto wakeup;
+ }
+
+ return len;
+
+wakeup:
+ wake_up(&dell_bl->wait_queue);
+ return i + 1;
+}
+
+static const struct serdev_device_ops dell_uart_bl_serdev_ops = {
+ .receive_buf = dell_uart_bl_receive,
+ .write_wakeup = serdev_device_write_wakeup,
+};
+
+static int dell_uart_bl_serdev_probe(struct serdev_device *serdev)
+{
+ /*
+ * Get Firmware Version: Tool uses this command to get firmware version.
+ * Command: 0x6A 0x06 0x8F (Length:3 Type:0x0A Cmd:6 Checksum:0x8F)
+ * Return data: 0x0D 0x06 Data Checksum (Length:13 Cmd:0x06
+ * Data:F/W version(APRILIA=APR27-Vxxx/PHINE=PHI23-Vxxx)
+ * Checksum:SUM(Length and Cmd and Data) xor 0xFF)
+ */
+ const u8 get_firmware_ver[] = { 0x6A, 0x06, 0x8F };
+ struct dell_uart_backlight *dell_bl;
+ struct backlight_properties props;
+ struct device *dev = &serdev->dev;
+ u8 get_firmware_ver_resp[80];
+ int ret;
+
+ dell_bl = devm_kzalloc(dev, sizeof(*dell_bl), GFP_KERNEL);
+ if (!dell_bl)
+ return -ENOMEM;
+
+ mutex_init(&dell_bl->mutex);
+ init_waitqueue_head(&dell_bl->wait_queue);
+ dell_bl->dev = dev;
+
+ ret = devm_serdev_device_open(dev, serdev);
+ if (ret)
+ return dev_err_probe(dev, ret, "opening UART device\n");
+
+ /* 9600 bps, no flow control, these are the default but set them to be sure */
+ serdev_device_set_baudrate(serdev, 9600);
+ serdev_device_set_flow_control(serdev, false);
+ serdev_device_set_drvdata(serdev, dell_bl);
+ serdev_device_set_client_ops(serdev, &dell_uart_bl_serdev_ops);
+
+ ret = dell_uart_bl_command(dell_bl, get_firmware_ver, ARRAY_SIZE(get_firmware_ver),
+ get_firmware_ver_resp, ARRAY_SIZE(get_firmware_ver_resp));
+ if (ret)
+ return dev_err_probe(dev, ret, "getting firmware version\n");
+
+ dev_dbg(dev, "Firmware version: %.*s\n", get_firmware_ver_resp[0] - 3,
+ get_firmware_ver_resp + 2);
+
+ /* Initialize bl_power to a known value */
+ ret = dell_uart_set_bl_power(dell_bl, FB_BLANK_UNBLANK);
+ if (ret)
+ return ret;
+
+ ret = dell_uart_get_brightness(dell_bl);
+ if (ret < 0)
+ return ret;
+
+ memset(&props, 0, sizeof(struct backlight_properties));
+ props.type = BACKLIGHT_PLATFORM;
+ props.brightness = ret;
+ props.max_brightness = 100;
+ props.power = dell_bl->power;
+
+ dell_bl->bl = devm_backlight_device_register(dev, "dell_uart_backlight",
+ dev, dell_bl,
+ &dell_uart_backlight_ops,
+ &props);
+ if (IS_ERR(dell_bl->bl))
+ return PTR_ERR(dell_bl->bl);
+
+ return 0;
+}
+
+struct serdev_device_driver dell_uart_bl_serdev_driver = {
+ .probe = dell_uart_bl_serdev_probe,
+ .driver = {
+ .name = KBUILD_MODNAME,
+ },
+};
+
+static int dell_uart_bl_pdev_probe(struct platform_device *pdev)
+{
+ struct serdev_device *serdev;
+ struct device *ctrl_dev;
+ int ret;
+
+ ctrl_dev = get_serdev_controller("DELL0501", "0", 0, "serial0");
+ if (IS_ERR(ctrl_dev))
+ return PTR_ERR(ctrl_dev);
+
+ serdev = serdev_device_alloc(to_serdev_controller(ctrl_dev));
+ put_device(ctrl_dev);
+ if (!serdev)
+ return -ENOMEM;
+
+ ret = serdev_device_add(serdev);
+ if (ret) {
+ dev_err(&pdev->dev, "error %d adding serdev\n", ret);
+ serdev_device_put(serdev);
+ return ret;
+ }
+
+ ret = serdev_device_driver_register(&dell_uart_bl_serdev_driver);
+ if (ret) {
+ serdev_device_remove(serdev);
+ return ret;
+ }
+
+ /*
+ * serdev device <-> driver matching relies on OF or ACPI matches and
+ * neither is available here, manually bind the driver.
+ */
+ ret = device_driver_attach(&dell_uart_bl_serdev_driver.driver, &serdev->dev);
+ if (ret) {
+ serdev_device_driver_unregister(&dell_uart_bl_serdev_driver);
+ serdev_device_remove(serdev);
+ return ret;
+ }
+
+ /* So that dell_uart_bl_pdev_remove() can remove the serdev */
+ platform_set_drvdata(pdev, serdev);
+ return 0;
+}
+
+static void dell_uart_bl_pdev_remove(struct platform_device *pdev)
+{
+ struct serdev_device *serdev = platform_get_drvdata(pdev);
+
+ serdev_device_driver_unregister(&dell_uart_bl_serdev_driver);
+ serdev_device_remove(serdev);
+}
+
+static struct platform_driver dell_uart_bl_pdev_driver = {
+ .probe = dell_uart_bl_pdev_probe,
+ .remove_new = dell_uart_bl_pdev_remove,
+ .driver = {
+ .name = "dell-uart-backlight",
+ },
+};
+module_platform_driver(dell_uart_bl_pdev_driver);
+
+MODULE_ALIAS("platform:dell-uart-backlight");
+MODULE_DESCRIPTION("Dell AIO Serial Backlight driver");
+MODULE_AUTHOR("Hans de Goede <hansg@xxxxxxxxxx>");
+MODULE_LICENSE("GPL");
--
2.44.0
From e91eb2c85ce633957b3406c2c07dfe048dbd4dd1 Mon Sep 17 00:00:00 2001
From: Hans de Goede <hdegoede@xxxxxxxxxx>
Date: Tue, 20 Feb 2024 22:09:42 +0100
Subject: [PATCH 2/3] platform/x86: dell-uart-backlight: Add ifdefs to make it
build with kernel 6.7/6.8
Local modification, do not upstream. Make the serdev helpers and
dell-uart-backlight work with kernel 6.7/6.8 for testing purposes.
Signed-off-by: Hans de Goede <hdegoede@xxxxxxxxxx>
---
drivers/platform/x86/dell/dell-uart-backlight.c | 4 ++++
drivers/platform/x86/serdev_helpers.h | 6 ++++++
2 files changed, 10 insertions(+)
diff --git a/drivers/platform/x86/dell/dell-uart-backlight.c b/drivers/platform/x86/dell/dell-uart-backlight.c
index d43f5990d93f..da4a640c0d88 100644
--- a/drivers/platform/x86/dell/dell-uart-backlight.c
+++ b/drivers/platform/x86/dell/dell-uart-backlight.c
@@ -198,7 +198,11 @@ static const struct backlight_ops dell_uart_backlight_ops = {
.get_brightness = dell_uart_get_brightness_op,
};
+#if LINUX_VERSION_CODE >= KERNEL_VERSION(6, 9, 0)
static size_t dell_uart_bl_receive(struct serdev_device *serdev, const u8 *data, size_t len)
+#else
+static ssize_t dell_uart_bl_receive(struct serdev_device *serdev, const u8 *data, size_t len)
+#endif
{
struct dell_uart_backlight *dell_bl = serdev_device_get_drvdata(serdev);
size_t i;
diff --git a/drivers/platform/x86/serdev_helpers.h b/drivers/platform/x86/serdev_helpers.h
index bcf3a0c356ea..3d254926c9ba 100644
--- a/drivers/platform/x86/serdev_helpers.h
+++ b/drivers/platform/x86/serdev_helpers.h
@@ -20,6 +20,7 @@
#include <linux/printk.h>
#include <linux/sprintf.h>
#include <linux/string.h>
+#include <linux/version.h>
static inline struct device *
get_serdev_controller(const char *serial_ctrl_hid,
@@ -49,7 +50,12 @@ get_serdev_controller(const char *serial_ctrl_hid,
}
/* Walk host -> uart-ctrl -> port -> serdev-ctrl */
+#if LINUX_VERSION_CODE >= KERNEL_VERSION(6, 8, 0)
for (i = 0; i < 3; i++) {
+#else
+ /* Kernels < 6.8 have host -> serdev-ctrl */
+ for (i = 2; i < 3; i++) {
+#endif
switch (i) {
case 0:
snprintf(name, sizeof(name), "%s:0", dev_name(ctrl_dev));
--
2.44.0
From 9677480af27d6f1d3c981e62b86a2ec56f9afe11 Mon Sep 17 00:00:00 2001
From: Hans de Goede <hdegoede@xxxxxxxxxx>
Date: Mon, 19 Feb 2024 17:41:57 +0100
Subject: [PATCH 3/3] tools arch x86: Add dell-uart-backlight-emulator
Dell All In One (AIO) models released after 2017 use a backlight controller
board connected to an UART.
Add a small emulator to allow development and testing of
the drivers/platform/x86/dell/dell-uart-backlight.c driver for
this board, without requiring access to an actual Dell All In One.
Signed-off-by: Hans de Goede <hdegoede@xxxxxxxxxx>
---
.../dell-uart-backlight-emulator/.gitignore | 1 +
.../x86/dell-uart-backlight-emulator/Makefile | 19 ++
.../x86/dell-uart-backlight-emulator/README | 46 +++++
.../dell-uart-backlight-emulator.c | 166 ++++++++++++++++++
4 files changed, 232 insertions(+)
create mode 100644 tools/arch/x86/dell-uart-backlight-emulator/.gitignore
create mode 100644 tools/arch/x86/dell-uart-backlight-emulator/Makefile
create mode 100644 tools/arch/x86/dell-uart-backlight-emulator/README
create mode 100644 tools/arch/x86/dell-uart-backlight-emulator/dell-uart-backlight-emulator.c
diff --git a/tools/arch/x86/dell-uart-backlight-emulator/.gitignore b/tools/arch/x86/dell-uart-backlight-emulator/.gitignore
new file mode 100644
index 000000000000..5c8cad8d72b9
--- /dev/null
+++ b/tools/arch/x86/dell-uart-backlight-emulator/.gitignore
@@ -0,0 +1 @@
+dell-uart-backlight-emulator
diff --git a/tools/arch/x86/dell-uart-backlight-emulator/Makefile b/tools/arch/x86/dell-uart-backlight-emulator/Makefile
new file mode 100644
index 000000000000..6ea1d9fd534b
--- /dev/null
+++ b/tools/arch/x86/dell-uart-backlight-emulator/Makefile
@@ -0,0 +1,19 @@
+# SPDX-License-Identifier: GPL-2.0
+# Makefile for Intel Software Defined Silicon provisioning tool
+
+dell-uart-backlight-emulator: dell-uart-backlight-emulator.c
+
+BINDIR ?= /usr/bin
+
+override CFLAGS += -O2 -Wall
+
+%: %.c
+ $(CC) $(CFLAGS) -o $@ $< $(LDFLAGS)
+
+.PHONY : clean
+clean :
+ @rm -f dell-uart-backlight-emulator
+
+install : dell-uart-backlight-emulator
+ install -d $(DESTDIR)$(BINDIR)
+ install -m 755 -p dell-uart-backlight-emulator $(DESTDIR)$(BINDIR)/dell-uart-backlight-emulator
diff --git a/tools/arch/x86/dell-uart-backlight-emulator/README b/tools/arch/x86/dell-uart-backlight-emulator/README
new file mode 100644
index 000000000000..a532d000d34a
--- /dev/null
+++ b/tools/arch/x86/dell-uart-backlight-emulator/README
@@ -0,0 +1,46 @@
+Emulator for DELL0501 UART attached backlight controller
+--------------------------------------------------------
+
+Dell All In One (AIO) models released after 2017 use a backlight controller
+board connected to an UART.
+
+In DSDT this uart port will be defined as:
+
+ Name (_HID, "DELL0501")
+ Name (_CID, EisaId ("PNP0501")
+
+With the DELL0501 indicating that we are dealing with an UART with
+the backlight controller board attached.
+
+This small emulator allows testing
+the drivers/platform/x86/dell/dell-uart-backlight.c driver without access
+to an actual Dell All In One.
+
+This requires:
+1. A (desktop) PC with a 16550 UART on the motherboard and a standard DB9
+ connector connected to this UART.
+2. A DB9 NULL modem cable.
+3. A second DB9 serial port, this can e.g. be a USB to serial convertor
+ with a DB9 connector plugged into the same desktop PC.
+4. A DSDT overlay for the desktop PC replacing the _HID of the 16550 UART
+ ACPI Device() with "DELL0501" and adding a _CID of "PNP0501", see
+ DSDT.patch for an example of the necessary DSDT changes.
+
+With everything setup and the NULL modem cable connected between
+the 2 serial ports run:
+
+./dell-uart-backlight-emulator <path-to-/dev/tty*S#-for-second-port>
+
+For example when using an USB to serial convertor for the second port:
+
+./dell-uart-backlight-emulator /dev/ttyUSB0
+
+And then (re)load the dell-uart-backlight driver:
+
+sudo rmmod dell-uart-backlight; sudo modprobe dell-uart-backlight dyndbg
+
+After this check "dmesg" to see if the driver correctly received
+the firmware version string from the emulator. If this works there
+should be a /sys/class/backlight/dell_uart_backlight/ directory now
+and writes to the brightness or bl_power files should be reflected
+by matching output from the emulator.
diff --git a/tools/arch/x86/dell-uart-backlight-emulator/dell-uart-backlight-emulator.c b/tools/arch/x86/dell-uart-backlight-emulator/dell-uart-backlight-emulator.c
new file mode 100644
index 000000000000..fab1babb26d0
--- /dev/null
+++ b/tools/arch/x86/dell-uart-backlight-emulator/dell-uart-backlight-emulator.c
@@ -0,0 +1,166 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ * Dell AIO Serial Backlight board emulator for testing
+ * the Linux dell-uart-backlight driver.
+ *
+ * Copyright (C) 2024 Hans de Goede <hansg@xxxxxxxxxx>
+ */
+#include <errno.h>
+#include <fcntl.h>
+#include <signal.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <sys/ioctl.h>
+#include <sys/stat.h>
+#include <sys/types.h>
+#include <sys/un.h>
+#include <termios.h>
+#include <unistd.h>
+
+int serial_fd;
+int brightness = 50;
+
+static unsigned char dell_uart_checksum(unsigned char *buf, int len)
+{
+ unsigned char val = 0;
+
+ while (len-- > 0)
+ val += buf[len];
+
+ return val ^ 0xff;
+}
+
+/* read() will return -1 on SIGINT / SIGTERM causing the mainloop to cleanly exit */
+void signalhdlr(int signum)
+{
+}
+
+int main(int argc, char *argv[])
+{
+ struct sigaction sigact = { .sa_handler = signalhdlr };
+ unsigned char buf[4], csum, response[32];
+ const char *version_str = "PHI23-V321";
+ struct termios tty, saved_tty;
+ int ret, idx, len = 0;
+
+ if (argc != 2) {
+ fprintf(stderr, "Invalid or missing arguments\n");
+ fprintf(stderr, "Usage: %s <serial-port>\n", argv[0]);
+ return 1;
+ }
+
+ serial_fd = open(argv[1], O_RDWR | O_NOCTTY);
+ if (serial_fd == -1) {
+ fprintf(stderr, "Error opening %s: %s\n", argv[1], strerror(errno));
+ return 1;
+ }
+
+ ret = tcgetattr(serial_fd, &tty);
+ if (ret == -1) {
+ fprintf(stderr, "Error getting tcattr: %s\n", strerror(errno));
+ goto out_close;
+ }
+ saved_tty = tty;
+
+ cfsetspeed(&tty, 9600);
+ cfmakeraw(&tty);
+ tty.c_cflag &= ~CSTOPB;
+ tty.c_cflag &= ~CRTSCTS;
+ tty.c_cflag |= CLOCAL | CREAD;
+
+ ret = tcsetattr(serial_fd, TCSANOW, &tty);
+ if (ret == -1) {
+ fprintf(stderr, "Error setting tcattr: %s\n", strerror(errno));
+ goto out_restore;
+ }
+
+ sigaction(SIGINT, &sigact, 0);
+ sigaction(SIGTERM, &sigact, 0);
+
+ idx = 0;
+ while (read(serial_fd, &buf[idx], 1) == 1) {
+ if (idx == 0) {
+ switch (buf[0]) {
+ /* 3 MSB bits: cmd-len + 01010 SOF marker */
+ case 0x6a: len = 3; break;
+ case 0x8a: len = 4; break;
+ default:
+ fprintf(stderr, "Error unexpected first byte: 0x%02x\n", buf[0]);
+ continue; /* Try to sync up with sender */
+ }
+ }
+
+ /* Process msg when len bytes have been received */
+ if (idx != (len - 1)) {
+ idx++;
+ continue;
+ }
+
+ /* Reset idx for next command */
+ idx = 0;
+
+ csum = dell_uart_checksum(buf, len - 1);
+ if (buf[len - 1] != csum) {
+ fprintf(stderr, "Error checksum mismatch got 0x%02x expected 0x%02x\n",
+ buf[len - 1], csum);
+ continue;
+ }
+
+ switch ((buf[0] << 8) | buf[1]) {
+ case 0x6a06:
+ /* cmd = 0x06, get version */
+ len = strlen(version_str);
+ strcpy((char *)&response[2], version_str);
+ printf("Get version, reply: %s\n", version_str);
+ break;
+ case 0x8a0b: /* 3 MSB bits: cmd-len + 01010 SOF marker */
+ /* cmd = 0x0b, set brightness */
+ if (buf[2] > 100) {
+ fprintf(stderr, "Error invalid brightness param: %d\n", buf[2]);
+ continue;
+ }
+
+ len = 0;
+ brightness = buf[2];
+ printf("Set brightness %d\n", brightness);
+ break;
+ case 0x6a0c:
+ /* cmd = 0x0c, get brightness */
+ len = 1;
+ response[2] = brightness;
+ printf("Get brightness, reply: %d\n", brightness);
+ break;
+ case 0x8a0e:
+ /* cmd = 0x0e, set backlight power */
+ if (buf[2] != 0 && buf[2] != 1) {
+ fprintf(stderr, "Error invalid set power param: %d\n", buf[2]);
+ continue;
+ }
+
+ len = 0;
+ printf("Set power %d\n", buf[2]);
+ break;
+ default:
+ fprintf(stderr, "Error unknown cmd 0x%04x\n",
+ (buf[0] << 8) | buf[1]);
+ continue;
+ }
+
+ /* Respond with <total-len> <cmd> <data...> <csum> */
+ response[0] = len + 3; /* response length in bytes */
+ response[1] = buf[1]; /* ack cmd */
+ csum = dell_uart_checksum(response, len + 2);
+ response[len + 2] = csum;
+ ret = write(serial_fd, response, len + 3);
+ if (ret != (len + 3))
+ fprintf(stderr, "Error writing %d bytes: %d\n",
+ len + 3, ret);
+ }
+
+out_restore:
+ tcsetattr(serial_fd, TCSANOW, &saved_tty);
+out_close:
+ close(serial_fd);
+ return ret;
+}
--
2.44.0
