Re: [PATCH v5 07/10] tty: serial: Add Cortina UART driver

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] tty: serial: cortina: Overrun errors reported when CREAD is cleared
--

commit 6c5059b7f2174557fc1b85a7365146c863b99a28
Author: Fil Dunsky <filipp.dunsky@gmail.com>

tty: serial: Add Cortina UART driver

This commit adds a new serial driver for the UART found in Cortina Systems
CS75xx SoCs. It supports the system console and earlycon, using the modern
serial core API.

> diff --git a/drivers/tty/serial/cortina-uart.c b/drivers/tty/serial/cortina-uart.c
> new file mode 100644
> index 0000000000000..58faddfe0f37d
> --- /dev/null
> +++ b/drivers/tty/serial/cortina-uart.c

[ ... ]

> +static void cortina_uart_rx_chars(struct uart_port *port, u32 status)
> +{
> +	struct tty_port *tport = &port->state->port;
> +	unsigned int budget = CORTINA_UART_RX_BUDGET;
> +	u32 data, err;

[ ... ]

> +	/* The characters left in the FIFO were received before the lost ones */
> +	if (status & CORTINA_UART_IRQ_RX_OVERRUN &
> +	    ~port->ignore_status_mask)
> +		tty_insert_flip_char(tport, 0, TTY_OVERRUN);
> +
> +	tty_flip_buffer_push(tport);
> +}

[ ... ]

> +static void cortina_uart_set_termios(struct uart_port *port,
> +				     struct ktermios *termios,
> +				     const struct ktermios *old)
> +{
> +	unsigned int baud, div;
> +	u32 cfg;

[ ... ]

> +	port->ignore_status_mask = 0;
> +	if (termios->c_iflag & IGNPAR)
> +		port->ignore_status_mask |= CORTINA_UART_IRQ_RX_ERR;
> +	if (!(termios->c_cflag & CREAD))
> +		port->ignore_status_mask |= CORTINA_UART_RXDAT_VALID;

[Severity: Low]
Does this leave the port vulnerable to reporting overrun errors even when
the receiver is supposed to be disabled?

When CREAD is cleared, the intent is to ignore all received data. However,
it appears CORTINA_UART_IRQ_RX_OVERRUN is omitted from the ignore mask here.

Since CORTINA_UART_CFG_RX_EN remains unconditionally set in the configuration,
the hardware receiver stays active and the FIFO can still overrun if data
arrives.

If an overrun occurs under these conditions, cortina_uart_rx_chars() would
inject a TTY_OVERRUN character into the buffer because the overrun status bit
isn't masked out by ignore_status_mask. Should CORTINA_UART_IRQ_RX_OVERRUN
also be added to the mask when CREAD is cleared to avoid this?

> +
> +	uart_update_timeout(port, termios->c_cflag, baud);
> +
> +	writel(cfg, port->membase + CORTINA_UART_CFG);
> +	/* Sample the received bits in the middle of the bit time */
> +	writel(div / 2, port->membase + CORTINA_UART_RX_SAMPLE);
> +
> +	if (tty_termios_baud_rate(termios))
> +		tty_termios_encode_baud_rate(termios, baud, baud);
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261013-b4-cs75xx-v5-0-4e72aa383d39@gmail.com?part=7