2023-07-31 03:02:39

by Sherry Sun

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

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 <[email protected]>
---
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



2023-07-31 05:56:55

by Jiri Slaby

[permalink] [raw]
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?

> 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 <[email protected]>
> ---
> 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


2023-07-31 06:38:31

by Sherry Sun

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



> -----Original Message-----
> From: Jiri Slaby <[email protected]>
> Sent: 2023年7月31日 13:24
> To: Sherry Sun <[email protected]>; [email protected];
> [email protected]; [email protected]
> Cc: [email protected]; [email protected]; dl-linux-imx
> <[email protected]>
> 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 <[email protected]>
> > ---
> > 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

2023-07-31 16:32:29

by Greg KH

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

On Mon, Jul 31, 2023 at 05:50:46AM +0000, Sherry Sun wrote:
>
>
> > -----Original Message-----
> > From: Jiri Slaby <[email protected]>
> > Sent: 2023年7月31日 13:24
> > To: Sherry Sun <[email protected]>; [email protected];
> > [email protected]; [email protected]
> > Cc: [email protected]; [email protected]; dl-linux-imx
> > <[email protected]>
> > 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