[PATCH/RFC 0/4] sh-sci : Do not derive regshift from regsize

[PATCH/RFC 0/4] sh-sci : Do not derive regshift from regsize

[Geert Uytterhoeven]

	Hi all,

This RFC patch series was sparked by noticing that commit 2d4dd0da45401c7a
("serial: sh-sci: Allow for compressed SCIF address") broke earlycon
support on most Renesas ARM SoCs using SCIF ports, and by the fragility of
deriving regshift from the register block size (which may be rounded up):
  1. The first patch is an old patch from Sato-san, which I never really
     understood.  But it turned out to be a dependency for patch 2.
  2. Patch 2 makes sure regshift is initialized when using earlycon,
     unbreaking the serial console on e.g. R-Car Gen2 and Gen3.
  3. Patch 3 reverts the patch that started deriving regshift from the
     register block size, and that removed the plat_sci_port.regshift
     field.  Which is a field I needed again in patch 4.
  4. Patch 4 removes the remaining regshift derivations on DT platforms.
 (5. I didn't bother writing patch 5, which involves adding .regshift
     initializations to all SH board files that need it.)

However, I'm not happy with the end result, so please DO NOT apply this!
As I spent almost a full day on this, and would still like to know the
story about "sh-sci: Use a separate sci_port for earlycon", I decided to
post it anyway.

As earlycon will be broken in v4.19-rc1 on RZ/A1, RZ/G, and R-Car, assuming
no other actions are taken, an alternative solution would be to:
  1. Revert commit 7acece71a517cad8 ("serial: sh-sci: Remove
     SCIx_RZ_SCIFA_REGTYPE"),
  2. Revert commit 2d4dd0da45401c7a ("serial: sh-sci: Allow for compressed
     SCIF address") alternative,
  3. Add an OF_EARLYCON_DECLARE() for RZ/A2, to fix earlycon on RZ/A2.

What do you think?
Thanks for your comments!

P.S. Apparently SCIx_SH4_SCIF_REGTYPE and SCIx_SH2_SCIF_FIFODATA_REGTYPE
     are identical?

Geert Uytterhoeven (3):
  [RFC] sh-sci: Take into account regshift to fix earlycon breakage
  [RFC] Revert "serial: sh-sci: Compute the regshift value for SCI
    ports"
  [RFC] sh-sci: Derive regshift value from DT compatible value

Yoshinori Sato (1):
  [RFC] sh-sci: Use a separate sci_port for earlycon

 arch/sh/kernel/cpu/sh3/setup-sh770x.c |  1 +
 arch/sh/kernel/cpu/sh4/setup-sh7750.c |  3 +-
 arch/sh/kernel/cpu/sh4/setup-sh7760.c | 10 +---
 drivers/tty/serial/sh-sci.c           | 68 +++++++++++++++++----------
 include/linux/serial_sci.h            |  1 +
 5 files changed, 49 insertions(+), 34 deletions(-)

-- 
2.17.1

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
							    -- Linus Torvalds

[PATCH/RFC 0/4] sh-sci : Do not derive regshift from regsize

[Geert Uytterhoeven]

	Hi all,

This RFC patch series was sparked by noticing that commit 2d4dd0da45401c7a
("serial: sh-sci: Allow for compressed SCIF address") broke earlycon
support on most Renesas ARM SoCs using SCIF ports, and by the fragility of
deriving regshift from the register block size (which may be rounded up):
  1. The first patch is an old patch from Sato-san, which I never really
     understood.  But it turned out to be a dependency for patch 2.
  2. Patch 2 makes sure regshift is initialized when using earlycon,
     unbreaking the serial console on e.g. R-Car Gen2 and Gen3.
  3. Patch 3 reverts the patch that started deriving regshift from the
     register block size, and that removed the plat_sci_port.regshift
     field.  Which is a field I needed again in patch 4.
  4. Patch 4 removes the remaining regshift derivations on DT platforms.
 (5. I didn't bother writing patch 5, which involves adding .regshift
     initializations to all SH board files that need it.)

However, I'm not happy with the end result, so please DO NOT apply this!
As I spent almost a full day on this, and would still like to know the
story about "sh-sci: Use a separate sci_port for earlycon", I decided to
post it anyway.

As earlycon will be broken in v4.19-rc1 on RZ/A1, RZ/G, and R-Car, assuming
no other actions are taken, an alternative solution would be to:
  1. Revert commit 7acece71a517cad8 ("serial: sh-sci: Remove
     SCIx_RZ_SCIFA_REGTYPE"),
  2. Revert commit 2d4dd0da45401c7a ("serial: sh-sci: Allow for compressed
     SCIF address") alternative,
  3. Add an OF_EARLYCON_DECLARE() for RZ/A2, to fix earlycon on RZ/A2.

What do you think?
Thanks for your comments!

P.S. Apparently SCIx_SH4_SCIF_REGTYPE and SCIx_SH2_SCIF_FIFODATA_REGTYPE
     are identical?

Geert Uytterhoeven (3):
  [RFC] sh-sci: Take into account regshift to fix earlycon breakage
  [RFC] Revert "serial: sh-sci: Compute the regshift value for SCI
    ports"
  [RFC] sh-sci: Derive regshift value from DT compatible value

Yoshinori Sato (1):
  [RFC] sh-sci: Use a separate sci_port for earlycon

 arch/sh/kernel/cpu/sh3/setup-sh770x.c |  1 +
 arch/sh/kernel/cpu/sh4/setup-sh7750.c |  3 +-
 arch/sh/kernel/cpu/sh4/setup-sh7760.c | 10 +---
 drivers/tty/serial/sh-sci.c           | 68 +++++++++++++++++----------
 include/linux/serial_sci.h            |  1 +
 5 files changed, 49 insertions(+), 34 deletions(-)

-- 
2.17.1

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
							    -- Linus Torvalds

[PATCH/RFC 1/4] sh-sci: Use a separate sci_port for earlycon

[Geert Uytterhoeven]

From: Yoshinori Sato <ysato@users.sourceforge.jp>

The first sh-sci port and earlycon use the same sci_port.
Hence after the initialization of the first port, earlycon may die.

Fis this by assigning a separate sci_port for earlycon.

Signed-off-by: Yoshinori Sato <ysato@users.sourceforge.jp>
[geert: Rebased, reworded]
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
I still don't fully understand why Sato-san needed this for H8/300, and
we never needed it on Renesas ARM SoCs before.

Also, I don't like the early index.
---
 drivers/tty/serial/sh-sci.c | 21 ++++++++++++++-------
 1 file changed, 14 insertions(+), 7 deletions(-)

diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
index ac4424bf6b136cc4..cf3195bed0246a3d 100644
--- a/drivers/tty/serial/sh-sci.c
+++ b/drivers/tty/serial/sh-sci.c
@@ -164,6 +164,8 @@ struct sci_port {
 #define SCI_NPORTS CONFIG_SERIAL_SH_SCI_NR_UARTS
 
 static struct sci_port sci_ports[SCI_NPORTS];
+#define EARLY_INDEX 256
+static struct sci_port earlycon_port;
 static unsigned long sci_ports_in_use;
 static struct uart_driver sci_uart_driver;
 
@@ -2941,12 +2943,16 @@ static void serial_console_putchar(struct uart_port *port, int ch)
 static void serial_console_write(struct console *co, const char *s,
 				 unsigned count)
 {
-	struct sci_port *sci_port = &sci_ports[co->index];
-	struct uart_port *port = &sci_port->port;
 	unsigned short bits, ctrl, ctrl_temp;
+	struct sci_port *sci_port;
+	struct uart_port *port;
 	unsigned long flags;
 	int locked = 1;
 
+	sci_port = (co->index != EARLY_INDEX) ?
+		&sci_ports[co->index] : &earlycon_port;
+	port = &sci_port->port;
+
 #if defined(SUPPORT_SYSRQ)
 	if (port->sysrq)
 		locked = 0;
@@ -3367,12 +3373,13 @@ static int __init early_console_setup(struct earlycon_device *device,
 	device->port.serial_in = sci_serial_in;
 	device->port.serial_out	= sci_serial_out;
 	device->port.type = type;
-	memcpy(&sci_ports[0].port, &device->port, sizeof(struct uart_port));
+	device->con->index = EARLY_INDEX;
+	memcpy(&earlycon_port.port, &device->port, sizeof(struct uart_port));
 	port_cfg.type = type;
-	sci_ports[0].cfg = &port_cfg;
-	sci_ports[0].params = sci_probe_regmap(&port_cfg);
-	port_cfg.scscr = sci_serial_in(&sci_ports[0].port, SCSCR);
-	sci_serial_out(&sci_ports[0].port, SCSCR,
+	earlycon_port.cfg = &port_cfg;
+	earlycon_port.params = sci_probe_regmap(&port_cfg);
+	port_cfg.scscr = sci_serial_in(&earlycon_port.port, SCSCR);
+	sci_serial_out(&earlycon_port.port, SCSCR,
 		       SCSCR_RE | SCSCR_TE | port_cfg.scscr);
 
 	device->con->write = serial_console_write;
-- 
2.17.1

[PATCH/RFC 1/4] sh-sci: Use a separate sci_port for earlycon

[Geert Uytterhoeven]

From: Yoshinori Sato <ysato@users.sourceforge.jp>

The first sh-sci port and earlycon use the same sci_port.
Hence after the initialization of the first port, earlycon may die.

Fis this by assigning a separate sci_port for earlycon.

Signed-off-by: Yoshinori Sato <ysato@users.sourceforge.jp>
[geert: Rebased, reworded]
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
I still don't fully understand why Sato-san needed this for H8/300, and
we never needed it on Renesas ARM SoCs before.

Also, I don't like the early index.
---
 drivers/tty/serial/sh-sci.c | 21 ++++++++++++++-------
 1 file changed, 14 insertions(+), 7 deletions(-)

diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
index ac4424bf6b136cc4..cf3195bed0246a3d 100644
--- a/drivers/tty/serial/sh-sci.c
+++ b/drivers/tty/serial/sh-sci.c
@@ -164,6 +164,8 @@ struct sci_port {
 #define SCI_NPORTS CONFIG_SERIAL_SH_SCI_NR_UARTS
 
 static struct sci_port sci_ports[SCI_NPORTS];
+#define EARLY_INDEX 256
+static struct sci_port earlycon_port;
 static unsigned long sci_ports_in_use;
 static struct uart_driver sci_uart_driver;
 
@@ -2941,12 +2943,16 @@ static void serial_console_putchar(struct uart_port *port, int ch)
 static void serial_console_write(struct console *co, const char *s,
 				 unsigned count)
 {
-	struct sci_port *sci_port = &sci_ports[co->index];
-	struct uart_port *port = &sci_port->port;
 	unsigned short bits, ctrl, ctrl_temp;
+	struct sci_port *sci_port;
+	struct uart_port *port;
 	unsigned long flags;
 	int locked = 1;
 
+	sci_port = (co->index != EARLY_INDEX) ?
+		&sci_ports[co->index] : &earlycon_port;
+	port = &sci_port->port;
+
 #if defined(SUPPORT_SYSRQ)
 	if (port->sysrq)
 		locked = 0;
@@ -3367,12 +3373,13 @@ static int __init early_console_setup(struct earlycon_device *device,
 	device->port.serial_in = sci_serial_in;
 	device->port.serial_out	= sci_serial_out;
 	device->port.type = type;
-	memcpy(&sci_ports[0].port, &device->port, sizeof(struct uart_port));
+	device->con->index = EARLY_INDEX;
+	memcpy(&earlycon_port.port, &device->port, sizeof(struct uart_port));
 	port_cfg.type = type;
-	sci_ports[0].cfg = &port_cfg;
-	sci_ports[0].params = sci_probe_regmap(&port_cfg);
-	port_cfg.scscr = sci_serial_in(&sci_ports[0].port, SCSCR);
-	sci_serial_out(&sci_ports[0].port, SCSCR,
+	earlycon_port.cfg = &port_cfg;
+	earlycon_port.params = sci_probe_regmap(&port_cfg);
+	port_cfg.scscr = sci_serial_in(&earlycon_port.port, SCSCR);
+	sci_serial_out(&earlycon_port.port, SCSCR,
 		       SCSCR_RE | SCSCR_TE | port_cfg.scscr);
 
 	device->con->write = serial_console_write;
-- 
2.17.1

[PATCH/RFC 2/4] sh-sci: Take into account regshift to fix earlycon breakage

[Geert Uytterhoeven]

With the compressed SCIF address range, the SCIF ports on most Renesas
SoCs now use regshift 1.

However, the earlycon setup code always assumes regshift zero, breaking
earlycon on R-Car, RZ/A1, and RZ/G SoCs.

Fix this by initializing regshift to 1 for the generic SCIF case, and
adding a special entry for RZ/A2 SoCs that do need regshift 0.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
Fixes: 2d4dd0da45401c7a ("serial: sh-sci: Allow for compressed SCIF address")
---
Tested on R-Car Gen2 and Gen3.

This depends on the previous patch.  Else the kernel hangs when
initializing SCIF0 (why??).
---
 drivers/tty/serial/sh-sci.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
index cf3195bed0246a3d..caf4422d9e2e59e4 100644
--- a/drivers/tty/serial/sh-sci.c
+++ b/drivers/tty/serial/sh-sci.c
@@ -3390,9 +3390,17 @@ static int __init sci_early_console_setup(struct earlycon_device *device,
 {
 	return early_console_setup(device, PORT_SCI);
 }
+static int __init rza2_early_console_setup(struct earlycon_device *device,
+					  const char *opt)
+{
+	/* RZ/A2 uses the default regshift zero */
+	return early_console_setup(device, PORT_SCIF);
+}
 static int __init scif_early_console_setup(struct earlycon_device *device,
 					  const char *opt)
 {
+	/* Other plain SCIF variants */
+	device->port.regshift = 1;
 	return early_console_setup(device, PORT_SCIF);
 }
 static int __init scifa_early_console_setup(struct earlycon_device *device,
@@ -3412,6 +3420,7 @@ static int __init hscif_early_console_setup(struct earlycon_device *device,
 }
 
 OF_EARLYCON_DECLARE(sci, "renesas,sci", sci_early_console_setup);
+OF_EARLYCON_DECLARE(scif, "renesas,scif-r7s9210", rza2_early_console_setup);
 OF_EARLYCON_DECLARE(scif, "renesas,scif", scif_early_console_setup);
 OF_EARLYCON_DECLARE(scifa, "renesas,scifa", scifa_early_console_setup);
 OF_EARLYCON_DECLARE(scifb, "renesas,scifb", scifb_early_console_setup);
-- 
2.17.1

[PATCH/RFC 2/4] sh-sci: Take into account regshift to fix earlycon breakage

[Geert Uytterhoeven]

With the compressed SCIF address range, the SCIF ports on most Renesas
SoCs now use regshift 1.

However, the earlycon setup code always assumes regshift zero, breaking
earlycon on R-Car, RZ/A1, and RZ/G SoCs.

Fix this by initializing regshift to 1 for the generic SCIF case, and
adding a special entry for RZ/A2 SoCs that do need regshift 0.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
Fixes: 2d4dd0da45401c7a ("serial: sh-sci: Allow for compressed SCIF address")
---
Tested on R-Car Gen2 and Gen3.

This depends on the previous patch.  Else the kernel hangs when
initializing SCIF0 (why??).
---
 drivers/tty/serial/sh-sci.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
index cf3195bed0246a3d..caf4422d9e2e59e4 100644
--- a/drivers/tty/serial/sh-sci.c
+++ b/drivers/tty/serial/sh-sci.c
@@ -3390,9 +3390,17 @@ static int __init sci_early_console_setup(struct earlycon_device *device,
 {
 	return early_console_setup(device, PORT_SCI);
 }
+static int __init rza2_early_console_setup(struct earlycon_device *device,
+					  const char *opt)
+{
+	/* RZ/A2 uses the default regshift zero */
+	return early_console_setup(device, PORT_SCIF);
+}
 static int __init scif_early_console_setup(struct earlycon_device *device,
 					  const char *opt)
 {
+	/* Other plain SCIF variants */
+	device->port.regshift = 1;
 	return early_console_setup(device, PORT_SCIF);
 }
 static int __init scifa_early_console_setup(struct earlycon_device *device,
@@ -3412,6 +3420,7 @@ static int __init hscif_early_console_setup(struct earlycon_device *device,
 }
 
 OF_EARLYCON_DECLARE(sci, "renesas,sci", sci_early_console_setup);
+OF_EARLYCON_DECLARE(scif, "renesas,scif-r7s9210", rza2_early_console_setup);
 OF_EARLYCON_DECLARE(scif, "renesas,scif", scif_early_console_setup);
 OF_EARLYCON_DECLARE(scifa, "renesas,scifa", scifa_early_console_setup);
 OF_EARLYCON_DECLARE(scifb, "renesas,scifb", scifb_early_console_setup);
-- 
2.17.1

[PATCH/RFC 3/4] Revert "serial: sh-sci: Compute the regshift value for SCI ports"

[Geert Uytterhoeven]

This reverts commit dfc80387aefb78161f83732804c6d01c89c24595.

Deriving the proper regshift value from the register block size is
fragile, as it may have been rounded up.

Furthermore we will need plat_sci_port.regshift again.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 arch/sh/kernel/cpu/sh3/setup-sh770x.c |  1 +
 arch/sh/kernel/cpu/sh4/setup-sh7750.c |  3 ++-
 arch/sh/kernel/cpu/sh4/setup-sh7760.c | 10 ++--------
 drivers/tty/serial/sh-sci.c           |  8 +-------
 include/linux/serial_sci.h            |  1 +
 5 files changed, 7 insertions(+), 16 deletions(-)

diff --git a/arch/sh/kernel/cpu/sh3/setup-sh770x.c b/arch/sh/kernel/cpu/sh3/setup-sh770x.c
index 59a88611df55ac8f..592cd9ab30c4272f 100644
--- a/arch/sh/kernel/cpu/sh3/setup-sh770x.c
+++ b/arch/sh/kernel/cpu/sh3/setup-sh770x.c
@@ -111,6 +111,7 @@ static struct platform_device rtc_device = {
 static struct plat_sci_port scif0_platform_data = {
 	.type		= PORT_SCI,
 	.ops		= &sh770x_sci_port_ops,
+	.regshift	= 1,
 };
 
 static struct resource scif0_resources[] = {
diff --git a/arch/sh/kernel/cpu/sh4/setup-sh7750.c b/arch/sh/kernel/cpu/sh4/setup-sh7750.c
index 57d30689204d03b9..d98a55416306baef 100644
--- a/arch/sh/kernel/cpu/sh4/setup-sh7750.c
+++ b/arch/sh/kernel/cpu/sh4/setup-sh7750.c
@@ -39,10 +39,11 @@ static struct platform_device rtc_device = {
 
 static struct plat_sci_port sci_platform_data = {
 	.type		= PORT_SCI,
+	.regshift	= 2,
 };
 
 static struct resource sci_resources[] = {
-	DEFINE_RES_MEM(0xffe00000, 0x20),
+	DEFINE_RES_MEM(0xffe00000, 0x100),
 	DEFINE_RES_IRQ(evt2irq(0x4e0)),
 };
 
diff --git a/arch/sh/kernel/cpu/sh4/setup-sh7760.c b/arch/sh/kernel/cpu/sh4/setup-sh7760.c
index e51fe1734e1368e8..0c0cdfc69dcc3e33 100644
--- a/arch/sh/kernel/cpu/sh4/setup-sh7760.c
+++ b/arch/sh/kernel/cpu/sh4/setup-sh7760.c
@@ -200,18 +200,12 @@ static struct platform_device scif2_device = {
 };
 
 static struct plat_sci_port scif3_platform_data = {
-	/*
-	 * This is actually a SIM card module serial port, based on an SCI with
-	 * additional registers. The sh-sci driver doesn't support the SIM port
-	 * type, declare it as a SCI. Don't declare the additional registers in
-	 * the memory resource or the driver will compute an incorrect regshift
-	 * value.
-	 */
 	.type		= PORT_SCI,
+	.regshift	= 2,
 };
 
 static struct resource scif3_resources[] = {
-	DEFINE_RES_MEM(0xfe480000, 0x10),
+	DEFINE_RES_MEM(0xfe480000, 0x100),
 	DEFINE_RES_IRQ(evt2irq(0xc00)),
 	DEFINE_RES_IRQ(evt2irq(0xc20)),
 	DEFINE_RES_IRQ(evt2irq(0xc40)),
diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
index caf4422d9e2e59e4..955c057dff6e8c78 100644
--- a/drivers/tty/serial/sh-sci.c
+++ b/drivers/tty/serial/sh-sci.c
@@ -2895,15 +2895,9 @@ static int sci_init_single(struct platform_device *dev,
 
 	port->type		= p->type;
 	port->flags		= UPF_FIXED_PORT | UPF_BOOT_AUTOCONF | p->flags;
+	port->regshift		= p->regshift;
 	port->fifosize		= sci_port->params->fifosize;
 
-	if (port->type = PORT_SCI) {
-		if (sci_port->reg_size >= 0x20)
-			port->regshift = 2;
-		else
-			port->regshift = 1;
-	}
-
 	if (regtype = SCIx_SH4_SCIF_REGTYPE)
 		if (sci_port->reg_size >= 0x20)
 			port->regshift = 1;
diff --git a/include/linux/serial_sci.h b/include/linux/serial_sci.h
index c0e795d95477daea..eebb12fc473f49a2 100644
--- a/include/linux/serial_sci.h
+++ b/include/linux/serial_sci.h
@@ -57,6 +57,7 @@ struct plat_sci_port {
 	/*
 	 * Platform overrides if necessary, defaults otherwise.
 	 */
+	unsigned char	regshift;
 	unsigned char	regtype;
 
 	struct plat_sci_port_ops	*ops;
-- 
2.17.1

[PATCH/RFC 3/4] Revert "serial: sh-sci: Compute the regshift value for SCI ports"

[Geert Uytterhoeven]

This reverts commit dfc80387aefb78161f83732804c6d01c89c24595.

Deriving the proper regshift value from the register block size is
fragile, as it may have been rounded up.

Furthermore we will need plat_sci_port.regshift again.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 arch/sh/kernel/cpu/sh3/setup-sh770x.c |  1 +
 arch/sh/kernel/cpu/sh4/setup-sh7750.c |  3 ++-
 arch/sh/kernel/cpu/sh4/setup-sh7760.c | 10 ++--------
 drivers/tty/serial/sh-sci.c           |  8 +-------
 include/linux/serial_sci.h            |  1 +
 5 files changed, 7 insertions(+), 16 deletions(-)

diff --git a/arch/sh/kernel/cpu/sh3/setup-sh770x.c b/arch/sh/kernel/cpu/sh3/setup-sh770x.c
index 59a88611df55ac8f..592cd9ab30c4272f 100644
--- a/arch/sh/kernel/cpu/sh3/setup-sh770x.c
+++ b/arch/sh/kernel/cpu/sh3/setup-sh770x.c
@@ -111,6 +111,7 @@ static struct platform_device rtc_device = {
 static struct plat_sci_port scif0_platform_data = {
 	.type		= PORT_SCI,
 	.ops		= &sh770x_sci_port_ops,
+	.regshift	= 1,
 };
 
 static struct resource scif0_resources[] = {
diff --git a/arch/sh/kernel/cpu/sh4/setup-sh7750.c b/arch/sh/kernel/cpu/sh4/setup-sh7750.c
index 57d30689204d03b9..d98a55416306baef 100644
--- a/arch/sh/kernel/cpu/sh4/setup-sh7750.c
+++ b/arch/sh/kernel/cpu/sh4/setup-sh7750.c
@@ -39,10 +39,11 @@ static struct platform_device rtc_device = {
 
 static struct plat_sci_port sci_platform_data = {
 	.type		= PORT_SCI,
+	.regshift	= 2,
 };
 
 static struct resource sci_resources[] = {
-	DEFINE_RES_MEM(0xffe00000, 0x20),
+	DEFINE_RES_MEM(0xffe00000, 0x100),
 	DEFINE_RES_IRQ(evt2irq(0x4e0)),
 };
 
diff --git a/arch/sh/kernel/cpu/sh4/setup-sh7760.c b/arch/sh/kernel/cpu/sh4/setup-sh7760.c
index e51fe1734e1368e8..0c0cdfc69dcc3e33 100644
--- a/arch/sh/kernel/cpu/sh4/setup-sh7760.c
+++ b/arch/sh/kernel/cpu/sh4/setup-sh7760.c
@@ -200,18 +200,12 @@ static struct platform_device scif2_device = {
 };
 
 static struct plat_sci_port scif3_platform_data = {
-	/*
-	 * This is actually a SIM card module serial port, based on an SCI with
-	 * additional registers. The sh-sci driver doesn't support the SIM port
-	 * type, declare it as a SCI. Don't declare the additional registers in
-	 * the memory resource or the driver will compute an incorrect regshift
-	 * value.
-	 */
 	.type		= PORT_SCI,
+	.regshift	= 2,
 };
 
 static struct resource scif3_resources[] = {
-	DEFINE_RES_MEM(0xfe480000, 0x10),
+	DEFINE_RES_MEM(0xfe480000, 0x100),
 	DEFINE_RES_IRQ(evt2irq(0xc00)),
 	DEFINE_RES_IRQ(evt2irq(0xc20)),
 	DEFINE_RES_IRQ(evt2irq(0xc40)),
diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
index caf4422d9e2e59e4..955c057dff6e8c78 100644
--- a/drivers/tty/serial/sh-sci.c
+++ b/drivers/tty/serial/sh-sci.c
@@ -2895,15 +2895,9 @@ static int sci_init_single(struct platform_device *dev,
 
 	port->type		= p->type;
 	port->flags		= UPF_FIXED_PORT | UPF_BOOT_AUTOCONF | p->flags;
+	port->regshift		= p->regshift;
 	port->fifosize		= sci_port->params->fifosize;
 
-	if (port->type == PORT_SCI) {
-		if (sci_port->reg_size >= 0x20)
-			port->regshift = 2;
-		else
-			port->regshift = 1;
-	}
-
 	if (regtype == SCIx_SH4_SCIF_REGTYPE)
 		if (sci_port->reg_size >= 0x20)
 			port->regshift = 1;
diff --git a/include/linux/serial_sci.h b/include/linux/serial_sci.h
index c0e795d95477daea..eebb12fc473f49a2 100644
--- a/include/linux/serial_sci.h
+++ b/include/linux/serial_sci.h
@@ -57,6 +57,7 @@ struct plat_sci_port {
 	/*
 	 * Platform overrides if necessary, defaults otherwise.
 	 */
+	unsigned char	regshift;
 	unsigned char	regtype;
 
 	struct plat_sci_port_ops	*ops;
-- 
2.17.1

Re: [PATCH/RFC 3/4] Revert "serial: sh-sci: Compute the regshift value for SCI ports"

[Laurent Pinchart]

Hi Geert,

Thank you for the patch.

On Monday, 6 August 2018 17:07:54 EEST Geert Uytterhoeven wrote:
> This reverts commit dfc80387aefb78161f83732804c6d01c89c24595.
> 
> Deriving the proper regshift value from the register block size is
> fragile, as it may have been rounded up.
> 
> Furthermore we will need plat_sci_port.regshift again.

Won't this break bisection ? Shouldn't you squash patches 3 and 4 together ?

Does this mechanism break anything on non-DT platforms ? If not I'd rather 
keep it for them, and only use compat strings for DT platforms, to avoiding 
adding back a platform data field.

> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
>  arch/sh/kernel/cpu/sh3/setup-sh770x.c |  1 +
>  arch/sh/kernel/cpu/sh4/setup-sh7750.c |  3 ++-
>  arch/sh/kernel/cpu/sh4/setup-sh7760.c | 10 ++--------
>  drivers/tty/serial/sh-sci.c           |  8 +-------
>  include/linux/serial_sci.h            |  1 +
>  5 files changed, 7 insertions(+), 16 deletions(-)
> 
> diff --git a/arch/sh/kernel/cpu/sh3/setup-sh770x.c
> b/arch/sh/kernel/cpu/sh3/setup-sh770x.c index
> 59a88611df55ac8f..592cd9ab30c4272f 100644
> --- a/arch/sh/kernel/cpu/sh3/setup-sh770x.c
> +++ b/arch/sh/kernel/cpu/sh3/setup-sh770x.c
> @@ -111,6 +111,7 @@ static struct platform_device rtc_device = {
>  static struct plat_sci_port scif0_platform_data = {
>  	.type		= PORT_SCI,
>  	.ops		= &sh770x_sci_port_ops,
> +	.regshift	= 1,
>  };
> 
>  static struct resource scif0_resources[] = {
> diff --git a/arch/sh/kernel/cpu/sh4/setup-sh7750.c
> b/arch/sh/kernel/cpu/sh4/setup-sh7750.c index
> 57d30689204d03b9..d98a55416306baef 100644
> --- a/arch/sh/kernel/cpu/sh4/setup-sh7750.c
> +++ b/arch/sh/kernel/cpu/sh4/setup-sh7750.c
> @@ -39,10 +39,11 @@ static struct platform_device rtc_device = {
> 
>  static struct plat_sci_port sci_platform_data = {
>  	.type		= PORT_SCI,
> +	.regshift	= 2,
>  };
> 
>  static struct resource sci_resources[] = {
> -	DEFINE_RES_MEM(0xffe00000, 0x20),
> +	DEFINE_RES_MEM(0xffe00000, 0x100),
>  	DEFINE_RES_IRQ(evt2irq(0x4e0)),
>  };
> 
> diff --git a/arch/sh/kernel/cpu/sh4/setup-sh7760.c
> b/arch/sh/kernel/cpu/sh4/setup-sh7760.c index
> e51fe1734e1368e8..0c0cdfc69dcc3e33 100644
> --- a/arch/sh/kernel/cpu/sh4/setup-sh7760.c
> +++ b/arch/sh/kernel/cpu/sh4/setup-sh7760.c
> @@ -200,18 +200,12 @@ static struct platform_device scif2_device = {
>  };
> 
>  static struct plat_sci_port scif3_platform_data = {
> -	/*
> -	 * This is actually a SIM card module serial port, based on an SCI with
> -	 * additional registers. The sh-sci driver doesn't support the SIM port
> -	 * type, declare it as a SCI. Don't declare the additional registers in
> -	 * the memory resource or the driver will compute an incorrect regshift
> -	 * value.
> -	 */
>  	.type		= PORT_SCI,
> +	.regshift	= 2,
>  };
> 
>  static struct resource scif3_resources[] = {
> -	DEFINE_RES_MEM(0xfe480000, 0x10),
> +	DEFINE_RES_MEM(0xfe480000, 0x100),
>  	DEFINE_RES_IRQ(evt2irq(0xc00)),
>  	DEFINE_RES_IRQ(evt2irq(0xc20)),
>  	DEFINE_RES_IRQ(evt2irq(0xc40)),
> diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
> index caf4422d9e2e59e4..955c057dff6e8c78 100644
> --- a/drivers/tty/serial/sh-sci.c
> +++ b/drivers/tty/serial/sh-sci.c
> @@ -2895,15 +2895,9 @@ static int sci_init_single(struct platform_device
> *dev,
> 
>  	port->type		= p->type;
>  	port->flags		= UPF_FIXED_PORT | UPF_BOOT_AUTOCONF | p->flags;
> +	port->regshift		= p->regshift;
>  	port->fifosize		= sci_port->params->fifosize;
> 
> -	if (port->type = PORT_SCI) {
> -		if (sci_port->reg_size >= 0x20)
> -			port->regshift = 2;
> -		else
> -			port->regshift = 1;
> -	}
> -
>  	if (regtype = SCIx_SH4_SCIF_REGTYPE)
>  		if (sci_port->reg_size >= 0x20)
>  			port->regshift = 1;
> diff --git a/include/linux/serial_sci.h b/include/linux/serial_sci.h
> index c0e795d95477daea..eebb12fc473f49a2 100644
> --- a/include/linux/serial_sci.h
> +++ b/include/linux/serial_sci.h
> @@ -57,6 +57,7 @@ struct plat_sci_port {
>  	/*
>  	 * Platform overrides if necessary, defaults otherwise.
>  	 */
> +	unsigned char	regshift;
>  	unsigned char	regtype;
> 
>  	struct plat_sci_port_ops	*ops;

-- 
Regards,

Laurent Pinchart

Re: [PATCH/RFC 3/4] Revert "serial: sh-sci: Compute the regshift value for SCI ports"

[Laurent Pinchart]

Hi Geert,

Thank you for the patch.

On Monday, 6 August 2018 17:07:54 EEST Geert Uytterhoeven wrote:
> This reverts commit dfc80387aefb78161f83732804c6d01c89c24595.
> 
> Deriving the proper regshift value from the register block size is
> fragile, as it may have been rounded up.
> 
> Furthermore we will need plat_sci_port.regshift again.

Won't this break bisection ? Shouldn't you squash patches 3 and 4 together ?

Does this mechanism break anything on non-DT platforms ? If not I'd rather 
keep it for them, and only use compat strings for DT platforms, to avoiding 
adding back a platform data field.

> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
>  arch/sh/kernel/cpu/sh3/setup-sh770x.c |  1 +
>  arch/sh/kernel/cpu/sh4/setup-sh7750.c |  3 ++-
>  arch/sh/kernel/cpu/sh4/setup-sh7760.c | 10 ++--------
>  drivers/tty/serial/sh-sci.c           |  8 +-------
>  include/linux/serial_sci.h            |  1 +
>  5 files changed, 7 insertions(+), 16 deletions(-)
> 
> diff --git a/arch/sh/kernel/cpu/sh3/setup-sh770x.c
> b/arch/sh/kernel/cpu/sh3/setup-sh770x.c index
> 59a88611df55ac8f..592cd9ab30c4272f 100644
> --- a/arch/sh/kernel/cpu/sh3/setup-sh770x.c
> +++ b/arch/sh/kernel/cpu/sh3/setup-sh770x.c
> @@ -111,6 +111,7 @@ static struct platform_device rtc_device = {
>  static struct plat_sci_port scif0_platform_data = {
>  	.type		= PORT_SCI,
>  	.ops		= &sh770x_sci_port_ops,
> +	.regshift	= 1,
>  };
> 
>  static struct resource scif0_resources[] = {
> diff --git a/arch/sh/kernel/cpu/sh4/setup-sh7750.c
> b/arch/sh/kernel/cpu/sh4/setup-sh7750.c index
> 57d30689204d03b9..d98a55416306baef 100644
> --- a/arch/sh/kernel/cpu/sh4/setup-sh7750.c
> +++ b/arch/sh/kernel/cpu/sh4/setup-sh7750.c
> @@ -39,10 +39,11 @@ static struct platform_device rtc_device = {
> 
>  static struct plat_sci_port sci_platform_data = {
>  	.type		= PORT_SCI,
> +	.regshift	= 2,
>  };
> 
>  static struct resource sci_resources[] = {
> -	DEFINE_RES_MEM(0xffe00000, 0x20),
> +	DEFINE_RES_MEM(0xffe00000, 0x100),
>  	DEFINE_RES_IRQ(evt2irq(0x4e0)),
>  };
> 
> diff --git a/arch/sh/kernel/cpu/sh4/setup-sh7760.c
> b/arch/sh/kernel/cpu/sh4/setup-sh7760.c index
> e51fe1734e1368e8..0c0cdfc69dcc3e33 100644
> --- a/arch/sh/kernel/cpu/sh4/setup-sh7760.c
> +++ b/arch/sh/kernel/cpu/sh4/setup-sh7760.c
> @@ -200,18 +200,12 @@ static struct platform_device scif2_device = {
>  };
> 
>  static struct plat_sci_port scif3_platform_data = {
> -	/*
> -	 * This is actually a SIM card module serial port, based on an SCI with
> -	 * additional registers. The sh-sci driver doesn't support the SIM port
> -	 * type, declare it as a SCI. Don't declare the additional registers in
> -	 * the memory resource or the driver will compute an incorrect regshift
> -	 * value.
> -	 */
>  	.type		= PORT_SCI,
> +	.regshift	= 2,
>  };
> 
>  static struct resource scif3_resources[] = {
> -	DEFINE_RES_MEM(0xfe480000, 0x10),
> +	DEFINE_RES_MEM(0xfe480000, 0x100),
>  	DEFINE_RES_IRQ(evt2irq(0xc00)),
>  	DEFINE_RES_IRQ(evt2irq(0xc20)),
>  	DEFINE_RES_IRQ(evt2irq(0xc40)),
> diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
> index caf4422d9e2e59e4..955c057dff6e8c78 100644
> --- a/drivers/tty/serial/sh-sci.c
> +++ b/drivers/tty/serial/sh-sci.c
> @@ -2895,15 +2895,9 @@ static int sci_init_single(struct platform_device
> *dev,
> 
>  	port->type		= p->type;
>  	port->flags		= UPF_FIXED_PORT | UPF_BOOT_AUTOCONF | p->flags;
> +	port->regshift		= p->regshift;
>  	port->fifosize		= sci_port->params->fifosize;
> 
> -	if (port->type == PORT_SCI) {
> -		if (sci_port->reg_size >= 0x20)
> -			port->regshift = 2;
> -		else
> -			port->regshift = 1;
> -	}
> -
>  	if (regtype == SCIx_SH4_SCIF_REGTYPE)
>  		if (sci_port->reg_size >= 0x20)
>  			port->regshift = 1;
> diff --git a/include/linux/serial_sci.h b/include/linux/serial_sci.h
> index c0e795d95477daea..eebb12fc473f49a2 100644
> --- a/include/linux/serial_sci.h
> +++ b/include/linux/serial_sci.h
> @@ -57,6 +57,7 @@ struct plat_sci_port {
>  	/*
>  	 * Platform overrides if necessary, defaults otherwise.
>  	 */
> +	unsigned char	regshift;
>  	unsigned char	regtype;
> 
>  	struct plat_sci_port_ops	*ops;

-- 
Regards,

Laurent Pinchart

Re: [PATCH/RFC 3/4] Revert "serial: sh-sci: Compute the regshift value for SCI ports"

[Geert Uytterhoeven]

Hi Laurent,

On Mon, Aug 6, 2018 at 4:16 PM Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> On Monday, 6 August 2018 17:07:54 EEST Geert Uytterhoeven wrote:
> > This reverts commit dfc80387aefb78161f83732804c6d01c89c24595.
> >
> > Deriving the proper regshift value from the register block size is
> > fragile, as it may have been rounded up.
> >
> > Furthermore we will need plat_sci_port.regshift again.
>
> Won't this break bisection ? Shouldn't you squash patches 3 and 4 together ?
>
> Does this mechanism break anything on non-DT platforms ? If not I'd rather
> keep it for them, and only use compat strings for DT platforms, to avoiding
> adding back a platform data field.

Actually I think the original commit broke SCI on H8/300, as regshift should be
zero on that platform.
But that was not discovered until recently.

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/RFC 3/4] Revert "serial: sh-sci: Compute the regshift value for SCI ports"

[Geert Uytterhoeven]

Hi Laurent,

On Mon, Aug 6, 2018 at 4:16 PM Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> On Monday, 6 August 2018 17:07:54 EEST Geert Uytterhoeven wrote:
> > This reverts commit dfc80387aefb78161f83732804c6d01c89c24595.
> >
> > Deriving the proper regshift value from the register block size is
> > fragile, as it may have been rounded up.
> >
> > Furthermore we will need plat_sci_port.regshift again.
>
> Won't this break bisection ? Shouldn't you squash patches 3 and 4 together ?
>
> Does this mechanism break anything on non-DT platforms ? If not I'd rather
> keep it for them, and only use compat strings for DT platforms, to avoiding
> adding back a platform data field.

Actually I think the original commit broke SCI on H8/300, as regshift should be
zero on that platform.
But that was not discovered until recently.

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/RFC 3/4] Revert "serial: sh-sci: Compute the regshift value for SCI ports"

[Laurent Pinchart]

Hi Geert,

On Monday, 6 August 2018 17:34:34 EEST Geert Uytterhoeven wrote:
> On Mon, Aug 6, 2018 at 4:16 PM Laurent Pinchart wrote:
> > On Monday, 6 August 2018 17:07:54 EEST Geert Uytterhoeven wrote:
> >> This reverts commit dfc80387aefb78161f83732804c6d01c89c24595.
> >> 
> >> Deriving the proper regshift value from the register block size is
> >> fragile, as it may have been rounded up.
> >> 
> >> Furthermore we will need plat_sci_port.regshift again.
> > 
> > Won't this break bisection ? Shouldn't you squash patches 3 and 4 together
> > ?
> > 
> > Does this mechanism break anything on non-DT platforms ? If not I'd rather
> > keep it for them, and only use compat strings for DT platforms, to
> > avoiding adding back a platform data field.
> 
> Actually I think the original commit broke SCI on H8/300, as regshift should
> be zero on that platform.
> But that was not discovered until recently.

So there are still people booting H3/800 ? :-)

Could we still keep the mechanism for SH and fix H8/300 with special handling 
somewhere ?

-- 
Regards,

Laurent Pinchart

Re: [PATCH/RFC 3/4] Revert "serial: sh-sci: Compute the regshift value for SCI ports"

[Laurent Pinchart]

Hi Geert,

On Monday, 6 August 2018 17:34:34 EEST Geert Uytterhoeven wrote:
> On Mon, Aug 6, 2018 at 4:16 PM Laurent Pinchart wrote:
> > On Monday, 6 August 2018 17:07:54 EEST Geert Uytterhoeven wrote:
> >> This reverts commit dfc80387aefb78161f83732804c6d01c89c24595.
> >> 
> >> Deriving the proper regshift value from the register block size is
> >> fragile, as it may have been rounded up.
> >> 
> >> Furthermore we will need plat_sci_port.regshift again.
> > 
> > Won't this break bisection ? Shouldn't you squash patches 3 and 4 together
> > ?
> > 
> > Does this mechanism break anything on non-DT platforms ? If not I'd rather
> > keep it for them, and only use compat strings for DT platforms, to
> > avoiding adding back a platform data field.
> 
> Actually I think the original commit broke SCI on H8/300, as regshift should
> be zero on that platform.
> But that was not discovered until recently.

So there are still people booting H3/800 ? :-)

Could we still keep the mechanism for SH and fix H8/300 with special handling 
somewhere ?

-- 
Regards,

Laurent Pinchart

Re: [PATCH/RFC 3/4] Revert "serial: sh-sci: Compute the regshift value for SCI ports"

[Geert Uytterhoeven]

Hi Laurent,

On Mon, Aug 6, 2018 at 4:40 PM Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> On Monday, 6 August 2018 17:34:34 EEST Geert Uytterhoeven wrote:
> > On Mon, Aug 6, 2018 at 4:16 PM Laurent Pinchart wrote:
> > > On Monday, 6 August 2018 17:07:54 EEST Geert Uytterhoeven wrote:
> > >> This reverts commit dfc80387aefb78161f83732804c6d01c89c24595.
> > >>
> > >> Deriving the proper regshift value from the register block size is
> > >> fragile, as it may have been rounded up.
> > >>
> > >> Furthermore we will need plat_sci_port.regshift again.
> > >
> > > Won't this break bisection ? Shouldn't you squash patches 3 and 4 together
> > > ?
> > >
> > > Does this mechanism break anything on non-DT platforms ? If not I'd rather
> > > keep it for them, and only use compat strings for DT platforms, to
> > > avoiding adding back a platform data field.
> >
> > Actually I think the original commit broke SCI on H8/300, as regshift should
> > be zero on that platform.
> > But that was not discovered until recently.
>
> So there are still people booting H3/800 ? :-)

Yes, remember, H8/300 was resurrected with full DT support.
SuperH is the legacy one ;-)

> Could we still keep the mechanism for SH and fix H8/300 with special handling
> somewhere ?

An independent H8/300 fix is "[PATCH v2] serial: sh-sci: byte allocated
register support" (https://www.spinics.net/lists/linux-sh/msg53175.html).

The main issue is: do we bother with regshift or not?
If yes, we have to handle it correctly for both normal serial port handling and
earlycon.
For the latter, we don't have the register block size available.

As of commit 2eaa790989e03900 ("earlycon: Use common framework for
earlycon declarations"), DT and non-DT based earlycon have been merged,
so your original commit may have impacted earlycon on non-DT based
systems, too.  I don't know for sure...

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/RFC 3/4] Revert "serial: sh-sci: Compute the regshift value for SCI ports"

[Geert Uytterhoeven]

Hi Laurent,

On Mon, Aug 6, 2018 at 4:40 PM Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> On Monday, 6 August 2018 17:34:34 EEST Geert Uytterhoeven wrote:
> > On Mon, Aug 6, 2018 at 4:16 PM Laurent Pinchart wrote:
> > > On Monday, 6 August 2018 17:07:54 EEST Geert Uytterhoeven wrote:
> > >> This reverts commit dfc80387aefb78161f83732804c6d01c89c24595.
> > >>
> > >> Deriving the proper regshift value from the register block size is
> > >> fragile, as it may have been rounded up.
> > >>
> > >> Furthermore we will need plat_sci_port.regshift again.
> > >
> > > Won't this break bisection ? Shouldn't you squash patches 3 and 4 together
> > > ?
> > >
> > > Does this mechanism break anything on non-DT platforms ? If not I'd rather
> > > keep it for them, and only use compat strings for DT platforms, to
> > > avoiding adding back a platform data field.
> >
> > Actually I think the original commit broke SCI on H8/300, as regshift should
> > be zero on that platform.
> > But that was not discovered until recently.
>
> So there are still people booting H3/800 ? :-)

Yes, remember, H8/300 was resurrected with full DT support.
SuperH is the legacy one ;-)

> Could we still keep the mechanism for SH and fix H8/300 with special handling
> somewhere ?

An independent H8/300 fix is "[PATCH v2] serial: sh-sci: byte allocated
register support" (https://www.spinics.net/lists/linux-sh/msg53175.html).

The main issue is: do we bother with regshift or not?
If yes, we have to handle it correctly for both normal serial port handling and
earlycon.
For the latter, we don't have the register block size available.

As of commit 2eaa790989e03900 ("earlycon: Use common framework for
earlycon declarations"), DT and non-DT based earlycon have been merged,
so your original commit may have impacted earlycon on non-DT based
systems, too.  I don't know for sure...

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

[PATCH/RFC 4/4] sh-sci: Derive regshift value from DT compatible value

[Geert Uytterhoeven]

Deriving the proper regshift value from the register block size is
fragile (it may have been rounded up in DT, and the mapping granularity
is usually PAGE_SIZE anyway), and turned out to be inappropriate for
earlycon support (the size is not easily available).

On DT systems, derive it from the compatible value instead.
This requires adding an entry for RZ/A2 serial ports, which use an
atypical regshift value.

On non-DT systems the regshift value is still derived from the register
block size, as before.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
  - Sato-san: I assume this fixes SCI on H8/300, too?
    Cfr. your patch "serial: sh-sci: byte allocated register support"
    (https://www.spinics.net/lists/linux-sh/msg53175.html).

  - Getting rid of the regshift setup for non-DT systems probably means
    we'll have to add ".regshift = 1" to each and every SH board file
    describing SCIF serial ports :-(

Any other suggestions?
---
 drivers/tty/serial/sh-sci.c | 32 ++++++++++++++++++++------------
 1 file changed, 20 insertions(+), 12 deletions(-)

diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
index 955c057dff6e8c78..c4342e61b8db72c3 100644
--- a/drivers/tty/serial/sh-sci.c
+++ b/drivers/tty/serial/sh-sci.c
@@ -2898,9 +2898,10 @@ static int sci_init_single(struct platform_device *dev,
 	port->regshift		= p->regshift;
 	port->fifosize		= sci_port->params->fifosize;
 
-	if (regtype = SCIx_SH4_SCIF_REGTYPE)
+	if (!dev->dev.of_node && regtype = SCIx_SH4_SCIF_REGTYPE) {
 		if (sci_port->reg_size >= 0x20)
 			port->regshift = 1;
+	}
 
 	/*
 	 * The UART port needs an IRQ value, so we peg this to the RX IRQ
@@ -3100,43 +3101,49 @@ static int sci_remove(struct platform_device *dev)
 }
 
 
-#define SCI_OF_DATA(type, regtype)	(void *)((type) << 16 | (regtype))
+#define SCI_OF_DATA(type, regtype, regshift)	\
+			(void *)((type) << 16 | (regtype) << 8 | (regshift))
 #define SCI_OF_TYPE(data)		((unsigned long)(data) >> 16)
-#define SCI_OF_REGTYPE(data)		((unsigned long)(data) & 0xffff)
+#define SCI_OF_REGTYPE(data)		(((unsigned long)(data) >> 8) & 0xff)
+#define SCI_OF_REGSHIFT(data)		((unsigned long)(data) & 0xff)
 
 static const struct of_device_id of_sci_match[] = {
 	/* SoC-specific types */
 	{
 		.compatible = "renesas,scif-r7s72100",
-		.data = SCI_OF_DATA(PORT_SCIF, SCIx_SH2_SCIF_FIFODATA_REGTYPE),
+		.data = SCI_OF_DATA(PORT_SCIF, SCIx_SH2_SCIF_FIFODATA_REGTYPE,
+				    0),
+	}, {
+		.compatible = "renesas,scif-r7s9210",
+		.data = SCI_OF_DATA(PORT_SCIF, SCIx_SH4_SCIF_REGTYPE, 0),
 	},
 	/* Family-specific types */
 	{
 		.compatible = "renesas,rcar-gen1-scif",
-		.data = SCI_OF_DATA(PORT_SCIF, SCIx_SH4_SCIF_BRG_REGTYPE),
+		.data = SCI_OF_DATA(PORT_SCIF, SCIx_SH4_SCIF_BRG_REGTYPE, 0),
 	}, {
 		.compatible = "renesas,rcar-gen2-scif",
-		.data = SCI_OF_DATA(PORT_SCIF, SCIx_SH4_SCIF_BRG_REGTYPE),
+		.data = SCI_OF_DATA(PORT_SCIF, SCIx_SH4_SCIF_BRG_REGTYPE, 0),
 	}, {
 		.compatible = "renesas,rcar-gen3-scif",
-		.data = SCI_OF_DATA(PORT_SCIF, SCIx_SH4_SCIF_BRG_REGTYPE),
+		.data = SCI_OF_DATA(PORT_SCIF, SCIx_SH4_SCIF_BRG_REGTYPE, 0),
 	},
 	/* Generic types */
 	{
 		.compatible = "renesas,scif",
-		.data = SCI_OF_DATA(PORT_SCIF, SCIx_SH4_SCIF_REGTYPE),
+		.data = SCI_OF_DATA(PORT_SCIF, SCIx_SH4_SCIF_REGTYPE, 1),
 	}, {
 		.compatible = "renesas,scifa",
-		.data = SCI_OF_DATA(PORT_SCIFA, SCIx_SCIFA_REGTYPE),
+		.data = SCI_OF_DATA(PORT_SCIFA, SCIx_SCIFA_REGTYPE, 0),
 	}, {
 		.compatible = "renesas,scifb",
-		.data = SCI_OF_DATA(PORT_SCIFB, SCIx_SCIFB_REGTYPE),
+		.data = SCI_OF_DATA(PORT_SCIFB, SCIx_SCIFB_REGTYPE, 0),
 	}, {
 		.compatible = "renesas,hscif",
-		.data = SCI_OF_DATA(PORT_HSCIF, SCIx_HSCIF_REGTYPE),
+		.data = SCI_OF_DATA(PORT_HSCIF, SCIx_HSCIF_REGTYPE, 0),
 	}, {
 		.compatible = "renesas,sci",
-		.data = SCI_OF_DATA(PORT_SCI, SCIx_SCI_REGTYPE),
+		.data = SCI_OF_DATA(PORT_SCI, SCIx_SCI_REGTYPE, 0),
 	}, {
 		/* Terminator */
 	},
@@ -3179,6 +3186,7 @@ static struct plat_sci_port *sci_parse_dt(struct platform_device *pdev,
 
 	p->type = SCI_OF_TYPE(data);
 	p->regtype = SCI_OF_REGTYPE(data);
+	p->regshift = SCI_OF_REGSHIFT(data);
 
 	sp->has_rtscts = of_property_read_bool(np, "uart-has-rtscts");
 
-- 
2.17.1

[PATCH/RFC 4/4] sh-sci: Derive regshift value from DT compatible value

[Geert Uytterhoeven]

Deriving the proper regshift value from the register block size is
fragile (it may have been rounded up in DT, and the mapping granularity
is usually PAGE_SIZE anyway), and turned out to be inappropriate for
earlycon support (the size is not easily available).

On DT systems, derive it from the compatible value instead.
This requires adding an entry for RZ/A2 serial ports, which use an
atypical regshift value.

On non-DT systems the regshift value is still derived from the register
block size, as before.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
  - Sato-san: I assume this fixes SCI on H8/300, too?
    Cfr. your patch "serial: sh-sci: byte allocated register support"
    (https://www.spinics.net/lists/linux-sh/msg53175.html).

  - Getting rid of the regshift setup for non-DT systems probably means
    we'll have to add ".regshift = 1" to each and every SH board file
    describing SCIF serial ports :-(

Any other suggestions?
---
 drivers/tty/serial/sh-sci.c | 32 ++++++++++++++++++++------------
 1 file changed, 20 insertions(+), 12 deletions(-)

diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
index 955c057dff6e8c78..c4342e61b8db72c3 100644
--- a/drivers/tty/serial/sh-sci.c
+++ b/drivers/tty/serial/sh-sci.c
@@ -2898,9 +2898,10 @@ static int sci_init_single(struct platform_device *dev,
 	port->regshift		= p->regshift;
 	port->fifosize		= sci_port->params->fifosize;
 
-	if (regtype == SCIx_SH4_SCIF_REGTYPE)
+	if (!dev->dev.of_node && regtype == SCIx_SH4_SCIF_REGTYPE) {
 		if (sci_port->reg_size >= 0x20)
 			port->regshift = 1;
+	}
 
 	/*
 	 * The UART port needs an IRQ value, so we peg this to the RX IRQ
@@ -3100,43 +3101,49 @@ static int sci_remove(struct platform_device *dev)
 }
 
 
-#define SCI_OF_DATA(type, regtype)	(void *)((type) << 16 | (regtype))
+#define SCI_OF_DATA(type, regtype, regshift)	\
+			(void *)((type) << 16 | (regtype) << 8 | (regshift))
 #define SCI_OF_TYPE(data)		((unsigned long)(data) >> 16)
-#define SCI_OF_REGTYPE(data)		((unsigned long)(data) & 0xffff)
+#define SCI_OF_REGTYPE(data)		(((unsigned long)(data) >> 8) & 0xff)
+#define SCI_OF_REGSHIFT(data)		((unsigned long)(data) & 0xff)
 
 static const struct of_device_id of_sci_match[] = {
 	/* SoC-specific types */
 	{
 		.compatible = "renesas,scif-r7s72100",
-		.data = SCI_OF_DATA(PORT_SCIF, SCIx_SH2_SCIF_FIFODATA_REGTYPE),
+		.data = SCI_OF_DATA(PORT_SCIF, SCIx_SH2_SCIF_FIFODATA_REGTYPE,
+				    0),
+	}, {
+		.compatible = "renesas,scif-r7s9210",
+		.data = SCI_OF_DATA(PORT_SCIF, SCIx_SH4_SCIF_REGTYPE, 0),
 	},
 	/* Family-specific types */
 	{
 		.compatible = "renesas,rcar-gen1-scif",
-		.data = SCI_OF_DATA(PORT_SCIF, SCIx_SH4_SCIF_BRG_REGTYPE),
+		.data = SCI_OF_DATA(PORT_SCIF, SCIx_SH4_SCIF_BRG_REGTYPE, 0),
 	}, {
 		.compatible = "renesas,rcar-gen2-scif",
-		.data = SCI_OF_DATA(PORT_SCIF, SCIx_SH4_SCIF_BRG_REGTYPE),
+		.data = SCI_OF_DATA(PORT_SCIF, SCIx_SH4_SCIF_BRG_REGTYPE, 0),
 	}, {
 		.compatible = "renesas,rcar-gen3-scif",
-		.data = SCI_OF_DATA(PORT_SCIF, SCIx_SH4_SCIF_BRG_REGTYPE),
+		.data = SCI_OF_DATA(PORT_SCIF, SCIx_SH4_SCIF_BRG_REGTYPE, 0),
 	},
 	/* Generic types */
 	{
 		.compatible = "renesas,scif",
-		.data = SCI_OF_DATA(PORT_SCIF, SCIx_SH4_SCIF_REGTYPE),
+		.data = SCI_OF_DATA(PORT_SCIF, SCIx_SH4_SCIF_REGTYPE, 1),
 	}, {
 		.compatible = "renesas,scifa",
-		.data = SCI_OF_DATA(PORT_SCIFA, SCIx_SCIFA_REGTYPE),
+		.data = SCI_OF_DATA(PORT_SCIFA, SCIx_SCIFA_REGTYPE, 0),
 	}, {
 		.compatible = "renesas,scifb",
-		.data = SCI_OF_DATA(PORT_SCIFB, SCIx_SCIFB_REGTYPE),
+		.data = SCI_OF_DATA(PORT_SCIFB, SCIx_SCIFB_REGTYPE, 0),
 	}, {
 		.compatible = "renesas,hscif",
-		.data = SCI_OF_DATA(PORT_HSCIF, SCIx_HSCIF_REGTYPE),
+		.data = SCI_OF_DATA(PORT_HSCIF, SCIx_HSCIF_REGTYPE, 0),
 	}, {
 		.compatible = "renesas,sci",
-		.data = SCI_OF_DATA(PORT_SCI, SCIx_SCI_REGTYPE),
+		.data = SCI_OF_DATA(PORT_SCI, SCIx_SCI_REGTYPE, 0),
 	}, {
 		/* Terminator */
 	},
@@ -3179,6 +3186,7 @@ static struct plat_sci_port *sci_parse_dt(struct platform_device *pdev,
 
 	p->type = SCI_OF_TYPE(data);
 	p->regtype = SCI_OF_REGTYPE(data);
+	p->regshift = SCI_OF_REGSHIFT(data);
 
 	sp->has_rtscts = of_property_read_bool(np, "uart-has-rtscts");
 
-- 
2.17.1

RE: [PATCH/RFC 4/4] sh-sci: Derive regshift value from DT compatible value

[Chris Brandt]

Hi Geert,

On Monday, August 06, 2018 1, linux-sh-owner@vger.kernel.org wrote:
> Deriving the proper regshift value from the register block size is
> fragile (it may have been rounded up in DT, and the mapping granularity
> is usually PAGE_SIZE anyway), and turned out to be inappropriate for
> earlycon support (the size is not easily available).
> 
> On DT systems, derive it from the compatible value instead.
> This requires adding an entry for RZ/A2 serial ports, which use an
> atypical regshift value.

I had a simple patch to add support for CONFIG_DEBUG_LL for RZ/A2 
because earlycon never worked because of RZ/A2's different register locations.

I'll have to see how things change (better?) with this patch.


Chris

RE: [PATCH/RFC 4/4] sh-sci: Derive regshift value from DT compatible value

[Chris Brandt]

Hi Geert,

On Monday, August 06, 2018 1, linux-sh-owner@vger.kernel.org wrote:
> Deriving the proper regshift value from the register block size is
> fragile (it may have been rounded up in DT, and the mapping granularity
> is usually PAGE_SIZE anyway), and turned out to be inappropriate for
> earlycon support (the size is not easily available).
> 
> On DT systems, derive it from the compatible value instead.
> This requires adding an entry for RZ/A2 serial ports, which use an
> atypical regshift value.

I had a simple patch to add support for CONFIG_DEBUG_LL for RZ/A2 
because earlycon never worked because of RZ/A2's different register locations.

I'll have to see how things change (better?) with this patch.


Chris

Re: [PATCH/RFC 4/4] sh-sci: Derive regshift value from DT compatible value

[Geert Uytterhoeven]

Hi Chris,

On Mon, Aug 6, 2018 at 4:18 PM Chris Brandt <Chris.Brandt@renesas.com> wrote:
> On Monday, August 06, 2018 1, linux-sh-owner@vger.kernel.org wrote:
> > Deriving the proper regshift value from the register block size is
> > fragile (it may have been rounded up in DT, and the mapping granularity
> > is usually PAGE_SIZE anyway), and turned out to be inappropriate for
> > earlycon support (the size is not easily available).
> >
> > On DT systems, derive it from the compatible value instead.
> > This requires adding an entry for RZ/A2 serial ports, which use an
> > atypical regshift value.
>
> I had a simple patch to add support for CONFIG_DEBUG_LL for RZ/A2
> because earlycon never worked because of RZ/A2's different register locations.

Yeah, sci_probe_regmap() assumed the wrong regtype for your TYPE_SCIF
port. You needed an OF_EARLYCON_DECLARE() line that also filled in
the correct regtype.

BTW, it would have been very valuable to know that earlycon didn't work, as that
would have helped in avoiding the earlycon breakage on other parts.

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/RFC 4/4] sh-sci: Derive regshift value from DT compatible value

[Geert Uytterhoeven]

Hi Chris,

On Mon, Aug 6, 2018 at 4:18 PM Chris Brandt <Chris.Brandt@renesas.com> wrote:
> On Monday, August 06, 2018 1, linux-sh-owner@vger.kernel.org wrote:
> > Deriving the proper regshift value from the register block size is
> > fragile (it may have been rounded up in DT, and the mapping granularity
> > is usually PAGE_SIZE anyway), and turned out to be inappropriate for
> > earlycon support (the size is not easily available).
> >
> > On DT systems, derive it from the compatible value instead.
> > This requires adding an entry for RZ/A2 serial ports, which use an
> > atypical regshift value.
>
> I had a simple patch to add support for CONFIG_DEBUG_LL for RZ/A2
> because earlycon never worked because of RZ/A2's different register locations.

Yeah, sci_probe_regmap() assumed the wrong regtype for your TYPE_SCIF
port. You needed an OF_EARLYCON_DECLARE() line that also filled in
the correct regtype.

BTW, it would have been very valuable to know that earlycon didn't work, as that
would have helped in avoiding the earlycon breakage on other parts.

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/RFC 4/4] sh-sci: Derive regshift value from DT compatible value

[Chris Brandt]

SGkgR2VlcnQsDQoNCk9uIE1vbmRheSwgQXVndXN0IDA2LCAyMDE4IDEsIEdlZXJ0IFV5dHRlcmhv
ZXZlbiB3cm90ZToNCj4gQlRXLCBpdCB3b3VsZCBoYXZlIGJlZW4gdmVyeSB2YWx1YWJsZSB0byBr
bm93IHRoYXQgZWFybHljb24gZGlkbid0IHdvcmssDQo+IGFzIHRoYXQNCj4gd291bGQgaGF2ZSBo
ZWxwZWQgaW4gYXZvaWRpbmcgdGhlIGVhcmx5Y29uIGJyZWFrYWdlIG9uIG90aGVyIHBhcnRzLg0K
DQpNeSBhcG9sb2dpZXMuDQoNCldoZW4gSSBoYWQgZmlyc3QgbG9va2VkIGF0IHRoaXMgbW9udGhz
IGFnbywgSSBxdWlja2x5IHJlYWxpemVkIHRoYXQgDQplYXJseWNvbiB3YXMgbm90IGdvaW5nIHRv
IHdvcmsgYmVjYXVzZSB0aGUgUlovQTIgU0NJRiB3YXMgbm90IGxpa2UgYWxsIHRoZSANCm90aGVy
IFNDSUYgZGV2aWNlcy4NClNvIGluc3RlYWQgSSB3ZW50IHdpdGggYSBzaW1wbGUgMiBsaW5lIGNo
YW5nZSB0byBERUJVR19MTC4gQWZ0ZXIgdGhhdCANCnBvaW50LCBJIGZvcmdvdCBhbGwgYWJvdXQg
ZWFybHljb24gdW50aWwgSSBzYXcgeW91IG1lbnRpb24gaXQgdGhpcyBtb3JuaW5nLg0KDQoNCkNo
cmlzDQoNCg=

RE: [PATCH/RFC 4/4] sh-sci: Derive regshift value from DT compatible value

[Chris Brandt]

Hi Geert,

On Monday, August 06, 2018 1, Geert Uytterhoeven wrote:
> BTW, it would have been very valuable to know that earlycon didn't work,
> as that
> would have helped in avoiding the earlycon breakage on other parts.

My apologies.

When I had first looked at this months ago, I quickly realized that 
earlycon was not going to work because the RZ/A2 SCIF was not like all the 
other SCIF devices.
So instead I went with a simple 2 line change to DEBUG_LL. After that 
point, I forgot all about earlycon until I saw you mention it this morning.


Chris

RE: [PATCH/RFC 4/4] sh-sci: Derive regshift value from DT compatible value

[Chris Brandt]

SGkgR2VlcnQsDQoNCk9uIE1vbmRheSwgQXVndXN0IDA2LCAyMDE4IDEsIEdlZXJ0IFV5dHRlcmhv
ZXZlbiB3cm90ZToNCj4gPiBJIGhhZCBhIHNpbXBsZSBwYXRjaCB0byBhZGQgc3VwcG9ydCBmb3Ig
Q09ORklHX0RFQlVHX0xMIGZvciBSWi9BMg0KPiA+IGJlY2F1c2UgZWFybHljb24gbmV2ZXIgd29y
a2VkIGJlY2F1c2Ugb2YgUlovQTIncyBkaWZmZXJlbnQgcmVnaXN0ZXINCj4gbG9jYXRpb25zLg0K
PiANCj4gWWVhaCwgc2NpX3Byb2JlX3JlZ21hcCgpIGFzc3VtZWQgdGhlIHdyb25nIHJlZ3R5cGUg
Zm9yIHlvdXIgVFlQRV9TQ0lGDQo+IHBvcnQuIFlvdSBuZWVkZWQgYW4gT0ZfRUFSTFlDT05fREVD
TEFSRSgpIGxpbmUgdGhhdCBhbHNvIGZpbGxlZCBpbg0KPiB0aGUgY29ycmVjdCByZWd0eXBlLg0K
DQoNCkkgZ2F2ZSB5b3VyIHBhdGNoIGEgdHJ5Lg0KV2hlbiBlYXJseWNvbiBpcyBlbmFibGVkLCBv
biBSWi9BMiwgaXQgZ2V0cyBzdHVjayBpbiBoZXJlOg0KDQpzdGF0aWMgdm9pZCBzY2lfcG9sbF9w
dXRfY2hhcihzdHJ1Y3QgdWFydF9wb3J0ICpwb3J0LCB1bnNpZ25lZCBjaGFyIGMpDQp7DQoJdW5z
aWduZWQgc2hvcnQgc3RhdHVzOw0KDQoJZG8gew0KCQlzdGF0dXMgPSBzZXJpYWxfcG9ydF9pbihw
b3J0LCBTQ3hTUik7DQoJfSB3aGlsZSAoIShzdGF0dXMgJiBTQ3hTUl9URHhFKHBvcnQpKSk7DQoN
CglzZXJpYWxfcG9ydF9vdXQocG9ydCwgU0N4VERSLCBjKTsNCglzY2lfY2xlYXJfU0N4U1IocG9y
dCwgU0N4U1JfVER4RV9DTEVBUihwb3J0KSAmIH5TQ3hTUl9URU5EKHBvcnQpKTsNCn0NCg0KDQpJ
IHNlZSB0aGF0IHlvdSBhZGRlZCB0aGlzOg0KDQpPRl9FQVJMWUNPTl9ERUNMQVJFKHNjaWYsICJy
ZW5lc2FzLHNjaWYtcjdzOTIxMCIsIHJ6YTJfZWFybHlfY29uc29sZV9zZXR1cCk7DQoNCiBhbmQg
InJlbmVzYXMsc2NpZi1yN3M5MjEwIiBtYXRjaGVzIHdoYXQgSSBoYXZlIGluIG15IC5kdHNpLg0K
DQpCdXQsIHdoZW4gSSBydW4gdGhlIGNvZGUsIEkgZW5kIHVwIGluIGZ1bmN0aW9uIHNjaWZfZWFy
bHlfY29uc29sZV9zZXR1cCwNCm5vdCByemEyX2Vhcmx5X2NvbnNvbGVfc2V0dXANCg0KSSdtIGFz
c3VtaW5nIEknbSBqdXN0IHN1cHBvc2VkIHRvIHVzZSB0aGlzIG9uIG15IGJvb3RhcmdzOg0KICAg
ZWFybHljb249c2NpZiwweEU4MDA5MDAwDQoNCg0KQ2hyaXMNCg0K

RE: [PATCH/RFC 4/4] sh-sci: Derive regshift value from DT compatible value

[Chris Brandt]

Hi Geert,

On Monday, August 06, 2018 1, Geert Uytterhoeven wrote:
> > I had a simple patch to add support for CONFIG_DEBUG_LL for RZ/A2
> > because earlycon never worked because of RZ/A2's different register
> locations.
> 
> Yeah, sci_probe_regmap() assumed the wrong regtype for your TYPE_SCIF
> port. You needed an OF_EARLYCON_DECLARE() line that also filled in
> the correct regtype.


I gave your patch a try.
When earlycon is enabled, on RZ/A2, it gets stuck in here:

static void sci_poll_put_char(struct uart_port *port, unsigned char c)
{
	unsigned short status;

	do {
		status = serial_port_in(port, SCxSR);
	} while (!(status & SCxSR_TDxE(port)));

	serial_port_out(port, SCxTDR, c);
	sci_clear_SCxSR(port, SCxSR_TDxE_CLEAR(port) & ~SCxSR_TEND(port));
}


I see that you added this:

OF_EARLYCON_DECLARE(scif, "renesas,scif-r7s9210", rza2_early_console_setup);

 and "renesas,scif-r7s9210" matches what I have in my .dtsi.

But, when I run the code, I end up in function scif_early_console_setup,
not rza2_early_console_setup

I'm assuming I'm just supposed to use this on my bootargs:
   earlycon=scif,0xE8009000


Chris

Re: [PATCH/RFC 4/4] sh-sci: Derive regshift value from DT compatible value

[Geert Uytterhoeven]

Hi Chris,

On Tue, Aug 7, 2018 at 9:24 PM Chris Brandt <Chris.Brandt@renesas.com> wrote:
> On Monday, August 06, 2018 1, Geert Uytterhoeven wrote:
> > > I had a simple patch to add support for CONFIG_DEBUG_LL for RZ/A2
> > > because earlycon never worked because of RZ/A2's different register
> > locations.
> >
> > Yeah, sci_probe_regmap() assumed the wrong regtype for your TYPE_SCIF
> > port. You needed an OF_EARLYCON_DECLARE() line that also filled in
> > the correct regtype.
>
>
> I gave your patch a try.
> When earlycon is enabled, on RZ/A2, it gets stuck in here:
>
> static void sci_poll_put_char(struct uart_port *port, unsigned char c)
> {
>         unsigned short status;
>
>         do {
>                 status = serial_port_in(port, SCxSR);
>         } while (!(status & SCxSR_TDxE(port)));
>
>         serial_port_out(port, SCxTDR, c);
>         sci_clear_SCxSR(port, SCxSR_TDxE_CLEAR(port) & ~SCxSR_TEND(port));
> }
>
>
> I see that you added this:
>
> OF_EARLYCON_DECLARE(scif, "renesas,scif-r7s9210", rza2_early_console_setup);
>
>  and "renesas,scif-r7s9210" matches what I have in my .dtsi.
>
> But, when I run the code, I end up in function scif_early_console_setup,
> not rza2_early_console_setup

Hmm, perhaps it doesn't pick the first/most specialized match.

> I'm assuming I'm just supposed to use this on my bootargs:
>    earlycon=scif,0xE8009000

Just "earlycon" should be sufficient. It'll find the right port from
chosen/stdout-path
in DT. Can you please retry using that?

Thanks!

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/RFC 4/4] sh-sci: Derive regshift value from DT compatible value

[Geert Uytterhoeven]

Hi Chris,

On Tue, Aug 7, 2018 at 9:24 PM Chris Brandt <Chris.Brandt@renesas.com> wrote:
> On Monday, August 06, 2018 1, Geert Uytterhoeven wrote:
> > > I had a simple patch to add support for CONFIG_DEBUG_LL for RZ/A2
> > > because earlycon never worked because of RZ/A2's different register
> > locations.
> >
> > Yeah, sci_probe_regmap() assumed the wrong regtype for your TYPE_SCIF
> > port. You needed an OF_EARLYCON_DECLARE() line that also filled in
> > the correct regtype.
>
>
> I gave your patch a try.
> When earlycon is enabled, on RZ/A2, it gets stuck in here:
>
> static void sci_poll_put_char(struct uart_port *port, unsigned char c)
> {
>         unsigned short status;
>
>         do {
>                 status = serial_port_in(port, SCxSR);
>         } while (!(status & SCxSR_TDxE(port)));
>
>         serial_port_out(port, SCxTDR, c);
>         sci_clear_SCxSR(port, SCxSR_TDxE_CLEAR(port) & ~SCxSR_TEND(port));
> }
>
>
> I see that you added this:
>
> OF_EARLYCON_DECLARE(scif, "renesas,scif-r7s9210", rza2_early_console_setup);
>
>  and "renesas,scif-r7s9210" matches what I have in my .dtsi.
>
> But, when I run the code, I end up in function scif_early_console_setup,
> not rza2_early_console_setup

Hmm, perhaps it doesn't pick the first/most specialized match.

> I'm assuming I'm just supposed to use this on my bootargs:
>    earlycon=scif,0xE8009000

Just "earlycon" should be sufficient. It'll find the right port from
chosen/stdout-path
in DT. Can you please retry using that?

Thanks!

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/RFC 4/4] sh-sci: Derive regshift value from DT compatible value

[Chris Brandt]

SGkgR2VlcnQsDQoNCk9uIFR1ZXNkYXksIEF1Z3VzdCAwNywgMjAxOCwgR2VlcnQgVXl0dGVyaG9l
dmVuIHdyb3RlOg0KPiA+IEJ1dCwgd2hlbiBJIHJ1biB0aGUgY29kZSwgSSBlbmQgdXAgaW4gZnVu
Y3Rpb24gc2NpZl9lYXJseV9jb25zb2xlX3NldHVwLA0KPiA+IG5vdCByemEyX2Vhcmx5X2NvbnNv
bGVfc2V0dXANCj4gDQo+IEhtbSwgcGVyaGFwcyBpdCBkb2Vzbid0IHBpY2sgdGhlIGZpcnN0L21v
c3Qgc3BlY2lhbGl6ZWQgbWF0Y2guDQo+IA0KPiA+IEknbSBhc3N1bWluZyBJJ20ganVzdCBzdXBw
b3NlZCB0byB1c2UgdGhpcyBvbiBteSBib290YXJnczoNCj4gPiAgICBlYXJseWNvbj1zY2lmLDB4
RTgwMDkwMDANCj4gDQo+IEp1c3QgImVhcmx5Y29uIiBzaG91bGQgYmUgc3VmZmljaWVudC4gSXQn
bGwgZmluZCB0aGUgcmlnaHQgcG9ydCBmcm9tDQo+IGNob3Nlbi9zdGRvdXQtcGF0aA0KPiBpbiBE
VC4gQ2FuIHlvdSBwbGVhc2UgcmV0cnkgdXNpbmcgdGhhdD8NCg0KV2l0aCBqdXN0IHVzaW5nICJl
YXJseWNvbiIsIEkgZ2V0IHRoZSBzYW1lIHJlc3VsdC4NCg0KSSB0aGluayB0aGUgY29kZSB0aGF0
IGRvZXMgdGhlIG1hdGNoaW5nIGlzIGluIGRyaXZlcnMvb2YvZmR0LmMNCkZ1bmN0aW9uICJlYXJs
eV9pbml0X2R0X3NjYW5fY2hvc2VuX3N0ZG91dCIuDQoNCkknbGwgc3RlcCB0aHJvdWdoIHRoZSBj
b2RlIGEgYml0IHRvIHNlZSBpZiBJIGNhbiBmaWd1cmUgb3V0IHdoeS4NCg0KQ2hyaXMNCg0KDQo

RE: [PATCH/RFC 4/4] sh-sci: Derive regshift value from DT compatible value

[Chris Brandt]

Hi Geert,

On Tuesday, August 07, 2018, Geert Uytterhoeven wrote:
> > But, when I run the code, I end up in function scif_early_console_setup,
> > not rza2_early_console_setup
> 
> Hmm, perhaps it doesn't pick the first/most specialized match.
> 
> > I'm assuming I'm just supposed to use this on my bootargs:
> >    earlycon=scif,0xE8009000
> 
> Just "earlycon" should be sufficient. It'll find the right port from
> chosen/stdout-path
> in DT. Can you please retry using that?

With just using "earlycon", I get the same result.

I think the code that does the matching is in drivers/of/fdt.c
Function "early_init_dt_scan_chosen_stdout".

I'll step through the code a bit to see if I can figure out why.

Chris

RE: [PATCH/RFC 4/4] sh-sci: Derive regshift value from DT compatible value

[Chris Brandt]

SGkgR2VlcnQsDQoNCk9uIFR1ZXNkYXksIEF1Z3VzdCAwNywgMjAxOCwgR2VlcnQgVXl0dGVyaG9l
dmVuIHdyb3RlOg0KPiA+IEkgc2VlIHRoYXQgeW91IGFkZGVkIHRoaXM6DQo+ID4NCj4gPiBPRl9F
QVJMWUNPTl9ERUNMQVJFKHNjaWYsICJyZW5lc2FzLHNjaWYtcjdzOTIxMCIsDQo+IHJ6YTJfZWFy
bHlfY29uc29sZV9zZXR1cCk7DQo+ID4NCj4gPiAgYW5kICJyZW5lc2FzLHNjaWYtcjdzOTIxMCIg
bWF0Y2hlcyB3aGF0IEkgaGF2ZSBpbiBteSAuZHRzaS4NCj4gPg0KPiA+IEJ1dCwgd2hlbiBJIHJ1
biB0aGUgY29kZSwgSSBlbmQgdXAgaW4gZnVuY3Rpb24gc2NpZl9lYXJseV9jb25zb2xlX3NldHVw
LA0KPiA+IG5vdCByemEyX2Vhcmx5X2NvbnNvbGVfc2V0dXANCj4gDQo+IEhtbSwgcGVyaGFwcyBp
dCBkb2Vzbid0IHBpY2sgdGhlIGZpcnN0L21vc3Qgc3BlY2lhbGl6ZWQgbWF0Y2guDQoNCkl0IHNl
ZW1zIGl0IHBpY2tzIHRoZSBmaXJzdCBtYXRjaC4NCg0KQnV0LCB0aGUgdGFibGUgaXMgYnVpbHQg
aW4gdGhlIG9wcG9zaXRlIG9yZGVyIGluIHdoaWNoIHRoZXkgYXJlIGRlY2xhcmVkDQppbiB0aGUg
ZmlsZS4gU28gInJlbmVzYXMsc2NpZiIgd2FzIGNvbWluZyBiZWZvcmUgDQoicmVuZXNhcyxzY2lm
LXI3czkyMTAiLg0KDQpTbywgYnkganVzdCBzd2FwcGluZyB0aGUgb3JkZXIgb2YgInJlbmVzYXMs
c2NpZiIgYW5kIA0KInJlbmVzYXMsc2NpZi1yN3M5MjEwIiBtYWRlIGl0IHdvcmshDQoNCg0KLS0t
IGEvZHJpdmVycy90dHkvc2VyaWFsL3NoLXNjaS5jDQorKysgYi9kcml2ZXJzL3R0eS9zZXJpYWwv
c2gtc2NpLmMNCkBAIC0zNDIzLDggKzM0MjMsOCBAQCBzdGF0aWMgaW50IF9faW5pdCBoc2NpZl9l
YXJseV9jb25zb2xlX3NldHVwKHN0cnVjdCBlYXJseWNvbl9kZXZpY2UgKmRldmljZSwNCiB9DQog
DQogT0ZfRUFSTFlDT05fREVDTEFSRShzY2ksICJyZW5lc2FzLHNjaSIsIHNjaV9lYXJseV9jb25z
b2xlX3NldHVwKTsNCi1PRl9FQVJMWUNPTl9ERUNMQVJFKHNjaWYsICJyZW5lc2FzLHNjaWYtcjdz
OTIxMCIsIHJ6YTJfZWFybHlfY29uc29sZV9zZXR1cCk7DQogT0ZfRUFSTFlDT05fREVDTEFSRShz
Y2lmLCAicmVuZXNhcyxzY2lmIiwgc2NpZl9lYXJseV9jb25zb2xlX3NldHVwKTsNCitPRl9FQVJM
WUNPTl9ERUNMQVJFKHNjaWYsICJyZW5lc2FzLHNjaWYtcjdzOTIxMCIsIHJ6YTJfZWFybHlfY29u
c29sZV9zZXR1cCk7DQogT0ZfRUFSTFlDT05fREVDTEFSRShzY2lmYSwgInJlbmVzYXMsc2NpZmEi
LCBzY2lmYV9lYXJseV9jb25zb2xlX3NldHVwKTsNCiBPRl9FQVJMWUNPTl9ERUNMQVJFKHNjaWZi
LCAicmVuZXNhcyxzY2lmYiIsIHNjaWZiX2Vhcmx5X2NvbnNvbGVfc2V0dXApOw0KIE9GX0VBUkxZ
Q09OX0RFQ0xBUkUoaHNjaWYsICJyZW5lc2FzLGhzY2lmIiwgaHNjaWZfZWFybHlfY29uc29sZV9z
ZXR1cCk7DQoNCg0KICAtLSBsb2cgLS0NCkJvb3RpbmcgTGludXguLi4NClsgICAgMC4wMDAwMDBd
IEJvb3RpbmcgTGludXggb24gcGh5c2ljYWwgQ1BVIDB4MA0KWyAgICAwLjAwMDAwMF0gTGludXgg
dmVyc2lvbiA0LjE4LjAtcmM3LTAwMDE3LWc3MGNkYjRmMjQzYzYtZGlydHkgKGNocmlzQHVidW50
dSkgKGdjYyB2ZXJzaW9uIDUuNC4xIDIwMTYxMjEzIChMaW5hcm8gR0NDIDUuNC0yMDE3LjAxKSkg
IzIzIFR1ZSBBdWcgNyAxOTowNDoyNSBFU1QgMjAxOA0KWyAgICAwLjAwMDAwMF0gQ1BVOiBBUk12
NyBQcm9jZXNzb3IgWzQxNGZjMDkxXSByZXZpc2lvbiAxIChBUk12NyksIGNyPTUwYzUzYzdkDQpb
ICAgIDAuMDAwMDAwXSBDUFU6IFBJUFQgLyBWSVBUIG5vbmFsaWFzaW5nIGRhdGEgY2FjaGUsIFZJ
UFQgYWxpYXNpbmcgaW5zdHJ1Y3Rpb24gY2FjaGUNClsgICAgMC4wMDAwMDBdIE9GOiBmZHQ6IE1h
Y2hpbmUgbW9kZWw6IFJaQTJNRVZCDQpbICAgIDAuMDAwMDAwXSBkZWJ1ZzogaWdub3JpbmcgbG9n
bGV2ZWwgc2V0dGluZy4NClsgICAgMC4wMDAwMDBdIGVhcmx5Y29uOiBzY2lmMCBhdCBNTUlPIDB4
ZTgwMDkwMDAgKG9wdGlvbnMgJzExNTIwMG44JykNClsgICAgMC4wMDAwMDBdIGJvb3Rjb25zb2xl
IFtzY2lmMjU2XSBlbmFibGVkDQoNCg0KDQpDaHJpcw0KDQo

RE: [PATCH/RFC 4/4] sh-sci: Derive regshift value from DT compatible value

[Chris Brandt]

Hi Geert,

On Tuesday, August 07, 2018, Geert Uytterhoeven wrote:
> > I see that you added this:
> >
> > OF_EARLYCON_DECLARE(scif, "renesas,scif-r7s9210",
> rza2_early_console_setup);
> >
> >  and "renesas,scif-r7s9210" matches what I have in my .dtsi.
> >
> > But, when I run the code, I end up in function scif_early_console_setup,
> > not rza2_early_console_setup
> 
> Hmm, perhaps it doesn't pick the first/most specialized match.

It seems it picks the first match.

But, the table is built in the opposite order in which they are declared
in the file. So "renesas,scif" was coming before 
"renesas,scif-r7s9210".

So, by just swapping the order of "renesas,scif" and 
"renesas,scif-r7s9210" made it work!


--- a/drivers/tty/serial/sh-sci.c
+++ b/drivers/tty/serial/sh-sci.c
@@ -3423,8 +3423,8 @@ static int __init hscif_early_console_setup(struct earlycon_device *device,
 }
 
 OF_EARLYCON_DECLARE(sci, "renesas,sci", sci_early_console_setup);
-OF_EARLYCON_DECLARE(scif, "renesas,scif-r7s9210", rza2_early_console_setup);
 OF_EARLYCON_DECLARE(scif, "renesas,scif", scif_early_console_setup);
+OF_EARLYCON_DECLARE(scif, "renesas,scif-r7s9210", rza2_early_console_setup);
 OF_EARLYCON_DECLARE(scifa, "renesas,scifa", scifa_early_console_setup);
 OF_EARLYCON_DECLARE(scifb, "renesas,scifb", scifb_early_console_setup);
 OF_EARLYCON_DECLARE(hscif, "renesas,hscif", hscif_early_console_setup);


  -- log --
Booting Linux...
[    0.000000] Booting Linux on physical CPU 0x0
[    0.000000] Linux version 4.18.0-rc7-00017-g70cdb4f243c6-dirty (chris@ubuntu) (gcc version 5.4.1 20161213 (Linaro GCC 5.4-2017.01)) #23 Tue Aug 7 19:04:25 EST 2018
[    0.000000] CPU: ARMv7 Processor [414fc091] revision 1 (ARMv7), cr=50c53c7d
[    0.000000] CPU: PIPT / VIPT nonaliasing data cache, VIPT aliasing instruction cache
[    0.000000] OF: fdt: Machine model: RZA2MEVB
[    0.000000] debug: ignoring loglevel setting.
[    0.000000] earlycon: scif0 at MMIO 0xe8009000 (options '115200n8')
[    0.000000] bootconsole [scif256] enabled



Chris

Re: [PATCH/RFC 4/4] sh-sci: Derive regshift value from DT compatible value

[Geert Uytterhoeven]

Hi Chris,

On Wed, Aug 8, 2018 at 2:16 AM Chris Brandt <Chris.Brandt@renesas.com> wrote:
> On Tuesday, August 07, 2018, Geert Uytterhoeven wrote:
> > > I see that you added this:
> > >
> > > OF_EARLYCON_DECLARE(scif, "renesas,scif-r7s9210",
> > rza2_early_console_setup);
> > >
> > >  and "renesas,scif-r7s9210" matches what I have in my .dtsi.
> > >
> > > But, when I run the code, I end up in function scif_early_console_setup,
> > > not rza2_early_console_setup
> >
> > Hmm, perhaps it doesn't pick the first/most specialized match.
>
> It seems it picks the first match.
>
> But, the table is built in the opposite order in which they are declared
> in the file. So "renesas,scif" was coming before
> "renesas,scif-r7s9210".
>
> So, by just swapping the order of "renesas,scif" and
> "renesas,scif-r7s9210" made it work!

Right, and since the RZ/A2 SCIF is not really compatible with
"renesas,scif" (see my conclusion as a reply to the cover letter), the
other order doesn't work.
BTW, I guess earlycon also works on RZ/A2 with current tty-next or
latest renesas-drivers?

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/RFC 4/4] sh-sci: Derive regshift value from DT compatible value

[Geert Uytterhoeven]

Hi Chris,

On Wed, Aug 8, 2018 at 2:16 AM Chris Brandt <Chris.Brandt@renesas.com> wrote:
> On Tuesday, August 07, 2018, Geert Uytterhoeven wrote:
> > > I see that you added this:
> > >
> > > OF_EARLYCON_DECLARE(scif, "renesas,scif-r7s9210",
> > rza2_early_console_setup);
> > >
> > >  and "renesas,scif-r7s9210" matches what I have in my .dtsi.
> > >
> > > But, when I run the code, I end up in function scif_early_console_setup,
> > > not rza2_early_console_setup
> >
> > Hmm, perhaps it doesn't pick the first/most specialized match.
>
> It seems it picks the first match.
>
> But, the table is built in the opposite order in which they are declared
> in the file. So "renesas,scif" was coming before
> "renesas,scif-r7s9210".
>
> So, by just swapping the order of "renesas,scif" and
> "renesas,scif-r7s9210" made it work!

Right, and since the RZ/A2 SCIF is not really compatible with
"renesas,scif" (see my conclusion as a reply to the cover letter), the
other order doesn't work.
BTW, I guess earlycon also works on RZ/A2 with current tty-next or
latest renesas-drivers?

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/RFC 4/4] sh-sci: Derive regshift value from DT compatible value

[Chris Brandt]

SGkgR2VlcnQsDQoNCk9uIFdlZG5lc2RheSwgQXVndXN0IDA4LCAyMDE4LCBHZWVydCBVeXR0ZXJo
b2V2ZW4gd3JvdGU6DQo+IEJUVywgSSBndWVzcyBlYXJseWNvbiBhbHNvIHdvcmtzIG9uIFJaL0Ey
IHdpdGggY3VycmVudCB0dHktbmV4dCBvcg0KPiBsYXRlc3QgcmVuZXNhcy1kcml2ZXJzPw0KDQpJ
IHdhcyB1c2luZyB5b3VyIFJGQyBwYXRjaGVzIG9uIHRvcCBvZiB0aGUgY3VycmVudCByZW5lc2Fz
LWRyaXZlcnMuDQoNCkNocmlzDQo

RE: [PATCH/RFC 4/4] sh-sci: Derive regshift value from DT compatible value

[Chris Brandt]

Hi Geert,

On Wednesday, August 08, 2018, Geert Uytterhoeven wrote:
> BTW, I guess earlycon also works on RZ/A2 with current tty-next or
> latest renesas-drivers?

I was using your RFC patches on top of the current renesas-drivers.

Chris

Re: [PATCH/RFC 4/4] sh-sci: Derive regshift value from DT compatible value

[Geert Uytterhoeven]

Hi Chris,

On Wed, Aug 8, 2018 at 12:39 PM Chris Brandt <Chris.Brandt@renesas.com> wrote:
> On Wednesday, August 08, 2018, Geert Uytterhoeven wrote:
> > BTW, I guess earlycon also works on RZ/A2 with current tty-next or
> > latest renesas-drivers?
>
> I was using your RFC patches on top of the current renesas-drivers.

I meant without the RFC patch series.

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/RFC 4/4] sh-sci: Derive regshift value from DT compatible value

[Geert Uytterhoeven]

Hi Chris,

On Wed, Aug 8, 2018 at 12:39 PM Chris Brandt <Chris.Brandt@renesas.com> wrote:
> On Wednesday, August 08, 2018, Geert Uytterhoeven wrote:
> > BTW, I guess earlycon also works on RZ/A2 with current tty-next or
> > latest renesas-drivers?
>
> I was using your RFC patches on top of the current renesas-drivers.

I meant without the RFC patch series.

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/RFC 0/4] sh-sci : Do not derive regshift from regsize

[Laurent Pinchart]

Hi Geert,

On Monday, 6 August 2018 17:07:51 EEST Geert Uytterhoeven wrote:
> 	Hi all,
> 
> This RFC patch series was sparked by noticing that commit 2d4dd0da45401c7a

Where can that commit be found ?

> ("serial: sh-sci: Allow for compressed SCIF address") broke earlycon
> support on most Renesas ARM SoCs using SCIF ports, and by the fragility of
> deriving regshift from the register block size (which may be rounded up):

Why should it be rounded up ?

>   1. The first patch is an old patch from Sato-san, which I never really
>      understood.  But it turned out to be a dependency for patch 2.
>   2. Patch 2 makes sure regshift is initialized when using earlycon,
>      unbreaking the serial console on e.g. R-Car Gen2 and Gen3.
>   3. Patch 3 reverts the patch that started deriving regshift from the
>      register block size, and that removed the plat_sci_port.regshift
>      field.  Which is a field I needed again in patch 4.
>   4. Patch 4 removes the remaining regshift derivations on DT platforms.
>  (5. I didn't bother writing patch 5, which involves adding .regshift
>      initializations to all SH board files that need it.)
> 
> However, I'm not happy with the end result, so please DO NOT apply this!
> As I spent almost a full day on this, and would still like to know the
> story about "sh-sci: Use a separate sci_port for earlycon", I decided to
> post it anyway.
> 
> As earlycon will be broken in v4.19-rc1 on RZ/A1, RZ/G, and R-Car, assuming
> no other actions are taken, an alternative solution would be to:
>   1. Revert commit 7acece71a517cad8 ("serial: sh-sci: Remove
>      SCIx_RZ_SCIFA_REGTYPE"),
>   2. Revert commit 2d4dd0da45401c7a ("serial: sh-sci: Allow for compressed
>      SCIF address") alternative,
>   3. Add an OF_EARLYCON_DECLARE() for RZ/A2, to fix earlycon on RZ/A2.
> 
> What do you think?
> Thanks for your comments!
> 
> P.S. Apparently SCIx_SH4_SCIF_REGTYPE and SCIx_SH2_SCIF_FIFODATA_REGTYPE
>      are identical?
> 
> Geert Uytterhoeven (3):
>   [RFC] sh-sci: Take into account regshift to fix earlycon breakage
>   [RFC] Revert "serial: sh-sci: Compute the regshift value for SCI
>     ports"
>   [RFC] sh-sci: Derive regshift value from DT compatible value
> 
> Yoshinori Sato (1):
>   [RFC] sh-sci: Use a separate sci_port for earlycon
> 
>  arch/sh/kernel/cpu/sh3/setup-sh770x.c |  1 +
>  arch/sh/kernel/cpu/sh4/setup-sh7750.c |  3 +-
>  arch/sh/kernel/cpu/sh4/setup-sh7760.c | 10 +---
>  drivers/tty/serial/sh-sci.c           | 68 +++++++++++++++++----------
>  include/linux/serial_sci.h            |  1 +
>  5 files changed, 49 insertions(+), 34 deletions(-)

-- 
Regards,

Laurent Pinchart

Re: [PATCH/RFC 0/4] sh-sci : Do not derive regshift from regsize

[Laurent Pinchart]

Hi Geert,

On Monday, 6 August 2018 17:07:51 EEST Geert Uytterhoeven wrote:
> 	Hi all,
> 
> This RFC patch series was sparked by noticing that commit 2d4dd0da45401c7a

Where can that commit be found ?

> ("serial: sh-sci: Allow for compressed SCIF address") broke earlycon
> support on most Renesas ARM SoCs using SCIF ports, and by the fragility of
> deriving regshift from the register block size (which may be rounded up):

Why should it be rounded up ?

>   1. The first patch is an old patch from Sato-san, which I never really
>      understood.  But it turned out to be a dependency for patch 2.
>   2. Patch 2 makes sure regshift is initialized when using earlycon,
>      unbreaking the serial console on e.g. R-Car Gen2 and Gen3.
>   3. Patch 3 reverts the patch that started deriving regshift from the
>      register block size, and that removed the plat_sci_port.regshift
>      field.  Which is a field I needed again in patch 4.
>   4. Patch 4 removes the remaining regshift derivations on DT platforms.
>  (5. I didn't bother writing patch 5, which involves adding .regshift
>      initializations to all SH board files that need it.)
> 
> However, I'm not happy with the end result, so please DO NOT apply this!
> As I spent almost a full day on this, and would still like to know the
> story about "sh-sci: Use a separate sci_port for earlycon", I decided to
> post it anyway.
> 
> As earlycon will be broken in v4.19-rc1 on RZ/A1, RZ/G, and R-Car, assuming
> no other actions are taken, an alternative solution would be to:
>   1. Revert commit 7acece71a517cad8 ("serial: sh-sci: Remove
>      SCIx_RZ_SCIFA_REGTYPE"),
>   2. Revert commit 2d4dd0da45401c7a ("serial: sh-sci: Allow for compressed
>      SCIF address") alternative,
>   3. Add an OF_EARLYCON_DECLARE() for RZ/A2, to fix earlycon on RZ/A2.
> 
> What do you think?
> Thanks for your comments!
> 
> P.S. Apparently SCIx_SH4_SCIF_REGTYPE and SCIx_SH2_SCIF_FIFODATA_REGTYPE
>      are identical?
> 
> Geert Uytterhoeven (3):
>   [RFC] sh-sci: Take into account regshift to fix earlycon breakage
>   [RFC] Revert "serial: sh-sci: Compute the regshift value for SCI
>     ports"
>   [RFC] sh-sci: Derive regshift value from DT compatible value
> 
> Yoshinori Sato (1):
>   [RFC] sh-sci: Use a separate sci_port for earlycon
> 
>  arch/sh/kernel/cpu/sh3/setup-sh770x.c |  1 +
>  arch/sh/kernel/cpu/sh4/setup-sh7750.c |  3 +-
>  arch/sh/kernel/cpu/sh4/setup-sh7760.c | 10 +---
>  drivers/tty/serial/sh-sci.c           | 68 +++++++++++++++++----------
>  include/linux/serial_sci.h            |  1 +
>  5 files changed, 49 insertions(+), 34 deletions(-)

-- 
Regards,

Laurent Pinchart

Re: [PATCH/RFC 0/4] sh-sci : Do not derive regshift from regsize

[Laurent Pinchart]

On Monday, 6 August 2018 17:37:45 EEST Laurent Pinchart wrote:
> On Monday, 6 August 2018 17:07:51 EEST Geert Uytterhoeven wrote:
> > 	Hi all,
> > 
> > This RFC patch series was sparked by noticing that commit 2d4dd0da45401c7a
> 
> Where can that commit be found ?

I found it in -next, sorry for the noise.

> > ("serial: sh-sci: Allow for compressed SCIF address") broke earlycon
> > support on most Renesas ARM SoCs using SCIF ports, and by the fragility of
> 
> > deriving regshift from the register block size (which may be rounded up):
> 
> Why should it be rounded up ?
> 
> >   1. The first patch is an old patch from Sato-san, which I never really
> >      understood.  But it turned out to be a dependency for patch 2.
> >   2. Patch 2 makes sure regshift is initialized when using earlycon,
> >      unbreaking the serial console on e.g. R-Car Gen2 and Gen3.
> >   3. Patch 3 reverts the patch that started deriving regshift from the
> >      register block size, and that removed the plat_sci_port.regshift
> >      field.  Which is a field I needed again in patch 4.
> >   4. Patch 4 removes the remaining regshift derivations on DT platforms.
> >  (5. I didn't bother writing patch 5, which involves adding .regshift
> >      initializations to all SH board files that need it.)
> > 
> > However, I'm not happy with the end result, so please DO NOT apply this!
> > As I spent almost a full day on this, and would still like to know the
> > story about "sh-sci: Use a separate sci_port for earlycon", I decided to
> > post it anyway.
> > 
> > As earlycon will be broken in v4.19-rc1 on RZ/A1, RZ/G, and R-Car,
> > assuming
> > 
> > no other actions are taken, an alternative solution would be to:
> >   1. Revert commit 7acece71a517cad8 ("serial: sh-sci: Remove
> >      SCIx_RZ_SCIFA_REGTYPE"),
> >   2. Revert commit 2d4dd0da45401c7a ("serial: sh-sci: Allow for compressed
> >      SCIF address") alternative,
> >   3. Add an OF_EARLYCON_DECLARE() for RZ/A2, to fix earlycon on RZ/A2.
> > 
> > What do you think?
> > Thanks for your comments!
> > 
> > P.S. Apparently SCIx_SH4_SCIF_REGTYPE and SCIx_SH2_SCIF_FIFODATA_REGTYPE
> > 
> >      are identical?
> > 
> > Geert Uytterhoeven (3):
> >   [RFC] sh-sci: Take into account regshift to fix earlycon breakage
> >   [RFC] Revert "serial: sh-sci: Compute the regshift value for SCI
> >     ports"
> >   [RFC] sh-sci: Derive regshift value from DT compatible value
> > 
> > Yoshinori Sato (1):
> >   [RFC] sh-sci: Use a separate sci_port for earlycon
> >  
> >  arch/sh/kernel/cpu/sh3/setup-sh770x.c |  1 +
> >  arch/sh/kernel/cpu/sh4/setup-sh7750.c |  3 +-
> >  arch/sh/kernel/cpu/sh4/setup-sh7760.c | 10 +---
> >  drivers/tty/serial/sh-sci.c           | 68 +++++++++++++++++----------
> >  include/linux/serial_sci.h            |  1 +
> >  5 files changed, 49 insertions(+), 34 deletions(-)

-- 
Regards,

Laurent Pinchart

Re: [PATCH/RFC 0/4] sh-sci : Do not derive regshift from regsize

[Laurent Pinchart]

On Monday, 6 August 2018 17:37:45 EEST Laurent Pinchart wrote:
> On Monday, 6 August 2018 17:07:51 EEST Geert Uytterhoeven wrote:
> > 	Hi all,
> > 
> > This RFC patch series was sparked by noticing that commit 2d4dd0da45401c7a
> 
> Where can that commit be found ?

I found it in -next, sorry for the noise.

> > ("serial: sh-sci: Allow for compressed SCIF address") broke earlycon
> > support on most Renesas ARM SoCs using SCIF ports, and by the fragility of
> 
> > deriving regshift from the register block size (which may be rounded up):
> 
> Why should it be rounded up ?
> 
> >   1. The first patch is an old patch from Sato-san, which I never really
> >      understood.  But it turned out to be a dependency for patch 2.
> >   2. Patch 2 makes sure regshift is initialized when using earlycon,
> >      unbreaking the serial console on e.g. R-Car Gen2 and Gen3.
> >   3. Patch 3 reverts the patch that started deriving regshift from the
> >      register block size, and that removed the plat_sci_port.regshift
> >      field.  Which is a field I needed again in patch 4.
> >   4. Patch 4 removes the remaining regshift derivations on DT platforms.
> >  (5. I didn't bother writing patch 5, which involves adding .regshift
> >      initializations to all SH board files that need it.)
> > 
> > However, I'm not happy with the end result, so please DO NOT apply this!
> > As I spent almost a full day on this, and would still like to know the
> > story about "sh-sci: Use a separate sci_port for earlycon", I decided to
> > post it anyway.
> > 
> > As earlycon will be broken in v4.19-rc1 on RZ/A1, RZ/G, and R-Car,
> > assuming
> > 
> > no other actions are taken, an alternative solution would be to:
> >   1. Revert commit 7acece71a517cad8 ("serial: sh-sci: Remove
> >      SCIx_RZ_SCIFA_REGTYPE"),
> >   2. Revert commit 2d4dd0da45401c7a ("serial: sh-sci: Allow for compressed
> >      SCIF address") alternative,
> >   3. Add an OF_EARLYCON_DECLARE() for RZ/A2, to fix earlycon on RZ/A2.
> > 
> > What do you think?
> > Thanks for your comments!
> > 
> > P.S. Apparently SCIx_SH4_SCIF_REGTYPE and SCIx_SH2_SCIF_FIFODATA_REGTYPE
> > 
> >      are identical?
> > 
> > Geert Uytterhoeven (3):
> >   [RFC] sh-sci: Take into account regshift to fix earlycon breakage
> >   [RFC] Revert "serial: sh-sci: Compute the regshift value for SCI
> >     ports"
> >   [RFC] sh-sci: Derive regshift value from DT compatible value
> > 
> > Yoshinori Sato (1):
> >   [RFC] sh-sci: Use a separate sci_port for earlycon
> >  
> >  arch/sh/kernel/cpu/sh3/setup-sh770x.c |  1 +
> >  arch/sh/kernel/cpu/sh4/setup-sh7750.c |  3 +-
> >  arch/sh/kernel/cpu/sh4/setup-sh7760.c | 10 +---
> >  drivers/tty/serial/sh-sci.c           | 68 +++++++++++++++++----------
> >  include/linux/serial_sci.h            |  1 +
> >  5 files changed, 49 insertions(+), 34 deletions(-)

-- 
Regards,

Laurent Pinchart

Re: [PATCH/RFC 0/4] sh-sci : Do not derive regshift from regsize

[Geert Uytterhoeven]

Hi Laurent,

On Mon, Aug 6, 2018 at 4:37 PM Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> On Monday, 6 August 2018 17:07:51 EEST Geert Uytterhoeven wrote:
> > This RFC patch series was sparked by noticing that commit 2d4dd0da45401c7a
>
> Where can that commit be found ?

tty-next

> > ("serial: sh-sci: Allow for compressed SCIF address") broke earlycon
> > support on most Renesas ARM SoCs using SCIF ports, and by the fragility of
> > deriving regshift from the register block size (which may be rounded up):
>
> Why should it be rounded up ?

There's no requirement to round it up.  But it's not uncommon to round up
register block sizes to the next power of two, or more.

In hindsight, perhaps we should have adopted the "reg-shift" DT property,
which is used by other serial port drivers, and by of_setup_earlycon().

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/RFC 0/4] sh-sci : Do not derive regshift from regsize

[Geert Uytterhoeven]

Hi Laurent,

On Mon, Aug 6, 2018 at 4:37 PM Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> On Monday, 6 August 2018 17:07:51 EEST Geert Uytterhoeven wrote:
> > This RFC patch series was sparked by noticing that commit 2d4dd0da45401c7a
>
> Where can that commit be found ?

tty-next

> > ("serial: sh-sci: Allow for compressed SCIF address") broke earlycon
> > support on most Renesas ARM SoCs using SCIF ports, and by the fragility of
> > deriving regshift from the register block size (which may be rounded up):
>
> Why should it be rounded up ?

There's no requirement to round it up.  But it's not uncommon to round up
register block sizes to the next power of two, or more.

In hindsight, perhaps we should have adopted the "reg-shift" DT property,
which is used by other serial port drivers, and by of_setup_earlycon().

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/RFC 0/4] sh-sci : Do not derive regshift from regsize

[Geert Uytterhoeven]

On Mon, Aug 6, 2018 at 4:08 PM Geert Uytterhoeven
<geert+renesas@glider.be> wrote:
> This RFC patch series was sparked by noticing that commit 2d4dd0da45401c7a
> ("serial: sh-sci: Allow for compressed SCIF address") broke earlycon
> support on most Renesas ARM SoCs using SCIF ports, and by the fragility of
> deriving regshift from the register block size (which may be rounded up):
>   1. The first patch is an old patch from Sato-san, which I never really
>      understood.  But it turned out to be a dependency for patch 2.
>   2. Patch 2 makes sure regshift is initialized when using earlycon,
>      unbreaking the serial console on e.g. R-Car Gen2 and Gen3.
>   3. Patch 3 reverts the patch that started deriving regshift from the
>      register block size, and that removed the plat_sci_port.regshift
>      field.  Which is a field I needed again in patch 4.
>   4. Patch 4 removes the remaining regshift derivations on DT platforms.
>  (5. I didn't bother writing patch 5, which involves adding .regshift
>      initializations to all SH board files that need it.)
>
> However, I'm not happy with the end result, so please DO NOT apply this!
> As I spent almost a full day on this, and would still like to know the
> story about "sh-sci: Use a separate sci_port for earlycon", I decided to
> post it anyway.
>
> As earlycon will be broken in v4.19-rc1 on RZ/A1, RZ/G, and R-Car, assuming
> no other actions are taken, an alternative solution would be to:
>   1. Revert commit 7acece71a517cad8 ("serial: sh-sci: Remove
>      SCIx_RZ_SCIFA_REGTYPE"),
>   2. Revert commit 2d4dd0da45401c7a ("serial: sh-sci: Allow for compressed
>      SCIF address") alternative,
>   3. Add an OF_EARLYCON_DECLARE() for RZ/A2, to fix earlycon on RZ/A2.
>
> What do you think?
> Thanks for your comments!

I gave this a bit more thought, and these are my conclusions:
  1. RZ/A2 SCIF is not really compatible with SCIF on RZ/A1, RZ/G, and
     R-Car, due to the absence of support for the DT "reg-shift" property.
     Given RZ/A1 and R-Car would need "reg-shift = <1>", we cannot
     retroactively add support for that for ports declared compatible with
     "renesas,scif" (assuming regshift = 1 in case the "reg-shift"
     property is not present may work for the main serial driver, but not
     for earlycon, where of_setup_earlycon() is in charge of all register
     block parsing).
     Sorry for suggesting this in the first place. Based on the regshift
     handling for SCI, I assumed it was fine, but apparently that
     particular code isn't without problems on SCI ports neither.
  2. SCI ports are the only ports where a DT "reg-shift" property makes
     sense, as it is the only port type with fixed (8-bit) register sizes,
     with registers spaced 1, 2 or 4 bytes apart (regshift = 0, 1, or 2).
     All other types use a mix of 8-bit and 16-bit registers, 2 or 4 bytes
     apart.
       - On H8/300 (DT only), regshift is always zero.
         AFAIK this is handled correctly for earlycon, but not for the main
         serial driver (presumably broken since commit
         dfc80387aefb78161f83732804c6d01c89c24595, cfr. Sato-san's patch
         "serial: sh-sci: byte allocated register support"
         (https://www.spinics.net/lists/linux-sh/msg53175.html)).
       - On SuperH (non-DT), regshift is 1 or 2.
         If/when DT support is ever added, the "reg-shift" DT property can
         be used to indicate that.

Suggestions for actions:
  1. Revert commit 7acece71a517cad8 ("serial: sh-sci: Remove
     SCIx_RZ_SCIFA_REGTYPE"), as this is a dependency for step 2,
  2. Revert commit 2d4dd0da45401c7a ("serial: sh-sci: Allow for compressed
     SCIF address"), to unbreak earlycon on RZ/A1, RZ/G, and R-Car SCIF
     ports,
  3. Restrict the regshift setup (1 or 2) for PORT_SCI to the
     !dev->dev.of_node case, to fix SCI ports instantiated from DT on
     H8/300.
  4. Drop "renesas,scif" from the compatible value in the (not yet
     submitted) r7s9210.dtsi (this may have to be clarified in the DT
     bindings, although they already say "renesas,scif" is only meant for
     ports compatible with the generic version, whatever that means ;-),
  5. Add an OF_EARLYCON_DECLARE() for RZ/A2, setting port_cfg.regtype to
     SCIx_RZ_SCIFA_REGTYPE, to (presumably) make earlycon work on RZ/A2,

1-2 are regression fixes that can be done immediately,
3 is a bug fix to be submitted,
5 is an enhancement to be submitted.

Any other opinions or comments?

As both Greg and I will be on holidays until after the v4.18 release, you
have a bit more time to polish all of this for the serial pull request
for v4.19...

Thanks!

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/RFC 0/4] sh-sci : Do not derive regshift from regsize

[Geert Uytterhoeven]

On Mon, Aug 6, 2018 at 4:08 PM Geert Uytterhoeven
<geert+renesas@glider.be> wrote:
> This RFC patch series was sparked by noticing that commit 2d4dd0da45401c7a
> ("serial: sh-sci: Allow for compressed SCIF address") broke earlycon
> support on most Renesas ARM SoCs using SCIF ports, and by the fragility of
> deriving regshift from the register block size (which may be rounded up):
>   1. The first patch is an old patch from Sato-san, which I never really
>      understood.  But it turned out to be a dependency for patch 2.
>   2. Patch 2 makes sure regshift is initialized when using earlycon,
>      unbreaking the serial console on e.g. R-Car Gen2 and Gen3.
>   3. Patch 3 reverts the patch that started deriving regshift from the
>      register block size, and that removed the plat_sci_port.regshift
>      field.  Which is a field I needed again in patch 4.
>   4. Patch 4 removes the remaining regshift derivations on DT platforms.
>  (5. I didn't bother writing patch 5, which involves adding .regshift
>      initializations to all SH board files that need it.)
>
> However, I'm not happy with the end result, so please DO NOT apply this!
> As I spent almost a full day on this, and would still like to know the
> story about "sh-sci: Use a separate sci_port for earlycon", I decided to
> post it anyway.
>
> As earlycon will be broken in v4.19-rc1 on RZ/A1, RZ/G, and R-Car, assuming
> no other actions are taken, an alternative solution would be to:
>   1. Revert commit 7acece71a517cad8 ("serial: sh-sci: Remove
>      SCIx_RZ_SCIFA_REGTYPE"),
>   2. Revert commit 2d4dd0da45401c7a ("serial: sh-sci: Allow for compressed
>      SCIF address") alternative,
>   3. Add an OF_EARLYCON_DECLARE() for RZ/A2, to fix earlycon on RZ/A2.
>
> What do you think?
> Thanks for your comments!

I gave this a bit more thought, and these are my conclusions:
  1. RZ/A2 SCIF is not really compatible with SCIF on RZ/A1, RZ/G, and
     R-Car, due to the absence of support for the DT "reg-shift" property.
     Given RZ/A1 and R-Car would need "reg-shift = <1>", we cannot
     retroactively add support for that for ports declared compatible with
     "renesas,scif" (assuming regshift == 1 in case the "reg-shift"
     property is not present may work for the main serial driver, but not
     for earlycon, where of_setup_earlycon() is in charge of all register
     block parsing).
     Sorry for suggesting this in the first place. Based on the regshift
     handling for SCI, I assumed it was fine, but apparently that
     particular code isn't without problems on SCI ports neither.
  2. SCI ports are the only ports where a DT "reg-shift" property makes
     sense, as it is the only port type with fixed (8-bit) register sizes,
     with registers spaced 1, 2 or 4 bytes apart (regshift = 0, 1, or 2).
     All other types use a mix of 8-bit and 16-bit registers, 2 or 4 bytes
     apart.
       - On H8/300 (DT only), regshift is always zero.
         AFAIK this is handled correctly for earlycon, but not for the main
         serial driver (presumably broken since commit
         dfc80387aefb78161f83732804c6d01c89c24595, cfr. Sato-san's patch
         "serial: sh-sci: byte allocated register support"
         (https://www.spinics.net/lists/linux-sh/msg53175.html)).
       - On SuperH (non-DT), regshift is 1 or 2.
         If/when DT support is ever added, the "reg-shift" DT property can
         be used to indicate that.

Suggestions for actions:
  1. Revert commit 7acece71a517cad8 ("serial: sh-sci: Remove
     SCIx_RZ_SCIFA_REGTYPE"), as this is a dependency for step 2,
  2. Revert commit 2d4dd0da45401c7a ("serial: sh-sci: Allow for compressed
     SCIF address"), to unbreak earlycon on RZ/A1, RZ/G, and R-Car SCIF
     ports,
  3. Restrict the regshift setup (1 or 2) for PORT_SCI to the
     !dev->dev.of_node case, to fix SCI ports instantiated from DT on
     H8/300.
  4. Drop "renesas,scif" from the compatible value in the (not yet
     submitted) r7s9210.dtsi (this may have to be clarified in the DT
     bindings, although they already say "renesas,scif" is only meant for
     ports compatible with the generic version, whatever that means ;-),
  5. Add an OF_EARLYCON_DECLARE() for RZ/A2, setting port_cfg.regtype to
     SCIx_RZ_SCIFA_REGTYPE, to (presumably) make earlycon work on RZ/A2,

1-2 are regression fixes that can be done immediately,
3 is a bug fix to be submitted,
5 is an enhancement to be submitted.

Any other opinions or comments?

As both Greg and I will be on holidays until after the v4.18 release, you
have a bit more time to polish all of this for the serial pull request
for v4.19...

Thanks!

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/RFC 0/4] sh-sci : Do not derive regshift from regsize

[Chris Brandt]

SGkgR2VlcnQsDQoNCk9uIFdlZG5lc2RheSwgQXVndXN0IDA4LCAyMDE4LCBHZWVydCBVeXR0ZXJo
b2V2ZW4gd3JvdGU6DQo+IFN1Z2dlc3Rpb25zIGZvciBhY3Rpb25zOg0KPiAgIDEuIFJldmVydCBj
b21taXQgN2FjZWNlNzFhNTE3Y2FkOCAoInNlcmlhbDogc2gtc2NpOiBSZW1vdmUNCj4gICAgICBT
Q0l4X1JaX1NDSUZBX1JFR1RZUEUiKSwgYXMgdGhpcyBpcyBhIGRlcGVuZGVuY3kgZm9yIHN0ZXAg
MiwNCj4gICAyLiBSZXZlcnQgY29tbWl0IDJkNGRkMGRhNDU0MDFjN2EgKCJzZXJpYWw6IHNoLXNj
aTogQWxsb3cgZm9yIGNvbXByZXNzZWQNCj4gICAgICBTQ0lGIGFkZHJlc3MiKSwgdG8gdW5icmVh
ayBlYXJseWNvbiBvbiBSWi9BMSwgUlovRywgYW5kIFItQ2FyIFNDSUYNCj4gICAgICBwb3J0cywN
Cg0KT2ggd2VsbC4gVGhlIGlkZWEgb2YgYSBzaGFyZWQgU0NJRiByZWd0eXBlIHdhcyBhIGdvb2Qg
aWRlYS4uLmp1c3QgdG9vIA0KbXVjaCBiYWdnYWdlIHRvIGRlYWwgd2l0aC4NCg0KDQo+ICAgNC4g
RHJvcCAicmVuZXNhcyxzY2lmIiBmcm9tIHRoZSBjb21wYXRpYmxlIHZhbHVlIGluIHRoZSAobm90
IHlldA0KPiAgICAgIHN1Ym1pdHRlZCkgcjdzOTIxMC5kdHNpICh0aGlzIG1heSBoYXZlIHRvIGJl
IGNsYXJpZmllZCBpbiB0aGUgRFQNCj4gICAgICBiaW5kaW5ncywgYWx0aG91Z2ggdGhleSBhbHJl
YWR5IHNheSAicmVuZXNhcyxzY2lmIiBpcyBvbmx5IG1lYW50IGZvcg0KPiAgICAgIHBvcnRzIGNv
bXBhdGlibGUgd2l0aCB0aGUgZ2VuZXJpYyB2ZXJzaW9uLCB3aGF0ZXZlciB0aGF0IG1lYW5zIDst
KSwNCg0KQXMgc29vbiBhcyBJIHVwZGF0ZSB0byB0aGUgbmV3IGNsb2NrIGRyaXZlciBzdHlsZSBh
bmQgZ2V0IHRoYXQgDQptYWlubGluZWQsIHRoZW4gSSBjYW4gc3VibWl0IHRoZSByN3M5MjEwLmR0
c2kuDQoNCj4gICA1LiBBZGQgYW4gT0ZfRUFSTFlDT05fREVDTEFSRSgpIGZvciBSWi9BMiwgc2V0
dGluZyBwb3J0X2NmZy5yZWd0eXBlIHRvDQo+ICAgICAgU0NJeF9SWl9TQ0lGQV9SRUdUWVBFLCB0
byAocHJlc3VtYWJseSkgbWFrZSBlYXJseWNvbiB3b3JrIG9uIFJaL0EyLA0KDQpUaGlzIGlzIHRo
ZSBvbmUgdGhhdCBJIHdhcyBoYXBweSB0byBsZWFybiBob3cgdG8gZG8gaXQgc2luY2Ugb3JpZ2lu
YWxseSANCkkgd2Fzbid0IHN1cmUgaG93IEkgY291bGQgZ2V0IGVhcmx5Y29uIHRvIHdvcmsgd2l0
aCB0aGUgb2RkYmFsbCBSWi9BMiANClNDSUYuDQoNClRoYW5rcywNCkNocmlzDQoNCg=

RE: [PATCH/RFC 0/4] sh-sci : Do not derive regshift from regsize

[Chris Brandt]

Hi Geert,

On Wednesday, August 08, 2018, Geert Uytterhoeven wrote:
> Suggestions for actions:
>   1. Revert commit 7acece71a517cad8 ("serial: sh-sci: Remove
>      SCIx_RZ_SCIFA_REGTYPE"), as this is a dependency for step 2,
>   2. Revert commit 2d4dd0da45401c7a ("serial: sh-sci: Allow for compressed
>      SCIF address"), to unbreak earlycon on RZ/A1, RZ/G, and R-Car SCIF
>      ports,

Oh well. The idea of a shared SCIF regtype was a good idea...just too 
much baggage to deal with.


>   4. Drop "renesas,scif" from the compatible value in the (not yet
>      submitted) r7s9210.dtsi (this may have to be clarified in the DT
>      bindings, although they already say "renesas,scif" is only meant for
>      ports compatible with the generic version, whatever that means ;-),

As soon as I update to the new clock driver style and get that 
mainlined, then I can submit the r7s9210.dtsi.

>   5. Add an OF_EARLYCON_DECLARE() for RZ/A2, setting port_cfg.regtype to
>      SCIx_RZ_SCIFA_REGTYPE, to (presumably) make earlycon work on RZ/A2,

This is the one that I was happy to learn how to do it since originally 
I wasn't sure how I could get earlycon to work with the oddball RZ/A2 
SCIF.

Thanks,
Chris

RE: [PATCH/RFC 0/4] sh-sci : Do not derive regshift from regsize

[Chris Brandt]

SGkgR2VlcnQsDQoNCkp1c3QgRllJLi4uDQoNCk9uIFdlZG5lc2RheSwgQXVndXN0IDA4LCAyMDE4
LCBHZWVydCBVeXR0ZXJob2V2ZW4gd3JvdGU6DQo+ICAgNC4gRHJvcCAicmVuZXNhcyxzY2lmIiBm
cm9tIHRoZSBjb21wYXRpYmxlIHZhbHVlIGluIHRoZSAobm90IHlldA0KPiAgICAgIHN1Ym1pdHRl
ZCkgcjdzOTIxMC5kdHNpICh0aGlzIG1heSBoYXZlIHRvIGJlIGNsYXJpZmllZCBpbiB0aGUgRFQN
Cj4gICAgICBiaW5kaW5ncywgYWx0aG91Z2ggdGhleSBhbHJlYWR5IHNheSAicmVuZXNhcyxzY2lm
IiBpcyBvbmx5IG1lYW50IGZvcg0KPiAgICAgIHBvcnRzIGNvbXBhdGlibGUgd2l0aCB0aGUgZ2Vu
ZXJpYyB2ZXJzaW9uLCB3aGF0ZXZlciB0aGF0IG1lYW5zIDstKSwNCg0KSnVzdCBhcyB5b3Ugd291
bGQgZXhwZWN0LCBpZiBJIGRyb3AgInJlbmVzYXMsc2NpZiIgZnJvbSByN3M5MjEwLmR0c2ksIA0K
dGhlbiBlYXJseWNvbiB3b3JrcyB3aXRoIHlvdXIgUkZDIHBhdGNoZXMgc2luY2Ugbm93IHRoZXJl
IGlzIG9ubHkgb25lDQpjb21wYXRpYmxlIGl0IGNhbiBwb3NzaWJseSBtYXRjaCBhZ2FpbnN0Lg0K
DQoNCkNocmlzDQo

RE: [PATCH/RFC 0/4] sh-sci : Do not derive regshift from regsize

[Chris Brandt]

Hi Geert,

Just FYI...

On Wednesday, August 08, 2018, Geert Uytterhoeven wrote:
>   4. Drop "renesas,scif" from the compatible value in the (not yet
>      submitted) r7s9210.dtsi (this may have to be clarified in the DT
>      bindings, although they already say "renesas,scif" is only meant for
>      ports compatible with the generic version, whatever that means ;-),

Just as you would expect, if I drop "renesas,scif" from r7s9210.dtsi, 
then earlycon works with your RFC patches since now there is only one
compatible it can possibly match against.


Chris