> -----Original Message----- > From: Sverdlin, Alexander <alexander.sverdlin@xxxxxxxxxxx> > Sent: Monday, March 4, 2024 3:14 PM > To: u.kleine-koenig@xxxxxxxxxxxxxx; Sherry Sun <sherry.sun@xxxxxxx>; > gregkh@xxxxxxxxxxxxxxxxxxx; ilpo.jarvinen@xxxxxxxxxxxxxxx; > robh@xxxxxxxxxx; Shenwei Wang <shenwei.wang@xxxxxxx>; > tglx@xxxxxxxxxxxxx; jirislaby@xxxxxxxxxx; robert.hodaszi@xxxxxxxx > Cc: linux-serial@xxxxxxxxxxxxxxx; imx@xxxxxxxxxxxxxxx; linux- > kernel@xxxxxxxxxxxxxxx; Frank Li <frank.li@xxxxxxx> > 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@xxxxxxx> > > --- > > 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
