diff mbox series

[v2,2/2] tools arch x86: Add dell-uart-backlight-emulator

Message ID 20240513111552.44880-3-hdegoede@redhat.com (mailing list archive)
State Superseded, archived
Headers show
Series platform/x86: Add new Dell UART backlight driver | expand

Commit Message

Hans de Goede May 13, 2024, 11:15 a.m. UTC
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@redhat.com>
---
Changes in v2:
- Ensure clean exit (return 0) when the emulator is quit by Ctrl+C
---
 .../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            | 167 ++++++++++++++++++
 4 files changed, 233 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

Comments

Andy Shevchenko May 13, 2024, 12:46 p.m. UTC | #1
On Mon, May 13, 2024 at 01:15:51PM +0200, Hans de Goede wrote:
> 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.

...

> +	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;

So, looking at the `man error` it works like your custom approach with an
additional things.

Also, don't you want to use either different error codes (above is +1 and all
below seems using -1), or be consistent and return -1 always?

> +	}

...

> +		/* 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);

response[0] can be reused here.

> +		if (ret != (len + 3))

And here.

> +			fprintf(stderr, "Error writing %d bytes: %d\n",
> +				len + 3, ret);
> +	}
Hans de Goede May 13, 2024, 1:25 p.m. UTC | #2
Hi,

On 5/13/24 2:46 PM, Andy Shevchenko wrote:
> On Mon, May 13, 2024 at 01:15:51PM +0200, Hans de Goede wrote:
>> 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.
> 
> ...
> 
>> +	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;
> 
> So, looking at the `man error` it works like your custom approach with an
> additional things.

But that is a GNU / glibc extension. I would prefer to stick with plain libc
here, so that this can be build with other libc-s too. 
> Also, don't you want to use either different error codes (above is +1 and all
> below seems using -1), or be consistent and return -1 always?

the +1 code is for when the cmdline argument is likely wrong , -1 / 255 is
for IO errors.

> 
>> +	}
> 
> ...
> 
>> +		/* 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);
> 
> response[0] can be reused here.
> 
>> +		if (ret != (len + 3))
> 
> And here.

Ack will fix.

Regards,

Hans


> 
>> +			fprintf(stderr, "Error writing %d bytes: %d\n",
>> +				len + 3, ret);
>> +	}
>
diff mbox series

Patch

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..c0d8e52046ee
--- /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 converter
+   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 converter 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..1cd264631a8f
--- /dev/null
+++ b/tools/arch/x86/dell-uart-backlight-emulator/dell-uart-backlight-emulator.c
@@ -0,0 +1,167 @@ 
+// 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@kernel.org>
+ */
+#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);
+	}
+
+	ret = 0;
+out_restore:
+	tcsetattr(serial_fd, TCSANOW, &saved_tty);
+out_close:
+	close(serial_fd);
+	return ret;
+}