[ +to Russell, GregKH ]
Hi Will,
On 03/29/2016 06:35 AM, William R Sowerbutts wrote:
> Hello
>
> While working on my homebrew 68030 system, I have found a bug in the Linux
> 8250 serial driver when using the TL16C750 UART with RTS/CTS flow control.
> I have tested and can reproduce the fault with Linux 4.5.
Thanks for testing.
When I added the UPSTAT_AUTOCTS steering to the serial core,
I specifically avoided changing this as I didn't have the necessary
hardware to test the AFE behavior.
IOW, the existing 8250 driver behavior is unchanged since this feature was
specifically added for the 16C750 by Russell back in 2004 for v2.6.6:
Author: Russell King <rmk@xxxxxxxxxxxxxxxxxxxxxx>
Date: Sun Apr 11 21:08:55 2004 +0100
[SERIAL] Add support for TI16C750 hardware flow control.
I pulled 2.6.6 and verified that the serial core + 8250 driver behavior
is identical to current mainline wrt to UART_CAP_AFE and modem status
changes absent modem status interrupts.
So this brings up 2 questions:
1) Are you testing with an actual 16c750?
2) Can you verify with your testing that previous kernels exhibit same
problem? Any kernel before 4.0 should also exhibit the same problem.
> I have prepared a patch which resolves the issue. This patch is attached.
>
> The TL16C750 UART has hardware-assisted control of the RTS and CTS lines
> ("autoflow control"). This is intended to reduce the number of interrupts to
> the CPU has by handling the RTS and CTS flow control lines in hardware.
>
> The datasheet explains the feature in detail:
> http://www.ti.com/lit/ds/symlink/tl16c750.pdf
The crucial piece of information left out is whether MSR[dCTS] reflects
AFE-directed changes; the datasheet is vague on that detail.
> Briefly: When this hardware-assist is enabled the hardware monitors the CTS
> line; it will not start transmitting a new character unless CTS indicates the
> peer has capacity to receive it, and it will not raise interrupts to report
> changes in CTS to the CPU. When the peer later changes CTS to indicate it has
> capacity to receive the transmitter will automatically start transmitting the
> next byte from the transmit FIFO.
>
> The bug: The 8250 driver will also check the CTS line in software even when
> the hardware autoflow control is enabled. During every interrupt it will
> check the CTS input.
Only if dCTS is indicated.
> If the peer indicates it has no capacity to receive the
> interrupt handler will not load any more characters into the transmit FIFO.
> However if the transmit FIFO empties just as the peer drops the CTS line,
> during the resulting interrupt to refill the transmit FIFO the driver will
> note the state of CTS and will not refill the transmit FIFO. When CTS is
> raised again the hardware cannot resume transmission since the transmit FIFO
> remains empty, it will not raise a second "transmitter empty" interrupt since
> no characters were loaded in response to the previous one, and the hardware
> will not raise an interrupt to alert the CPU to the change in CTS (since the
> autoflow control is enabled).
>
> No further characters will be transmitted on the port until another interrupt
> is raised and causes the driver to re-examine CTS. For example hitting a key
> on an attached serial terminal will re-start the output when this bug is
> triggered.
All of this makes sense.
> The attached patch sets the UPSTAT_AUTOCTS bit in uart_port.status.
> uart_softcts_mode() will then correctly return false for this port when
> autoflow control is enabled. This means that uart_handle_cts_change, called
> by serial8250_modem_status from serial8250_handle_irq, will not inspect the
> CTS line and will no longer halt the transmitter.
That is the correct fix for the observed problem.
> I am driving a DEC VT510 terminal with my TL16C750, with RTS/CTS flow
> control. I observed that large amounts of output to the terminal would pause
> apparently at random, and would not resume until a key was pressed. The
> terminal behaved correctly when connected to another UART (FTDI FT232RL). The
> terminal also behaved correctly if I modified the 8250 driver not to enable
> the "autoflow control" feature on the TL16C750 (ie commented out the lines
> which set the AFE bit in MCR).
>
> There's a bit more info to be found, including oscilloscope traces
> illustrating the fault condition, here:
> https://www.retrobrewcomputers.org/forum/index.php?t=msg&goto=444&#msg_444
>
> It seems plausible that the UPSTAT_AUTORTS bit should also be set when
> enabling the autoflow control since setting bit 5 (AFE) in MCR always enables
> auto-CTS and also enables auto-RTS control when MCR bit 1 (RTS) is set.
> However I have not had any issues with RTS, have not tested this change, and
> I do not believe a stall will occur if both hardware and software control
> this line. Perhaps someone who better understands the implications of setting
> UPSTAT_AUTORTS would be able to comment on this?
>
> I hope you will consider merging this patch.
Please read Documentation/SubmittingPatches; specifically items 5, 7, 12, 15.
Regards,
Peter Hurley
> Many thanks
>
> Will Sowerbutts
>
> _________________________________________________________________________
> William R Sowerbutts will@xxxxxxxxxxxxxx
> "Carpe post meridiem" http://sowerbutts.com
> main(){char*s=">#=0> ^#X@#@^7=",c=0,m;for(;c<15;c++)for
> (m=-1;m<7;putchar(m++/6&c%3/2?10:s[c]-31&1<<m?42:32));}
>
--- >% ---
--- linux-4.5/drivers/tty/serial/8250/8250_port.c 2016-03-14 04:28:54.000000000 +0000
+++ linux-4.5-kiss/drivers/tty/serial/8250/8250_port.c 2016-03-28 17:55:45.580249260 +0100
@@ -2309,8 +2309,12 @@
*/
if (up->capabilities & UART_CAP_AFE && port->fifosize >= 32) {
up->mcr &= ~UART_MCR_AFE;
- if (termios->c_cflag & CRTSCTS)
+ port->status &= ~UPSTAT_AUTOCTS;
+
+ if (termios->c_cflag & CRTSCTS) {
up->mcr |= UART_MCR_AFE;
+ port->status |= UPSTAT_AUTOCTS;
+ }
}
/*
--
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
