[PATCH 0/4] serail: imx: patches for DMA

[PATCH 0/4] serail: imx: patches for DMA

[Huang Shijie]

After i tested the DMA for uart with the Bluetooth. I found several small
places need to be fixed.

The patch set contains several fixes for the DMA support.

Huang Shijie (4):
  serial: imx: implement the flush_buffer hook
  serial: imx: check the DMA for imx_tx_empty
  serial: imx: fix the wrong number of scatterlist entries when
    xmit->head is 0
  serial: imx: use the dmaengine_tx_status

 drivers/tty/serial/imx.c |   26 ++++++++++++++++++++++----
 1 files changed, 22 insertions(+), 4 deletions(-)

-- 
1.7.2.rc3

[PATCH 1/4] serial: imx: implement the flush_buffer hook

[Huang Shijie]

The current driver does not implement the flush_buffer hook for
uart_ops. When we enable the DMA for the driver, and test it with Bluetooth,
we may meet the following bug for TX:

    [1] User application may call the flush operation at any time.
        The uart_flush_buffer() calls the uart_circ_clear() to set
        the xmit->head and xmit->tail with 0.

    [2] The TX DMA callback can be called at any time too.
        The dma_tx_call() will update the xmit->tail.

    If [2] occurs just after the [1], we will get the wrong xmit->tail.

This patch implements the flush_buffer hook to fix this issue.

Signed-off-by: Huang Shijie <b32955@freescale.com>
---
 drivers/tty/serial/imx.c |   11 +++++++++++
 1 files changed, 11 insertions(+), 0 deletions(-)

diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
index c07d9bb..708ba89 100644
--- a/drivers/tty/serial/imx.c
+++ b/drivers/tty/serial/imx.c
@@ -1303,6 +1303,16 @@ static void imx_shutdown(struct uart_port *port)
 	clk_disable_unprepare(sport->clk_ipg);
 }
 
+static void imx_flush_buffer(struct uart_port *port)
+{
+	struct imx_port *sport = (struct imx_port *)port;
+
+	if (sport->dma_is_enabled) {
+		sport->tx_bytes = 0;
+		dmaengine_terminate_all(sport->dma_chan_tx);
+	}
+}
+
 static void
 imx_set_termios(struct uart_port *port, struct ktermios *termios,
 		   struct ktermios *old)
@@ -1623,6 +1633,7 @@ static struct uart_ops imx_pops = {
 	.break_ctl	= imx_break_ctl,
 	.startup	= imx_startup,
 	.shutdown	= imx_shutdown,
+	.flush_buffer	= imx_flush_buffer,
 	.set_termios	= imx_set_termios,
 	.type		= imx_type,
 	.release_port	= imx_release_port,
-- 
1.7.2.rc3

[PATCH 1/4] serial: imx: implement the flush_buffer hook

[Wang, Jiada (ESD)]

Hi Shijie

By having this commit,
I am seeing random hit of BUG_ON() in dma_cookie_complete(),
When large data file is set to UART port with relately high baud rate.

I suspect even after dmaengine_terminate_all() is called in .flush_buffer(),
By some reason, the 'terminated sdma channel' 's irq handler is still got called sometimes
Thus cause a extra of dma_cookie_complete() executed randomly.

To Shawn and Sascha
I found your discussion about sdma_disable_channel in
http://permalink.gmane.org/gmane.linux.ports.arm.kernel/103283

this seems like a similar issue to the one I have observed in UART-SDMA usage
my understanding is there is potential race between stop of SDMA channel
and finish of SDMA transfer.
Do you have any ideas how to avoid this race condition?


Thanks,
Jiada

-----Original Message-----
From: linux-arm-kernel [mailto:linux-arm-kernel-bounces at lists.infradead.org] On Behalf Of Huang Shijie
Sent: Friday, October 11, 2013 7:31 PM
To: gregkh at linuxfoundation.org
Cc: Huang Shijie; shawn.guo at linaro.org; linux-arm-kernel at lists.infradead.org; linux-serial at vger.kernel.org
Subject: [PATCH 1/4] serial: imx: implement the flush_buffer hook

The current driver does not implement the flush_buffer hook for uart_ops. When we enable the DMA for the driver, and test it with Bluetooth, we may meet the following bug for TX:

    [1] User application may call the flush operation at any time.
        The uart_flush_buffer() calls the uart_circ_clear() to set
        the xmit->head and xmit->tail with 0.

    [2] The TX DMA callback can be called at any time too.
        The dma_tx_call() will update the xmit->tail.

    If [2] occurs just after the [1], we will get the wrong xmit->tail.

This patch implements the flush_buffer hook to fix this issue.

Signed-off-by: Huang Shijie <b32955@freescale.com>
---
 drivers/tty/serial/imx.c |   11 +++++++++++
 1 files changed, 11 insertions(+), 0 deletions(-)

diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c index c07d9bb..708ba89 100644
--- a/drivers/tty/serial/imx.c
+++ b/drivers/tty/serial/imx.c
@@ -1303,6 +1303,16 @@ static void imx_shutdown(struct uart_port *port)
 	clk_disable_unprepare(sport->clk_ipg);
 }
 
+static void imx_flush_buffer(struct uart_port *port) {
+	struct imx_port *sport = (struct imx_port *)port;
+
+	if (sport->dma_is_enabled) {
+		sport->tx_bytes = 0;
+		dmaengine_terminate_all(sport->dma_chan_tx);
+	}
+}
+
 static void
 imx_set_termios(struct uart_port *port, struct ktermios *termios,
 		   struct ktermios *old)
@@ -1623,6 +1633,7 @@ static struct uart_ops imx_pops = {
 	.break_ctl	= imx_break_ctl,
 	.startup	= imx_startup,
 	.shutdown	= imx_shutdown,
+	.flush_buffer	= imx_flush_buffer,
 	.set_termios	= imx_set_termios,
 	.type		= imx_type,
 	.release_port	= imx_release_port,
--
1.7.2.rc3



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 2/4] serial: imx: check the DMA for imx_tx_empty

[Huang Shijie]

Assume the following situation:

  If the DMA is enabled, and the a TX DMA operation is working,
But we have not issued the TX DMA operation (or we have issued the
TX DMA operation with dma_async_issue_pending(), but the DMA has not
started to move the data from the memory to the TXFIFO).

At this time, we may get the wrong status of the transmitter when we
call the imx_tx_empty. In such situation, only check the USR2_TXDC
does not enough for us.

This patch checks the DMA's situation, and return 0 when the TX DMA is
working.

Signed-off-by: Huang Shijie <b32955@freescale.com>
---
 drivers/tty/serial/imx.c |    9 ++++++++-
 1 files changed, 8 insertions(+), 1 deletions(-)

diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
index 708ba89..c6c3b16 100644
--- a/drivers/tty/serial/imx.c
+++ b/drivers/tty/serial/imx.c
@@ -795,8 +795,15 @@ static irqreturn_t imx_int(int irq, void *dev_id)
 static unsigned int imx_tx_empty(struct uart_port *port)
 {
 	struct imx_port *sport = (struct imx_port *)port;
+	unsigned int ret;
 
-	return (readl(sport->port.membase + USR2) & USR2_TXDC) ?  TIOCSER_TEMT : 0;
+	ret = (readl(sport->port.membase + USR2) & USR2_TXDC) ?  TIOCSER_TEMT : 0;
+
+	/* If the TX DMA is working, return 0. */
+	if (sport->dma_is_enabled && sport->dma_is_txing)
+		ret = 0;
+
+	return ret;
 }
 
 /*
-- 
1.7.2.rc3

[PATCH 3/4] serial: imx: fix the wrong number of scatterlist entries when xmit->head is 0

[Huang Shijie]

When the (xmit->tail > xmit->head) is true and the xmit->head is 0,
we only need one DMA scatterlist in actually. Current code uses two DMA
scatterlists in this case, this is obviously wrong.

This patch fixes it.

Signed-off-by: Huang Shijie <b32955@freescale.com>
---
 drivers/tty/serial/imx.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
index c6c3b16..cadf7e5 100644
--- a/drivers/tty/serial/imx.c
+++ b/drivers/tty/serial/imx.c
@@ -532,7 +532,7 @@ static void dma_tx_work(struct work_struct *w)
 		return;
 	}
 
-	if (xmit->tail > xmit->head) {
+	if (xmit->tail > xmit->head && xmit->head > 0) {
 		sport->dma_tx_nents = 2;
 		sg_init_table(sgl, 2);
 		sg_set_buf(sgl, xmit->buf + xmit->tail,
-- 
1.7.2.rc3

[PATCH 4/4] serial: imx: use the dmaengine_tx_status

[Huang Shijie]

Use the dmaengine_tx_status to simplify the code, do not change any logic.

Signed-off-by: Huang Shijie <b32955@freescale.com>
---
 drivers/tty/serial/imx.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
index cadf7e5..13c2c2d 100644
--- a/drivers/tty/serial/imx.c
+++ b/drivers/tty/serial/imx.c
@@ -521,7 +521,7 @@ static void dma_tx_work(struct work_struct *w)
 	unsigned long flags;
 	int ret;
 
-	status = chan->device->device_tx_status(chan, (dma_cookie_t)0, NULL);
+	status = dmaengine_tx_status(chan, (dma_cookie_t)0, NULL);
 	if (DMA_IN_PROGRESS == status)
 		return;
 
@@ -926,7 +926,7 @@ static void dma_rx_callback(void *data)
 	/* unmap it first */
 	dma_unmap_sg(sport->port.dev, sgl, 1, DMA_FROM_DEVICE);
 
-	status = chan->device->device_tx_status(chan, (dma_cookie_t)0, &state);
+	status = dmaengine_tx_status(chan, (dma_cookie_t)0, &state);
 	count = RX_BUF_SIZE - state.residue;
 	dev_dbg(sport->port.dev, "We get %d bytes.\n", count);
 
-- 
1.7.2.rc3