| Message ID | 1578646684-17379-1-git-send-email-loic.poulain@linaro.org (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 76460fbd845b2b55d6fdb651b9dac4ef8122038b |
| Headers | show |
| Series | tty: serial: msm_serial: RX SW/FIFO mode fallback | expand |
On Fri, Jan 10, 2020 at 1:55 AM Loic Poulain <loic.poulain@linaro.org> wrote: > > During db410c stress test and when the system is low on memory, > the UART/console becomes unresponsive and never recover back. > This has been narrowed down to the msm_start_rx_dma which does > not manage error cases correctly (e.g. dma mapping failure), > indeed, when an error happens, dma transfer is simply discarded > and so never completed, leading to unconfigured RX path. > > This patch fixes this issue by switching to SW/FIFO mode in case > of DMA issue. This mainly consists in resetting the receiver to > apply RX BAM/DMA disabling change and re-enabling the RX level > and stale interrupts (previously disabled for DMA transfers). > > The DMA will be re-enabled once memory is available since the > SW/FIFO read function (msm_handle_rx_dm) retries to start dma > on completion. Is there a risk of the same thing occurring in the TX path? > > Signed-off-by: Loic Poulain <loic.poulain@linaro.org> > --- > drivers/tty/serial/msm_serial.c | 18 +++++++++++++++++- > 1 file changed, 17 insertions(+), 1 deletion(-) > > diff --git a/drivers/tty/serial/msm_serial.c b/drivers/tty/serial/msm_serial.c > index 1cbae07..a63b703 100644 > --- a/drivers/tty/serial/msm_serial.c > +++ b/drivers/tty/serial/msm_serial.c > @@ -610,7 +610,7 @@ static void msm_start_rx_dma(struct msm_port *msm_port) > UARTDM_RX_SIZE, dma->dir); > ret = dma_mapping_error(uart->dev, dma->phys); > if (ret) > - return; > + goto sw_mode; > > dma->desc = dmaengine_prep_slave_single(dma->chan, dma->phys, > UARTDM_RX_SIZE, DMA_DEV_TO_MEM, > @@ -661,6 +661,22 @@ static void msm_start_rx_dma(struct msm_port *msm_port) > return; > unmap: > dma_unmap_single(uart->dev, dma->phys, UARTDM_RX_SIZE, dma->dir); > + > +sw_mode: > + /* > + * Switch from DMA to SW/FIFO mode. After clearing Rx BAM (UARTDM_DMEN), Where does this clear of UARTDM_DMEN occur? > + * receiver must be reset. > + */ > + msm_write(uart, UART_CR_CMD_RESET_RX, UART_CR); > + msm_write(uart, UART_CR_RX_ENABLE, UART_CR); > + > + msm_write(uart, UART_CR_CMD_RESET_STALE_INT, UART_CR); > + msm_write(uart, 0xFFFFFF, UARTDM_DMRX); > + msm_write(uart, UART_CR_CMD_STALE_EVENT_ENABLE, UART_CR); > + > + /* Re-enable RX interrupts */ > + msm_port->imr |= (UART_IMR_RXLEV | UART_IMR_RXSTALE); > + msm_write(uart, msm_port->imr, UART_IMR); > } > > static void msm_stop_rx(struct uart_port *port) > -- > 2.7.4 >
On Mon, 13 Jan 2020 at 20:53, Jeffrey Hugo <jeffrey.l.hugo@gmail.com> wrote: > > On Fri, Jan 10, 2020 at 1:55 AM Loic Poulain <loic.poulain@linaro.org> wrote: > > > > During db410c stress test and when the system is low on memory, > > the UART/console becomes unresponsive and never recover back. > > This has been narrowed down to the msm_start_rx_dma which does > > not manage error cases correctly (e.g. dma mapping failure), > > indeed, when an error happens, dma transfer is simply discarded > > and so never completed, leading to unconfigured RX path. > > > > This patch fixes this issue by switching to SW/FIFO mode in case > > of DMA issue. This mainly consists in resetting the receiver to > > apply RX BAM/DMA disabling change and re-enabling the RX level > > and stale interrupts (previously disabled for DMA transfers). > > > > The DMA will be re-enabled once memory is available since the > > SW/FIFO read function (msm_handle_rx_dm) retries to start dma > > on completion. > > Is there a risk of the same thing occurring in the TX path? Yes, but the TX case is already handled. If msm_handle_tx_dma fails, msm_handle_tx_pio is used to directly write the FIFO. > > > > > Signed-off-by: Loic Poulain <loic.poulain@linaro.org> > > --- > > drivers/tty/serial/msm_serial.c | 18 +++++++++++++++++- > > 1 file changed, 17 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/tty/serial/msm_serial.c b/drivers/tty/serial/msm_serial.c > > index 1cbae07..a63b703 100644 > > --- a/drivers/tty/serial/msm_serial.c > > +++ b/drivers/tty/serial/msm_serial.c > > @@ -610,7 +610,7 @@ static void msm_start_rx_dma(struct msm_port *msm_port) > > UARTDM_RX_SIZE, dma->dir); > > ret = dma_mapping_error(uart->dev, dma->phys); > > if (ret) > > - return; > > + goto sw_mode; > > > > dma->desc = dmaengine_prep_slave_single(dma->chan, dma->phys, > > UARTDM_RX_SIZE, DMA_DEV_TO_MEM, > > @@ -661,6 +661,22 @@ static void msm_start_rx_dma(struct msm_port *msm_port) > > return; > > unmap: > > dma_unmap_single(uart->dev, dma->phys, UARTDM_RX_SIZE, dma->dir); > > + > > +sw_mode: > > + /* > > + * Switch from DMA to SW/FIFO mode. After clearing Rx BAM (UARTDM_DMEN), > > Where does this clear of UARTDM_DMEN occur? The DMEN clear occurs on DMA RX completion (msm_complete_tx_dma()) and set on DMA restart (msm_start_rx_dma). In our case, since DMA start fails, we just reset the RX path to effectively apply the SW/FIFO mode. > > > + * receiver must be reset. > > + */ > > + msm_write(uart, UART_CR_CMD_RESET_RX, UART_CR); > > + msm_write(uart, UART_CR_RX_ENABLE, UART_CR); > > + > > + msm_write(uart, UART_CR_CMD_RESET_STALE_INT, UART_CR); > > + msm_write(uart, 0xFFFFFF, UARTDM_DMRX); > > + msm_write(uart, UART_CR_CMD_STALE_EVENT_ENABLE, UART_CR); > > + > > + /* Re-enable RX interrupts */ > > + msm_port->imr |= (UART_IMR_RXLEV | UART_IMR_RXSTALE); > > + msm_write(uart, msm_port->imr, UART_IMR); > > } > > > > static void msm_stop_rx(struct uart_port *port) > > -- > > 2.7.4 > > Regards, Loic
On Tue, Jan 14, 2020 at 12:52 AM Loic Poulain <loic.poulain@linaro.org> wrote: > > On Mon, 13 Jan 2020 at 20:53, Jeffrey Hugo <jeffrey.l.hugo@gmail.com> wrote: > > > > On Fri, Jan 10, 2020 at 1:55 AM Loic Poulain <loic.poulain@linaro.org> wrote: > > > > > > During db410c stress test and when the system is low on memory, > > > the UART/console becomes unresponsive and never recover back. > > > This has been narrowed down to the msm_start_rx_dma which does > > > not manage error cases correctly (e.g. dma mapping failure), > > > indeed, when an error happens, dma transfer is simply discarded > > > and so never completed, leading to unconfigured RX path. > > > > > > This patch fixes this issue by switching to SW/FIFO mode in case > > > of DMA issue. This mainly consists in resetting the receiver to > > > apply RX BAM/DMA disabling change and re-enabling the RX level > > > and stale interrupts (previously disabled for DMA transfers). > > > > > > The DMA will be re-enabled once memory is available since the > > > SW/FIFO read function (msm_handle_rx_dm) retries to start dma > > > on completion. > > > > Is there a risk of the same thing occurring in the TX path? > > Yes, but the TX case is already handled. If msm_handle_tx_dma fails, > msm_handle_tx_pio is used to directly write the FIFO. > > > > > > > > > Signed-off-by: Loic Poulain <loic.poulain@linaro.org> > > > --- > > > drivers/tty/serial/msm_serial.c | 18 +++++++++++++++++- > > > 1 file changed, 17 insertions(+), 1 deletion(-) > > > > > > diff --git a/drivers/tty/serial/msm_serial.c b/drivers/tty/serial/msm_serial.c > > > index 1cbae07..a63b703 100644 > > > --- a/drivers/tty/serial/msm_serial.c > > > +++ b/drivers/tty/serial/msm_serial.c > > > @@ -610,7 +610,7 @@ static void msm_start_rx_dma(struct msm_port *msm_port) > > > UARTDM_RX_SIZE, dma->dir); > > > ret = dma_mapping_error(uart->dev, dma->phys); > > > if (ret) > > > - return; > > > + goto sw_mode; > > > > > > dma->desc = dmaengine_prep_slave_single(dma->chan, dma->phys, > > > UARTDM_RX_SIZE, DMA_DEV_TO_MEM, > > > @@ -661,6 +661,22 @@ static void msm_start_rx_dma(struct msm_port *msm_port) > > > return; > > > unmap: > > > dma_unmap_single(uart->dev, dma->phys, UARTDM_RX_SIZE, dma->dir); > > > + > > > +sw_mode: > > > + /* > > > + * Switch from DMA to SW/FIFO mode. After clearing Rx BAM (UARTDM_DMEN), > > > > Where does this clear of UARTDM_DMEN occur? > > The DMEN clear occurs on DMA RX completion (msm_complete_tx_dma()) and > set on DMA > restart (msm_start_rx_dma). In our case, since DMA start fails, we > just reset the RX path to > effectively apply the SW/FIFO mode. Oh, now I see the entire flow. Not bad. Seems sane to me from reviewing the code. > > > > > > + * receiver must be reset. > > > + */ > > > + msm_write(uart, UART_CR_CMD_RESET_RX, UART_CR); > > > + msm_write(uart, UART_CR_RX_ENABLE, UART_CR); > > > + > > > + msm_write(uart, UART_CR_CMD_RESET_STALE_INT, UART_CR); > > > + msm_write(uart, 0xFFFFFF, UARTDM_DMRX); > > > + msm_write(uart, UART_CR_CMD_STALE_EVENT_ENABLE, UART_CR); > > > + > > > + /* Re-enable RX interrupts */ > > > + msm_port->imr |= (UART_IMR_RXLEV | UART_IMR_RXSTALE); > > > + msm_write(uart, msm_port->imr, UART_IMR); > > > } > > > > > > static void msm_stop_rx(struct uart_port *port) > > > -- > > > 2.7.4 > > > > > Regards, > Loic
diff --git a/drivers/tty/serial/msm_serial.c b/drivers/tty/serial/msm_serial.c index 1cbae07..a63b703 100644 --- a/drivers/tty/serial/msm_serial.c +++ b/drivers/tty/serial/msm_serial.c @@ -610,7 +610,7 @@ static void msm_start_rx_dma(struct msm_port *msm_port) UARTDM_RX_SIZE, dma->dir); ret = dma_mapping_error(uart->dev, dma->phys); if (ret) - return; + goto sw_mode; dma->desc = dmaengine_prep_slave_single(dma->chan, dma->phys, UARTDM_RX_SIZE, DMA_DEV_TO_MEM, @@ -661,6 +661,22 @@ static void msm_start_rx_dma(struct msm_port *msm_port) return; unmap: dma_unmap_single(uart->dev, dma->phys, UARTDM_RX_SIZE, dma->dir); + +sw_mode: + /* + * Switch from DMA to SW/FIFO mode. After clearing Rx BAM (UARTDM_DMEN), + * receiver must be reset. + */ + msm_write(uart, UART_CR_CMD_RESET_RX, UART_CR); + msm_write(uart, UART_CR_RX_ENABLE, UART_CR); + + msm_write(uart, UART_CR_CMD_RESET_STALE_INT, UART_CR); + msm_write(uart, 0xFFFFFF, UARTDM_DMRX); + msm_write(uart, UART_CR_CMD_STALE_EVENT_ENABLE, UART_CR); + + /* Re-enable RX interrupts */ + msm_port->imr |= (UART_IMR_RXLEV | UART_IMR_RXSTALE); + msm_write(uart, msm_port->imr, UART_IMR); } static void msm_stop_rx(struct uart_port *port)
During db410c stress test and when the system is low on memory, the UART/console becomes unresponsive and never recover back. This has been narrowed down to the msm_start_rx_dma which does not manage error cases correctly (e.g. dma mapping failure), indeed, when an error happens, dma transfer is simply discarded and so never completed, leading to unconfigured RX path. This patch fixes this issue by switching to SW/FIFO mode in case of DMA issue. This mainly consists in resetting the receiver to apply RX BAM/DMA disabling change and re-enabling the RX level and stale interrupts (previously disabled for DMA transfers). The DMA will be re-enabled once memory is available since the SW/FIFO read function (msm_handle_rx_dm) retries to start dma on completion. Signed-off-by: Loic Poulain <loic.poulain@linaro.org> --- drivers/tty/serial/msm_serial.c | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-)