[PATCH] serial: 8250: restore FIFO in serial8250_console_restore()

[PATCH] serial: 8250: restore FIFO in serial8250_console_restore()

[fuzheng.meng]

When console_suspend is disabled, the serial console stays enabled
over a system suspend, and serial8250_suspend_port() writes a canary
value to the SCR register (if the port has one) to detect whether the
UART lost power during the sleep cycle.

If the hardware did lose power, all registers are reset, and the
first console write after resume detects the canary mismatch and
calls serial8250_console_restore(), which reprograms divisor, LCR and
MCR, but not the FCR.

serial8250_console_write() decides whether to take the FIFO write
path from the cached up->fcr, which still carries
UART_FCR_ENABLE_FIFO. With the hardware FIFO left disabled by the
reset, the transmit holding register holds only one byte, while
serial8250_console_fifo_write() pushes up to tx_loadsz bytes into the
port after waiting for THRE only once. The burst overwrites itself
and the console output right after resume is garbled.

Factor the FIFO write path condition out into a new
serial8250_console_use_fifo() helper and use it from both
serial8250_console_write() and serial8250_console_restore(). Restore
the FIFO in serial8250_console_restore() when the FIFO write path is
going to be used: first write UART_FCR_ENABLE_FIFO, then the cached
fcr, as dw8250_idle_exit() does after its registers are reset.

Tested on a DesignWare 8250 based platform with console_suspend=0
over a suspend/resume cycle.

Fixes: 8f3631f0f6eb ("serial/8250: Use fifo in 8250 console driver")
Cc: stable@vger.kernel.org
Signed-off-by: fuzheng.meng <fuzheng.meng@semidrive.com>
---
 drivers/tty/serial/8250/8250_port.c | 64 ++++++++++++++++++++---------
 1 file changed, 45 insertions(+), 19 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
index 36b2b0a2c05c..ee6b02cff015 100644
--- a/drivers/tty/serial/8250/8250_port.c
+++ b/drivers/tty/serial/8250/8250_port.c
@@ -3321,6 +3321,37 @@ static void serial8250_console_wait_putchar(struct uart_port *port, unsigned cha
 	serial8250_console_putchar(port, ch);
 }
 
+/*
+ * Console output can go through the TX FIFO in one tx_loadsz chunk only
+ * if the port has a FIFO and the software state expects it enabled. The
+ * same conditions tell console_restore() whether the FIFO has to be
+ * brought back after a system sleep reset.
+ */
+static bool serial8250_console_use_fifo(struct uart_8250_port *up)
+{
+	struct uart_port *port = &up->port;
+
+	return (up->capabilities & UART_CAP_FIFO) &&
+		/*
+		 * BCM283x requires to check the fifo
+		 * after each byte.
+		 */
+		!(up->capabilities & UART_CAP_MINI) &&
+		/*
+		 * tx_loadsz contains the transmit fifo size
+		 */
+		up->tx_loadsz > 1 &&
+		(up->fcr & UART_FCR_ENABLE_FIFO) &&
+		port->state &&
+		test_bit(TTY_PORT_INITIALIZED, &port->state->port.iflags) &&
+		/*
+		 * After we put a data in the fifo, the controller will send
+		 * it regardless of the CTS state. Therefore, only use fifo
+		 * if we don't use control flow.
+		 */
+		!uart_console_hwflow_active(port);
+}
+
 /*
  *	Restore serial console when h/w power-off detected
  */
@@ -3345,6 +3376,19 @@ static void serial8250_console_restore(struct uart_8250_port *up)
 	serial8250_set_divisor(port, baud, quot, frac);
 	serial_port_out(port, UART_LCR, up->lcr);
 	serial8250_out_MCR(up, up->mcr | UART_MCR_DTR | UART_MCR_RTS);
+
+	/*
+	 * System sleep may reset the FCR, leaving the FIFO disabled,
+	 * while the cached up->fcr still tells console_write() to push
+	 * tx_loadsz bytes into the FIFO at once; with the hardware FIFO
+	 * off, the THR holds only one byte and earlier ones are
+	 * overwritten, which garbles the console output after resume.
+	 * Restore the FCR when the FIFO write path is going to be used.
+	 */
+	if (serial8250_console_use_fifo(up)) {
+		serial_port_out(port, UART_FCR, UART_FCR_ENABLE_FIFO);
+		serial_port_out(port, UART_FCR, up->fcr);
+	}
 }
 
 /*
@@ -3495,25 +3539,7 @@ void serial8250_console_write(struct uart_8250_port *up,
 		mdelay(port->rs485.delay_rts_before_send);
 	}
 
-	use_fifo = (up->capabilities & UART_CAP_FIFO) &&
-		/*
-		 * BCM283x requires to check the fifo
-		 * after each byte.
-		 */
-		!(up->capabilities & UART_CAP_MINI) &&
-		/*
-		 * tx_loadsz contains the transmit fifo size
-		 */
-		up->tx_loadsz > 1 &&
-		(up->fcr & UART_FCR_ENABLE_FIFO) &&
-		port->state &&
-		test_bit(TTY_PORT_INITIALIZED, &port->state->port.iflags) &&
-		/*
-		 * After we put a data in the fifo, the controller will send
-		 * it regardless of the CTS state. Therefore, only use fifo
-		 * if we don't use control flow.
-		 */
-		!uart_console_hwflow_active(&up->port);
+	use_fifo = serial8250_console_use_fifo(up);
 
 	nbcon_exit_unsafe(wctxt);
 
-- 
2.43.0
注意：本电子邮件及其任何附件均归北京芯驰半导体科技股份有限公司及其各分子公司（"芯驰科技"）所有。本电子邮件及其附件仅供指定的收件人或收件实体使用，并可能包含芯驰科技的机密信息。禁止任何未经授权的使用、复制、传播或其他因依赖本电子邮件而采取或不采取的行为。如果您不是此电子邮件的预期收件人，请立即通过回复邮件通知发件人，并从您的系统中删除此邮件及任何附件。非常感谢。
Attention: This email and any attachment hereto belong to Beijing Semidrive Technology Corporation and each of its affiliates and subsidiaries ("SemiDrive Group"). This email and its attachments are intended solely for the use of the individual or entity to which they are addressed and may contain confidential information of SemiDrive Group. Any unauthorized use, copying, dissemination or other action taken or omitted in reliance upon this email is strictly prohibited. If you are not the intended recipient, please notify the sender immediately by reply email and delete this message and any attachments from your system. Thank you.

Re: [PATCH] serial: 8250: restore FIFO in serial8250_console_restore()

[Greg KH]

On Mon, Nov 02, 2026 at 05:36:09PM +0800, fuzheng.meng wrote:
> When console_suspend is disabled, the serial console stays enabled
> over a system suspend, and serial8250_suspend_port() writes a canary
> value to the SCR register (if the port has one) to detect whether the
> UART lost power during the sleep cycle.
> 
> If the hardware did lose power, all registers are reset, and the
> first console write after resume detects the canary mismatch and
> calls serial8250_console_restore(), which reprograms divisor, LCR and
> MCR, but not the FCR.
> 
> serial8250_console_write() decides whether to take the FIFO write
> path from the cached up->fcr, which still carries
> UART_FCR_ENABLE_FIFO. With the hardware FIFO left disabled by the
> reset, the transmit holding register holds only one byte, while
> serial8250_console_fifo_write() pushes up to tx_loadsz bytes into the
> port after waiting for THRE only once. The burst overwrites itself
> and the console output right after resume is garbled.
> 
> Factor the FIFO write path condition out into a new
> serial8250_console_use_fifo() helper and use it from both
> serial8250_console_write() and serial8250_console_restore(). Restore
> the FIFO in serial8250_console_restore() when the FIFO write path is
> going to be used: first write UART_FCR_ENABLE_FIFO, then the cached
> fcr, as dw8250_idle_exit() does after its registers are reset.
> 
> Tested on a DesignWare 8250 based platform with console_suspend=0
> over a suspend/resume cycle.
> 
> Fixes: 8f3631f0f6eb ("serial/8250: Use fifo in 8250 console driver")
> Cc: stable@vger.kernel.org
> Signed-off-by: fuzheng.meng <fuzheng.meng@semidrive.com>

Real name please, not an email alias.

thanks,

greg k-h

[PATCH v2] serial: 8250: restore FIFO in serial8250_console_restore()

[Fuzheng Meng]

When console_suspend is disabled, the serial console stays enabled
over a system suspend, and serial8250_suspend_port() writes a canary
value to the SCR register (if the port has one) to detect whether the
UART lost power during the sleep cycle.

If the hardware did lose power, all registers are reset, and the
first console write after resume detects the canary mismatch and
calls serial8250_console_restore(), which reprograms divisor, LCR and
MCR, but not the FCR.

serial8250_console_write() decides whether to take the FIFO write
path from the cached up->fcr, which still carries
UART_FCR_ENABLE_FIFO. With the hardware FIFO left disabled by the
reset, the transmit holding register holds only one byte, while
serial8250_console_fifo_write() pushes up to tx_loadsz bytes into the
port after waiting for THRE only once. The burst overwrites itself
and the console output right after resume is garbled.

Factor the FIFO write path condition out into a new
serial8250_console_use_fifo() helper and use it from both
serial8250_console_write() and serial8250_console_restore(). Restore
the FIFO in serial8250_console_restore() when the FIFO write path is
going to be used: first write UART_FCR_ENABLE_FIFO, then the cached
fcr, as dw8250_idle_exit() does after its registers are reset.

Tested on a DesignWare 8250 based platform with console_suspend=0
over a suspend/resume cycle.

Fixes: 8f3631f0f6eb ("serial/8250: Use fifo in 8250 console driver")
Cc: stable@vger.kernel.org
Signed-off-by: Fuzheng Meng <fuzheng.meng@semidrive.com>
---
Changes in v2:
- Use my real name (Fuzheng Meng) in the From: header and Signed-off-by
  tag instead of the account alias, as requested in the review of v1.
  No code changes.
 drivers/tty/serial/8250/8250_port.c | 64 ++++++++++++++++++++---------
 1 file changed, 45 insertions(+), 19 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
index 36b2b0a2c05c..ee6b02cff015 100644
--- a/drivers/tty/serial/8250/8250_port.c
+++ b/drivers/tty/serial/8250/8250_port.c
@@ -3321,6 +3321,37 @@ static void serial8250_console_wait_putchar(struct uart_port *port, unsigned cha
 	serial8250_console_putchar(port, ch);
 }
 
+/*
+ * Console output can go through the TX FIFO in one tx_loadsz chunk only
+ * if the port has a FIFO and the software state expects it enabled. The
+ * same conditions tell console_restore() whether the FIFO has to be
+ * brought back after a system sleep reset.
+ */
+static bool serial8250_console_use_fifo(struct uart_8250_port *up)
+{
+	struct uart_port *port = &up->port;
+
+	return (up->capabilities & UART_CAP_FIFO) &&
+		/*
+		 * BCM283x requires to check the fifo
+		 * after each byte.
+		 */
+		!(up->capabilities & UART_CAP_MINI) &&
+		/*
+		 * tx_loadsz contains the transmit fifo size
+		 */
+		up->tx_loadsz > 1 &&
+		(up->fcr & UART_FCR_ENABLE_FIFO) &&
+		port->state &&
+		test_bit(TTY_PORT_INITIALIZED, &port->state->port.iflags) &&
+		/*
+		 * After we put a data in the fifo, the controller will send
+		 * it regardless of the CTS state. Therefore, only use fifo
+		 * if we don't use control flow.
+		 */
+		!uart_console_hwflow_active(port);
+}
+
 /*
  *	Restore serial console when h/w power-off detected
  */
@@ -3345,6 +3376,19 @@ static void serial8250_console_restore(struct uart_8250_port *up)
 	serial8250_set_divisor(port, baud, quot, frac);
 	serial_port_out(port, UART_LCR, up->lcr);
 	serial8250_out_MCR(up, up->mcr | UART_MCR_DTR | UART_MCR_RTS);
+
+	/*
+	 * System sleep may reset the FCR, leaving the FIFO disabled,
+	 * while the cached up->fcr still tells console_write() to push
+	 * tx_loadsz bytes into the FIFO at once; with the hardware FIFO
+	 * off, the THR holds only one byte and earlier ones are
+	 * overwritten, which garbles the console output after resume.
+	 * Restore the FCR when the FIFO write path is going to be used.
+	 */
+	if (serial8250_console_use_fifo(up)) {
+		serial_port_out(port, UART_FCR, UART_FCR_ENABLE_FIFO);
+		serial_port_out(port, UART_FCR, up->fcr);
+	}
 }
 
 /*
@@ -3495,25 +3539,7 @@ void serial8250_console_write(struct uart_8250_port *up,
 		mdelay(port->rs485.delay_rts_before_send);
 	}
 
-	use_fifo = (up->capabilities & UART_CAP_FIFO) &&
-		/*
-		 * BCM283x requires to check the fifo
-		 * after each byte.
-		 */
-		!(up->capabilities & UART_CAP_MINI) &&
-		/*
-		 * tx_loadsz contains the transmit fifo size
-		 */
-		up->tx_loadsz > 1 &&
-		(up->fcr & UART_FCR_ENABLE_FIFO) &&
-		port->state &&
-		test_bit(TTY_PORT_INITIALIZED, &port->state->port.iflags) &&
-		/*
-		 * After we put a data in the fifo, the controller will send
-		 * it regardless of the CTS state. Therefore, only use fifo
-		 * if we don't use control flow.
-		 */
-		!uart_console_hwflow_active(&up->port);
+	use_fifo = serial8250_console_use_fifo(up);
 
 	nbcon_exit_unsafe(wctxt);
 
-- 
2.43.0
注意：本电子邮件及其任何附件均归北京芯驰半导体科技股份有限公司及其各分子公司（"芯驰科技"）所有。本电子邮件及其附件仅供指定的收件人或收件实体使用，并可能包含芯驰科技的机密信息。禁止任何未经授权的使用、复制、传播或其他因依赖本电子邮件而采取或不采取的行为。如果您不是此电子邮件的预期收件人，请立即通过回复邮件通知发件人，并从您的系统中删除此邮件及任何附件。非常感谢。
Attention: This email and any attachment hereto belong to Beijing Semidrive Technology Corporation and each of its affiliates and subsidiaries ("SemiDrive Group"). This email and its attachments are intended solely for the use of the individual or entity to which they are addressed and may contain confidential information of SemiDrive Group. Any unauthorized use, copying, dissemination or other action taken or omitted in reliance upon this email is strictly prohibited. If you are not the intended recipient, please notify the sender immediately by reply email and delete this message and any attachments from your system. Thank you.