2016-04-29 12:59:34

by Geert Uytterhoeven

[permalink] [raw]
Subject: [PATCH v2 00/11] serial: sh-sci: Hardware Flow Control Updates

Hi all,

This patch series contains updates to the Renesas SCI UART driver,
related to hardware flow control:
- Patches 1-2: Device Tree binding updates for GPIO-controlled modem
lines, and for dedicated RTS/CTS modem lines,
- Patches 3-4: Driver support for GPIO-controlled modem lines, using
the serial_mctrl_gpio helpers,
- New patches 5-11: Driver support for hardware-assisted (automatic)
hardware flow control using the dedicated RTS/CTS modem lines.

Changes compared to v1 (more details in the individual patches):
- Add Acked-by,
- Refer to the Generic Serial DT Bindings,
- Drop out[12]-gpios,
- Reject combining GPIO and dedicated RTS/CTS,
- Drop "serial: sh-sci: Replace SCIx_HAVE_RTSCTS by standard
UPF_HARD_FLOW",
- Add support for hardware-assisted automatic RTS/CTS control on
(H)SCIF, SCIFA, and SCIFB.

Dependencies:
- The patches apply to next-20160429,
- The DT bindings refer to the Generic Serial DT Bindings that were
added by "doc: DT: Add Generic Serial Device Tree Bindings", which
is in tty-testing.

This was tested on r8a7791/koelsch and r8a7740/armadillo, using UARTs
and GPIO pins on expansion connectors.

Expected behavior, using GPIO-based RTS/CTS as a reference:
- When CRTSCTS is disabled:
- Reading from a UART asserts the RTS line,
- Writing to a UART asserts the RTS line.
- When CRTSCTS is enabled:
- Reading from a UART asserts the RTS line,
- Writing to a UART asserts the RTS line.
- Writing is blocked until CTS is asserted.

Hardware-assisted hardware flow control with HSCIF and SCIFB on
r8a7791/koelsch behaves as expected when dedicated RTS/CTS modem lines
are enabled (beware the large transmission FIFOs on SCIFB ;-)

For SCIFA on r8a7740/armadillo, manually overriding (asserting) the RTS
signal doesn't seem to work. Hence when CRTSCTS is disabled, RTS is
never asserted. When CRTSCTS is enabled, hardware-assisted RTS/CTS does
control RTS as expected. As SCIFA on r8a7791 doesn't have dedicated
RTS/CTS modem lines, I cannot compare.

DT overlays for testing can be found in the topic/renesas-overlays
branch of
https://git.kernel.org/cgit/linux/kernel/git/geert/renesas-drivers.git.

Regression testing on platforms without DT and/or GPIOLIB support
(SuperH) would be appreciated.
Compile-tested on ecovec24_defconfig(GPIOLIB=y) and se7780_defconfig
(GPIOLIB=n).

Thanks!

Geert Uytterhoeven (11):
serial: sh-sci: Update DT binding documentation for GPIO modem lines
serial: sh-sci: Update DT binding documentation for dedicated RTS/CTS
serial: sh-sci: Always set TIOCM_CTS in .get_mctrl() callback
serial: sh-sci: Add support for GPIO-controlled modem lines
serial: sh-sci: Do not open-code sci_getreg()
serial: sh-sci: Add more Serial Port Register documentation
serial: sh-sci: Add more Serial Port Control/Data Register
documentation
serial: sh-sci: Correct pin initialization on (H)SCIF
serial: sh-sci: Add pin initialization for SCIFA/SCIFB
serial: sh-sci: Fix support for hardware-assisted RTS/CTS
serial: sh-sci: Add DT support for dedicated RTS/CTS

.../bindings/serial/renesas,sci-serial.txt | 4 +
drivers/tty/serial/Kconfig | 1 +
drivers/tty/serial/sh-sci.c | 177 ++++++++++++++++++---
drivers/tty/serial/sh-sci.h | 24 ++-
4 files changed, 173 insertions(+), 33 deletions(-)

--
1.9.1

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

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


2016-04-29 12:59:35

by Geert Uytterhoeven

[permalink] [raw]
Subject: [PATCH v2 10/11] serial: sh-sci: Fix support for hardware-assisted RTS/CTS

The existing support for hardware-assisted RTS/CTS is rudimentary and
doesn't work.

Add support for hardware-assisted RTS/CTS hardware flow control for the
(H)SCIF, SCIFA, and SCIFB variants.

Signed-off-by: Geert Uytterhoeven <[email protected]>
---
v2:
- New.
---
drivers/tty/serial/sh-sci.c | 90 +++++++++++++++++++++++++++++++++++++++++----
1 file changed, 83 insertions(+), 7 deletions(-)

diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
index b9d027af0f3e71f6..02b240a02dc6a593 100644
--- a/drivers/tty/serial/sh-sci.c
+++ b/drivers/tty/serial/sh-sci.c
@@ -141,6 +141,8 @@ struct sci_port {
struct timer_list rx_timer;
unsigned int rx_timeout;
#endif
+
+ bool autorts;
};

#define SCI_NPORTS CONFIG_SERIAL_SH_SCI_NR_UARTS
@@ -1811,6 +1813,46 @@ static unsigned int sci_tx_empty(struct uart_port *port)
return (status & SCxSR_TEND(port)) && !in_tx_fifo ? TIOCSER_TEMT : 0;
}

+static void sci_set_rts(struct uart_port *port, bool state)
+{
+ if (port->type == PORT_SCIFA || port->type == PORT_SCIFB) {
+ u16 data = serial_port_in(port, SCPDR);
+
+ /* Active low */
+ if (state)
+ data &= ~SCPDR_RTSD;
+ else
+ data |= SCPDR_RTSD;
+ serial_port_out(port, SCPDR, data);
+
+ /* RTS# is output */
+ serial_port_out(port, SCPCR,
+ serial_port_in(port, SCPCR) | SCPCR_RTSC);
+ } else if (sci_getreg(port, SCSPTR)->size) {
+ u16 ctrl = serial_port_in(port, SCSPTR);
+
+ /* Active low */
+ if (state)
+ ctrl &= ~SCSPTR_RTSDT;
+ else
+ ctrl |= SCSPTR_RTSDT;
+ serial_port_out(port, SCSPTR, ctrl);
+ }
+}
+
+static bool sci_get_cts(struct uart_port *port)
+{
+ if (port->type == PORT_SCIFA || port->type == PORT_SCIFB) {
+ /* Active low */
+ return !(serial_port_in(port, SCPDR) & SCPDR_CTSD);
+ } else if (sci_getreg(port, SCSPTR)->size) {
+ /* Active low */
+ return !(serial_port_in(port, SCSPTR) & SCSPTR_CTSDT);
+ }
+
+ return true;
+}
+
/*
* Modem control is a bit of a mixed bag for SCI(F) ports. Generally
* CTS/RTS is supported in hardware by at least one port and controlled
@@ -1841,6 +1883,31 @@ static void sci_set_mctrl(struct uart_port *port, unsigned int mctrl)
}

mctrl_gpio_set(s->gpios, mctrl);
+
+ if (!(s->cfg->capabilities & SCIx_HAVE_RTSCTS))
+ return;
+
+ if (!(mctrl & TIOCM_RTS)) {
+ /* Disable Auto RTS */
+ serial_port_out(port, SCFCR,
+ serial_port_in(port, SCFCR) & ~SCFCR_MCE);
+
+ /* Clear RTS */
+ sci_set_rts(port, 0);
+ } else if (s->autorts) {
+ if (port->type == PORT_SCIFA || port->type == PORT_SCIFB) {
+ /* Enable RTS# pin function */
+ serial_port_out(port, SCPCR,
+ serial_port_in(port, SCPCR) & ~SCPCR_RTSC);
+ }
+
+ /* Enable Auto RTS */
+ serial_port_out(port, SCFCR,
+ serial_port_in(port, SCFCR) | SCFCR_MCE);
+ } else {
+ /* Set RTS */
+ sci_set_rts(port, 1);
+ }
}

static unsigned int sci_get_mctrl(struct uart_port *port)
@@ -1853,10 +1920,14 @@ static unsigned int sci_get_mctrl(struct uart_port *port)

/*
* CTS/RTS is handled in hardware when supported, while nothing
- * else is wired up. Keep it simple and simply assert CTS/DSR/CAR.
+ * else is wired up.
*/
- if (IS_ERR_OR_NULL(mctrl_gpio_to_gpiod(gpios, UART_GPIO_CTS)))
+ if (s->autorts) {
+ if (sci_get_cts(port))
+ mctrl |= TIOCM_CTS;
+ } else if (IS_ERR_OR_NULL(mctrl_gpio_to_gpiod(gpios, UART_GPIO_CTS))) {
mctrl |= TIOCM_CTS;
+ }
if (IS_ERR_OR_NULL(mctrl_gpio_to_gpiod(gpios, UART_GPIO_DSR)))
mctrl |= TIOCM_DSR;
if (IS_ERR_OR_NULL(mctrl_gpio_to_gpiod(gpios, UART_GPIO_DCD)))
@@ -1927,6 +1998,7 @@ static void sci_shutdown(struct uart_port *port)

dev_dbg(port->dev, "%s(%d)\n", __func__, port->line);

+ s->autorts = false;
mctrl_gpio_disable_ms(to_sci_port(port)->gpios);

spin_lock_irqsave(&port->lock, flags);
@@ -2248,15 +2320,18 @@ done:

sci_init_pins(port, termios->c_cflag);

+ port->status &= ~UPSTAT_AUTOCTS;
+ s->autorts = false;
reg = sci_getreg(port, SCFCR);
if (reg->size) {
unsigned short ctrl = serial_port_in(port, SCFCR);

- if (s->cfg->capabilities & SCIx_HAVE_RTSCTS) {
- if (termios->c_cflag & CRTSCTS)
- ctrl |= SCFCR_MCE;
- else
- ctrl &= ~SCFCR_MCE;
+ if ((port->flags & UPF_HARD_FLOW) &&
+ (termios->c_cflag & CRTSCTS)) {
+ /* There is no CTS interrupt to restart the hardware */
+ port->status |= UPSTAT_AUTOCTS;
+ /* MCE is enabled when RTS is raised */
+ s->autorts = true;
}

/*
@@ -2958,6 +3033,7 @@ static int sci_probe_single(struct platform_device *dev,
dev_err(&dev->dev, "Conflicting RTS/CTS config\n");
return -EINVAL;
}
+ sciport->port.flags |= UPF_HARD_FLOW;
}

ret = uart_add_one_port(&sci_uart_driver, &sciport->port);
--
1.9.1

2016-04-29 12:59:33

by Geert Uytterhoeven

[permalink] [raw]
Subject: [PATCH v2 07/11] serial: sh-sci: Add more Serial Port Control/Data Register documentation

Improve documentation for the SCIFA/SCIFB Serial Port Control and Data
Registers:
- State clearly that the RTS and CTS lines are active-low,
- Document the bits related to the serial port's SCK, RXD, and TXD
pins.

Signed-off-by: Geert Uytterhoeven <[email protected]>
---
v2:
- New.
---
drivers/tty/serial/sh-sci.h | 14 ++++++++++----
1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/drivers/tty/serial/sh-sci.h b/drivers/tty/serial/sh-sci.h
index 85a2b81ba0a845e5..e7d2bc692a581e9e 100644
--- a/drivers/tty/serial/sh-sci.h
+++ b/drivers/tty/serial/sh-sci.h
@@ -121,12 +121,18 @@ enum {
#define HSCIF_SRE BIT(15) /* Sampling Rate Register Enable */

/* SCPCR (Serial Port Control Register), SCIFA/SCIFB only */
-#define SCPCR_RTSC BIT(4) /* Serial Port RTS Pin / Output Pin */
-#define SCPCR_CTSC BIT(3) /* Serial Port CTS Pin / Input Pin */
+#define SCPCR_RTSC BIT(4) /* Serial Port RTS# Pin / Output Pin */
+#define SCPCR_CTSC BIT(3) /* Serial Port CTS# Pin / Input Pin */
+#define SCPCR_SCKC BIT(2) /* Serial Port SCK Pin / Output Pin */
+#define SCPCR_RXDC BIT(1) /* Serial Port RXD Pin / Input Pin */
+#define SCPCR_TXDC BIT(0) /* Serial Port TXD Pin / Output Pin */

/* SCPDR (Serial Port Data Register), SCIFA/SCIFB only */
-#define SCPDR_RTSD BIT(4) /* Serial Port RTS Output Pin Data */
-#define SCPDR_CTSD BIT(3) /* Serial Port CTS Input Pin Data */
+#define SCPDR_RTSD BIT(4) /* Serial Port RTS# Output Pin Data */
+#define SCPDR_CTSD BIT(3) /* Serial Port CTS# Input Pin Data */
+#define SCPDR_SCKD BIT(2) /* Serial Port SCK Output Pin Data */
+#define SCPDR_RXDD BIT(1) /* Serial Port RXD Input Pin Data */
+#define SCPDR_TXDD BIT(0) /* Serial Port TXD Output Pin Data */

/*
* BRG Clock Select Register (Some SCIF and HSCIF)
--
1.9.1

2016-04-29 12:59:32

by Geert Uytterhoeven

[permalink] [raw]
Subject: [PATCH v2 04/11] serial: sh-sci: Add support for GPIO-controlled modem lines

Enhance the Renesas SCI UART driver to add support for GPIO-controlled
modem lines (CTS, DSR, DCD, RNG, RTS, DTR), using the serial_mctrl_gpio
helpers.

GPIO-controlled modem lines can be used when dedicated modem lines are
not available. Invalid configurations specifying both GPIO RTS/CTS and
dedicated RTS/CTS are rejected.

Signed-off-by: Geert Uytterhoeven <[email protected]>
---
Testing for regressions on platforms without DT and/or GPIOLIB support
(SuperH) would be appreciated.
Compile-tested on ecovec24_defconfig(GPIOLIB=y) and se7780_defconfig
(GPIOLIB=n).

v2:
- Drop RFC status.
- Fix typo s/UART_GPIO_CAR/UART_GPIO_DCD/ (too many different names
for the same thing: CAR, DCD, CD),
- Drop OUT[12] from the patch description,
- Use intermediate variables to avoid breaking long lines,
- Reject combining GPIO and dedicated RTS/CTS.
---
drivers/tty/serial/Kconfig | 1 +
drivers/tty/serial/sh-sci.c | 46 ++++++++++++++++++++++++++++++++++++++++++++-
2 files changed, 46 insertions(+), 1 deletion(-)

diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
index 13d4ed6caac4a78b..ebf91bbfdf0f999d 100644
--- a/drivers/tty/serial/Kconfig
+++ b/drivers/tty/serial/Kconfig
@@ -735,6 +735,7 @@ config SERIAL_SH_SCI
tristate "SuperH SCI(F) serial port support"
depends on SUPERH || ARCH_RENESAS || H8300 || COMPILE_TEST
select SERIAL_CORE
+ select SERIAL_MCTRL_GPIO if GPIOLIB

config SERIAL_SH_SCI_NR_UARTS
int "Maximum number of SCI(F) serial ports"
diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
index 135f836642ab1c5a..bf3780a7f700cad8 100644
--- a/drivers/tty/serial/sh-sci.c
+++ b/drivers/tty/serial/sh-sci.c
@@ -57,6 +57,7 @@
#include <asm/sh_bios.h>
#endif

+#include "serial_mctrl_gpio.h"
#include "sh-sci.h"

/* Offsets into the sci_port->irqs array */
@@ -111,6 +112,7 @@ struct sci_port {
unsigned int error_clear;
unsigned int sampling_rate_mask;
resource_size_t reg_size;
+ struct mctrl_gpios *gpios;

/* Break timer */
struct timer_list break_timer;
@@ -1817,6 +1819,8 @@ static unsigned int sci_tx_empty(struct uart_port *port)
*/
static void sci_set_mctrl(struct uart_port *port, unsigned int mctrl)
{
+ struct sci_port *s = to_sci_port(port);
+
if (mctrl & TIOCM_LOOP) {
const struct plat_sci_reg *reg;

@@ -1829,15 +1833,35 @@ static void sci_set_mctrl(struct uart_port *port, unsigned int mctrl)
serial_port_in(port, SCFCR) |
SCFCR_LOOP);
}
+
+ mctrl_gpio_set(s->gpios, mctrl);
}

static unsigned int sci_get_mctrl(struct uart_port *port)
{
+ struct sci_port *s = to_sci_port(port);
+ struct mctrl_gpios *gpios = s->gpios;
+ unsigned int mctrl = 0;
+
+ mctrl_gpio_get(gpios, &mctrl);
+
/*
* CTS/RTS is handled in hardware when supported, while nothing
* else is wired up. Keep it simple and simply assert CTS/DSR/CAR.
*/
- return TIOCM_CTS | TIOCM_DSR | TIOCM_CAR;
+ if (IS_ERR_OR_NULL(mctrl_gpio_to_gpiod(gpios, UART_GPIO_CTS)))
+ mctrl |= TIOCM_CTS;
+ if (IS_ERR_OR_NULL(mctrl_gpio_to_gpiod(gpios, UART_GPIO_DSR)))
+ mctrl |= TIOCM_DSR;
+ if (IS_ERR_OR_NULL(mctrl_gpio_to_gpiod(gpios, UART_GPIO_DCD)))
+ mctrl |= TIOCM_CAR;
+
+ return mctrl;
+}
+
+static void sci_enable_ms(struct uart_port *port)
+{
+ mctrl_gpio_enable_ms(to_sci_port(port)->gpios);
}

static void sci_break_ctl(struct uart_port *port, int break_state)
@@ -1899,6 +1923,8 @@ static void sci_shutdown(struct uart_port *port)

dev_dbg(port->dev, "%s(%d)\n", __func__, port->line);

+ mctrl_gpio_disable_ms(to_sci_port(port)->gpios);
+
spin_lock_irqsave(&port->lock, flags);
sci_stop_rx(port);
sci_stop_tx(port);
@@ -2300,6 +2326,9 @@ done:
sci_start_rx(port);

sci_port_disable(s);
+
+ if (UART_ENABLE_MS(port, termios->c_cflag))
+ sci_enable_ms(port);
}

static void sci_pm(struct uart_port *port, unsigned int state,
@@ -2425,6 +2454,7 @@ static struct uart_ops sci_uart_ops = {
.start_tx = sci_start_tx,
.stop_tx = sci_stop_tx,
.stop_rx = sci_stop_rx,
+ .enable_ms = sci_enable_ms,
.break_ctl = sci_break_ctl,
.startup = sci_startup,
.shutdown = sci_shutdown,
@@ -2912,6 +2942,20 @@ static int sci_probe_single(struct platform_device *dev,
if (ret)
return ret;

+ sciport->gpios = mctrl_gpio_init(&sciport->port, 0);
+ if (IS_ERR(sciport->gpios) && PTR_ERR(sciport->gpios) != -ENOSYS)
+ return PTR_ERR(sciport->gpios);
+
+ if (p->capabilities & SCIx_HAVE_RTSCTS) {
+ if (!IS_ERR_OR_NULL(mctrl_gpio_to_gpiod(sciport->gpios,
+ UART_GPIO_CTS)) ||
+ !IS_ERR_OR_NULL(mctrl_gpio_to_gpiod(sciport->gpios,
+ UART_GPIO_RTS))) {
+ dev_err(&dev->dev, "Conflicting RTS/CTS config\n");
+ return -EINVAL;
+ }
+ }
+
ret = uart_add_one_port(&sci_uart_driver, &sciport->port);
if (ret) {
sci_cleanup_single(sciport);
--
1.9.1

2016-04-29 12:59:31

by Geert Uytterhoeven

[permalink] [raw]
Subject: [PATCH v2 01/11] serial: sh-sci: Update DT binding documentation for GPIO modem lines

Amend the DT bindings for the Renesas SCI driver to allow describing
optional GPIO-controlled modem lines, which can be used where dedicated
modem lines are not available.

The property naming is dictated by the Generic Serial DT Bindings.

Signed-off-by: Geert Uytterhoeven <[email protected]>
Acked-by: Rob Herring <[email protected]>
Cc: [email protected]
---
v2:
- Add Acked-by,
- Drop RFC status,
- Refer to the Generic Serial DT Bindings,
- Drop out[12]-gpios.
---
Documentation/devicetree/bindings/serial/renesas,sci-serial.txt | 2 ++
1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/serial/renesas,sci-serial.txt b/Documentation/devicetree/bindings/serial/renesas,sci-serial.txt
index 528c3b90f23cb04b..997baeaa869baa6f 100644
--- a/Documentation/devicetree/bindings/serial/renesas,sci-serial.txt
+++ b/Documentation/devicetree/bindings/serial/renesas,sci-serial.txt
@@ -76,6 +76,8 @@ Optional properties:
- dmas: Must contain a list of two references to DMA specifiers, one for
transmission, and one for reception.
- dma-names: Must contain a list of two DMA names, "tx" and "rx".
+ - {cts,dsr,dcd,rng,rts,dtr}-gpios: Specify GPIOs for modem lines, cfr. the
+ generic serial DT bindings in serial.txt.

Example:
aliases {
--
1.9.1

2016-04-29 13:00:59

by Geert Uytterhoeven

[permalink] [raw]
Subject: [PATCH v2 08/11] serial: sh-sci: Correct pin initialization on (H)SCIF

Correct pin initialization on (H)SCIF:
- RTS must be deasserted (it's active low),
- SCK must be an input, as it may be used as the optional external
clock input.

Initial pin configuration must always be done:
- Regardless of the presence of dedicated RTS and CTS pins: if the
register exists, the RTS/CTS bits exist, too,
- Regardless of hardware flow control being enabled or not: RTS must
be deasserted.

Signed-off-by: Geert Uytterhoeven <[email protected]>
---
v2:
- New.
---
drivers/tty/serial/sh-sci.c | 23 ++++++++---------------
1 file changed, 8 insertions(+), 15 deletions(-)

diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
index ce7bd165929ea078..c46999f20917964e 100644
--- a/drivers/tty/serial/sh-sci.c
+++ b/drivers/tty/serial/sh-sci.c
@@ -712,21 +712,14 @@ static void sci_init_pins(struct uart_port *port, unsigned int cflag)
return;
}

- /*
- * For the generic path SCSPTR is necessary. Bail out if that's
- * unavailable, too.
- */
- if (!sci_getreg(port, SCSPTR)->size)
- return;
-
- if ((s->cfg->capabilities & SCIx_HAVE_RTSCTS) &&
- ((!(cflag & CRTSCTS)))) {
- unsigned short status;
-
- status = serial_port_in(port, SCSPTR);
- status &= ~SCSPTR_CTSIO;
- status |= SCSPTR_RTSIO;
- serial_port_out(port, SCSPTR, status); /* Set RTS = 1 */
+ if (sci_getreg(port, SCSPTR)->size) {
+ u16 status = serial_port_in(port, SCSPTR);
+
+ /* RTS# is output, driven 1 */
+ status |= SCSPTR_RTSIO | SCSPTR_RTSDT;
+ /* CTS# and SCK are inputs */
+ status &= ~(SCSPTR_CTSIO | SCSPTR_SCKIO);
+ serial_port_out(port, SCSPTR, status);
}
}

--
1.9.1

2016-04-29 13:00:56

by Geert Uytterhoeven

[permalink] [raw]
Subject: [PATCH v2 05/11] serial: sh-sci: Do not open-code sci_getreg()

Replace open-coded variants of sci_getreg() by function calls, and drop
intermediate variables where appropriate.

Signed-off-by: Geert Uytterhoeven <[email protected]>
---
v2:
- New.
---
drivers/tty/serial/sh-sci.c | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
index bf3780a7f700cad8..ce7bd165929ea078 100644
--- a/drivers/tty/serial/sh-sci.c
+++ b/drivers/tty/serial/sh-sci.c
@@ -703,7 +703,6 @@ static void sci_poll_put_char(struct uart_port *port, unsigned char c)
static void sci_init_pins(struct uart_port *port, unsigned int cflag)
{
struct sci_port *s = to_sci_port(port);
- const struct plat_sci_reg *reg = sci_regmap[s->cfg->regtype] + SCSPTR;

/*
* Use port-specific handler if provided.
@@ -717,7 +716,7 @@ static void sci_init_pins(struct uart_port *port, unsigned int cflag)
* For the generic path SCSPTR is necessary. Bail out if that's
* unavailable, too.
*/
- if (!reg->size)
+ if (!sci_getreg(port, SCSPTR)->size)
return;

if ((s->cfg->capabilities & SCIx_HAVE_RTSCTS) &&
@@ -1866,12 +1865,10 @@ static void sci_enable_ms(struct uart_port *port)

static void sci_break_ctl(struct uart_port *port, int break_state)
{
- struct sci_port *s = to_sci_port(port);
- const struct plat_sci_reg *reg = sci_regmap[s->cfg->regtype] + SCSPTR;
unsigned short scscr, scsptr;

/* check wheter the port has SCSPTR */
- if (!reg->size) {
+ if (!sci_getreg(port, SCSPTR)->size) {
/*
* Not supported by hardware. Most parts couple break and rx
* interrupts together, with break detection always enabled.
--
1.9.1

2016-04-29 13:04:30

by Geert Uytterhoeven

[permalink] [raw]
Subject: [PATCH v2 03/11] serial: sh-sci: Always set TIOCM_CTS in .get_mctrl() callback

Documentation/serial/driver clearly states:

If the port does not support CTS, DCD or DSR, the driver should
indicate that the signal is permanently active.

Hence always set TIOCM_CTS, as we currently don't look at the CTS
hardware line state at all.

FWIW, this fixes the transmit path when hardware-assisted flow control
is enabled, and userspace enables CRTSCTS.
The receive path is still broken, as RTS is never asserted.

Signed-off-by: Geert Uytterhoeven <[email protected]>
---
v2:
- Drop RFC status.
---
drivers/tty/serial/sh-sci.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
index 0130feb069aee02f..135f836642ab1c5a 100644
--- a/drivers/tty/serial/sh-sci.c
+++ b/drivers/tty/serial/sh-sci.c
@@ -1835,9 +1835,9 @@ static unsigned int sci_get_mctrl(struct uart_port *port)
{
/*
* CTS/RTS is handled in hardware when supported, while nothing
- * else is wired up. Keep it simple and simply assert DSR/CAR.
+ * else is wired up. Keep it simple and simply assert CTS/DSR/CAR.
*/
- return TIOCM_DSR | TIOCM_CAR;
+ return TIOCM_CTS | TIOCM_DSR | TIOCM_CAR;
}

static void sci_break_ctl(struct uart_port *port, int break_state)
--
1.9.1

2016-04-29 13:05:07

by Geert Uytterhoeven

[permalink] [raw]
Subject: [PATCH v2 09/11] serial: sh-sci: Add pin initialization for SCIFA/SCIFB

Before, the driver relied on initialization by the boot loader, or by
implicit reset state.

Note that unlike on (H)SCIF, the RTS/CTS bits exist only if dedicated
RTS/CTS pins are available, which depends on the SoC and UART instance.

Signed-off-by: Geert Uytterhoeven <[email protected]>
---
v2:
- New.
---
drivers/tty/serial/sh-sci.c | 16 +++++++++++++++-
1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
index c46999f20917964e..b9d027af0f3e71f6 100644
--- a/drivers/tty/serial/sh-sci.c
+++ b/drivers/tty/serial/sh-sci.c
@@ -712,7 +712,21 @@ static void sci_init_pins(struct uart_port *port, unsigned int cflag)
return;
}

- if (sci_getreg(port, SCSPTR)->size) {
+ if (port->type == PORT_SCIFA || port->type == PORT_SCIFB) {
+ u16 ctrl = serial_port_in(port, SCPCR);
+
+ /* Enable RXD and TXD pin functions */
+ ctrl &= ~(SCPCR_RXDC | SCPCR_TXDC);
+ if (to_sci_port(port)->cfg->capabilities & SCIx_HAVE_RTSCTS) {
+ /* RTS# is output, driven 1 */
+ ctrl |= SCPCR_RTSC;
+ serial_port_out(port, SCPDR,
+ serial_port_in(port, SCPDR) | SCPDR_RTSD);
+ /* Enable CTS# pin function */
+ ctrl &= ~SCPCR_CTSC;
+ }
+ 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 */
--
1.9.1

2016-04-29 13:05:06

by Geert Uytterhoeven

[permalink] [raw]
Subject: [PATCH v2 11/11] serial: sh-sci: Add DT support for dedicated RTS/CTS

Add support for indicating the availability of dedicated lines for
RTS/CTS hardware flow control, using the standard "uart-has-rtscts" DT
property.

Signed-off-by: Geert Uytterhoeven <[email protected]>
---
v2:
- New.
---
drivers/tty/serial/sh-sci.c | 3 +++
1 file changed, 3 insertions(+)

diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
index 02b240a02dc6a593..d9cb0d70fceef07d 100644
--- a/drivers/tty/serial/sh-sci.c
+++ b/drivers/tty/serial/sh-sci.c
@@ -2999,6 +2999,9 @@ sci_parse_dt(struct platform_device *pdev, unsigned int *dev_id)
p->regtype = SCI_OF_REGTYPE(match->data);
p->scscr = SCSCR_RE | SCSCR_TE;

+ if (of_find_property(np, "uart-has-rtscts", NULL))
+ p->capabilities |= SCIx_HAVE_RTSCTS;
+
return p;
}

--
1.9.1

2016-04-29 13:05:04

by Geert Uytterhoeven

[permalink] [raw]
Subject: [PATCH v2 02/11] serial: sh-sci: Update DT binding documentation for dedicated RTS/CTS

Some Renesas SCIF UARTs have dedicated lines for RTS/CTS hardware flow
control. Whether these lines exist depends on SoC and UART instance
inside the SoC. Whether these lines can be used for hardware flow
control depends on board wiring.

Amend the DT bindings with an optional property to indicate that RTS/CTS
hardware flow control lines exist, and can be used as such, according to
the Generic Serial DT Bindings.

Signed-off-by: Geert Uytterhoeven <[email protected]>
Cc: [email protected]
---
v2:
- Drop RFC status,
- Use "uart-has-rtscts" instead of "renesas,uart-has-rtscts",
- Refer to the Generic Serial DT Bindings.
---
Documentation/devicetree/bindings/serial/renesas,sci-serial.txt | 2 ++
1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/serial/renesas,sci-serial.txt b/Documentation/devicetree/bindings/serial/renesas,sci-serial.txt
index 997baeaa869baa6f..7a0e15053db97481 100644
--- a/Documentation/devicetree/bindings/serial/renesas,sci-serial.txt
+++ b/Documentation/devicetree/bindings/serial/renesas,sci-serial.txt
@@ -78,6 +78,8 @@ Optional properties:
- dma-names: Must contain a list of two DMA names, "tx" and "rx".
- {cts,dsr,dcd,rng,rts,dtr}-gpios: Specify GPIOs for modem lines, cfr. the
generic serial DT bindings in serial.txt.
+ - uart-has-rtscts: Indicates dedicated lines for RTS/CTS hardware flow
+ control, cfr. the generic serial DT bindings in serial.txt.

Example:
aliases {
--
1.9.1

2016-04-29 13:05:00

by Geert Uytterhoeven

[permalink] [raw]
Subject: [PATCH v2 06/11] serial: sh-sci: Add more Serial Port Register documentation

Improve documentation for the (H)SCIF Serial Port Register:
- Make it clear the RTS and CTS lines are active-low,
- Document the bits related to the serial port's clock pin.

Signed-off-by: Geert Uytterhoeven <[email protected]>
---
v2:
- New.
---
drivers/tty/serial/sh-sci.h | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/tty/serial/sh-sci.h b/drivers/tty/serial/sh-sci.h
index 7a4fa185b93ef307..85a2b81ba0a845e5 100644
--- a/drivers/tty/serial/sh-sci.h
+++ b/drivers/tty/serial/sh-sci.h
@@ -108,10 +108,12 @@ enum {
#define SCLSR_ORER BIT(0) /* Overrun Error */

/* SCSPTR (Serial Port Register), optional */
-#define SCSPTR_RTSIO BIT(7) /* Serial Port RTS Pin Input/Output */
-#define SCSPTR_RTSDT BIT(6) /* Serial Port RTS Pin Data */
-#define SCSPTR_CTSIO BIT(5) /* Serial Port CTS Pin Input/Output */
-#define SCSPTR_CTSDT BIT(4) /* Serial Port CTS Pin Data */
+#define SCSPTR_RTSIO BIT(7) /* Serial Port RTS# Pin Input/Output */
+#define SCSPTR_RTSDT BIT(6) /* Serial Port RTS# Pin Data */
+#define SCSPTR_CTSIO BIT(5) /* Serial Port CTS# Pin Input/Output */
+#define SCSPTR_CTSDT BIT(4) /* Serial Port CTS# Pin Data */
+#define SCSPTR_SCKIO BIT(3) /* Serial Port Clock Pin Input/Output */
+#define SCSPTR_SCKDT BIT(2) /* Serial Port Clock Pin Data */
#define SCSPTR_SPB2IO BIT(1) /* Serial Port Break Input/Output */
#define SCSPTR_SPB2DT BIT(0) /* Serial Port Break Data */

--
1.9.1

2016-04-29 15:38:23

by Peter Hurley

[permalink] [raw]
Subject: Re: [PATCH v2 03/11] serial: sh-sci: Always set TIOCM_CTS in .get_mctrl() callback

On 04/29/2016 05:58 AM, Geert Uytterhoeven wrote:
> Documentation/serial/driver clearly states:
>
> If the port does not support CTS, DCD or DSR, the driver should
> indicate that the signal is permanently active.
>
> Hence always set TIOCM_CTS, as we currently don't look at the CTS
> hardware line state at all.
>
> FWIW, this fixes the transmit path when hardware-assisted flow control
> is enabled, and userspace enables CRTSCTS.
> The receive path is still broken, as RTS is never asserted.

Reviewed-by: Peter Hurley <[email protected]>

2016-04-29 15:39:47

by Peter Hurley

[permalink] [raw]
Subject: Re: [PATCH v2 05/11] serial: sh-sci: Do not open-code sci_getreg()

On 04/29/2016 05:58 AM, Geert Uytterhoeven wrote:
> Replace open-coded variants of sci_getreg() by function calls, and drop
> intermediate variables where appropriate.

Reviewed-by: Peter Hurley <[email protected]>

2016-04-29 15:39:37

by Peter Hurley

[permalink] [raw]
Subject: Re: [PATCH v2 04/11] serial: sh-sci: Add support for GPIO-controlled modem lines

On 04/29/2016 05:58 AM, Geert Uytterhoeven wrote:
> Enhance the Renesas SCI UART driver to add support for GPIO-controlled
> modem lines (CTS, DSR, DCD, RNG, RTS, DTR), using the serial_mctrl_gpio
> helpers.
>
> GPIO-controlled modem lines can be used when dedicated modem lines are
> not available. Invalid configurations specifying both GPIO RTS/CTS and
> dedicated RTS/CTS are rejected.

Reviewed-by: Peter Hurley <[email protected]>

2016-04-29 15:41:17

by Peter Hurley

[permalink] [raw]
Subject: Re: [PATCH v2 06/11] serial: sh-sci: Add more Serial Port Register documentation

On 04/29/2016 05:58 AM, Geert Uytterhoeven wrote:
> Improve documentation for the (H)SCIF Serial Port Register:
> - Make it clear the RTS and CTS lines are active-low,
> - Document the bits related to the serial port's clock pin.

Reviewed-by: Peter Hurley <[email protected]>

2016-04-29 15:42:24

by Peter Hurley

[permalink] [raw]
Subject: Re: [PATCH v2 07/11] serial: sh-sci: Add more Serial Port Control/Data Register documentation

On 04/29/2016 05:58 AM, Geert Uytterhoeven wrote:
> Improve documentation for the SCIFA/SCIFB Serial Port Control and Data
> Registers:
> - State clearly that the RTS and CTS lines are active-low,
> - Document the bits related to the serial port's SCK, RXD, and TXD
> pins.

Reviewed-by: Peter Hurley <[email protected]>

2016-04-29 15:59:45

by Peter Hurley

[permalink] [raw]
Subject: Re: [PATCH v2 08/11] serial: sh-sci: Correct pin initialization on (H)SCIF

On 04/29/2016 05:58 AM, Geert Uytterhoeven wrote:
> Correct pin initialization on (H)SCIF:
> - RTS must be deasserted (it's active low),
> - SCK must be an input, as it may be used as the optional external
> clock input.
>
> Initial pin configuration must always be done:
> - Regardless of the presence of dedicated RTS and CTS pins: if the
> register exists, the RTS/CTS bits exist, too,
> - Regardless of hardware flow control being enabled or not: RTS must
> be deasserted.

This is always setting RTS active; why?

Normally you want to program RTS only in response to ->set_mctrl()
from serial core. For example, this will set RTS active even though
baud might be set to B0.


> Signed-off-by: Geert Uytterhoeven <[email protected]>
> ---
> v2:
> - New.
> ---
> drivers/tty/serial/sh-sci.c | 23 ++++++++---------------
> 1 file changed, 8 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
> index ce7bd165929ea078..c46999f20917964e 100644
> --- a/drivers/tty/serial/sh-sci.c
> +++ b/drivers/tty/serial/sh-sci.c
> @@ -712,21 +712,14 @@ static void sci_init_pins(struct uart_port *port, unsigned int cflag)
> return;
> }
>
> - /*
> - * For the generic path SCSPTR is necessary. Bail out if that's
> - * unavailable, too.
> - */
> - if (!sci_getreg(port, SCSPTR)->size)
> - return;
> -
> - if ((s->cfg->capabilities & SCIx_HAVE_RTSCTS) &&
> - ((!(cflag & CRTSCTS)))) {
> - unsigned short status;
> -
> - status = serial_port_in(port, SCSPTR);
> - status &= ~SCSPTR_CTSIO;
> - status |= SCSPTR_RTSIO;
> - serial_port_out(port, SCSPTR, status); /* Set RTS = 1 */
> + if (sci_getreg(port, SCSPTR)->size) {
> + u16 status = serial_port_in(port, SCSPTR);
> +
> + /* RTS# is output, driven 1 */
> + status |= SCSPTR_RTSIO | SCSPTR_RTSDT;
> + /* CTS# and SCK are inputs */
> + status &= ~(SCSPTR_CTSIO | SCSPTR_SCKIO);
> + serial_port_out(port, SCSPTR, status);
> }
> }
>
>

2016-04-29 20:00:09

by Geert Uytterhoeven

[permalink] [raw]
Subject: Re: [PATCH v2 08/11] serial: sh-sci: Correct pin initialization on (H)SCIF

Hi Peter,

On Fri, Apr 29, 2016 at 5:59 PM, Peter Hurley <[email protected]> wrote:
> On 04/29/2016 05:58 AM, Geert Uytterhoeven wrote:
>> Correct pin initialization on (H)SCIF:
>> - RTS must be deasserted (it's active low),
>> - SCK must be an input, as it may be used as the optional external
>> clock input.
>>
>> Initial pin configuration must always be done:
>> - Regardless of the presence of dedicated RTS and CTS pins: if the
>> register exists, the RTS/CTS bits exist, too,
>> - Regardless of hardware flow control being enabled or not: RTS must
>> be deasserted.
>
> This is always setting RTS active; why?

No, it deasserts RTS. RTS is active-low, so setting the pin state to 1/high
(setting the SCSPTR_RTSDT bit), makes RTS inactive.

Before, the code didn't set the SCSPTR_RTSDT bit, so the pin might have
been low, i.e. RTS may have been active.

> Normally you want to program RTS only in response to ->set_mctrl()
> from serial core. For example, this will set RTS active even though
> baud might be set to B0.

Yes, that's also my understanding.

>> Signed-off-by: Geert Uytterhoeven <[email protected]>
>> ---
>> v2:
>> - New.
>> ---
>> drivers/tty/serial/sh-sci.c | 23 ++++++++---------------
>> 1 file changed, 8 insertions(+), 15 deletions(-)
>>
>> diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
>> index ce7bd165929ea078..c46999f20917964e 100644
>> --- a/drivers/tty/serial/sh-sci.c
>> +++ b/drivers/tty/serial/sh-sci.c
>> @@ -712,21 +712,14 @@ static void sci_init_pins(struct uart_port *port, unsigned int cflag)
>> return;
>> }
>>
>> - /*
>> - * For the generic path SCSPTR is necessary. Bail out if that's
>> - * unavailable, too.
>> - */
>> - if (!sci_getreg(port, SCSPTR)->size)
>> - return;
>> -
>> - if ((s->cfg->capabilities & SCIx_HAVE_RTSCTS) &&
>> - ((!(cflag & CRTSCTS)))) {
>> - unsigned short status;
>> -
>> - status = serial_port_in(port, SCSPTR);
>> - status &= ~SCSPTR_CTSIO;
>> - status |= SCSPTR_RTSIO;
>> - serial_port_out(port, SCSPTR, status); /* Set RTS = 1 */
>> + if (sci_getreg(port, SCSPTR)->size) {
>> + u16 status = serial_port_in(port, SCSPTR);
>> +
>> + /* RTS# is output, driven 1 */
>> + status |= SCSPTR_RTSIO | SCSPTR_RTSDT;
>> + /* CTS# and SCK are inputs */
>> + status &= ~(SCSPTR_CTSIO | SCSPTR_SCKIO);
>> + serial_port_out(port, SCSPTR, status);
>> }
>> }

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

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