linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/4] serail: imx: patches for DMA
@ 2013-10-11 10:30 Huang Shijie
  2013-10-11 10:30 ` [PATCH 1/4] serial: imx: implement the flush_buffer hook Huang Shijie
                   ` (3 more replies)
  0 siblings, 4 replies; 6+ messages in thread
From: Huang Shijie @ 2013-10-11 10:30 UTC (permalink / raw)
  To: linux-arm-kernel

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 1/4] serial: imx: implement the flush_buffer hook
  2013-10-11 10:30 [PATCH 0/4] serail: imx: patches for DMA Huang Shijie
@ 2013-10-11 10:30 ` Huang Shijie
  2014-06-03  5:51   ` Wang, Jiada (ESD)
  2013-10-11 10:30 ` [PATCH 2/4] serial: imx: check the DMA for imx_tx_empty Huang Shijie
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 6+ messages in thread
From: Huang Shijie @ 2013-10-11 10:30 UTC (permalink / raw)
  To: linux-arm-kernel

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

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 2/4] serial: imx: check the DMA for imx_tx_empty
  2013-10-11 10:30 [PATCH 0/4] serail: imx: patches for DMA Huang Shijie
  2013-10-11 10:30 ` [PATCH 1/4] serial: imx: implement the flush_buffer hook Huang Shijie
@ 2013-10-11 10:30 ` Huang Shijie
  2013-10-11 10:31 ` [PATCH 3/4] serial: imx: fix the wrong number of scatterlist entries when xmit->head is 0 Huang Shijie
  2013-10-11 10:31 ` [PATCH 4/4] serial: imx: use the dmaengine_tx_status Huang Shijie
  3 siblings, 0 replies; 6+ messages in thread
From: Huang Shijie @ 2013-10-11 10:30 UTC (permalink / raw)
  To: linux-arm-kernel

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

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 3/4] serial: imx: fix the wrong number of scatterlist entries when xmit->head is 0
  2013-10-11 10:30 [PATCH 0/4] serail: imx: patches for DMA Huang Shijie
  2013-10-11 10:30 ` [PATCH 1/4] serial: imx: implement the flush_buffer hook Huang Shijie
  2013-10-11 10:30 ` [PATCH 2/4] serial: imx: check the DMA for imx_tx_empty Huang Shijie
@ 2013-10-11 10:31 ` Huang Shijie
  2013-10-11 10:31 ` [PATCH 4/4] serial: imx: use the dmaengine_tx_status Huang Shijie
  3 siblings, 0 replies; 6+ messages in thread
From: Huang Shijie @ 2013-10-11 10:31 UTC (permalink / raw)
  To: linux-arm-kernel

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

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 4/4] serial: imx: use the dmaengine_tx_status
  2013-10-11 10:30 [PATCH 0/4] serail: imx: patches for DMA Huang Shijie
                   ` (2 preceding siblings ...)
  2013-10-11 10:31 ` [PATCH 3/4] serial: imx: fix the wrong number of scatterlist entries when xmit->head is 0 Huang Shijie
@ 2013-10-11 10:31 ` Huang Shijie
  3 siblings, 0 replies; 6+ messages in thread
From: Huang Shijie @ 2013-10-11 10:31 UTC (permalink / raw)
  To: linux-arm-kernel

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

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 1/4] serial: imx: implement the flush_buffer hook
  2013-10-11 10:30 ` [PATCH 1/4] serial: imx: implement the flush_buffer hook Huang Shijie
@ 2014-06-03  5:51   ` Wang, Jiada (ESD)
  0 siblings, 0 replies; 6+ messages in thread
From: Wang, Jiada (ESD) @ 2014-06-03  5:51 UTC (permalink / raw)
  To: linux-arm-kernel

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2014-06-03  5:51 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-10-11 10:30 [PATCH 0/4] serail: imx: patches for DMA Huang Shijie
2013-10-11 10:30 ` [PATCH 1/4] serial: imx: implement the flush_buffer hook Huang Shijie
2014-06-03  5:51   ` Wang, Jiada (ESD)
2013-10-11 10:30 ` [PATCH 2/4] serial: imx: check the DMA for imx_tx_empty Huang Shijie
2013-10-11 10:31 ` [PATCH 3/4] serial: imx: fix the wrong number of scatterlist entries when xmit->head is 0 Huang Shijie
2013-10-11 10:31 ` [PATCH 4/4] serial: imx: use the dmaengine_tx_status Huang Shijie

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).