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

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

[Uwe Kleine-König]

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>
---
 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

--
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

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

[Uwe Kleine-König]

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

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

[Greg Kroah-Hartman]

On Thu, Jul 04, 2013 at 11:27:16AM +0200, Uwe Kleine-König wrote:
> 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 :-(

You sent it after my tree closed for 3.11, 3.10 is a stretch :)

I'll look at it after the 3.11-rc1 merge window is closed, give me a
chance, I'm a bit busy for the next week or so with the fallout of
that...

thanks,

greg k-h
--
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

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

[Uwe Kleine-König]

Hi Greg,

On Thu, Jul 04, 2013 at 08:58:36AM -0700, Greg Kroah-Hartman wrote:
> On Thu, Jul 04, 2013 at 11:27:16AM +0200, Uwe Kleine-König wrote:
> > 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 :-(
> 
> You sent it after my tree closed for 3.11, 3.10 is a stretch :)
You close your tree for fixes?

Uwe

-- 
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

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

[Greg Kroah-Hartman]

On Thu, Jul 04, 2013 at 06:02:50PM +0200, Uwe Kleine-König wrote:
> Hi Greg,
> 
> On Thu, Jul 04, 2013 at 08:58:36AM -0700, Greg Kroah-Hartman wrote:
> > On Thu, Jul 04, 2013 at 11:27:16AM +0200, Uwe Kleine-König wrote:
> > > 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 :-(
> > 
> > You sent it after my tree closed for 3.11, 3.10 is a stretch :)
> You close your tree for fixes?

2 days before the kernel is to be released?  Yes, I do.  This will get
into 3.11-rc2 or -rc3, and backported to 3.10 and others, so please just
give it a bit of time.  It's just a driver issue, nothing keeping
machines from booting properly :)

thanks,

greg k-h