public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH tty v3 0/6] 8250: Add console flow control
@ 2026-04-17 10:24 John Ogness
  2026-04-17 10:24 ` [PATCH tty v3 1/6] serial: core: Add dedicated uart_port field for console flow John Ogness
                   ` (5 more replies)
  0 siblings, 6 replies; 8+ messages in thread
From: John Ogness @ 2026-04-17 10:24 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Jiri Slaby
  Cc: linux-kernel, linux-serial, Krzysztof Kozlowski, Alim Akhtar,
	David S. Miller, Ilpo Järvinen, Andy Shevchenko,
	Thomas Fourier, Kees Cook, linux-arm-kernel, linux-samsung-soc,
	sparclinux, Biju Das, Geert Uytterhoeven, Lad Prabhakar,
	Wolfram Sang, Thierry Bultel, Thomas Gleixner, Osama Abdelkader,
	Xin Zhao, Ingo Molnar, Andy Shevchenko, Krzysztof Kozlowski,
	Gerhard Engleder, Lukas Wunner, Dr. David Alan Gilbert,
	Joseph Tilahun

Hi,

This is v3 of a series to implement console flow control for the
8250 serial driver. v2 is here [0].

The 8250 driver already has code in place to support console flow
control. However, there is no way to activate it and it is
incomplete. This series provides the necessary missing pieces while
attempting to be as conservative as possible, so as not to introduce
any side effects into the many 8250 variants or other non-8250 serial
drivers.

Note that as of v3 I am now deprecating UPF_CONS_FLOW in favor of a
separate boolean field. In the commit message (patch 1) I explain my
reasoning for this decision.

For patch 2 I used the following Coccinelle script to perform the
modifications...

===== BEGIN cons_flow.cocci =====
// SPDX-License-Identifier: GPL-2.0-only
// Options: --all-includes

virtual patch

@r1@
type T1;
identifier U;
@@

T1 {
   ...
   struct uart_port U;
   ...
};

@r2@
r1.T1 *E;
@@

- (E->port.flags & UPF_CONS_FLOW)
+ uart_get_cons_flow(&E->port)

@r3@
struct uart_port *U;
@@

- (U->flags & UPF_CONS_FLOW)
+ uart_get_cons_flow(U)

@r4@
struct uart_port *U;
@@

- U->flags |= UPF_CONS_FLOW
+ uart_set_cons_flow(U, true)
===== END cons_flow.cocci =====

Changes since v2:

- Deprecate UPF_CONS_FLOW. Provide separate boolean with wrappers as
  alternative.

- Update all UPF_CONS_FLOW users to new cons_flow wrappers.

- Use irqsave variant of spin lock for status update.

- When 8250 console flow control is not specified, clear the policy.

Changes since v1:

- Prepend a patch to perform an extra LSR wait after CTS assertion if
  the initial LSR wait timed out.

- Close a window in serial8250_register_8250_port() where console
  flow control was briefly disabled.

- Add port lock synchronization to the port->status RMW update in
  uart_set_options().

John Ogness

[0] https://lore.kernel.org/lkml/20260410144949.16581-1-john.ogness@linutronix.de

John Ogness (6):
  serial: core: Add dedicated uart_port field for console flow
  serial: Replace driver usage of UPF_CONS_FLOW
  serial: sh-sci: Avoid deprecated UPF_CONS_FLOW
  serial: 8250: Set cons_flow on port registration
  serial: 8250: Check LSR timeout on console flow control
  serial: 8250: Add support for console flow control

 drivers/tty/serial/8250/8250_core.c |  6 ++++++
 drivers/tty/serial/8250/8250_port.c | 21 +++++++++++++++++----
 drivers/tty/serial/bcm63xx_uart.c   |  2 +-
 drivers/tty/serial/omap-serial.c    |  2 +-
 drivers/tty/serial/pch_uart.c       |  2 +-
 drivers/tty/serial/pxa.c            |  2 +-
 drivers/tty/serial/samsung_tty.c    |  8 ++++----
 drivers/tty/serial/serial_core.c    | 21 ++++++++++++++++++++-
 drivers/tty/serial/serial_txx9.c    |  4 ++--
 drivers/tty/serial/sh-sci.c         |  5 ++++-
 drivers/tty/serial/sunsu.c          |  2 +-
 include/linux/serial_core.h         | 20 ++++++++++++++++++++
 12 files changed, 78 insertions(+), 17 deletions(-)


base-commit: a1a81aef99e853dec84241d701fbf587d713eb5b
-- 
2.47.3


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH tty v3 1/6] serial: core: Add dedicated uart_port field for console flow
  2026-04-17 10:24 [PATCH tty v3 0/6] 8250: Add console flow control John Ogness
@ 2026-04-17 10:24 ` John Ogness
  2026-04-17 13:15   ` John Ogness
  2026-04-17 10:24 ` [PATCH tty v3 2/6] serial: Replace driver usage of UPF_CONS_FLOW John Ogness
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 8+ messages in thread
From: John Ogness @ 2026-04-17 10:24 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Jiri Slaby; +Cc: linux-kernel, linux-serial

Currently the UPF_CONS_FLOW bit in the uart_port.flags field is used
by serial console drivers to identify if a user has configured flow
control on the console. This policy is most commonly setup during
early boot, but can be changed at runtime.

The bits in uart_port.flags are either hardware and driver properties
that are initialized before usage or are properties that can be
changed via the tty layer.

The UPF_CONS_FLOW is an exception because it is a console-only policy
that can change at runtime and its setting and usage have nothing to
do with the tty layer. This actually causes a problem for its usage
because uart_port.flags is synchronized by a related tty_port.mutex,
but a console has no relation to a tty (other than sharing the port).

This is probably why console flow control is not properly available
for most serial drivers. And it is hindering being able to provide a
proper implementation. Commit d01f4d181c92 ("serial: core: Privatize
tty->hw_stopped") addressed a similar issue to deal with software
assisted CTS flow state tracking.

Add a new uart_port boolean field "cons_flow" to store the user
configuration for console flow control. Add get/set wrappers to allow
for adding more policies later and/or locking constraint validation.

Mark UPF_CONS_FLOW as deprecated.

Signed-off-by: John Ogness <john.ogness@linutronix.de>
---
 include/linux/serial_core.h | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
index 666430b478997..2327a364ded16 100644
--- a/include/linux/serial_core.h
+++ b/include/linux/serial_core.h
@@ -533,6 +533,7 @@ struct uart_port {
 #define UPF_HARD_FLOW		((__force upf_t) (UPF_AUTO_CTS | UPF_AUTO_RTS))
 /* Port has hardware-assisted s/w flow control */
 #define UPF_SOFT_FLOW		((__force upf_t) BIT_ULL(22))
+/* Deprecated: use uart_get_cons_flow()/uart_set_cons_flow() instead. */
 #define UPF_CONS_FLOW		((__force upf_t) BIT_ULL(23))
 #define UPF_SHARE_IRQ		((__force upf_t) BIT_ULL(24))
 #define UPF_EXAR_EFR		((__force upf_t) BIT_ULL(25))
@@ -567,6 +568,7 @@ struct uart_port {
 #define UPSTAT_SYNC_FIFO	((__force upstat_t) (1 << 5))
 
 	bool			hw_stopped;		/* sw-assisted CTS flow state */
+	bool			cons_flow;		/* user specified console flow control */
 	unsigned int		mctrl;			/* current modem ctrl settings */
 	unsigned int		frame_time;		/* frame timing in ns */
 	unsigned int		type;			/* port type */
@@ -1163,6 +1165,16 @@ static inline bool uart_softcts_mode(struct uart_port *uport)
 	return ((uport->status & mask) == UPSTAT_CTS_ENABLE);
 }
 
+static inline void uart_set_cons_flow(struct uart_port *uport, bool on)
+{
+	uport->cons_flow = on;
+}
+
+static inline bool uart_get_cons_flow(const struct uart_port *uport)
+{
+	return uport->cons_flow;
+}
+
 /*
  * The following are helper functions for the low level drivers.
  */
-- 
2.47.3


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH tty v3 2/6] serial: Replace driver usage of UPF_CONS_FLOW
  2026-04-17 10:24 [PATCH tty v3 0/6] 8250: Add console flow control John Ogness
  2026-04-17 10:24 ` [PATCH tty v3 1/6] serial: core: Add dedicated uart_port field for console flow John Ogness
@ 2026-04-17 10:24 ` John Ogness
  2026-04-17 10:24 ` [PATCH tty v3 3/6] serial: sh-sci: Avoid deprecated UPF_CONS_FLOW John Ogness
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 8+ messages in thread
From: John Ogness @ 2026-04-17 10:24 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Jiri Slaby
  Cc: linux-kernel, Krzysztof Kozlowski, Alim Akhtar, David S. Miller,
	Ilpo Järvinen, Andy Shevchenko, Thomas Fourier, Kees Cook,
	linux-serial, linux-arm-kernel, linux-samsung-soc, sparclinux

Rather than using the UPF_CONS_FLOW bit of uart_port.flags to track
the user configuration of console flow control, use the newly added
uart_port.cons_flow (via its get/set functions).

A coccinelle script was used to perform the search/replace.

Note1: The sh-sci driver is blindly copying platform data
       configuration flags to uart_port.flags. Thus UPF_CONS_FLOW
       could get set. A follow-up commit will address this.

Note2: Aside from sh-sci, the samsung_tty driver is also using
       UPF_CONS_FLOW as a platform data configuration flag.

Signed-off-by: John Ogness <john.ogness@linutronix.de>
---
 drivers/tty/serial/8250/8250_port.c | 4 ++--
 drivers/tty/serial/bcm63xx_uart.c   | 2 +-
 drivers/tty/serial/omap-serial.c    | 2 +-
 drivers/tty/serial/pch_uart.c       | 2 +-
 drivers/tty/serial/pxa.c            | 2 +-
 drivers/tty/serial/samsung_tty.c    | 8 ++++----
 drivers/tty/serial/serial_txx9.c    | 4 ++--
 drivers/tty/serial/sunsu.c          | 2 +-
 8 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
index af78cc02f38e7..c91b0fa7111a7 100644
--- a/drivers/tty/serial/8250/8250_port.c
+++ b/drivers/tty/serial/8250/8250_port.c
@@ -1988,7 +1988,7 @@ static void wait_for_xmitr(struct uart_8250_port *up, int bits)
 	wait_for_lsr(up, bits);
 
 	/* Wait up to 1s for flow control if necessary */
-	if (up->port.flags & UPF_CONS_FLOW) {
+	if (uart_get_cons_flow(&up->port)) {
 		for (tmout = 1000000; tmout; tmout--) {
 			unsigned int msr = serial_in(up, UART_MSR);
 			up->msr_saved_flags |= msr & MSR_SAVE_FLAGS;
@@ -3351,7 +3351,7 @@ void serial8250_console_write(struct uart_8250_port *up, const char *s,
 		 * it regardless of the CTS state. Therefore, only use fifo
 		 * if we don't use control flow.
 		 */
-		!(up->port.flags & UPF_CONS_FLOW);
+		!uart_get_cons_flow(&up->port);
 
 	if (likely(use_fifo))
 		serial8250_console_fifo_write(up, s, count);
diff --git a/drivers/tty/serial/bcm63xx_uart.c b/drivers/tty/serial/bcm63xx_uart.c
index 51df9d2d8bfc5..be6777dfdc532 100644
--- a/drivers/tty/serial/bcm63xx_uart.c
+++ b/drivers/tty/serial/bcm63xx_uart.c
@@ -675,7 +675,7 @@ static void wait_for_xmitr(struct uart_port *port)
 	}
 
 	/* Wait up to 1s for flow control if necessary */
-	if (port->flags & UPF_CONS_FLOW) {
+	if (uart_get_cons_flow(port)) {
 		tmout = 1000000;
 		while (--tmout) {
 			unsigned int val;
diff --git a/drivers/tty/serial/omap-serial.c b/drivers/tty/serial/omap-serial.c
index 0b85f47ff19e0..a9879bc655745 100644
--- a/drivers/tty/serial/omap-serial.c
+++ b/drivers/tty/serial/omap-serial.c
@@ -1092,7 +1092,7 @@ static void __maybe_unused wait_for_xmitr(struct uart_omap_port *up)
 	} while (!uart_lsr_tx_empty(status));
 
 	/* Wait up to 1s for flow control if necessary */
-	if (up->port.flags & UPF_CONS_FLOW) {
+	if (uart_get_cons_flow(&up->port)) {
 		for (tmout = 1000000; tmout; tmout--) {
 			unsigned int msr = serial_in(up, UART_MSR);
 
diff --git a/drivers/tty/serial/pch_uart.c b/drivers/tty/serial/pch_uart.c
index 6729d8e83c3c5..08cb9ff30506f 100644
--- a/drivers/tty/serial/pch_uart.c
+++ b/drivers/tty/serial/pch_uart.c
@@ -1444,7 +1444,7 @@ static void wait_for_xmitr(struct eg20t_port *up, int bits)
 	}
 
 	/* Wait up to 1s for flow control if necessary */
-	if (up->port.flags & UPF_CONS_FLOW) {
+	if (uart_get_cons_flow(&up->port)) {
 		unsigned int tmout;
 		for (tmout = 1000000; tmout; tmout--) {
 			unsigned int msr = ioread8(up->membase + UART_MSR);
diff --git a/drivers/tty/serial/pxa.c b/drivers/tty/serial/pxa.c
index fea0255067ccd..80afa47f09880 100644
--- a/drivers/tty/serial/pxa.c
+++ b/drivers/tty/serial/pxa.c
@@ -573,7 +573,7 @@ static void wait_for_xmitr(struct uart_pxa_port *up)
 	} while (!uart_lsr_tx_empty(status));
 
 	/* Wait up to 1s for flow control if necessary */
-	if (up->port.flags & UPF_CONS_FLOW) {
+	if (uart_get_cons_flow(&up->port)) {
 		tmout = 1000000;
 		while (--tmout &&
 		       ((serial_in(up, UART_MSR) & UART_MSR_CTS) == 0))
diff --git a/drivers/tty/serial/samsung_tty.c b/drivers/tty/serial/samsung_tty.c
index e27806bf2cf3e..f9b0dbded1f43 100644
--- a/drivers/tty/serial/samsung_tty.c
+++ b/drivers/tty/serial/samsung_tty.c
@@ -319,7 +319,7 @@ static void s3c24xx_serial_stop_tx(struct uart_port *port)
 	ourport->tx_enabled = 0;
 	ourport->tx_in_progress = 0;
 
-	if (port->flags & UPF_CONS_FLOW)
+	if (uart_get_cons_flow(port))
 		s3c24xx_serial_rx_enable(port);
 
 	ourport->tx_mode = 0;
@@ -493,7 +493,7 @@ static void s3c24xx_serial_start_tx(struct uart_port *port)
 	struct tty_port *tport = &port->state->port;
 
 	if (!ourport->tx_enabled) {
-		if (port->flags & UPF_CONS_FLOW)
+		if (uart_get_cons_flow(port))
 			s3c24xx_serial_rx_disable(port);
 
 		ourport->tx_enabled = 1;
@@ -781,7 +781,7 @@ static void s3c24xx_serial_rx_drain_fifo(struct s3c24xx_uart_port *ourport)
 		uerstat = rd_regl(port, S3C2410_UERSTAT);
 		ch = rd_reg(port, S3C2410_URXH);
 
-		if (port->flags & UPF_CONS_FLOW) {
+		if (uart_get_cons_flow(port)) {
 			bool txe = s3c24xx_serial_txempty_nofifo(port);
 
 			if (ourport->rx_enabled) {
@@ -1830,7 +1830,7 @@ static int s3c24xx_serial_init_port(struct s3c24xx_uart_port *ourport,
 
 	if (cfg->uart_flags & UPF_CONS_FLOW) {
 		dev_dbg(port->dev, "enabling flow control\n");
-		port->flags |= UPF_CONS_FLOW;
+		uart_set_cons_flow(port, true);
 	}
 
 	/* sort our the physical and virtual addresses for each UART */
diff --git a/drivers/tty/serial/serial_txx9.c b/drivers/tty/serial/serial_txx9.c
index 436a559234dfe..103f03c1fe748 100644
--- a/drivers/tty/serial/serial_txx9.c
+++ b/drivers/tty/serial/serial_txx9.c
@@ -422,7 +422,7 @@ static void wait_for_xmitr(struct uart_port *up)
 		udelay(1);
 
 	/* Wait up to 1s for flow control if necessary */
-	if (up->flags & UPF_CONS_FLOW) {
+	if (uart_get_cons_flow(up)) {
 		tmout = 1000000;
 		while (--tmout &&
 		       (sio_in(up, TXX9_SICISR) & TXX9_SICISR_CTSS))
@@ -857,7 +857,7 @@ serial_txx9_console_write(struct console *co, const char *s, unsigned int count)
 	 *	Disable flow-control if enabled (and unnecessary)
 	 */
 	flcr = sio_in(up, TXX9_SIFLCR);
-	if (!(up->flags & UPF_CONS_FLOW) && (flcr & TXX9_SIFLCR_TES))
+	if (!uart_get_cons_flow(up) && (flcr & TXX9_SIFLCR_TES))
 		sio_out(up, TXX9_SIFLCR, flcr & ~TXX9_SIFLCR_TES);
 
 	uart_console_write(up, s, count, serial_txx9_console_putchar);
diff --git a/drivers/tty/serial/sunsu.c b/drivers/tty/serial/sunsu.c
index 6505a1930da9a..97019b5ec49e2 100644
--- a/drivers/tty/serial/sunsu.c
+++ b/drivers/tty/serial/sunsu.c
@@ -1245,7 +1245,7 @@ static void wait_for_xmitr(struct uart_sunsu_port *up)
 	} while (!uart_lsr_tx_empty(status));
 
 	/* Wait up to 1s for flow control if necessary */
-	if (up->port.flags & UPF_CONS_FLOW) {
+	if (uart_get_cons_flow(&up->port)) {
 		tmout = 1000000;
 		while (--tmout &&
 		       ((serial_in(up, UART_MSR) & UART_MSR_CTS) == 0))
-- 
2.47.3


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH tty v3 3/6] serial: sh-sci: Avoid deprecated UPF_CONS_FLOW
  2026-04-17 10:24 [PATCH tty v3 0/6] 8250: Add console flow control John Ogness
  2026-04-17 10:24 ` [PATCH tty v3 1/6] serial: core: Add dedicated uart_port field for console flow John Ogness
  2026-04-17 10:24 ` [PATCH tty v3 2/6] serial: Replace driver usage of UPF_CONS_FLOW John Ogness
@ 2026-04-17 10:24 ` John Ogness
  2026-04-17 10:24 ` [PATCH tty v3 4/6] serial: 8250: Set cons_flow on port registration John Ogness
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 8+ messages in thread
From: John Ogness @ 2026-04-17 10:24 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Jiri Slaby
  Cc: linux-kernel, Biju Das, Geert Uytterhoeven, Lad Prabhakar,
	Wolfram Sang, Thierry Bultel, linux-serial

Avoid setting the uart_port.flags deprecated UPF_CONS_FLOW bit if it
has been configured in the platform data. Use the new cons_flow
wrappers instead.

Signed-off-by: John Ogness <john.ogness@linutronix.de>
---
 drivers/tty/serial/sh-sci.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
index 6c819b6b24258..07d272dc4e74a 100644
--- a/drivers/tty/serial/sh-sci.c
+++ b/drivers/tty/serial/sh-sci.c
@@ -3369,9 +3369,12 @@ static int sci_init_single(struct platform_device *dev,
 	}
 
 	port->type		= SCI_PUBLIC_PORT_ID(p->type);
-	port->flags		= UPF_FIXED_PORT | UPF_BOOT_AUTOCONF | p->flags;
+	port->flags		= UPF_FIXED_PORT | UPF_BOOT_AUTOCONF |
+				  (p->flags & ~UPF_CONS_FLOW);
 	port->fifosize		= sci_port->params->fifosize;
 
+	uart_set_cons_flow(port, p->flags & UPF_CONS_FLOW);
+
 	if (p->type == PORT_SCI && !dev->dev.of_node) {
 		if (sci_port->reg_size >= 0x20)
 			port->regshift = 2;
-- 
2.47.3


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH tty v3 4/6] serial: 8250: Set cons_flow on port registration
  2026-04-17 10:24 [PATCH tty v3 0/6] 8250: Add console flow control John Ogness
                   ` (2 preceding siblings ...)
  2026-04-17 10:24 ` [PATCH tty v3 3/6] serial: sh-sci: Avoid deprecated UPF_CONS_FLOW John Ogness
@ 2026-04-17 10:24 ` John Ogness
  2026-04-17 10:24 ` [PATCH tty v3 5/6] serial: 8250: Check LSR timeout on console flow control John Ogness
  2026-04-17 10:24 ` [PATCH tty v3 6/6] serial: 8250: Add support for " John Ogness
  5 siblings, 0 replies; 8+ messages in thread
From: John Ogness @ 2026-04-17 10:24 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Jiri Slaby
  Cc: linux-kernel, Ilpo Järvinen, Thomas Gleixner,
	Osama Abdelkader, Xin Zhao, Ingo Molnar, linux-serial

Since console flow control policy is no longer part of uart_port.flags,
explicitly set the policy for the port.

Signed-off-by: John Ogness <john.ogness@linutronix.de>
---
 drivers/tty/serial/8250/8250_core.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
index a428e88938eb7..32b894ab26768 100644
--- a/drivers/tty/serial/8250/8250_core.c
+++ b/drivers/tty/serial/8250/8250_core.c
@@ -746,6 +746,8 @@ int serial8250_register_8250_port(const struct uart_8250_port *up)
 	uart->lsr_save_mask	= up->lsr_save_mask;
 	uart->dma		= up->dma;
 
+	uart_set_cons_flow(&uart->port, uart_get_cons_flow(&up->port));
+
 	/* Take tx_loadsz from fifosize if it wasn't set separately */
 	if (uart->port.fifosize && !uart->tx_loadsz)
 		uart->tx_loadsz = uart->port.fifosize;
-- 
2.47.3


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH tty v3 5/6] serial: 8250: Check LSR timeout on console flow control
  2026-04-17 10:24 [PATCH tty v3 0/6] 8250: Add console flow control John Ogness
                   ` (3 preceding siblings ...)
  2026-04-17 10:24 ` [PATCH tty v3 4/6] serial: 8250: Set cons_flow on port registration John Ogness
@ 2026-04-17 10:24 ` John Ogness
  2026-04-17 10:24 ` [PATCH tty v3 6/6] serial: 8250: Add support for " John Ogness
  5 siblings, 0 replies; 8+ messages in thread
From: John Ogness @ 2026-04-17 10:24 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Jiri Slaby
  Cc: linux-kernel, Ilpo Järvinen, Andy Shevchenko, linux-serial

wait_for_xmitr() calls wait_for_lsr() to wait for the transmission
registers to be empty. wait_for_lsr() can timeout after a reasonable
amount of time.

When console flow control is active, wait_for_xmitr() additionally
polls CTS, waiting for the peer to signal that it is ready to receive
more data.

If hardware flow control is enabled (auto CTS) and the peer deasserts
CTS, wait_for_lsr() will timeout. If additionally console flow
control is active and while polling CTS the peer asserts CTS, the
console will assume it can immediately transmit, even though the
transmission registers may not be empty. This can lead to data loss.

Avoid this problem by performing an extra wait_for_lsr() upon CTS
assertion if wait_for_lsr() previously timed out.

Signed-off-by: John Ogness <john.ogness@linutronix.de>
---
 drivers/tty/serial/8250/8250_port.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
index c91b0fa7111a7..b3247c55eebd5 100644
--- a/drivers/tty/serial/8250/8250_port.c
+++ b/drivers/tty/serial/8250/8250_port.c
@@ -1984,16 +1984,20 @@ static bool wait_for_lsr(struct uart_8250_port *up, int bits)
 static void wait_for_xmitr(struct uart_8250_port *up, int bits)
 {
 	unsigned int tmout;
+	bool tx_ready;
 
-	wait_for_lsr(up, bits);
+	tx_ready = wait_for_lsr(up, bits);
 
 	/* Wait up to 1s for flow control if necessary */
 	if (uart_get_cons_flow(&up->port)) {
 		for (tmout = 1000000; tmout; tmout--) {
 			unsigned int msr = serial_in(up, UART_MSR);
 			up->msr_saved_flags |= msr & MSR_SAVE_FLAGS;
-			if (msr & UART_MSR_CTS)
+			if (msr & UART_MSR_CTS) {
+				if (!tx_ready)
+					wait_for_lsr(up, bits);
 				break;
+			}
 			udelay(1);
 			touch_nmi_watchdog();
 		}
-- 
2.47.3


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH tty v3 6/6] serial: 8250: Add support for console flow control
  2026-04-17 10:24 [PATCH tty v3 0/6] 8250: Add console flow control John Ogness
                   ` (4 preceding siblings ...)
  2026-04-17 10:24 ` [PATCH tty v3 5/6] serial: 8250: Check LSR timeout on console flow control John Ogness
@ 2026-04-17 10:24 ` John Ogness
  5 siblings, 0 replies; 8+ messages in thread
From: John Ogness @ 2026-04-17 10:24 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Jiri Slaby
  Cc: linux-kernel, Ilpo Järvinen, Ingo Molnar, Osama Abdelkader,
	Andy Shevchenko, Krzysztof Kozlowski, Gerhard Engleder,
	Lukas Wunner, Dr. David Alan Gilbert, Joseph Tilahun,
	linux-serial

The kernel documentation specifies that the console option 'r' can
be used to enable hardware flow control for console writes. The 8250
driver does include code for hardware flow control on the console if
cons_flow is set, but there is no code path that actually sets this.
However, that is not the only issue. The problems are:

1. Specifying the console option 'r' does not lead to cons_flow being
   set.

2. Even if cons_flow would be set, serial8250_register_8250_port()
   clears it.

3. When the console option 'r' is specified, uart_set_options()
   attempts to initialize the port for CRTSCTS. However, afterwards
   it does not set the UPSTAT_CTS_ENABLE status bit and therefore on
   boot, uart_cts_enabled() is always false. This policy bit is
   important for console drivers as a criteria if they may poll CTS.

4. Even though uart_set_options() attempts to initialize the port
   for CRTSCTS, the 8250 set_termios() callback does not enable the
   RTS signal (TIOCM_RTS) and thus the hardware is not properly
   initialized for CTS polling.

5. Even if modem control was properly setup for CTS polling
   (TIOCM_RTS), uart_configure_port() clears TIOCM_RTS, thus
   breaking CTS polling.

6. wait_for_xmitr() and serial8250_console_write() use cons_flow
   to decide if CTS polling should occur. However, the condition
   should also include a check that it is not in RS485 mode and
   CRTSCTS is actually enabled in the hardware.

Address all these issues as conservatively as possible by gating them
behind checks focussed on the user specifying console hardware flow
control support and the hardware being configured for CTS polling
at the time of the write to the UART.

Since checking the UPSTAT_CTS_ENABLE status bit is a part of the new
condition gate, these changes also support runtime termios updates to
disable/enable CRTSCTS.

Signed-off-by: John Ogness <john.ogness@linutronix.de>
---
 drivers/tty/serial/8250/8250_core.c |  6 +++++-
 drivers/tty/serial/8250/8250_port.c | 13 +++++++++++--
 drivers/tty/serial/serial_core.c    | 21 ++++++++++++++++++++-
 include/linux/serial_core.h         |  8 ++++++++
 4 files changed, 44 insertions(+), 4 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
index 32b894ab26768..a19646e24d883 100644
--- a/drivers/tty/serial/8250/8250_core.c
+++ b/drivers/tty/serial/8250/8250_core.c
@@ -693,6 +693,7 @@ static void serial_8250_overrun_backoff_work(struct work_struct *work)
 int serial8250_register_8250_port(const struct uart_8250_port *up)
 {
 	struct uart_8250_port *uart;
+	bool cons_flow;
 	int ret;
 
 	if (up->port.uartclk == 0)
@@ -716,6 +717,9 @@ int serial8250_register_8250_port(const struct uart_8250_port *up)
 	if (uart->port.type == PORT_8250_CIR)
 		return -ENODEV;
 
+	/* Preserve specified console flow control. */
+	cons_flow = uart_get_cons_flow(&uart->port);
+
 	if (uart->port.dev)
 		uart_remove_one_port(&serial8250_reg, &uart->port);
 
@@ -746,7 +750,7 @@ int serial8250_register_8250_port(const struct uart_8250_port *up)
 	uart->lsr_save_mask	= up->lsr_save_mask;
 	uart->dma		= up->dma;
 
-	uart_set_cons_flow(&uart->port, uart_get_cons_flow(&up->port));
+	uart_set_cons_flow(&uart->port, uart_get_cons_flow(&up->port) | cons_flow);
 
 	/* Take tx_loadsz from fifosize if it wasn't set separately */
 	if (uart->port.fifosize && !uart->tx_loadsz)
diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
index b3247c55eebd5..f585145a6211a 100644
--- a/drivers/tty/serial/8250/8250_port.c
+++ b/drivers/tty/serial/8250/8250_port.c
@@ -1989,7 +1989,7 @@ static void wait_for_xmitr(struct uart_8250_port *up, int bits)
 	tx_ready = wait_for_lsr(up, bits);
 
 	/* Wait up to 1s for flow control if necessary */
-	if (uart_get_cons_flow(&up->port)) {
+	if (uart_console_hwflow_active(&up->port)) {
 		for (tmout = 1000000; tmout; tmout--) {
 			unsigned int msr = serial_in(up, UART_MSR);
 			up->msr_saved_flags |= msr & MSR_SAVE_FLAGS;
@@ -2786,6 +2786,12 @@ serial8250_do_set_termios(struct uart_port *port, struct ktermios *termios,
 		serial8250_set_efr(port, termios);
 		serial8250_set_divisor(port, baud, quot, frac);
 		serial8250_set_fcr(port, termios);
+		/* Consoles manually poll CTS for hardware flow control. */
+		if (uart_console(port) &&
+		    !(port->rs485.flags & SER_RS485_ENABLED)
+		    && termios->c_cflag & CRTSCTS) {
+			port->mctrl |= TIOCM_RTS;
+		}
 		serial8250_set_mctrl(port, port->mctrl);
 	}
 
@@ -3355,7 +3361,7 @@ void serial8250_console_write(struct uart_8250_port *up, const char *s,
 		 * it regardless of the CTS state. Therefore, only use fifo
 		 * if we don't use control flow.
 		 */
-		!uart_get_cons_flow(&up->port);
+		!uart_console_hwflow_active(&up->port);
 
 	if (likely(use_fifo))
 		serial8250_console_fifo_write(up, s, count);
@@ -3425,6 +3431,9 @@ int serial8250_console_setup(struct uart_port *port, char *options, bool probe)
 	if (ret)
 		return ret;
 
+	/* Track user-specified console flow control. */
+	uart_set_cons_flow(port, flow == 'r');
+
 	if (port->dev)
 		pm_runtime_get_sync(port->dev);
 
diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
index 89cebdd278410..840336f95c5f6 100644
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@ -2235,6 +2235,18 @@ uart_set_options(struct uart_port *port, struct console *co,
 	port->mctrl |= TIOCM_DTR;
 
 	port->ops->set_termios(port, &termios, &dummy);
+
+	/*
+	 * If console hardware flow control was specified and is supported,
+	 * the related policy UPSTAT_CTS_ENABLE must be set to allow console
+	 * drivers to identify if CTS should be used for polling.
+	 */
+	if (flow == 'r' && (termios.c_cflag & CRTSCTS)) {
+		/* Synchronize @status RMW update against the console. */
+		guard(uart_port_lock_irqsave)(port);
+		port->status |= UPSTAT_CTS_ENABLE;
+	}
+
 	/*
 	 * Allow the setting of the UART parameters with a NULL console
 	 * too:
@@ -2541,7 +2553,14 @@ uart_configure_port(struct uart_driver *drv, struct uart_state *state,
 		 * We probably don't need a spinlock around this, but
 		 */
 		scoped_guard(uart_port_lock_irqsave, port) {
-			port->mctrl &= TIOCM_DTR;
+			unsigned int mask = TIOCM_DTR;
+
+			/* Console hardware flow control polls CTS. */
+			if (uart_console_hwflow_active(port))
+				mask |= TIOCM_RTS;
+
+			port->mctrl &= mask;
+
 			if (!(port->rs485.flags & SER_RS485_ENABLED))
 				port->ops->set_mctrl(port, port->mctrl);
 		}
diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
index 2327a364ded16..ff1145e86088b 100644
--- a/include/linux/serial_core.h
+++ b/include/linux/serial_core.h
@@ -1175,6 +1175,14 @@ static inline bool uart_get_cons_flow(const struct uart_port *uport)
 	return uport->cons_flow;
 }
 
+static inline bool uart_console_hwflow_active(struct uart_port *uport)
+{
+	return uart_console(uport) &&
+	       !(uport->rs485.flags & SER_RS485_ENABLED) &&
+	       uart_get_cons_flow(uport) &&
+	       uart_cts_enabled(uport);
+}
+
 /*
  * The following are helper functions for the low level drivers.
  */
-- 
2.47.3


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH tty v3 1/6] serial: core: Add dedicated uart_port field for console flow
  2026-04-17 10:24 ` [PATCH tty v3 1/6] serial: core: Add dedicated uart_port field for console flow John Ogness
@ 2026-04-17 13:15   ` John Ogness
  0 siblings, 0 replies; 8+ messages in thread
From: John Ogness @ 2026-04-17 13:15 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Jiri Slaby; +Cc: linux-kernel, linux-serial

On 2026-04-17, Sashiko wrote:
>> diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
>> index 666430b478997..2327a364ded16 100644
>> --- a/include/linux/serial_core.h
>> +++ b/include/linux/serial_core.h
>> @@ -567,6 +568,7 @@ struct uart_port {
>>  #define UPSTAT_SYNC_FIFO	((__force upstat_t) (1 << 5))
>>  
>>  	bool			hw_stopped;		/* sw-assisted CTS flow state */
>> +	bool			cons_flow;		/* user specified console flow control */
>>  	unsigned int		mctrl;			/* current modem ctrl settings */
>>  	unsigned int		frame_time;		/* frame timing in ns */
>>  	unsigned int		type;			/* port type */
>
> Since cons_flow is now separate from the flags field, do manual
> assignments of struct uart_port need to be updated to copy this new
> field?

My goal was to catch everywhere where uart_port.flags is set explicitly
or from some "external" source. I did not want to filter anytime one
uart_port.flags is to another uart_port.flags. If UPF_CONS_FLOW is set
in the source uart_port.flags, then _that_ is the problem.

> While subsequent patches in this series update
> serial8250_register_8250_port() to explicitly copy this new field,
> early_serial_setup() does not appear to receive a similar update.
>
> For example, looking at early_serial_setup() in
> drivers/tty/serial/8250/8250_core.c:
>
> int __init early_serial_setup(struct uart_port *port)
> {
> 	...
> 	p = &serial8250_ports[port->line].port;
> 	p->iobase       = port->iobase;
> 	...
> 	p->iotype       = port->iotype;
> 	p->flags        = port->flags;
> 	p->mapbase      = port->mapbase;
> 	...
> }
>
> Because UPF_CONS_FLOW was previously copied as part of port->flags,
> will omitting cons_flow here cause a regression where the
> configuration is silently lost if any platform sets console flow
> control prior to early registration?

In this particular case, I checked all callers of
early_serial_setup(). They are passing explicit flags and UPF_CONS_FLOW
is not one of them.

Likewise, I could not find any examples, where UPF_CONS_FLOW could slip
into uart_port.flags, aside from the 2 examples I found and deal with in
my series.

I have no control over out-of-tree drivers that call
early_serial_setup() with UPF_CONS_FLOW. I could sprinkle bitmasks and
WARN_ON's to try to catch such out-of-tree misfits. But honestly, I
think that would introduce unnecesary code churn and complexity.

Considering the current questionable kernel state of console flow
control today, I doubt any out-of-tree drivers are really using the
appropriate policies anyway... meaning that the out-of-tree driver is
probably just unsafely using (uart_port.flags & UPF_CONS_FLOW) now,
which would likely continue to work as good/bad as it does now.

John

P.S. It looks like Sashiko ran out of gas [0] trying to review the 2nd
patch. So hopefully another reviewer (human or !human) will find some
time to take a look at the series.

At this point my only real concern is the community opinions on
deprecating UPF_CONS_FLOW.

[0] https://sashiko.dev/#/patchset/20260417102423.40984-1-john.ogness%40linutronix.de

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2026-04-17 13:15 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-17 10:24 [PATCH tty v3 0/6] 8250: Add console flow control John Ogness
2026-04-17 10:24 ` [PATCH tty v3 1/6] serial: core: Add dedicated uart_port field for console flow John Ogness
2026-04-17 13:15   ` John Ogness
2026-04-17 10:24 ` [PATCH tty v3 2/6] serial: Replace driver usage of UPF_CONS_FLOW John Ogness
2026-04-17 10:24 ` [PATCH tty v3 3/6] serial: sh-sci: Avoid deprecated UPF_CONS_FLOW John Ogness
2026-04-17 10:24 ` [PATCH tty v3 4/6] serial: 8250: Set cons_flow on port registration John Ogness
2026-04-17 10:24 ` [PATCH tty v3 5/6] serial: 8250: Check LSR timeout on console flow control John Ogness
2026-04-17 10:24 ` [PATCH tty v3 6/6] serial: 8250: Add support for " John Ogness

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox