From patchwork Tue Oct 1 17:18:08 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tim Kryger X-Patchwork-Id: 20725 Return-Path: X-Original-To: linaro@patches.linaro.org Delivered-To: linaro@patches.linaro.org Received: from mail-ve0-f200.google.com (mail-ve0-f200.google.com [209.85.128.200]) by ip-10-151-82-157.ec2.internal (Postfix) with ESMTPS id 4880D23920 for ; Tue, 1 Oct 2013 17:19:51 +0000 (UTC) Received: by mail-ve0-f200.google.com with SMTP id oy12sf8555020veb.7 for ; Tue, 01 Oct 2013 10:19:50 -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=GHmT0TBm8P/7bPX4f/iF1L3hDtq9Bqv7k7omrZMbH68=; b=K36K7W+BI8SumCrXrlAZcKVOILVQRIk8JqtNVIo8ycu5woyGdOdk1XACTIX3v29fki /LJXQQ5p1lOvPxThHFPRUFc+TV+F1vw5NOfeNvuEAuNeS7l7t23Y8Ewe2Qy7BrekOEcm JjaoH25R0XicXfiHt9SPiZpSpIWqZr62tc/fMBvV/Mr3hCTsktPraaxTpEozkeCKfqRi IwpX2OHCIY9tLUFW2Y7TqeOniVImfhtAqEaxs+U86fhD38Z//WjvO9MxAmhbSLXU+rKO H2WqGa2MfTuNfnr4LTI06Qdx1xWoHO3Nly+sarh+8QYBn/IgNu1Fif4wwWTCyYicgIDW m+eQ== X-Received: by 10.236.14.100 with SMTP id c64mr10485496yhc.38.1380647990765; Tue, 01 Oct 2013 10:19:50 -0700 (PDT) X-BeenThere: patchwork-forward@linaro.org Received: by 10.49.18.199 with SMTP id y7ls152673qed.78.gmail; Tue, 01 Oct 2013 10:19:50 -0700 (PDT) X-Received: by 10.220.184.70 with SMTP id cj6mr1664861vcb.23.1380647990663; Tue, 01 Oct 2013 10:19:50 -0700 (PDT) Received: from mail-vc0-f170.google.com (mail-vc0-f170.google.com [209.85.220.170]) by mx.google.com with ESMTPS id dt10si1533626vdb.138.1969.12.31.16.00.00 (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Tue, 01 Oct 2013 10:19:50 -0700 (PDT) Received-SPF: neutral (google.com: 209.85.220.170 is neither permitted nor denied by best guess record for domain of patch+caf_=patchwork-forward=linaro.org@linaro.org) client-ip=209.85.220.170; Received: by mail-vc0-f170.google.com with SMTP id lc6so4894500vcb.29 for ; Tue, 01 Oct 2013 10:19:50 -0700 (PDT) X-Gm-Message-State: ALoCoQnb2uZCFHcAlfwn3BldDCp6quXWi4EDsybcRkMNq1losSi2iTXeUneHPNLFGgQDdrZ6NByk X-Received: by 10.220.237.208 with SMTP id kp16mr22818650vcb.4.1380647990308; Tue, 01 Oct 2013 10:19:50 -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 u4csp33579vcz; Tue, 1 Oct 2013 10:19:49 -0700 (PDT) X-Received: by 10.50.87.4 with SMTP id t4mr19052587igz.18.1380647989577; Tue, 01 Oct 2013 10:19:49 -0700 (PDT) Received: from mms3.broadcom.com (mms3.broadcom.com. [216.31.210.19]) by mx.google.com with ESMTP id lo7si7373373icc.26.1969.12.31.16.00.00; Tue, 01 Oct 2013 10:19:49 -0700 (PDT) Received-SPF: pass (google.com: best guess record for domain of tkryger@broadcom.com designates 216.31.210.19 as permitted sender) client-ip=216.31.210.19; Received: from [10.9.208.55] by mms3.broadcom.com with ESMTP (Broadcom SMTP Relay (Email Firewall v6.5)); Tue, 01 Oct 2013 10:08:49 -0700 X-Server-Uuid: B86B6450-0931-4310-942E-F00ED04CA7AF 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, 1 Oct 2013 10:19:35 -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, 1 Oct 2013 10:19:35 -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 97349207C0; Tue, 1 Oct 2013 10:19:35 -0700 (PDT) Received: by mps-infra-lab3.broadcom.com (Postfix, from userid 1004) id 6F1414589E0; Tue, 1 Oct 2013 10:19:35 -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 v2] serial: 8250_dw: Improve unwritable LCR workaround Date: Tue, 1 Oct 2013 10:18:08 -0700 Message-ID: <1380647888-32473-1-git-send-email-tim.kryger@linaro.org> X-Mailer: git-send-email 1.8.0.1 MIME-Version: 1.0 X-WSS-ID: 7E55DE2B2L890423793-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.220.170 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: , When configured with UART_16550_COMPATIBLE=NO or in versions prior to the introduction of this option, the Designware UART will ignore writes to the LCR if the UART is busy. The current workaround saves a copy of the last written LCR and re-writes it in the ISR for a special interrupt that 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 reads back the LCR after each write to confirm that the new value was accepted by the hardware. Should the hardware ignore a write, the TX/RX FIFOs are cleared and the receive buffer read before attempting to rewrite the LCR out of 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 Reviewed-by: Heikki Krogerus --- Changes in v2: - Rebased on tty-next - Updated commit messsage to mention UART_16550_COMPATIBLE - Removed potentially unnecessary read of LSR and MSR - Only attempt workaround when LCR write is ignored drivers/tty/serial/8250/8250_dw.c | 41 ++++++++++++++++++++++++++++++--------- 1 file changed, 32 insertions(+), 9 deletions(-) diff --git a/drivers/tty/serial/8250/8250_dw.c b/drivers/tty/serial/8250/8250_dw.c index d04a037..4658e3e 100644 --- a/drivers/tty/serial/8250/8250_dw.c +++ b/drivers/tty/serial/8250/8250_dw.c @@ -57,7 +57,6 @@ struct dw8250_data { u8 usr_reg; - int last_lcr; int last_mcr; int line; struct clk *clk; @@ -77,17 +76,33 @@ static inline int dw8250_modify_msr(struct uart_port *p, int offset, int value) return value; } +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_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)); + + /* Make sure LCR write wasn't ignored */ + 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); + } } static unsigned int dw8250_serial_in(struct uart_port *p, int offset) @@ -108,13 +123,22 @@ 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)); + + /* Make sure LCR write wasn't ignored */ + 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); + } } static unsigned int dw8250_serial_in32(struct uart_port *p, int offset) @@ -132,9 +156,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; }