Re: [PATCH tty v3 6/6] serial: 8250: Add support for console flow control

Andy Shevchenko
2026-04-20T09:13:35Z

On Fri, Apr 17, 2026 at 1:24 PM John Ogness <john.ogness@linutronix.de> 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.

...

> struct uart_8250_port *uart;
> + bool cons_flow;

> + /* Preserve specified console flow control. */
> + cons_flow = uart_get_cons_flow(&uart->port);

Sorry for nit-picking, but when I see 'bool' and see a getter, I got
confused. I would rather see getter/setter to be something like

cons_flow = uart_is_cons_flow_enabled(&uart->port);
uart_enable_cons_flow(&uart->port);
uart_disable_cons_flow(&uart->port);

--
With Best Regards,
Andy Shevchenko