> Guys,
>
> should this diff really be like this?
> --- linux-3.4.orig/drivers/tty/serial/8250/8250.c
> +++ linux-3.4/drivers/tty/serial/8250/8250.c
> @@ -2430,15 +2430,16 @@ serial8250_do_set_termios(struct uart_po
> * LCR DLAB must be set to enable 64-byte FIFO mode. If the FCR
> * is written without DLAB set, this mode will be disabled.
> */
> - if (port->type == PORT_16750)
> + if (port->type == PORT_16750) {
> + /* emulated UARTs (Lucent Venus 167x) need two steps */
> + if (fcr & UART_FCR_ENABLE_FIFO)
> + serial_port_out(port, UART_FCR, UART_FCR_ENABLE_FIFO);
> serial_port_out(port, UART_FCR, fcr);
> + }
>
> serial_port_out(port, UART_LCR, cval); /* reset DLAB */
> up->lcr = cval; /* Save LCR */
> if (port->type != PORT_16750) {
> - /* emulated UARTs (Lucent Venus 167x) need two steps */
> - if (fcr & UART_FCR_ENABLE_FIFO)
> - serial_port_out(port, UART_FCR, UART_FCR_ENABLE_FIFO);
> serial_port_out(port, UART_FCR, fcr); /* set fcr */
> }
> serial8250_set_mctrl(port, port->mctrl);
No, it shouldn't. The buggy UART is emulating a generic 16550, so that's
the branch it has to be in.
A 16750 is special in that some of its FCR bits can only be written
with DLAB set. But you can't write the FCR before clearing DLAB for all
UARTS, because others put a completely different register, the extended
functions register (EFR) at the same offset when DLAB is set.
> For setting FCR, only PORT_16750 is special where FCR can be written
> directly. Everything else falls under later if() case. why is the 2
> step case applied for all UARTs?
I don't understand this. See above for an explanatiom of why a 16750 is
special. It's applied to all other UARTs because it's harmless and
simpler than trying to figure out when it's needed.
> Since most of the UARTs defined in uart_config[] has 2 ENABLE_FIFO,
> sounds like the 2 step should have been something like:
>
> if ( port_is_emulated and (fcr && ENABLE_FIFO) )
> serial_port_out(port, UART_FCR, UART_FCR_ENABLE_FIFO);
There is no way for the serial driver to detect an emulated port.
If the hardware designers did their job right, it behaves exactly
like a standard UART.
Don't get hung up about the fact it's an emulated UART. All the
software cares about is that it's a *buggy* 16550 clone.
> Am a bit confused here...is it possible that this was not reviewed
> thoroughly back in those days?
It's an obviously safe change; it won't hurt non-buggy UARTs, and
fixes known brokenness with buggy ones.
Now, *any* change can find latent bugs in some *other* UARTs, but the
only way to find that is by widespread testing. And the way to get
widespred testing is to push it out the door.
Which is what they did, and nobody complained for fifteen years.
Fifteen years of worldwide deployment is the most thorough review
I can imagine.
Now, you have finally found that heretofore-mythical buggy UART that
has different bugs, and conflicts with the workaround.
There are two ways forward:
1) Find an old Lucent Venus PCI modem and learn more about its bugs to
find a different bug workaround. They made millions, so I'm sure there
are plenty still out there, but it would be a pain to find one, or
2) Study what goes wrong with *your* buggy UART and how to identify *it*,
and make *that* workaround very specific.
Since you have one sitting right in front of you, how about we explore
option 2 for a bit?
Pariticularly because it's a synthesized UART and I seem to recall you
saying that you had hardware source code for it. That should enable
*very* precise characterization of the issue.
Is there some way that your UART is different from an original 16550A?
Even without that, perhaps we can find a way to *detect* the bug and
reset the UART out of it.
To start with, dump all of the registers that you can before, between,
and after the two FCR writes, and look for what changes.
Remember Linus's policy on regressions: it's Not Allowed to break old
stuff while fixing new stuff. Unless you can argue that there are zero
people who will want to run a 4.3+ kernel with an old PCI modem, just
ripping out legacy support is not an option, even if it's the simplest
one for you.
--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
