[PATCH] serial: fix au1xxx UART0 irq setup

[PATCH] serial: fix au1xxx UART0 irq setup

[Jan Nikitenko]

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

UART0 on Alchemy mips platforms (au1xxx) does not use real uart's hw
irq, causing 'ttyS0: 1 input overrun(s)' kernel message with data loss,
when more characters than uart's fifo size were to be received by the uart.

This problem can be experienced for example when uart0 is used as a
serial console on au1550 and more than 16 characters are pasted from
clipboard to the console.

The is_real_interrupt(irq) macro is defined in drivers/serial/8250.c as
a check, if the irq number is other than zero.
Because UART0 on au1xxx platforms uses irq number 0, the
is_real_interrupt() check fails and serial8250_backup_timeout() is used
instead of uart's hw irq.

The patch redefines the is_real_interrupt(irq) macro, as suggested in
the comment above the macro definition in 8250.c, in the
asm-mips/serial.h to be always true for CONFIG_SERIAL_8250_AU1X00.
This allows the irq number 0 to be used as hw irq for the alchemy uart0
and fixes the overrun problem.

Signed-off-by: Jan Nikitenko <jan.nikitenko@gmail.com>

---

 include/asm-mips/serial.h |    5 +++++
 1 file changed, 5 insertions(+)

[-- Attachment #2: serial-fix-au1xxx-uart0-irq-setup.patch --]
[-- Type: text/plain, Size: 355 bytes --]

diff --git a/include/asm-mips/serial.h b/include/asm-mips/serial.h
index c07ebd8..526bd2e 100644
--- a/include/asm-mips/serial.h
+++ b/include/asm-mips/serial.h
@@ -19,4 +19,9 @@
  */
 #define BASE_BAUD (1843200 / 16)
 
+#ifdef CONFIG_SERIAL_8250_AU1X00
+#undef is_real_interrupt
+#define is_real_interrupt(irq)  (1)
+#endif
+
 #endif /* _ASM_SERIAL_H */

Re: [PATCH] serial: fix au1xxx UART0 irq setup

[Ralf Baechle]

On Thu, Oct 25, 2007 at 03:58:54PM +0200, Jan Nikitenko wrote:

> UART0 on Alchemy mips platforms (au1xxx) does not use real uart's hw
> irq, causing 'ttyS0: 1 input overrun(s)' kernel message with data loss,
> when more characters than uart's fifo size were to be received by the uart.
> 
> This problem can be experienced for example when uart0 is used as a
> serial console on au1550 and more than 16 characters are pasted from
> clipboard to the console.
> 
> The is_real_interrupt(irq) macro is defined in drivers/serial/8250.c as
> a check, if the irq number is other than zero.
> Because UART0 on au1xxx platforms uses irq number 0, the
> is_real_interrupt() check fails and serial8250_backup_timeout() is used
> instead of uart's hw irq.
> 
> The patch redefines the is_real_interrupt(irq) macro, as suggested in
> the comment above the macro definition in 8250.c, in the
> asm-mips/serial.h to be always true for CONFIG_SERIAL_8250_AU1X00.
> This allows the irq number 0 to be used as hw irq for the alchemy uart0
> and fixes the overrun problem.
> 
> Signed-off-by: Jan Nikitenko <jan.nikitenko@gmail.com>

Fairly unelegent imho but anyway, for 2.6.24 I've added support for
tickless to MIPS which in turn required a bit of a cleanup on the Alchemy
code so I renumbered the Alchemy interrupt numbers, so what used to be
IRQ 0 is now IRQ 8 which means your patch is no longer needed for master.

That said, irq 0 is imho totally valid (take the good old PIT timer
interrupt of the PC as the classic example) and treating it as an invalid 
interrupt number is broken.

It's however equally pretty crude hack to undefine a symbol in a file other
than the one defining it ...

  Ralf

Re: [PATCH] serial: fix au1xxx UART0 irq setup

[Jan Nikitenko]

Ralf Baechle wrote:
> On Thu, Oct 25, 2007 at 03:58:54PM +0200, Jan Nikitenko wrote:
> 
>> UART0 on Alchemy mips platforms (au1xxx) does not use real uart's hw
>> irq, causing 'ttyS0: 1 input overrun(s)' kernel message with data loss,
>> when more characters than uart's fifo size were to be received by the uart.
>>
>> This problem can be experienced for example when uart0 is used as a
>> serial console on au1550 and more than 16 characters are pasted from
>> clipboard to the console.
>>
>> The is_real_interrupt(irq) macro is defined in drivers/serial/8250.c as
>> a check, if the irq number is other than zero.
>> Because UART0 on au1xxx platforms uses irq number 0, the
>> is_real_interrupt() check fails and serial8250_backup_timeout() is used
>> instead of uart's hw irq.
>>
>> The patch redefines the is_real_interrupt(irq) macro, as suggested in
>> the comment above the macro definition in 8250.c, in the
>> asm-mips/serial.h to be always true for CONFIG_SERIAL_8250_AU1X00.
>> This allows the irq number 0 to be used as hw irq for the alchemy uart0
>> and fixes the overrun problem.
>>
>> Signed-off-by: Jan Nikitenko <jan.nikitenko@gmail.com>
> 
> Fairly unelegent imho but anyway, for 2.6.24 I've added support for
> tickless to MIPS which in turn required a bit of a cleanup on the Alchemy
> code so I renumbered the Alchemy interrupt numbers, so what used to be
> IRQ 0 is now IRQ 8 which means your patch is no longer needed for master.

That's good to know.

> 
> That said, irq 0 is imho totally valid (take the good old PIT timer
> interrupt of the PC as the classic example) and treating it as an invalid 
> interrupt number is broken.
> 
> It's however equally pretty crude hack to undefine a symbol in a file other
> than the one defining it ...

I did exactly as the comment in 8250.c suggested (that arch include
could redefine that macro) - I did not like it either...

Jan

Re: [PATCH] serial: fix au1xxx UART0 irq setup

[Maciej W. Rozycki]

On Thu, 25 Oct 2007, Ralf Baechle wrote:

> That said, irq 0 is imho totally valid (take the good old PIT timer
> interrupt of the PC as the classic example) and treating it as an invalid 
> interrupt number is broken.

 I would rather -1 stood for the invalid IRQ number -- unlike with 0 
chances are nobody will need 4G of interrupt lines or vectors (as 
applicable) in a single system.  We sort of escape the problem with the 
MIPS processors because the IP0 bit of the Cause register is a software 
interrupt that is not used by devices, but still some platforms bypass the 
built-in interrupt "controller" as only a single source is used in the 
Cause register and want to start the numbering of lines in the external 
controller from 0.

  Maciej

Re: [PATCH] serial: fix au1xxx UART0 irq setup

[Ralf Baechle]

On Thu, Oct 25, 2007 at 03:57:01PM +0100, Maciej W. Rozycki wrote:

> > That said, irq 0 is imho totally valid (take the good old PIT timer
> > interrupt of the PC as the classic example) and treating it as an invalid 
> > interrupt number is broken.
> 
>  I would rather -1 stood for the invalid IRQ number -- unlike with 0 
> chances are nobody will need 4G of interrupt lines or vectors (as 
> applicable) in a single system.  We sort of escape the problem with the 
> MIPS processors because the IP0 bit of the Cause register is a software 
> interrupt that is not used by devices, but still some platforms bypass the 
> built-in interrupt "controller" as only a single source is used in the 
> Cause register and want to start the numbering of lines in the external 
> controller from 0.

Time to convert those platforms to irq_cpu.c as well.

Anyway, I recall there was an argument against using -1 as the invalid
irq number - but since -1 is not a valid index into the irq_desc array
for example I would consider such use broken.  But does anybody recall
the details?

  Ralf

Re: [PATCH] serial: fix au1xxx UART0 irq setup

[Sergei Shtylyov]

Jan Nikitenko wrote:

> UART0 on Alchemy mips platforms (au1xxx) does not use real uart's hw
> irq, causing 'ttyS0: 1 input overrun(s)' kernel message with data loss,
> when more characters than uart's fifo size were to be received by the uart.

> This problem can be experienced for example when uart0 is used as a
> serial console on au1550 and more than 16 characters are pasted from
> clipboard to the console.
> 
> The is_real_interrupt(irq) macro is defined in drivers/serial/8250.c as
> a check, if the irq number is other than zero.
> Because UART0 on au1xxx platforms uses irq number 0, the
> is_real_interrupt() check fails and serial8250_backup_timeout() is used
> instead of uart's hw irq.
> 
> The patch redefines the is_real_interrupt(irq) macro, as suggested in
> the comment above the macro definition in 8250.c, in the
> asm-mips/serial.h to be always true for CONFIG_SERIAL_8250_AU1X00.
> This allows the irq number 0 to be used as hw irq for the alchemy uart0
> and fixes the overrun problem.
> 
> Signed-off-by: Jan Nikitenko <jan.nikitenko@gmail.com>
> 
> ---
> 
>  include/asm-mips/serial.h |    5 +++++
>  1 file changed, 5 insertions(+)
> 
> 
> ------------------------------------------------------------------------
> 
> diff --git a/include/asm-mips/serial.h b/include/asm-mips/serial.h
> index c07ebd8..526bd2e 100644
> --- a/include/asm-mips/serial.h
> +++ b/include/asm-mips/serial.h
> @@ -19,4 +19,9 @@
>   */
>  #define BASE_BAUD (1843200 / 16)
>  
> +#ifdef CONFIG_SERIAL_8250_AU1X00
> +#undef is_real_interrupt
> +#define is_real_interrupt(irq)  (1)
> +#endif
> +
>  #endif /* _ASM_SERIAL_H */

     I wonder why such patch hasn't still been checked, despite being 
suggested several times...

WBR, Sergei

Re: [PATCH] serial: fix au1xxx UART0 irq setup

[Ralf Baechle]

On Thu, Oct 25, 2007 at 03:58:54PM +0200, Jan Nikitenko wrote:

> UART0 on Alchemy mips platforms (au1xxx) does not use real uart's hw
> irq, causing 'ttyS0: 1 input overrun(s)' kernel message with data loss,
> when more characters than uart's fifo size were to be received by the uart.
> 
> This problem can be experienced for example when uart0 is used as a
> serial console on au1550 and more than 16 characters are pasted from
> clipboard to the console.
> 
> The is_real_interrupt(irq) macro is defined in drivers/serial/8250.c as
> a check, if the irq number is other than zero.
> Because UART0 on au1xxx platforms uses irq number 0, the
> is_real_interrupt() check fails and serial8250_backup_timeout() is used
> instead of uart's hw irq.
> 
> The patch redefines the is_real_interrupt(irq) macro, as suggested in
> the comment above the macro definition in 8250.c, in the
> asm-mips/serial.h to be always true for CONFIG_SERIAL_8250_AU1X00.
> This allows the irq number 0 to be used as hw irq for the alchemy uart0
> and fixes the overrun problem.
> 
> Signed-off-by: Jan Nikitenko <jan.nikitenko@gmail.com>

So applied to all of lmo's -stable branches.

  Ralf