Hi Anssi, Thanks for the patch. Minor nit below. The call to the cdns_uart_handle_rx could be prevented in cdns_uart_isr > -----Original Message----- > From: Michal Simek [mailto:michal.simek@xxxxxxxxxx] > Sent: Thursday, February 14, 2019 12:06 PM > To: Anssi Hannula <anssi.hannula@xxxxxxxxxx>; Michal Simek > <michals@xxxxxxxxxx>; Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx> > Cc: Jiri Slaby <jslaby@xxxxxxxx>; linux-serial@xxxxxxxxxxxxxxx; linux-arm- > kernel@xxxxxxxxxxxxxxxxxxx; stable@xxxxxxxxxxxxxxx; Shubhrajyoti Datta > <shubhraj@xxxxxxxxxx> > Subject: Re: [PATCH] serial: uartps: Fix stuck ISR if RX disabled with non- > empty FIFO > > + Shubhrajyoti > > On 06. 02. 19 15:08, Anssi Hannula wrote: > > If RX is disabled while there are still unprocessed bytes in RX FIFO, > > cdns_uart_handle_rx() called from interrupt handler will get stuck in > > the receive loop as read bytes will not get removed from the RX FIFO > > and CDNS_UART_SR_RXEMPTY bit will never get set. > > > > Avoid the stuck handler by checking first if RX is disabled. > > port->lock protects against race with RX-disabling functions. > > > > This HW behavior was mentioned by Nathan Rossi in 43e98facc4a3 ("tty: > > xuartps: Fix RX hang, and TX corruption in termios call") which fixed > > a similar issue in cdns_uart_set_termios(). > > The behavior can also be easily verified by e.g. setting > > CDNS_UART_CR_RX_DIS at the beginning of cdns_uart_handle_rx() - the > > following loop will then get stuck. > > > > Resetting the FIFO using RXRST would not set RXEMPTY either so simply > > issuing a reset after RX-disable would not work. > > > > I observe this frequently on a ZynqMP board during heavy RX load at 1M > > baudrate when the reader process exits and thus RX gets disabled. > > Reviewed-by: Shubhrajyoti Datta <shubhrajyori.datta@xxxxxxxxxx> > > Fixes: 61ec9016988f ("tty/serial: add support for Xilinx PS UART") > > Signed-off-by: Anssi Hannula <anssi.hannula@xxxxxxxxxx> > > Cc: stable@xxxxxxxxxxxxxxx > > --- > > drivers/tty/serial/xilinx_uartps.c | 7 +++++++ > > 1 file changed, 7 insertions(+) > > > > diff --git a/drivers/tty/serial/xilinx_uartps.c > > b/drivers/tty/serial/xilinx_uartps.c > > index 094f2958cb2b..f0c4f59d9314 100644 > > --- a/drivers/tty/serial/xilinx_uartps.c > > +++ b/drivers/tty/serial/xilinx_uartps.c > > @@ -219,6 +219,13 @@ static void cdns_uart_handle_rx(void *dev_id, > > unsigned int isrstatus) > > > > is_rxbs_support = cdns_uart->quirks & CDNS_UART_RXBS_SUPPORT; > > > > + /* > > + * RXEMPTY will never be set if RX is disabled as read bytes > > + * will not be removed from the FIFO > > + */ > > + if (readl(port->membase + CDNS_UART_CR) & > CDNS_UART_CR_RX_DIS) > > + return; > > + > > while ((readl(port->membase + CDNS_UART_SR) & > > CDNS_UART_SR_RXEMPTY) != CDNS_UART_SR_RXEMPTY) { > > if (is_rxbs_support) > > > > > Please review. > M
