Re: [PATCH v2 2/2] platform/x86: dell-uart-backlight: recover from stray EC response byte

[Ilpo Järvinen]

On Thu, 15 Oct 2026, chitranshmandhaniya@xxxxxxxxx wrote:

> 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 frame longer than the minimum size,
> 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 and re-validate.
> 
> 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 pass on the first check and never enter
> the recovery 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.
> 
> Signed-off-by: Chitransh Mandhaniya <Chitransh_Mandhaniya@xxxxxxxx>
> ---
>  .../platform/x86/dell/dell-uart-backlight.c   | 23 ++++++++++++++++++-
>  1 file changed, 22 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/platform/x86/dell/dell-uart-backlight.c b/drivers/platform/x86/dell/dell-uart-backlight.c
> index b61499d7b3f8..085dcc055554 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);
> @@ -270,8 +272,27 @@ static size_t dell_uart_bl_receive(struct serdev_device *serdev, const u8 *data,
>  	if (ret < 0)
>  		return len; /* Response not complete yet */
>  
> +	/* Stray byte after cmd-echo? Drop it, shift data down, re-fetch checksum. */
> +	if (dell_bl->status == -EIO &&
> +	    !dell_bl->resync_used && 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);

sashiko.dev (an AI review tool so be reasonably critical to its findings) 
mentions that if ->resp_len check fails due to len being too large, 
->status is -EIO and this may end up moving more than there's room in the 
buffer.

> +		dell_bl->resp_idx = dell_bl->resp_len - 1;
> +
> +		/* Re-fetch checksum byte from remaining data */
> +		ret = dell_uart_bl_receive_data(dell_bl, data + ret, len - ret);
> +		if (ret < 0)
> +			return len; /* Need more data for checksum byte */
> +	}
> +
>  	wake_up(&dell_bl->wait_queue);
> -	return ret;
> +	return len;
>  }
>  
>  static const struct serdev_device_ops dell_uart_bl_serdev_ops = {
> 

-- 
 i.