[PATCH v2 0/3] serial: sh-sci: Assorted flow control fixes

[PATCH v2 0/3] serial: sh-sci: Assorted flow control fixes

[Geert Uytterhoeven]

	Hi Greg, Jiri,

This patch series fixes several issues related to hardware flow control
on Renesas SCIF UARTs.

For changes compared to v1 ("[PATCH 0/2] serial: sh-sci: Assorted flow
control fixes", https://lkml.org/lkml/2016/12/2/226), please refer to
the comments in the individual patches.

Thanks!

Geert Uytterhoeven (3):
  serial: sh-sci: Fix hang in sci_reset()
  serial: sh-sci: Fix late enablement of AUTORTS
  serial: sh-sci: Fix (AUTO)RTS in sci_init_pins()

 drivers/tty/serial/sh-sci.c | 33 +++++++++++++++++++++++----------
 1 file changed, 23 insertions(+), 10 deletions(-)

-- 
2.7.4

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

[PATCH v2 1/3] serial: sh-sci: Fix hang in sci_reset()

[Geert Uytterhoeven]

When the .set_termios() callback resets the UART, it first waits (busy
loops) until all characters in the transmit FIFO have been transmitted,
to prevent a port configuration change from impacting these characters.

However, if the UART has dedicated RTS/CTS hardware flow control
enabled, these characters may have been stuck in the FIFO due to CTS not
being asserted by the remote side.

  - When a new user opens the port, .set_termios() is called while
    transmission is still disabled, leading to an infinite loop:

	NMI watchdog: BUG: soft lockup - CPU#0 stuck for 22s!

  - When an active user changes port configuration without waiting for
    the draining of the transmit FIFO, this may also block indefinitely,
    until CTS is asserted by the remote side.

This has been observed with SCIFA (on r8a7740/armadillo), and SCIFB and
HSCIF (on r8a7791/koelsch).

To fix this, remove the code that waits for the draining of the transmit
FIFO.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
To reproduce:

    stty -echo < /dev/scifb0
    stty crtscts < /dev/scifb0
    echo hello > /dev/scifb0	# returns early (wrote to FIFO)
    echo hello > /dev/scifb0	# hangs

v2:
  - Remove the draining of the transmit FIFO completely, instead of
    skipping it when tranmission was not enabled, as the hang could
    still be reproduced when an active user changes port configuration
    without waiting for the draining of the transmit FIFO.
---
 drivers/tty/serial/sh-sci.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
index 1df57461ece4337b..446a23bee14008bb 100644
--- a/drivers/tty/serial/sh-sci.c
+++ b/drivers/tty/serial/sh-sci.c
@@ -2159,10 +2159,6 @@ static void sci_reset(struct uart_port *port)
 	unsigned int status;
 	struct sci_port *s = to_sci_port(port);
 
-	do {
-		status = serial_port_in(port, SCxSR);
-	} while (!(status & SCxSR_TEND(port)));
-
 	serial_port_out(port, SCSCR, 0x00);	/* TE=0, RE=0, CKE1=0 */
 
 	reg = sci_getreg(port, SCFCR);
-- 
2.7.4

[PATCH v2 2/3] serial: sh-sci: Fix late enablement of AUTORTS

[Geert Uytterhoeven]

When changing hardware control flow for a UART with dedicated RTS/CTS
pins, the new AUTORTS state is not immediately reflected in the
hardware, but only when RTS is raised.  However, the serial core does
not 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.

To fix this, 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.) <cbaumann@visteon.com>
Fixes: 33f50ffc253854cf ("serial: sh-sci: Fix support for hardware-assisted RTS/CTS")
Signed-off-by: Geert Uytterhoeven <geert+renesas@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

v2:
  - Remove the paragraph about sci_init_pins() and reword.
    When hardware control flow is disabled, there can still be small
    glitches in the RTS signal.
    These glitches are now addressed in a separate patch.
---
 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 446a23bee14008bb..6e405fb5a23f0b4a 100644
--- a/drivers/tty/serial/sh-sci.c
+++ b/drivers/tty/serial/sh-sci.c
@@ -2372,6 +2372,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 |= SCSCR_RE | SCSCR_TE |
 		   (s->cfg->scscr & ~(SCSCR_CKE1 | SCSCR_CKE0));
-- 
2.7.4

[PATCH v2 3/3] serial: sh-sci: Fix (AUTO)RTS in sci_init_pins()

[Geert Uytterhoeven]

If a UART has dedicated RTS/CTS pins, and 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().

To fix this, consider the current (AUTO)RTS state when (re)initializing
the pins.  Note that for SCIFA/SCIFB, AUTORTS needs explicit
configuration of the RTS# pin function, while (H)SCIF handles this
automatically.

Fixes: d2b9775d795ec05f ("serial: sh-sci: Correct pin initialization on (H)SCIF")
Fixes: e9d7a45a03991349 ("serial: sh-sci: Add pin initialization for SCIFA/SCIFB")
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
v2:
  - New.
---
 drivers/tty/serial/sh-sci.c | 25 +++++++++++++++++++------
 1 file changed, 19 insertions(+), 6 deletions(-)

diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
index 6e405fb5a23f0b4a..71707e8e6e3ffe76 100644
--- a/drivers/tty/serial/sh-sci.c
+++ b/drivers/tty/serial/sh-sci.c
@@ -683,24 +683,37 @@ static void sci_init_pins(struct uart_port *port, unsigned int cflag)
 	}
 
 	if (port->type == PORT_SCIFA || port->type == PORT_SCIFB) {
+		u16 data = serial_port_in(port, SCPDR);
 		u16 ctrl = serial_port_in(port, SCPCR);
 
 		/* Enable RXD and TXD pin functions */
 		ctrl &= ~(SCPCR_RXDC | SCPCR_TXDC);
 		if (to_sci_port(port)->has_rtscts) {
-			/* RTS# is output, driven 1 */
-			ctrl |= SCPCR_RTSC;
-			serial_port_out(port, SCPDR,
-				serial_port_in(port, SCPDR) | SCPDR_RTSD);
+			/* RTS# is output, active low, unless autorts */
+			if (!(port->mctrl & TIOCM_RTS)) {
+				ctrl |= SCPCR_RTSC;
+				data |= SCPDR_RTSD;
+			} else if (!s->autorts) {
+				ctrl |= SCPCR_RTSC;
+				data &= ~SCPDR_RTSD;
+			} else {
+				/* Enable RTS# pin function */
+				ctrl &= ~SCPCR_RTSC;
+			}
 			/* Enable CTS# pin function */
 			ctrl &= ~SCPCR_CTSC;
 		}
+		serial_port_out(port, SCPDR, data);
 		serial_port_out(port, SCPCR, ctrl);
 	} else if (sci_getreg(port, SCSPTR)->size) {
 		u16 status = serial_port_in(port, SCSPTR);
 
-		/* RTS# is output, driven 1 */
-		status |= SCSPTR_RTSIO | SCSPTR_RTSDT;
+		/* RTS# is always output; and active low, unless autorts */
+		status |= SCSPTR_RTSIO;
+		if (!(port->mctrl & TIOCM_RTS))
+			status |= SCSPTR_RTSDT;
+		else if (!s->autorts)
+			status &= ~SCSPTR_RTSDT;
 		/* CTS# and SCK are inputs */
 		status &= ~(SCSPTR_CTSIO | SCSPTR_SCKIO);
 		serial_port_out(port, SCSPTR, status);
-- 
2.7.4