[PATCH] tty: serial: fsl_lpuart: Clear the error flags by writing 1 for lpuart32 platforms

[PATCH] tty: serial: fsl_lpuart: Clear the error flags by writing 1 for lpuart32 platforms

[Sherry Sun]

Do not read the data register to clear the error flags for lpuart32
platforms, the additional read may cause the receive FIFO underflow
since the DMA has already read the data register.
Now all lpuart32 platforms support write 1 to clear those error bits,
let's use this method to better clear the error flags.

Fixes: 42b68768e51b ("serial: fsl_lpuart: DMA support for 32-bit variant")
Signed-off-by: Sherry Sun <sherry.sun@nxp.com>
---
 drivers/tty/serial/fsl_lpuart.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/serial/fsl_lpuart.c b/drivers/tty/serial/fsl_lpuart.c
index f6644c5989d3..f72e1340b47d 100644
--- a/drivers/tty/serial/fsl_lpuart.c
+++ b/drivers/tty/serial/fsl_lpuart.c
@@ -1120,8 +1120,8 @@ static void lpuart_copy_rx_to_tty(struct lpuart_port *sport)
 		unsigned long sr = lpuart32_read(&sport->port, UARTSTAT);
 
 		if (sr & (UARTSTAT_PE | UARTSTAT_FE)) {
-			/* Read DR to clear the error flags */
-			lpuart32_read(&sport->port, UARTDATA);
+			/* Clear the error flags */
+			lpuart32_write(&sport->port, sr, UARTSTAT);
 
 			if (sr & UARTSTAT_PE)
 				sport->port.icount.parity++;
-- 
2.17.1

Re: [PATCH] tty: serial: fsl_lpuart: Clear the error flags by writing 1 for lpuart32 platforms

[Jiri Slaby]

On 31. 07. 23, 3:50, Sherry Sun wrote:
> Do not read the data register to clear the error flags for lpuart32
> platforms, the additional read may cause the receive FIFO underflow
> since the DMA has already read the data register.
> Now all lpuart32 platforms support write 1 to clear those error bits,

What does this "Now" mean? Will this change break some older platforms?

> let's use this method to better clear the error flags.
> 
> Fixes: 42b68768e51b ("serial: fsl_lpuart: DMA support for 32-bit variant")
> Signed-off-by: Sherry Sun <sherry.sun@nxp.com>
> ---
>   drivers/tty/serial/fsl_lpuart.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/tty/serial/fsl_lpuart.c b/drivers/tty/serial/fsl_lpuart.c
> index f6644c5989d3..f72e1340b47d 100644
> --- a/drivers/tty/serial/fsl_lpuart.c
> +++ b/drivers/tty/serial/fsl_lpuart.c
> @@ -1120,8 +1120,8 @@ static void lpuart_copy_rx_to_tty(struct lpuart_port *sport)
>   		unsigned long sr = lpuart32_read(&sport->port, UARTSTAT);
>   
>   		if (sr & (UARTSTAT_PE | UARTSTAT_FE)) {
> -			/* Read DR to clear the error flags */
> -			lpuart32_read(&sport->port, UARTDATA);
> +			/* Clear the error flags */
> +			lpuart32_write(&sport->port, sr, UARTSTAT);
>   
>   			if (sr & UARTSTAT_PE)
>   				sport->port.icount.parity++;

thanks,
-- 
js
suse labs

RE: [PATCH] tty: serial: fsl_lpuart: Clear the error flags by writing 1 for lpuart32 platforms

[Sherry Sun]

> -----Original Message-----
> From: Jiri Slaby <jirislaby@kernel.org>
> Sent: 2023年7月31日 13:24
> To: Sherry Sun <sherry.sun@nxp.com>; gregkh@linuxfoundation.org;
> tomonori.sakita@sord.co.jp; atsushi.nemoto@sord.co.jp
> Cc: linux-serial@vger.kernel.org; linux-kernel@vger.kernel.org; dl-linux-imx
> <linux-imx@nxp.com>
> Subject: Re: [PATCH] tty: serial: fsl_lpuart: Clear the error flags by writing 1
> for lpuart32 platforms
> 
> On 31. 07. 23, 3:50, Sherry Sun wrote:
> > Do not read the data register to clear the error flags for lpuart32
> > platforms, the additional read may cause the receive FIFO underflow
> > since the DMA has already read the data register.
> > Now all lpuart32 platforms support write 1 to clear those error bits,
> 
> What does this "Now" mean? Will this change break some older platforms?

Hi Jiri,

Sorry for the confusion, maybe the "Now" should be removed here. I can send a V2 to improve the commit message if needed.
I checked the Reference Manuals corresponding to the following compatible platforms, all can support W1C for those error bits. So I don't think it will break the supported platforms.

    { .compatible = "fsl,ls1021a-lpuart",   .data = &ls1021a_data, },
    { .compatible = "fsl,ls1028a-lpuart",   .data = &ls1028a_data, },
    { .compatible = "fsl,imx7ulp-lpuart",   .data = &imx7ulp_data, },
    { .compatible = "fsl,imx8ulp-lpuart",   .data = &imx8ulp_data, },
    { .compatible = "fsl,imx8qxp-lpuart",   .data = &imx8qxp_data, },
    { .compatible = "fsl,imxrt1050-lpuart", .data = &imxrt1050_data},

Best Regards
Sherry

> 
> > let's use this method to better clear the error flags.
> >
> > Fixes: 42b68768e51b ("serial: fsl_lpuart: DMA support for 32-bit
> > variant")
> > Signed-off-by: Sherry Sun <sherry.sun@nxp.com>
> > ---
> >   drivers/tty/serial/fsl_lpuart.c | 4 ++--
> >   1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/tty/serial/fsl_lpuart.c
> > b/drivers/tty/serial/fsl_lpuart.c index f6644c5989d3..f72e1340b47d
> > 100644
> > --- a/drivers/tty/serial/fsl_lpuart.c
> > +++ b/drivers/tty/serial/fsl_lpuart.c
> > @@ -1120,8 +1120,8 @@ static void lpuart_copy_rx_to_tty(struct
> lpuart_port *sport)
> >   		unsigned long sr = lpuart32_read(&sport->port, UARTSTAT);
> >
> >   		if (sr & (UARTSTAT_PE | UARTSTAT_FE)) {
> > -			/* Read DR to clear the error flags */
> > -			lpuart32_read(&sport->port, UARTDATA);
> > +			/* Clear the error flags */
> > +			lpuart32_write(&sport->port, sr, UARTSTAT);
> >
> >   			if (sr & UARTSTAT_PE)
> >   				sport->port.icount.parity++;
> 
> thanks,
> --
> js
> suse labs

Re: [PATCH] tty: serial: fsl_lpuart: Clear the error flags by writing 1 for lpuart32 platforms

[gregkh]

On Mon, Jul 31, 2023 at 05:50:46AM +0000, Sherry Sun wrote:
> 
> 
> > -----Original Message-----
> > From: Jiri Slaby <jirislaby@kernel.org>
> > Sent: 2023年7月31日 13:24
> > To: Sherry Sun <sherry.sun@nxp.com>; gregkh@linuxfoundation.org;
> > tomonori.sakita@sord.co.jp; atsushi.nemoto@sord.co.jp
> > Cc: linux-serial@vger.kernel.org; linux-kernel@vger.kernel.org; dl-linux-imx
> > <linux-imx@nxp.com>
> > Subject: Re: [PATCH] tty: serial: fsl_lpuart: Clear the error flags by writing 1
> > for lpuart32 platforms
> > 
> > On 31. 07. 23, 3:50, Sherry Sun wrote:
> > > Do not read the data register to clear the error flags for lpuart32
> > > platforms, the additional read may cause the receive FIFO underflow
> > > since the DMA has already read the data register.
> > > Now all lpuart32 platforms support write 1 to clear those error bits,
> > 
> > What does this "Now" mean? Will this change break some older platforms?
> 
> Hi Jiri,
> 
> Sorry for the confusion, maybe the "Now" should be removed here. I can
> send a V2 to improve the commit message if needed.

Please do, thanks!

greg k-h