RE: [PATCH] tty: serial: fsl_lpuart: flush RX and TX FIFO when lpuart shutdown

[Sherry Sun]

> -----Original Message-----
> From: Frank Li <frank.li@xxxxxxx>
> Sent: Thursday, January 9, 2025 12:41 AM
> To: Sherry Sun <sherry.sun@xxxxxxx>
> Cc: gregkh@xxxxxxxxxxxxxxxxxxx; jirislaby@xxxxxxxxxx; linux-
> serial@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; imx@xxxxxxxxxxxxxxx
> Subject: Re: [PATCH] tty: serial: fsl_lpuart: flush RX and TX FIFO when lpuart
> shutdown
> 
> On Wed, Jan 08, 2025 at 03:03:05AM +0000, Sherry Sun wrote:
> >
> >
> > > -----Original Message-----
> > > From: Frank Li <frank.li@xxxxxxx>
> > > Sent: Tuesday, January 7, 2025 11:46 PM
> > > To: Sherry Sun <sherry.sun@xxxxxxx>
> > > Cc: gregkh@xxxxxxxxxxxxxxxxxxx; jirislaby@xxxxxxxxxx; linux-
> > > serial@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx;
> > > imx@xxxxxxxxxxxxxxx
> > > Subject: Re: [PATCH] tty: serial: fsl_lpuart: flush RX and TX FIFO
> > > when lpuart shutdown
> > >
> > > On Tue, Jan 07, 2025 at 03:48:34PM +0800, Sherry Sun wrote:
> > > > Need to flush UART RX and TX FIFO when lpuart is shutting down to
> > > > make sure restore a clean data transfer environment.
> > >
> > > why not flush it at open()?
> >
> > Hi Frank,
> >
> > Some background: We observed an issue during imx952 zebu simulation,
> imx952 edma IP has a bug that if an edma error occurs, it will directly return
> an error without marking the current request completed, so the current uart
> transfer will pending, the data will stuck in the FIFO even if we close the uart
> port and reopen it, which will impact the next data transfer.
> > Actually when we configure and enable the FIFO during uart startup, we
> > also flush the RX/TX FIFO, but it is done after the rx/tx dma are
> > started,
> 
> Please wrap at 75 char to read easily.

Hi Frank, sorry for that, will pay attention next time. :)

> 
> It looks like make sense to move flash before start dma.

Yes, as we discussed internally, this is an improvement needs to
be done. Thanks for the suggestions.

> 
> > so the dma request is still triggered by mistake.
> > And I think it is reasonable to flush the RX/TX FIFO when closing the uart
> port, so add this behavior in shutdown() to avoid changing the workflow of
> startup().
> 
> the target is make driver logic reasonable, not avoid changing ...
> if external devices continue send data, even you flash fifo in closing, it may
> still have data in FIFO if uart have not disabled yet.
> 

Since we flush the FIFO after disabling the receiver and transmitter, so this
won't happen.

Best Regards
Sherry


> > >
> > > >
> > > > Signed-off-by: Sherry Sun <sherry.sun@xxxxxxx>
> > > > ---
> > > >  drivers/tty/serial/fsl_lpuart.c | 5 +++++
> > > >  1 file changed, 5 insertions(+)
> > > >
> > > > diff --git a/drivers/tty/serial/fsl_lpuart.c
> > > > b/drivers/tty/serial/fsl_lpuart.c index 7cb1e36fdaab..c91b9d9818cd
> > > > 100644
> > > > --- a/drivers/tty/serial/fsl_lpuart.c
> > > > +++ b/drivers/tty/serial/fsl_lpuart.c
> > > > @@ -1965,6 +1965,11 @@ static void lpuart32_shutdown(struct
> > > > uart_port
> > > *port)
> > > >  			UARTCTRL_TIE | UARTCTRL_TCIE | UARTCTRL_RIE |
> > > UARTCTRL_SBK);
> > > >  	lpuart32_write(port, temp, UARTCTRL);
> > > >
> > > > +	/* flush Rx/Tx FIFO */
> > > > +	temp = lpuart32_read(port, UARTFIFO);
> > > > +	temp |= UARTFIFO_TXFLUSH | UARTFIFO_RXFLUSH;
> > > > +	lpuart32_write(port, temp, UARTFIFO);
> > > > +
> > > >  	uart_port_unlock_irqrestore(port, flags);
> > > >
> > > >  	lpuart_dma_shutdown(sport);
> > > > --
> > > > 2.34.1
> > > >