All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] 8250.c: less than 2400 baud fix.
@ 2012-04-26  6:59 Christian Melki
  2012-04-30  2:07 ` gregkh
  0 siblings, 1 reply; 3+ messages in thread
From: Christian Melki @ 2012-04-26  6:59 UTC (permalink / raw)
  To: gregkh; +Cc: linux-serial

From: Christian Melki <christian.melki@ericsson.se>

We noticed that we were loosing data at speed less than 2400 baud.
It turned out our (TI16750 compatible) uart with 64 byte outgoing fifo was truncated to 16 byte (bit 5 sets fifo len) when modifying the fcr reg.
The input code still fills the buffer with 64 bytes if I remember correctly and thus data is lost.
Our fix was to remove whiping of the fcr content and just add the TRIGGER_1 which we want for latency.
I can't see why this would not work on less than 2400 always, for all uarts...
Otherwise one would have to make sure the filling of the fifo re-checks the current state of available fifo size (urrk).

Signed-off-by: Christian Melki <christian.melki@ericsson.se>
---

diff -urpN linux-3.3.2.orig//drivers/tty/serial/8250/8250.c linux-3.3.2/drivers/tty/serial/8250/8250.c
--- linux-3.3.2.orig//drivers/tty/serial/8250/8250.c	2012-04-25 10:31:29.000000000 +0200
+++ linux-3.3.2/drivers/tty/serial/8250/8250.c	2012-04-26 08:46:29.000000000 +0200
@@ -2299,10 +2299,11 @@ serial8250_do_set_termios(struct uart_po
 		quot++;
 
 	if (up->capabilities & UART_CAP_FIFO && up->port.fifosize > 1) {
-		if (baud < 2400)
-			fcr = UART_FCR_ENABLE_FIFO | UART_FCR_TRIGGER_1;
-		else
-			fcr = uart_config[up->port.type].fcr;
+		fcr = uart_config[up->port.type].fcr;
+		if (baud < 2400) {
+			fcr &= ~UART_FCR_TRIGGER_MASK;
+			fcr |= UART_FCR_TRIGGER_1;
+		}
 	}
 
 	/*



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

* Re: [PATCH] 8250.c: less than 2400 baud fix.
  2012-04-26  6:59 [PATCH] 8250.c: less than 2400 baud fix Christian Melki
@ 2012-04-30  2:07 ` gregkh
  2012-04-30  9:21   ` Christian Melki
  0 siblings, 1 reply; 3+ messages in thread
From: gregkh @ 2012-04-30  2:07 UTC (permalink / raw)
  To: Christian Melki; +Cc: linux-serial

On Thu, Apr 26, 2012 at 08:59:50AM +0200, Christian Melki wrote:
> From: Christian Melki <christian.melki@ericsson.se>
> 
> We noticed that we were loosing data at speed less than 2400 baud.
> It turned out our (TI16750 compatible) uart with 64 byte outgoing fifo
> was truncated to 16 byte (bit 5 sets fifo len) when modifying the fcr
> reg.
> The input code still fills the buffer with 64 bytes if I remember
> correctly and thus data is lost.
> Our fix was to remove whiping of the fcr content and just add the
> TRIGGER_1 which we want for latency.
> I can't see why this would not work on less than 2400 always, for all
> uarts ...
> Otherwise one would have to make sure the filling of the fifo re-checks
> the current state of available fifo size (urrk).
> 
> Signed-off-by: Christian Melki <christian.melki@ericsson.se>
> ---
> 
> diff -urpN linux-3.3.2.orig//drivers/tty/serial/8250/8250.c linux-3.3.2/drivers/tty/serial/8250/8250.c
> --- linux-3.3.2.orig//drivers/tty/serial/8250/8250.c	2012-04-25 10:31:29.000000000 +0200
> +++ linux-3.3.2/drivers/tty/serial/8250/8250.c	2012-04-26 08:46:29.000000000 +0200
> @@ -2299,10 +2299,11 @@ serial8250_do_set_termios(struct uart_po
>  		quot++;
>  
>  	if (up->capabilities & UART_CAP_FIFO && up->port.fifosize > 1) {
> -		if (baud < 2400)
> -			fcr = UART_FCR_ENABLE_FIFO | UART_FCR_TRIGGER_1;
> -		else
> -			fcr = uart_config[up->port.type].fcr;
> +		fcr = uart_config[up->port.type].fcr;
> +		if (baud < 2400) {
> +			fcr &= ~UART_FCR_TRIGGER_MASK;
> +			fcr |= UART_FCR_TRIGGER_1;
> +		}
>  	}

This patch doesn't apply at all to my tree, and I can't see why with a
quick glance :(

Can you redo this against the linux-next tree and resend it please?

thanks,

greg k-h

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

* RE: [PATCH] 8250.c: less than 2400 baud fix.
  2012-04-30  2:07 ` gregkh
@ 2012-04-30  9:21   ` Christian Melki
  0 siblings, 0 replies; 3+ messages in thread
From: Christian Melki @ 2012-04-30  9:21 UTC (permalink / raw)
  To: gregkh; +Cc: linux-serial

Sorry about that.

Diff should have handled that simple line offset. Now it applies cleanly to my linux-next atleast.

Signed-off-by: Christian Melki <christian.melki@ericsson.se>
---

diff -urpN linux-next.orig/drivers/tty/serial/8250/8250.c linux-next/drivers/tty/serial/8250/8250.c
--- linux-next.orig/drivers/tty/serial/8250/8250.c	2012-04-30 10:58:13.000000000 +0200
+++ linux-next/drivers/tty/serial/8250/8250.c	2012-04-30 11:03:12.000000000 +0200
@@ -2259,10 +2259,11 @@ serial8250_do_set_termios(struct uart_po
 		quot++;
 
 	if (up->capabilities & UART_CAP_FIFO && port->fifosize > 1) {
-		if (baud < 2400)
-			fcr = UART_FCR_ENABLE_FIFO | UART_FCR_TRIGGER_1;
-		else
-			fcr = uart_config[port->type].fcr;
+		fcr = uart_config[port->type].fcr;
+		if (baud < 2400) {
+			fcr &= ~UART_FCR_TRIGGER_MASK;
+			fcr |= UART_FCR_TRIGGER_1;
+		}
 	}
 
 	/*

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

end of thread, other threads:[~2012-04-30  9:21 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-04-26  6:59 [PATCH] 8250.c: less than 2400 baud fix Christian Melki
2012-04-30  2:07 ` gregkh
2012-04-30  9:21   ` Christian Melki

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.