[PATCH 0/7] Renesas *SCIF* RX FIFO support

[PATCH 0/7] Renesas *SCIF* RX FIFO support

[Ulrich Hecht]

Hi!

This series implements support for using RX FIFO thresholds higher than one
in PIO mode on SCIF, HSCIF, SCIFA and SCIFB serial ports.

This was originally implemented to fulfill a request for a software
timer-based solution, but I have found that it is not sensible to implement
such a feature on SCIF and HSCIF. These IPs do not support disabling of the
hardware timeout feature that all SCIFs have, and that triggers an interrupt
if no new data has arrived for more than 1.5 frames.

This series therefore enables the hardware timeout for SCIFs and HSCIFs, and
the software timeout for SCIFAs and SCIFBs only.

(Hardware timeout on SCIFA/B is possible, but has not been implemented here
due to a lack of time. The same goes for the limited amount of tuning of
the hardware timeout that is possible on HSCIF ports.)

Two sysfs attributes are added to tune the FIFO behavior:

- rx_fifo_trigger: Controls the trigger level at which an interrupt is
  forced. Works for both hardware and software timeout.
- rx_fifo_timeout: Software timeout in frames upon which the FIFOs are
  checked even if no interrupt is triggered. Zero (default) disables the
  timeout. (SCIFA and SCIFB only)

CU
Uli


Ulrich Hecht (7):
  serial: sh-sci: add FIFO trigger bits
  serial: sh-sci: consider DR (data ready) bit adequately
  serial: sh-sci: implement FIFO threshold register setting
  serial: sh-sci: increase RX FIFO trigger defaults for (H)SCIF
  serial: sh-sci: SCIFA/B RX FIFO software timeout
  serial: sh-sci: make RX FIFO parameters tunable via sysfs
  arm64: dts: r8a7796: salvator-x: add SCIF1 (DEBUG1)

 arch/arm64/boot/dts/renesas/r8a7796-salvator-x.dts |  12 +
 drivers/tty/serial/sh-sci.c                        | 254 ++++++++++++++++++---
 drivers/tty/serial/sh-sci.h                        |   7 +
 3 files changed, 238 insertions(+), 35 deletions(-)

-- 
2.7.4

[PATCH 1/7] serial: sh-sci: add FIFO trigger bits

[Ulrich Hecht]

Defines the bits controlling FIFO thresholds, adds the additional
HSCIF registers to the register map.

Signed-off-by: Ulrich Hecht <ulrich.hecht+renesas@gmail.com>
---
 drivers/tty/serial/sh-sci.c | 2 ++
 drivers/tty/serial/sh-sci.h | 6 ++++++
 2 files changed, 8 insertions(+)

diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
index 4b26252..385afbe 100644
--- a/drivers/tty/serial/sh-sci.c
+++ b/drivers/tty/serial/sh-sci.c
@@ -377,6 +377,8 @@ static const struct plat_sci_reg sci_regmap[SCIx_NR_REGTYPES][SCIx_NR_REGS] = {
 		[SCPDR]		= sci_reg_invalid,
 		[SCDL]		= { 0x30, 16 },
 		[SCCKS]		= { 0x34, 16 },
+		[HSRTRGR]	= { 0x54, 16 },
+		[HSTTRGR]	= { 0x58, 16 },
 	},
 
 	/*
diff --git a/drivers/tty/serial/sh-sci.h b/drivers/tty/serial/sh-sci.h
index ffa6d68..f51f919 100644
--- a/drivers/tty/serial/sh-sci.h
+++ b/drivers/tty/serial/sh-sci.h
@@ -29,6 +29,8 @@ enum {
 	SCPDR,				/* Serial Port Data Register */
 	SCDL,				/* BRG Frequency Division Register */
 	SCCKS,				/* BRG Clock Select Register */
+	HSRTRGR,			/* Receive FIFO Data Count Register */
+	HSTTRGR,			/* Transmit FIFO Data Count Register */
 
 	SCIx_NR_REGS,
 };
@@ -99,6 +101,10 @@ enum {
 #define SCIF_BREAK_CLEAR	(u32)(~(SCIF_PER | SCIF_FER | SCIF_BRK))
 
 /* SCFCR (FIFO Control Register) */
+#define SCFCR_RTRG1	BIT(7)	/* Receive FIFO Data Count Trigger */
+#define SCFCR_RTRG0	BIT(6)
+#define SCFCR_TTRG1	BIT(5)	/* Transmit FIFO Data Count Trigger */
+#define SCFCR_TTRG0	BIT(4)
 #define SCFCR_MCE	BIT(3)	/* Modem Control Enable */
 #define SCFCR_TFRST	BIT(2)	/* Transmit FIFO Data Register Reset */
 #define SCFCR_RFRST	BIT(1)	/* Receive FIFO Data Register Reset */
-- 
2.7.4

Re: [PATCH 1/7] serial: sh-sci: add FIFO trigger bits

[Geert Uytterhoeven]

Hi Ulrich,

On Fri, Dec 9, 2016 at 1:36 PM, Ulrich Hecht
<ulrich.hecht+renesas@gmail.com> wrote:
> Defines the bits controlling FIFO thresholds, adds the additional
> HSCIF registers to the register map.
>
> Signed-off-by: Ulrich Hecht <ulrich.hecht+renesas@gmail.com>

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

Minor comment below.

> --- a/drivers/tty/serial/sh-sci.h
> +++ b/drivers/tty/serial/sh-sci.h
> @@ -29,6 +29,8 @@ enum {
>         SCPDR,                          /* Serial Port Data Register */
>         SCDL,                           /* BRG Frequency Division Register */
>         SCCKS,                          /* BRG Clock Select Register */
> +       HSRTRGR,                        /* Receive FIFO Data Count Register */

Receive FIFO Data Count Trigger Register

(to avoid confusion with the FIFO Data Count Register)

> +       HSTTRGR,                        /* Transmit FIFO Data Count Register */

Transmit FIFO Data Count Trigger Register

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 2/7] serial: sh-sci: consider DR (data ready) bit adequately

[Ulrich Hecht]

To allow operation with a higher RX FIFO interrupt threshold in PIO
mode, it is necessary to consider the DR bit ("FIFO not full, but no
data received for 1.5 frames") as an indicator that data can be read.
Otherwise the driver will let data rot in the FIFO until the threshold
is reached.

Signed-off-by: Ulrich Hecht <ulrich.hecht+renesas@gmail.com>
---
 drivers/tty/serial/sh-sci.c | 9 +++++----
 drivers/tty/serial/sh-sci.h | 1 +
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
index 385afbe..de25db8 100644
--- a/drivers/tty/serial/sh-sci.c
+++ b/drivers/tty/serial/sh-sci.c
@@ -677,7 +677,7 @@ static int sci_poll_get_char(struct uart_port *port)
 		break;
 	} while (1);
 
-	if (!(status & SCxSR_RDxF(port)))
+	if (!(status & (SCxSR_RDxF(port) | SCxSR_DR(port))))
 		return NO_POLL_CHAR;
 
 	c = serial_port_in(port, SCxRDR);
@@ -773,7 +773,8 @@ static int sci_rxfill(struct uart_port *port)
 	if (reg->size)
 		return serial_port_in(port, SCFDR) & ((port->fifosize << 1) - 1);
 
-	return (serial_port_in(port, SCxSR) & SCxSR_RDxF(port)) != 0;
+	return (serial_port_in(port, SCxSR) &
+		(SCxSR_RDxF(port) | SCxSR_DR(port))) != 0;
 }
 
 /*
@@ -864,7 +865,7 @@ static void sci_receive_chars(struct uart_port *port)
 	unsigned char flag;
 
 	status = serial_port_in(port, SCxSR);
-	if (!(status & SCxSR_RDxF(port)))
+	if (!(status & (SCxSR_RDxF(port) | SCxSR_DR(port))))
 		return;
 
 	while (1) {
@@ -1672,7 +1673,7 @@ static irqreturn_t sci_mpxed_interrupt(int irq, void *ptr)
 	 * Rx Interrupt: if we're using DMA, the DMA controller clears RDF /
 	 * DR flags
 	 */
-	if (((ssr_status & SCxSR_RDxF(port)) || s->chan_rx) &&
+	if (((ssr_status & (SCxSR_RDxF(port) | SCxSR_DR(port))) || s->chan_rx) &&
 	    (scr_status & SCSCR_RIE))
 		ret = sci_rx_interrupt(irq, ptr);
 
diff --git a/drivers/tty/serial/sh-sci.h b/drivers/tty/serial/sh-sci.h
index f51f919..44997a5 100644
--- a/drivers/tty/serial/sh-sci.h
+++ b/drivers/tty/serial/sh-sci.h
@@ -156,6 +156,7 @@ enum {
 #define SCxSR_FER(port)		(((port)->type == PORT_SCI) ? SCI_FER    : SCIF_FER)
 #define SCxSR_PER(port)		(((port)->type == PORT_SCI) ? SCI_PER    : SCIF_PER)
 #define SCxSR_BRK(port)		(((port)->type == PORT_SCI) ? 0x00       : SCIF_BRK)
+#define SCxSR_DR(port)		(((port)->type == PORT_SCI) ? 0x00       : SCIF_DR)
 
 #define SCxSR_ERRORS(port)	(to_sci_port(port)->error_mask)
 
-- 
2.7.4

Re: [PATCH 2/7] serial: sh-sci: consider DR (data ready) bit adequately

[Geert Uytterhoeven]

Hi Uli,

On Fri, Dec 9, 2016 at 1:36 PM, Ulrich Hecht
<ulrich.hecht+renesas@gmail.com> wrote:
> To allow operation with a higher RX FIFO interrupt threshold in PIO
> mode, it is necessary to consider the DR bit ("FIFO not full, but no
> data received for 1.5 frames") as an indicator that data can be read.
> Otherwise the driver will let data rot in the FIFO until the threshold
> is reached.
>
> Signed-off-by: Ulrich Hecht <ulrich.hecht+renesas@gmail.com>

> --- a/drivers/tty/serial/sh-sci.c
> +++ b/drivers/tty/serial/sh-sci.c
> @@ -677,7 +677,7 @@ static int sci_poll_get_char(struct uart_port *port)
>                 break;
>         } while (1);
>
> -       if (!(status & SCxSR_RDxF(port)))
> +       if (!(status & (SCxSR_RDxF(port) | SCxSR_DR(port))))
>                 return NO_POLL_CHAR;
>
>         c = serial_port_in(port, SCxRDR);

> --- a/drivers/tty/serial/sh-sci.h
> +++ b/drivers/tty/serial/sh-sci.h
> @@ -156,6 +156,7 @@ enum {
>  #define SCxSR_FER(port)                (((port)->type == PORT_SCI) ? SCI_FER    : SCIF_FER)
>  #define SCxSR_PER(port)                (((port)->type == PORT_SCI) ? SCI_PER    : SCIF_PER)
>  #define SCxSR_BRK(port)                (((port)->type == PORT_SCI) ? 0x00       : SCIF_BRK)
> +#define SCxSR_DR(port)         (((port)->type == PORT_SCI) ? 0x00       : SCIF_DR)

Makes sense, as SCIF_RDxF_CLEAR already includes SCIF_DR, and we thus already
clear both RDF and DR.

However, if you would handle this inside the SCxSR_RDxF() macro, your patch
would reduce to a single line:

-#define SCxSR_RDxF(port)        (((port)->type == PORT_SCI) ?
SCI_RDRF   : SCIF_RDF)
+#define SCxSR_RDxF(port)        (((port)->type == PORT_SCI) ?
SCI_RDRF   : SCIF_DR | SCIF_RDF)

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 3/7] serial: sh-sci: implement FIFO threshold register setting

[Ulrich Hecht]

Sets the closest match for a desired RX trigger level.

Signed-off-by: Ulrich Hecht <ulrich.hecht+renesas@gmail.com>
---
 drivers/tty/serial/sh-sci.c | 55 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 55 insertions(+)

diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
index de25db8..844288f 100644
--- a/drivers/tty/serial/sh-sci.c
+++ b/drivers/tty/serial/sh-sci.c
@@ -1103,6 +1103,61 @@ static int sci_handle_breaks(struct uart_port *port)
 	return copied;
 }
 
+static int scif_set_rtrg(struct uart_port *port, int rx_trig)
+{
+	unsigned int bits;
+
+	if (rx_trig < 1)
+		rx_trig = 1;
+	if (rx_trig >= port->fifosize)
+		rx_trig = port->fifosize;
+
+	/* HSCIF can be set to an arbitrary level. */
+	if (sci_getreg(port, HSRTRGR)->size) {
+		serial_port_out(port, HSRTRGR, rx_trig);
+		return rx_trig;
+	}
+
+	if (port->type == PORT_SCIF) {
+		if (rx_trig < 4) {
+			bits = 0;
+			rx_trig = 1;
+		} else if (rx_trig < 8) {
+			bits = SCFCR_RTRG0;
+			rx_trig = 4;
+		} else if (rx_trig < 14) {
+			bits = SCFCR_RTRG1;
+			rx_trig = 8;
+		} else {
+			bits = SCFCR_RTRG0 | SCFCR_RTRG1;
+			rx_trig = 14;
+		}
+	} else if (port->type == PORT_SCIFA || port->type == PORT_SCIFB) {
+		if (rx_trig < 16) {
+			bits = 0;
+			rx_trig = 1;
+		} else if (rx_trig < 32) {
+			bits = SCFCR_RTRG0;
+			rx_trig = 16;
+		} else if (rx_trig < 48) {
+			bits = SCFCR_RTRG1;
+			rx_trig = 32;
+		} else {
+			bits = SCFCR_RTRG0 | SCFCR_RTRG1;
+			rx_trig = 48;
+		}
+	} else {
+		WARN(1, "unknown FIFO configuration");
+		return 1;
+	}
+
+	serial_port_out(port, SCFCR,
+		(serial_port_in(port, SCFCR) &
+		~(SCFCR_RTRG1 | SCFCR_RTRG0)) | bits);
+
+	return rx_trig;
+}
+
 #ifdef CONFIG_SERIAL_SH_SCI_DMA
 static void sci_dma_tx_complete(void *arg)
 {
-- 
2.7.4

Re: [PATCH 3/7] serial: sh-sci: implement FIFO threshold register setting

[Sergei Shtylyov]

Hello!

On 12/9/2016 3:36 PM, Ulrich Hecht wrote:

> Sets the closest match for a desired RX trigger level.
>
> Signed-off-by: Ulrich Hecht <ulrich.hecht+renesas@gmail.com>
> ---
>  drivers/tty/serial/sh-sci.c | 55 +++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 55 insertions(+)
>
> diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
> index de25db8..844288f 100644
> --- a/drivers/tty/serial/sh-sci.c
> +++ b/drivers/tty/serial/sh-sci.c
> @@ -1103,6 +1103,61 @@ static int sci_handle_breaks(struct uart_port *port)
>  	return copied;
>  }
>
> +static int scif_set_rtrg(struct uart_port *port, int rx_trig)
> +{
> +	unsigned int bits;
> +
> +	if (rx_trig < 1)
> +		rx_trig = 1;
> +	if (rx_trig >= port->fifosize)
> +		rx_trig = port->fifosize;
> +
> +	/* HSCIF can be set to an arbitrary level. */
> +	if (sci_getreg(port, HSRTRGR)->size) {
> +		serial_port_out(port, HSRTRGR, rx_trig);
> +		return rx_trig;
> +	}
> +
> +	if (port->type == PORT_SCIF) {

    This is asking to be a *switch* statement instead.

> +		if (rx_trig < 4) {
> +			bits = 0;
> +			rx_trig = 1;
> +		} else if (rx_trig < 8) {
> +			bits = SCFCR_RTRG0;
> +			rx_trig = 4;
> +		} else if (rx_trig < 14) {
> +			bits = SCFCR_RTRG1;
> +			rx_trig = 8;
> +		} else {
> +			bits = SCFCR_RTRG0 | SCFCR_RTRG1;
> +			rx_trig = 14;
> +		}
> +	} else if (port->type == PORT_SCIFA || port->type == PORT_SCIFB) {
> +		if (rx_trig < 16) {
> +			bits = 0;
> +			rx_trig = 1;
> +		} else if (rx_trig < 32) {
> +			bits = SCFCR_RTRG0;
> +			rx_trig = 16;
> +		} else if (rx_trig < 48) {
> +			bits = SCFCR_RTRG1;
> +			rx_trig = 32;
> +		} else {
> +			bits = SCFCR_RTRG0 | SCFCR_RTRG1;
> +			rx_trig = 48;
> +		}
> +	} else {
> +		WARN(1, "unknown FIFO configuration");
> +		return 1;
> +	}

[...]

MBR, Sergei

Re: [PATCH 3/7] serial: sh-sci: implement FIFO threshold register setting

[Geert Uytterhoeven]

Hi Uli,

On Fri, Dec 9, 2016 at 1:36 PM, Ulrich Hecht
<ulrich.hecht+renesas@gmail.com> wrote:
> Sets the closest match for a desired RX trigger level.
>
> Signed-off-by: Ulrich Hecht <ulrich.hecht+renesas@gmail.com>

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

> index de25db8..844288f 100644
> --- a/drivers/tty/serial/sh-sci.c
> +++ b/drivers/tty/serial/sh-sci.c
> @@ -1103,6 +1103,61 @@ static int sci_handle_breaks(struct uart_port *port)
>         return copied;
>  }
>
> +static int scif_set_rtrg(struct uart_port *port, int rx_trig)
> +{

> +       if (port->type == PORT_SCIF) {

> +       } else if (port->type == PORT_SCIFA || port->type == PORT_SCIFB) {

> +       } else {
> +               WARN(1, "unknown FIFO configuration");

This cannot happen, right?

> +               return 1;
> +       }

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 4/7] serial: sh-sci: increase RX FIFO trigger defaults for (H)SCIF

[Ulrich Hecht]

Signed-off-by: Ulrich Hecht <ulrich.hecht+renesas@gmail.com>
---
 drivers/tty/serial/sh-sci.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
index 844288f..ce3cf03 100644
--- a/drivers/tty/serial/sh-sci.c
+++ b/drivers/tty/serial/sh-sci.c
@@ -141,6 +141,7 @@ struct sci_port {
 	struct timer_list		rx_timer;
 	unsigned int			rx_timeout;
 #endif
+	int				rx_trigger;
 
 	bool autorts;
 };
@@ -2204,6 +2205,7 @@ static void sci_reset(struct uart_port *port)
 {
 	const struct plat_sci_reg *reg;
 	unsigned int status;
+	struct sci_port *s = to_sci_port(port);
 
 	do {
 		status = serial_port_in(port, SCxSR);
@@ -2223,6 +2225,9 @@ static void sci_reset(struct uart_port *port)
 		status &= ~(SCLSR_TO | SCLSR_ORER);
 		serial_port_out(port, SCLSR, status);
 	}
+
+	if (s->rx_trigger > 1)
+		scif_set_rtrg(port, s->rx_trigger);
 }
 
 static void sci_set_termios(struct uart_port *port, struct ktermios *termios,
@@ -2729,6 +2734,7 @@ static int sci_init_single(struct platform_device *dev,
 		sci_port->overrun_reg = SCLSR;
 		sci_port->overrun_mask = SCLSR_ORER;
 		sci_port->sampling_rate_mask = SCI_SR_RANGE(8, 32);
+		sci_port->rx_trigger = 64;
 		break;
 	case PORT_SCIFA:
 		port->fifosize = 64;
@@ -2747,12 +2753,14 @@ static int sci_init_single(struct platform_device *dev,
 			sci_port->overrun_mask = SCLSR_ORER;
 			sci_port->sampling_rate_mask = SCI_SR(32);
 		}
+		sci_port->rx_trigger = 8;
 		break;
 	default:
 		port->fifosize = 1;
 		sci_port->overrun_reg = SCxSR;
 		sci_port->overrun_mask = SCI_ORER;
 		sci_port->sampling_rate_mask = SCI_SR(32);
+		sci_port->rx_trigger = 1;
 		break;
 	}
 
-- 
2.7.4

Re: [PATCH 4/7] serial: sh-sci: increase RX FIFO trigger defaults for (H)SCIF

[Geert Uytterhoeven]

Hi Uli,

On Fri, Dec 9, 2016 at 1:36 PM, Ulrich Hecht
<ulrich.hecht+renesas@gmail.com> wrote:
> Signed-off-by: Ulrich Hecht <ulrich.hecht+renesas@gmail.com>
> ---
>  drivers/tty/serial/sh-sci.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
>
> diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
> index 844288f..ce3cf03 100644
> --- a/drivers/tty/serial/sh-sci.c
> +++ b/drivers/tty/serial/sh-sci.c

> @@ -2729,6 +2734,7 @@ static int sci_init_single(struct platform_device *dev,
>                 sci_port->overrun_reg = SCLSR;
>                 sci_port->overrun_mask = SCLSR_ORER;
>                 sci_port->sampling_rate_mask = SCI_SR_RANGE(8, 32);
> +               sci_port->rx_trigger = 64;
>                 break;
>         case PORT_SCIFA:
>                 port->fifosize = 64;
> @@ -2747,12 +2753,14 @@ static int sci_init_single(struct platform_device *dev,
>                         sci_port->overrun_mask = SCLSR_ORER;
>                         sci_port->sampling_rate_mask = SCI_SR(32);
>                 }
> +               sci_port->rx_trigger = 8;

PORT_SCIF is also used with SCIx_SH7705_SCIF_REGTYPE, so this should
be moved inside the "else" branch above.
Then, what to do with the "if" branch?

>From the sh7705/sh7720/sh7721 datasheets, it looks a lot like SCIFA, and thus
would need "sci_port->rx_trigger = 32", and the same handling as SCIFA in
scif_set_rtrg().

See also "[RFC] serial: sh-sci: Correct FIFO stages on sh7705/sh7720/sh7721"
(https://patchwork.kernel.org/patch/6810191/).

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 5/7] serial: sh-sci: SCIFA/B RX FIFO software timeout

[Ulrich Hecht]

Implements support for FIFO fill thresholds greater than one with software
timeout.

This mechanism is not possible (or at least not useful) on SCIF family
hardware other than SCIFA and SCIFB because they do not support turning off
the DR hardware timeout interrupt separately from the RI interrupt.

Signed-off-by: Ulrich Hecht <ulrich.hecht+renesas@gmail.com>
---
 drivers/tty/serial/sh-sci.c | 102 ++++++++++++++++++++++++++++++--------------
 1 file changed, 69 insertions(+), 33 deletions(-)

diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
index ce3cf03..0af4997 100644
--- a/drivers/tty/serial/sh-sci.c
+++ b/drivers/tty/serial/sh-sci.c
@@ -141,7 +141,10 @@ struct sci_port {
 	struct timer_list		rx_timer;
 	unsigned int			rx_timeout;
 #endif
+	unsigned int			rx_frame;
 	int				rx_trigger;
+	struct timer_list		rx_fifo_timer;
+	int				rx_fifo_timeout;
 
 	bool autorts;
 };
@@ -1159,6 +1162,24 @@ static int scif_set_rtrg(struct uart_port *port, int rx_trig)
 	return rx_trig;
 }
 
+static int scif_rtrg_enabled(struct uart_port *port)
+{
+	if (sci_getreg(port, HSRTRGR)->size)
+		return serial_port_in(port, HSRTRGR) != 0;
+	else
+		return (serial_port_in(port, SCFCR) &
+			(SCFCR_RTRG0 | SCFCR_RTRG1)) != 0;
+}
+
+static void rx_fifo_timer_fn(unsigned long arg)
+{
+	struct sci_port *s = (struct sci_port *)arg;
+	struct uart_port *port = &s->port;
+
+	dev_dbg(port->dev, "Rx timed out\n");
+	scif_set_rtrg(port, 1);
+}
+
 #ifdef CONFIG_SERIAL_SH_SCI_DMA
 static void sci_dma_tx_complete(void *arg)
 {
@@ -1615,9 +1636,9 @@ static inline void sci_free_dma(struct uart_port *port)
 
 static irqreturn_t sci_rx_interrupt(int irq, void *ptr)
 {
-#ifdef CONFIG_SERIAL_SH_SCI_DMA
 	struct uart_port *port = ptr;
 	struct sci_port *s = to_sci_port(port);
+#ifdef CONFIG_SERIAL_SH_SCI_DMA
 
 	if (s->chan_rx) {
 		u16 scr = serial_port_in(port, SCSCR);
@@ -1643,6 +1664,14 @@ static irqreturn_t sci_rx_interrupt(int irq, void *ptr)
 	}
 #endif
 
+	if (s->rx_trigger > 1 && s->rx_fifo_timeout > 0) {
+		if (!scif_rtrg_enabled(port))
+			scif_set_rtrg(port, s->rx_trigger);
+
+		mod_timer(&s->rx_fifo_timer, jiffies + DIV_ROUND_UP(
+			  s->rx_frame * s->rx_fifo_timeout, 1000));
+	}
+
 	/* I think sci_receive_chars has to be called irrespective
 	 * of whether the I_IXOFF is set, otherwise, how is the interrupt
 	 * to be disabled?
@@ -2226,14 +2255,20 @@ static void sci_reset(struct uart_port *port)
 		serial_port_out(port, SCLSR, status);
 	}
 
-	if (s->rx_trigger > 1)
-		scif_set_rtrg(port, s->rx_trigger);
+	if (s->rx_trigger > 1) {
+		if (s->rx_fifo_timeout) {
+			scif_set_rtrg(port, 1);
+			setup_timer(&s->rx_fifo_timer, rx_fifo_timer_fn,
+				    (unsigned long)s);
+		} else
+			scif_set_rtrg(port, s->rx_trigger);
+	}
 }
 
 static void sci_set_termios(struct uart_port *port, struct ktermios *termios,
 			    struct ktermios *old)
 {
-	unsigned int baud, smr_val = SCSMR_ASYNC, scr_val = 0, i;
+	unsigned int baud, smr_val = SCSMR_ASYNC, scr_val = 0, i, bits;
 	unsigned int brr = 255, cks = 0, srr = 15, dl = 0, sccks = 0;
 	unsigned int brr1 = 255, cks1 = 0, srr1 = 15, dl1 = 0;
 	struct sci_port *s = to_sci_port(port);
@@ -2428,7 +2463,6 @@ static void sci_set_termios(struct uart_port *port, struct ktermios *termios,
 		udelay(DIV_ROUND_UP(10 * 1000000, baud));
 	}
 
-#ifdef CONFIG_SERIAL_SH_SCI_DMA
 	/*
 	 * Calculate delay for 2 DMA buffers (4 FIFO).
 	 * See serial_core.c::uart_update_timeout().
@@ -2439,36 +2473,34 @@ static void sci_set_termios(struct uart_port *port, struct ktermios *termios,
 	 * value obtained by this formula is too small. Therefore, if the value
 	 * is smaller than 20ms, use 20ms as the timeout value for DMA.
 	 */
-	if (s->chan_rx) {
-		unsigned int bits;
+	/* byte size and parity */
+	switch (termios->c_cflag & CSIZE) {
+	case CS5:
+		bits = 7;
+		break;
+	case CS6:
+		bits = 8;
+		break;
+	case CS7:
+		bits = 9;
+		break;
+	default:
+		bits = 10;
+		break;
+	}
 
-		/* byte size and parity */
-		switch (termios->c_cflag & CSIZE) {
-		case CS5:
-			bits = 7;
-			break;
-		case CS6:
-			bits = 8;
-			break;
-		case CS7:
-			bits = 9;
-			break;
-		default:
-			bits = 10;
-			break;
-		}
+	if (termios->c_cflag & CSTOPB)
+		bits++;
+	if (termios->c_cflag & PARENB)
+		bits++;
 
-		if (termios->c_cflag & CSTOPB)
-			bits++;
-		if (termios->c_cflag & PARENB)
-			bits++;
-		s->rx_timeout = DIV_ROUND_UP((s->buf_len_rx * 2 * bits * HZ) /
-					     (baud / 10), 10);
-		dev_dbg(port->dev, "DMA Rx t-out %ums, tty t-out %u jiffies\n",
-			s->rx_timeout * 1000 / HZ, port->timeout);
-		if (s->rx_timeout < msecs_to_jiffies(20))
-			s->rx_timeout = msecs_to_jiffies(20);
-	}
+	s->rx_frame = (100 * bits * HZ) / (baud / 10);
+#ifdef CONFIG_SERIAL_SH_SCI_DMA
+	s->rx_timeout = DIV_ROUND_UP(s->buf_len_rx * 2 * s->rx_frame, 1000);
+	dev_dbg(port->dev, "DMA Rx t-out %ums, tty t-out %u jiffies\n",
+		s->rx_timeout * 1000 / HZ, port->timeout);
+	if (s->rx_timeout < msecs_to_jiffies(20))
+		s->rx_timeout = msecs_to_jiffies(20);
 #endif
 
 	if ((termios->c_cflag & CREAD) != 0)
@@ -2728,6 +2760,7 @@ static int sci_init_single(struct platform_device *dev,
 		sci_port->overrun_reg = SCxSR;
 		sci_port->overrun_mask = SCIFA_ORER;
 		sci_port->sampling_rate_mask = SCI_SR_SCIFAB;
+		sci_port->rx_trigger = 48;
 		break;
 	case PORT_HSCIF:
 		port->fifosize = 128;
@@ -2741,6 +2774,7 @@ static int sci_init_single(struct platform_device *dev,
 		sci_port->overrun_reg = SCxSR;
 		sci_port->overrun_mask = SCIFA_ORER;
 		sci_port->sampling_rate_mask = SCI_SR_SCIFAB;
+		sci_port->rx_trigger = 32;
 		break;
 	case PORT_SCIF:
 		port->fifosize = 16;
@@ -2764,6 +2798,8 @@ static int sci_init_single(struct platform_device *dev,
 		break;
 	}
 
+	sci_port->rx_fifo_timeout = 0;
+
 	/* SCIFA on sh7723 and sh7724 need a custom sampling rate that doesn't
 	 * match the SoC datasheet, this should be investigated. Let platform
 	 * data override the sampling rate for now.
-- 
2.7.4

Re: [PATCH 5/7] serial: sh-sci: SCIFA/B RX FIFO software timeout

[Geert Uytterhoeven]

Hi Uli.

On Fri, Dec 9, 2016 at 1:36 PM, Ulrich Hecht
<ulrich.hecht+renesas@gmail.com> wrote:
> Implements support for FIFO fill thresholds greater than one with software
> timeout.
>
> This mechanism is not possible (or at least not useful) on SCIF family
> hardware other than SCIFA and SCIFB because they do not support turning off
> the DR hardware timeout interrupt separately from the RI interrupt.

What about HSCIF? Your code does handle HSCIF (cfr. HSRTRGR below)?

> --- a/drivers/tty/serial/sh-sci.c
> +++ b/drivers/tty/serial/sh-sci.c

> @@ -1159,6 +1162,24 @@ static int scif_set_rtrg(struct uart_port *port, int rx_trig)
>         return rx_trig;
>  }
>
> +static int scif_rtrg_enabled(struct uart_port *port)
> +{
> +       if (sci_getreg(port, HSRTRGR)->size)
> +               return serial_port_in(port, HSRTRGR) != 0;
> +       else
> +               return (serial_port_in(port, SCFCR) &
> +                       (SCFCR_RTRG0 | SCFCR_RTRG1)) != 0;
> +}

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

Re: [PATCH 5/7] serial: sh-sci: SCIFA/B RX FIFO software timeout

[Ulrich Hecht]

On Wed, Dec 14, 2016 at 3:02 PM, Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
> On Fri, Dec 9, 2016 at 1:36 PM, Ulrich Hecht
> <ulrich.hecht+renesas@gmail.com> wrote:
>> Implements support for FIFO fill thresholds greater than one with software
>> timeout.
>>
>> This mechanism is not possible (or at least not useful) on SCIF family
>> hardware other than SCIFA and SCIFB because they do not support turning off
>> the DR hardware timeout interrupt separately from the RI interrupt.
>
> What about HSCIF? Your code does handle HSCIF (cfr. HSRTRGR below)?

Same as the other non-A/B SCIFs.

>> --- a/drivers/tty/serial/sh-sci.c
>> +++ b/drivers/tty/serial/sh-sci.c
>
>> @@ -1159,6 +1162,24 @@ static int scif_set_rtrg(struct uart_port *port, int rx_trig)
>>         return rx_trig;
>>  }
>>
>> +static int scif_rtrg_enabled(struct uart_port *port)
>> +{
>> +       if (sci_getreg(port, HSRTRGR)->size)
>> +               return serial_port_in(port, HSRTRGR) != 0;
>> +       else
>> +               return (serial_port_in(port, SCFCR) &
>> +                       (SCFCR_RTRG0 | SCFCR_RTRG1)) != 0;
>> +}

I implemented this to work with all SCIFs, but ATM it is only used in
the "SCIFA/B, software timeout" case.

CU
Uli

[PATCH 6/7] serial: sh-sci: make RX FIFO parameters tunable via sysfs

[Ulrich Hecht]

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@gmail.com>
---
 drivers/tty/serial/sh-sci.c | 82 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 82 insertions(+)

diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
index 0af4997..5ad08d6 100644
--- a/drivers/tty/serial/sh-sci.c
+++ b/drivers/tty/serial/sh-sci.c
@@ -1180,6 +1180,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);
+	return count;
+}
+
+static DEVICE_ATTR(rx_fifo_trigger, 0644, rx_trigger_show, rx_trigger_store);
+
+static ssize_t rx_fifo_timeout_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_fifo_timeout);
+}
+
+static ssize_t rx_fifo_timeout_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_fifo_timeout = r;
+	scif_set_rtrg(port, 1);
+	if (r > 0)
+		setup_timer(&sci->rx_fifo_timer, rx_fifo_timer_fn,
+			    (unsigned long)sci);
+	return count;
+}
+
+static DEVICE_ATTR(rx_fifo_timeout, 0644, rx_fifo_timeout_show, rx_fifo_timeout_store);
+
+
 #ifdef CONFIG_SERIAL_SH_SCI_DMA
 static void sci_dma_tx_complete(void *arg)
 {
@@ -3028,6 +3088,15 @@ static int sci_remove(struct platform_device *dev)
 
 	sci_cleanup_single(port);
 
+	if (port->port.fifosize > 1) {
+		sysfs_remove_file(&dev->dev.kobj,
+				  &dev_attr_rx_fifo_trigger.attr);
+	}
+	if (port->port.type == PORT_SCIFA || port->port.type == PORT_SCIFB) {
+		sysfs_remove_file(&dev->dev.kobj,
+				  &dev_attr_rx_fifo_timeout.attr);
+	}
+
 	return 0;
 }
 
@@ -3193,6 +3262,19 @@ static int sci_probe(struct platform_device *dev)
 	if (ret)
 		return ret;
 
+	if (sp->port.fifosize > 1) {
+		ret = sysfs_create_file(&dev->dev.kobj,
+				&dev_attr_rx_fifo_trigger.attr);
+		if (ret)
+			return ret;
+	}
+	if (sp->port.type == PORT_SCIFA || sp->port.type ==  PORT_SCIFB) {
+		ret = sysfs_create_file(&dev->dev.kobj,
+				&dev_attr_rx_fifo_timeout.attr);
+		if (ret)
+			return ret;
+	}
+
 #ifdef CONFIG_SH_STANDARD_BIOS
 	sh_bios_gdb_detach();
 #endif
-- 
2.7.4

Re: [PATCH 6/7] serial: sh-sci: make RX FIFO parameters tunable via sysfs

[Geert Uytterhoeven]

Hi Uli,

On Fri, Dec 9, 2016 at 1:36 PM, Ulrich Hecht
<ulrich.hecht+renesas@gmail.com> 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@gmail.com>

> --- a/drivers/tty/serial/sh-sci.c
> +++ b/drivers/tty/serial/sh-sci.c

> @@ -3193,6 +3262,19 @@ static int sci_probe(struct platform_device *dev)
>         if (ret)
>                 return ret;
>
> +       if (sp->port.fifosize > 1) {
> +               ret = sysfs_create_file(&dev->dev.kobj,
> +                               &dev_attr_rx_fifo_trigger.attr);
> +               if (ret)
> +                       return ret;
> +       }
> +       if (sp->port.type == PORT_SCIFA || sp->port.type ==  PORT_SCIFB) {
> +               ret = sysfs_create_file(&dev->dev.kobj,
> +                               &dev_attr_rx_fifo_timeout.attr);
> +               if (ret)
> +                       return ret;

Returning here may leak the rx_fifo_trigger file created above.

> +       }

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 7/7] arm64: dts: r8a7796: salvator-x: add SCIF1 (DEBUG1)

[Ulrich Hecht]

Enables the SCIF hooked up to the DEBUG1 connector.

Signed-off-by: Ulrich Hecht <ulrich.hecht+renesas@gmail.com>
---
 arch/arm64/boot/dts/renesas/r8a7796-salvator-x.dts | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/arch/arm64/boot/dts/renesas/r8a7796-salvator-x.dts b/arch/arm64/boot/dts/renesas/r8a7796-salvator-x.dts
index da46ac7..9204691 100644
--- a/arch/arm64/boot/dts/renesas/r8a7796-salvator-x.dts
+++ b/arch/arm64/boot/dts/renesas/r8a7796-salvator-x.dts
@@ -18,6 +18,7 @@
 
 	aliases {
 		serial0 = &scif2;
+		serial1 = &scif1;
 		ethernet0 = &avb;
 	};
 
@@ -140,6 +141,11 @@
 	pinctrl-0 = <&scif_clk_pins>;
 	pinctrl-names = "default";
 
+	scif1_pins: scif1 {
+		groups = "scif1_data_a";
+		function = "scif1";
+	};
+
 	scif2_pins: scif2 {
 		groups = "scif2_data_a";
 		function = "scif2";
@@ -280,6 +286,12 @@
 	status = "okay";
 };
 
+&scif1 {
+	pinctrl-0 = <&scif1_pins>;
+	pinctrl-names = "default";
+	status = "okay";
+};
+
 &scif2 {
 	pinctrl-0 = <&scif2_pins>;
 	pinctrl-names = "default";
-- 
2.7.4

Re: [PATCH 7/7] arm64: dts: r8a7796: salvator-x: add SCIF1 (DEBUG1)

[Geert Uytterhoeven]

Hi Uli,

On Fri, Dec 9, 2016 at 1:36 PM, Ulrich Hecht
<ulrich.hecht+renesas@gmail.com> wrote:
> Enables the SCIF hooked up to the DEBUG1 connector.
>
> Signed-off-by: Ulrich Hecht <ulrich.hecht+renesas@gmail.com>
> ---
>  arch/arm64/boot/dts/renesas/r8a7796-salvator-x.dts | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/renesas/r8a7796-salvator-x.dts b/arch/arm64/boot/dts/renesas/r8a7796-salvator-x.dts
> index da46ac7..9204691 100644
> --- a/arch/arm64/boot/dts/renesas/r8a7796-salvator-x.dts
> +++ b/arch/arm64/boot/dts/renesas/r8a7796-salvator-x.dts
> @@ -18,6 +18,7 @@
>
>         aliases {
>                 serial0 = &scif2;
> +               serial1 = &scif1;

Is there any specific reason you chose scif1 over hscif1?

>                 ethernet0 = &avb;
>         };
>
> @@ -140,6 +141,11 @@
>         pinctrl-0 = <&scif_clk_pins>;
>         pinctrl-names = "default";
>
> +       scif1_pins: scif1 {
> +               groups = "scif1_data_a";

Please add "scif1_ctrl_a".

> +               function = "scif1";
> +       };
> +
>         scif2_pins: scif2 {
>                 groups = "scif2_data_a";
>                 function = "scif2";
> @@ -280,6 +286,12 @@
>         status = "okay";
>  };
>
> +&scif1 {
> +       pinctrl-0 = <&scif1_pins>;
> +       pinctrl-names = "default";

Please add "uart-has-rtscts".

> +       status = "okay";
> +};

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