[PATCH v3 2/2] platform/x86: dell-uart-backlight: recover from stray

[chitranshmandhaniya]

From: Chitransh Mandhaniya <Chitransh_Mandhaniya@xxxxxxxx>

On some Dell AIO machines the EC occasionally inserts a spurious byte
after the cmd-echo in UART backlight response frames. A get-brightness
reply "04 0c 0a e5" arrives as "04 0c 0d 0a e5". The length field still
reflects the original frame size, so the parser consumes the stray byte
as payload and mistakes the real payload for the checksum, returning
-EIO.

On the first checksum failure for a fully received frame, drop the byte
at the first payload offset, shift the remaining data down by one, and
call dell_uart_bl_receive_data() again to re-fetch the checksum byte
from the wire. Track total consumed bytes so the exact count is
returned to serdev.

The resync is guarded by resp_idx == resp_len to ensure a complete frame
was received, preventing memmove on a truncated or out-of-range length
field.

Limit this single-byte resync to once per command via a resync_used flag
cleared at the start of each transaction.  A second mismatch is still a
hard error. Well-formed replies never enter the recovery path.

While at it, switch mutex_init() to devm_mutex_init() so the mutex is
automatically destroyed via devres, fixing a missing mutex_destroy()
on the teardown path.

Tested on Dell OptiPlex AIO 7410 65W (0TKWC4) where the EC inserts a
stray byte on every get-brightness response -- resync fixes it
transparently.

Suggested-by: ilpo.jarvinen@xxxxxxxxxxxxxxx
Signed-off-by: Chitransh Mandhaniya <Chitransh_Mandhaniya@xxxxxxxx>
---
 .../platform/x86/dell/dell-uart-backlight.c   | 35 ++++++++++++++++---
 1 file changed, 31 insertions(+), 4 deletions(-)

diff --git a/drivers/platform/x86/dell/dell-uart-backlight.c b/drivers/platform/x86/dell/dell-uart-backlight.c
index af9c5f9a566f..b0df479a4658 100644
--- a/drivers/platform/x86/dell/dell-uart-backlight.c
+++ b/drivers/platform/x86/dell/dell-uart-backlight.c
@@ -60,6 +60,7 @@ struct dell_uart_backlight {
 	u8 resp_len;
 	u8 resp_max_len;
 	u8 pending_cmd;
+	bool resync_used;
 	int status;
 	int power;
 };
@@ -91,6 +92,7 @@ static int dell_uart_bl_command(struct dell_uart_backlight *dell_bl,
 	dell_bl->resp_len = -1; /* Invalid / unset */
 	dell_bl->resp_max_len = resp_max_len;
 	dell_bl->pending_cmd = cmd[1];
+	dell_bl->resync_used = false;
 
 	/* 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);
@@ -256,7 +258,7 @@ static size_t dell_uart_bl_receive_data(struct dell_uart_backlight *dell_bl,
 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 ret;
+	size_t consumed;
 
 	dev_dbg(dell_bl->dev, "Recv: %*ph\n", (int)len, data);
 
@@ -266,12 +268,35 @@ static size_t dell_uart_bl_receive(struct serdev_device *serdev, const u8 *data,
 		return len;
 	}
 
-	ret = dell_uart_bl_receive_data(dell_bl, data, len);
+	consumed = dell_uart_bl_receive_data(dell_bl, data, len);
 	if (dell_bl->status == -EBUSY)
 		return len; /* Response not complete yet */
 
+	/*
+	 * Stray byte after cmd-echo? Drop it, shift data down, re-fetch
+	 * checksum. Only attempt when a full frame was received (resp_idx ==
+	 * resp_len) to avoid acting on a truncated length field.
+	 */
+	if (dell_bl->status == -EIO && !dell_bl->resync_used &&
+	    dell_bl->resp_idx == dell_bl->resp_len &&
+	    dell_bl->resp_len > MIN_RESP_LEN) {
+		dell_bl->resync_used = true;
+		dell_bl->status = -EBUSY;
+
+		dev_dbg(dell_bl->dev, "Resync: dropping stray byte 0x%02x at offset %d\n",
+			dell_bl->resp[RESP_DATA], RESP_DATA);
+
+		memmove(&dell_bl->resp[RESP_DATA], &dell_bl->resp[RESP_DATA + 1],
+			dell_bl->resp_len - RESP_DATA - 1);
+		dell_bl->resp_idx = dell_bl->resp_len - 1;
+
+		consumed += dell_uart_bl_receive_data(dell_bl, data + consumed, len - consumed);
+		if (dell_bl->status == -EBUSY)
+			return len; /* Need more data for checksum byte */
+	}
+
 	wake_up(&dell_bl->wait_queue);
-	return ret;
+	return consumed;
 }
 
 static const struct serdev_device_ops dell_uart_bl_serdev_ops = {
@@ -291,7 +316,9 @@ static int dell_uart_bl_serdev_probe(struct serdev_device *serdev)
 	if (!dell_bl)
 		return -ENOMEM;
 
-	mutex_init(&dell_bl->mutex);
+	ret = devm_mutex_init(dev, &dell_bl->mutex);
+	if (ret)
+		return ret;
 	init_waitqueue_head(&dell_bl->wait_queue);
 	dell_bl->dev = dev;
 
-- 
2.43.0