[PATCH RFC] Remove minimum FIFO size check for enabling AFE

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



    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;
--
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


[Index of Archives]     [Kernel Newbies]     [Security]     [Netfilter]     [Bugtraq]     [Linux PPP]     [Linux FS]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Linmodem]     [Device Mapper]     [Linux Kernel for ARM]

  Powered by Linux