Re: [PATCH net-next v6 4/4] net: pse-pd: realtek-pse-mcu: add UART transport

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Delayed MCU responses can silently desynchronize the UART protocol and cause the driver to accept stale data for new requests, leading to silent data corruption (e.g., reporting incorrect port status).
--

--- Patch [4]: [PATCH net-next v6 4/4] net: pse-pd: realtek-pse-mcu: add UART transport ---
commit 4b8238c08610aa670e6bef20fc86bbd99d379d19
Author: Jonas Jelonek <jelonek.jonas@gmail.com>

net: pse-pd: realtek-pse-mcu: add UART transport

This commit adds a serdev (UART) transport for the Realtek PSE MCU core.
It registers the MCU as a serdev device and provides send/receive
callbacks to exchange 12-byte frames asynchronously.

> diff --git a/drivers/net/pse-pd/realtek-pse-mcu-uart.c b/drivers/net/pse-pd/realtek-pse-mcu-uart.c
> new file mode 100644
> --- /dev/null
> +++ b/drivers/net/pse-pd/realtek-pse-mcu-uart.c

[ ... ]

> +static int rtpse_mcu_uart_send(struct rtpse_mcu_ctrl *pse, const struct rtpse_mcu_msg *req)
> +{
> +	struct rtpse_mcu_uart *ctx = to_rtpse_mcu_uart(pse);
> +	int written;
> +
> +	/* clear any leftover rx state before transmitting */
> +	scoped_guard(spinlock_irqsave, &ctx->rx_lock) {
> +		reinit_completion(&ctx->rx_done);
> +		ctx->rx_len = 0;
> +	}

[Severity: Medium]
Can a delayed MCU response silently desynchronize the UART protocol here and
cause the driver to accept stale data for new requests?

If a previous request takes longer than RTPSE_MCU_UART_RX_TIMEOUT, it times
out and the operation fails. When the next request is sent, this block
clears ctx->rx_len.

If the delayed 12-byte response from the first request then arrives, it
will fill the newly cleared rx_buf and complete the receive wait. Because
the core driver hardcodes seq_num to 0xff for all messages:

drivers/net/pse-pd/realtek-pse-mcu-core.c:rtpse_mcu_msg_init() {
        memset(msg, 0xff, sizeof(*msg));
        msg->opcode = opcode;
}

it cannot distinguish the stale response from the new one if the opcodes
match. Would it be safer to flush the serdev buffers before transmitting,
or to implement a rolling sequence number in the core to prevent silent
data corruption?

> +
> +	written = serdev_device_write(ctx->serdev, (const u8 *)req, sizeof(*req),
> +				      RTPSE_MCU_UART_TX_TIMEOUT);
> +	if (written < 0)
> +		return written;

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260709194125.2784507-1-jelonek.jonas@gmail.com?part=4