Linux-Serial Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH] tty: serial: msm_serial: RX SW/FIFO mode fallback
@ 2020-01-10  8:58 Loic Poulain
  2020-01-13 19:53 ` Jeffrey Hugo
  0 siblings, 1 reply; 4+ messages in thread
From: Loic Poulain @ 2020-01-10  8:58 UTC (permalink / raw)
  To: agross, bjorn.andersson; +Cc: linux-arm-msm, linux-serial, Loic Poulain

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(-)

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)
-- 
2.7.4


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] tty: serial: msm_serial: RX SW/FIFO mode fallback
  2020-01-10  8:58 [PATCH] tty: serial: msm_serial: RX SW/FIFO mode fallback Loic Poulain
@ 2020-01-13 19:53 ` Jeffrey Hugo
  2020-01-14  7:55   ` Loic Poulain
  0 siblings, 1 reply; 4+ messages in thread
From: Jeffrey Hugo @ 2020-01-13 19:53 UTC (permalink / raw)
  To: Loic Poulain; +Cc: Andy Gross, Bjorn Andersson, MSM, linux-serial

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
>

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] tty: serial: msm_serial: RX SW/FIFO mode fallback
  2020-01-13 19:53 ` Jeffrey Hugo
@ 2020-01-14  7:55   ` Loic Poulain
  2020-01-14 15:50     ` Jeffrey Hugo
  0 siblings, 1 reply; 4+ messages in thread
From: Loic Poulain @ 2020-01-14  7:55 UTC (permalink / raw)
  To: Jeffrey Hugo; +Cc: Andy Gross, Bjorn Andersson, MSM, linux-serial

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] tty: serial: msm_serial: RX SW/FIFO mode fallback
  2020-01-14  7:55   ` Loic Poulain
@ 2020-01-14 15:50     ` Jeffrey Hugo
  0 siblings, 0 replies; 4+ messages in thread
From: Jeffrey Hugo @ 2020-01-14 15:50 UTC (permalink / raw)
  To: Loic Poulain; +Cc: Andy Gross, Bjorn Andersson, MSM, linux-serial

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, back to index

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-10  8:58 [PATCH] tty: serial: msm_serial: RX SW/FIFO mode fallback Loic Poulain
2020-01-13 19:53 ` Jeffrey Hugo
2020-01-14  7:55   ` Loic Poulain
2020-01-14 15:50     ` Jeffrey Hugo

Linux-Serial Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-serial/0 linux-serial/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-serial linux-serial/ https://lore.kernel.org/linux-serial \
		linux-serial@vger.kernel.org
	public-inbox-index linux-serial

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-serial


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git