2020-06-09 07:25:56

by Frieder Schrempf

[permalink] [raw]
Subject: [PATCH v2] serial: imx: Fix handling of TC irq in combination with DMA

From: Uwe Kleine-König <[email protected]>

commit 1866541492641c02874bf51f9d8712b5510f2c64 upstream

When using RS485 half duplex the Transmitter Complete irq is needed to
determine the moment when the transmitter can be disabled. When using
DMA this irq must only be enabled when DMA has completed to transfer all
data. Otherwise the CPU might busily trigger this irq which is not
properly handled and so the also pending irq for the DMA transfer cannot
trigger.

Cc: <[email protected]> # v4.14.x
Signed-off-by: Uwe Kleine-König <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>
[Backport to v4.14]
Signed-off-by: Frieder Schrempf <[email protected]>
---
When using RS485 with DMA enabled simply transmitting some data on our
i.MX6ULL based boards often freezes the system completely. The higher
the baudrate, the easier it is to reproduce the issue. To test this I
simply used:

stty -F /dev/ttymxc1 speed 115200
while true; do echo TEST > /dev/ttymxc1; done

Without the patch this leads to an almost immediate system freeze,
with the patch applied, everything keeps working as expected.
---
drivers/tty/serial/imx.c | 22 ++++++++++++++++++----
1 file changed, 18 insertions(+), 4 deletions(-)

diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
index 3f2605edd855..70c737236870 100644
--- a/drivers/tty/serial/imx.c
+++ b/drivers/tty/serial/imx.c
@@ -538,6 +538,11 @@ static void dma_tx_callback(void *data)

if (!uart_circ_empty(xmit) && !uart_tx_stopped(&sport->port))
imx_dma_tx(sport);
+ else if (sport->port.rs485.flags & SER_RS485_ENABLED) {
+ temp = readl(sport->port.membase + UCR4);
+ temp |= UCR4_TCEN;
+ writel(temp, sport->port.membase + UCR4);
+ }

spin_unlock_irqrestore(&sport->port.lock, flags);
}
@@ -555,6 +560,10 @@ static void imx_dma_tx(struct imx_port *sport)
if (sport->dma_is_txing)
return;

+ temp = readl(sport->port.membase + UCR4);
+ temp &= ~UCR4_TCEN;
+ writel(temp, sport->port.membase + UCR4);
+
sport->tx_bytes = uart_circ_chars_pending(xmit);

if (xmit->tail < xmit->head || xmit->head == 0) {
@@ -617,10 +626,15 @@ static void imx_start_tx(struct uart_port *port)
if (!(port->rs485.flags & SER_RS485_RX_DURING_TX))
imx_stop_rx(port);

- /* enable transmitter and shifter empty irq */
- temp = readl(port->membase + UCR4);
- temp |= UCR4_TCEN;
- writel(temp, port->membase + UCR4);
+ /*
+ * Enable transmitter and shifter empty irq only if DMA is off.
+ * In the DMA case this is done in the tx-callback.
+ */
+ if (!sport->dma_is_enabled) {
+ temp = readl(port->membase + UCR4);
+ temp |= UCR4_TCEN;
+ writel(temp, port->membase + UCR4);
+ }
}

if (!sport->dma_is_enabled) {
--
2.17.1


2020-06-09 08:25:46

by Frieder Schrempf

[permalink] [raw]
Subject: Re: [PATCH v2] serial: imx: Fix handling of TC irq in combination with DMA

On 09.06.20 09:23, Schrempf Frieder wrote:
> From: Uwe Kleine-König <[email protected]>
>
> commit 1866541492641c02874bf51f9d8712b5510f2c64 upstream
>
> When using RS485 half duplex the Transmitter Complete irq is needed to
> determine the moment when the transmitter can be disabled. When using
> DMA this irq must only be enabled when DMA has completed to transfer all
> data. Otherwise the CPU might busily trigger this irq which is not
> properly handled and so the also pending irq for the DMA transfer cannot
> trigger.
>
> Cc: <[email protected]> # v4.14.x
> Signed-off-by: Uwe Kleine-König <[email protected]>
> Signed-off-by: Greg Kroah-Hartman <[email protected]>
> [Backport to v4.14]
> Signed-off-by: Frieder Schrempf <[email protected]>
> ---

Sorry, forgot the changelog:

Changes for backport v2:
* Remove an unrelated additional blank line.

> When using RS485 with DMA enabled simply transmitting some data on our
> i.MX6ULL based boards often freezes the system completely. The higher
> the baudrate, the easier it is to reproduce the issue. To test this I
> simply used:
>
> stty -F /dev/ttymxc1 speed 115200
> while true; do echo TEST > /dev/ttymxc1; done
>
> Without the patch this leads to an almost immediate system freeze,
> with the patch applied, everything keeps working as expected.
> ---
> drivers/tty/serial/imx.c | 22 ++++++++++++++++++----
> 1 file changed, 18 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
> index 3f2605edd855..70c737236870 100644
> --- a/drivers/tty/serial/imx.c
> +++ b/drivers/tty/serial/imx.c
> @@ -538,6 +538,11 @@ static void dma_tx_callback(void *data)
>
> if (!uart_circ_empty(xmit) && !uart_tx_stopped(&sport->port))
> imx_dma_tx(sport);
> + else if (sport->port.rs485.flags & SER_RS485_ENABLED) {
> + temp = readl(sport->port.membase + UCR4);
> + temp |= UCR4_TCEN;
> + writel(temp, sport->port.membase + UCR4);
> + }
>
> spin_unlock_irqrestore(&sport->port.lock, flags);
> }
> @@ -555,6 +560,10 @@ static void imx_dma_tx(struct imx_port *sport)
> if (sport->dma_is_txing)
> return;
>
> + temp = readl(sport->port.membase + UCR4);
> + temp &= ~UCR4_TCEN;
> + writel(temp, sport->port.membase + UCR4);
> +
> sport->tx_bytes = uart_circ_chars_pending(xmit);
>
> if (xmit->tail < xmit->head || xmit->head == 0) {
> @@ -617,10 +626,15 @@ static void imx_start_tx(struct uart_port *port)
> if (!(port->rs485.flags & SER_RS485_RX_DURING_TX))
> imx_stop_rx(port);
>
> - /* enable transmitter and shifter empty irq */
> - temp = readl(port->membase + UCR4);
> - temp |= UCR4_TCEN;
> - writel(temp, port->membase + UCR4);
> + /*
> + * Enable transmitter and shifter empty irq only if DMA is off.
> + * In the DMA case this is done in the tx-callback.
> + */
> + if (!sport->dma_is_enabled) {
> + temp = readl(port->membase + UCR4);
> + temp |= UCR4_TCEN;
> + writel(temp, port->membase + UCR4);
> + }
> }
>
> if (!sport->dma_is_enabled) {
>

2020-06-11 11:20:59

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH v2] serial: imx: Fix handling of TC irq in combination with DMA

On Tue, Jun 09, 2020 at 07:23:40AM +0000, Schrempf Frieder wrote:
> From: Uwe Kleine-K?nig <[email protected]>
>
> commit 1866541492641c02874bf51f9d8712b5510f2c64 upstream
>
> When using RS485 half duplex the Transmitter Complete irq is needed to
> determine the moment when the transmitter can be disabled. When using
> DMA this irq must only be enabled when DMA has completed to transfer all
> data. Otherwise the CPU might busily trigger this irq which is not
> properly handled and so the also pending irq for the DMA transfer cannot
> trigger.
>
> Cc: <[email protected]> # v4.14.x
> Signed-off-by: Uwe Kleine-K?nig <[email protected]>
> Signed-off-by: Greg Kroah-Hartman <[email protected]>
> [Backport to v4.14]
> Signed-off-by: Frieder Schrempf <[email protected]>
> ---
> When using RS485 with DMA enabled simply transmitting some data on our
> i.MX6ULL based boards often freezes the system completely. The higher
> the baudrate, the easier it is to reproduce the issue. To test this I
> simply used:
>
> stty -F /dev/ttymxc1 speed 115200
> while true; do echo TEST > /dev/ttymxc1; done
>
> Without the patch this leads to an almost immediate system freeze,
> with the patch applied, everything keeps working as expected.
> ---
> drivers/tty/serial/imx.c | 22 ++++++++++++++++++----
> 1 file changed, 18 insertions(+), 4 deletions(-)

Now queued up, thanks.

greg k-h