Message-Id: <20250723023322.464-1-cuiyunhui@bytedance.com>
Date: Wed, 23 Jul 2025 10:33:21 +0800
From: Yunhui Cui <cuiyunhui@...edance.com>
To: gregkh@...uxfoundation.org,
	jirislaby@...nel.org,
	ilpo.jarvinen@...ux.intel.com,
	john.ogness@...utronix.de,
	andriy.shevchenko@...ux.intel.com,
	matt.porter@...aro.org,
	tim.kryger@...aro.org,
	markus.mayer@...aro.org,
	heikki.krogerus@...ux.intel.com,
	linux-kernel@...r.kernel.org,
	linux-serial@...r.kernel.org
Cc: Yunhui Cui <cuiyunhui@...edance.com>
Subject: [PATCH v10 0/1] Serial: 8250: Fix PSLVERR related issues

Cause of PSLVERR:
When the PSLVERR_RESP_EN parameter is set to 1, the device generates
an error response if an attempt is made to read an empty RBR
(Receive Buffer Register) while the FIFO is enabled.

Patch[1]: Fixes a panic caused by PSLVERR due to concurrent UART access.
Patch[2]: Fixes a panic caused by PSLVERR during RX_TIMEOUT conditions.
Patch[3] & Patch[4]: Improvements to minimize the occurrence of PSLVERR.

v1 -> v2:
Added UART_LSR_DR check in shutdown() to avoid PSLVERR issues.
Added Fixes: tag to reference upstream issues.
v2 -> v3:
Added lock protection in more functions (e.g., autoconfig_irq()) to
ensure atomicity.
Used lockdep_assert_held_once to detect potential deadlock risks early.
v3 -> v4:
Introduced serial8250_discard_data() to unify data read logic and avoid
code duplication.
Addressed PSLVERR caused by RX_TIMEOUT.
Split complex fixes into multiple patches (1/4 to 4/4).
v4 -> v5:
Removed reads from UART_FCR, using up->fcr to determine FIFO enable status.
Removed return value from serial8250_discard_data().
v5 -> v6:
Based on latest linux-next code: Resolved redundant dont_test_tx_en code.
Updated comments and git commit descriptions.
v6 -> v7:
Reverted PSLVERR-related changes in serial8250_get_poll_char().
v7 -> v8:
Added Cc: stable@...r.kernel.org to patch[1] and patch[4].
v8 -> v9:
Reordered the patches: bugfixes as 1-2, improvements as 3-4.
v9->v10:
Patch[4] has been removed, and patch[2] still needs discussion.
For patch[3], although it has the "Reviewed-by: John Ogness john.ogness@...utronix.de" tag,
a problem was found when rebasing to the linux-next top commit: 

If serial8250_clear_interrupts() is modified to:
static void serial8250_clear_interrupts(struct uart_port *port)
{
	u16 lsr;

	lsr = serial_port_in(port, UART_LSR);
	if (lsr & UART_LSR_DR)
		serial_port_in(port, UART_RX);
	serial_port_in(port, UART_IIR);
	serial_port_in(port, UART_MSR);
}
It may not truly achieve the purpose of clearing interrupts, especially
during the second call to serial8250_clear_interrupts(port) in
serial8250_do_startup().

static inline void serial8250_discard_data(struct uart_8250_port *up)
{
	u16 lsr;

	lsr = serial_in(up, UART_LSR);
	if (lsr & UART_LSR_DR)
		serial_in(up, UART_RX);
}

PSLVERR is addressed through two methods. One is to introduce
serial8250_discard_data() to check whether UART_LSR_DR is set
before reading UART_RX, thus solving the PSLVERR issue when the
FIFO is enabled. The other is to place FIFO clearing and reading
of UART_RX under port->lock.

During the first call to serial8250_clear_interrupts() in
serial8250_do_startup(), which occurs right after serial8250_clear_fifos(up),
the first method to prevent PSLVERR is satisfied. However, during the second
call to serial8250_clear_interrupts() in serial8250_do_startup(), the FIFO may
have been enabled. It would be best to prevent PSLVERR via serial8250_discard_data(),
but this might result in not calling serial_in(up, UART_RX), thus failing to achieve
the original intent of clearing interrupts.

Therefore, let's submit patch [1] first, and discuss patches [2]/[3] later.

Yunhui Cui (1):
  serial: 8250: fix panic due to PSLVERR

 drivers/tty/serial/8250/8250_port.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

-- 
2.39.5

Powered by blists - more mailing lists