Re: [PATCH] serial/mxs-auart: increase time to wait for transmitter to become idle

["Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>]

Hi Greg,

On Fri, Jun 28, 2013 at 11:49:41AM +0200, Uwe Kleine-König wrote:
> Without this patch the driver waits ~1 ms for the UART to become idle. At
> 115200n8 this time is (theoretically) enough to transfer 11.5 characters
> (= 115200 bits/s / (10 Bits/char) * 1ms). As the mxs-auart has a fifo size
> of 16 characters the clock is gated too early. The problem is worse for
> lower baud rates.
> 
> This only happens to really shut down the transmitter in the middle of a
> transfer if /dev/ttyAPPx isn't opened in userspace (e.g. by a getty) but
> was at least once (because the bootloader doesn't disable the transmitter).
> 
> So increase the timeout to 20 ms which should be enough for 9600n8, too.
> Moreover skip gating the clock if the timeout is elapsed.
> 
> Cc: stable@vger.kernel.org # v2.6.39+
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Any comments on this. I hoped this patch would make it in for 3.10 :-(

Best regards
Uwe

> ---
>  drivers/tty/serial/mxs-auart.c | 21 +++++++++++++--------
>  1 file changed, 13 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/tty/serial/mxs-auart.c b/drivers/tty/serial/mxs-auart.c
> index 4f5f161..7645079 100644
> --- a/drivers/tty/serial/mxs-auart.c
> +++ b/drivers/tty/serial/mxs-auart.c
> @@ -850,7 +850,7 @@ auart_console_write(struct console *co, const char *str, unsigned int count)
>  	struct mxs_auart_port *s;
>  	struct uart_port *port;
>  	unsigned int old_ctrl0, old_ctrl2;
> -	unsigned int to = 1000;
> +	unsigned int to = 20000;
>  
>  	if (co->index >= MXS_AUART_PORTS || co->index < 0)
>  		return;
> @@ -871,18 +871,23 @@ auart_console_write(struct console *co, const char *str, unsigned int count)
>  
>  	uart_console_write(port, str, count, mxs_auart_console_putchar);
>  
> -	/*
> -	 * Finally, wait for transmitter to become empty
> -	 * and restore the TCR
> -	 */
> +	/* Finally, wait for transmitter to become empty ... */
>  	while (readl(port->membase + AUART_STAT) & AUART_STAT_BUSY) {
> +		udelay(1);
>  		if (!to--)
>  			break;
> -		udelay(1);
>  	}
>  
> -	writel(old_ctrl0, port->membase + AUART_CTRL0);
> -	writel(old_ctrl2, port->membase + AUART_CTRL2);
> +	/*
> +	 * ... and restore the TCR if we waited long enough for the transmitter
> +	 * to be idle. This might keep the transmitter enabled although it is
> +	 * unused, but that is better than to disable it while it is still
> +	 * transmitting.
> +	 */
> +	if (!(readl(port->membase + AUART_STAT) & AUART_STAT_BUSY)) {
> +		writel(old_ctrl0, port->membase + AUART_CTRL0);
> +		writel(old_ctrl2, port->membase + AUART_CTRL2);
> +	}
>  
>  	clk_disable(s->clk);
>  }
> -- 
> 1.8.3.1
> 
> 

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html