[PATCH v4 03/13] dt-bindings: serial: Add compatible for Renesas RZ/T2H SoC in sci

[PATCH v4 03/13] dt-bindings: serial: Add compatible for Renesas RZ/T2H SoC in sci

[Thierry Bultel]

The SCI of RZ/T2H SoC (a.k.a r9a09g077), as a lot
of similarities with other Renesas SoC like G2L, G3S, V2L;
However, it has a different set of registers, and in addition to serial,
this IP also supports SCIe (encoder), SmartCard, i2c and spi.
This is why the 'renesas,sci' fallback for generic SCI does not apply for it.

Signed-off-by: Thierry Bultel <thierry.bultel.yh@bp.renesas.com>
---
Changes v3->v4:
  - Added more details in commit description about why renesas,sci 
    does not apply.
  - Removed uart-has-rtscts for !rzsci.
----
 .../bindings/serial/renesas,sci.yaml          | 63 ++++++++++++-------
 1 file changed, 39 insertions(+), 24 deletions(-)

diff --git a/Documentation/devicetree/bindings/serial/renesas,sci.yaml b/Documentation/devicetree/bindings/serial/renesas,sci.yaml
index 64d3db6e54e5..13c5c47cd72f 100644
--- a/Documentation/devicetree/bindings/serial/renesas,sci.yaml
+++ b/Documentation/devicetree/bindings/serial/renesas,sci.yaml
@@ -9,9 +9,6 @@ title: Renesas Serial Communication Interface
 maintainers:
   - Geert Uytterhoeven <geert+renesas@glider.be>
 
-allOf:
-  - $ref: serial.yaml#
-
 properties:
   compatible:
     oneOf:
@@ -22,6 +19,8 @@ properties:
               - renesas,r9a07g054-sci     # RZ/V2L
           - const: renesas,sci            # generic SCI compatible UART
 
+      - const: renesas,r9a09g077-sci      # RZ/T2H
+
       - items:
           - const: renesas,sci            # generic SCI compatible UART
 
@@ -54,8 +53,6 @@ properties:
         - fck # UART functional clock
         - sck # optional external clock input
 
-  uart-has-rtscts: false
-
 required:
   - compatible
   - reg
@@ -63,25 +60,43 @@ required:
   - clocks
   - clock-names
 
-if:
-  properties:
-    compatible:
-      contains:
-        enum:
-          - renesas,r9a07g043-sci
-          - renesas,r9a07g044-sci
-          - renesas,r9a07g054-sci
-then:
-  properties:
-    resets:
-      maxItems: 1
-
-    power-domains:
-      maxItems: 1
-
-  required:
-    - resets
-    - power-domains
+allOf:
+  - $ref: serial.yaml#
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: renesas,r9a09g077-sci
+    then:
+      properties:
+        power-domains:
+          maxItems: 1
+      required:
+        - power-domains
+
+    else:
+      properties:
+        uart-has-rtscts: false
+
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - renesas,r9a07g043-sci
+              - renesas,r9a07g044-sci
+              - renesas,r9a07g054-sci
+    then:
+      properties:
+        resets:
+          maxItems: 1
+
+        power-domains:
+          maxItems: 1
+
+      required:
+        - resets
+        - power-domains
 
 unevaluatedProperties: false
 
-- 
2.43.0

[PATCH v4 07/13] serial: sh-sci: Fix a comment about SCIFA

[Thierry Bultel]

The comment was correct when it was added, at that time RZ/T1 was
the only SoC in the RZ/T line. Since then, further SoCs have been
added with RZ/T names which do not use the same SCIFA register
layout and so the comment is now misleading.

So we update the comment to explicitly reference only RZ/T1 SoCs.

Signed-off-by: Thierry Bultel <thierry.bultel.yh@bp.renesas.com>
Reviewed-by: Paul Barker <paul.barker.ct@bp.renesas.com>
---
Changes v3->v4: none
---
 drivers/tty/serial/sh-sci.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
index 1c8480d0338e..d7a060033a89 100644
--- a/drivers/tty/serial/sh-sci.c
+++ b/drivers/tty/serial/sh-sci.c
@@ -310,7 +310,7 @@ static const struct sci_port_params sci_port_params[SCIx_NR_REGTYPES] = {
 	},
 
 	/*
-	 * The "SCIFA" that is in RZ/A2, RZ/G2L and RZ/T.
+	 * The "SCIFA" that is in RZ/A2, RZ/G2L and RZ/T1.
 	 * It looks like a normal SCIF with FIFO data, but with a
 	 * compressed address space. Also, the break out of interrupts
 	 * are different: ERI/BRI, RXI, TXI, TEI, DRI.
-- 
2.43.0

[PATCH v4 08/13] serial: sh-sci: Introduced function pointers

[Thierry Bultel]

The aim here is to prepare support for new sci controllers like
the T2H/RSCI whose registers are too much different for being
handled in common code.

This named serial controller also has 32 bits register,
so some return types had to be changed.

The needed generic functions are no longer static, with prototypes
defined in sh-sci-common.h so that they can be used from specific
implementation in a separate file, to keep this driver as little
changed as possible.

For doing so, a set of 'ops' is added to struct sci_port.

Signed-off-by: Thierry Bultel <thierry.bultel.yh@bp.renesas.com>
---
Changes v3->v4:
   - Add missing #include <bitfield.h>
   - Remove sci_poll_get_char sci_poll_put_char from sh-sci-common.h (both 
     function are not used by rzsci yet).
   - Add missing #ifdef around .poll_put_char pointer initialization.
   - More registers to save & restore due to rebase on tty-next
---
 drivers/tty/serial/sh-sci-common.h | 158 ++++++++++
 drivers/tty/serial/sh-sci.c        | 482 +++++++++++++++--------------
 drivers/tty/serial/sh-sci.h        |   2 -
 3 files changed, 405 insertions(+), 237 deletions(-)
 create mode 100644 drivers/tty/serial/sh-sci-common.h

diff --git a/drivers/tty/serial/sh-sci-common.h b/drivers/tty/serial/sh-sci-common.h
new file mode 100644
index 000000000000..3ea116033d53
--- /dev/null
+++ b/drivers/tty/serial/sh-sci-common.h
@@ -0,0 +1,158 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+#ifndef __SH_SCI_COMMON_H__
+#define __SH_SCI_COMMON_H__
+
+#include <linux/serial_core.h>
+
+enum SCI_CLKS {
+	SCI_FCK,		/* Functional Clock */
+	SCI_SCK,		/* Optional External Clock */
+	SCI_BRG_INT,		/* Optional BRG Internal Clock Source */
+	SCI_SCIF_CLK,		/* Optional BRG External Clock Source */
+	SCI_NUM_CLKS
+};
+
+/* Offsets into the sci_port->irqs array */
+enum {
+	SCIx_ERI_IRQ,
+	SCIx_RXI_IRQ,
+	SCIx_TXI_IRQ,
+	SCIx_BRI_IRQ,
+	SCIx_DRI_IRQ,
+	SCIx_TEI_IRQ,
+	SCIx_NR_IRQS,
+
+	SCIx_MUX_IRQ = SCIx_NR_IRQS,	/* special case */
+};
+
+/* Bit x set means sampling rate x + 1 is supported */
+#define SCI_SR(x)		BIT((x) - 1)
+#define SCI_SR_RANGE(x, y)	GENMASK((y) - 1, (x) - 1)
+
+void sci_release_port(struct uart_port *port);
+int sci_request_port(struct uart_port *port);
+void sci_config_port(struct uart_port *port, int flags);
+int sci_verify_port(struct uart_port *port, struct serial_struct *ser);
+void sci_pm(struct uart_port *port, unsigned int state,
+		   unsigned int oldstate);
+
+struct plat_sci_reg {
+	u8 offset;
+	u8 size;
+};
+
+struct sci_port_params_bits {
+	unsigned int rxtx_enable;
+	unsigned int te_clear;
+	unsigned int poll_sent_bits;
+};
+
+struct sci_common_regs {
+	unsigned int status;
+	unsigned int control;
+};
+
+/* The actual number of needed registers. This is used by sci only */
+#define SCI_NR_REGS 20
+
+struct sci_port_params {
+	const struct plat_sci_reg regs[SCI_NR_REGS];
+	const struct sci_common_regs *common_regs;
+	const struct sci_port_params_bits *param_bits;
+	unsigned int fifosize;
+	unsigned int overrun_reg;
+	unsigned int overrun_mask;
+	unsigned int sampling_rate_mask;
+	unsigned int error_mask;
+	unsigned int error_clear;
+};
+
+struct sci_port_ops {
+	u32 (*read_reg)(struct uart_port *port, int reg);
+	void (*write_reg)(struct uart_port *port, int reg, int value);
+	void (*clear_SCxSR)(struct uart_port *port, unsigned int mask);
+
+	void (*transmit_chars)(struct uart_port *port);
+	void (*receive_chars)(struct uart_port *port);
+
+	void (*poll_put_char)(struct uart_port *port, unsigned char c);
+
+	int (*set_rtrg)(struct uart_port *port, int rx_trig);
+	int (*rtrg_enabled)(struct uart_port *port);
+
+	void (*shutdown_complete)(struct uart_port *port);
+
+	void (*prepare_console_write)(struct uart_port *port, u32 ctrl);
+	void (*console_save)(struct uart_port *port);
+	void (*console_restore)(struct uart_port *port);
+	size_t (*suspend_regs_size)(void);
+};
+
+struct sci_port {
+	struct uart_port	port;
+
+	/* Platform configuration */
+	const struct sci_port_params *params;
+	const struct plat_sci_port *cfg;
+
+	unsigned int		sampling_rate_mask;
+	resource_size_t		reg_size;
+	struct mctrl_gpios	*gpios;
+
+	/* Clocks */
+	struct clk		*clks[SCI_NUM_CLKS];
+	unsigned long		clk_rates[SCI_NUM_CLKS];
+
+	int			irqs[SCIx_NR_IRQS];
+	char			*irqstr[SCIx_NR_IRQS];
+
+	struct dma_chan			*chan_tx;
+	struct dma_chan			*chan_rx;
+
+	struct reset_control		*rstc;
+	struct sci_suspend_regs		*suspend_regs;
+
+#ifdef CONFIG_SERIAL_SH_SCI_DMA
+	struct dma_chan			*chan_tx_saved;
+	struct dma_chan			*chan_rx_saved;
+	dma_cookie_t			cookie_tx;
+	dma_cookie_t			cookie_rx[2];
+	dma_cookie_t			active_rx;
+	dma_addr_t			tx_dma_addr;
+	unsigned int			tx_dma_len;
+	struct scatterlist		sg_rx[2];
+	void				*rx_buf[2];
+	size_t				buf_len_rx;
+	struct work_struct		work_tx;
+	struct hrtimer			rx_timer;
+	unsigned int			rx_timeout;	/* microseconds */
+#endif
+	unsigned int			rx_frame;
+	int				rx_trigger;
+	struct timer_list		rx_fifo_timer;
+	int				rx_fifo_timeout;
+	u16				hscif_tot;
+
+	const struct sci_port_ops *ops;
+
+	bool has_rtscts;
+	bool autorts;
+	bool tx_occurred;
+};
+
+#define to_sci_port(uart) container_of((uart), struct sci_port, port)
+
+void sci_port_disable(struct sci_port *sci_port);
+void sci_port_enable(struct sci_port *sci_port);
+
+int sci_startup(struct uart_port *port);
+
+#define min_sr(_port)		ffs((_port)->sampling_rate_mask)
+#define max_sr(_port)		fls((_port)->sampling_rate_mask)
+
+#ifdef CONFIG_SERIAL_SH_SCI_EARLYCON
+int __init scix_early_console_setup(struct earlycon_device *device, int);
+#endif
+
+#endif /* __SH_SCI_COMMON_H__ */
diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
index d7a060033a89..4e82055a41c8 100644
--- a/drivers/tty/serial/sh-sci.c
+++ b/drivers/tty/serial/sh-sci.c
@@ -56,19 +56,7 @@
 
 #include "serial_mctrl_gpio.h"
 #include "sh-sci.h"
-
-/* Offsets into the sci_port->irqs array */
-enum {
-	SCIx_ERI_IRQ,
-	SCIx_RXI_IRQ,
-	SCIx_TXI_IRQ,
-	SCIx_BRI_IRQ,
-	SCIx_DRI_IRQ,
-	SCIx_TEI_IRQ,
-	SCIx_NR_IRQS,
-
-	SCIx_MUX_IRQ = SCIx_NR_IRQS,	/* special case */
-};
+#include "sh-sci-common.h"
 
 #define SCIx_IRQ_IS_MUXED(port)			\
 	((port)->irqs[SCIx_ERI_IRQ] ==	\
@@ -76,32 +64,38 @@ enum {
 	((port)->irqs[SCIx_ERI_IRQ] &&	\
 	 ((port)->irqs[SCIx_RXI_IRQ] < 0))
 
-enum SCI_CLKS {
-	SCI_FCK,		/* Functional Clock */
-	SCI_SCK,		/* Optional External Clock */
-	SCI_BRG_INT,		/* Optional BRG Internal Clock Source */
-	SCI_SCIF_CLK,		/* Optional BRG External Clock Source */
-	SCI_NUM_CLKS
-};
-
-/* Bit x set means sampling rate x + 1 is supported */
-#define SCI_SR(x)		BIT((x) - 1)
-#define SCI_SR_RANGE(x, y)	GENMASK((y) - 1, (x) - 1)
-
 #define SCI_SR_SCIFAB		SCI_SR(5) | SCI_SR(7) | SCI_SR(11) | \
 				SCI_SR(13) | SCI_SR(16) | SCI_SR(17) | \
 				SCI_SR(19) | SCI_SR(27)
 
-#define min_sr(_port)		ffs((_port)->sampling_rate_mask)
-#define max_sr(_port)		fls((_port)->sampling_rate_mask)
-
 /* Iterate over all supported sampling rates, from high to low */
 #define for_each_sr(_sr, _port)						\
 	for ((_sr) = max_sr(_port); (_sr) >= min_sr(_port); (_sr)--)	\
 		if ((_port)->sampling_rate_mask & SCI_SR((_sr)))
 
-struct plat_sci_reg {
-	u8 offset, size;
+#define SCI_NPORTS CONFIG_SERIAL_SH_SCI_NR_UARTS
+
+static struct sci_port sci_ports[SCI_NPORTS];
+static unsigned long sci_ports_in_use;
+static struct uart_driver sci_uart_driver;
+static bool sci_uart_earlycon;
+static bool sci_uart_earlycon_dev_probing;
+
+static const struct sci_port_params_bits sci_sci_port_params_bits = {
+	.rxtx_enable = SCSCR_RE | SCSCR_TE,
+	.te_clear = SCSCR_TE | SCSCR_TEIE,
+	.poll_sent_bits = SCI_TDRE | SCI_TEND
+};
+
+static const struct sci_port_params_bits sci_scif_port_params_bits = {
+	.rxtx_enable = SCSCR_RE | SCSCR_TE,
+	.te_clear = SCSCR_TE | SCSCR_TEIE,
+	.poll_sent_bits = SCIF_TDFE | SCIF_TEND
+};
+
+static const struct sci_common_regs sci_common_regs = {
+	.status = SCxSR,
+	.control = SCSCR,
 };
 
 struct sci_suspend_regs {
@@ -118,77 +112,9 @@ struct sci_suspend_regs {
 	u8 semr;
 };
 
-struct sci_port_params {
-	const struct plat_sci_reg regs[SCIx_NR_REGS];
-	unsigned int fifosize;
-	unsigned int overrun_reg;
-	unsigned int overrun_mask;
-	unsigned int sampling_rate_mask;
-	unsigned int error_mask;
-	unsigned int error_clear;
-};
-
-struct sci_port {
-	struct uart_port	port;
-
-	/* Platform configuration */
-	const struct sci_port_params *params;
-	const struct plat_sci_port *cfg;
-	unsigned int		sampling_rate_mask;
-	resource_size_t		reg_size;
-	struct mctrl_gpios	*gpios;
-
-	/* Clocks */
-	struct clk		*clks[SCI_NUM_CLKS];
-	unsigned long		clk_rates[SCI_NUM_CLKS];
-
-	int			irqs[SCIx_NR_IRQS];
-	char			*irqstr[SCIx_NR_IRQS];
-
-	struct dma_chan			*chan_tx;
-	struct dma_chan			*chan_rx;
-
-	struct reset_control		*rstc;
-
-#ifdef CONFIG_SERIAL_SH_SCI_DMA
-	struct dma_chan			*chan_tx_saved;
-	struct dma_chan			*chan_rx_saved;
-	dma_cookie_t			cookie_tx;
-	dma_cookie_t			cookie_rx[2];
-	dma_cookie_t			active_rx;
-	dma_addr_t			tx_dma_addr;
-	unsigned int			tx_dma_len;
-	struct scatterlist		sg_rx[2];
-	void				*rx_buf[2];
-	size_t				buf_len_rx;
-	struct work_struct		work_tx;
-	struct hrtimer			rx_timer;
-	unsigned int			rx_timeout;	/* microseconds */
-#endif
-	unsigned int			rx_frame;
-	int				rx_trigger;
-	struct timer_list		rx_fifo_timer;
-	int				rx_fifo_timeout;
-	struct sci_suspend_regs		suspend_regs;
-	u16				hscif_tot;
-
-	bool has_rtscts;
-	bool autorts;
-	bool tx_occurred;
-};
-
-#define SCI_NPORTS CONFIG_SERIAL_SH_SCI_NR_UARTS
-
-static struct sci_port sci_ports[SCI_NPORTS];
-static unsigned long sci_ports_in_use;
-static struct uart_driver sci_uart_driver;
-static bool sci_uart_earlycon;
-static bool sci_uart_earlycon_dev_probing;
-
-static inline struct sci_port *
-to_sci_port(struct uart_port *uart)
+static size_t sci_suspend_regs_size(void)
 {
-	return container_of(uart, struct sci_port, port);
+	return sizeof(struct sci_suspend_regs);
 }
 
 static const struct sci_port_params sci_port_params[SCIx_NR_REGTYPES] = {
@@ -211,6 +137,8 @@ static const struct sci_port_params sci_port_params[SCIx_NR_REGTYPES] = {
 		.sampling_rate_mask = SCI_SR(32),
 		.error_mask = SCI_DEFAULT_ERROR_MASK | SCI_ORER,
 		.error_clear = SCI_ERROR_CLEAR & ~SCI_ORER,
+		.param_bits = &sci_sci_port_params_bits,
+		.common_regs = &sci_common_regs,
 	},
 
 	/*
@@ -233,6 +161,8 @@ static const struct sci_port_params sci_port_params[SCIx_NR_REGTYPES] = {
 		.sampling_rate_mask = SCI_SR(32),
 		.error_mask = SCI_DEFAULT_ERROR_MASK | SCI_ORER,
 		.error_clear = SCI_ERROR_CLEAR & ~SCI_ORER,
+		.param_bits = &sci_scif_port_params_bits,
+		.common_regs = &sci_common_regs,
 	},
 
 	/*
@@ -257,6 +187,8 @@ static const struct sci_port_params sci_port_params[SCIx_NR_REGTYPES] = {
 		.sampling_rate_mask = SCI_SR_SCIFAB,
 		.error_mask = SCIF_DEFAULT_ERROR_MASK | SCIFA_ORER,
 		.error_clear = SCIF_ERROR_CLEAR & ~SCIFA_ORER,
+		.param_bits = &sci_scif_port_params_bits,
+		.common_regs = &sci_common_regs,
 	},
 
 	/*
@@ -282,6 +214,8 @@ static const struct sci_port_params sci_port_params[SCIx_NR_REGTYPES] = {
 		.sampling_rate_mask = SCI_SR_SCIFAB,
 		.error_mask = SCIF_DEFAULT_ERROR_MASK | SCIFA_ORER,
 		.error_clear = SCIF_ERROR_CLEAR & ~SCIFA_ORER,
+		.param_bits = &sci_scif_port_params_bits,
+		.common_regs = &sci_common_regs,
 	},
 
 	/*
@@ -307,6 +241,8 @@ static const struct sci_port_params sci_port_params[SCIx_NR_REGTYPES] = {
 		.sampling_rate_mask = SCI_SR(32),
 		.error_mask = SCIF_DEFAULT_ERROR_MASK,
 		.error_clear = SCIF_ERROR_CLEAR,
+		.param_bits = &sci_scif_port_params_bits,
+		.common_regs = &sci_common_regs,
 	},
 
 	/*
@@ -335,6 +271,8 @@ static const struct sci_port_params sci_port_params[SCIx_NR_REGTYPES] = {
 		.sampling_rate_mask = SCI_SR(32),
 		.error_mask = SCIF_DEFAULT_ERROR_MASK,
 		.error_clear = SCIF_ERROR_CLEAR,
+		.param_bits = &sci_scif_port_params_bits,
+		.common_regs = &sci_common_regs,
 	},
 
 	/*
@@ -366,6 +304,8 @@ static const struct sci_port_params sci_port_params[SCIx_NR_REGTYPES] = {
 		.sampling_rate_mask = SCI_SR(32),
 		.error_mask = SCIF_DEFAULT_ERROR_MASK,
 		.error_clear = SCIF_ERROR_CLEAR,
+		.param_bits = &sci_scif_port_params_bits,
+		.common_regs = &sci_common_regs,
 	},
 
 	/*
@@ -388,6 +328,8 @@ static const struct sci_port_params sci_port_params[SCIx_NR_REGTYPES] = {
 		.sampling_rate_mask = SCI_SR(32),
 		.error_mask = SCIF_DEFAULT_ERROR_MASK,
 		.error_clear = SCIF_ERROR_CLEAR,
+		.param_bits = &sci_scif_port_params_bits,
+		.common_regs = &sci_common_regs,
 	},
 
 	/*
@@ -412,6 +354,8 @@ static const struct sci_port_params sci_port_params[SCIx_NR_REGTYPES] = {
 		.sampling_rate_mask = SCI_SR(32),
 		.error_mask = SCIF_DEFAULT_ERROR_MASK,
 		.error_clear = SCIF_ERROR_CLEAR,
+		.param_bits = &sci_scif_port_params_bits,
+		.common_regs = &sci_common_regs,
 	},
 
 	/*
@@ -439,6 +383,8 @@ static const struct sci_port_params sci_port_params[SCIx_NR_REGTYPES] = {
 		.sampling_rate_mask = SCI_SR(32),
 		.error_mask = SCIF_DEFAULT_ERROR_MASK,
 		.error_clear = SCIF_ERROR_CLEAR,
+		.param_bits = &sci_scif_port_params_bits,
+		.common_regs = &sci_common_regs,
 	},
 
 	/*
@@ -468,6 +414,8 @@ static const struct sci_port_params sci_port_params[SCIx_NR_REGTYPES] = {
 		.sampling_rate_mask = SCI_SR_RANGE(8, 32),
 		.error_mask = SCIF_DEFAULT_ERROR_MASK,
 		.error_clear = SCIF_ERROR_CLEAR,
+		.param_bits = &sci_scif_port_params_bits,
+		.common_regs = &sci_common_regs,
 	},
 
 	/*
@@ -492,6 +440,8 @@ static const struct sci_port_params sci_port_params[SCIx_NR_REGTYPES] = {
 		.sampling_rate_mask = SCI_SR(32),
 		.error_mask = SCIF_DEFAULT_ERROR_MASK,
 		.error_clear = SCIF_ERROR_CLEAR,
+		.param_bits = &sci_scif_port_params_bits,
+		.common_regs = &sci_common_regs,
 	},
 
 	/*
@@ -519,6 +469,8 @@ static const struct sci_port_params sci_port_params[SCIx_NR_REGTYPES] = {
 		.sampling_rate_mask = SCI_SR(32),
 		.error_mask = SCIF_DEFAULT_ERROR_MASK,
 		.error_clear = SCIF_ERROR_CLEAR,
+		.param_bits = &sci_scif_port_params_bits,
+		.common_regs = &sci_common_regs,
 	},
 
 	/*
@@ -542,6 +494,8 @@ static const struct sci_port_params sci_port_params[SCIx_NR_REGTYPES] = {
 		.sampling_rate_mask = SCI_SR(16),
 		.error_mask = SCIF_DEFAULT_ERROR_MASK | SCIFA_ORER,
 		.error_clear = SCIF_ERROR_CLEAR & ~SCIFA_ORER,
+		.param_bits = &sci_scif_port_params_bits,
+		.common_regs = &sci_common_regs,
 	},
 };
 
@@ -579,7 +533,7 @@ static void sci_serial_out(struct uart_port *p, int offset, int value)
 		WARN(1, "Invalid register access\n");
 }
 
-static void sci_port_enable(struct sci_port *sci_port)
+void sci_port_enable(struct sci_port *sci_port)
 {
 	unsigned int i;
 
@@ -595,7 +549,7 @@ static void sci_port_enable(struct sci_port *sci_port)
 	sci_port->port.uartclk = sci_port->clk_rates[SCI_FCK];
 }
 
-static void sci_port_disable(struct sci_port *sci_port)
+void sci_port_disable(struct sci_port *sci_port)
 {
 	unsigned int i;
 
@@ -735,12 +689,13 @@ static void sci_clear_SCxSR(struct uart_port *port, unsigned int mask)
 static int sci_poll_get_char(struct uart_port *port)
 {
 	unsigned short status;
+	struct sci_port *s = to_sci_port(port);
 	int c;
 
 	do {
 		status = sci_serial_in(port, SCxSR);
 		if (status & SCxSR_ERRORS(port)) {
-			sci_clear_SCxSR(port, SCxSR_ERROR_CLEAR(port));
+			s->ops->clear_SCxSR(port, SCxSR_ERROR_CLEAR(port));
 			continue;
 		}
 		break;
@@ -753,7 +708,7 @@ static int sci_poll_get_char(struct uart_port *port)
 
 	/* Dummy read */
 	sci_serial_in(port, SCxSR);
-	sci_clear_SCxSR(port, SCxSR_RDxF_CLEAR(port));
+	s->ops->clear_SCxSR(port, SCxSR_RDxF_CLEAR(port));
 
 	return c;
 }
@@ -761,14 +716,16 @@ static int sci_poll_get_char(struct uart_port *port)
 
 static void sci_poll_put_char(struct uart_port *port, unsigned char c)
 {
-	unsigned short status;
+	struct sci_port *s = to_sci_port(port);
+	const struct sci_common_regs *regs = s->params->common_regs;
+	unsigned int status;
 
 	do {
-		status = sci_serial_in(port, SCxSR);
+		status = s->ops->read_reg(port, regs->status);
 	} while (!(status & SCxSR_TDxE(port)));
 
 	sci_serial_out(port, SCxTDR, c);
-	sci_clear_SCxSR(port, SCxSR_TDxE_CLEAR(port) & ~SCxSR_TEND(port));
+	s->ops->clear_SCxSR(port, SCxSR_TDxE_CLEAR(port) & ~SCxSR_TEND(port));
 }
 #endif /* CONFIG_CONSOLE_POLL || CONFIG_SERIAL_SH_SCI_CONSOLE ||
 	  CONFIG_SERIAL_SH_SCI_EARLYCON */
@@ -911,7 +868,7 @@ static void sci_transmit_chars(struct uart_port *port)
 		port->icount.tx++;
 	} while (--count > 0);
 
-	sci_clear_SCxSR(port, SCxSR_TDxE_CLEAR(port));
+	s->ops->clear_SCxSR(port, SCxSR_TDxE_CLEAR(port));
 
 	if (kfifo_len(&tport->xmit_fifo) < WAKEUP_CHARS)
 		uart_write_wakeup(port);
@@ -930,6 +887,7 @@ static void sci_transmit_chars(struct uart_port *port)
 static void sci_receive_chars(struct uart_port *port)
 {
 	struct tty_port *tport = &port->state->port;
+	struct sci_port *s = to_sci_port(port);
 	int i, count, copied = 0;
 	unsigned short status;
 	unsigned char flag;
@@ -984,7 +942,7 @@ static void sci_receive_chars(struct uart_port *port)
 		}
 
 		sci_serial_in(port, SCxSR); /* dummy read */
-		sci_clear_SCxSR(port, SCxSR_RDxF_CLEAR(port));
+		s->ops->clear_SCxSR(port, SCxSR_RDxF_CLEAR(port));
 
 		copied += count;
 		port->icount.rx += count;
@@ -997,16 +955,17 @@ static void sci_receive_chars(struct uart_port *port)
 		/* TTY buffers full; read from RX reg to prevent lockup */
 		sci_serial_in(port, SCxRDR);
 		sci_serial_in(port, SCxSR); /* dummy read */
-		sci_clear_SCxSR(port, SCxSR_RDxF_CLEAR(port));
+		s->ops->clear_SCxSR(port, SCxSR_RDxF_CLEAR(port));
 	}
 }
 
 static int sci_handle_errors(struct uart_port *port)
 {
 	int copied = 0;
-	unsigned short status = sci_serial_in(port, SCxSR);
-	struct tty_port *tport = &port->state->port;
 	struct sci_port *s = to_sci_port(port);
+	const struct sci_common_regs *regs = s->params->common_regs;
+	unsigned int status = s->ops->read_reg(port, regs->status);
+	struct tty_port *tport = &port->state->port;
 
 	/* Handle overruns */
 	if (status & s->params->overrun_mask) {
@@ -1165,7 +1124,7 @@ static void rx_fifo_timer_fn(struct timer_list *t)
 	struct uart_port *port = &s->port;
 
 	dev_dbg(port->dev, "Rx timed out\n");
-	scif_set_rtrg(port, 1);
+	s->ops->set_rtrg(port, 1);
 }
 
 static ssize_t rx_fifo_trigger_show(struct device *dev,
@@ -1190,9 +1149,9 @@ static ssize_t rx_fifo_trigger_store(struct device *dev,
 	if (ret)
 		return ret;
 
-	sci->rx_trigger = scif_set_rtrg(port, r);
+	sci->rx_trigger = sci->ops->set_rtrg(port, r);
 	if (port->type == PORT_SCIFA || port->type == PORT_SCIFB)
-		scif_set_rtrg(port, 1);
+		sci->ops->set_rtrg(port, 1);
 
 	return count;
 }
@@ -1235,7 +1194,7 @@ static ssize_t rx_fifo_timeout_store(struct device *dev,
 		sci->hscif_tot = r << HSSCR_TOT_SHIFT;
 	} else {
 		sci->rx_fifo_timeout = r;
-		scif_set_rtrg(port, 1);
+		sci->ops->set_rtrg(port, 1);
 		if (r > 0)
 			timer_setup(&sci->rx_fifo_timer, rx_fifo_timer_fn, 0);
 	}
@@ -1360,7 +1319,7 @@ static void sci_dma_rx_reenable_irq(struct sci_port *s)
 	    s->cfg->regtype == SCIx_RZ_SCIFA_REGTYPE) {
 		enable_irq(s->irqs[SCIx_RXI_IRQ]);
 		if (s->cfg->regtype == SCIx_RZ_SCIFA_REGTYPE)
-			scif_set_rtrg(port, s->rx_trigger);
+			s->ops->set_rtrg(port, s->rx_trigger);
 		else
 			scr &= ~SCSCR_RDRQE;
 	}
@@ -1798,7 +1757,7 @@ static irqreturn_t sci_rx_interrupt(int irq, void *ptr)
 		    s->cfg->regtype == SCIx_RZ_SCIFA_REGTYPE) {
 			disable_irq_nosync(s->irqs[SCIx_RXI_IRQ]);
 			if (s->cfg->regtype == SCIx_RZ_SCIFA_REGTYPE) {
-				scif_set_rtrg(port, 1);
+				s->ops->set_rtrg(port, 1);
 				scr |= SCSCR_RIE;
 			} else {
 				scr |= SCSCR_RDRQE;
@@ -1824,8 +1783,8 @@ 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);
+		if (!s->ops->rtrg_enabled(port))
+			s->ops->set_rtrg(port, s->rx_trigger);
 
 		mod_timer(&s->rx_fifo_timer, jiffies + DIV_ROUND_UP(
 			  s->rx_frame * HZ * s->rx_fifo_timeout, 1000000));
@@ -1835,7 +1794,7 @@ static irqreturn_t sci_rx_interrupt(int irq, void *ptr)
 	 * of whether the I_IXOFF is set, otherwise, how is the interrupt
 	 * to be disabled?
 	 */
-	sci_receive_chars(port);
+	s->ops->receive_chars(port);
 
 	return IRQ_HANDLED;
 }
@@ -1844,9 +1803,10 @@ static irqreturn_t sci_tx_interrupt(int irq, void *ptr)
 {
 	struct uart_port *port = ptr;
 	unsigned long flags;
+	struct sci_port *s = to_sci_port(port);
 
 	uart_port_lock_irqsave(port, &flags);
-	sci_transmit_chars(port);
+	s->ops->transmit_chars(port);
 	uart_port_unlock_irqrestore(port, flags);
 
 	return IRQ_HANDLED;
@@ -1855,16 +1815,18 @@ static irqreturn_t sci_tx_interrupt(int irq, void *ptr)
 static irqreturn_t sci_tx_end_interrupt(int irq, void *ptr)
 {
 	struct uart_port *port = ptr;
+	struct sci_port *s = to_sci_port(port);
+	const struct sci_common_regs *regs = s->params->common_regs;
 	unsigned long flags;
-	unsigned short ctrl;
+	u32 ctrl;
 
 	if (port->type != PORT_SCI)
 		return sci_tx_interrupt(irq, ptr);
 
 	uart_port_lock_irqsave(port, &flags);
-	ctrl = sci_serial_in(port, SCSCR);
-	ctrl &= ~(SCSCR_TE | SCSCR_TEIE);
-	sci_serial_out(port, SCSCR, ctrl);
+	ctrl = s->ops->read_reg(port, regs->control) &
+		~(s->params->param_bits->te_clear);
+	s->ops->write_reg(port, regs->control, ctrl);
 	uart_port_unlock_irqrestore(port, flags);
 
 	return IRQ_HANDLED;
@@ -1873,6 +1835,7 @@ static irqreturn_t sci_tx_end_interrupt(int irq, void *ptr)
 static irqreturn_t sci_br_interrupt(int irq, void *ptr)
 {
 	struct uart_port *port = ptr;
+	struct sci_port *s = to_sci_port(port);
 
 	/* Handle BREAKs */
 	sci_handle_breaks(port);
@@ -1880,7 +1843,7 @@ static irqreturn_t sci_br_interrupt(int irq, void *ptr)
 	/* drop invalid character received before break was detected */
 	sci_serial_in(port, SCxRDR);
 
-	sci_clear_SCxSR(port, SCxSR_BREAK_CLEAR(port));
+	s->ops->clear_SCxSR(port, SCxSR_BREAK_CLEAR(port));
 
 	return IRQ_HANDLED;
 }
@@ -1908,15 +1871,15 @@ static irqreturn_t sci_er_interrupt(int irq, void *ptr)
 		if (sci_handle_errors(port)) {
 			/* discard character in rx buffer */
 			sci_serial_in(port, SCxSR);
-			sci_clear_SCxSR(port, SCxSR_RDxF_CLEAR(port));
+			s->ops->clear_SCxSR(port, SCxSR_RDxF_CLEAR(port));
 		}
 	} else {
 		sci_handle_fifo_overrun(port);
 		if (!s->chan_rx)
-			sci_receive_chars(port);
+			s->ops->receive_chars(port);
 	}
 
-	sci_clear_SCxSR(port, SCxSR_ERROR_CLEAR(port));
+	s->ops->clear_SCxSR(port, SCxSR_ERROR_CLEAR(port));
 
 	/* Kick the transmission */
 	if (!s->chan_tx)
@@ -2286,7 +2249,17 @@ static void sci_break_ctl(struct uart_port *port, int break_state)
 	uart_port_unlock_irqrestore(port, flags);
 }
 
-static int sci_startup(struct uart_port *port)
+static void sci_shutdown_complete(struct uart_port *port)
+{
+	struct sci_port *s = to_sci_port(port);
+	u16 scr;
+
+	scr = sci_serial_in(port, SCSCR);
+	sci_serial_out(port, SCSCR,
+		       scr & (SCSCR_CKE1 | SCSCR_CKE0 | s->hscif_tot));
+}
+
+int sci_startup(struct uart_port *port)
 {
 	struct sci_port *s = to_sci_port(port);
 	int ret;
@@ -2309,7 +2282,6 @@ static void sci_shutdown(struct uart_port *port)
 {
 	struct sci_port *s = to_sci_port(port);
 	unsigned long flags;
-	u16 scr;
 
 	dev_dbg(port->dev, "%s(%d)\n", __func__, port->line);
 
@@ -2319,13 +2291,7 @@ static void sci_shutdown(struct uart_port *port)
 	uart_port_lock_irqsave(port, &flags);
 	sci_stop_rx(port);
 	sci_stop_tx(port);
-	/*
-	 * Stop RX and TX, disable related interrupts, keep clock source
-	 * and HSCIF TOT bits
-	 */
-	scr = sci_serial_in(port, SCSCR);
-	sci_serial_out(port, SCSCR,
-		       scr & (SCSCR_CKE1 | SCSCR_CKE0 | s->hscif_tot));
+	s->ops->shutdown_complete(port);
 	uart_port_unlock_irqrestore(port, flags);
 
 #ifdef CONFIG_SERIAL_SH_SCI_DMA
@@ -2402,8 +2368,8 @@ static int sci_brg_calc(struct sci_port *s, unsigned int bps,
 
 /* calculate sample rate, BRR, and clock select */
 static int sci_scbrr_calc(struct sci_port *s, unsigned int bps,
-			  unsigned int *brr, unsigned int *srr,
-			  unsigned int *cks)
+		   unsigned int *brr, unsigned int *srr,
+		   unsigned int *cks)
 {
 	unsigned long freq = s->clk_rates[SCI_FCK];
 	unsigned int sr, br, prediv, scrate, c;
@@ -2480,9 +2446,9 @@ static void sci_reset(struct uart_port *port)
 	if (reg->size)
 		sci_serial_out(port, SCFCR, SCFCR_RFRST | SCFCR_TFRST);
 
-	sci_clear_SCxSR(port,
-			SCxSR_RDxF_CLEAR(port) & SCxSR_ERROR_CLEAR(port) &
-			SCxSR_BREAK_CLEAR(port));
+	s->ops->clear_SCxSR(port,
+			    SCxSR_RDxF_CLEAR(port) & SCxSR_ERROR_CLEAR(port) &
+			    SCxSR_BREAK_CLEAR(port));
 	if (sci_getreg(port, SCLSR)->size) {
 		status = sci_serial_in(port, SCLSR);
 		status &= ~(SCLSR_TO | SCLSR_ORER);
@@ -2491,14 +2457,14 @@ static void sci_reset(struct uart_port *port)
 
 	if (s->rx_trigger > 1) {
 		if (s->rx_fifo_timeout) {
-			scif_set_rtrg(port, 1);
+			s->ops->set_rtrg(port, 1);
 			timer_setup(&s->rx_fifo_timer, rx_fifo_timer_fn, 0);
 		} else {
 			if (port->type == PORT_SCIFA ||
 			    port->type == PORT_SCIFB)
-				scif_set_rtrg(port, 1);
+				s->ops->set_rtrg(port, 1);
 			else
-				scif_set_rtrg(port, s->rx_trigger);
+				s->ops->set_rtrg(port, s->rx_trigger);
 		}
 	}
 }
@@ -2758,7 +2724,7 @@ static void sci_set_termios(struct uart_port *port, struct ktermios *termios,
 		sci_enable_ms(port);
 }
 
-static void sci_pm(struct uart_port *port, unsigned int state,
+void sci_pm(struct uart_port *port, unsigned int state,
 		   unsigned int oldstate)
 {
 	struct sci_port *sci_port = to_sci_port(port);
@@ -2821,7 +2787,7 @@ static int sci_remap_port(struct uart_port *port)
 	return 0;
 }
 
-static void sci_release_port(struct uart_port *port)
+void sci_release_port(struct uart_port *port)
 {
 	struct sci_port *sport = to_sci_port(port);
 
@@ -2833,7 +2799,7 @@ static void sci_release_port(struct uart_port *port)
 	release_mem_region(port->mapbase, sport->reg_size);
 }
 
-static int sci_request_port(struct uart_port *port)
+int sci_request_port(struct uart_port *port)
 {
 	struct resource *res;
 	struct sci_port *sport = to_sci_port(port);
@@ -2855,7 +2821,7 @@ static int sci_request_port(struct uart_port *port)
 	return 0;
 }
 
-static void sci_config_port(struct uart_port *port, int flags)
+void sci_config_port(struct uart_port *port, int flags)
 {
 	if (flags & UART_CONFIG_TYPE) {
 		struct sci_port *sport = to_sci_port(port);
@@ -2865,7 +2831,7 @@ static void sci_config_port(struct uart_port *port, int flags)
 	}
 }
 
-static int sci_verify_port(struct uart_port *port, struct serial_struct *ser)
+int sci_verify_port(struct uart_port *port, struct serial_struct *ser)
 {
 	if (ser->baud_base < 2400)
 		/* No paper tape reader for Mitch.. */
@@ -2874,6 +2840,75 @@ static int sci_verify_port(struct uart_port *port, struct serial_struct *ser)
 	return 0;
 }
 
+static void sci_prepare_console_write(struct uart_port *port, u32 ctrl)
+{
+	struct sci_port *s = to_sci_port(port);
+	u32 ctrl_temp =
+		s->params->param_bits->rxtx_enable |
+		(s->cfg->scscr & ~(SCSCR_CKE1 | SCSCR_CKE0)) |
+		(ctrl & (SCSCR_CKE1 | SCSCR_CKE0)) |
+		s->hscif_tot;
+	sci_serial_out(port, SCSCR, ctrl_temp);
+}
+
+static void sci_console_save(struct uart_port *port)
+{
+	struct sci_port *s = to_sci_port(port);
+	struct sci_suspend_regs *regs = s->suspend_regs;
+
+	if (sci_getreg(port, SCDL)->size)
+		regs->scdl = sci_serial_in(port, SCDL);
+	if (sci_getreg(port, SCCKS)->size)
+		regs->sccks = sci_serial_in(port, SCCKS);
+	if (sci_getreg(port, SCSMR)->size)
+		regs->scsmr = sci_serial_in(port, SCSMR);
+	if (sci_getreg(port, SCSCR)->size)
+		regs->scscr = sci_serial_in(port, SCSCR);
+	if (sci_getreg(port, SCFCR)->size)
+		regs->scfcr = sci_serial_in(port, SCFCR);
+	if (sci_getreg(port, SCSPTR)->size)
+		regs->scsptr = sci_serial_in(port, SCSPTR);
+	if (sci_getreg(port, SCBRR)->size)
+		regs->scbrr = sci_serial_in(port, SCBRR);
+	if (sci_getreg(port, HSSRR)->size)
+		regs->hssrr = sci_serial_in(port, HSSRR);
+	if (sci_getreg(port, SCPCR)->size)
+		regs->scpcr = sci_serial_in(port, SCPCR);
+	if (sci_getreg(port, SCPDR)->size)
+		regs->scpdr = sci_serial_in(port, SCPDR);
+	if (sci_getreg(port, SEMR)->size)
+		regs->semr = sci_serial_in(port, SEMR);
+}
+
+static void sci_console_restore(struct uart_port *port)
+{
+	struct sci_port *s = to_sci_port(port);
+	struct sci_suspend_regs *regs = s->suspend_regs;
+
+	if (sci_getreg(port, SCDL)->size)
+		sci_serial_out(port, SCDL, regs->scdl);
+	if (sci_getreg(port, SCCKS)->size)
+		sci_serial_out(port, SCCKS, regs->sccks);
+	if (sci_getreg(port, SCSMR)->size)
+		sci_serial_out(port, SCSMR, regs->scsmr);
+	if (sci_getreg(port, SCSCR)->size)
+		sci_serial_out(port, SCSCR, regs->scscr);
+	if (sci_getreg(port, SCFCR)->size)
+		sci_serial_out(port, SCFCR, regs->scfcr);
+	if (sci_getreg(port, SCSPTR)->size)
+		sci_serial_out(port, SCSPTR, regs->scsptr);
+	if (sci_getreg(port, SCBRR)->size)
+		sci_serial_out(port, SCBRR, regs->scbrr);
+	if (sci_getreg(port, HSSRR)->size)
+		sci_serial_out(port, HSSRR, regs->hssrr);
+	if (sci_getreg(port, SCPCR)->size)
+		sci_serial_out(port, SCPCR, regs->scpcr);
+	if (sci_getreg(port, SCPDR)->size)
+		sci_serial_out(port, SCPDR, regs->scpdr);
+	if (sci_getreg(port, SEMR)->size)
+		sci_serial_out(port, SEMR, regs->semr);
+}
+
 static const struct uart_ops sci_uart_ops = {
 	.tx_empty	= sci_tx_empty,
 	.set_mctrl	= sci_set_mctrl,
@@ -2899,6 +2934,25 @@ static const struct uart_ops sci_uart_ops = {
 #endif
 };
 
+static const struct sci_port_ops sci_port_ops = {
+	.read_reg		= sci_serial_in,
+	.write_reg		= sci_serial_out,
+	.clear_SCxSR		= sci_clear_SCxSR,
+	.transmit_chars		= sci_transmit_chars,
+	.receive_chars		= sci_receive_chars,
+#if defined(CONFIG_SERIAL_SH_SCI_CONSOLE) || \
+    defined(CONFIG_SERIAL_SH_SCI_EARLYCON)
+	.poll_put_char		= sci_poll_put_char,
+#endif
+	.set_rtrg		= scif_set_rtrg,
+	.rtrg_enabled		= scif_rtrg_enabled,
+	.shutdown_complete	= sci_shutdown_complete,
+	.prepare_console_write	= sci_prepare_console_write,
+	.console_save		= sci_console_save,
+	.console_restore	= sci_console_restore,
+	.suspend_regs_size	= sci_suspend_regs_size,
+};
+
 static int sci_init_clocks(struct sci_port *sci_port, struct device *dev)
 {
 	const char *clk_names[] = {
@@ -2992,6 +3046,7 @@ static int sci_init_single(struct platform_device *dev,
 	int ret;
 
 	sci_port->cfg	= p;
+	sci_port->ops	= &sci_port_ops;
 
 	port->ops	= &sci_uart_ops;
 	port->iotype	= UPIO_MEM;
@@ -3104,7 +3159,7 @@ static int sci_init_single(struct platform_device *dev,
     defined(CONFIG_SERIAL_SH_SCI_EARLYCON)
 static void serial_console_putchar(struct uart_port *port, unsigned char ch)
 {
-	sci_poll_put_char(port, ch);
+	to_sci_port(port)->ops->poll_put_char(port, ch);
 }
 
 /*
@@ -3116,7 +3171,9 @@ static void serial_console_write(struct console *co, const char *s,
 {
 	struct sci_port *sci_port = &sci_ports[co->index];
 	struct uart_port *port = &sci_port->port;
-	unsigned short bits, ctrl, ctrl_temp;
+	const struct sci_common_regs *regs = sci_port->params->common_regs;
+	unsigned int bits;
+	u32 ctrl;
 	unsigned long flags;
 	int locked = 1;
 
@@ -3128,21 +3185,21 @@ static void serial_console_write(struct console *co, const char *s,
 		uart_port_lock_irqsave(port, &flags);
 
 	/* first save SCSCR then disable interrupts, keep clock source */
-	ctrl = sci_serial_in(port, SCSCR);
-	ctrl_temp = SCSCR_RE | SCSCR_TE |
-		    (sci_port->cfg->scscr & ~(SCSCR_CKE1 | SCSCR_CKE0)) |
-		    (ctrl & (SCSCR_CKE1 | SCSCR_CKE0));
-	sci_serial_out(port, SCSCR, ctrl_temp | sci_port->hscif_tot);
+
+	ctrl = sci_port->ops->read_reg(port, regs->control);
+	sci_port->ops->prepare_console_write(port, ctrl);
 
 	uart_console_write(port, s, count, serial_console_putchar);
 
 	/* wait until fifo is empty and last bit has been transmitted */
-	bits = SCxSR_TDxE(port) | SCxSR_TEND(port);
-	while ((sci_serial_in(port, SCxSR) & bits) != bits)
+
+	bits = sci_port->params->param_bits->poll_sent_bits;
+
+	while ((sci_port->ops->read_reg(port, regs->status) & bits) != bits)
 		cpu_relax();
 
 	/* restore the SCSCR */
-	sci_serial_out(port, SCSCR, ctrl);
+	sci_port->ops->write_reg(port, regs->control, ctrl);
 
 	if (locked)
 		uart_port_unlock_irqrestore(port, flags);
@@ -3275,7 +3332,6 @@ static void sci_remove(struct platform_device *dev)
 		device_remove_file(&dev->dev, &dev_attr_rx_fifo_timeout);
 }
 
-
 #define SCI_OF_DATA(type, regtype)	(void *)((type) << 16 | (regtype))
 #define SCI_OF_TYPE(data)		((unsigned long)(data) >> 16)
 #define SCI_OF_REGTYPE(data)		((unsigned long)(data) & 0xffff)
@@ -3512,6 +3568,11 @@ static int sci_probe(struct platform_device *dev)
 	}
 
 	sp = &sci_ports[dev_id];
+	sp->suspend_regs = devm_kzalloc(&dev->dev,
+					sp->ops->suspend_regs_size(),
+					GFP_KERNEL);
+	if (!sp->suspend_regs)
+		return -ENOMEM;
 
 	/*
 	 * In case:
@@ -3563,64 +3624,6 @@ static int sci_probe(struct platform_device *dev)
 	return 0;
 }
 
-static void sci_console_save(struct sci_port *s)
-{
-	struct sci_suspend_regs *regs = &s->suspend_regs;
-	struct uart_port *port = &s->port;
-
-	if (sci_getreg(port, SCDL)->size)
-		regs->scdl = sci_serial_in(port, SCDL);
-	if (sci_getreg(port, SCCKS)->size)
-		regs->sccks = sci_serial_in(port, SCCKS);
-	if (sci_getreg(port, SCSMR)->size)
-		regs->scsmr = sci_serial_in(port, SCSMR);
-	if (sci_getreg(port, SCSCR)->size)
-		regs->scscr = sci_serial_in(port, SCSCR);
-	if (sci_getreg(port, SCFCR)->size)
-		regs->scfcr = sci_serial_in(port, SCFCR);
-	if (sci_getreg(port, SCSPTR)->size)
-		regs->scsptr = sci_serial_in(port, SCSPTR);
-	if (sci_getreg(port, SCBRR)->size)
-		regs->scbrr = sci_serial_in(port, SCBRR);
-	if (sci_getreg(port, HSSRR)->size)
-		regs->hssrr = sci_serial_in(port, HSSRR);
-	if (sci_getreg(port, SCPCR)->size)
-		regs->scpcr = sci_serial_in(port, SCPCR);
-	if (sci_getreg(port, SCPDR)->size)
-		regs->scpdr = sci_serial_in(port, SCPDR);
-	if (sci_getreg(port, SEMR)->size)
-		regs->semr = sci_serial_in(port, SEMR);
-}
-
-static void sci_console_restore(struct sci_port *s)
-{
-	struct sci_suspend_regs *regs = &s->suspend_regs;
-	struct uart_port *port = &s->port;
-
-	if (sci_getreg(port, SCDL)->size)
-		sci_serial_out(port, SCDL, regs->scdl);
-	if (sci_getreg(port, SCCKS)->size)
-		sci_serial_out(port, SCCKS, regs->sccks);
-	if (sci_getreg(port, SCSMR)->size)
-		sci_serial_out(port, SCSMR, regs->scsmr);
-	if (sci_getreg(port, SCSCR)->size)
-		sci_serial_out(port, SCSCR, regs->scscr);
-	if (sci_getreg(port, SCFCR)->size)
-		sci_serial_out(port, SCFCR, regs->scfcr);
-	if (sci_getreg(port, SCSPTR)->size)
-		sci_serial_out(port, SCSPTR, regs->scsptr);
-	if (sci_getreg(port, SCBRR)->size)
-		sci_serial_out(port, SCBRR, regs->scbrr);
-	if (sci_getreg(port, HSSRR)->size)
-		sci_serial_out(port, HSSRR, regs->hssrr);
-	if (sci_getreg(port, SCPCR)->size)
-		sci_serial_out(port, SCPCR, regs->scpcr);
-	if (sci_getreg(port, SCPDR)->size)
-		sci_serial_out(port, SCPDR, regs->scpdr);
-	if (sci_getreg(port, SEMR)->size)
-		sci_serial_out(port, SEMR, regs->semr);
-}
-
 static __maybe_unused int sci_suspend(struct device *dev)
 {
 	struct sci_port *sport = dev_get_drvdata(dev);
@@ -3628,8 +3631,10 @@ static __maybe_unused int sci_suspend(struct device *dev)
 	if (sport) {
 		uart_suspend_port(&sci_uart_driver, &sport->port);
 
-		if (!console_suspend_enabled && uart_console(&sport->port))
-			sci_console_save(sport);
+		if (!console_suspend_enabled && uart_console(&sport->port)) {
+			if (sport->ops->console_save)
+				sport->ops->console_save(&sport->port);
+		}
 		else
 			return reset_control_assert(sport->rstc);
 	}
@@ -3643,7 +3648,8 @@ static __maybe_unused int sci_resume(struct device *dev)
 
 	if (sport) {
 		if (!console_suspend_enabled && uart_console(&sport->port)) {
-			sci_console_restore(sport);
+			if (sport->ops->console_restore)
+				sport->ops->console_restore(&sport->port);
 		} else {
 			int ret = reset_control_deassert(sport->rstc);
 
@@ -3707,9 +3713,11 @@ static int early_console_exit(struct console *co)
 	return 0;
 }
 
-static int __init early_console_setup(struct earlycon_device *device,
+int __init scix_early_console_setup(struct earlycon_device *device,
 				      int type)
 {
+	const struct sci_common_regs *regs;
+
 	if (!device->port.membase)
 		return -ENODEV;
 
@@ -3717,11 +3725,15 @@ static int __init early_console_setup(struct earlycon_device *device,
 	sci_ports[0].port = device->port;
 	port_cfg.type = type;
 	sci_ports[0].cfg = &port_cfg;
+	sci_ports[0].ops = &sci_port_ops;
 	sci_ports[0].params = sci_probe_regmap(&port_cfg);
 	sci_uart_earlycon = true;
-	port_cfg.scscr = sci_serial_in(&sci_ports[0].port, SCSCR);
-	sci_serial_out(&sci_ports[0].port, SCSCR,
-		       SCSCR_RE | SCSCR_TE | port_cfg.scscr);
+	regs = sci_ports[0].params->common_regs;
+
+	port_cfg.scscr = sci_ports[0].ops->read_reg(&sci_ports[0].port, regs->control);
+	sci_ports[0].ops->write_reg(&sci_ports[0].port,
+				    regs->control,
+				    sci_ports[0].params->param_bits->rxtx_enable | port_cfg.scscr);
 
 	device->con->write = serial_console_write;
 	device->con->exit = early_console_exit;
@@ -3731,41 +3743,41 @@ static int __init early_console_setup(struct earlycon_device *device,
 static int __init sci_early_console_setup(struct earlycon_device *device,
 					  const char *opt)
 {
-	return early_console_setup(device, PORT_SCI);
+	return scix_early_console_setup(device, PORT_SCI);
 }
 static int __init scif_early_console_setup(struct earlycon_device *device,
 					  const char *opt)
 {
-	return early_console_setup(device, PORT_SCIF);
+	return scix_early_console_setup(device, PORT_SCIF);
 }
 static int __init rzscifa_early_console_setup(struct earlycon_device *device,
 					  const char *opt)
 {
 	port_cfg.regtype = SCIx_RZ_SCIFA_REGTYPE;
-	return early_console_setup(device, PORT_SCIF);
+	return scix_early_console_setup(device, PORT_SCIF);
 }
 
 static int __init rzv2hscif_early_console_setup(struct earlycon_device *device,
 						const char *opt)
 {
 	port_cfg.regtype = SCIx_RZV2H_SCIF_REGTYPE;
-	return early_console_setup(device, PORT_SCIF);
+	return scix_early_console_setup(device, PORT_SCIF);
 }
 
 static int __init scifa_early_console_setup(struct earlycon_device *device,
 					  const char *opt)
 {
-	return early_console_setup(device, PORT_SCIFA);
+	return scix_early_console_setup(device, PORT_SCIFA);
 }
 static int __init scifb_early_console_setup(struct earlycon_device *device,
 					  const char *opt)
 {
-	return early_console_setup(device, PORT_SCIFB);
+	return scix_early_console_setup(device, PORT_SCIFB);
 }
 static int __init hscif_early_console_setup(struct earlycon_device *device,
 					  const char *opt)
 {
-	return early_console_setup(device, PORT_HSCIF);
+	return scix_early_console_setup(device, PORT_HSCIF);
 }
 
 OF_EARLYCON_DECLARE(sci, "renesas,sci", sci_early_console_setup);
diff --git a/drivers/tty/serial/sh-sci.h b/drivers/tty/serial/sh-sci.h
index 0b65563c4e9e..951681aba586 100644
--- a/drivers/tty/serial/sh-sci.h
+++ b/drivers/tty/serial/sh-sci.h
@@ -32,8 +32,6 @@ enum {
 	HSRTRGR,			/* Rx FIFO Data Count Trigger Register */
 	HSTTRGR,			/* Tx FIFO Data Count Trigger Register */
 	SEMR,				/* Serial extended mode register */
-
-	SCIx_NR_REGS,
 };
 
 
-- 
2.43.0

[PATCH v4 09/13] serial: sh-sci: Introduced sci_of_data

[Thierry Bultel]

The aim here is to provide an easier support to more different SCI
controllers, like the RZ/T2H one.

The existing .data field of_sci_match is changed to a structure containing
all what that can be statically initialized, and avoid a call to
'sci_probe_regmap', in both 'sci_init_single', and 'early_console_setup'.

'sci_probe_regmap' is now assumed to be called in the only case where the
device description is from a board file instead of a dts.

In this way, there is no need to patch 'sci_probe_regmap' for adding new
SCI type, and also, the specific sci_port_params for a new SCI type can be
provided by an external file.

Signed-off-by: Thierry Bultel <thierry.bultel.yh@bp.renesas.com>
---
Changes v3->v4:
   - Fix the bot compilation error on superh in sci_probe_earlyprink()
---
 drivers/tty/serial/sh-sci-common.h |  10 +-
 drivers/tty/serial/sh-sci.c        | 164 +++++++++++++++++++++--------
 2 files changed, 131 insertions(+), 43 deletions(-)

diff --git a/drivers/tty/serial/sh-sci-common.h b/drivers/tty/serial/sh-sci-common.h
index 3ea116033d53..16b41da4afd9 100644
--- a/drivers/tty/serial/sh-sci-common.h
+++ b/drivers/tty/serial/sh-sci-common.h
@@ -89,6 +89,14 @@ struct sci_port_ops {
 	size_t (*suspend_regs_size)(void);
 };
 
+struct sci_of_data {
+	const struct sci_port_params *params;
+	const struct uart_ops *uart_ops;
+	const struct sci_port_ops *ops;
+	unsigned short regtype;
+	unsigned short type;
+};
+
 struct sci_port {
 	struct uart_port	port;
 
@@ -152,7 +160,7 @@ int sci_startup(struct uart_port *port);
 #define max_sr(_port)		fls((_port)->sampling_rate_mask)
 
 #ifdef CONFIG_SERIAL_SH_SCI_EARLYCON
-int __init scix_early_console_setup(struct earlycon_device *device, int);
+int __init scix_early_console_setup(struct earlycon_device *device, const struct sci_of_data *data);
 #endif
 
 #endif /* __SH_SCI_COMMON_H__ */
diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
index 4e82055a41c8..a8e7f739c4ec 100644
--- a/drivers/tty/serial/sh-sci.c
+++ b/drivers/tty/serial/sh-sci.c
@@ -2996,10 +2996,13 @@ static int sci_init_clocks(struct sci_port *sci_port, struct device *dev)
 }
 
 static const struct sci_port_params *
-sci_probe_regmap(const struct plat_sci_port *cfg)
+sci_probe_regmap(const struct plat_sci_port *cfg, struct sci_port *sci_port)
 {
 	unsigned int regtype;
 
+	sci_port->ops = &sci_port_ops;
+	sci_port->port.ops = &sci_uart_ops;
+
 	if (cfg->regtype != SCIx_PROBE_REGTYPE)
 		return &sci_port_params[cfg->regtype];
 
@@ -3046,9 +3049,7 @@ static int sci_init_single(struct platform_device *dev,
 	int ret;
 
 	sci_port->cfg	= p;
-	sci_port->ops	= &sci_port_ops;
 
-	port->ops	= &sci_uart_ops;
 	port->iotype	= UPIO_MEM;
 	port->line	= index;
 	port->has_sysrq = IS_ENABLED(CONFIG_SERIAL_SH_SCI_CONSOLE);
@@ -3088,10 +3089,6 @@ static int sci_init_single(struct platform_device *dev,
 		for (i = 1; i < ARRAY_SIZE(sci_port->irqs); i++)
 			sci_port->irqs[i] = sci_port->irqs[0];
 
-	sci_port->params = sci_probe_regmap(p);
-	if (unlikely(sci_port->params == NULL))
-		return -EINVAL;
-
 	switch (p->type) {
 	case PORT_SCIFB:
 		sci_port->rx_trigger = 48;
@@ -3277,13 +3274,18 @@ static struct console early_serial_console = {
 static int sci_probe_earlyprintk(struct platform_device *pdev)
 {
 	const struct plat_sci_port *cfg = dev_get_platdata(&pdev->dev);
+	struct sci_port *sp = &sci_ports[pdev->id];
 
 	if (early_serial_console.data)
 		return -EEXIST;
 
 	early_serial_console.index = pdev->id;
 
-	sci_init_single(pdev, &sci_ports[pdev->id], pdev->id, cfg, true);
+	sp->params = sci_probe_regmap(cfg, sp);
+	if (!sp->params)
+		return -ENODEV;
+
+	sci_init_single(pdev, sp, pdev->id, cfg, true);
 
 	if (!strstr(early_serial_buf, "keep"))
 		early_serial_console.flags |= CON_BOOT;
@@ -3332,58 +3334,126 @@ static void sci_remove(struct platform_device *dev)
 		device_remove_file(&dev->dev, &dev_attr_rx_fifo_timeout);
 }
 
-#define SCI_OF_DATA(type, regtype)	(void *)((type) << 16 | (regtype))
-#define SCI_OF_TYPE(data)		((unsigned long)(data) >> 16)
-#define SCI_OF_REGTYPE(data)		((unsigned long)(data) & 0xffff)
+static const struct sci_of_data of_sci_scif_sh2 = {
+	.type = PORT_SCIF,
+	.regtype = SCIx_SH2_SCIF_FIFODATA_REGTYPE,
+	.ops = &sci_port_ops,
+	.uart_ops = &sci_uart_ops,
+	.params = &sci_port_params[SCIx_SH2_SCIF_FIFODATA_REGTYPE],
+};
+
+static const struct sci_of_data of_sci_scif_rz_scifa = {
+	.type = PORT_SCIF,
+	.regtype = SCIx_RZ_SCIFA_REGTYPE,
+	.ops = &sci_port_ops,
+	.uart_ops = &sci_uart_ops,
+	.params = &sci_port_params[SCIx_RZ_SCIFA_REGTYPE],
+};
+
+static const struct sci_of_data of_sci_scif_rzv2h = {
+	.type = PORT_SCIF,
+	.regtype = SCIx_RZV2H_SCIF_REGTYPE,
+	.ops = &sci_port_ops,
+	.uart_ops = &sci_uart_ops,
+	.params = &sci_port_params[SCIx_RZV2H_SCIF_REGTYPE],
+};
+
+static const struct sci_of_data of_sci_rcar_scif = {
+	.type = PORT_SCIF,
+	.regtype = SCIx_SH4_SCIF_BRG_REGTYPE,
+	.ops = &sci_port_ops,
+	.uart_ops = &sci_uart_ops,
+	.params = &sci_port_params[SCIx_SH4_SCIF_BRG_REGTYPE],
+};
+
+static const struct sci_of_data of_sci_scif_sh4 = {
+	.type = PORT_SCIF,
+	.regtype = SCIx_SH4_SCIF_REGTYPE,
+	.ops = &sci_port_ops,
+	.uart_ops = &sci_uart_ops,
+	.params = &sci_port_params[SCIx_SH4_SCIF_REGTYPE],
+};
+
+static const struct sci_of_data of_sci_scifa = {
+	.type = PORT_SCIFA,
+	.regtype = SCIx_SCIFA_REGTYPE,
+	.ops = &sci_port_ops,
+	.uart_ops = &sci_uart_ops,
+	.params = &sci_port_params[SCIx_SCIFA_REGTYPE],
+};
+
+static const struct sci_of_data of_sci_scifb = {
+	.type = PORT_SCIFB,
+	.regtype = SCIx_SCIFB_REGTYPE,
+	.ops = &sci_port_ops,
+	.uart_ops = &sci_uart_ops,
+	.params = &sci_port_params[SCIx_SCIFB_REGTYPE],
+};
+
+static const struct sci_of_data of_sci_hscif = {
+	.type = PORT_HSCIF,
+	.regtype = SCIx_HSCIF_REGTYPE,
+	.ops = &sci_port_ops,
+	.uart_ops = &sci_uart_ops,
+	.params = &sci_port_params[SCIx_HSCIF_REGTYPE],
+};
+
+static const struct sci_of_data of_sci_sci = {
+	.type = PORT_SCI,
+	.regtype = SCIx_SCI_REGTYPE,
+	.ops = &sci_port_ops,
+	.uart_ops = &sci_uart_ops,
+	.params = &sci_port_params[SCIx_SCI_REGTYPE],
+};
 
 static const struct of_device_id of_sci_match[] __maybe_unused = {
 	/* SoC-specific types */
 	{
 		.compatible = "renesas,scif-r7s72100",
-		.data = SCI_OF_DATA(PORT_SCIF, SCIx_SH2_SCIF_FIFODATA_REGTYPE),
+		.data = &of_sci_scif_sh2,
 	},
 	{
 		.compatible = "renesas,scif-r7s9210",
-		.data = SCI_OF_DATA(PORT_SCIF, SCIx_RZ_SCIFA_REGTYPE),
+		.data = &of_sci_scif_rz_scifa,
 	},
 	{
 		.compatible = "renesas,scif-r9a07g044",
-		.data = SCI_OF_DATA(PORT_SCIF, SCIx_RZ_SCIFA_REGTYPE),
+		.data = &of_sci_scif_rz_scifa,
 	},
 	{
 		.compatible = "renesas,scif-r9a09g057",
-		.data = SCI_OF_DATA(PORT_SCIF, SCIx_RZV2H_SCIF_REGTYPE),
+		.data = &of_sci_scif_rzv2h,
 	},
 	/* Family-specific types */
 	{
 		.compatible = "renesas,rcar-gen1-scif",
-		.data = SCI_OF_DATA(PORT_SCIF, SCIx_SH4_SCIF_BRG_REGTYPE),
+		.data = &of_sci_rcar_scif,
 	}, {
 		.compatible = "renesas,rcar-gen2-scif",
-		.data = SCI_OF_DATA(PORT_SCIF, SCIx_SH4_SCIF_BRG_REGTYPE),
+		.data = &of_sci_rcar_scif,
 	}, {
 		.compatible = "renesas,rcar-gen3-scif",
-		.data = SCI_OF_DATA(PORT_SCIF, SCIx_SH4_SCIF_BRG_REGTYPE),
+		.data = &of_sci_rcar_scif
 	}, {
 		.compatible = "renesas,rcar-gen4-scif",
-		.data = SCI_OF_DATA(PORT_SCIF, SCIx_SH4_SCIF_BRG_REGTYPE),
+		.data = &of_sci_rcar_scif
 	},
 	/* Generic types */
 	{
 		.compatible = "renesas,scif",
-		.data = SCI_OF_DATA(PORT_SCIF, SCIx_SH4_SCIF_REGTYPE),
+		.data = &of_sci_scif_sh4,
 	}, {
 		.compatible = "renesas,scifa",
-		.data = SCI_OF_DATA(PORT_SCIFA, SCIx_SCIFA_REGTYPE),
+		.data = &of_sci_scifa,
 	}, {
 		.compatible = "renesas,scifb",
-		.data = SCI_OF_DATA(PORT_SCIFB, SCIx_SCIFB_REGTYPE),
+		.data = &of_sci_scifb,
 	}, {
 		.compatible = "renesas,hscif",
-		.data = SCI_OF_DATA(PORT_HSCIF, SCIx_HSCIF_REGTYPE),
+		.data = &of_sci_hscif,
 	}, {
 		.compatible = "renesas,sci",
-		.data = SCI_OF_DATA(PORT_SCI, SCIx_SCI_REGTYPE),
+		.data = &of_sci_sci,
 	}, {
 		/* Terminator */
 	},
@@ -3402,7 +3472,7 @@ static struct plat_sci_port *sci_parse_dt(struct platform_device *pdev,
 	struct reset_control *rstc;
 	struct plat_sci_port *p;
 	struct sci_port *sp;
-	const void *data;
+	const struct sci_of_data *data;
 	int id, ret;
 
 	if (!IS_ENABLED(CONFIG_OF) || !np)
@@ -3449,8 +3519,12 @@ static struct plat_sci_port *sci_parse_dt(struct platform_device *pdev,
 	sp->rstc = rstc;
 	*dev_id = id;
 
-	p->type = SCI_OF_TYPE(data);
-	p->regtype = SCI_OF_REGTYPE(data);
+	p->type = data->type;
+	p->regtype = data->regtype;
+
+	sp->ops = data->ops;
+	sp->port.ops = data->uart_ops;
+	sp->params = data->params;
 
 	sp->has_rtscts = of_property_read_bool(np, "uart-has-rtscts");
 
@@ -3557,6 +3631,7 @@ static int sci_probe(struct platform_device *dev)
 		p = sci_parse_dt(dev, &dev_id);
 		if (IS_ERR(p))
 			return PTR_ERR(p);
+		sp = &sci_ports[dev_id];
 	} else {
 		p = dev->dev.platform_data;
 		if (p == NULL) {
@@ -3565,9 +3640,12 @@ static int sci_probe(struct platform_device *dev)
 		}
 
 		dev_id = dev->id;
+		sp = &sci_ports[dev_id];
+		sp->params = sci_probe_regmap(p, sp);
+		if (!sp->params)
+			return -ENODEV;
 	}
 
-	sp = &sci_ports[dev_id];
 	sp->suspend_regs = devm_kzalloc(&dev->dev,
 					sp->ops->suspend_regs_size(),
 					GFP_KERNEL);
@@ -3714,19 +3792,23 @@ static int early_console_exit(struct console *co)
 }
 
 int __init scix_early_console_setup(struct earlycon_device *device,
-				      int type)
+				    const struct sci_of_data *data)
 {
 	const struct sci_common_regs *regs;
 
 	if (!device->port.membase)
 		return -ENODEV;
 
-	device->port.type = type;
+	device->port.type = data->type;
 	sci_ports[0].port = device->port;
-	port_cfg.type = type;
+
+	port_cfg.type = data->type;
+	port_cfg.regtype = data->regtype;
+
 	sci_ports[0].cfg = &port_cfg;
-	sci_ports[0].ops = &sci_port_ops;
-	sci_ports[0].params = sci_probe_regmap(&port_cfg);
+	sci_ports[0].params = data->params;
+	sci_ports[0].ops = data->ops;
+	sci_ports[0].port.ops = data->uart_ops;
 	sci_uart_earlycon = true;
 	regs = sci_ports[0].params->common_regs;
 
@@ -3743,41 +3825,39 @@ int __init scix_early_console_setup(struct earlycon_device *device,
 static int __init sci_early_console_setup(struct earlycon_device *device,
 					  const char *opt)
 {
-	return scix_early_console_setup(device, PORT_SCI);
+	return scix_early_console_setup(device, &of_sci_sci);
 }
 static int __init scif_early_console_setup(struct earlycon_device *device,
 					  const char *opt)
 {
-	return scix_early_console_setup(device, PORT_SCIF);
+	return scix_early_console_setup(device, &of_sci_scif_sh4);
 }
 static int __init rzscifa_early_console_setup(struct earlycon_device *device,
 					  const char *opt)
 {
-	port_cfg.regtype = SCIx_RZ_SCIFA_REGTYPE;
-	return scix_early_console_setup(device, PORT_SCIF);
+	return scix_early_console_setup(device, &of_sci_scif_rz_scifa);
 }
 
 static int __init rzv2hscif_early_console_setup(struct earlycon_device *device,
 						const char *opt)
 {
-	port_cfg.regtype = SCIx_RZV2H_SCIF_REGTYPE;
-	return scix_early_console_setup(device, PORT_SCIF);
+	return scix_early_console_setup(device, &of_sci_scif_rzv2h);
 }
 
 static int __init scifa_early_console_setup(struct earlycon_device *device,
 					  const char *opt)
 {
-	return scix_early_console_setup(device, PORT_SCIFA);
+	return scix_early_console_setup(device, &of_sci_scifa);
 }
 static int __init scifb_early_console_setup(struct earlycon_device *device,
 					  const char *opt)
 {
-	return scix_early_console_setup(device, PORT_SCIFB);
+	return scix_early_console_setup(device, &of_sci_scifb);
 }
 static int __init hscif_early_console_setup(struct earlycon_device *device,
 					  const char *opt)
 {
-	return scix_early_console_setup(device, PORT_HSCIF);
+	return scix_early_console_setup(device, &of_sci_hscif);
 }
 
 OF_EARLYCON_DECLARE(sci, "renesas,sci", sci_early_console_setup);
-- 
2.43.0

[PATCH v4 10/13] serial: sh-sci: Add support for RZ/T2H SCI

[Thierry Bultel]

Define a new RZSCI port type, and the RSCI 32 bits registers set.
The RZ/T2H SCI has a a fifo, and a quite different set of registers
from the orginal SH SCI ones.
DMA is not supported yet.

Signed-off-by: Thierry Bultel <thierry.bultel.yh@bp.renesas.com>
---
Changes v3->v4:
  - Added missing #include <bitfield.h>
  - Fix christmas tree code style in rzsci_transmit_chars.
---
 drivers/tty/serial/Kconfig       |   7 +
 drivers/tty/serial/Makefile      |   1 +
 drivers/tty/serial/rzsci.c       | 466 +++++++++++++++++++++++++++++++
 drivers/tty/serial/rzsci.h       |  12 +
 drivers/tty/serial/sh-sci.c      |  20 +-
 include/linux/serial_sci.h       |   3 +-
 include/uapi/linux/serial_core.h |   3 +
 7 files changed, 506 insertions(+), 6 deletions(-)
 create mode 100644 drivers/tty/serial/rzsci.c
 create mode 100644 drivers/tty/serial/rzsci.h

diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
index 79a8186d3361..40114ae48677 100644
--- a/drivers/tty/serial/Kconfig
+++ b/drivers/tty/serial/Kconfig
@@ -675,6 +675,13 @@ config SERIAL_SH_SCI_DMA
 	depends on SERIAL_SH_SCI && DMA_ENGINE
 	default ARCH_RENESAS
 
+config SERIAL_RZ_SCI
+	tristate "Support for Renesas RZ/T2H SCI variant"
+	depends on SERIAL_SH_SCI
+	help
+	  Support for the RZ/T2H SCI variant with fifo.
+	  Say Y if you want to be able to use the RZ/T2H SCI serial port.
+
 config SERIAL_HS_LPC32XX
 	tristate "LPC32XX high speed serial port support"
 	depends on ARCH_LPC32XX || COMPILE_TEST
diff --git a/drivers/tty/serial/Makefile b/drivers/tty/serial/Makefile
index d58d9f719889..28f00ac64bcb 100644
--- a/drivers/tty/serial/Makefile
+++ b/drivers/tty/serial/Makefile
@@ -71,6 +71,7 @@ obj-$(CONFIG_SERIAL_QCOM_GENI)		+= qcom_geni_serial.o
 obj-$(CONFIG_SERIAL_QE)			+= ucc_uart.o
 obj-$(CONFIG_SERIAL_RDA)		+= rda-uart.o
 obj-$(CONFIG_SERIAL_RP2)		+= rp2.o
+obj-$(CONFIG_SERIAL_RZ_SCI)		+= rzsci.o
 obj-$(CONFIG_SERIAL_SA1100)		+= sa1100.o
 obj-$(CONFIG_SERIAL_SAMSUNG)		+= samsung_tty.o
 obj-$(CONFIG_SERIAL_SB1250_DUART)	+= sb1250-duart.o
diff --git a/drivers/tty/serial/rzsci.c b/drivers/tty/serial/rzsci.c
new file mode 100644
index 000000000000..13bf016b3e94
--- /dev/null
+++ b/drivers/tty/serial/rzsci.c
@@ -0,0 +1,466 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2025 Renesas Electronics Corp.
+ */
+
+#include <linux/bitfield.h>
+#include <linux/bitops.h>
+#include <linux/io.h>
+#include <linux/iopoll.h>
+#include <linux/serial_core.h>
+#include <linux/serial_sci.h>
+#include <linux/tty_flip.h>
+#include "rzsci.h"
+
+/* RSCI registers */
+#define RDR	0x00
+#define TDR	0x04
+#define CCR0	0x08
+#define CCR1	0x0C
+#define CCR2	0x10
+#define CCR3	0x14
+#define CCR4	0x18
+#define FCR	0x24
+#define DCR	0x30
+#define CSR	0x48
+#define FRSR	0x50
+#define FTSR	0x54
+#define CFCLR	0x68
+#define FFCLR	0x70
+
+/* RDR (Receive Data Register) */
+#define RDR_FFER		BIT(12) /* FIFO Framing Error */
+#define RDR_FPER		BIT(11) /* FIFO Parity Error */
+#define RDR_RDAT_MSK		GENMASK(8, 0)
+
+/* TDR (Transmit Data Register) */
+#define TDR_MPBT		BIT(9)	/* Multiprocessor Transfer */
+#define TDR_TDAT_9BIT_LSHIFT	0
+#define TDR_TDAT_9BIT_VAL	0x1FF
+#define TDR_TDAT_9BIT_MSK	(TDR_TDAT_9BIT_VAL << TDR_TDAT_9BIT_LSHIFT)
+
+/* CCR0 (Common Control Register 0) */
+#define CCR0_SSE		BIT(24)	/* SSn# Pin Function Enable */
+#define CCR0_TEIE		BIT(21)	/* Transmit End Interrupt Enable */
+#define CCR0_TIE		BIT(20)	/* Transmit Interrupt Enable */
+#define CCR0_RIE		BIT(16)	/* Receive Interrupt Enable */
+#define CCR0_IDSEL		BIT(10)	/* ID Frame Select */
+#define CCR0_DCME		BIT(9)	/* Data Compare Match Enable */
+#define CCR0_MPIE		BIT(8)	/* Multiprocessor Interrupt Enable */
+#define CCR0_TE			BIT(4)	/* Transmit Enable */
+#define CCR0_RE			BIT(0)	/* Receive Enable */
+
+/* CCR1 (Common Control Register 1) */
+#define CCR1_NFEN		BIT(28)	/* Digital Noise Filter Function */
+#define CCR1_SHARPS		BIT(20)	/* Half -duplex Communication Select */
+#define CCR1_SPLP		BIT(16)	/* Loopback Control */
+#define CCR1_RINV		BIT(13)	/* RxD invert */
+#define CCR1_TINV		BIT(12)	/* TxD invert */
+#define CCR1_PM			BIT(9)	/* Parity Mode */
+#define CCR1_PE			BIT(8)	/* Parity Enable */
+#define CCR1_SPB2IO		BIT(5)	/* Serial Port Break I/O */
+#define CCR1_SPB2DT		BIT(4)	/* Serial Port Break Data Select */
+#define CCR1_CTSPEN		BIT(1)	/* CTS External Pin Enable */
+#define CCR1_CTSE		BIT(0)	/* CTS Enable */
+
+/* FCR (FIFO Control Register) */
+#define FCR_RFRST		BIT(23)	/* Receive FIFO Data Register Reset */
+#define FCR_TFRST		BIT(15)	/* Transmit FIFO Data Register Reset */
+#define FCR_DRES		BIT(0)	/* Incoming Data Ready Error Select */
+#define FCR_RTRG4_0		GENMASK(20, 16)
+#define FCR_TTRG		GENMASK(12, 8)
+
+/* CSR (Common Status Register) */
+#define CSR_RDRF		BIT(31)	/* Receive Data Full */
+#define CSR_TEND		BIT(30)	/* Transmit End Flag */
+#define CSR_TDRE		BIT(29)	/* Transmit Data Empty */
+#define CSR_FER			BIT(28)	/* Framing Error */
+#define CSR_PER			BIT(27)	/* Parity Error */
+#define CSR_MFF			BIT(26)	/* Mode Fault Error */
+#define CSR_ORER		BIT(24)	/* Overrun Error */
+#define CSR_DFER		BIT(18)	/* Data Compare Match Framing Error */
+#define CSR_DPER		BIT(17)	/* Data Compare Match Parity Error */
+#define CSR_DCMF		BIT(16)	/* Data Compare Match */
+#define CSR_RXDMON		BIT(15)	/* Serial Input Data Monitor */
+#define CSR_ERS			BIT(4)	/* Error Signal Status */
+
+#define SCxSR_ERRORS(port)	(to_sci_port(port)->params->error_mask)
+#define SCxSR_ERROR_CLEAR(port)	(to_sci_port(port)->params->error_clear)
+
+#define RSCI_DEFAULT_ERROR_MASK	(CSR_PER | CSR_FER)
+
+#define RSCI_RDxF_CLEAR		(CFCLR_RDRFC)
+#define RSCI_ERROR_CLEAR	(CFCLR_PERC | CFCLR_FERC)
+#define RSCI_TDxE_CLEAR		(CFCLR_TDREC)
+#define RSCI_BREAK_CLEAR	(CFCLR_PERC | CFCLR_FERC | CFCLR_ORERC)
+
+/* FRSR (FIFO Receive Status Register) */
+#define FRSR_R5_0		GENMASK(13, 8)	/* Receive FIFO Data Count */
+#define FRSR_DR			BIT(0)	/* Receive Data Ready */
+
+/* CFCLR (Common Flag CLear Register) */
+#define CFCLR_RDRFC		BIT(31)	/* RDRF Clear */
+#define CFCLR_TDREC		BIT(29)	/* TDRE Clear */
+#define CFCLR_FERC		BIT(28)	/* FER Clear */
+#define CFCLR_PERC		BIT(27)	/* PER Clear */
+#define CFCLR_MFFC		BIT(26)	/* MFF Clear */
+#define CFCLR_ORERC		BIT(24)	/* ORER Clear */
+#define CFCLR_DFERC		BIT(18)	/* DFER Clear */
+#define CFCLR_DPERC		BIT(17)	/* DPER Clear */
+#define CFCLR_DCMFC		BIT(16)	/* DCMF Clear */
+#define CFCLR_ERSC		BIT(4)	/* ERS Clear */
+#define CFCLR_CLRFLAG		(CFCLR_RDRFC | CFCLR_FERC | CFCLR_PERC | \
+				 CFCLR_MFFC | CFCLR_ORERC | CFCLR_DFERC | \
+				 CFCLR_DPERC | CFCLR_DCMFC | CFCLR_ERSC)
+
+/* FFCLR (FIFO Flag CLear Register) */
+#define FFCLR_DRC		BIT(0)	/* DR Clear */
+
+#define DCR_DEPOL		BIT(0)
+
+static u32 rzsci_serial_in(struct uart_port *p, int offset)
+{
+	return readl(p->membase + offset);
+}
+
+static void rzsci_serial_out(struct uart_port *p, int offset, int value)
+{
+	writel(value, p->membase + offset);
+}
+
+static void rzsci_clear_DRxC(struct uart_port *port)
+{
+	rzsci_serial_out(port, CFCLR, CFCLR_RDRFC);
+	rzsci_serial_out(port, FFCLR, FFCLR_DRC);
+}
+
+static void rzsci_clear_SCxSR(struct uart_port *port, unsigned int mask)
+{
+	rzsci_serial_out(port, CFCLR, mask);
+}
+
+static void rzsci_start_rx(struct uart_port *port)
+{
+	unsigned int ctrl;
+
+	ctrl = rzsci_serial_in(port, CCR0);
+	ctrl |= CCR0_RIE;
+	rzsci_serial_out(port, CCR0, ctrl);
+}
+
+static void rzsci_set_termios(struct uart_port *port, struct ktermios *termios,
+			      const struct ktermios *old)
+{
+	struct sci_port *s = to_sci_port(port);
+	unsigned long flags;
+
+	sci_port_enable(s);
+	uart_port_lock_irqsave(port, &flags);
+
+	/* For now, only RX enabling is supported */
+	if (termios->c_cflag & CREAD)
+		rzsci_start_rx(port);
+
+	uart_port_unlock_irqrestore(port, flags);
+	sci_port_disable(s);
+}
+
+static int rzsci_txfill(struct uart_port *port)
+{
+	return rzsci_serial_in(port, FTSR);
+}
+
+static int rzsci_rxfill(struct uart_port *port)
+{
+	u32 val = rzsci_serial_in(port, FRSR);
+
+	return FIELD_GET(FRSR_R5_0, val);
+}
+
+static unsigned int rzsci_tx_empty(struct uart_port *port)
+{
+	unsigned int status = rzsci_serial_in(port, CSR);
+	unsigned int in_tx_fifo = rzsci_txfill(port);
+
+	return (status & CSR_TEND) && !in_tx_fifo ? TIOCSER_TEMT : 0;
+}
+
+static void rzsci_set_mctrl(struct uart_port *port, unsigned int mctrl)
+{
+	/* Not supported yet */
+}
+
+static unsigned int rzsci_get_mctrl(struct uart_port *port)
+{
+	/* Not supported yet */
+	return 0;
+}
+
+static void rzsci_clear_CFC(struct uart_port *port, unsigned int mask)
+{
+	rzsci_serial_out(port, CFCLR, mask);
+}
+
+static void rzsci_start_tx(struct uart_port *port)
+{
+	struct sci_port *sp = to_sci_port(port);
+	u32 ctrl;
+
+	if (sp->chan_tx)
+		return;
+
+	/*
+	 * TE (Transmit Enable) must be set after setting TIE
+	 * (Transmit Interrupt Enable) or in the same instruction
+	 * to start the transmit process.
+	 */
+	ctrl = rzsci_serial_in(port, CCR0);
+	ctrl |= CCR0_TIE | CCR0_TE;
+	rzsci_serial_out(port, CCR0, ctrl);
+}
+
+static void rzsci_stop_tx(struct uart_port *port)
+{
+	u32 ctrl;
+
+	ctrl = rzsci_serial_in(port, CCR0);
+	ctrl &= ~CCR0_TIE;
+	rzsci_serial_out(port, CCR0, ctrl);
+}
+
+static void rzsci_stop_rx(struct uart_port *port)
+{
+	u32 ctrl;
+
+	ctrl = rzsci_serial_in(port, CCR0);
+	ctrl &= ~CCR0_RIE;
+	rzsci_serial_out(port, CCR0, ctrl);
+}
+
+static int rzsci_txroom(struct uart_port *port)
+{
+	return port->fifosize - rzsci_txfill(port);
+}
+
+static void rzsci_transmit_chars(struct uart_port *port)
+{
+	unsigned int stopped = uart_tx_stopped(port);
+	struct tty_port *tport = &port->state->port;
+	u32 status, ctrl;
+	int count;
+
+	status = rzsci_serial_in(port, CSR);
+	if (!(status & CSR_TDRE)) {
+		ctrl = rzsci_serial_in(port, CCR0);
+		if (kfifo_is_empty(&tport->xmit_fifo))
+			ctrl &= ~CCR0_TIE;
+		else
+			ctrl |= CCR0_TIE;
+		rzsci_serial_out(port, CCR0, ctrl);
+		return;
+	}
+
+	count = rzsci_txroom(port);
+
+	do {
+		unsigned char c;
+
+		if (port->x_char) {
+			c = port->x_char;
+			port->x_char = 0;
+		} else if (stopped || !kfifo_get(&tport->xmit_fifo, &c)) {
+			break;
+		}
+
+		rzsci_clear_CFC(port, CFCLR_TDREC);
+		rzsci_serial_out(port, TDR, c);
+
+		port->icount.tx++;
+	} while (--count > 0);
+
+	if (kfifo_len(&tport->xmit_fifo) < WAKEUP_CHARS)
+		uart_write_wakeup(port);
+
+	if (kfifo_is_empty(&tport->xmit_fifo)) {
+		ctrl = rzsci_serial_in(port, CCR0);
+		ctrl &= ~CCR0_TIE;
+		ctrl |= CCR0_TEIE;
+		rzsci_serial_out(port, CCR0, ctrl);
+	}
+}
+
+static void rzsci_receive_chars(struct uart_port *port)
+{
+	struct tty_port *tport = &port->state->port;
+	u32 rdat, status, frsr_status = 0;
+	int i, count, copied = 0;
+	unsigned char flag;
+
+	status = rzsci_serial_in(port, CSR);
+	frsr_status = rzsci_serial_in(port, FRSR);
+
+	if (!(status & CSR_RDRF) && !(frsr_status & FRSR_DR))
+		return;
+
+	while (1) {
+		/* Don't copy more bytes than there is room for in the buffer */
+		count = tty_buffer_request_room(tport, rzsci_rxfill(port));
+
+		/* If for any reason we can't copy more data, we're done! */
+		if (count == 0)
+			break;
+
+		for (i = 0; i < count; i++) {
+			char c;
+
+			rdat = rzsci_serial_in(port, RDR);
+			/* 9-bits data is not supported yet */
+			c = rdat & RDR_RDAT_MSK;
+
+			if (uart_handle_sysrq_char(port, c)) {
+				count--;
+				i--;
+				continue;
+			}
+
+			/* Store data and status.
+			 * Non FIFO mode is not supported
+			 */
+			if (rdat & RDR_FFER) {
+				flag = TTY_FRAME;
+				port->icount.frame++;
+			} else if (rdat & RDR_FPER) {
+				flag = TTY_PARITY;
+				port->icount.parity++;
+			} else {
+				flag = TTY_NORMAL;
+			}
+
+			tty_insert_flip_char(tport, c, flag);
+		}
+
+		rzsci_serial_in(port, CSR); /* dummy read */
+		rzsci_clear_DRxC(port);
+
+		copied += count;
+		port->icount.rx += count;
+	}
+
+	if (copied) {
+		/* Tell the rest of the system the news. New characters! */
+		tty_flip_buffer_push(tport);
+	} else {
+		/* TTY buffers full; read from RX reg to prevent lockup */
+		rzsci_serial_in(port, RDR);
+		rzsci_serial_in(port, CSR); /* dummy read */
+		rzsci_clear_DRxC(port);
+	}
+}
+
+static void rzsci_poll_put_char(struct uart_port *port, unsigned char c)
+{
+	u32 status;
+	int ret;
+
+	ret = readl_relaxed_poll_timeout_atomic(port->membase + CSR, status,
+						(status & CSR_TDRE), 100,
+						USEC_PER_SEC);
+	if (ret != 0) {
+		dev_err(port->dev,
+			"Error while sending data in UART TX : %d\n", ret);
+		goto done;
+	}
+	rzsci_serial_out(port, TDR, c);
+done:
+	rzsci_clear_SCxSR(port, CFCLR_TDREC);
+}
+
+static void rzsci_prepare_console_write(struct uart_port *port, u32 ctrl)
+{
+	struct sci_port *s = to_sci_port(port);
+	u32 ctrl_temp =
+		s->params->param_bits->rxtx_enable |
+		CCR0_TIE |
+		s->hscif_tot;
+	rzsci_serial_out(port, CCR0, ctrl_temp);
+}
+
+static const char *rzsci_type(struct uart_port *port)
+{
+	return "rzsci";
+}
+
+static size_t rzsci_suspend_regs_size(void)
+{
+	return 0;
+}
+
+static const struct sci_common_regs rzsci_common_regs = {
+	.status = CSR,
+	.control = CCR0,
+};
+
+static const struct sci_port_params_bits rzsci_port_param_bits = {
+	.rxtx_enable = CCR0_RE | CCR0_TE,
+	.te_clear = CCR0_TE | CCR0_TEIE,
+	.poll_sent_bits = CSR_TDRE | CSR_TEND,
+};
+
+static const struct sci_port_params rzsci_port_params = {
+	.fifosize = 16,
+	.overrun_reg = CSR,
+	.overrun_mask = CSR_ORER,
+	.sampling_rate_mask = SCI_SR(32),
+	.error_mask = RSCI_DEFAULT_ERROR_MASK,
+	.error_clear = RSCI_ERROR_CLEAR,
+	.param_bits = &rzsci_port_param_bits,
+	.common_regs = &rzsci_common_regs,
+};
+
+static const struct uart_ops rzt2_sci_uart_ops = {
+	.tx_empty	= rzsci_tx_empty,
+	.set_mctrl	= rzsci_set_mctrl,
+	.get_mctrl	= rzsci_get_mctrl,
+	.start_tx	= rzsci_start_tx,
+	.stop_tx	= rzsci_stop_tx,
+	.stop_rx	= rzsci_stop_rx,
+	.startup	= sci_startup,
+	.set_termios	= rzsci_set_termios,
+	.pm		= sci_pm,
+	.type		= rzsci_type,
+	.release_port	= sci_release_port,
+	.request_port	= sci_request_port,
+	.config_port	= sci_config_port,
+	.verify_port	= sci_verify_port,
+};
+
+static const struct sci_port_ops rzsci_port_ops = {
+	.read_reg		= rzsci_serial_in,
+	.write_reg		= rzsci_serial_out,
+	.clear_SCxSR		= rzsci_clear_SCxSR,
+	.transmit_chars		= rzsci_transmit_chars,
+	.receive_chars		= rzsci_receive_chars,
+	.poll_put_char		= rzsci_poll_put_char,
+	.prepare_console_write	= rzsci_prepare_console_write,
+	.suspend_regs_size	= rzsci_suspend_regs_size,
+};
+
+struct sci_of_data sci_r9a09g077_data = {
+	.type = PORT_RZSCI,
+	.regtype = SCIx_RZT2H_SCI_REGTYPE,
+	.ops = &rzsci_port_ops,
+	.uart_ops = &rzt2_sci_uart_ops,
+	.params = &rzsci_port_params,
+};
+
+#ifdef CONFIG_SERIAL_SH_SCI_EARLYCON
+
+static int __init rzt2hsci_early_console_setup(struct earlycon_device *device,
+					       const char *opt)
+{
+	return scix_early_console_setup(device, &sci_r9a09g077_data);
+}
+
+OF_EARLYCON_DECLARE(rzsci, "renesas,r9a09g077-sci", rzt2hsci_early_console_setup);
+
+#endif /* CONFIG_SERIAL_SH_SCI_EARLYCON */
diff --git a/drivers/tty/serial/rzsci.h b/drivers/tty/serial/rzsci.h
new file mode 100644
index 000000000000..e0edc8fb6fd2
--- /dev/null
+++ b/drivers/tty/serial/rzsci.h
@@ -0,0 +1,12 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+#ifndef __RZT2_SCI_H__
+#define __RZT2_SCI_H__
+
+#include "sh-sci-common.h"
+
+#ifdef CONFIG_SERIAL_RZ_SCI
+extern struct sci_of_data sci_r9a09g077_data;
+#endif
+
+#endif /* __RZT2_SCI_H__ */
diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
index a8e7f739c4ec..b225d1771dc8 100644
--- a/drivers/tty/serial/sh-sci.c
+++ b/drivers/tty/serial/sh-sci.c
@@ -54,6 +54,7 @@
 #include <asm/platform_early.h>
 #endif
 
+#include "rzsci.h"
 #include "serial_mctrl_gpio.h"
 #include "sh-sci.h"
 #include "sh-sci-common.h"
@@ -1820,7 +1821,7 @@ static irqreturn_t sci_tx_end_interrupt(int irq, void *ptr)
 	unsigned long flags;
 	u32 ctrl;
 
-	if (port->type != PORT_SCI)
+	if (port->type != PORT_SCI && port->type != PORT_RZSCI)
 		return sci_tx_interrupt(irq, ptr);
 
 	uart_port_lock_irqsave(port, &flags);
@@ -3069,10 +3070,10 @@ static int sci_init_single(struct platform_device *dev,
 	}
 
 	/*
-	 * The fourth interrupt on SCI port is transmit end interrupt, so
+	 * The fourth interrupt on SCI and RZSCI port is transmit end interrupt, so
 	 * shuffle the interrupts.
 	 */
-	if (p->type == PORT_SCI)
+	if (p->type == PORT_SCI || p->type == PORT_RZSCI)
 		swap(sci_port->irqs[SCIx_BRI_IRQ], sci_port->irqs[SCIx_TEI_IRQ]);
 
 	/* The SCI generates several interrupts. They can be muxed together or
@@ -3106,6 +3107,9 @@ static int sci_init_single(struct platform_device *dev,
 		else
 			sci_port->rx_trigger = 8;
 		break;
+	case PORT_RZSCI:
+		sci_port->rx_trigger = 15;
+		break;
 	default:
 		sci_port->rx_trigger = 1;
 		break;
@@ -3330,7 +3334,7 @@ static void sci_remove(struct platform_device *dev)
 
 	if (port->port.fifosize > 1)
 		device_remove_file(&dev->dev, &dev_attr_rx_fifo_trigger);
-	if (type == PORT_SCIFA || type == PORT_SCIFB || type == PORT_HSCIF)
+	if (type == PORT_SCIFA || type == PORT_SCIFB || type == PORT_HSCIF || type == PORT_RZSCI)
 		device_remove_file(&dev->dev, &dev_attr_rx_fifo_timeout);
 }
 
@@ -3424,6 +3428,12 @@ static const struct of_device_id of_sci_match[] __maybe_unused = {
 		.compatible = "renesas,scif-r9a09g057",
 		.data = &of_sci_scif_rzv2h,
 	},
+#ifdef CONFIG_SERIAL_RZ_SCI
+	{
+		.compatible = "renesas,r9a09g077-sci",
+		.data = &sci_r9a09g077_data,
+	},
+#endif	/* CONFIG_SERIAL_RZ_SCI */
 	/* Family-specific types */
 	{
 		.compatible = "renesas,rcar-gen1-scif",
@@ -3683,7 +3693,7 @@ static int sci_probe(struct platform_device *dev)
 			return ret;
 	}
 	if (sp->port.type == PORT_SCIFA || sp->port.type == PORT_SCIFB ||
-	    sp->port.type == PORT_HSCIF) {
+	    sp->port.type == PORT_HSCIF || sp->port.type == PORT_RZSCI) {
 		ret = device_create_file(&dev->dev, &dev_attr_rx_fifo_timeout);
 		if (ret) {
 			if (sp->port.fifosize > 1) {
diff --git a/include/linux/serial_sci.h b/include/linux/serial_sci.h
index 0f2f50b8d28e..787fd9a96711 100644
--- a/include/linux/serial_sci.h
+++ b/include/linux/serial_sci.h
@@ -38,6 +38,7 @@ enum {
 	SCIx_HSCIF_REGTYPE,
 	SCIx_RZ_SCIFA_REGTYPE,
 	SCIx_RZV2H_SCIF_REGTYPE,
+	SCIx_RZT2H_SCI_REGTYPE,
 
 	SCIx_NR_REGTYPES,
 };
@@ -50,7 +51,7 @@ struct plat_sci_port_ops {
  * Platform device specific platform_data struct
  */
 struct plat_sci_port {
-	unsigned int	type;			/* SCI / SCIF / IRDA / HSCIF */
+	unsigned int	type;			/* SCI / SCIF / IRDA / HSCIF / RZSCI */
 	upf_t		flags;			/* UPF_* flags */
 
 	unsigned int	sampling_rate;
diff --git a/include/uapi/linux/serial_core.h b/include/uapi/linux/serial_core.h
index 9c007a106330..30fbbef599f4 100644
--- a/include/uapi/linux/serial_core.h
+++ b/include/uapi/linux/serial_core.h
@@ -231,6 +231,9 @@
 /* Sunplus UART */
 #define PORT_SUNPLUS	123
 
+/* SH-SCI */
+#define PORT_RZSCI	124
+
 /* Generic type identifier for ports which type is not important to userspace. */
 #define PORT_GENERIC	(-1)
 
-- 
2.43.0

Re: [PATCH v4 03/13] dt-bindings: serial: Add compatible for Renesas RZ/T2H SoC in sci

[Rob Herring (Arm)]

On Thu, 06 Mar 2025 16:24:37 +0100, Thierry Bultel wrote:
> The SCI of RZ/T2H SoC (a.k.a r9a09g077), as a lot
> of similarities with other Renesas SoC like G2L, G3S, V2L;
> However, it has a different set of registers, and in addition to serial,
> this IP also supports SCIe (encoder), SmartCard, i2c and spi.
> This is why the 'renesas,sci' fallback for generic SCI does not apply for it.
> 
> Signed-off-by: Thierry Bultel <thierry.bultel.yh@bp.renesas.com>
> ---
> Changes v3->v4:
>   - Added more details in commit description about why renesas,sci
>     does not apply.
>   - Removed uart-has-rtscts for !rzsci.
> ----
>  .../bindings/serial/renesas,sci.yaml          | 63 ++++++++++++-------
>  1 file changed, 39 insertions(+), 24 deletions(-)
> 

Reviewed-by: Rob Herring (Arm) <robh@kernel.org>

Re: [PATCH v4 08/13] serial: sh-sci: Introduced function pointers

[Geert Uytterhoeven]

Hi Thierry,

On Thu, 6 Mar 2025 at 16:26, Thierry Bultel
<thierry.bultel.yh@bp.renesas.com> wrote:
> The aim here is to prepare support for new sci controllers like
> the T2H/RSCI whose registers are too much different for being
> handled in common code.
>
> This named serial controller also has 32 bits register,
> so some return types had to be changed.
>
> The needed generic functions are no longer static, with prototypes
> defined in sh-sci-common.h so that they can be used from specific
> implementation in a separate file, to keep this driver as little
> changed as possible.
>
> For doing so, a set of 'ops' is added to struct sci_port.
>
> Signed-off-by: Thierry Bultel <thierry.bultel.yh@bp.renesas.com>
> ---
> Changes v3->v4:
>    - Add missing #include <bitfield.h>
>    - Remove sci_poll_get_char sci_poll_put_char from sh-sci-common.h (both
>      function are not used by rzsci yet).
>    - Add missing #ifdef around .poll_put_char pointer initialization.
>    - More registers to save & restore due to rebase on tty-next

Thanks for the update!

While most rough edges have been polished by now (thanks!), and the
driver seems to still work on a variety of platforms, I am still
worried about the impact of this change:
  - Maintainability and future bug fixing?
  - Performance of the additional indirections on slow platforms (SH)?

What do other people think?
Thanks!

Full series:
https://lore.kernel.org/all/20250306152451.2356762-1-thierry.bultel.yh@bp.renesas.com
My initial comments and RFC:
https://lore.kernel.org/all/CAMuHMdVD1dLP53V_zOhxpqazDdPDVafJ6iohY8u6WPQrmYH5Sw@mail.gmail.com

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 v4 08/13] serial: sh-sci: Introduced function pointers

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 1428 bytes --]

Hi all,

sorry for missing this series so far and thanks to Geert for pulling me
into the loop.

> While most rough edges have been polished by now (thanks!), and the
> driver seems to still work on a variety of platforms, I am still
> worried about the impact of this change:
>   - Maintainability and future bug fixing?

I hate to see development work going to waste, yet I have to say I am
also concerned about the maintainability of this driver after this very
intrusive changeset. The driver is already quite complex. Adding another
layer of complexity (function pointers) will make proper bugfixing for
all supported instances quite harder, I'd think.

Has it been discussed to have this as a separate driver? Were there
reasons against it? This is really an open question. Maybe it is
justified to do it like this if we have reasons for it.

Seeing that SCI core needs 800+ lines changed and we still have a
seperate driver with 460 lines driver, I do wonder if copying the logic
from SCI core to a seperate driver would make sense. I am aware that the
core has currently 3500+ lines currently. I'd estimate it would shrink
quite a bit when copying because you won't need to handle all the
differences to other SCI entries.

Again, this is not a request to follow my suggestion, it is an open
question to make sure all paths have been considered.

Thanks and happy hacking,

   Wolfram


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v4 08/13] serial: sh-sci: Introduced function pointers

[Paul Barker]

[-- Attachment #1.1.1: Type: text/plain, Size: 2490 bytes --]

On 17/03/2025 09:55, Wolfram Sang wrote:
> Hi all,
> 
> sorry for missing this series so far and thanks to Geert for pulling me
> into the loop.
> 
>> While most rough edges have been polished by now (thanks!), and the
>> driver seems to still work on a variety of platforms, I am still
>> worried about the impact of this change:
>>   - Maintainability and future bug fixing?
> 
> I hate to see development work going to waste, yet I have to say I am
> also concerned about the maintainability of this driver after this very
> intrusive changeset. The driver is already quite complex. Adding another
> layer of complexity (function pointers) will make proper bugfixing for
> all supported instances quite harder, I'd think.
> 
> Has it been discussed to have this as a separate driver? Were there
> reasons against it? This is really an open question. Maybe it is
> justified to do it like this if we have reasons for it.
> 
> Seeing that SCI core needs 800+ lines changed and we still have a
> seperate driver with 460 lines driver, I do wonder if copying the logic
> from SCI core to a seperate driver would make sense. I am aware that the
> core has currently 3500+ lines currently. I'd estimate it would shrink
> quite a bit when copying because you won't need to handle all the
> differences to other SCI entries.
> 
> Again, this is not a request to follow my suggestion, it is an open
> question to make sure all paths have been considered.

Hi Geert, Wolfram,

Thierry is out of the office this week so we can follow this up next
week, but I do want to give some input in the meantime.

We discussed both approaches internally and did an initial
proof-of-concept of a separate driver. The result was over 1,000 lines
of code copy-pasted from the existing sh-sci driver into the new driver,
which is generally something maintainers want us to avoid doing. The
trade off here is whether we want a single more complex driver, or two
copies of much of the code so that bugfixes/improvements to the common
sections in the future need to be duplicated.

The RZ/V2H and RZ/G3E have interfaces of both the existing sh-sci
register layout ("SCIF" ports in RZ/V2H & RZ/G3E manual) and the RZ/T2H
style register layout ("RSCI" ports in RZ/V2H manual, "SCI" ports in
RZ/G3E manual), so keeping things closely aligned as we move forward
will be beneficial. I expect that this will be easier with a combined
driver.

Thanks,

-- 
Paul Barker

[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 3577 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 236 bytes --]

Re: [PATCH v4 08/13] serial: sh-sci: Introduced function pointers

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 961 bytes --]

Hi Paul,

> We discussed both approaches internally and did an initial
> proof-of-concept of a separate driver. The result was over 1,000 lines
> of code copy-pasted from the existing sh-sci driver into the new driver,
> which is generally something maintainers want us to avoid doing.

Darn, 1000 lines of logic is a lot...

> trade off here is whether we want a single more complex driver, or two
> copies of much of the code so that bugfixes/improvements to the common
> sections in the future need to be duplicated.

Exactly.

> The RZ/V2H and RZ/G3E have interfaces of both the existing sh-sci
> register layout ("SCIF" ports in RZ/V2H & RZ/G3E manual) and the RZ/T2H
> style register layout ("RSCI" ports in RZ/V2H manual, "SCI" ports in
> RZ/G3E manual), so keeping things closely aligned as we move forward
> will be beneficial. I expect that this will be easier with a combined
> driver.

I will have a look at the series.

Happy hacking,

   Wolfram


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v4 07/13] serial: sh-sci: Fix a comment about SCIFA

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 588 bytes --]

On Thu, Mar 06, 2025 at 04:24:41PM +0100, Thierry Bultel wrote:
> The comment was correct when it was added, at that time RZ/T1 was
> the only SoC in the RZ/T line. Since then, further SoCs have been
> added with RZ/T names which do not use the same SCIFA register
> layout and so the comment is now misleading.
> 
> So we update the comment to explicitly reference only RZ/T1 SoCs.
> 
> Signed-off-by: Thierry Bultel <thierry.bultel.yh@bp.renesas.com>
> Reviewed-by: Paul Barker <paul.barker.ct@bp.renesas.com>

Reviewed-by: Wolfram Sang <wsa+renesas@sang-engineering.com>


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v4 08/13] serial: sh-sci: Introduced function pointers

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 2980 bytes --]

On Thu, Mar 06, 2025 at 04:24:42PM +0100, Thierry Bultel wrote:
> The aim here is to prepare support for new sci controllers like
> the T2H/RSCI whose registers are too much different for being
> handled in common code.
> 
> This named serial controller also has 32 bits register,
> so some return types had to be changed.
> 
> The needed generic functions are no longer static, with prototypes
> defined in sh-sci-common.h so that they can be used from specific
> implementation in a separate file, to keep this driver as little
> changed as possible.
> 
> For doing so, a set of 'ops' is added to struct sci_port.
> 
> Signed-off-by: Thierry Bultel <thierry.bultel.yh@bp.renesas.com>

Okay, the discussion about the general approach convinced me that we can
go this road. I will not do a line-by-line review of these patches, but
just check that it looks good to me in general. This patch here merely
shuffles code around and adds some inderection. If it works, it seems
good enough for me and we can improve on it incrementally:

Reviewed-by: Wolfram Sang <wsa+renesas@sang-engineering.com>

That means, though, that testing this series on a variety of SoCs is
especially important and I'd like to get confirmed that you did these
tests on SCI variations which are available on RZ hardware. According to
my research it would be those:

	[SCIx_SCI_REGTYPE]
		/* RZ/Five, RZ/G2UL, RZ/V2L */
		.compatible = "renesas,sci",

	[SCIx_RZ_SCIFA_REGTYPE]
		 /* The "SCIFA" that is in RZ/A2, RZ/G2L and RZ/T1 */
		.compatible = "renesas,scif-r7s9210",
		.compatible = "renesas,scif-r9a07g044",

	[SCIx_RZV2H_SCIF_REGTYPE]
		 /* RZ/V2H */
		.compatible = "renesas,scif-r9a09g057",

	[SCIx_SH4_SCIF_BRG_REGTYPE]
		/* a lot of RZ, too */
		.compatible = "renesas,rcar-gen1-scif",
		.compatible = "renesas,rcar-gen2-scif",
		.compatible = "renesas,rcar-gen3-scif",
		.compatible = "renesas,rcar-gen4-scif",

	[SCIx_HSCIF_REGTYPE]
		/* R-Car Gen2-5 */
		/* a lot of RZ */
		.compatible = "renesas,hscif",

Please double check that I did not make a mistake. I'd think Geert tests
these on in his board farm anyway:

	[SCIx_SH4_SCIF_REGTYPE]
		/* landisk */
		.compatible = "renesas,scif",

	[SCIx_SCIFA_REGTYPE]
		/* R-Car Gen2 */
		.compatible = "renesas,scifa",

	[SCIx_SCIFB_REGTYPE]
		/* R-Car Gen2 */
		.compatible = "renesas,scifb",

	[SCIx_SH2_SCIF_FIFODATA_REGTYPE]
		/* RZ/A1 */
		.compatible = "renesas,scif-r7s72100",

We maybe can get hold of the next board. I will figure this out
internally (not super important for this series, but nice to have):

	[SCIx_SH4_SCIF_NO_SCSPTR_REGTYPE]
	/* SH Ecovec */
	arch/sh/kernel/cpu/sh4a/setup-sh7723.c: .regtype        = SCIx_SH4_SCIF_NO_SCSPTR_REGTYPE,

That leaves some older SH boards out of the loop, but I think this is
OK. A quick research didn't let me obtain boards for these anymore.

So far, so good? Comments?

Happy hacking,

   Wolfram


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v4 09/13] serial: sh-sci: Introduced sci_of_data

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 1186 bytes --]

On Thu, Mar 06, 2025 at 04:24:43PM +0100, Thierry Bultel wrote:
> The aim here is to provide an easier support to more different SCI
> controllers, like the RZ/T2H one.
> 
> The existing .data field of_sci_match is changed to a structure containing
> all what that can be statically initialized, and avoid a call to
> 'sci_probe_regmap', in both 'sci_init_single', and 'early_console_setup'.
> 
> 'sci_probe_regmap' is now assumed to be called in the only case where the
> device description is from a board file instead of a dts.
> 
> In this way, there is no need to patch 'sci_probe_regmap' for adding new
> SCI type, and also, the specific sci_port_params for a new SCI type can be
> provided by an external file.
> 
> Signed-off-by: Thierry Bultel <thierry.bultel.yh@bp.renesas.com>

Same comments about not-per-line-reviewed from me and much testing
needed as for patch 8. So:

Reviewed-by: Wolfram Sang <wsa+renesas@sang-engineering.com>

>  	}, {
>  		.compatible = "renesas,rcar-gen4-scif",
> -		.data = SCI_OF_DATA(PORT_SCIF, SCIx_SH4_SCIF_BRG_REGTYPE),
> +		.data = &of_sci_rcar_scif
>  	},

This change to a proper struct is really nice IMHO!


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v4 10/13] serial: sh-sci: Add support for RZ/T2H SCI

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 612 bytes --]


> +config SERIAL_RZ_SCI

I think this name is too generic. Most RZ-variants so far do not have
this SoC. Would 'RZT2H' work or is it too narrow then?

> +	SCIx_RZT2H_SCI_REGTYPE,

This name is better.

>  struct plat_sci_port {
> -	unsigned int	type;			/* SCI / SCIF / IRDA / HSCIF */
> +	unsigned int	type;			/* SCI / SCIF / IRDA / HSCIF / RZSCI */

"RZT2" in the comment as well.

> +/* SH-SCI */
> +#define PORT_RZSCI	124
> +
>  /* Generic type identifier for ports which type is not important to userspace. */
>  #define PORT_GENERIC	(-1)

Does userspace need to know this port? Can't we use PORT_GENERIC?


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v4 08/13] serial: sh-sci: Introduced function pointers

[Geert Uytterhoeven]

On Mon, 24 Mar 2025 at 10:25, Wolfram Sang
<wsa+renesas@sang-engineering.com> wrote:
> On Thu, Mar 06, 2025 at 04:24:42PM +0100, Thierry Bultel wrote:
> > The aim here is to prepare support for new sci controllers like
> > the T2H/RSCI whose registers are too much different for being
> > handled in common code.
> >
> > This named serial controller also has 32 bits register,
> > so some return types had to be changed.
> >
> > The needed generic functions are no longer static, with prototypes
> > defined in sh-sci-common.h so that they can be used from specific
> > implementation in a separate file, to keep this driver as little
> > changed as possible.
> >
> > For doing so, a set of 'ops' is added to struct sci_port.
> >
> > Signed-off-by: Thierry Bultel <thierry.bultel.yh@bp.renesas.com>
>
> Okay, the discussion about the general approach convinced me that we can
> go this road. I will not do a line-by-line review of these patches, but
> just check that it looks good to me in general. This patch here merely
> shuffles code around and adds some inderection. If it works, it seems
> good enough for me and we can improve on it incrementally:
>
> Reviewed-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
>
> That means, though, that testing this series on a variety of SoCs is
> especially important and I'd like to get confirmed that you did these
> tests on SCI variations which are available on RZ hardware. According to
> my research it would be those:
>
>         [SCIx_SCI_REGTYPE]
>                 /* RZ/Five, RZ/G2UL, RZ/V2L */
>                 .compatible = "renesas,sci",

Tested as console on RZ/Five...

>         [SCIx_RZ_SCIFA_REGTYPE]
>                  /* The "SCIFA" that is in RZ/A2, RZ/G2L and RZ/T1 */
>                 .compatible = "renesas,scif-r7s9210",
>                 .compatible = "renesas,scif-r9a07g044",

...RZA2MEVB...

>         [SCIx_SH4_SCIF_BRG_REGTYPE]
>                 /* a lot of RZ, too */
>                 .compatible = "renesas,rcar-gen1-scif",
>                 .compatible = "renesas,rcar-gen2-scif",
>                 .compatible = "renesas,rcar-gen3-scif",
>                 .compatible = "renesas,rcar-gen4-scif",

...R-Car Gen1/2/3...

>
>         [SCIx_HSCIF_REGTYPE]
>                 /* R-Car Gen2-5 */

... R-Car Gen3/4.

>                 /* a lot of RZ */
>                 .compatible = "renesas,hscif",
>
> Please double check that I did not make a mistake. I'd think Geert tests
> these on in his board farm anyway:
>
>         [SCIx_SH4_SCIF_REGTYPE]
>                 /* landisk */
>                 .compatible = "renesas,scif",

Tested as console on Landisk and QEMU RTS7751R2D (both without DT,
though)...

>
>         [SCIx_SCIFA_REGTYPE]
>                 /* R-Car Gen2 */
>                 .compatible = "renesas,scifa",

... SH/R-Mobile...

>         [SCIx_SCIFB_REGTYPE]
>                 /* R-Car Gen2 */
>                 .compatible = "renesas,scifb",

Not tested. SCIFB is very similar to SCIFA, so I am confident it is OK
(all tests for PORT_SCIFA and PORT_SCIFB come in pairs).

>         [SCIx_SH2_SCIF_FIFODATA_REGTYPE]
>                 /* RZ/A1 */
>                 .compatible = "renesas,scif-r7s72100",

Tested as console on RSK+RZA1.

> We maybe can get hold of the next board. I will figure this out
> internally (not super important for this series, but nice to have):
>
>         [SCIx_SH4_SCIF_NO_SCSPTR_REGTYPE]
>         /* SH Ecovec */
>         arch/sh/kernel/cpu/sh4a/setup-sh7723.c: .regtype        = SCIx_SH4_SCIF_NO_SCSPTR_REGTYPE,
>
> That leaves some older SH boards out of the loop, but I think this is
> OK. A quick research didn't let me obtain boards for these anymore.

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

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

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 v4 09/13] serial: sh-sci: Introduced sci_of_data

[Geert Uytterhoeven]

On Thu, 6 Mar 2025 at 16:26, Thierry Bultel
<thierry.bultel.yh@bp.renesas.com> wrote:
> The aim here is to provide an easier support to more different SCI
> controllers, like the RZ/T2H one.
>
> The existing .data field of_sci_match is changed to a structure containing
> all what that can be statically initialized, and avoid a call to
> 'sci_probe_regmap', in both 'sci_init_single', and 'early_console_setup'.
>
> 'sci_probe_regmap' is now assumed to be called in the only case where the
> device description is from a board file instead of a dts.
>
> In this way, there is no need to patch 'sci_probe_regmap' for adding new
> SCI type, and also, the specific sci_port_params for a new SCI type can be
> provided by an external file.
>
> Signed-off-by: Thierry Bultel <thierry.bultel.yh@bp.renesas.com>
> ---
> Changes v3->v4:
>    - Fix the bot compilation error on superh in sci_probe_earlyprink()

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

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 v4 10/13] serial: sh-sci: Add support for RZ/T2H SCI

[Thierry Bultel]

Hi Wolfram,

> -----Original Message-----
> From: Wolfram Sang <wsa+renesas@sang-engineering.com>
> Sent: lundi 24 mars 2025 10:43
> To: Thierry Bultel <thierry.bultel.yh@bp.renesas.com>
> Cc: thierry.bultel@linatsea.fr; linux-renesas-soc@vger.kernel.org;
> geert@linux-m68k.org; Paul Barker <paul.barker.ct@bp.renesas.com>; linux-
> kernel@vger.kernel.org; linux-serial@vger.kernel.org
> Subject: Re: [PATCH v4 10/13] serial: sh-sci: Add support for RZ/T2H SCI
> 
> 
> > +config SERIAL_RZ_SCI
> 
> I think this name is too generic. Most RZ-variants so far do not have this
> SoC. Would 'RZT2H' work or is it too narrow then?

This is too narrow, because for instance the RZ/N2H , which is very similar, has the same SCI

> 
> > +	SCIx_RZT2H_SCI_REGTYPE,
> 
> This name is better.
> 
> >  struct plat_sci_port {
> > -	unsigned int	type;			/* SCI / SCIF / IRDA / HSCIF */
> > +	unsigned int	type;			/* SCI / SCIF / IRDA / HSCIF /
> RZSCI */
> 
> "RZT2" in the comment as well.
> 
> > +/* SH-SCI */
> > +#define PORT_RZSCI	124
> > +
> >  /* Generic type identifier for ports which type is not important to
> userspace. */
> >  #define PORT_GENERIC	(-1)
> 
> Does userspace need to know this port? Can't we use PORT_GENERIC?

Userspace unlikely needs it. But unfortunately, the port type needs to be checked at a number (5) of places in the driver, so the definition is needed.

Thierry

Re: [PATCH v4 10/13] serial: sh-sci: Add support for RZ/T2H SCI

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 680 bytes --]


> > > +config SERIAL_RZ_SCI
> > 
> > I think this name is too generic. Most RZ-variants so far do not have this
> > SoC. Would 'RZT2H' work or is it too narrow then?
> 
> This is too narrow, because for instance the RZ/N2H , which is very similar, has the same SCI

You know the differences better, what could be a suitable name?

> > >  #define PORT_GENERIC	(-1)
> > 
> > Does userspace need to know this port? Can't we use PORT_GENERIC?
> 
> Userspace unlikely needs it. But unfortunately, the port type needs to
> be checked at a number (5) of places in the driver, so the definition
> is needed.

Yes, true. You just keep consistent with the driver. OK.


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

RE: [PATCH v4 10/13] serial: sh-sci: Add support for RZ/T2H SCI

[Biju Das]

Hi Thierry and Wolfram,

> -----Original Message-----
> From: Wolfram Sang <wsa+renesas@sang-engineering.com>
> Sent: 24 March 2025 21:57
> Subject: Re: [PATCH v4 10/13] serial: sh-sci: Add support for RZ/T2H SCI
> 
> 
> > > > +config SERIAL_RZ_SCI
> > >
> > > I think this name is too generic. Most RZ-variants so far do not
> > > have this SoC. Would 'RZT2H' work or is it too narrow then?
> >
> > This is too narrow, because for instance the RZ/N2H , which is very
> > similar, has the same SCI
> 
> You know the differences better, what could be a suitable name?

Please consider RZ/G3E and RZ/V2H SCI as well as it is almost similar IP.

Cheers,
Biju

Re: [PATCH v4 10/13] serial: sh-sci: Add support for RZ/T2H SCI

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 672 bytes --]


> > > > > +config SERIAL_RZ_SCI
> > > >
> > > > I think this name is too generic. Most RZ-variants so far do not
> > > > have this SoC. Would 'RZT2H' work or is it too narrow then?
> > >
> > > This is too narrow, because for instance the RZ/N2H , which is very
> > > similar, has the same SCI
> > 
> > You know the differences better, what could be a suitable name?
> 
> Please consider RZ/G3E and RZ/V2H SCI as well as it is almost similar IP.

So, I am thinking to not use a name based on SoC but based on feature
like SERIAL_SCI_32BIT or something. But I don't know the HW details
enough to make the best possible name or maybe this is a bogus idea.


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

RE: [PATCH v4 10/13] serial: sh-sci: Add support for RZ/T2H SCI

[Thierry Bultel]

Hi Wolfram,

> -----Original Message-----
> From: Wolfram Sang <wsa+renesas@sang-engineering.com>
> Sent: mardi 25 mars 2025 08:51
> To: Biju Das <biju.das.jz@bp.renesas.com>
> Cc: Thierry Bultel <thierry.bultel.yh@bp.renesas.com>;
> thierry.bultel@linatsea.fr; linux-renesas-soc@vger.kernel.org;
> geert@linux-m68k.org; Paul Barker <paul.barker.ct@bp.renesas.com>; linux-
> kernel@vger.kernel.org; linux-serial@vger.kernel.org
> Subject: Re: [PATCH v4 10/13] serial: sh-sci: Add support for RZ/T2H SCI
> 
> 
> > > > > > +config SERIAL_RZ_SCI
> > > > >
> > > > > I think this name is too generic. Most RZ-variants so far do not
> > > > > have this SoC. Would 'RZT2H' work or is it too narrow then?
> > > >
> > > > This is too narrow, because for instance the RZ/N2H , which is
> > > > very similar, has the same SCI
> > >
> > > You know the differences better, what could be a suitable name?
> >
> > Please consider RZ/G3E and RZ/V2H SCI as well as it is almost similar
> IP.
> 
> So, I am thinking to not use a name based on SoC but based on feature like
> SERIAL_SCI_32BIT or something. But I don't know the HW details enough to
> make the best possible name or maybe this is a bogus idea.

This seems a little bit confusing, and like said in former discussions,
the 32 bits registers are not the main difference.

Here are the known SoCs that have this IP, up to now:

RZ/T2H
RZ/N2H
RZ/G3E
RZ/V2H

So that seems reasonable to keep RZ in the name, even there are other RZ SoCs that
do not have it.

The HW documentation does not mention a better name, or revision,
so, the suggestion is to arbitrarily consider it as a new 'T2' type.

Would SERIAL_RZ_SCI_T2 (and rz-sci-t2 for the driver) be specific enough ?

Thanks !
Thierry

Re: [PATCH v4 10/13] serial: sh-sci: Add support for RZ/T2H SCI

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 491 bytes --]


> Would SERIAL_RZ_SCI_T2 (and rz-sci-t2 for the driver) be specific enough ?

For me, yes. That means at least "as first supported in RZ/T2". Only
RZ_SCI seems more arbitrary to me. It might show up in non-RZ platforms
later, and then RZ_SCI basically has no information gain.

That being said, I see that there doesn't seem to be an easily available
better name, and we are entering bike-shedding area. I do prefer
SERIAL_RZ_SCI_T2 but if you really want to keep this version, so be it.



[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v4 10/13] serial: sh-sci: Add support for RZ/T2H SCI

[Geert Uytterhoeven]

Hi Thierry,

On Tue, 25 Mar 2025 at 11:49, Thierry Bultel
<thierry.bultel.yh@bp.renesas.com> wrote:
> > From: Wolfram Sang <wsa+renesas@sang-engineering.com>
> > > > > > > +config SERIAL_RZ_SCI
> > > > > >
> > > > > > I think this name is too generic. Most RZ-variants so far do not
> > > > > > have this SoC. Would 'RZT2H' work or is it too narrow then?
> > > > >
> > > > > This is too narrow, because for instance the RZ/N2H , which is
> > > > > very similar, has the same SCI
> > > >
> > > > You know the differences better, what could be a suitable name?
> > >
> > > Please consider RZ/G3E and RZ/V2H SCI as well as it is almost similar
> > IP.
> >
> > So, I am thinking to not use a name based on SoC but based on feature like
> > SERIAL_SCI_32BIT or something. But I don't know the HW details enough to
> > make the best possible name or maybe this is a bogus idea.
>
> This seems a little bit confusing, and like said in former discussions,
> the 32 bits registers are not the main difference.
>
> Here are the known SoCs that have this IP, up to now:
>
> RZ/T2H
> RZ/N2H
> RZ/G3E
> RZ/V2H

+ RZ/V2N

> So that seems reasonable to keep RZ in the name, even there are other RZ SoCs that
> do not have it.
>
> The HW documentation does not mention a better name, or revision,

While the RZ/T2H and RZ/N2H documentation just call it "SCI" ("SCIE"
for a reduced-functionality variant), the RZ/G3E, RZV2H, and RZ/V2N
documentation calls it "RSCI". More below...

> so, the suggestion is to arbitrarily consider it as a new 'T2' type.
>
> Would SERIAL_RZ_SCI_T2 (and rz-sci-t2 for the driver) be specific enough ?

Please don't put the "SCI" in the middle of the part name
=> SERIAL_RZT2_SCI.

"RSCI" does not seem to be present on any Linux-capable Renesas SH,
ARM or RISC-V SoC I have documentation for.  However it seems to
originate from the RX series of microcontrollers:
  - RX Family Application Note: Comparison of the Differences Between
    the RSCI Module and the SCI Module[1],
  - The RX26T documentation[2] shows RSCI on RX26T is very similar
    to RSCI on the five RZ SoCs listed above, but not identical.
    The RZ variant seems to be a reduced version with 16 instead of 32
    FIFO entries, and less "special" (non-UART) modes.

So I'm in favor of calling it "RSCI" (CONFIG_SERIAL_RSCI).

[1] https://www.renesas.com/en/document/apn/comparison-differences-between-rsci-module-and-sci-module-rev100
[2] https://www.renesas.com/en/products/microcontrollers-microprocessors/rx-32-bit-performance-efficiency-mcus/rx26t-32-bit-microcontroller-optimized-dual-motor-and-pfc-control

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