[PATCH v2 2/2] serial: mxs-auart: wait for FIFO to flush before shutdown
From: Uwe Kleine-König <hidden>
Date: 2013-10-02 20:18:34
Also in:
linux-serial
Hello Hector, On Wed, Oct 02, 2013 at 04:01:47PM +0200, Hector Palacios wrote:
quoted hunk ↗ jump to hunk
On 10/02/2013 02:22 PM, Uwe Kleine-K?nig wrote:quoted
On Wed, Oct 02, 2013 at 02:02:44PM +0200, Hector Palacios wrote:quoted
The shutdown function was not waiting for the FIFO (which may be the real 16 byte FIFO or the DMA buffer, if DMA is enabled) to flush before disabling the AUART. This lead to many bytes not being transferred (specially at low baudrates), as they were still in the DMA buffer when the AUART was shutdown. This patch also adds the check for the BUSY flag before disabling the AUART. Signed-off-by: Hector Palacios <redacted> --- drivers/tty/serial/mxs-auart.c | 13 +++++++++++++ 1 file changed, 13 insertions(+)diff --git a/drivers/tty/serial/mxs-auart.c b/drivers/tty/serial/mxs-auart.c index 9f046177..589b595 100644 --- a/drivers/tty/serial/mxs-auart.c +++ b/drivers/tty/serial/mxs-auart.c@@ -757,9 +757,22 @@ static int mxs_auart_startup(struct uart_port *u) return 0; } +static unsigned int mxs_auart_tx_empty(struct uart_port *u); + static void mxs_auart_shutdown(struct uart_port *u) { struct mxs_auart_port *s = to_auart_port(u); + unsigned int to; + + /* Wait for the FIFO to flush */ + to = u->timeout; + while (!mxs_auart_tx_empty(u) && to--) + mdelay(1); + + /* Wait for UART to become idle ... */ + to = u->timeout; + while ((readl(u->membase + AUART_STAT) & AUART_STAT_BUSY) && to--) + mdelay(1);If the 2nd loop is needed the tx_empty callback is buggy. According to Documentation/serial/driver tx_empty "tests whether the transmitter fifo and shifter for the port [...] is empty". I guess it only tests for the fifo part? Time for another fix ...Do you mean the tx_empty should check both for TX_FIFO and !BUSY, right? I did it so because I thought the BUSY was also set during rx, but it is really only used to signal tx operation. That would turn this patch into this:@@ -755,13 +755,21 @@ static int mxs_auart_startup(struct uart_port *u) writel(AUART_LINECTRL_FEN, u->membase + AUART_LINECTRL_SET); return 0; } +static unsigned int mxs_auart_tx_empty(struct uart_port *u);
There is no need for a forward declaration if you move the whole function.
quoted hunk ↗ jump to hunk
+ static void mxs_auart_shutdown(struct uart_port *u) { struct mxs_auart_port *s = to_auart_port(u); + unsigned int to; + + /* Wait for the FIFO to flush */ + to = u->timeout; + while (!mxs_auart_tx_empty(u) && to--) + mdelay(1); if (auart_dma_enabled(s)) mxs_auart_dma_exit(s); writel(AUART_CTRL2_UARTEN, u->membase + AUART_CTRL2_CLR);@@ -774,14 +782,15 @@ static void mxs_auart_shutdown(struct uart_port *u) clk_disable_unprepare(s->clk); } static unsigned int mxs_auart_tx_empty(struct uart_port *u) { - if (readl(u->membase + AUART_STAT) & AUART_STAT_TXFE) - return TIOCSER_TEMT; - else - return 0; + if (!((readl(u->membase + AUART_STAT) & AUART_STAT_BUSY))) + if (readl(u->membase + AUART_STAT) & AUART_STAT_TXFE) + return TIOCSER_TEMT;
if you do stat = readl(u->membase + AUART_STAT); if ((stat & (AUART_STAT_BUSY | AUART_STAT_TXFE)) == AUART_STAT_TXFE) return TIOCSER_TEMT; you save one register read and a small race condition. Best regards Uwe -- Pengutronix e.K. | Uwe Kleine-K?nig | Industrial Linux Solutions | http://www.pengutronix.de/ |