lkml.org 
[lkml]   [2014]   [May]   [30]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
From
SubjectRE: [PATCH 1/2] serial: imx: remove the DMA wait queue
Date
Hi Shijie

After apply this patch into our kernel,
We are facing data hang issue when sending big size file (2M used in test) to uart port
Note: Rx port is also keep receiving data.

After read the implementation of uart_stop(),
I feel like, stop_tx() is used to perform flow control when like a XOFF is received.
Which means no data should be dropped, as they may need to be sent out,
When next start_tx() is called.

But by calling dmaengine_termiate_all(), the data already be submitted to DMA engine,
May be lost, thus cause data hang.

What do you think?

Thanks,
Jiada

-----Original Message-----
From: linux-arm-kernel [mailto:linux-arm-kernel-bounces@lists.infradead.org] On Behalf Of Huang Shijie
Sent: Friday, May 23, 2014 1:33 PM
To: gregkh@linuxfoundation.org
Cc: linux-arm-kernel@lists.infradead.org; Huang Shijie; linux-kernel@vger.kernel.org; linux-serial@vger.kernel.org
Subject: [PATCH 1/2] serial: imx: remove the DMA wait queue

The DMA wait queue makes the code very complicated:
For RX, the @->stop_rx hook does not really stop the RX;
For TX, the @->stop_tx hook does not really stop the TX.

The above make the imx_shutdown has to wait the RX/TX DMA to be finished.

In order to make code more simple, this patch removes the DMA wait queue.
By calling the dmaengine_terminate_all, this patch makes the RX stops immediately after we call the @->stop_rx hook, so does the TX.

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

diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c index d1f16d3..ed6cdf7 100644
--- a/drivers/tty/serial/imx.c
+++ b/drivers/tty/serial/imx.c
@@ -225,7 +225,6 @@ struct imx_port {
void *rx_buf;
unsigned int tx_bytes;
unsigned int dma_tx_nents;
- wait_queue_head_t dma_wait;
unsigned int saved_reg[11];
};

@@ -417,12 +416,10 @@ static void imx_stop_tx(struct uart_port *port)
return;
}

- /*
- * We are maybe in the SMP context, so if the DMA TX thread is running
- * on other cpu, we have to wait for it to finish.
- */
- if (sport->dma_is_enabled && sport->dma_is_txing)
- return;
+ if (sport->dma_is_enabled && sport->dma_is_txing) {
+ dmaengine_terminate_all(sport->dma_chan_tx);
+ sport->dma_is_txing = 0;
+ }

temp = readl(sport->port.membase + UCR1);
writel(temp & ~UCR1_TXMPTYEN, sport->port.membase + UCR1); @@ -436,12 +433,10 @@ static void imx_stop_rx(struct uart_port *port)
struct imx_port *sport = (struct imx_port *)port;
unsigned long temp;

- /*
- * We are maybe in the SMP context, so if the DMA TX thread is running
- * on other cpu, we have to wait for it to finish.
- */
- if (sport->dma_is_enabled && sport->dma_is_rxing)
- return;
+ if (sport->dma_is_enabled && sport->dma_is_rxing) {
+ dmaengine_terminate_all(sport->dma_chan_rx);
+ sport->dma_is_rxing = 0;
+ }

temp = readl(sport->port.membase + UCR2);
writel(temp & ~UCR2_RXEN, sport->port.membase + UCR2); @@ -498,12 +493,6 @@ static void dma_tx_callback(void *data)
dev_dbg(sport->port.dev, "we finish the TX DMA.\n");

uart_write_wakeup(&sport->port);
-
- if (waitqueue_active(&sport->dma_wait)) {
- wake_up(&sport->dma_wait);
- dev_dbg(sport->port.dev, "exit in %s.\n", __func__);
- return;
- }
}

static void imx_dma_tx(struct imx_port *sport) @@ -876,10 +865,6 @@ static void imx_rx_dma_done(struct imx_port *sport)
writel(temp, sport->port.membase + UCR1);

sport->dma_is_rxing = 0;
-
- /* Is the shutdown waiting for us? */
- if (waitqueue_active(&sport->dma_wait))
- wake_up(&sport->dma_wait);
}

/*
@@ -1026,8 +1011,6 @@ static void imx_enable_dma(struct imx_port *sport) {
unsigned long temp;

- init_waitqueue_head(&sport->dma_wait);
-
/* set UCR1 */
temp = readl(sport->port.membase + UCR1);
temp |= UCR1_RDMAEN | UCR1_TDMAEN | UCR1_ATDMAEN | @@ -1219,10 +1202,13 @@ static void imx_shutdown(struct uart_port *port)
unsigned long flags;

if (sport->dma_is_enabled) {
- /* We have to wait for the DMA to finish. */
- wait_event(sport->dma_wait,
- !sport->dma_is_rxing && !sport->dma_is_txing);
+ /*
+ * The upper layer may does not call the @->stop_tx and
+ * @->stop_rx, so we call them ourselves.
+ */
+ imx_stop_tx(port);
imx_stop_rx(port);
+
imx_disable_dma(sport);
imx_uart_dma_exit(sport);
}
--
1.7.8

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

\
 
 \ /
  Last update: 2014-05-30 12:01    [W:0.151 / U:0.032 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site