Skip to contentSkip to conversation

serial: fix au1xxx UART0 irq setup

PATCH In review Jan Nikitenko 4 participants 7 messages Oct 25, 2007 – Oct 29, 2007

Subsystems (2)
MIPS TTY layer and serial drivers
lore
Share & tools
lore

Jan Nikitenko

author
#0 lore raw

PATCH 1/1 serial: fix au1xxx UART0 irq setup

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 <[email protected]>

---

 include/asm-mips/serial.h |    5 +++++
 1 file changed, 5 insertions(+)
include/asm-mips/serial.h +5 -0
index c07ebd8..526bd2e 100644
@@ -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 */
#1 lore raw
Quoted 35 lines · Jan Nikitenko

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 <[email protected]>

---

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

------------------------------------------------------------------------

include/asm-mips/serial.h +0 -0
index c07ebd8..526bd2e 100644

include/asm-mips/serial.h line 27

Quoted 9 lines
include/asm-mips/serial.h +5 -0
@@ -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

#2 lore raw
In reply to Jan Nikitenko
Quoted 14 lines · Jan Nikitenko

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.

Quoted 7 lines

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 <[email protected]>

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

author
#3 lore raw
Quoted 22 lines · Ralf Baechle
Quoted 20 lines · Jan Nikitenko

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.

Quoted 6 lines
Quoted 1 line
Signed-off-by Jan Nikitenko <[email protected]>

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.

Quoted 7 lines

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

#4 lore raw
In reply to Ralf Baechle
Quoted 3 lines · Ralf Baechle

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

#5 lore raw
Quoted 4 lines · Maciej W. Rozycki
Quoted 3 lines

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.

Quoted 8 lines

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

#6 lore raw
In reply to Jan Nikitenko
Quoted 14 lines · Jan Nikitenko

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.

Quoted 7 lines

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 <[email protected]>

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

Ralf