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

[Andy Shevchenko <andy.shevchenko@gmail.com>]

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