Re: [PATCH] serial imx.c: fix CTS trigger level lower to avoid lost chars

Re: [PATCH] serial imx.c: fix CTS trigger level lower to avoid lost chars

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 2701 bytes --]

CCing linux-serial

On Thu, Jan 21, 2010 at 10:26:03PM +0100, Valentin Longchamp wrote:
> The imx CTS trigger level is left at its reset value that is 32
> chars. Since the RX FIFO has 32 entries, when CTS is raised, the
> FIFO already is full. However, some serial port devices first empty
> their TX FIFO before stopping when CTS is raised, resulting in lost
> chars.

? Isn't that a flaw of the other side? Have you spotted other serial drivers
doing the same as your patch?

> 
> This patch sets the trigger level lower so that other chars arrive
> after CTS is raised, there is still room for 16 of them.
> 
> Signed-off-by: Valentin Longchamp <valentin.longchamp@epfl.ch>
> Tested-by: Philippe Rétornaz <philippe.retornaz@epfl.ch>
> ---
>  drivers/serial/imx.c |   10 +++++++++-
>  1 files changed, 9 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/serial/imx.c b/drivers/serial/imx.c
> index 18130f1..f7d2da5 100644
> --- a/drivers/serial/imx.c
> +++ b/drivers/serial/imx.c
> @@ -119,7 +119,8 @@
>  #define  MX2_UCR3_RXDMUXSEL	 (1<<2)  /* RXD Muxed Input Select, on mx2/mx3 */
>  #define  UCR3_INVT  	 (1<<1)  /* Inverted Infrared transmission */
>  #define  UCR3_BPEN  	 (1<<0)  /* Preset registers enable */
> -#define  UCR4_CTSTL_32   (32<<10) /* CTS trigger level (32 chars) */
> +#define  UCR4_CTSTL_SHF  10      /* CTS trigger level shift */
> +#define  UCR4_CTSTL_MASK 0x3F    /* CTS trigger is 6 bits wide */
>  #define  UCR4_INVR  	 (1<<9)  /* Inverted infrared reception */
>  #define  UCR4_ENIRI 	 (1<<8)  /* Serial infrared interrupt enable */
>  #define  UCR4_WKEN  	 (1<<7)  /* Wake interrupt enable */
> @@ -590,6 +591,8 @@ static int imx_setup_ufcr(struct imx_port *sport, unsigned int mode)
>  	return 0;
>  }
>  
> +#define CTSTL 16
> +
>  static int imx_startup(struct uart_port *port)
>  {
>  	struct imx_port *sport = (struct imx_port *)port;
> @@ -606,6 +609,11 @@ static int imx_startup(struct uart_port *port)
>  	if (USE_IRDA(sport))
>  		temp |= UCR4_IRSC;
>  
> +	/* set the trigger level for CTS to 16 by default
> +	 */
> +	temp &= ~(UCR4_CTSTL_MASK << UCR4_CTSTL_SHF);
> +	temp |= CTSTL << UCR4_CTSTL_SHF;
> +
>  	writel(temp & ~UCR4_DREN, sport->port.membase + UCR4);
>  
>  	if (USE_IRDA(sport)) {
> -- 
> 1.6.3.3
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Regards,

   Wolfram

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH] serial imx.c: fix CTS trigger level lower to avoid lost chars

[Russell King - ARM Linux]

On Thu, Jan 21, 2010 at 11:06:53PM +0100, Wolfram Sang wrote:
> CCing linux-serial
> 
> On Thu, Jan 21, 2010 at 10:26:03PM +0100, Valentin Longchamp wrote:
> > The imx CTS trigger level is left at its reset value that is 32
> > chars. Since the RX FIFO has 32 entries, when CTS is raised, the
> > FIFO already is full. However, some serial port devices first empty
> > their TX FIFO before stopping when CTS is raised, resulting in lost
> > chars.
> 
> ? Isn't that a flaw of the other side? Have you spotted other serial drivers
> doing the same as your patch?

Arguably, but it's common behaviour of 16550A's and similar.

Re: [PATCH] serial imx.c: fix CTS trigger level lower to avoid lost chars

[Valentin Longchamp]

Russell King - ARM Linux wrote:
> On Thu, Jan 21, 2010 at 11:06:53PM +0100, Wolfram Sang wrote:
>> CCing linux-serial
>>
>> On Thu, Jan 21, 2010 at 10:26:03PM +0100, Valentin Longchamp wrote:
>>> The imx CTS trigger level is left at its reset value that is 32
>>> chars. Since the RX FIFO has 32 entries, when CTS is raised, the
>>> FIFO already is full. However, some serial port devices first empty
>>> their TX FIFO before stopping when CTS is raised, resulting in lost
>>> chars.
>> ? Isn't that a flaw of the other side? Have you spotted other serial drivers
>> doing the same as your patch?
> 
> Arguably, but it's common behaviour of 16550A's and similar.

First I wanted to fix the fact that the imx serial hardware only raises 
the CTS pin when the oferflowing char begins to be set. I would have 
solved this by setting the CTS trigger value to 31 instead of 32. But 
when I talked with one colleague he told me about 16550A (that is 
present nearly everywhere) that empty their TX FIFO, so I lowered it to 16.

Val

-- 
Valentin Longchamp, PhD Student, EPFL-STI-LSRO1
valentin.longchamp@epfl.ch, Phone: +41216937827
http://people.epfl.ch/valentin.longchamp
MEB3494, Station 9, CH-1015 Lausanne

Re: [PATCH] serial imx.c: fix CTS trigger level lower to avoid lost chars

[Jamie Lokier]

Valentin Longchamp wrote:
> First I wanted to fix the fact that the imx serial hardware only raises 
> the CTS pin when the oferflowing char begins to be set. I would have 
> solved this by setting the CTS trigger value to 31 instead of 32. But 
> when I talked with one colleague he told me about 16550A (that is 
> present nearly everywhere) that empty their TX FIFO, so I lowered it to 16.

That seems quite peculiar because a 16550A only has a 16 byte receive
buffer, doesn't it?  And therefore it would have trouble talking
reliably to another 16550A if it worked like that.  I wonder what I'm
not seeing in this picture.

-- Jamie

Re: [PATCH] serial imx.c: fix CTS trigger level lower to avoid lost chars

[Russell King - ARM Linux]

On Fri, Jan 22, 2010 at 11:07:01AM +0000, Jamie Lokier wrote:
> Valentin Longchamp wrote:
> > First I wanted to fix the fact that the imx serial hardware only raises 
> > the CTS pin when the oferflowing char begins to be set. I would have 
> > solved this by setting the CTS trigger value to 31 instead of 32. But 
> > when I talked with one colleague he told me about 16550A (that is 
> > present nearly everywhere) that empty their TX FIFO, so I lowered it to 16.
> 
> That seems quite peculiar because a 16550A only has a 16 byte receive
> buffer, doesn't it?  And therefore it would have trouble talking
> reliably to another 16550A if it worked like that.  I wonder what I'm
> not seeing in this picture.

What you're assuming is that flow control was there to prevent overruns
on the hardware receiver.  That's not the way it works on these devices;
flow control is entirely managed in software - there is no hardware
assistance.

The flow control implemented for non-FIFO and non-hardware assisted
UARTs is purely to do with preventing the software buffer behind the
UART from overflowing - it can't prevent the device's receiver buffering
from overrunning.  (Non-FIFO devices have the shift register and a
buffer register - complete reception of a second character before the
first is read causes an overrun condition.)

If you have overruns on the receiver, that's an interrupt latency
problem and its an error that's reported to the receiver side only.

Yes, later devices have the ability in hardware to deassert RTS when
the receiver FIFO gets above a certain threshold to /help/ prevent
overruns occuring when there's high interrupt latency - but normal
system operation should still ensure that the FIFO is emptied in a
timely manner.

They also gained the ability to stop the transmitter once CTS is
deasserted, mainly because the transmitter FIFOs in these devices
is soo large (maybe 32 to 128 bytes deep.)

Re: [PATCH] serial imx.c: fix CTS trigger level lower to avoid lost chars

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 1551 bytes --]

> What you're assuming is that flow control was there to prevent overruns
> on the hardware receiver.  That's not the way it works on these devices;
> flow control is entirely managed in software - there is no hardware
> assistance.
> 
> The flow control implemented for non-FIFO and non-hardware assisted
> UARTs is purely to do with preventing the software buffer behind the
> UART from overflowing - it can't prevent the device's receiver buffering
> from overrunning.  (Non-FIFO devices have the shift register and a
> buffer register - complete reception of a second character before the
> first is read causes an overrun condition.)
> 
> If you have overruns on the receiver, that's an interrupt latency
> problem and its an error that's reported to the receiver side only.
> 
> Yes, later devices have the ability in hardware to deassert RTS when
> the receiver FIFO gets above a certain threshold to /help/ prevent
> overruns occuring when there's high interrupt latency - but normal
> system operation should still ensure that the FIFO is emptied in a
> timely manner.
> 
> They also gained the ability to stop the transmitter once CTS is
> deasserted, mainly because the transmitter FIFOs in these devices
> is soo large (maybe 32 to 128 bytes deep.)

Russell, thanks for this explanation. Patch seems principally OK to me, then.

Regards,

   Wolfram

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH] serial imx.c: fix CTS trigger level lower to avoid lost chars

[Jamie Lokier]

Wolfram Sang wrote:
> > What you're assuming is that flow control was there to prevent overruns
> > on the hardware receiver.  That's not the way it works on these devices;
> > flow control is entirely managed in software - there is no hardware
> > assistance.
> > 
> > The flow control implemented for non-FIFO and non-hardware assisted
> > UARTs is purely to do with preventing the software buffer behind the
> > UART from overflowing - it can't prevent the device's receiver buffering
> > from overrunning.  (Non-FIFO devices have the shift register and a
> > buffer register - complete reception of a second character before the
> > first is read causes an overrun condition.)
> > 
> > If you have overruns on the receiver, that's an interrupt latency
> > problem and its an error that's reported to the receiver side only.
> > 
> > Yes, later devices have the ability in hardware to deassert RTS when
> > the receiver FIFO gets above a certain threshold to /help/ prevent
> > overruns occuring when there's high interrupt latency - but normal
> > system operation should still ensure that the FIFO is emptied in a
> > timely manner.
> > 
> > They also gained the ability to stop the transmitter once CTS is
> > deasserted, mainly because the transmitter FIFOs in these devices
> > is soo large (maybe 32 to 128 bytes deep.)
> 
> Russell, thanks for this explanation. Patch seems principally OK to me, then.

I agree.

-- Jamie

Re: [PATCH] serial imx.c: fix CTS trigger level lower to avoid lost chars

[Jamie Lokier]

Russell King - ARM Linux wrote:
> On Fri, Jan 22, 2010 at 11:07:01AM +0000, Jamie Lokier wrote:
> > Valentin Longchamp wrote:
> > > First I wanted to fix the fact that the imx serial hardware only raises 
> > > the CTS pin when the oferflowing char begins to be set. I would have 
> > > solved this by setting the CTS trigger value to 31 instead of 32. But 
> > > when I talked with one colleague he told me about 16550A (that is 
> > > present nearly everywhere) that empty their TX FIFO, so I
> > > lowered it to 16.
> > 
> > That seems quite peculiar because a 16550A only has a 16 byte receive
> > buffer, doesn't it?  And therefore it would have trouble talking
> > reliably to another 16550A if it worked like that.  I wonder what I'm
> > not seeing in this picture.
> 
> What you're assuming is that flow control was there to prevent overruns
> on the hardware receiver.  That's not the way it works on these devices;
> flow control is entirely managed in software - there is no hardware
> assistance.
> 
> The flow control implemented for non-FIFO and non-hardware assisted
> UARTs is purely to do with preventing the software buffer behind the
> UART from overflowing - it can't prevent the device's receiver buffering
> from overrunning.  (Non-FIFO devices have the shift register and a
> buffer register - complete reception of a second character before the
> first is read causes an overrun condition.)

Thanks.  That is an excellent explanation.

> If you have overruns on the receiver, that's an interrupt latency
> problem and its an error that's reported to the receiver side only.

Yeah :-/ On an ARM-related note, I have a 166MHz ARM device here which
cannot receive reliably at 38400 baud.  The serial port isn't an ARM
one, but it's quite straightforward and has a 16 byte FIFO, which
translates to 240 irqs per second.  I believe the problem is an
interrupt latency problem, especially when it's busy doing other
things too like disk and network access, but surprisingly even when
it's not very active with other things.

Any tips on how to make the serial receive irq latency more reliable
on ARM, with a boringly generic serial driver?

> Yes, later devices have the ability in hardware to deassert RTS when
> the receiver FIFO gets above a certain threshold to /help/ prevent
> overruns occuring when there's high interrupt latency - but normal
> system operation should still ensure that the FIFO is emptied in a
> timely manner.
> 
> They also gained the ability to stop the transmitter once CTS is
> deasserted, mainly because the transmitter FIFOs in these devices
> is soo large (maybe 32 to 128 bytes deep.)

If only they were all like that. :-/

-- Jamie

Re: [PATCH] serial imx.c: fix CTS trigger level lower to avoid lost chars

[Russell King - ARM Linux]

On Fri, Jan 22, 2010 at 04:47:13PM +0000, Jamie Lokier wrote:
> Any tips on how to make the serial receive irq latency more reliable
> on ARM, with a boringly generic serial driver?

What you could do is arrange for a backtrace to be printed when
you read an overrun condition from the UART - if interrupts were
disabled by something taking a long time, and then enabled, you
should see the point where they were just enabled in the backtrace.

It could be down to another IRQF_DISABLED interrupt - in which case
you won't see it in the backtrace.

Of course, printing a backtrace will cause subsequent overruns...

Re: [PATCH] serial imx.c: fix CTS trigger level lower to avoid lost chars

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 2630 bytes --]

On Thu, Jan 21, 2010 at 10:26:03PM +0100, Valentin Longchamp wrote:
> The imx CTS trigger level is left at its reset value that is 32
> chars. Since the RX FIFO has 32 entries, when CTS is raised, the
> FIFO already is full. However, some serial port devices first empty
> their TX FIFO before stopping when CTS is raised, resulting in lost
> chars.
> 
> This patch sets the trigger level lower so that other chars arrive
> after CTS is raised, there is still room for 16 of them.
> 
> Signed-off-by: Valentin Longchamp <valentin.longchamp@epfl.ch>
> Tested-by: Philippe Rétornaz <philippe.retornaz@epfl.ch>

If you fix the comment, feel free to add my:

Acked-by: Wolfram Sang <w.sang@pengutronix.de>

> ---
>  drivers/serial/imx.c |   10 +++++++++-
>  1 files changed, 9 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/serial/imx.c b/drivers/serial/imx.c
> index 18130f1..f7d2da5 100644
> --- a/drivers/serial/imx.c
> +++ b/drivers/serial/imx.c
> @@ -119,7 +119,8 @@
>  #define  MX2_UCR3_RXDMUXSEL	 (1<<2)  /* RXD Muxed Input Select, on mx2/mx3 */
>  #define  UCR3_INVT  	 (1<<1)  /* Inverted Infrared transmission */
>  #define  UCR3_BPEN  	 (1<<0)  /* Preset registers enable */
> -#define  UCR4_CTSTL_32   (32<<10) /* CTS trigger level (32 chars) */
> +#define  UCR4_CTSTL_SHF  10      /* CTS trigger level shift */
> +#define  UCR4_CTSTL_MASK 0x3F    /* CTS trigger is 6 bits wide */
>  #define  UCR4_INVR  	 (1<<9)  /* Inverted infrared reception */
>  #define  UCR4_ENIRI 	 (1<<8)  /* Serial infrared interrupt enable */
>  #define  UCR4_WKEN  	 (1<<7)  /* Wake interrupt enable */
> @@ -590,6 +591,8 @@ static int imx_setup_ufcr(struct imx_port *sport, unsigned int mode)
>  	return 0;
>  }
>  
> +#define CTSTL 16
> +

Maybe a comment that it is set to half the buffer size? Dunno...

>  static int imx_startup(struct uart_port *port)
>  {
>  	struct imx_port *sport = (struct imx_port *)port;
> @@ -606,6 +609,11 @@ static int imx_startup(struct uart_port *port)
>  	if (USE_IRDA(sport))
>  		temp |= UCR4_IRSC;
>  
> +	/* set the trigger level for CTS to 16 by default
> +	 */

Make this comment a one-liner. Also remove the "to 16 by default" part as this
value is #defined and, who knows, it may change.

> +	temp &= ~(UCR4_CTSTL_MASK << UCR4_CTSTL_SHF);
> +	temp |= CTSTL << UCR4_CTSTL_SHF;
> +
>  	writel(temp & ~UCR4_DREN, sport->port.membase + UCR4);
>  
>  	if (USE_IRDA(sport)) {

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

[PATCH] serial imx.c: fix CTS trigger level lower to avoid lost chars

[Valentin Longchamp]

The imx CTS trigger level is left at its reset value that is 32
chars. Since the RX FIFO has 32 entries, when CTS is raised, the
FIFO already is full. However, some serial port devices first empty
their TX FIFO before stopping when CTS is raised, resulting in lost
chars.

This patch sets the trigger level lower so that other chars arrive
after CTS is raised, there is still room for 16 of them.

Signed-off-by: Valentin Longchamp <valentin.longchamp@epfl.ch>
Tested-by: Philippe Rétornaz <philippe.retornaz@epfl.ch>
Acked-by: Wolfram Sang <w.sang@pengutronix.de>
---
 drivers/serial/imx.c |   10 +++++++++-
 1 files changed, 9 insertions(+), 1 deletions(-)

diff --git a/drivers/serial/imx.c b/drivers/serial/imx.c
index 60d665a..071c614 100644
--- a/drivers/serial/imx.c
+++ b/drivers/serial/imx.c
@@ -119,7 +119,8 @@
 #define  MX2_UCR3_RXDMUXSEL	 (1<<2)  /* RXD Muxed Input Select, on mx2/mx3 */
 #define  UCR3_INVT  	 (1<<1)  /* Inverted Infrared transmission */
 #define  UCR3_BPEN  	 (1<<0)  /* Preset registers enable */
-#define  UCR4_CTSTL_32   (32<<10) /* CTS trigger level (32 chars) */
+#define  UCR4_CTSTL_SHF  10      /* CTS trigger level shift */
+#define  UCR4_CTSTL_MASK 0x3F    /* CTS trigger is 6 bits wide */
 #define  UCR4_INVR  	 (1<<9)  /* Inverted infrared reception */
 #define  UCR4_ENIRI 	 (1<<8)  /* Serial infrared interrupt enable */
 #define  UCR4_WKEN  	 (1<<7)  /* Wake interrupt enable */
@@ -590,6 +591,9 @@ static int imx_setup_ufcr(struct imx_port *sport, unsigned int mode)
 	return 0;
 }
 
+/* half the RX buffer size */
+#define CTSTL 16
+
 static int imx_startup(struct uart_port *port)
 {
 	struct imx_port *sport = (struct imx_port *)port;
@@ -606,6 +610,10 @@ static int imx_startup(struct uart_port *port)
 	if (USE_IRDA(sport))
 		temp |= UCR4_IRSC;
 
+	/* set the trigger level for CTS */
+	temp &= ~(UCR4_CTSTL_MASK << UCR4_CTSTL_SHF);
+	temp |= CTSTL << UCR4_CTSTL_SHF;
+
 	writel(temp & ~UCR4_DREN, sport->port.membase + UCR4);
 
 	if (USE_IRDA(sport)) {
-- 
1.6.3.3

--
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 imx.c: fix CTS trigger level lower to avoid lost chars

[Valentin Longchamp]

Hello,

I had sent this patch more than 3 months ago and noone has pushed it 
into mainline. It is very important for our use and I think it may 
affect other people later.

Is there a linux-serial tree where it could be added to be pulled from 
for mainline then ? What is the usual path for serial patches ?

Val

On 01/24/2010 11:48 AM, Valentin Longchamp wrote:
> The imx CTS trigger level is left at its reset value that is 32
> chars. Since the RX FIFO has 32 entries, when CTS is raised, the
> FIFO already is full. However, some serial port devices first empty
> their TX FIFO before stopping when CTS is raised, resulting in lost
> chars.
>
> This patch sets the trigger level lower so that other chars arrive
> after CTS is raised, there is still room for 16 of them.
>
> Signed-off-by: Valentin Longchamp<valentin.longchamp@epfl.ch>
> Tested-by: Philippe Rétornaz<philippe.retornaz@epfl.ch>
> Acked-by: Wolfram Sang<w.sang@pengutronix.de>
> ---
>   drivers/serial/imx.c |   10 +++++++++-
>   1 files changed, 9 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/serial/imx.c b/drivers/serial/imx.c
> index 60d665a..071c614 100644
> --- a/drivers/serial/imx.c
> +++ b/drivers/serial/imx.c
> @@ -119,7 +119,8 @@
>   #define  MX2_UCR3_RXDMUXSEL	 (1<<2)  /* RXD Muxed Input Select, on mx2/mx3 */
>   #define  UCR3_INVT  	 (1<<1)  /* Inverted Infrared transmission */
>   #define  UCR3_BPEN  	 (1<<0)  /* Preset registers enable */
> -#define  UCR4_CTSTL_32   (32<<10) /* CTS trigger level (32 chars) */
> +#define  UCR4_CTSTL_SHF  10      /* CTS trigger level shift */
> +#define  UCR4_CTSTL_MASK 0x3F    /* CTS trigger is 6 bits wide */
>   #define  UCR4_INVR  	 (1<<9)  /* Inverted infrared reception */
>   #define  UCR4_ENIRI 	 (1<<8)  /* Serial infrared interrupt enable */
>   #define  UCR4_WKEN  	 (1<<7)  /* Wake interrupt enable */
> @@ -590,6 +591,9 @@ static int imx_setup_ufcr(struct imx_port *sport, unsigned int mode)
>   	return 0;
>   }
>
> +/* half the RX buffer size */
> +#define CTSTL 16
> +
>   static int imx_startup(struct uart_port *port)
>   {
>   	struct imx_port *sport = (struct imx_port *)port;
> @@ -606,6 +610,10 @@ static int imx_startup(struct uart_port *port)
>   	if (USE_IRDA(sport))
>   		temp |= UCR4_IRSC;
>
> +	/* set the trigger level for CTS */
> +	temp&= ~(UCR4_CTSTL_MASK<<  UCR4_CTSTL_SHF);
> +	temp |= CTSTL<<  UCR4_CTSTL_SHF;
> +
>   	writel(temp&  ~UCR4_DREN, sport->port.membase + UCR4);
>
>   	if (USE_IRDA(sport)) {


-- 
Valentin Longchamp, PhD Student, EPFL-STI-LSRO1
valentin.longchamp@epfl.ch, Phone: +41216937827
http://people.epfl.ch/valentin.longchamp
MEB3494, Station 9, CH-1015 Lausanne
--
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 imx.c: fix CTS trigger level lower to avoid lost chars

[Greg KH]

On Wed, May 05, 2010 at 11:47:07AM +0200, Valentin Longchamp wrote:
> Hello,
> 
> I had sent this patch more than 3 months ago and noone has pushed it
> into mainline. It is very important for our use and I think it may
> affect other people later.
> 
> Is there a linux-serial tree where it could be added to be pulled
> from for mainline then ? What is the usual path for serial patches ?

I've been taking serial patches lately as-needed, so feel free to send
them to me.  I'll go queue this one up.

thanks,

greg k-h

patch serial-imx.c-fix-cts-trigger-level-lower-to-avoid-lost-chars.patch added to gregkh-2.6 tree

[gregkh]

This is a note to let you know that I've just added the patch titled

    Subject: serial: imx.c: fix CTS trigger level lower to avoid lost chars

to my gregkh-2.6 tree.  Its filename is

    serial-imx.c-fix-cts-trigger-level-lower-to-avoid-lost-chars.patch

This tree can be found at 
    http://www.kernel.org/pub/linux/kernel/people/gregkh/gregkh-2.6/patches/


From valentin.longchamp@epfl.ch  Wed May  5 11:15:04 2010
From: Valentin Longchamp <valentin.longchamp@epfl.ch>
Date: Wed, 05 May 2010 11:47:07 +0200
Subject: serial: imx.c: fix CTS trigger level lower to avoid lost chars
To: Alan Cox <alan@linux.intel.com>
Cc: "s.hauer@pengutronix.de" <s.hauer@pengutronix.de>, "w.sang@pengutronix.de" <w.sang@pengutronix.de>, "linux-serial@vger.kernel.org" <linux-serial@vger.kernel.org>, Greg Kroah-Hartman <gregkh@suse.de>
Message-ID: <4BE13E9B.6050406@epfl.ch>


The imx CTS trigger level is left at its reset value that is 32
chars. Since the RX FIFO has 32 entries, when CTS is raised, the
FIFO already is full. However, some serial port devices first empty
their TX FIFO before stopping when CTS is raised, resulting in lost
chars.

This patch sets the trigger level lower so that other chars arrive
after CTS is raised, there is still room for 16 of them.

Signed-off-by: Valentin Longchamp<valentin.longchamp@epfl.ch>
Tested-by: Philippe Rétornaz<philippe.retornaz@epfl.ch>
Acked-by: Wolfram Sang<w.sang@pengutronix.de>
Cc: stable <stable@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>

---
 drivers/serial/imx.c |   10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

--- a/drivers/serial/imx.c
+++ b/drivers/serial/imx.c
@@ -120,7 +120,8 @@
 #define  MX2_UCR3_RXDMUXSEL	 (1<<2)  /* RXD Muxed Input Select, on mx2/mx3 */
 #define  UCR3_INVT  	 (1<<1)  /* Inverted Infrared transmission */
 #define  UCR3_BPEN  	 (1<<0)  /* Preset registers enable */
-#define  UCR4_CTSTL_32   (32<<10) /* CTS trigger level (32 chars) */
+#define  UCR4_CTSTL_SHF  10      /* CTS trigger level shift */
+#define  UCR4_CTSTL_MASK 0x3F    /* CTS trigger is 6 bits wide */
 #define  UCR4_INVR  	 (1<<9)  /* Inverted infrared reception */
 #define  UCR4_ENIRI 	 (1<<8)  /* Serial infrared interrupt enable */
 #define  UCR4_WKEN  	 (1<<7)  /* Wake interrupt enable */
@@ -591,6 +592,9 @@ static int imx_setup_ufcr(struct imx_por
 	return 0;
 }
 
+/* half the RX buffer size */
+#define CTSTL 16
+
 static int imx_startup(struct uart_port *port)
 {
 	struct imx_port *sport = (struct imx_port *)port;
@@ -607,6 +611,10 @@ static int imx_startup(struct uart_port
 	if (USE_IRDA(sport))
 		temp |= UCR4_IRSC;
 
+	/* set the trigger level for CTS */
+	temp &= ~(UCR4_CTSTL_MASK<<  UCR4_CTSTL_SHF);
+	temp |= CTSTL<<  UCR4_CTSTL_SHF;
+
 	writel(temp & ~UCR4_DREN, sport->port.membase + UCR4);
 
 	if (USE_IRDA(sport)) {

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