From patchwork Wed Feb 8 10:04:27 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ulrich Hecht X-Patchwork-Id: 9562159 X-Patchwork-Delegate: geert@linux-m68k.org Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 5F09F602B1 for ; Wed, 8 Feb 2017 10:33:25 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 4D86628485 for ; Wed, 8 Feb 2017 10:33:25 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 40A67284C2; Wed, 8 Feb 2017 10:33:25 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.3 required=2.0 tests=BAYES_00, DKIM_ADSP_CUSTOM_MED, DKIM_SIGNED, FREEMAIL_FROM, RCVD_IN_DNSWL_HI, RCVD_IN_SORBS_SPAM, T_DKIM_INVALID autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 27AEE28485 for ; Wed, 8 Feb 2017 10:33:24 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752710AbdBHKdX (ORCPT ); Wed, 8 Feb 2017 05:33:23 -0500 Received: from mail-ot0-f194.google.com ([74.125.82.194]:32921 "EHLO mail-ot0-f194.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752664AbdBHKat (ORCPT ); Wed, 8 Feb 2017 05:30:49 -0500 Received: by mail-ot0-f194.google.com with SMTP id f9so17514014otd.0 for ; Wed, 08 Feb 2017 02:30:49 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:sender:in-reply-to:references:from:date:message-id :subject:to:cc; bh=/evRFYrOu0ID+asCW2RxQDEAd5gW4ld4/FVsSvLDE20=; b=F1/4pSRI7jck0RYtz6GCnBXUlz+CZDd31yHLIl6KtdGMKwgb0IG9Iu1OZ/JEGNZPSW JuF31qcBaxzjKMiwoEA/DPlxobRigZoOE74QxfAtPCC1qD6156c1FSnfk3/62Yj1Aobr uRgPirmL1ivpm2D3mpznYmCbRp/sDU5MqCYV3BllpwJeBZj8T21P1cW1xHjMUhjl0b8b Gi5h7kXWehtcg8W8xM5QoFOBY66UtamoA9NUOImIwV930yDCE/e2yyMSO8RaTHjLeLhe ML0GPZUZ8ftdYAnW+Mfx5msIa7mDLSsZuQBM1tOICk5gKxO1ZyKU+Gab2BVFi0FiS5PR eyQA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:sender:in-reply-to:references:from :date:message-id:subject:to:cc; bh=/evRFYrOu0ID+asCW2RxQDEAd5gW4ld4/FVsSvLDE20=; b=gsay3A9y3CC6xEVtAvpLJwLFJ6ZyZcoqXCP1mmmhcEFST7u9WUtTrHuox+ruTZQalp 8o69yYpvM82MK3DZMD7Zq0JyVHzZJH6A7Qhp1Y6TRit3pzJnE6lPoEUxrGAvvGO6HOCZ mNhQB3cKTPQCZnTXChhpzgPKyPyFz/QAuwwG6DIVFe9tC3nDS9wGCbGCMc7TaXarYQe7 CxgFDZs759Y7+J6uKNjTPu+0RMXbbWA/7htkKFg+JRnQjekvcKA62ufmELnBuTw7KQ5+ 1OleAUBYcdTtEIB3b+66wmG+nuioJz0gRVIS1xMjAry1D4WlqK5qXB7aHvd/8icn6q1f dtmQ== X-Gm-Message-State: AMke39nWwr9kFiFDwlhbZsvx4xDIKG+6/k9Izr1WIY4RD5pHe5OhEiegisonKgiwK3itBDvqapyARX1nRF30ng== X-Received: by 10.157.60.238 with SMTP id t43mr11831477otf.178.1486548268270; Wed, 08 Feb 2017 02:04:28 -0800 (PST) MIME-Version: 1.0 Received: by 10.157.68.156 with HTTP; Wed, 8 Feb 2017 02:04:27 -0800 (PST) In-Reply-To: References: <1486118300-12633-1-git-send-email-ulrich.hecht+renesas@gmail.com> <1486118300-12633-4-git-send-email-ulrich.hecht+renesas@gmail.com> From: Ulrich Hecht Date: Wed, 8 Feb 2017 11:04:27 +0100 X-Google-Sender-Auth: dnMLBKoUNgIMisW7iATl6CBmZTU Message-ID: Subject: Re: [PATCH v4 3/4] serial: sh-sci: make RX FIFO parameters tunable via sysfs To: Geert Uytterhoeven Cc: Linux-Renesas , Greg KH , Wolfram Sang , "linux-serial@vger.kernel.org" , Magnus Damm , Sergei Shtylyov Sender: linux-renesas-soc-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-renesas-soc@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On Tue, Feb 7, 2017 at 3:40 PM, Geert Uytterhoeven wrote: > Hi Ulrich, > > On Fri, Feb 3, 2017 at 11:38 AM, Ulrich Hecht > wrote: >> Allows tuning of the RX FIFO fill threshold and timeout. (The latter is >> only applicable to SCIFA and SCIFB). >> >> Signed-off-by: Ulrich Hecht >> Reviewed-by: Geert Uytterhoeven >> --- >> drivers/tty/serial/sh-sci.c | 87 +++++++++++++++++++++++++++++++++++++++++++++ >> 1 file changed, 87 insertions(+) >> >> diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c >> index 4a165ed..f95a56c 100644 >> --- a/drivers/tty/serial/sh-sci.c >> +++ b/drivers/tty/serial/sh-sci.c >> @@ -1055,6 +1055,66 @@ static void rx_fifo_timer_fn(unsigned long arg) >> scif_set_rtrg(port, 1); >> } >> >> +static ssize_t rx_trigger_show(struct device *dev, >> + struct device_attribute *attr, >> + char *buf) >> +{ >> + struct uart_port *port = dev_get_drvdata(dev); >> + struct sci_port *sci = to_sci_port(port); >> + >> + return sprintf(buf, "%d\n", sci->rx_trigger); >> +} >> + >> +static ssize_t rx_trigger_store(struct device *dev, >> + struct device_attribute *attr, >> + const char *buf, >> + size_t count) >> +{ >> + struct uart_port *port = dev_get_drvdata(dev); >> + struct sci_port *sci = to_sci_port(port); >> + long r; >> + >> + if (kstrtol(buf, 0, &r) == -EINVAL) >> + return -EINVAL; >> + sci->rx_trigger = scif_set_rtrg(port, r); >> + scif_set_rtrg(port, 1); > > I seem to have missed the above function call during my earlier review. > What's the purpose of resetting the trigger immediately to 1? For the software timeout case, the timeout and the trigger levels are set in the interrupt handler. Setting the threshold to 1 will trigger that when the next byte of data comes in, and it is easier than duplicating the logic here. (There actually is a bug here, in that the threshold should only be reset to 1 for software timeout IPs (SCIFA and SCIFB), but that is not what breaks SCIFA, of course.) > I.e. "echo 1 > /sys/class/tty/ttySC0/device/rx_fifo_trigger" fixes serial > console input on e.g. armadillo, but echoing 32 into rx_fifo_trigger doesn't > break it again. This is intended to work that way. For software timeout devices (SCIFA and SCIFB), the trigger level is not set in hardware unless an rx_fifo_timeout > 0 is set as well. The bug that breaks the SCIFA console is in sci_reset(), which sets a hardware threshold > 1 for devices for software timeout devices even though the rx_fifo_timeout is 0. Something like this should fix it: Could you try and check if that works for you? CU Uli Tested-by: Geert Uytterhoeven --- a/drivers/tty/serial/sh-sci.c +++ b/drivers/tty/serial/sh-sci.c @@ -2179,7 +2179,11 @@ static void sci_reset(struct uart_port *port) setup_timer(&s->rx_fifo_timer, rx_fifo_timer_fn, (unsigned long)s); } else { - scif_set_rtrg(port, s->rx_trigger); + if (port->type == PORT_SCIFA || + port->type == PORT_SCIFB) + scif_set_rtrg(port, 1); + else + scif_set_rtrg(port, s->rx_trigger); } } }