Hi Geert,
Thank you for the patch.
On Friday 02 Dec 2016 13:35:10 Geert Uytterhoeven wrote:
> If a UART has dedicated RTS/CTS pins, there are some issues:
>
> 1. When changing hardware control flow, the new AUTORTS state is not
> immediately reflected in the hardware, but only when RTS is raised.
> However, the serial core doesn't call .set_mctrl() after
> .set_termios(), hence AUTORTS may only become effective when the port
> is closed, and reopened later.
> Note that this problem does not happen when manually using stty to
> change CRTSCTS, as AUTORTS will work fine on next open.
>
> 2. When hardware control flow is disabled (or AUTORTS is not yet
> effective), changing any serial port configuration deasserts RTS, as
> .set_termios() calls sci_init_pins().
Isn't this still a problem with this patch applied ? Calling sci_set_mctrl()
should reconfigure the pins properly, but won't there be a short window during
which the configuration will be wrong ?
> To fix both issues, call .set_mctrl() from .set_termios() when dedicated
> RTS/CTS pins are present, to refresh the AUTORTS or RTS state.
> This is similar to what other drivers supporting AUTORTS do (e.g.
> omap-serial).
>
> Reported-by: Baumann, Christoph (C.) <cbaum...@visteon.com> (issue 1)
> Fixes: 33f50ffc253854cf ("serial: sh-sci: Fix support for hardware-assisted
> RTS/CTS")
> Signed-off-by: Geert Uytterhoeven <geert+rene...@glider.be>
> ---
> Tested on r8a7791/koelsch with HSCIF1 (GPIO hardware flow control),
> and HSCIF2 and SCIFB0 (dedicated hardware flow control).
>
> A simple test program (basically "cat" with CRTSCTS configuration
> capability) can be found at https://github.com/geertu/sercat
>
> Without this patch the following will fail:
>
> 1. sercat -f /dev/hscif2 &
> seq 100 120 | sercat -f -w /dev/hscif1 # hangs
>
> 2. seq 200 220 | sercat -f -w /dev/hscif2 &
> sercat -f /dev/hscif1 # no data received
> ---
> drivers/tty/serial/sh-sci.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
> index 91e7dddbf72cd3de..c503db1900f003ed 100644
> --- a/drivers/tty/serial/sh-sci.c
> +++ b/drivers/tty/serial/sh-sci.c
> @@ -2340,6 +2340,10 @@ static void sci_set_termios(struct uart_port *port,
> struct ktermios *termios,
>
> serial_port_out(port, SCFCR, ctrl);
> }
> + if (port->flags & UPF_HARD_FLOW) {
> + /* Refresh (Auto) RTS */
> + sci_set_mctrl(port, port->mctrl);
> + }
>
> scr_val |= s->cfg->scscr & ~(SCSCR_CKE1 | SCSCR_CKE0);
> dev_dbg(port->dev, "SCSCR 0x%x\n", scr_val);
--
Regards,
Laurent Pinchart
