diff mbox series

[V2] tty: serial: lpuart: disable flow control while waiting for the transmit engine to complete

Message ID 20220804070420.32186-1-sherry.sun@nxp.com
State Superseded
Headers show
Series [V2] tty: serial: lpuart: disable flow control while waiting for the transmit engine to complete | expand

Commit Message

Sherry Sun Aug. 4, 2022, 7:04 a.m. UTC
When the user initializes the uart port, and waits for the transmit
engine to complete in lpuart32_set_termios(), if the UART TX fifo has
dirty data and the UARTMODIR enable the flow control, the TX fifo may
never be empty. So here we should disable the flow control first to make
sure the transmit engin can complete.

Signed-off-by: Sherry Sun <sherry.sun@nxp.com>
---
Changes in V2:
1. Rephrase the commit log as suggested by Jiri.
---
 drivers/tty/serial/fsl_lpuart.c | 1 +
 1 file changed, 1 insertion(+)

Comments

Sherry Sun Aug. 15, 2022, 8:04 a.m. UTC | #1
Gentle ping...

Best regards
Sherry

> -----Original Message-----
> From: Sherry Sun
> Sent: 2022年8月4日 15:06
> To: gregkh@linuxfoundation.org; jirislaby@kernel.org
> Cc: linux-serial@vger.kernel.org; linux-kernel@vger.kernel.org; dl-linux-imx
> <linux-imx@nxp.com>
> Subject: [PATCH V2] tty: serial: lpuart: disable flow control while waiting for
> the transmit engine to complete
> 
> When the user initializes the uart port, and waits for the transmit engine to
> complete in lpuart32_set_termios(), if the UART TX fifo has dirty data and the
> UARTMODIR enable the flow control, the TX fifo may never be empty. So
> here we should disable the flow control first to make sure the transmit engin
> can complete.
> 
> Signed-off-by: Sherry Sun <sherry.sun@nxp.com>
> ---
> Changes in V2:
> 1. Rephrase the commit log as suggested by Jiri.
> ---
>  drivers/tty/serial/fsl_lpuart.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/tty/serial/fsl_lpuart.c b/drivers/tty/serial/fsl_lpuart.c
> index fc7d235a1e27..f0fccd2ff7ac 100644
> --- a/drivers/tty/serial/fsl_lpuart.c
> +++ b/drivers/tty/serial/fsl_lpuart.c
> @@ -2172,6 +2172,7 @@ lpuart32_set_termios(struct uart_port *port,
> struct ktermios *termios,
>  	uart_update_timeout(port, termios->c_cflag, baud);
> 
>  	/* wait transmit engin complete */
> +	lpuart32_write(&sport->port, 0, UARTMODIR);
>  	lpuart32_wait_bit_set(&sport->port, UARTSTAT, UARTSTAT_TC);
> 
>  	/* disable transmit and receive */
> --
> 2.17.1
Greg Kroah-Hartman Aug. 21, 2022, 8:38 a.m. UTC | #2
On Thu, Aug 04, 2022 at 03:04:20PM +0800, Sherry Sun wrote:
> When the user initializes the uart port, and waits for the transmit
> engine to complete in lpuart32_set_termios(), if the UART TX fifo has
> dirty data and the UARTMODIR enable the flow control, the TX fifo may
> never be empty. So here we should disable the flow control first to make
> sure the transmit engin can complete.
> 
> Signed-off-by: Sherry Sun <sherry.sun@nxp.com>
> ---
> Changes in V2:
> 1. Rephrase the commit log as suggested by Jiri.
> ---
>  drivers/tty/serial/fsl_lpuart.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/tty/serial/fsl_lpuart.c b/drivers/tty/serial/fsl_lpuart.c
> index fc7d235a1e27..f0fccd2ff7ac 100644
> --- a/drivers/tty/serial/fsl_lpuart.c
> +++ b/drivers/tty/serial/fsl_lpuart.c
> @@ -2172,6 +2172,7 @@ lpuart32_set_termios(struct uart_port *port, struct ktermios *termios,
>  	uart_update_timeout(port, termios->c_cflag, baud);
>  
>  	/* wait transmit engin complete */
> +	lpuart32_write(&sport->port, 0, UARTMODIR);
>  	lpuart32_wait_bit_set(&sport->port, UARTSTAT, UARTSTAT_TC);
>  
>  	/* disable transmit and receive */
> -- 
> 2.17.1

What commit id does this fix?  Should it be backported to older stable
kernels?

thanks,

greg k-h
Sherry Sun Aug. 21, 2022, 10:01 a.m. UTC | #3
> On Thu, Aug 04, 2022 at 03:04:20PM +0800, Sherry Sun wrote:
> > When the user initializes the uart port, and waits for the transmit
> > engine to complete in lpuart32_set_termios(), if the UART TX fifo has
> > dirty data and the UARTMODIR enable the flow control, the TX fifo may
> > never be empty. So here we should disable the flow control first to
> > make sure the transmit engin can complete.
> >
> > Signed-off-by: Sherry Sun <sherry.sun@nxp.com>
> > ---
> > Changes in V2:
> > 1. Rephrase the commit log as suggested by Jiri.
> > ---
> >  drivers/tty/serial/fsl_lpuart.c | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/drivers/tty/serial/fsl_lpuart.c
> > b/drivers/tty/serial/fsl_lpuart.c index fc7d235a1e27..f0fccd2ff7ac
> > 100644
> > --- a/drivers/tty/serial/fsl_lpuart.c
> > +++ b/drivers/tty/serial/fsl_lpuart.c
> > @@ -2172,6 +2172,7 @@ lpuart32_set_termios(struct uart_port *port,
> struct ktermios *termios,
> >  	uart_update_timeout(port, termios->c_cflag, baud);
> >
> >  	/* wait transmit engin complete */
> > +	lpuart32_write(&sport->port, 0, UARTMODIR);
> >  	lpuart32_wait_bit_set(&sport->port, UARTSTAT, UARTSTAT_TC);
> >
> >  	/* disable transmit and receive */
> > --
> > 2.17.1
> 
> What commit id does this fix?  Should it be backported to older stable kernels?

This issue existed when the lpuart32_set_termios() was introduced. So the Fixes tag should be:
Fixes: 380c966c093e ("tty: serial: fsl_lpuart: add 32-bit register interface support"), and I believe it can be backported to the older stable kernels.

Should I send a V2 patch to add the Fixes tag?

Best regards
Sherry

> 
> thanks,
> 
> greg k-h
Greg Kroah-Hartman Aug. 21, 2022, 10:07 a.m. UTC | #4
On Sun, Aug 21, 2022 at 10:01:45AM +0000, Sherry Sun wrote:
> > On Thu, Aug 04, 2022 at 03:04:20PM +0800, Sherry Sun wrote:
> > > When the user initializes the uart port, and waits for the transmit
> > > engine to complete in lpuart32_set_termios(), if the UART TX fifo has
> > > dirty data and the UARTMODIR enable the flow control, the TX fifo may
> > > never be empty. So here we should disable the flow control first to
> > > make sure the transmit engin can complete.
> > >
> > > Signed-off-by: Sherry Sun <sherry.sun@nxp.com>
> > > ---
> > > Changes in V2:
> > > 1. Rephrase the commit log as suggested by Jiri.
> > > ---
> > >  drivers/tty/serial/fsl_lpuart.c | 1 +
> > >  1 file changed, 1 insertion(+)
> > >
> > > diff --git a/drivers/tty/serial/fsl_lpuart.c
> > > b/drivers/tty/serial/fsl_lpuart.c index fc7d235a1e27..f0fccd2ff7ac
> > > 100644
> > > --- a/drivers/tty/serial/fsl_lpuart.c
> > > +++ b/drivers/tty/serial/fsl_lpuart.c
> > > @@ -2172,6 +2172,7 @@ lpuart32_set_termios(struct uart_port *port,
> > struct ktermios *termios,
> > >  	uart_update_timeout(port, termios->c_cflag, baud);
> > >
> > >  	/* wait transmit engin complete */
> > > +	lpuart32_write(&sport->port, 0, UARTMODIR);
> > >  	lpuart32_wait_bit_set(&sport->port, UARTSTAT, UARTSTAT_TC);
> > >
> > >  	/* disable transmit and receive */
> > > --
> > > 2.17.1
> > 
> > What commit id does this fix?  Should it be backported to older stable kernels?
> 
> This issue existed when the lpuart32_set_termios() was introduced. So the Fixes tag should be:
> Fixes: 380c966c093e ("tty: serial: fsl_lpuart: add 32-bit register interface support"), and I believe it can be backported to the older stable kernels.
> 
> Should I send a V2 patch to add the Fixes tag?

Yes please.
diff mbox series

Patch

diff --git a/drivers/tty/serial/fsl_lpuart.c b/drivers/tty/serial/fsl_lpuart.c
index fc7d235a1e27..f0fccd2ff7ac 100644
--- a/drivers/tty/serial/fsl_lpuart.c
+++ b/drivers/tty/serial/fsl_lpuart.c
@@ -2172,6 +2172,7 @@  lpuart32_set_termios(struct uart_port *port, struct ktermios *termios,
 	uart_update_timeout(port, termios->c_cflag, baud);
 
 	/* wait transmit engin complete */
+	lpuart32_write(&sport->port, 0, UARTMODIR);
 	lpuart32_wait_bit_set(&sport->port, UARTSTAT, UARTSTAT_TC);
 
 	/* disable transmit and receive */