[PATCH tty v5 0/3] 8250: Add console flow control

[PATCH tty v5 0/3] 8250: Add console flow control

[John Ogness]

Hi,

As requested by Greg [0], this is a resend of the last 3 patches from
v4 of a series to implement console flow control for the 8250 serial
driver. v4 is here [1] (including all the details about the entirety
of the series).

John Ogness

[0] https://lore.kernel.org/lkml/2026051134-revisable-sherry-e5b5@gregkh
[1] https://lore.kernel.org/lkml/20260506121606.5805-1-john.ogness@linutronix.de

John Ogness (3):
  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/serial_core.c    | 21 ++++++++++++++++++++-
 include/linux/serial_core.h         |  8 ++++++++
 4 files changed, 51 insertions(+), 5 deletions(-)


base-commit: 4a9a0b1a82a8b23eb68032dd19b120e82cd67004
-- 
2.47.3

[PATCH tty v5 1/3] serial: 8250: Set cons_flow on port registration

[John Ogness]

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..b0275204e1167 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_enabled(&uart->port, uart_cons_flow_enabled(&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

[PATCH tty v5 2/3] serial: 8250: Check LSR timeout on console flow control

[John Ogness]

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 e4e6a53ebea39..fe2e0f1e66c21 100644
--- a/drivers/tty/serial/8250/8250_port.c
+++ b/drivers/tty/serial/8250/8250_port.c
@@ -1986,16 +1986,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_cons_flow_enabled(&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

[PATCH tty v5 3/3] serial: 8250: Add support for console flow control

[John Ogness]

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 b0275204e1167..1f03da85e3414 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_cons_flow_enabled(&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_enabled(&uart->port, uart_cons_flow_enabled(&up->port));
+	uart_set_cons_flow_enabled(&uart->port, uart_cons_flow_enabled(&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 fe2e0f1e66c21..ef245114105bc 100644
--- a/drivers/tty/serial/8250/8250_port.c
+++ b/drivers/tty/serial/8250/8250_port.c
@@ -1991,7 +1991,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_cons_flow_enabled(&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;
@@ -2788,6 +2788,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);
 	}
 
@@ -3357,7 +3363,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_cons_flow_enabled(&up->port);
+		!uart_console_hwflow_active(&up->port);
 
 	if (likely(use_fifo))
 		serial8250_console_fifo_write(up, s, count);
@@ -3427,6 +3433,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_enabled(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 4f7bbdd900176..17fcff466e301 100644
--- a/include/linux/serial_core.h
+++ b/include/linux/serial_core.h
@@ -1175,6 +1175,14 @@ static inline bool uart_cons_flow_enabled(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_cons_flow_enabled(uport) &&
+	       uart_cts_enabled(uport);
+}
+
 /*
  * The following are helper functions for the low level drivers.
  */
-- 
2.47.3

Re: [PATCH tty v5 3/3] serial: 8250: Add support for console flow control

[Ilpo Järvinen]

On Mon, 11 May 2026, John Ogness wrote:

> 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 b0275204e1167..1f03da85e3414 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_cons_flow_enabled(&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_enabled(&uart->port, uart_cons_flow_enabled(&up->port));
> +	uart_set_cons_flow_enabled(&uart->port, uart_cons_flow_enabled(&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 fe2e0f1e66c21..ef245114105bc 100644
> --- a/drivers/tty/serial/8250/8250_port.c
> +++ b/drivers/tty/serial/8250/8250_port.c
> @@ -1991,7 +1991,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_cons_flow_enabled(&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;
> @@ -2788,6 +2788,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);
>  	}
>  
> @@ -3357,7 +3363,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_cons_flow_enabled(&up->port);
> +		!uart_console_hwflow_active(&up->port);
>  
>  	if (likely(use_fifo))
>  		serial8250_console_fifo_write(up, s, count);
> @@ -3427,6 +3433,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_enabled(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 4f7bbdd900176..17fcff466e301 100644
> --- a/include/linux/serial_core.h
> +++ b/include/linux/serial_core.h
> @@ -1175,6 +1175,14 @@ static inline bool uart_cons_flow_enabled(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_cons_flow_enabled(uport) &&
> +	       uart_cts_enabled(uport);
> +}
> +
>  /*
>   * The following are helper functions for the low level drivers.
>   */
> 

Hi,

Did you miss Andy's comments or choose to not act on them?

-- 
 i.

Re: [PATCH tty v5 3/3] serial: 8250: Add support for console flow control

[John Ogness]

On 2026-05-11, Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> wrote:
>> diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
>> index 4f7bbdd900176..17fcff466e301 100644
>> --- a/include/linux/serial_core.h
>> +++ b/include/linux/serial_core.h
>> @@ -1175,6 +1175,14 @@ static inline bool uart_cons_flow_enabled(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_cons_flow_enabled(uport) &&
>> +	       uart_cts_enabled(uport);
>> +}
>> +
>>  /*
>>   * The following are helper functions for the low level drivers.
>>   */
>
> Did you miss Andy's comments or choose to not act on them?

You mean these [0]? For the getter, yes. (Although I left the "is_" off
so that it matches other existing flag checking functions.)

For the setter, Andy suggested explicit enable/disable
variants. However, all the users enable/disable based on some
condition. That would mean that there would be a repeated pattern of:

	if (condition)
		uart_cons_flow_enable();
	else
		uart_cons_flow_disable();

So I decided to keep a single setter.

	uart_set_cons_flow_enabled(condition);

I renamed the setter so that it is clearer that only the enabled flag is
being set and not any other special procedures.

John Ogness

[0] https://lore.kernel.org/lkml/CAHp75Vf4YZVqXa7eH-RFeVsycdzoHijcWFnUbGv2PSmtPya1-w@mail.gmail.com