linux-kernel - Re: [PATCH] tty: serial: fsl_lpuart: avoid idle preamble pending if CTS is enabled

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <2e1b1eae2e9d3cedcd270e35cfcf8086b914b7ff.camel@siemens.com>
Date: Mon, 4 Mar 2024 07:13:32 +0000
From: "Sverdlin, Alexander" <alexander.sverdlin@...mens.com>
To: "u.kleine-koenig@...gutronix.de" <u.kleine-koenig@...gutronix.de>,
	"sherry.sun@....com" <sherry.sun@....com>, "gregkh@...uxfoundation.org"
	<gregkh@...uxfoundation.org>, "ilpo.jarvinen@...ux.intel.com"
	<ilpo.jarvinen@...ux.intel.com>, "robh@...nel.org" <robh@...nel.org>,
	"shenwei.wang@....com" <shenwei.wang@....com>, "tglx@...utronix.de"
	<tglx@...utronix.de>, "jirislaby@...nel.org" <jirislaby@...nel.org>,
	"robert.hodaszi@...i.com" <robert.hodaszi@...i.com>
CC: "linux-serial@...r.kernel.org" <linux-serial@...r.kernel.org>,
	"imx@...ts.linux.dev" <imx@...ts.linux.dev>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>, "frank.li@....com" <frank.li@....com>
Subject: Re: [PATCH] tty: serial: fsl_lpuart: avoid idle preamble pending if
 CTS is enabled

Hi Sherry,

thank you for the fix!

On Mon, 2024-03-04 at 10:07 +0800, Sherry Sun wrote:
> If the remote uart device is not connected or not enabled after booting
> up, the CTS line is high by default. At this time, if we enable the flow
> control when opening the device(for example, using “stty -F /dev/ttyLP4
> crtscts” command), there will be a pending idle preamble(first writing 0
> and then writing 1 to UARTCTRL_TE will queue an idle preamble) that
> cannot be sent out, resulting in the uart port fail to close(waiting for
> TX empty), so the user space stty will have to wait for a long time or
> forever.
> 
> This is an LPUART IP bug(idle preamble has higher priority than CTS),
> here add a workaround patch to enable TX CTS after enabling UARTCTRL_TE,
> so that the idle preamble does not get stuck due to CTS is deasserted.
> 
> Signed-off-by: Sherry Sun <sherry.sun@....com>
> ---
>  drivers/tty/serial/fsl_lpuart.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/tty/serial/fsl_lpuart.c b/drivers/tty/serial/fsl_lpuart.c
> index 5ddf110aedbe..dce1b5851de0 100644
> --- a/drivers/tty/serial/fsl_lpuart.c
> +++ b/drivers/tty/serial/fsl_lpuart.c
> @@ -2345,8 +2345,12 @@ lpuart32_set_termios(struct uart_port *port, struct ktermios *termios,
>  
>         lpuart32_write(&sport->port, bd, UARTBAUD);
>         lpuart32_serial_setbrg(sport, baud);
> -       lpuart32_write(&sport->port, modem, UARTMODIR);
> +       /* disable CTS before enabling UARTCTRL_TE to avoid pending idle preamble */
> +       lpuart32_write(&sport->port, modem & ~UARTMODIR_TXCTSE, UARTMODIR);
>         lpuart32_write(&sport->port, ctrl, UARTCTRL);
> +       /* re-enable the CTS if needed */
> +       lpuart32_write(&sport->port, modem, UARTMODIR);
> +
>         /* restore control register */

The fix makes sense to me!
I see only one issue with above comment, which commit 380c966c093e7
already have put quite sub-optimal (after the actual write), but
your commit now shifts it even further making it completely dangling.
Should it be before last UARTCTRL write? 

>  
>         if ((ctrl & (UARTCTRL_PE | UARTCTRL_M)) == UARTCTRL_PE)

-- 
Alexander Sverdlin
Siemens AG
www.siemens.com