[PATCH 7/9] serial: imx: ensure that RX irqs are off if RX is off

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Make sure that UCR1.RXDMAEN and UCR1.ATDMAEN (for the DMA case) and
UCR1.RRDYEN and UCR2.ATEN (for the PIO case) are off when UCR1.RXEN is
disabled. This ensures that the fifo isn't read with RX disabled which
results in an exception.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@xxxxxxxxxxxxxx>
---
 drivers/tty/serial/imx.c | 156 +++++++++++++++++++++++++++++------------------
 1 file changed, 97 insertions(+), 59 deletions(-)

diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
index 1dc79091200c..328534f0a950 100644
--- a/drivers/tty/serial/imx.c
+++ b/drivers/tty/serial/imx.c
@@ -403,11 +403,34 @@ static void imx_port_rts_auto(struct imx_port *sport, unsigned long *ucr2)
 	*ucr2 |= UCR2_CTSC;
 }
 
+/* called with port.lock taken and irqs off */
+static void imx_start_rx(struct uart_port *port)
+{
+	struct imx_port *sport = (struct imx_port *)port;
+	unsigned int ucr1, ucr2;
+
+	ucr1 = imx_uart_readl(sport, UCR1);
+	ucr2 = imx_uart_readl(sport, UCR2);
+
+	ucr2 |= UCR2_RXEN;
+
+	if (sport->dma_is_enabled) {
+		ucr1 |= UCR1_RXDMAEN | UCR1_ATDMAEN;
+	} else {
+		ucr1 |= UCR1_RRDYEN;
+		ucr2 |= UCR2_ATEN;
+	}
+
+	/* Write UCR2 first as it includes RXEN */
+	imx_uart_writel(sport, ucr2, UCR2);
+	imx_uart_writel(sport, ucr1, UCR1);
+}
+
 /* called with port.lock taken and irqs off */
 static void imx_stop_tx(struct uart_port *port)
 {
 	struct imx_port *sport = (struct imx_port *)port;
-	unsigned long temp;
+	unsigned int ucr1;
 
 	/*
 	 * We are maybe in the SMP context, so if the DMA TX thread is running
@@ -416,23 +439,25 @@ static void imx_stop_tx(struct uart_port *port)
 	if (sport->dma_is_txing)
 		return;
 
-	temp = imx_uart_readl(sport, UCR1);
-	imx_uart_writel(sport, temp & ~UCR1_TXMPTYEN, UCR1);
+	ucr1 = imx_uart_readl(sport, UCR1);
+	ucr1 &= ~UCR1_TXMPTYEN;
+	imx_uart_writel(sport, ucr1, UCR1);
 
 	/* in rs485 mode disable transmitter if shifter is empty */
 	if (port->rs485.flags & SER_RS485_ENABLED &&
 	    imx_uart_readl(sport, USR2) & USR2_TXDC) {
-		temp = imx_uart_readl(sport, UCR2);
+		unsigned long ucr2 = imx_uart_readl(sport, UCR2), ucr4;
 		if (port->rs485.flags & SER_RS485_RTS_AFTER_SEND)
-			imx_port_rts_active(sport, &temp);
+			imx_port_rts_active(sport, &ucr2);
 		else
-			imx_port_rts_inactive(sport, &temp);
-		temp |= UCR2_RXEN;
-		imx_uart_writel(sport, temp, UCR2);
+			imx_port_rts_inactive(sport, &ucr2);
+		imx_uart_writel(sport, ucr2, UCR2);
 
-		temp = imx_uart_readl(sport, UCR4);
-		temp &= ~UCR4_TCEN;
-		imx_uart_writel(sport, temp, UCR4);
+		imx_start_rx(port);
+
+		ucr4 = imx_uart_readl(sport, UCR4);
+		ucr4 &= ~UCR4_TCEN;
+		imx_uart_writel(sport, ucr4, UCR4);
 	}
 }
 
@@ -440,23 +465,21 @@ static void imx_stop_tx(struct uart_port *port)
 static void imx_stop_rx(struct uart_port *port)
 {
 	struct imx_port *sport = (struct imx_port *)port;
-	unsigned long temp;
+	unsigned int ucr1, ucr2;
 
-	if (sport->dma_is_rxing) {
-		if (sport->port.suspended) {
-			dmaengine_terminate_all(sport->dma_chan_rx);
-			sport->dma_is_rxing = 0;
-		} else {
-			return;
-		}
-	}
+	ucr1 = imx_uart_readl(sport, UCR1);
+	ucr2 = imx_uart_readl(sport, UCR2);
 
-	temp = imx_uart_readl(sport, UCR2);
-	imx_uart_writel(sport, temp & ~UCR2_RXEN, UCR2);
+	if (sport->dma_is_enabled) {
+		ucr1 &= ~(UCR1_RXDMAEN | UCR1_ATDMAEN);
+	} else {
+		ucr1 &= ~UCR1_RRDYEN;
+		ucr2 &= ~UCR2_ATEN;
+	}
+	imx_uart_writel(sport, ucr1, UCR1);
 
-	/* disable the `Receiver Ready Interrrupt` */
-	temp = imx_uart_readl(sport, UCR1);
-	imx_uart_writel(sport, temp & ~UCR1_RRDYEN, UCR1);
+	ucr2 &= ~UCR2_RXEN;
+	imx_uart_writel(sport, ucr2, UCR2);
 }
 
 /* called with port.lock taken and irqs off */
@@ -626,10 +649,11 @@ static void imx_start_tx(struct uart_port *port)
 			imx_port_rts_active(sport, &temp);
 		else
 			imx_port_rts_inactive(sport, &temp);
-		if (!(port->rs485.flags & SER_RS485_RX_DURING_TX))
-			temp &= ~UCR2_RXEN;
 		imx_uart_writel(sport, temp, UCR2);
 
+		if (!(port->rs485.flags & SER_RS485_RX_DURING_TX))
+			imx_stop_rx(port);
+
 		/* enable transmitter and shifter empty irq */
 		temp = imx_uart_readl(sport, UCR4);
 		temp |= UCR4_TCEN;
@@ -1225,13 +1249,13 @@ static void imx_enable_dma(struct imx_port *sport)
 {
 	unsigned long temp;
 
+	imx_setup_ufcr(sport, TXTL_DMA, RXTL_DMA);
+
 	/* set UCR1 */
 	temp = imx_uart_readl(sport, UCR1);
 	temp |= UCR1_RXDMAEN | UCR1_TXDMAEN | UCR1_ATDMAEN;
 	imx_uart_writel(sport, temp, UCR1);
 
-	imx_setup_ufcr(sport, TXTL_DMA, RXTL_DMA);
-
 	sport->dma_is_enabled = 1;
 }
 
@@ -1307,15 +1331,10 @@ static int imx_startup(struct uart_port *port)
 	imx_uart_writel(sport, USR1_RTSD | USR1_DTRD, USR1);
 	imx_uart_writel(sport, USR2_ORE, USR2);
 
-	if (dma_is_inited)
-		imx_enable_dma(sport);
-
 	temp = imx_uart_readl(sport, UCR1) & ~UCR1_RRDYEN;
-	if (!sport->dma_is_enabled)
-		temp |= UCR1_RRDYEN;
 	temp |= UCR1_UARTEN;
 	if (sport->have_rtscts)
-			temp |= UCR1_RTSDEN;
+		temp |= UCR1_RTSDEN;
 
 	imx_uart_writel(sport, temp, UCR1);
 
@@ -1328,6 +1347,7 @@ static int imx_startup(struct uart_port *port)
 	temp |= (UCR2_RXEN | UCR2_TXEN);
 	if (!sport->have_rtscts)
 		temp |= UCR2_IRTS;
+
 	/*
 	 * make sure the edge sensitive RTS-irq is disabled,
 	 * we're using RTSD instead.
@@ -1353,13 +1373,18 @@ static int imx_startup(struct uart_port *port)
 	 */
 	imx_enable_ms(&sport->port);
 
-	/*
-	 * Start RX DMA immediately instead of waiting for RX FIFO interrupts.
-	 * In our iMX53 the average delay for the first reception dropped from
-	 * approximately 35000 microseconds to 1000 microseconds.
-	 */
-	if (sport->dma_is_enabled)
+	if (dma_is_inited) {
+		imx_enable_dma(sport);
 		start_rx_dma(sport);
+	} else {
+		temp = imx_uart_readl(sport, UCR1);
+		temp |= UCR1_RRDYEN;
+		imx_uart_writel(sport, temp, UCR1);
+
+		temp = imx_uart_readl(sport, UCR2);
+		temp |= UCR2_ATEN;
+		imx_uart_writel(sport, temp, UCR2);
+	}
 
 	spin_unlock_irqrestore(&sport->port.lock, flags);
 
@@ -1390,7 +1415,7 @@ static void imx_shutdown(struct uart_port *port)
 
 	spin_lock_irqsave(&sport->port.lock, flags);
 	temp = imx_uart_readl(sport, UCR2);
-	temp &= ~(UCR2_TXEN);
+	temp &= ~(UCR2_TXEN | UCR2_ATEN);
 	imx_uart_writel(sport, temp, UCR2);
 	spin_unlock_irqrestore(&sport->port.lock, flags);
 
@@ -1405,7 +1430,7 @@ static void imx_shutdown(struct uart_port *port)
 
 	spin_lock_irqsave(&sport->port.lock, flags);
 	temp = imx_uart_readl(sport, UCR1);
-	temp &= ~(UCR1_TXMPTYEN | UCR1_RRDYEN | UCR1_RTSDEN | UCR1_UARTEN);
+	temp &= ~(UCR1_TXMPTYEN | UCR1_RRDYEN | UCR1_RTSDEN | UCR1_UARTEN | UCR1_RXDMAEN | UCR1_ATDMAEN);
 
 	imx_uart_writel(sport, temp, UCR1);
 	spin_unlock_irqrestore(&sport->port.lock, flags);
@@ -1577,13 +1602,14 @@ imx_set_termios(struct uart_port *port, struct ktermios *termios,
 	imx_uart_writel(sport,
 			old_ucr1 & ~(UCR1_TXMPTYEN | UCR1_RRDYEN | UCR1_RTSDEN),
 			UCR1);
+	old_ucr2 = imx_uart_readl(sport, UCR2);
+	imx_uart_writel(sport, old_ucr2 & ~UCR2_ATEN, UCR2);
 
 	while (!(imx_uart_readl(sport, USR2) & USR2_TXDC))
 		barrier();
 
 	/* then, disable everything */
-	old_ucr2 = imx_uart_readl(sport, UCR2);
-	imx_uart_writel(sport, old_ucr2 & ~(UCR2_TXEN | UCR2_RXEN), UCR2);
+	imx_uart_writel(sport, old_ucr2 & ~(UCR2_TXEN | UCR2_RXEN | UCR2_ATEN), UCR2);
 	old_ucr2 &= (UCR2_TXEN | UCR2_RXEN | UCR2_ATEN);
 
 	/* custom-baudrate handling */
@@ -1683,7 +1709,7 @@ static int imx_poll_init(struct uart_port *port)
 {
 	struct imx_port *sport = (struct imx_port *)port;
 	unsigned long flags;
-	unsigned long temp;
+	unsigned int ucr1, ucr2;
 	int retval;
 
 	retval = clk_prepare_enable(sport->clk_ipg);
@@ -1697,16 +1723,31 @@ static int imx_poll_init(struct uart_port *port)
 
 	spin_lock_irqsave(&sport->port.lock, flags);
 
-	temp = imx_uart_readl(sport, UCR1);
+	/*
+	 * Be careful about the order of enabling bits here. First enable the
+	 * receiver (UARTEN + RXEN) and only then the corresponding irqs.
+	 * This prevents that a character that already sits in the RX fifo is
+	 * triggering an irq but the try to fetch it from there results in an
+	 * exception because UARTEN or RXEN is still off.
+	 */
+	ucr1 = imx_uart_readl(sport, UCR1);
+	ucr2 = imx_uart_readl(sport, UCR2);
+
 	if (is_imx1_uart(sport))
-		temp |= IMX1_UCR1_UARTCLKEN;
-	temp |= UCR1_UARTEN | UCR1_RRDYEN;
-	temp &= ~(UCR1_TXMPTYEN | UCR1_RTSDEN);
-	imx_uart_writel(sport, temp, UCR1);
+		ucr1 |= IMX1_UCR1_UARTCLKEN;
 
-	temp = imx_uart_readl(sport, UCR2);
-	temp |= UCR2_RXEN;
-	imx_uart_writel(sport, temp, UCR2);
+	ucr1 |= UCR1_UARTEN;
+	ucr1 &= ~(UCR1_TXMPTYEN | UCR1_RTSDEN | UCR1_RRDYEN);
+
+	ucr2 |= UCR2_RXEN;
+	ucr2 &= ~UCR2_ATEN;
+
+	imx_uart_writel(sport, ucr1, UCR1);
+	imx_uart_writel(sport, ucr2, UCR2);
+
+	/* now enable irqs */
+	imx_uart_writel(sport, ucr1 | UCR1_RRDYEN, UCR1);
+	imx_uart_writel(sport, ucr2 | UCR2_ATEN, UCR2);
 
 	spin_unlock_irqrestore(&sport->port.lock, flags);
 
@@ -1767,11 +1808,8 @@ static int imx_rs485_config(struct uart_port *port,
 
 	/* Make sure Rx is enabled in case Tx is active with Rx disabled */
 	if (!(rs485conf->flags & SER_RS485_ENABLED) ||
-	    rs485conf->flags & SER_RS485_RX_DURING_TX) {
-		temp = imx_uart_readl(sport, UCR2);
-		temp |= UCR2_RXEN;
-		imx_uart_writel(sport, temp, UCR2);
-	}
+	    rs485conf->flags & SER_RS485_RX_DURING_TX)
+		imx_start_rx(port);
 
 	port->rs485 = *rs485conf;
 
-- 
2.16.1

--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Kernel Newbies]     [Security]     [Netfilter]     [Bugtraq]     [Linux PPP]     [Linux FS]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Linmodem]     [Device Mapper]     [Linux Kernel for ARM]

  Powered by Linux