On Tue, Feb 7, 2017 at 3:40 PM, Geert Uytterhoeven <geert@xxxxxxxxxxxxxx> wrote:
> Hi Ulrich,
>
> On Fri, Feb 3, 2017 at 11:38 AM, Ulrich Hecht
> <ulrich.hecht+renesas@xxxxxxxxx> 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 <ulrich.hecht+renesas@xxxxxxxxx>
>> Reviewed-by: Geert Uytterhoeven <geert+renesas@xxxxxxxxx>
>> ---
>> 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:
--- 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?
CU
Uli
