Sure George. There might as well be bug in our UART. Am not denying
it. Am novice to UARTs/modems, so trying to understand the code and
how they behave.
It sounds like the 2 step FCR write was done for "buggy" Lucent Venus.
However, other UARTs are able to handle double FCR write even if its
necessary or not.
Thanks.
On Tue, Sep 8, 2015 at 12:45 AM, George Spelvin <linux@xxxxxxxxxxx> wrote:
>> 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
