| Message ID | 8B957E110B62714A84290A01A597805F05CA0AA0@Exchange.discretix.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
On Sunday 15 November 2009, Alon Ziv wrote: > *)PORT_16450, }, > - { .type = "serial", .compatible = "ns16550", .data = (void *)PORT_16550, }, > + { .type = "serial", .compatible = "ns16550", .data = (void *)PORT_16550A, }, Does not seem logical. If the device claims compatibility with ns16550, we should not automatically assume it's an ns16550a. Why not add another line for { .type = "serial", .compatible = "ns16550a", .data = (void *)PORT_16550A, }, Arnd <><
Hi, On Monday, November 16, 2009, Arnd wrote: > > - { .type = "serial", .compatible = "ns16550", .data = (void *)PORT_16550, }, > > + { .type = "serial", .compatible = "ns16550", .data = (void *)PORT_16550A, }, > > Does not seem logical. If the device claims compatibility with ns16550, we should > not automatically assume it's an ns16550a. Why not add another line for > Unfortunately, there is no way to change what the device claims--it's encoded into the OpenFirmware tree by the EDK tools. And, in any case, the device is actually not lying: it _is_ compatible with NS16550--just with a non-buggy one. Unfortunately the kernel driver for 8250-class UARTs makes the conservative choice to assume any 16550 is one of the (early, buggy) revisions where the FIFO was non-functional; any 16550 with working UART is classed as a 16550A. -az ********************************************************************************************** IMPORTANT: The contents of this email and any attachments are confidential. They are intended for the named recipient(s) only. If you have received this email in error, please notify the system manager or the sender immediately and do not disclose the contents to anyone or make copies thereof.
On Thursday 19 November 2009, Alon Ziv wrote: > On Monday, November 16, 2009, Arnd wrote: > > > - { .type = "serial", .compatible = "ns16550", .data = (void > *)PORT_16550, }, > > > + { .type = "serial", .compatible = "ns16550", .data = (void > *)PORT_16550A, }, > > > > Does not seem logical. If the device claims compatibility with > ns16550, we should > > not automatically assume it's an ns16550a. Why not add another line > for > > > > Unfortunately, there is no way to change what the device claims--it's > encoded into the OpenFirmware tree by the EDK tools. > And, in any case, the device is actually not lying: it is compatible > with NS16550--just with a non-buggy one. Unfortunately the kernel > driver for 8250-class UARTs makes the conservative choice to assume any > 16550 is one of the (early, buggy) revisions where the FIFO was > non-functional; any 16550 with working UART is classed as a 16550A. In that case, add another entry for the device encoded in the firmware itself. The ns16550 entry should be the second one after a more specific one telling which device it is exactly. Arnd <><
> -----Original Message----- > From: linuxppc-dev-bounces+stephen=neuendorffer.name@lists.ozlabs.org [mailto:linuxppc-dev- > bounces+stephen=neuendorffer.name@lists.ozlabs.org] On Behalf Of Alon Ziv > Sent: Thursday, November 19, 2009 4:47 AM > To: Arnd Bergmann; linuxppc-dev@lists.ozlabs.org > Subject: RE: Bug in drivers/serial/of_serial.c? > > Hi, > > On Monday, November 16, 2009, Arnd wrote: > > > - { .type = "serial", .compatible = "ns16550", .data = (void > *)PORT_16550, }, > > > + { .type = "serial", .compatible = "ns16550", .data = (void > *)PORT_16550A, }, > > > > Does not seem logical. If the device claims compatibility with > ns16550, we should > > not automatically assume it's an ns16550a. Why not add another line > for > > > > Unfortunately, there is no way to change what the device claims--it's > encoded into the OpenFirmware tree by the EDK tools. > And, in any case, the device is actually not lying: it _is_ compatible > with NS16550--just with a non-buggy one. Unfortunately the kernel > driver for 8250-class UARTs makes the conservative choice to assume any > 16550 is one of the (early, buggy) revisions where the FIFO was > non-functional; any 16550 with working UART is classed as a 16550A. Definitely changing what is generated by EDK seems to make sense here... Steve This email and any attachments are intended for the sole use of the named recipient(s) and contain(s) confidential information that may be proprietary, privileged or copyrighted under applicable law. If you are not the intended recipient, do not read, copy, or forward this email message or any attachments. Delete this email message and any attachments immediately.
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, },
As of 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 to be the correct way: --- Xilinx 16550 UART is actually 16550A-compatible + { .type = "serial", .compatible = "ns16550", .data = (void *)PORT_16550A, }, { .type = "serial", .compatible = "ns16750", .data = (void *)PORT_16750, }, { .type = "serial", .compatible = "ns16850", .data = (void *)PORT_16850, }, #ifdef CONFIG_SERIAL_OF_PLATFORM_NWPSERIAL ********************************************************************************************** IMPORTANT: The contents of this email and any attachments are confidential. They are intended for the named recipient(s) only. If you have received this email in error, please notify the system manager or the sender immediately and do not disclose the contents to anyone or make copies thereof.