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

From: "Sverdlin, Alexander" <alexander.sverdlin@siemens.com>
To: "u.kleine-koenig@pengutronix.de" <u.kleine-koenig@pengutronix.de>,
	"sherry.sun@nxp.com" <sherry.sun@nxp.com>,
	"gregkh@linuxfoundation.org" <gregkh@linuxfoundation.org>,
	"ilpo.jarvinen@linux.intel.com" <ilpo.jarvinen@linux.intel.com>,
	"robh@kernel.org" <robh@kernel.org>,
	"shenwei.wang@nxp.com" <shenwei.wang@nxp.com>,
	"tglx@linutronix.de" <tglx@linutronix.de>,
	"jirislaby@kernel.org" <jirislaby@kernel.org>,
	"robert.hodaszi@digi.com" <robert.hodaszi@digi.com>
Cc: "linux-serial@vger.kernel.org" <linux-serial@vger.kernel.org>,
	"imx@lists.linux.dev" <imx@lists.linux.dev>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"frank.li@nxp.com" <frank.li@nxp.com>
Subject: Re: [PATCH V2] tty: serial: fsl_lpuart: avoid idle preamble pending if CTS is enabled
Date: Tue, 5 Mar 2024 07:26:47 +0000	[thread overview]
Message-ID: <317ddcba7d49ab22c3a138c34235a1118f9c9297.camel@siemens.com> (raw)
In-Reply-To: <20240305015706.1050769-1-sherry.sun@nxp.com>

Hi Sherry,

thanks for re-spin!

On Tue, 2024-03-05 at 09:57 +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.
> 
> Fixes: 380c966c093e ("tty: serial: fsl_lpuart: add 32-bit register interface support")
> Signed-off-by: Sherry Sun <sherry.sun@nxp.com>

Reviewed-by: Alexander Sverdlin <alexander.sverdlin@siemens.com>

> ---
> Changes in V2:
> 1. Move the "restore control register" comment message to the appropriate place.
> 2. Add Fixes tag.
> ---
>  drivers/tty/serial/fsl_lpuart.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/tty/serial/fsl_lpuart.c b/drivers/tty/serial/fsl_lpuart.c
> index 5ddf110aedbe..bbcbc91482af 100644
> --- a/drivers/tty/serial/fsl_lpuart.c
> +++ b/drivers/tty/serial/fsl_lpuart.c
> @@ -2345,9 +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);
> -       lpuart32_write(&sport->port, ctrl, UARTCTRL);
> +       /* disable CTS before enabling UARTCTRL_TE to avoid pending idle preamble */
> +       lpuart32_write(&sport->port, modem & ~UARTMODIR_TXCTSE, UARTMODIR);
>         /* restore control register */
> +       lpuart32_write(&sport->port, ctrl, UARTCTRL);
> +       /* re-enable the CTS if needed */
> +       lpuart32_write(&sport->port, modem, UARTMODIR);
>  
>         if ((ctrl & (UARTCTRL_PE | UARTCTRL_M)) == UARTCTRL_PE)
>                 sport->is_cs7 = true;

-- 
Alexander Sverdlin
Siemens AG
www.siemens.com