| Message ID | 20250107074834.3115230-1-sherry.sun@nxp.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | tty: serial: fsl_lpuart: flush RX and TX FIFO when lpuart shutdown | expand |
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()? Frank > > Signed-off-by: Sherry Sun <sherry.sun@nxp.com> > --- > 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 >
> -----Original Message----- > From: Frank Li <frank.li@nxp.com> > Sent: Tuesday, January 7, 2025 11:46 PM > To: Sherry Sun <sherry.sun@nxp.com> > Cc: gregkh@linuxfoundation.org; jirislaby@kernel.org; linux- > serial@vger.kernel.org; linux-kernel@vger.kernel.org; imx@lists.linux.dev > 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, 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(). Best Regards Sherry > > > > > Signed-off-by: Sherry Sun <sherry.sun@nxp.com> > > --- > > 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 > >
On Wed, Jan 08, 2025 at 03:03:05AM +0000, Sherry Sun wrote: > > > > -----Original Message----- > > From: Frank Li <frank.li@nxp.com> > > Sent: Tuesday, January 7, 2025 11:46 PM > > To: Sherry Sun <sherry.sun@nxp.com> > > Cc: gregkh@linuxfoundation.org; jirislaby@kernel.org; linux- > > serial@vger.kernel.org; linux-kernel@vger.kernel.org; imx@lists.linux.dev > > 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. It looks like make sense to move flash before start dma. > 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. Frank > > Best Regards > Sherry > > > > > > > > > Signed-off-by: Sherry Sun <sherry.sun@nxp.com> > > > --- > > > 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 > > >
> -----Original Message----- > From: Frank Li <frank.li@nxp.com> > Sent: Thursday, January 9, 2025 12:41 AM > To: Sherry Sun <sherry.sun@nxp.com> > Cc: gregkh@linuxfoundation.org; jirislaby@kernel.org; linux- > serial@vger.kernel.org; linux-kernel@vger.kernel.org; imx@lists.linux.dev > 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@nxp.com> > > > Sent: Tuesday, January 7, 2025 11:46 PM > > > To: Sherry Sun <sherry.sun@nxp.com> > > > Cc: gregkh@linuxfoundation.org; jirislaby@kernel.org; linux- > > > serial@vger.kernel.org; linux-kernel@vger.kernel.org; > > > imx@lists.linux.dev > > > 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@nxp.com> > > > > --- > > > > 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 > > > >
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);
Need to flush UART RX and TX FIFO when lpuart is shutting down to make sure restore a clean data transfer environment. Signed-off-by: Sherry Sun <sherry.sun@nxp.com> --- drivers/tty/serial/fsl_lpuart.c | 5 +++++ 1 file changed, 5 insertions(+)