linux-serial.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Raphael Assenat <raph@8d.com>
To: linux-serial@vger.kernel.org
Subject: [PATCH RFC] Remove minimum FIFO size check for enabling AFE
Date: Wed, 11 Jul 2012 16:02:00 -0400	[thread overview]
Message-ID: <20120711200200.GD4260@8d.com> (raw)

    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;

                 reply	other threads:[~2012-07-11 20:02 UTC|newest]

Thread overview: [no followups] expand[flat|nested]  mbox.gz  Atom feed

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20120711200200.GD4260@8d.com \
    --to=raph@8d.com \
    --cc=linux-serial@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).