ARM i.MX serial: fix tx buffer overflows

ARM i.MX serial: fix tx buffer overflows

[Sascha Hauer]

This patch fixes occasional tx buffer overflows in the i.MX serial
driver which came from the fact that space in the buffer was checked
after sending the first byte. Also, fifosize is 32 bytes, not 8.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de

---
 drivers/serial/imx.c |   11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

Index: linux-2.6.20-rc3/drivers/serial/imx.c
===================================================================
--- linux-2.6.20-rc3.orig/drivers/serial/imx.c
+++ linux-2.6.20-rc3/drivers/serial/imx.c
@@ -154,7 +154,7 @@ static inline void imx_transmit_buffer(s
 {
 	struct circ_buf *xmit = &sport->port.info->xmit;
 
-	do {
+	while (!(UTS((u32)sport->port.membase) & UTS_TXFULL)) {
 		/* send xmit->buf[xmit->tail]
 		 * out the port here */
 		URTX0((u32)sport->port.membase) = xmit->buf[xmit->tail];
@@ -163,7 +163,7 @@ static inline void imx_transmit_buffer(s
 		sport->port.icount.tx++;
 		if (uart_circ_empty(xmit))
 			break;
-	} while (!(UTS((u32)sport->port.membase) & UTS_TXFULL));
+	}
 
 	if (uart_circ_empty(xmit))
 		imx_stop_tx(&sport->port);
@@ -178,8 +178,7 @@ static void imx_start_tx(struct uart_por
 
 	UCR1((u32)sport->port.membase) |= UCR1_TXMPTYEN;
 
-	if(UTS((u32)sport->port.membase) & UTS_TXEMPTY)
-		imx_transmit_buffer(sport);
+	imx_transmit_buffer(sport);
 }
 
 static irqreturn_t imx_rtsint(int irq, void *dev_id)
@@ -678,7 +677,7 @@ static struct imx_port imx_ports[] = {
 		.mapbase	= IMX_UART1_BASE, /* FIXME */
 		.irq		= UART1_MINT_RX,
 		.uartclk	= 16000000,
-		.fifosize	= 8,
+		.fifosize	= 32,
 		.flags		= UPF_BOOT_AUTOCONF,
 		.ops		= &imx_pops,
 		.line		= 0,
@@ -694,7 +693,7 @@ static struct imx_port imx_ports[] = {
 		.mapbase	= IMX_UART2_BASE, /* FIXME */
 		.irq		= UART2_MINT_RX,
 		.uartclk	= 16000000,
-		.fifosize	= 8,
+		.fifosize	= 32,
 		.flags		= UPF_BOOT_AUTOCONF,
 		.ops		= &imx_pops,
 		.line		= 1,

Re: ARM i.MX serial: fix tx buffer overflows

[Pavel Pisa]

On Friday 05 January 2007 16:51, Sascha Hauer wrote:
> This patch fixes occasional tx buffer overflows in the i.MX serial
> driver which came from the fact that space in the buffer was checked
> after sending the first byte. Also, fifosize is 32 bytes, not 8.
>
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de

Acked-by: Pavel Pisa <pisa@cmp.felk.cvut.cz>

Hello Sascha,

I have tested your change on 2.6.19-rt14 kernel
which I have on hand there. It is only very short
test, but I have not noticed any problems.

In the fact, I think, that it is possible, that
I have noticed some mentioned problem during
my CPU-Freq testing on RT kernels before.
I have noticed some problems sometimes with BusyBox
shell history editing during frequency throttling.
May it be, that RT+changed frequency invoked the problem.

I have some other small fix for i.MX uart there.
Can you add it into patch, or should I send it
as separate one-liner?

If RTS interrupt is caused by RTS senzing logic inside
i.MX UART module the IRQ type cannot be set.

It applies only for interrupts going through GPIO layer.
The problem has been noticed by Konstantin Kletschke
some time ago.

  No IRQF_TRIGGER set_type function for IRQ 26 (MPU)

I would not change type to fixed 0, because it could
be possible to use different GPIO MX1 pin for RTS
in the theory. On the other hand it is only for documentation
purposes now, because RTS read code would have to be adjusted
in such case.

Health and success wishes
in year 2007 from

                     Pavel Pisa

---

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

Index: linux-2.6.19-rt/drivers/serial/imx.c
===================================================================
--- linux-2.6.19-rt.orig/drivers/serial/imx.c
+++ linux-2.6.19-rt/drivers/serial/imx.c
@@ -403,7 +403,8 @@ static int imx_startup(struct uart_port 
 	if (retval) goto error_out2;
 
 	retval = request_irq(sport->rtsirq, imx_rtsint,
-			     IRQF_TRIGGER_FALLING | IRQF_TRIGGER_RISING,
+			     (sport->rtsirq < IMX_IRQS) ? 0 :
+			       IRQF_TRIGGER_FALLING | IRQF_TRIGGER_RISING,
 			     DRIVER_NAME, sport);
 	if (retval) goto error_out3;
 

Re: ARM i.MX serial: fix tx buffer overflows

[Konstantin Kletschke]

Am 2007-01-05 19:01 +0100 schrieb Pavel Pisa:

> It applies only for interrupts going through GPIO layer.
> The problem has been noticed by Konstantin Kletschke
> some time ago.
> 
>   No IRQF_TRIGGER set_type function for IRQ 26 (MPU)

Yes. I reported this also.

>  drivers/serial/imx.c |    3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> Index: linux-2.6.19-rt/drivers/serial/imx.c
> ===================================================================
> +++ linux-2.6.19-rt/drivers/serial/imx.c
> @@ -403,7 +403,8 @@ static int imx_startup(struct uart_port 
>  	if (retval) goto error_out2;
>  
>  	retval = request_irq(sport->rtsirq, imx_rtsint,
> -			     IRQF_TRIGGER_FALLING | IRQF_TRIGGER_RISING,
> +			     (sport->rtsirq < IMX_IRQS) ? 0 :
> +			       IRQF_TRIGGER_FALLING | IRQF_TRIGGER_RISING,
>  			     DRIVER_NAME, sport);
>  	if (retval) goto error_out3;


Okay, this indeed fixes my

No IRQF_TRIGGER set_type function for IRQ 26 (MPU)

here!

Regards, Konsti

-- 
GPG KeyID EF62FCEF
Fingerprint: 13C9 B16B 9844 EC15 CC2E  A080 1E69 3FDA EF62 FCEF

Re: ARM i.MX serial: fix tx buffer overflows

[Russell King]

On Mon, Jan 29, 2007 at 03:47:59PM +0100, Konstantin Kletschke wrote:
> Am 2007-01-05 19:01 +0100 schrieb Pavel Pisa:
> 
> > It applies only for interrupts going through GPIO layer.
> > The problem has been noticed by Konstantin Kletschke
> > some time ago.
> > 
> >   No IRQF_TRIGGER set_type function for IRQ 26 (MPU)
> 
> Yes. I reported this also.
> 
> >  drivers/serial/imx.c |    3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > Index: linux-2.6.19-rt/drivers/serial/imx.c
> > ===================================================================
> > +++ linux-2.6.19-rt/drivers/serial/imx.c
> > @@ -403,7 +403,8 @@ static int imx_startup(struct uart_port 
> >  	if (retval) goto error_out2;
> >  
> >  	retval = request_irq(sport->rtsirq, imx_rtsint,
> > -			     IRQF_TRIGGER_FALLING | IRQF_TRIGGER_RISING,
> > +			     (sport->rtsirq < IMX_IRQS) ? 0 :
> > +			       IRQF_TRIGGER_FALLING | IRQF_TRIGGER_RISING,
> >  			     DRIVER_NAME, sport);
> >  	if (retval) goto error_out3;
> 
> 
> Okay, this indeed fixes my
> 
> No IRQF_TRIGGER set_type function for IRQ 26 (MPU)

Is it really worth adding additional code to shut up this (imho) silly
warning?  It's just adding needless complexity to drivers.

What happens if a driver is used on multiple platforms, some of which
support trigger setting and others which don't?  Are we going to end
up with a large #ifdef in every driver?

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:

Re: ARM i.MX serial: fix tx buffer overflows

[Konstantin Kletschke]

Am 2007-01-29 15:37 +0000 schrieb Russell King:

> Is it really worth adding additional code to shut up this (imho) silly
> warning?  It's just adding needless complexity to drivers.

As I pointed out in 
http://lists.arm.linux.org.uk/pipermail/linux-arm-kernel/2006-November/037192.html
the console gets flooded with this "warning" to unusable state sometimes.

> What happens if a driver is used on multiple platforms, some of which
> support trigger setting and others which don't?  Are we going to end
> up with a large #ifdef in every driver?

I don't know exactly. But in addition to the fact, that this warning
floods my console to unusable state I am used to sell software without
warnings. If there are warnings my boss some things are broken.

Konsti

-- 
GPG KeyID EF62FCEF
Fingerprint: 13C9 B16B 9844 EC15 CC2E  A080 1E69 3FDA EF62 FCEF