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:
 <AS8PR04MB8404FEA637E51E3B258BC28C92232@AS8PR04MB8404.eurprd04.prod.outlook.com>
Date: Mon, 4 Mar 2024 07:31:49 +0000
From: Sherry Sun <sherry.sun@....com>
To: "Sverdlin, Alexander" <alexander.sverdlin@...mens.com>,
	"u.kleine-koenig@...gutronix.de" <u.kleine-koenig@...gutronix.de>,
	"gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>,
	"ilpo.jarvinen@...ux.intel.com" <ilpo.jarvinen@...ux.intel.com>,
	"robh@...nel.org" <robh@...nel.org>, Shenwei Wang <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 <frank.li@....com>
Subject: RE: [PATCH] tty: serial: fsl_lpuart: avoid idle preamble pending if
 CTS is enabled



> -----Original Message-----
> From: Sverdlin, Alexander <alexander.sverdlin@...mens.com>
> Sent: Monday, March 4, 2024 3:14 PM
> To: u.kleine-koenig@...gutronix.de; Sherry Sun <sherry.sun@....com>;
> gregkh@...uxfoundation.org; ilpo.jarvinen@...ux.intel.com;
> robh@...nel.org; Shenwei Wang <shenwei.wang@....com>;
> tglx@...utronix.de; jirislaby@...nel.org; robert.hodaszi@...i.com
> Cc: linux-serial@...r.kernel.org; imx@...ts.linux.dev; linux-
> kernel@...r.kernel.org; Frank Li <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?

Hi Alexander, good catch, I will move the "/* restore control register */" message to the appropriate place in V2. Thanks!

Best Regards
Sherry