Skip to contentSkip to conversation

tty: serial: imx: disable ageing timer interrupt if dma in use

PATCH New Troy Kisky 4 participants 7 messages Oct 20, 2017 – Nov 4, 2017

Subsystems (2)
lore
Share & tools
lore

Troy Kisky

author
#0 lore raw

PATCH v2 1/3 tty: serial: imx: disable ageing timer interrupt if dma in use

Since commit 4dec2f119e86 ("imx-serial: RX DMA startup latency") the interrupt routine no longer will start rx dma.
So, we no longer need to enable this interrupt to start dma.

---
v2: split into a series of 2 patches

---
 drivers/tty/serial/imx.c | 4 ----
 1 file changed, 4 deletions(-)
drivers/tty/serial/imx.c +0 -4
index dfeff3951f93..bf47b15df2e8 100644
@@ -1207,10 +1207,6 @@ static void imx_enable_dma(struct imx_port *sport)
 	temp |= UCR1_RDMAEN | UCR1_TDMAEN | UCR1_ATDMAEN;
 	writel(temp, sport->port.membase + UCR1);
 
-	temp = readl(sport->port.membase + UCR2);
-	temp |= UCR2_ATEN;
-	writel(temp, sport->port.membase + UCR2);
-
 	imx_setup_ufcr(sport, TXTL_DMA, RXTL_DMA);
 
 	sport->dma_is_enabled = 1;
Signature · 1 line
2.11.0
author
#1 lore raw

PATCH v2 2/3 tty: serial: imx: remove dead code imx_dma_rxint

Since commit 4dec2f119e86 ("imx-serial: RX DMA startup latency") the interrupt routine no longer will start rx dma.

imx_dma_rxint no longer needs to be called to try and start dma. It won't start dma because dma_is_rxing is
already true meaning dma is already started.

---
v2: split into 2 patches

---
 drivers/tty/serial/imx.c | 30 ++----------------------------
 1 file changed, 2 insertions(+), 28 deletions(-)
drivers/tty/serial/imx.c +2 -28
index bf47b15df2e8..15b0ecb4cf60 100644
@@ -732,29 +732,6 @@ static void imx_disable_rx_int(struct imx_port *sport)
 }
 
 static void clear_rx_errors(struct imx_port *sport);
-static int start_rx_dma(struct imx_port *sport);
-/*
- * If the RXFIFO is filled with some data, and then we
- * arise a DMA operation to receive them.
- */
-static void imx_dma_rxint(struct imx_port *sport)
-{
-	unsigned long temp;
-	unsigned long flags;
-
-	spin_lock_irqsave(&sport->port.lock, flags);
-
-	temp = readl(sport->port.membase + USR2);
-	if ((temp & USR2_RDR) && !sport->dma_is_rxing) {
-
-		imx_disable_rx_int(sport);
-
-		/* tell the DMA to receive the data. */
-		start_rx_dma(sport);
-	}
-
-	spin_unlock_irqrestore(&sport->port.lock, flags);
-}
 
 /*
  * We have a modem side uart, so the meanings of RTS and CTS are inverted.
@@ -816,11 +793,8 @@ static irqreturn_t imx_int(int irq, void *dev_id)
 	sts = readl(sport->port.membase + USR1);
 	sts2 = readl(sport->port.membase + USR2);
 
-	if (sts & (USR1_RRDY | USR1_AGTIM)) {
-		if (sport->dma_is_enabled)
-			imx_dma_rxint(sport);
-		else
-			imx_rxint(irq, dev_id);
+	if (!sport->dma_is_enabled && (sts & (USR1_RRDY | USR1_AGTIM))) {
+		imx_rxint(irq, dev_id);
 		ret = IRQ_HANDLED;
 	}
 
Signature · 1 line
2.11.0
author
#2 lore raw

PATCH v2 3/3 tty: serial: imx: remove imx_disable_rx_int

In reply to Troy Kisky

Since imx_disable_rx_int is only called by imx_startup, let's integrate it into that function.

---
v2: new patch

---
 drivers/tty/serial/imx.c | 39 ++++++++++-----------------------------
 1 file changed, 10 insertions(+), 29 deletions(-)
drivers/tty/serial/imx.c +10 -29
index 15b0ecb4cf60..506fcd742b47 100644
@@ -710,27 +710,6 @@ static irqreturn_t imx_rxint(int irq, void *dev_id)
 	return IRQ_HANDLED;
 }
 
-static void imx_disable_rx_int(struct imx_port *sport)
-{
-	unsigned long temp;
-
-	sport->dma_is_rxing = 1;
-
-	/* disable the receiver ready and aging timer interrupts */
-	temp = readl(sport->port.membase + UCR1);
-	temp &= ~(UCR1_RRDYEN);
-	writel(temp, sport->port.membase + UCR1);
-
-	temp = readl(sport->port.membase + UCR2);
-	temp &= ~(UCR2_ATEN);
-	writel(temp, sport->port.membase + UCR2);
-
-	/* disable the rx errors interrupts */
-	temp = readl(sport->port.membase + UCR4);
-	temp &= ~UCR4_OREN;
-	writel(temp, sport->port.membase + UCR4);
-}
-
 static void clear_rx_errors(struct imx_port *sport);
 
 /*
@@ -1024,6 +1003,7 @@ static int start_rx_dma(struct imx_port *sport)
 	struct dma_async_tx_descriptor *desc;
 	int ret;
 
+	sport->dma_is_rxing = 1;
 	sport->rx_ring.head = 0;
 	sport->rx_ring.tail = 0;
 	sport->rx_periods = RX_DMA_PERIODS;
@@ -1260,18 +1240,21 @@ static int imx_startup(struct uart_port *port)
 	if (sport->dma_is_inited && !sport->dma_is_enabled)
 		imx_enable_dma(sport);
 
-	temp = readl(sport->port.membase + UCR1);
-	temp |= UCR1_RRDYEN | UCR1_UARTEN;
+	temp = readl(sport->port.membase + UCR1) & ~UCR1_RRDYEN;
+	if (!sport->dma_is_enabled)
+		temp |= UCR1_RRDYEN;
+	temp |= UCR1_UARTEN;
 	if (sport->have_rtscts)
 			temp |= UCR1_RTSDEN;
 
 	writel(temp, sport->port.membase + UCR1);
 
-	temp = readl(sport->port.membase + UCR4);
-	temp |= UCR4_OREN;
+	temp = readl(sport->port.membase + UCR4) & ~UCR4_OREN;
+	if (!sport->dma_is_enabled)
+		temp |= UCR4_OREN;
 	writel(temp, sport->port.membase + UCR4);
 
-	temp = readl(sport->port.membase + UCR2);
+	temp = readl(sport->port.membase + UCR2) & ~UCR2_ATEN;
 	temp |= (UCR2_RXEN | UCR2_TXEN);
 	if (!sport->have_rtscts)
 		temp |= UCR2_IRTS;
@@ -1305,10 +1288,8 @@ static int imx_startup(struct uart_port *port)
 	 * In our iMX53 the average delay for the first reception dropped from
 	 * approximately 35000 microseconds to 1000 microseconds.
 	 */
-	if (sport->dma_is_enabled) {
-		imx_disable_rx_int(sport);
+	if (sport->dma_is_enabled)
 		start_rx_dma(sport);
-	}
 
 	spin_unlock_irqrestore(&sport->port.lock, flags);
 
Signature · 1 line
2.11.0
author
#3 lore raw

PATCH v2 3/3 tty: serial: imx: remove imx_disable_rx_int

Quoted 9 lines · Troy Kisky

Since imx_disable_rx_int is only called by imx_startup, let's integrate it into that function.

---
v2: new patch

---
 drivers/tty/serial/imx.c | 39 ++++++++++-----------------------------
 1 file changed, 10 insertions(+), 29 deletions(-)

While testing this series, I noticed that sending a break on the serial port caused

imx-uart 2020000.serial: DMA transaction error.

Is that normal ?

#4 lore raw

PATCH v2 3/3 tty: serial: imx: remove imx_disable_rx_int

Hello,

Quoted 11 lines · Troy Kisky
Quoted 9 lines · Troy Kisky

Since imx_disable_rx_int is only called by imx_startup, let's integrate it into that function.

---
v2: new patch

---
 drivers/tty/serial/imx.c | 39 ++++++++++-----------------------------
 1 file changed, 10 insertions(+), 29 deletions(-)
Quoted 5 lines

While testing this series, I noticed that sending a break on the serial port caused

imx-uart 2020000.serial: DMA transaction error.

Is that normal ?

I remember that J?rgen (added to To explictly) hit this or a similar problem before, maybe he can commen?

Best regards
Uwe

Signature · 2 lines
Pengutronix e.K.                           | Uwe Kleine-K?nig            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
#5 lore raw

PATCH v2 3/3 tty: serial: imx: remove imx_disable_rx_int

Hi,

Quoted 16 lines · On Sunday 22 October 2017 20:49:56 Uwe Kleine-K?nig wrote:
Quoted 15 lines · Troy Kisky
Quoted 10 lines · Troy Kisky

Since imx_disable_rx_int is only called by imx_startup, let's integrate it into that function.

---
v2: new patch

---
 drivers/tty/serial/imx.c | 39

++++++++++----------------------------- 1 file changed, 10 insertions(+), 29 deletions(-)

While testing this series, I noticed that sending a break on the serial port caused

Quoted 6 lines
Quoted 3 lines

imx-uart 2020000.serial: DMA transaction error.

Is that normal ?

I remember that J?rgen (added to To explictly) hit this or a similar problem before, maybe he can commen?

Yes, it hit me on i.MX53 after DMA was enabled by default. The driver currently doesn't handle the "break" event when DMA is active. In this case the DMA callback returns with an error (that's the message you see, but it's not an error in this case...), the driver clears the flags and drops the (break) event itself.

jb

Signature · 2 lines
Pengutronix e.K. ? ? ? ? ? ? ? ? ? ? ? ? ?? ?| Juergen Borleis ? ? ? ? ? ? |
Industrial Linux Solutions ? ? ? ? ? ? ? ?? ?| http://www.pengutronix.de/ ?|
maintainer
#6 lore raw

PATCH v2 3/3 tty: serial: imx: remove imx_disable_rx_int

In reply to Troy Kisky
Quoted 9 lines · Troy Kisky

Since imx_disable_rx_int is only called by imx_startup, let's integrate it into that function.

---
v2: new patch

---
 drivers/tty/serial/imx.c | 39 ++++++++++-----------------------------
 1 file changed, 10 insertions(+), 29 deletions(-)

Does not apply :(