Bug in drivers/serial/of_serial.c?

[alonz]

Since commit eedacbf0, drivers/serial/of_serial.c forces the
serial core to recognize the xps-uart16550-2.00.b as an NS16550 (which
had no FIFO). Prior to this commit, the kernel's auto-configuration
logic would correctly recognize the UART as NS16550A (with 16-byte
FIFO).

The following patch corrects the issue in what I believe is the right way:

diff --git a/drivers/serial/of_serial.c b/drivers/serial/of_serial.c
index 02406ba..78267af 100644
--- a/drivers/serial/of_serial.c
+++ b/drivers/serial/of_serial.c
@@ -161,7 +161,7 @@ static int of_platform_serial_remove(struct of_device *ofdev
 static struct of_device_id __devinitdata of_platform_serial_table[] = {
        { .type = "serial", .compatible = "ns8250",   .data = (void *)PORT_8250,
        { .type = "serial", .compatible = "ns16450",  .data = (void *)PORT_16450
-       { .type = "serial", .compatible = "ns16550",  .data = (void *)PORT_16550
+       { .type = "serial", .compatible = "ns16550",  .data = (void *)PORT_16550
        { .type = "serial", .compatible = "ns16750",  .data = (void *)PORT_16750
        { .type = "serial", .compatible = "ns16850",  .data = (void *)PORT_16850
 #ifdef CONFIG_SERIAL_OF_PLATFORM_NWPSERIAL

[linnj]

I don't understand the problem completely, can you clarify?

Are you saying that we're ending up with a ns16550 driver which doesn't support FIFOs and we should be using a ns16550a driver which will use the FIFOs? 

I don't see that this patch does anything, what am I not seeing as the 2 lines look the same?

 -       { .type = "serial", .compatible = "ns16550",  .data = (void *)PORT_16550
+       { .type = "serial", .compatible = "ns16550",  .data = (void *)PORT_16550

Thanks.

[alonz]

Aiee... Something mangled my patch :smileysad:

It was supposed to be

 -       { .type = "serial", .compatible = "ns16550",  .data = (void *)PORT_16550
+       { .type = "serial", .compatible = "ns16550",  .data = (void *)PORT_16550A

Since the UART block reports (via openfirmware) that it is compatible with ns16550, yet the kernel's idea of 16550 conservatively assumes it has a buggy FIFO... (I gather some early 16550's did)

By announcing compatibility with ns16550A, we just tell the world our FIFO is working.

-az

[linnj]

Now I understand, and saw the other thread on the mailing list also.

Thanks.