Re: [PATCH v4 3/4] serial: sh-sci: make RX FIFO parameters tunable via sysfs
From: Geert Uytterhoeven <geert@linux-m68k.org>
Date: 2017-02-08 10:38:25
Also in:
linux-renesas-soc
Hi Uli, On Wed, Feb 8, 2017 at 11:04 AM, Ulrich Hecht [off-list ref] wrote:
On Tue, Feb 7, 2017 at 3:40 PM, Geert Uytterhoeven [off-list ref] wrote:quoted
On Fri, Feb 3, 2017 at 11:38 AM, Ulrich Hecht [off-list ref] wrote:quoted
Allows tuning of the RX FIFO fill threshold and timeout. (The latter is only applicable to SCIFA and SCIFB). Signed-off-by: Ulrich Hecht <redacted> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> --- 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.
OK.
(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 guess you'll send a patch to fix that, too?
quoted hunk ↗ jump to hunk
quoted
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:--- 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); } } }Could you try and check if that works for you?
Thanks, that unbroke serial console input on all my boards with SCIFA consoles.
Tested-by: Geert Uytterhoeven <geert+renesas@glider.be>
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds