From patchwork Wed Sep 25 00:39:09 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tim Kryger X-Patchwork-Id: 20556 Return-Path: X-Original-To: linaro@patches.linaro.org Delivered-To: linaro@patches.linaro.org Received: from mail-oa0-f72.google.com (mail-oa0-f72.google.com [209.85.219.72]) by ip-10-151-82-157.ec2.internal (Postfix) with ESMTPS id 7ACE525E41 for ; Wed, 25 Sep 2013 00:40:26 +0000 (UTC) Received: by mail-oa0-f72.google.com with SMTP id l10sf843607oag.3 for ; Tue, 24 Sep 2013 17:40:25 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:delivered-to:from:to:cc:subject:date:message-id :mime-version:x-original-sender:x-original-authentication-results :precedence:mailing-list:list-id:list-post:list-help:list-archive :list-unsubscribe:content-type:content-transfer-encoding; bh=wzErfTkbFTUp7z6V4yOBOdncn4KHnpwaxZA95oxp4cs=; b=d9rVLyNe4VztgHswqyPcdLINB+XQxHU+GIPxdtHy9JAELSPP9AgoKEZkQZ8WFC0w60 /K8yGbiU1NMkJwPvvdgOCA09QMfmYp1SSr4fZ97gXWS/4jT3Bb8mZJYERLyH1IXqu1ae R7ryoEY1O2SmLT0DQuYYkxj5yiUS6/sogulLrdXxhp4SThdt0+fi9KLUQGSSiym2ocno PW/wKKD0HEgsnV5xyNGDiJ5LyL+5dy+ejaaWEJQGc6BK5qQTOCE+yIsydNP/vG/fEAFN tBimV7fCTHEaX/x/3EeiLy7hkJB+cyk1qxaUNLr//vPoPVjW/by73yyQKvyyaxQxMnTD OrDQ== X-Gm-Message-State: ALoCoQmoWBSVTEvlC3uKb0FjVv1rj+lPj8rvXNSeXhYctWN7xGQZCYLSIRjG6/wV64agNPVtdpla X-Received: by 10.182.133.38 with SMTP id oz6mr3358618obb.45.1380069625743; Tue, 24 Sep 2013 17:40:25 -0700 (PDT) X-BeenThere: patchwork-forward@linaro.org Received: by 10.182.232.133 with SMTP id to5ls627220obc.8.gmail; Tue, 24 Sep 2013 17:40:25 -0700 (PDT) X-Received: by 10.52.98.131 with SMTP id ei3mr25360675vdb.4.1380069625545; Tue, 24 Sep 2013 17:40:25 -0700 (PDT) Received: from mail-vb0-f50.google.com (mail-vb0-f50.google.com [209.85.212.50]) by mx.google.com with ESMTPS id aj7si9166646vec.97.1969.12.31.16.00.00 (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Tue, 24 Sep 2013 17:40:25 -0700 (PDT) Received-SPF: neutral (google.com: 209.85.212.50 is neither permitted nor denied by best guess record for domain of patch+caf_=patchwork-forward=linaro.org@linaro.org) client-ip=209.85.212.50; Received: by mail-vb0-f50.google.com with SMTP id x14so4075434vbb.9 for ; Tue, 24 Sep 2013 17:40:25 -0700 (PDT) X-Received: by 10.52.119.228 with SMTP id kx4mr25371278vdb.12.1380069625346; Tue, 24 Sep 2013 17:40:25 -0700 (PDT) X-Forwarded-To: patchwork-forward@linaro.org X-Forwarded-For: patch@linaro.org patchwork-forward@linaro.org Delivered-To: patches@linaro.org Received: by 10.220.174.196 with SMTP id u4csp273641vcz; Tue, 24 Sep 2013 17:40:24 -0700 (PDT) X-Received: by 10.43.104.73 with SMTP id dl9mr13274885icc.39.1380069624529; Tue, 24 Sep 2013 17:40:24 -0700 (PDT) Received: from mms1.broadcom.com (mms1.broadcom.com. [216.31.210.17]) by mx.google.com with ESMTP id n8si2776803icw.1.1969.12.31.16.00.00; Tue, 24 Sep 2013 17:40:24 -0700 (PDT) Received-SPF: pass (google.com: best guess record for domain of tkryger@broadcom.com designates 216.31.210.17 as permitted sender) client-ip=216.31.210.17; Received: from [10.9.208.55] by mms1.broadcom.com with ESMTP (Broadcom SMTP Relay (Email Firewall v6.5)); Tue, 24 Sep 2013 17:36:15 -0700 X-Server-Uuid: 06151B78-6688-425E-9DE2-57CB27892261 Received: from IRVEXCHSMTP3.corp.ad.broadcom.com (10.9.207.53) by IRVEXCHCAS07.corp.ad.broadcom.com (10.9.208.55) with Microsoft SMTP Server (TLS) id 14.1.438.0; Tue, 24 Sep 2013 17:40:14 -0700 Received: from mail-sj1-12.sj.broadcom.com (10.10.10.20) by IRVEXCHSMTP3.corp.ad.broadcom.com (10.9.207.53) with Microsoft SMTP Server id 14.1.438.0; Tue, 24 Sep 2013 17:40:14 -0700 Received: from mps-infra-lab3.broadcom.com ( mps-infra-lab3.sj.broadcom.com [10.19.114.109]) by mail-sj1-12.sj.broadcom.com (Postfix) with ESMTP id 65C12207C0; Tue, 24 Sep 2013 17:40:14 -0700 (PDT) Received: by mps-infra-lab3.broadcom.com (Postfix, from userid 1004) id 24A144589D8; Tue, 24 Sep 2013 17:40:14 -0700 (PDT) From: "Tim Kryger" To: "Greg Kroah-Hartman" , "Heikki Krogerus" cc: "Tim Kryger" , linux-serial@vger.kernel.org, linux-kernel@vger.kernel.org, patches@linaro.org Subject: [PATCH] serial: 8250_dw: Improve unwritable LCR workaround Date: Tue, 24 Sep 2013 17:39:09 -0700 Message-ID: <1380069549-9176-1-git-send-email-tim.kryger@linaro.org> X-Mailer: git-send-email 1.8.0.1 MIME-Version: 1.0 X-WSS-ID: 7E5CF0750UO10711878-01-01 X-Removed-Original-Auth: Dkim didn't pass. X-Original-Sender: tim.kryger@linaro.org X-Original-Authentication-Results: mx.google.com; spf=neutral (google.com: 209.85.212.50 is neither permitted nor denied by best guess record for domain of patch+caf_=patchwork-forward=linaro.org@linaro.org) smtp.mail=patch+caf_=patchwork-forward=linaro.org@linaro.org Precedence: list Mailing-list: list patchwork-forward@linaro.org; contact patchwork-forward+owners@linaro.org List-ID: X-Google-Group-Id: 836684582541 List-Post: , List-Help: , List-Archive: List-Unsubscribe: , The Designware UART has a limitation where it ignores writes into the LCR if the UART is busy. The current workaround stashes a copy of the last written LCR and writes it back down to the hardware if it receives a special busy interrupt which is raised when a write was ignored. Unfortunately, interrupts are typically disabled prior to performing a sequence of register writes that include the LCR so the point at which the retry occurs is too late. An example is serial8250_do_set_termios() where an ignored LCR write results in the baud divisor not being set and instead a garbage character is sent out the transmitter. Furthermore, since serial_port_out() offers no way to indicate failure, a serious effort must be made to ensure that the LCR is actually updated before returning back to the caller. This is difficult, however, as a UART that was busy during the first attempt is likely to still be busy when a subsequent attempt is made unless some extra action is taken. This updated workaround takes the extreme action of clearing the TX/RX FIFOs and reading the receive buffer before writing down the LCR in the hope that doing so will force the UART into an idle state. While this may seem unnecessarily aggressive, writes to the LCR are used to change the baud rate, parity, stop bit, or data length so the data that may be lost is likely not important. Admittedly, this is far from ideal but it seems to be the best that can be done given the hardware limitations. Lastly, the revised workaround doesn't touch the LCR in the ISR, so it avoids the possibility of a "serial8250: too much work for irq" lock up. This problem is rare in real situations but can be reproduced easily by wiring up two UARTs and running the following commands. # stty -F /dev/ttyS1 echo # stty -F /dev/ttyS2 echo # cat /dev/ttyS1 & [1] 375 # echo asdf > /dev/ttyS1 asdf [ 27.700000] serial8250: too much work for irq96 [ 27.700000] serial8250: too much work for irq96 [ 27.710000] serial8250: too much work for irq96 [ 27.710000] serial8250: too much work for irq96 [ 27.720000] serial8250: too much work for irq96 [ 27.720000] serial8250: too much work for irq96 [ 27.730000] serial8250: too much work for irq96 [ 27.730000] serial8250: too much work for irq96 [ 27.740000] serial8250: too much work for irq96 Signed-off-by: Tim Kryger Reviewed-by: Matt Porter Reviewed-by: Markus Mayer --- drivers/tty/serial/8250/8250_dw.c | 57 +++++++++++++++++++++++++++------------ 1 file changed, 40 insertions(+), 17 deletions(-) diff --git a/drivers/tty/serial/8250/8250_dw.c b/drivers/tty/serial/8250/8250_dw.c index daf710f..ffbb69c 100644 --- a/drivers/tty/serial/8250/8250_dw.c +++ b/drivers/tty/serial/8250/8250_dw.c @@ -56,7 +56,6 @@ struct dw8250_data { - int last_lcr; int last_mcr; int line; struct clk *clk; @@ -76,17 +75,35 @@ static inline int dw8250_modify_msr(struct uart_port *p, int offset, int value) return value; } +/* The UART will ignore writes to LCR when busy we take aggressive steps + * to ensure that it is idle before attempting to write to LCR */ +static void dw8250_force_idle(struct uart_port *p) +{ + serial8250_clear_and_reinit_fifos(container_of + (p, struct uart_8250_port, port)); + (void)p->serial_in(p, UART_LSR); + (void)p->serial_in(p, UART_MSR); + (void)p->serial_in(p, UART_RX); +} + static void dw8250_serial_out(struct uart_port *p, int offset, int value) { struct dw8250_data *d = p->private_data; - if (offset == UART_LCR) - d->last_lcr = value; - - if (offset == UART_MCR) - d->last_mcr = value; - - writeb(value, p->membase + (offset << p->regshift)); + if (offset == UART_LCR) { + int tries = 1000; + while (tries--) { + if (value == p->serial_in(p, UART_LCR)) + return; + dw8250_force_idle(p); + writeb(value, p->membase + (UART_LCR << p->regshift)); + } + dev_err(p->dev, "Couldn't set LCR to %d\n", value); + } else { + if (offset == UART_MCR) + d->last_mcr = value; + writeb(value, p->membase + (offset << p->regshift)); + } } static unsigned int dw8250_serial_in(struct uart_port *p, int offset) @@ -107,13 +124,20 @@ static void dw8250_serial_out32(struct uart_port *p, int offset, int value) { struct dw8250_data *d = p->private_data; - if (offset == UART_LCR) - d->last_lcr = value; - - if (offset == UART_MCR) - d->last_mcr = value; - - writel(value, p->membase + (offset << p->regshift)); + if (offset == UART_LCR) { + int tries = 1000; + while (tries--) { + if (value == p->serial_in(p, UART_LCR)) + return; + dw8250_force_idle(p); + writel(value, p->membase + (UART_LCR << p->regshift)); + } + dev_err(p->dev, "Couldn't set LCR to %d\n", value); + } else { + if (offset == UART_MCR) + d->last_mcr = value; + writel(value, p->membase + (offset << p->regshift)); + } } static unsigned int dw8250_serial_in32(struct uart_port *p, int offset) @@ -131,9 +155,8 @@ static int dw8250_handle_irq(struct uart_port *p) if (serial8250_handle_irq(p, iir)) { return 1; } else if ((iir & UART_IIR_BUSY) == UART_IIR_BUSY) { - /* Clear the USR and write the LCR again. */ + /* Clear the USR */ (void)p->serial_in(p, d->usr_reg); - p->serial_out(p, UART_LCR, d->last_lcr); return 1; }