Bug in 8250.c - uninitialized FIFOs

["Ondřej Pužman" <puzman@gmail.com>]

Hello,
I have found a bug in 8250.c driver which causes that 16550A uart FIFOs 
are not turned on during initialization if they are manually configured 
by setserial. UART is then working only as plain 16450 without FIFOs. On 
systems with higher interrupt latency this causes buffer overruns and 
loss of received data when using higher communication speeds.

I'm working for a company which produces industrial computers. These 
devices typically contain high number (8 or more) of traditional 16550A 
uarts - we use TL16C554A chips, but that is not much relevant. UARTs are 
connected to the CPU by ISA bus (Celeron based devices) or LPC bus (Atom 
based devices).

In the Linux the UARTs are using standard 8250.c driver and are 
initialized using setserial command:
setserial /dev/ttyS4 uart 16550A port 0x3E0 irq 10 baud_base 115200

This executes the UART initialization through serial8250_startup() 
function. At the beginning of the function up->capabilities is 
initialized from uart_config:
 up->capabilities = uart_config[up->port.type].flags;
Please note that neither up->port.fifosize nor up->tx_loadsz is 
initialized here!!

Later in the same function serial8250_clear_fifos() is called and 
disables FIFOs. The above comment says that they will be reenabled in 
set_termios (they won't ...)

After serial8250_startup() the serial8250_set_termios() is called. In 
this function the following check fails because up->port.fifosize is 
zero because it is not initialized correctly.

        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 variable remains zero and in the end the FCR register is set to zero 
which results in disabled FIFOs even if the UART type is 16550A. This is 
also true for other types of UARTs with FIFOs.

If the UART is autoconfigured via 'setserial /dev/ttySx autoconfig' then 
port.fifosize and tx_loadsz are initialized correctly in the 
autoconfig() function and the UART is working correctly then.

I checked the source codes and I can say that this bug is present in 
2.6.x series of kernels for a couple of years. Namely I can confirm its 
presence in 2.6.16.57, 2.6.32.24 and 2.6.36.1 (tested all of them on our 
hardware).

I think it was not noticed before because not many people use manually 
configured non PNP UARTs on ISA/LPC bus these days. Also the data loss 
caused by buffer overruns occures only if  IRQ latency is higher then 
time needed to receive one character on given communication speed.
For example our hardware looses received characters only if the UARTs 
are connected throught LPC bus with SERIRQ (serial IRQ transport) and 
not if they are connected to ISA bus because LPC SERIRQ has higher 
interrupt latency then parallel ISA interupt lines.

Here is the patch to correct the bug created against 2.6.36.1:

diff -Naur a/drivers/serial/8250.c b/drivers/serial/8250.c
--- a/drivers/serial/8250.c     2010-11-22 20:03:49.000000000 +0100
+++ b/drivers/serial/8250.c     2010-12-03 18:32:54.000000000 +0100
@@ -1952,6 +1952,8 @@
        unsigned char lsr, iir;
        int retval;

+       up->port.fifosize = uart_config[up->port.type].fifo_size;
+       up->tx_loadsz = uart_config[up->port.type].tx_loadsz;
        up->capabilities = uart_config[up->port.type].flags;
        up->mcr = 0;


The patch should work on most 2.6.x kernels.
I hope that someone will be able to put that in the official kernel tree.
--
Ondrej Puzman
HW & SW developer

AMiT, spol. s.r.o.