From patchwork Thu Mar 20 23:47:52 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Felipe Balbi X-Patchwork-Id: 3868291 Return-Path: X-Original-To: patchwork-linux-arm@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork2.web.kernel.org (Postfix) with ESMTP id EB609BF540 for ; Thu, 20 Mar 2014 23:50:23 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 03C8620213 for ; Thu, 20 Mar 2014 23:50:23 +0000 (UTC) Received: from casper.infradead.org (casper.infradead.org [85.118.1.10]) (using TLSv1.2 with cipher DHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id A886120176 for ; Thu, 20 Mar 2014 23:50:21 +0000 (UTC) Received: from merlin.infradead.org ([2001:4978:20e::2]) by casper.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1WQmj1-00013X-6W; Thu, 20 Mar 2014 23:50:15 +0000 Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.80.1 #2 (Red Hat Linux)) id 1WQmiy-00030v-Md; Thu, 20 Mar 2014 23:50:12 +0000 Received: from comal.ext.ti.com ([198.47.26.152]) by merlin.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1WQmiv-00030H-Us for linux-arm-kernel@lists.infradead.org; Thu, 20 Mar 2014 23:50:10 +0000 Received: from dflxv15.itg.ti.com ([128.247.5.124]) by comal.ext.ti.com (8.13.7/8.13.7) with ESMTP id s2KNndnj002027; Thu, 20 Mar 2014 18:49:39 -0500 Received: from DFLE73.ent.ti.com (dfle73.ent.ti.com [128.247.5.110]) by dflxv15.itg.ti.com (8.14.3/8.13.8) with ESMTP id s2KNndup004258; Thu, 20 Mar 2014 18:49:39 -0500 Received: from dlep32.itg.ti.com (157.170.170.100) by DFLE73.ent.ti.com (128.247.5.110) with Microsoft SMTP Server id 14.3.174.1; Thu, 20 Mar 2014 18:49:39 -0500 Received: from localhost (ileax41-snat.itg.ti.com [10.172.224.153]) by dlep32.itg.ti.com (8.14.3/8.13.8) with ESMTP id s2KNnckP006626; Thu, 20 Mar 2014 18:49:39 -0500 Date: Thu, 20 Mar 2014 18:47:52 -0500 From: Felipe Balbi To: Russell King Subject: UART 8250 and HW Flow Control Message-ID: <20140320234752.GA26964@saruman.home> MIME-Version: 1.0 User-Agent: Mutt/1.5.23 (2014-03-12) X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20140320_195010_074674_443662AC X-CRM114-Status: GOOD ( 21.53 ) X-Spam-Score: -6.9 (------) Cc: Linux ARM Kernel Mailing List , Greg KH , Muralidharan Karicheri , Linux Kernel Mailing List , linux-serial@vger.kernel.org X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.15 Precedence: list Reply-To: balbi@ti.com List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org X-Spam-Status: No, score=-4.2 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_MED, T_RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP Hi Russell, long ago you wrote a patch (see below) which converted TI16C750 flow control quirk into a capability flag followed by a fifosize check. I've tried to fully understand the rationale behind that commit and, specifically why 32 bytes. If I understood correctly the problem is that RTS goes low as soon as FIFO Trigger Level is reached which means that if the far end isn't fast enough, it might miss our Request To Send. But then, this causes a problem for UARTs with small FIFOs. I mean, if the far end _does_ use auto-CTS, then we could enable auto-RTS in our side, but that commit prevents that unless we patch that check. Right now we're dealing with a use case which is using a BT device over UART at 3Mbaud and the UART on the SoC has 16 bytes of FIFO (it's a 16550a compatible UART). Our first (quickest, dirtiest, crappiest) option is to just change 32 into 16 get it over with. HW Flow Control gets enabled and everybody is happy, soon after that "what ifs" started to pop :-) I wonder if a better (albeit with impact to performance, probably) approach would be to patch tty_{un,}throttle() so that we don't write an ammount >= FIFO_TRIGGER_LEVEL in affected UARTs, but that doesn't sound very nice either, or does it ? Do you have any extra insider's information about the reason for that patch and/or any suggestions on how to patch it in a better way ? I know it's an old commit (took it from historic Linux tree) but worth asking anyway. commit 1ad14ded82602e3753c724ce4647b44faf4dc658 Author: Russell King Date: Tue Oct 19 16:37:05 2004 +0100 [SERIAL] Convert TI16C750 flow control into a port capability. Add UART_CAP_AFE, and use this to enable TI16C750 flow control, but only if we have 32 bytes or more of FIFO. diff --git a/drivers/serial/8250.c b/drivers/serial/8250.c index 6e2e62d..7e63a7f 100644 --- a/drivers/serial/8250.c +++ b/drivers/serial/8250.c @@ -210,7 +210,7 @@ static const struct serial8250_config uart_config[] = { .tx_loadsz = 64, .fcr = UART_FCR_ENABLE_FIFO | UART_FCR_R_TRIG_10 | UART_FCR7_64BYTE, - .flags = UART_CAP_FIFO | UART_CAP_SLEEP, + .flags = UART_CAP_FIFO | UART_CAP_SLEEP | UART_CAP_AFE, }, [PORT_STARTECH] = { .name = "Startech", @@ -1584,11 +1584,14 @@ serial8250_set_termios(struct uart_port *port, struct termios *termios, } /* - * TI16C750: hardware flow control and 64 byte FIFOs. 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. + * 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. */ - if (up->port.type == PORT_16750) { + if (up->capabilities & UART_CAP_AFE && up->port.fifosize >= 32) { up->mcr &= ~UART_MCR_AFE; if (termios->c_cflag & CRTSCTS) up->mcr |= UART_MCR_AFE; diff --git a/drivers/serial/8250.h b/drivers/serial/8250.h index bbf767d..dcf5859 100644 --- a/drivers/serial/8250.h +++ b/drivers/serial/8250.h @@ -47,6 +47,7 @@ struct serial8250_config { #define UART_CAP_FIFO (1 << 8) /* UART has FIFO */ #define UART_CAP_EFR (1 << 9) /* UART has EFR */ #define UART_CAP_SLEEP (1 << 10) /* UART has IER sleep */ +#define UART_CAP_AFE (1 << 11) /* MCR-based hw flow control */ #undef SERIAL_DEBUG_PCI