[PATCH v1 00/15] serial: imx: misc fix and features

[PATCH v1 00/15] serial: imx: misc fix and features

[Jiada Wang]

Hi Guys,

This patch attempts to add several features and fix issues existing
in imx UART driver.

Tested-ON: i.MX6Q SabreAuto board.

Anton Bondarenko (1):
  serial: imx: initialized DMA w/o HW flow enabled

Dirk Behme (6):
  serial: imx: remove unneeded imx_transmit_buffer() from imx_start_tx()
  serial: imx: TX DMA: clean up sg initialization
  serial: imx: unmap sg in case of dmaengine_prep_slave_sg() failure
  serial: imx: unmap scatter gather list in imx_flush_buffer
  serial: imx: use dma_is_txing to synchronize dma_tx_callback and
    imx_dma_tx
  serial: imx: disable TDMAEN in imx_flush_buffer()

Jiada Wang (7):
  serial: imx: add CREAD flag support
  serial: imx: use locking to stop concurrent access of UCR1
  Revert "serial: imx: always wake up the processes in the TX callback"
  serial: imx: call imx_dma_tx() again in dma_tx_callback
  serial: imx: Enable UCR4_OREN in startup interface
  serial: imx: Fix issue in software flow control
  serial: imx: Support sw flow control in DMA mode

Robin Gong (1):
  serial: imx: start rx_dma once RXFIFO is not empty

 drivers/tty/serial/imx.c | 160 +++++++++++++++++++++++++++++++++++++----------
 1 file changed, 126 insertions(+), 34 deletions(-)

-- 
1.9.3

[PATCH v1 01/15] serial: imx: add CREAD flag support

[Jiada Wang]

Add CREAD flag hanlding in set_termios and UART DMA mode
which ignores all received chars when CREAD flag cleared.

Signed-off-by: Jiada Wang <jiada_wang@mentor.com>
Signed-off-by: Anton Bondarenko <anton_bondarenko@mentor.com>
---
 drivers/tty/serial/imx.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
index 8f62a3c..4d3fa53 100644
--- a/drivers/tty/serial/imx.c
+++ b/drivers/tty/serial/imx.c
@@ -74,6 +74,7 @@
 #define IMX21_UTS 0xb4 /* UART Test Register on all other i.mx*/
 
 /* UART Control Register Bit Fields.*/
+#define URXD_DUMMY_READ (1<<16)
 #define URXD_CHARRDY	(1<<15)
 #define URXD_ERR	(1<<14)
 #define URXD_OVRRUN	(1<<13)
@@ -710,6 +711,9 @@ static irqreturn_t imx_rxint(int irq, void *dev_id)
 #endif
 		}
 
+		if (sport->port.ignore_status_mask & URXD_DUMMY_READ)
+			goto out;
+
 		tty_insert_flip_char(port, rx, flg);
 	}
 
@@ -910,7 +914,8 @@ static void dma_rx_callback(void *data)
 	dev_dbg(sport->port.dev, "We get %d bytes.\n", count);
 
 	if (count) {
-		tty_insert_flip_string(port, sport->rx_buf, count);
+		if (!(sport->port.ignore_status_mask & URXD_DUMMY_READ))
+			tty_insert_flip_string(port, sport->rx_buf, count);
 		tty_flip_buffer_push(port);
 
 		start_rx_dma(sport);
@@ -1382,6 +1387,9 @@ imx_set_termios(struct uart_port *port, struct ktermios *termios,
 			sport->port.ignore_status_mask |= URXD_OVRRUN;
 	}
 
+	if ((termios->c_cflag & CREAD) == 0)
+		sport->port.ignore_status_mask |= URXD_DUMMY_READ;
+
 	/*
 	 * Update the per-port timeout.
 	 */
-- 
1.9.3

[PATCH v1 02/15] serial: imx: use locking to stop concurrent access of UCR1

[Jiada Wang]

Several places are accessing the UCR1 register without locking.
This probably will cause a race issue when another thread
is accessing the same register.
Add locking to preventing concurrent access of the UCR1 register.

Signed-off-by: Jiada Wang <jiada_wang@mentor.com>
---
 drivers/tty/serial/imx.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
index 4d3fa53..969bc65 100644
--- a/drivers/tty/serial/imx.c
+++ b/drivers/tty/serial/imx.c
@@ -731,6 +731,9 @@ static int start_rx_dma(struct imx_port *sport);
 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) {
@@ -744,6 +747,8 @@ static void imx_dma_rxint(struct imx_port *sport)
 		/* tell the DMA to receive the data. */
 		start_rx_dma(sport);
 	}
+
+	spin_unlock_irqrestore(&sport->port.lock, flags);
 }
 
 static irqreturn_t imx_int(int irq, void *dev_id)
@@ -873,6 +878,9 @@ static int imx_setup_ufcr(struct imx_port *sport, unsigned int mode)
 static void imx_rx_dma_done(struct imx_port *sport)
 {
 	unsigned long temp;
+	unsigned long flags;
+
+	spin_lock_irqsave(&sport->port.lock, flags);
 
 	/* Enable this interrupt when the RXFIFO is empty. */
 	temp = readl(sport->port.membase + UCR1);
@@ -884,6 +892,8 @@ static void imx_rx_dma_done(struct imx_port *sport)
 	/* Is the shutdown waiting for us? */
 	if (waitqueue_active(&sport->dma_wait))
 		wake_up(&sport->dma_wait);
+
+	spin_unlock_irqrestore(&sport->port.lock, flags);
 }
 
 /*
@@ -1235,9 +1245,11 @@ static void imx_shutdown(struct uart_port *port)
 			dmaengine_terminate_all(sport->dma_chan_tx);
 			dmaengine_terminate_all(sport->dma_chan_rx);
 		}
+		spin_lock_irqsave(&sport->port.lock, flags);
 		imx_stop_tx(port);
 		imx_stop_rx(port);
 		imx_disable_dma(sport);
+		spin_unlock_irqrestore(&sport->port.lock, flags);
 		imx_uart_dma_exit(sport);
 	}
 
-- 
1.9.3

[PATCH v1 03/15] serial: imx: remove unneeded imx_transmit_buffer() from imx_start_tx()

[Jiada Wang]

From: Dirk Behme <dirk.behme@de.bosch.com>

Use imx_start_tx() just to enable the TX interrupt. It's the job of the
TX interrupt ISR to fill the transmit buffer, then. If the transmit buffer
is empty, the TX interrupt should be executed as soon as the start_tx()
enables the interrupt, so there is no reason for the extra
imx_transmit_buffer() call, here. Remove it.

Signed-off-by: Dirk Behme <dirk.behme@de.bosch.com>
Signed-off-by: Andy Lowe <andy_lowe@mentor.com>
---
 drivers/tty/serial/imx.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
index 969bc65..be05b75 100644
--- a/drivers/tty/serial/imx.c
+++ b/drivers/tty/serial/imx.c
@@ -621,9 +621,6 @@ static void imx_start_tx(struct uart_port *port)
 			imx_dma_tx(sport);
 		return;
 	}
-
-	if (readl(sport->port.membase + uts_reg(sport)) & UTS_TXEMPTY)
-		imx_transmit_buffer(sport);
 }
 
 static irqreturn_t imx_rtsint(int irq, void *dev_id)
-- 
1.9.3

[PATCH v1 04/15] serial: imx: TX DMA: clean up sg initialization

[Jiada Wang]

From: Dirk Behme <dirk.behme@de.bosch.com>

Inverting the logic of the if statement for the sg initialization
makes the if statement easier and better to read.

No functional change.

Signed-off-by: Dirk Behme <dirk.behme@de.bosch.com>
---
 drivers/tty/serial/imx.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
index be05b75..f4b0f6b 100644
--- a/drivers/tty/serial/imx.c
+++ b/drivers/tty/serial/imx.c
@@ -539,15 +539,15 @@ static void imx_dma_tx(struct imx_port *sport)
 
 	sport->tx_bytes = uart_circ_chars_pending(xmit);
 
-	if (xmit->tail > xmit->head && xmit->head > 0) {
+	if (xmit->tail < xmit->head) {
+		sport->dma_tx_nents = 1;
+		sg_init_one(sgl, xmit->buf + xmit->tail, sport->tx_bytes);
+	} else {
 		sport->dma_tx_nents = 2;
 		sg_init_table(sgl, 2);
 		sg_set_buf(sgl, xmit->buf + xmit->tail,
 				UART_XMIT_SIZE - xmit->tail);
 		sg_set_buf(sgl + 1, xmit->buf, xmit->head);
-	} else {
-		sport->dma_tx_nents = 1;
-		sg_init_one(sgl, xmit->buf + xmit->tail, sport->tx_bytes);
 	}
 
 	ret = dma_map_sg(dev, sgl, sport->dma_tx_nents, DMA_TO_DEVICE);
-- 
1.9.3

[PATCH v1 05/15] serial: imx: unmap sg in case of dmaengine_prep_slave_sg() failure

[Jiada Wang]

From: Dirk Behme <dirk.behme@de.bosch.com>

In case dmaengine_prep_slave_sg() fails, add the missing dma_unmap_sg().

Signed-off-by: Dirk Behme <dirk.behme@de.bosch.com>
Signed-off-by: Anton Bondarenko <anton_bondarenko@mentor.com>
---
 drivers/tty/serial/imx.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
index f4b0f6b..a0bfccf 100644
--- a/drivers/tty/serial/imx.c
+++ b/drivers/tty/serial/imx.c
@@ -558,6 +558,8 @@ static void imx_dma_tx(struct imx_port *sport)
 	desc = dmaengine_prep_slave_sg(chan, sgl, sport->dma_tx_nents,
 					DMA_MEM_TO_DEV, DMA_PREP_INTERRUPT);
 	if (!desc) {
+		dma_unmap_sg(dev, sgl, sport->dma_tx_nents,
+			     DMA_TO_DEVICE);
 		dev_err(dev, "We cannot prepare for the TX slave dma!\n");
 		return;
 	}
@@ -947,6 +949,7 @@ static int start_rx_dma(struct imx_port *sport)
 	desc = dmaengine_prep_slave_sg(chan, sgl, 1, DMA_DEV_TO_MEM,
 					DMA_PREP_INTERRUPT);
 	if (!desc) {
+		dma_unmap_sg(dev, sgl, 1, DMA_FROM_DEVICE);
 		dev_err(dev, "We cannot prepare for the RX slave dma!\n");
 		return -EINVAL;
 	}
-- 
1.9.3

[PATCH v1 06/15] serial: imx: unmap scatter gather list in imx_flush_buffer

[Jiada Wang]

From: Dirk Behme <dirk.behme@de.bosch.com>

First, reformat the code to exit immediately. This allows us to add
more code in more readable format.

In case the TX DMA was still running, remove and disable it's resources.

Signed-off-by: Dirk Behme <dirk.behme@de.bosch.com>
---
 drivers/tty/serial/imx.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
index a0bfccf..e53a058 100644
--- a/drivers/tty/serial/imx.c
+++ b/drivers/tty/serial/imx.c
@@ -1302,10 +1302,17 @@ static void imx_shutdown(struct uart_port *port)
 static void imx_flush_buffer(struct uart_port *port)
 {
 	struct imx_port *sport = (struct imx_port *)port;
+	struct scatterlist *sgl = &sport->tx_sgl[0];
 
-	if (sport->dma_is_enabled) {
-		sport->tx_bytes = 0;
-		dmaengine_terminate_all(sport->dma_chan_tx);
+	if (!sport->dma_chan_tx)
+		return;
+
+	sport->tx_bytes = 0;
+	dmaengine_terminate_all(sport->dma_chan_tx);
+	if (sport->dma_is_txing) {
+		dma_unmap_sg(sport->port.dev, sgl, sport->dma_tx_nents,
+			     DMA_TO_DEVICE);
+		sport->dma_is_txing = false;
 	}
 }
 
-- 
1.9.3

[PATCH v1 07/15] serial: imx: use dma_is_txing to synchronize dma_tx_callback and imx_dma_tx

[Jiada Wang]

From: Dirk Behme <dirk.behme@de.bosch.com>

To synchronize between dma_tx_callback() and imx_dma_tx() use the same
variable, dma_is_txing. This prevents any race between these two functions
and ensures that a new DMA can start only after the first has been
finished.

Before the new DMA can be set up, update the circular buffer logic, first.
Therefore, change dma_is_txing after that update, instead of before.

While doing this, in dma_tx_callback() extend the locking to dma_unmap_sg()
and the update of dma_is_txing.

Signed-off-by: Dirk Behme <dirk.behme@de.bosch.com>
---
 drivers/tty/serial/imx.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
index e53a058..3db6a5b 100644
--- a/drivers/tty/serial/imx.c
+++ b/drivers/tty/serial/imx.c
@@ -502,18 +502,20 @@ static void dma_tx_callback(void *data)
 	struct circ_buf *xmit = &sport->port.state->xmit;
 	unsigned long flags;
 
-	dma_unmap_sg(sport->port.dev, sgl, sport->dma_tx_nents, DMA_TO_DEVICE);
+	spin_lock_irqsave(&sport->port.lock, flags);
 
-	sport->dma_is_txing = 0;
+	dma_unmap_sg(sport->port.dev, sgl, sport->dma_tx_nents, DMA_TO_DEVICE);
 
 	/* update the stat */
-	spin_lock_irqsave(&sport->port.lock, flags);
 	xmit->tail = (xmit->tail + sport->tx_bytes) & (UART_XMIT_SIZE - 1);
 	sport->port.icount.tx += sport->tx_bytes;
-	spin_unlock_irqrestore(&sport->port.lock, flags);
 
 	dev_dbg(sport->port.dev, "we finish the TX DMA.\n");
 
+	sport->dma_is_txing = 0;
+
+	spin_unlock_irqrestore(&sport->port.lock, flags);
+
 	uart_write_wakeup(&sport->port);
 
 	if (waitqueue_active(&sport->dma_wait)) {
@@ -530,11 +532,9 @@ static void imx_dma_tx(struct imx_port *sport)
 	struct dma_async_tx_descriptor *desc;
 	struct dma_chan	*chan = sport->dma_chan_tx;
 	struct device *dev = sport->port.dev;
-	enum dma_status status;
 	int ret;
 
-	status = dmaengine_tx_status(chan, (dma_cookie_t)0, NULL);
-	if (DMA_IN_PROGRESS == status)
+	if (sport->dma_is_txing)
 		return;
 
 	sport->tx_bytes = uart_circ_chars_pending(xmit);
-- 
1.9.3

[PATCH v1 08/15] Revert "serial: imx: always wake up the processes in the TX callback"

[Jiada Wang]

This reverts commit 2ad28e3efee21a5bbf940c83d1f0395b76bd3efb.

Instead of always wake up write_wait process in TX callback,
TX callback should call imx_dma_tx() again, and let imx_dma_tx
transfer the remaining data in circle buffer.

The issue with commit 2ad28e3 is, in case there is remaining
data in circle buffer, but no process is waiting on write_wait
queue, then as no following uart_write() will be called after
uart_write_wakeup(), thus cause data loss.

Moreover according to Documentation/serial/driver, uart_write_wakeup()
should be called in case the transmit buffer have dropped below
a threshold.

Signed-off-by: Jiada Wang <jiada_wang@mentor.com>
---
 drivers/tty/serial/imx.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
index 3db6a5b..3d86851 100644
--- a/drivers/tty/serial/imx.c
+++ b/drivers/tty/serial/imx.c
@@ -516,7 +516,8 @@ static void dma_tx_callback(void *data)
 
 	spin_unlock_irqrestore(&sport->port.lock, flags);
 
-	uart_write_wakeup(&sport->port);
+	if (uart_circ_chars_pending(xmit) < WAKEUP_CHARS)
+		uart_write_wakeup(&sport->port);
 
 	if (waitqueue_active(&sport->dma_wait)) {
 		wake_up(&sport->dma_wait);
-- 
1.9.3

[PATCH v1 09/15] serial: imx: call imx_dma_tx() again in dma_tx_callback

[Jiada Wang]

Currently in dma_tx_callback(), no matter if there is still
remaining data pending in circle buffer or not, DMA transmit
will be terminated.

This will result in some data never get transmitted.
In order to fix this issue, call imx_dma_tx() again in
dma_tx_callback, when there is pending data and uart hasn't
been stopped.

Signed-off-by: Jiada Wang <jiada_wang@mentor.com>
Signed-off-by: Dirk Behme <dirk.behme@de.bosch.com>
---
 drivers/tty/serial/imx.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
index 3d86851..c9e6891 100644
--- a/drivers/tty/serial/imx.c
+++ b/drivers/tty/serial/imx.c
@@ -495,6 +495,7 @@ static inline void imx_transmit_buffer(struct imx_port *sport)
 		imx_stop_tx(&sport->port);
 }
 
+static void imx_dma_tx(struct imx_port *sport);
 static void dma_tx_callback(void *data)
 {
 	struct imx_port *sport = data;
@@ -524,6 +525,11 @@ static void dma_tx_callback(void *data)
 		dev_dbg(sport->port.dev, "exit in %s.\n", __func__);
 		return;
 	}
+
+	spin_lock_irqsave(&sport->port.lock, flags);
+	if (!uart_circ_empty(xmit) && !uart_tx_stopped(&sport->port))
+		imx_dma_tx(sport);
+	spin_unlock_irqrestore(&sport->port.lock, flags);
 }
 
 static void imx_dma_tx(struct imx_port *sport)
-- 
1.9.3

[PATCH v1 10/15] serial: imx: disable TDMAEN in imx_flush_buffer()

[Jiada Wang]

From: Dirk Behme <dirk.behme@de.bosch.com>

Terminating the DMA, make sure the interrupt is disabled, too.
This fixes random kernel Oops due to dma_tx_call() called for
invalid transmissions.

If we disable the TDMAEN, make sure it's enabled again if a TX
DMA is started.

Signed-off-by: Jiada Wang <jiada_wang@mentor.com>
Signed-off-by: Dirk Behme <dirk.behme@de.bosch.com>
---
 drivers/tty/serial/imx.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
index c9e6891..d501628 100644
--- a/drivers/tty/serial/imx.c
+++ b/drivers/tty/serial/imx.c
@@ -502,11 +502,16 @@ static void dma_tx_callback(void *data)
 	struct scatterlist *sgl = &sport->tx_sgl[0];
 	struct circ_buf *xmit = &sport->port.state->xmit;
 	unsigned long flags;
+	unsigned long temp;
 
 	spin_lock_irqsave(&sport->port.lock, flags);
 
 	dma_unmap_sg(sport->port.dev, sgl, sport->dma_tx_nents, DMA_TO_DEVICE);
 
+	temp = readl(sport->port.membase + UCR1);
+	temp &= ~UCR1_TDMAEN;
+	writel(temp, sport->port.membase + UCR1);
+
 	/* update the stat */
 	xmit->tail = (xmit->tail + sport->tx_bytes) & (UART_XMIT_SIZE - 1);
 	sport->port.icount.tx += sport->tx_bytes;
@@ -539,6 +544,7 @@ static void imx_dma_tx(struct imx_port *sport)
 	struct dma_async_tx_descriptor *desc;
 	struct dma_chan	*chan = sport->dma_chan_tx;
 	struct device *dev = sport->port.dev;
+	unsigned long temp;
 	int ret;
 
 	if (sport->dma_is_txing)
@@ -575,6 +581,11 @@ static void imx_dma_tx(struct imx_port *sport)
 
 	dev_dbg(dev, "TX: prepare to send %lu bytes by DMA.\n",
 			uart_circ_chars_pending(xmit));
+
+	temp = readl(sport->port.membase + UCR1);
+	temp |= UCR1_TDMAEN;
+	writel(temp, sport->port.membase + UCR1);
+
 	/* fire it */
 	sport->dma_is_txing = 1;
 	dmaengine_submit(desc);
@@ -1310,6 +1321,7 @@ static void imx_flush_buffer(struct uart_port *port)
 {
 	struct imx_port *sport = (struct imx_port *)port;
 	struct scatterlist *sgl = &sport->tx_sgl[0];
+	unsigned long temp;
 
 	if (!sport->dma_chan_tx)
 		return;
@@ -1319,6 +1331,9 @@ static void imx_flush_buffer(struct uart_port *port)
 	if (sport->dma_is_txing) {
 		dma_unmap_sg(sport->port.dev, sgl, sport->dma_tx_nents,
 			     DMA_TO_DEVICE);
+		temp = readl(sport->port.membase + UCR1);
+		temp &= ~UCR1_TDMAEN;
+		writel(temp, sport->port.membase + UCR1);
 		sport->dma_is_txing = false;
 	}
 }
-- 
1.9.3

[PATCH v1 11/15] serial: imx: initialized DMA w/o HW flow enabled

[Jiada Wang]

From: Anton Bondarenko <anton_bondarenko@mentor.com>

DMA mode for UART can be used even w/o HW flow control with RTS/CTS.
So it need to be initialized and enabled earlier.

Signed-off-by: Anton Bondarenko <anton_bondarenko@mentor.com>
Signed-off-by: Jiada Wang <jiada_wang@mentor.com>
---
 drivers/tty/serial/imx.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
index d501628..a0f27b4 100644
--- a/drivers/tty/serial/imx.c
+++ b/drivers/tty/serial/imx.c
@@ -1173,12 +1173,20 @@ static int imx_startup(struct uart_port *port)
 		}
 	}
 
+	/* Can we enable the DMA support? */
+	if (is_imx6q_uart(sport) && !uart_console(port) &&
+	    !sport->dma_is_inited)
+		imx_uart_dma_init(sport);
+
 	spin_lock_irqsave(&sport->port.lock, flags);
 	/*
 	 * Finally, clear and enable interrupts
 	 */
 	writel(USR1_RTSD, sport->port.membase + USR1);
 
+	if (sport->dma_is_inited && !sport->dma_is_enabled)
+		imx_enable_dma(sport);
+
 	temp = readl(sport->port.membase + UCR1);
 	temp |= UCR1_RRDYEN | UCR1_RTSDEN | UCR1_UARTEN;
 
@@ -1378,11 +1386,6 @@ imx_set_termios(struct uart_port *port, struct ktermios *termios,
 		if (sport->have_rtscts) {
 			ucr2 &= ~UCR2_IRTS;
 			ucr2 |= UCR2_CTSC;
-
-			/* Can we enable the DMA support? */
-			if (is_imx6q_uart(sport) && !uart_console(port)
-				&& !sport->dma_is_inited)
-				imx_uart_dma_init(sport);
 		} else {
 			termios->c_cflag &= ~CRTSCTS;
 		}
@@ -1504,8 +1507,6 @@ imx_set_termios(struct uart_port *port, struct ktermios *termios,
 	if (UART_ENABLE_MS(&sport->port, termios->c_cflag))
 		imx_enable_ms(&sport->port);
 
-	if (sport->dma_is_inited && !sport->dma_is_enabled)
-		imx_enable_dma(sport);
 	spin_unlock_irqrestore(&sport->port.lock, flags);
 }
 
-- 
1.9.3

Re: [PATCH v1 11/15] serial: imx: initialized DMA w/o HW flow enabled

[Sebastian Andrzej Siewior]

On 2014-12-09 18:11:32 [+0900], Jiada Wang wrote:
> From: Anton Bondarenko <anton_bondarenko@mentor.com>
> 
> DMA mode for UART can be used even w/o HW flow control with RTS/CTS.
> So it need to be initialized and enabled earlier.
> 
> Signed-off-by: Anton Bondarenko <anton_bondarenko@mentor.com>
> Signed-off-by: Jiada Wang <jiada_wang@mentor.com>

How was this tested? I read you used the Sambe IMX6Q. I have here the
Wandboard which is IMX6D and a PBAB01 which is IMX6Q. Both have the same
issue with this patch, that is once DMA is enabled I receive the data in
question plus one extra byte which is 0x00.
That extra byte was not part of transaction. After that, the SDMA driver
is handling interrupts like crazing and feeding one byte data (usually
0x00 but sometimes 0x02 not sure if this new or whatever was there
before) to the core. Those one byte transaction are bogus of course.

My question is how was this tested. Before your patch none of my boards
were using DMA because RTS/CTS is not in use and this was a key
requirement. Now SDMA goes crazy. Is there a SDMA firmware required for
this to work?

Sebastian

Re: [PATCH v1 11/15] serial: imx: initialized DMA w/o HW flow enabled

[jiwang]

Hi Sebastian

On 04/09/2015 09:00 PM, Sebastian Andrzej Siewior wrote:
> On 2014-12-09 18:11:32 [+0900], Jiada Wang wrote:
>> From: Anton Bondarenko <anton_bondarenko@mentor.com>
>>
>> DMA mode for UART can be used even w/o HW flow control with RTS/CTS.
>> So it need to be initialized and enabled earlier.
>>
>> Signed-off-by: Anton Bondarenko <anton_bondarenko@mentor.com>
>> Signed-off-by: Jiada Wang <jiada_wang@mentor.com>
> How was this tested? I read you used the Sambe IMX6Q. I have here the
> Wandboard which is IMX6D and a PBAB01 which is IMX6Q. Both have the same
> issue with this patch, that is once DMA is enabled I receive the data in
> question plus one extra byte which is 0x00.
> That extra byte was not part of transaction. After that, the SDMA driver
> is handling interrupts like crazing and feeding one byte data (usually
> 0x00 but sometimes 0x02 not sure if this new or whatever was there
> before) to the core. Those one byte transaction are bogus of course.
>
> My question is how was this tested. Before your patch none of my boards
> were using DMA because RTS/CTS is not in use and this was a key
> requirement. Now SDMA goes crazy. Is there a SDMA firmware required for
> this to work?
We tested the patch set with our modified kernel tree,
and I find upstream kernel is not building SDMA firmware,
I will submit another patch to add it.

Thanks,
Jiada
> Sebastian

Re: [PATCH v1 11/15] serial: imx: initialized DMA w/o HW flow enabled

[Sebastian Andrzej Siewior]

On Mon, Apr 13, 2015 at 04:40:15PM +0900, jiwang wrote:
> Hi Sebastian
Hi Jiada,

> >My question is how was this tested. Before your patch none of my boards
> >were using DMA because RTS/CTS is not in use and this was a key
> >requirement. Now SDMA goes crazy. Is there a SDMA firmware required for
> >this to work?
> We tested the patch set with our modified kernel tree,
> and I find upstream kernel is not building SDMA firmware,
> I will submit another patch to add it.

Please make sure it is tagged stable. There is no hint that this is required
and as of it now, it breaks v4.0.
One question, where do you have the firmware from? I picked the one from FSL's
v3.10 SDK and I ended up with FIFO-overflows so it was clearly the wrong one
(and the built-in SDMA firmware does not work as expected as you mentioned).

Is this firmware only required the imx6 series or also for older versions like
imx5?

> Thanks,
> Jiada

Sebastian

Re: [PATCH v1 11/15] serial: imx: initialized DMA w/o HW flow enabled

[jiwang]

Hello Sebastian

On 04/13/2015 05:06 PM, Sebastian Andrzej Siewior wrote:
> On Mon, Apr 13, 2015 at 04:40:15PM +0900, jiwang wrote:
>> Hi Sebastian
> Hi Jiada,
>
>>> My question is how was this tested. Before your patch none of my boards
>>> were using DMA because RTS/CTS is not in use and this was a key
>>> requirement. Now SDMA goes crazy. Is there a SDMA firmware required for
>>> this to work?
>> We tested the patch set with our modified kernel tree,
>> and I find upstream kernel is not building SDMA firmware,
>> I will submit another patch to add it.
> Please make sure it is tagged stable. There is no hint that this is required
> and as of it now, it breaks v4.0.
Due to missing of SDMA firmware, uart sdma mode is broken,
but it is uncovered by this commit on your environment.

> One question, where do you have the firmware from? I picked the one from FSL's
> v3.10 SDK and I ended up with FIFO-overflows so it was clearly the wrong one
> (and the built-in SDMA firmware does not work as expected as you mentioned).
I got sdma firmware from FSL kernel tree.
> Is this firmware only required the imx6 series or also for older versions like
> imx5?
the firmware only supports imx6 series, AFAIK,
I checked with Freescale imx6 support team, due to
licensing issue, I am not entitled to upstream FSL SDMA firmware.
so seems currently disable SDMA support for uart is our only option
at the moment.

Thanks,
Jiada
>> Thanks,
>> Jiada
> Sebastian

Re: [PATCH v1 11/15] serial: imx: initialized DMA w/o HW flow enabled

[Sebastian Andrzej Siewior]

On 04/17/2015 11:18 AM, jiwang wrote:
> Hello Sebastian

Hallo Jiada,

>> Please make sure it is tagged stable. There is no hint that this is
>> required
>> and as of it now, it breaks v4.0.
> Due to missing of SDMA firmware, uart sdma mode is broken,
> but it is uncovered by this commit on your environment.
> 
>> One question, where do you have the firmware from? I picked the one
>> from FSL's
>> v3.10 SDK and I ended up with FIFO-overflows so it was clearly the
>> wrong one
>> (and the built-in SDMA firmware does not work as expected as you
>> mentioned).
> I got sdma firmware from FSL kernel tree.

Can you post a link please?

>> Is this firmware only required the imx6 series or also for older
>> versions like
>> imx5?
> the firmware only supports imx6 series, AFAIK,

What I meant is if imx5 (and earlier) have a working DMA without the
SDMA firmware which seems to be required for IMX6.

> I checked with Freescale imx6 support team, due to
> licensing issue, I am not entitled to upstream FSL SDMA firmware.
> so seems currently disable SDMA support for uart is our only option
> at the moment.
Disable it please, add a hint in the source so one knows _why_ it has
been disabled and push it stable. In future you would need some kind of
a hint from the sdma driver to let the uart know that the proper
firmware is in place.

Is it okay to send a patch against the linux-firmware tree?
	https://git.kernel.org/cgit/linux/kernel/git/firmware/linux-firmware.git/

> 
> Thanks,
> Jiada

Sebastian

Re: [PATCH v1 11/15] serial: imx: initialized DMA w/o HW flow enabled

[Nicolae Rosia]

Hi,
I believe I'm also affected by this BUG. I'm receiving lots of 0 and 1
(ASCII, as presented by cat /dev/ttymxcX) and RX buffer is
overflowing.

> the firmware only supports imx6 series, AFAIK,
> I checked with Freescale imx6 support team, due to
> licensing issue, I am not entitled to upstream FSL SDMA firmware.
> so seems currently disable SDMA support for uart is our only option
> at the moment.
>

Have you submited a patch for this issue?
How do I disable SDMA?

Best regards,
Nicolae Rosia

Re: [PATCH v1 11/15] serial: imx: initialized DMA w/o HW flow enabled

[Sebastian Andrzej Siewior]

On 05/06/2015 06:08 PM, Nicolae Rosia wrote:
> Hi,
> I believe I'm also affected by this BUG. I'm receiving lots of 0 and 1
> (ASCII, as presented by cat /dev/ttymxcX) and RX buffer is
> overflowing.
> 
>> the firmware only supports imx6 series, AFAIK,
>> I checked with Freescale imx6 support team, due to
>> licensing issue, I am not entitled to upstream FSL SDMA firmware.
>> so seems currently disable SDMA support for uart is our only option
>> at the moment.
>>
> 
> Have you submited a patch for this issue?
> How do I disable SDMA?

I reverted this patch. I am going to send a revert to Greg tomorrow
including why this is done and CC stable. I haven't yet received a
working SDMA firmware nor a hint where it is.

> Best regards,
> Nicolae Rosia

Sebastian

Re: [PATCH v1 11/15] serial: imx: initialized DMA w/o HW flow enabled

[Nicolae Rosia]

Hi,

> I reverted this patch. I am going to send a revert to Greg tomorrow
> including why this is done and CC stable. I haven't yet received a
> working SDMA firmware nor a hint where it is.
You reverted this patch and it works again?
I believe it is this one [1], but I will be only able to test tomorrow.

[1] http://git.freescale.com/git/cgit.cgi/imx/linux-2.6-imx.git/tree/firmware/imx/sdma?h=imx_3.14.28_1.0.0_ga

Best regards,
Nicolae Rosia

Re: [PATCH v1 11/15] serial: imx: initialized DMA w/o HW flow enabled

[Sebastian Andrzej Siewior]

On 05/06/2015 06:46 PM, Nicolae Rosia wrote:
> Hi,

Hi,

>> I reverted this patch. I am going to send a revert to Greg tomorrow
>> including why this is done and CC stable. I haven't yet received a
>> working SDMA firmware nor a hint where it is.
> You reverted this patch and it works again?
> I believe it is this one [1], but I will be only able to test tomorrow.

Okay, please inform about your results. I think I had that one and the
result was no UART communication at all and a lot overrun errors.

> [1] http://git.freescale.com/git/cgit.cgi/imx/linux-2.6-imx.git/tree/firmware/imx/sdma?h=imx_3.14.28_1.0.0_ga
> 
> Best regards,
> Nicolae Rosia
> 
Sebastian

Re: [PATCH v1 11/15] serial: imx: initialized DMA w/o HW flow enabled

[Nicolae Rosia]

Hi,
>
> Okay, please inform about your results. I think I had that one and the
> result was no UART communication at all and a lot overrun errors.

I have reverted this patch and the serial comm is working well.
I haven't been able to test with the SDMA firmware, yet.
Thank you for the bug report and fix!

Best regards,
Nicolae Rosia

Re: [PATCH v1 11/15] serial: imx: initialized DMA w/o HW flow enabled

[Sebastian Andrzej Siewior]

On 05/07/2015 03:55 PM, Nicolae Rosia wrote:
> Hi,

Hi,

>> Okay, please inform about your results. I think I had that one and the
>> result was no UART communication at all and a lot overrun errors.
> 
> I have reverted this patch and the serial comm is working well.
> I haven't been able to test with the SDMA firmware, yet.
> Thank you for the bug report and fix!

Okay. So I am definitely sending a revert tonight since I haven't got
any news about the firmware. If you happen to have the time to test
with SDMA firmware, please let me know (no matter if it worked or not).

> Best regards,
> Nicolae Rosia

Sebastian

[PATCH v1 12/15] serial: imx: start rx_dma once RXFIFO is not empty

[Jiada Wang]

From: Robin Gong <b38343@freescale.com>

Start rx_dma once RXFIFO is not empty that can avoid dma request lost
and causes data delay issue.

Signed-off-by: Robin Gong <b38343@freescale.com>
Signed-off-by: Fugang Duan <B38611@freescale.com>
---
 drivers/tty/serial/imx.c | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
index a0f27b4..f0df233 100644
--- a/drivers/tty/serial/imx.c
+++ b/drivers/tty/serial/imx.c
@@ -946,8 +946,21 @@ static void dma_rx_callback(void *data)
 		tty_flip_buffer_push(port);
 
 		start_rx_dma(sport);
-	} else
+	} else if (readl(sport->port.membase + USR2) & USR2_RDR) {
+		/*
+		 * start rx_dma directly once data in RXFIFO, more efficient
+		 * than before:
+		 *	1. call imx_rx_dma_done to stop dma if no data received
+		 *	2. wait next  RDR interrupt to start dma transfer.
+		 */
+		start_rx_dma(sport);
+	} else {
+		/*
+		 * stop dma to prevent too many IDLE event trigged if no data
+		 * in RXFIFO
+		 */
 		imx_rx_dma_done(sport);
+	}
 }
 
 static int start_rx_dma(struct imx_port *sport)
-- 
1.9.3

[PATCH v1 13/15] serial: imx: Enable UCR4_OREN in startup interface

[Jiada Wang]

Other than enable Receiver Overrun Interrupt Enable (UCR4_OREN)
in start_tx interface, UCR4_OREN should be enabled before enable
of Receiver.

Signed-off-by: Jiada Wang <jiada_wang@mentor.com>
---
 drivers/tty/serial/imx.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
index f0df233..94ee7c5 100644
--- a/drivers/tty/serial/imx.c
+++ b/drivers/tty/serial/imx.c
@@ -611,13 +611,6 @@ static void imx_start_tx(struct uart_port *port)
 		temp &= ~(UCR1_RRDYEN);
 		writel(temp, sport->port.membase + UCR1);
 	}
-	/* Clear any pending ORE flag before enabling interrupt */
-	temp = readl(sport->port.membase + USR2);
-	writel(temp | USR2_ORE, sport->port.membase + USR2);
-
-	temp = readl(sport->port.membase + UCR4);
-	temp |= UCR4_OREN;
-	writel(temp, sport->port.membase + UCR4);
 
 	if (!sport->dma_is_enabled) {
 		temp = readl(sport->port.membase + UCR1);
@@ -1210,6 +1203,14 @@ static int imx_startup(struct uart_port *port)
 
 	writel(temp, sport->port.membase + UCR1);
 
+	/* Clear any pending ORE flag before enabling interrupt */
+	temp = readl(sport->port.membase + USR2);
+	writel(temp | USR2_ORE, sport->port.membase + USR2);
+
+	temp = readl(sport->port.membase + UCR4);
+	temp |= UCR4_OREN;
+	writel(temp, sport->port.membase + UCR4);
+
 	temp = readl(sport->port.membase + UCR2);
 	temp |= (UCR2_RXEN | UCR2_TXEN);
 	if (!sport->have_rtscts)
-- 
1.9.3

[PATCH v1 14/15] serial: imx: Fix issue in software flow control

[Jiada Wang]

After send out x_char in UART driver, x_char needs to be cleared
by UART driver itself, otherwise data in TXFIFO can no longer be
sent out.
Also tx counter needs to be increased to keep track of correct
number of transmitted data.

Signed-off-by: Jiada Wang <jiada_wang@mentor.com>
---
 drivers/tty/serial/imx.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
index 94ee7c5..301cf8c 100644
--- a/drivers/tty/serial/imx.c
+++ b/drivers/tty/serial/imx.c
@@ -471,6 +471,8 @@ static inline void imx_transmit_buffer(struct imx_port *sport)
 	if (sport->port.x_char) {
 		/* Send next char */
 		writel(sport->port.x_char, sport->port.membase + URTX0);
+		sport->port.icount.tx++;
+		sport->port.x_char = 0;
 		return;
 	}
 
-- 
1.9.3

[PATCH v1 15/15] serial: imx: Support sw flow control in DMA mode

[Jiada Wang]

This patch adds Software flow control support in DMA mode.

Signed-off-by: Jiada Wang <jiada_wang@mentor.com>
---
 drivers/tty/serial/imx.c | 30 ++++++++++++++++++++++++++++--
 1 file changed, 28 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
index 301cf8c..642e88e 100644
--- a/drivers/tty/serial/imx.c
+++ b/drivers/tty/serial/imx.c
@@ -464,9 +464,11 @@ static void imx_enable_ms(struct uart_port *port)
 	mod_timer(&sport->timer, jiffies);
 }
 
+static void imx_dma_tx(struct imx_port *sport);
 static inline void imx_transmit_buffer(struct imx_port *sport)
 {
 	struct circ_buf *xmit = &sport->port.state->xmit;
+	unsigned long temp;
 
 	if (sport->port.x_char) {
 		/* Send next char */
@@ -481,6 +483,22 @@ static inline void imx_transmit_buffer(struct imx_port *sport)
 		return;
 	}
 
+	if (sport->dma_is_enabled) {
+		/*
+		 * We've just sent a X-char Ensure the TX DMA is enabled
+		 * and the TX IRQ is disabled.
+		 **/
+		temp = readl(sport->port.membase + UCR1);
+		temp &= ~UCR1_TXMPTYEN;
+		if (sport->dma_is_txing) {
+			temp |= UCR1_TDMAEN;
+			writel(temp, sport->port.membase + UCR1);
+		} else {
+			writel(temp, sport->port.membase + UCR1);
+			imx_dma_tx(sport);
+		}
+	}
+
 	while (!uart_circ_empty(xmit) &&
 	       !(readl(sport->port.membase + uts_reg(sport)) & UTS_TXFULL)) {
 		/* send xmit->buf[xmit->tail]
@@ -497,7 +515,6 @@ static inline void imx_transmit_buffer(struct imx_port *sport)
 		imx_stop_tx(&sport->port);
 }
 
-static void imx_dma_tx(struct imx_port *sport);
 static void dma_tx_callback(void *data)
 {
 	struct imx_port *sport = data;
@@ -630,7 +647,16 @@ static void imx_start_tx(struct uart_port *port)
 	}
 
 	if (sport->dma_is_enabled) {
-		/* FIXME: port->x_char must be transmitted if != 0 */
+		if (sport->port.x_char) {
+			/* We have X-char to send, so enable TX IRQ and
+			 * disable TX DMA to let TX interrupt to send X-char */
+			temp = readl(sport->port.membase + UCR1);
+			temp &= ~UCR1_TDMAEN;
+			temp |= UCR1_TXMPTYEN;
+			writel(temp, sport->port.membase + UCR1);
+			return;
+		}
+
 		if (!uart_circ_empty(&port->state->xmit) &&
 		    !uart_tx_stopped(port))
 			imx_dma_tx(sport);
-- 
1.9.3