* [PATCH RFC] Remove minimum FIFO size check for enabling AFE
@ 2012-07-11 20:02 Raphael Assenat
0 siblings, 0 replies; only message in thread
From: Raphael Assenat @ 2012-07-11 20:02 UTC (permalink / raw)
To: linux-serial
When CRTSCTS is requested and the hardware supports AFE, I don't
understand why having a FIFO of less than 32 bytes is a reason not
to enable it.
The logic currently in place prevents AFE from being enabled when
we use a SC16C550 UART due to its 16 byte FIFO.
The original comment is not wrong. Indeed, if the remote uart is not
using automatic flow control, it will probably send extra bytes
even after RTS is deasserted.
But requiring a local UART FIFO of at least 32 bytes does not make
sense to me because:
1) If AFE is not enabled (due to a small FIFO of 16 bytes, as in my case),
what mechanism will save us from receive overrun? None, and we loose data
if the UART FIFO fills up.
2) If AFE is enabled AND the remote latency is such that it stops
sending data only after several bytes, filling the FIFO, we still loose
data. But at least we /tried/ to stop the remote UART. By doing so,
if the remote UART had been using auto flow control (or would have reacted
fast enough anyway), then the flow would have been properly interrupted and
no data would have been lost. Isn't this better?
3) In the worst case of remote software latency, we can imagine the remote
UART emptying its whole FIFO before stopping. If this FIFO is 64 bytes,
the 32 bytes minimum wouldn't be enough anyway. The real problem here
would be the remote device which really should honor flow control signals
with a more acceptable latency.
Given all the above, I would suggest simply removing the FIFO size check
with the following patch. I don't think anything should break because of
this, but as I could not find when this fifo size check was added, I could
not read about the reasons why it is there...
If this patch is not acceptable, alternatives that would still solve my
problem include reducing the minimum FIFO size to 16 or adding a check
for this specific UART at the same place.
diff --git a/drivers/tty/serial/8250/8250.c b/drivers/tty/serial/8250/8250.c
index ffea4f3..e09b393 100644
--- a/drivers/tty/serial/8250/8250.c
+++ b/drivers/tty/serial/8250/8250.c
@@ -2252,14 +2252,9 @@ serial8250_do_set_termios(struct uart_port *port, struct ktermios *termios,
}
/*
- * MCR-based auto flow control. When AFE is enabled, RTS will be
- * deasserted when the receive FIFO contains more characters than
- * the trigger, or the MCR RTS bit is cleared. In the case where
- * the remote UART is not using CTS auto flow control, we must
- * have sufficient FIFO entries for the latency of the remote
- * UART to respond. IOW, at least 32 bytes of FIFO.
+ * MCR-based auto flow control.
*/
- if (up->capabilities & UART_CAP_AFE && port->fifosize >= 32) {
+ if (up->capabilities & UART_CAP_AFE) {
up->mcr &= ~UART_MCR_AFE;
if (termios->c_cflag & CRTSCTS)
up->mcr |= UART_MCR_AFE;
^ permalink raw reply related [flat|nested] only message in thread
only message in thread, other threads:[~2012-07-11 20:02 UTC | newest]
Thread overview: (only message) (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-07-11 20:02 [PATCH RFC] Remove minimum FIFO size check for enabling AFE Raphael Assenat
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).