ARM: shmobile: Add early debugging support using SCIF(A)

PATCH New Geert Uytterhoeven 4 participants 9 messages Nov 6, 2014 – Nov 11, 2014

Subsystems (4)

Arm (32-bit) Arm sub-architectures SuperH Arm/RISC-V/Renesas architecture

lore

Share & tools

lore

Geert Uytterhoeven

author maintainer

2014-11-06 11:38 UTC #0 lore raw

PATCH 1/1 ARM: shmobile: Add early debugging support using SCIF(A)

Add serial port debug macros for the SCIF(A) serial ports. This includes all shmobile SoCs, except for EMEV2.

The configuration logic (both Kconfig and #ifdef) is more complicated than one would expect, for several reasons:
1. Not all SoCs have the same serial ports, and they're not always at the same addresses.
2. There are two different types: SCIF and SCIFA. Fortunately they can easily be distinguished by physical address.
3. Not all boards use the same serial port for the console. The defaults correspond to the boards that are supported in mainline. If you want to use a different serial port, just change the value of CONFIG_DEBUG_UART_PHYS, and the rest will auto-adapt.
4. On SoCs using the legacy machine_desc.map_io(), an identity mapping of the SCIF(A) registers must be set up in .map_io(). On all but r8a7779 this was already done.
On other SoCs, debug_ll_io_init() maps the SCIF(A) registers to a fixed virtual address. 0xfdxxxxxx was chosen, as it should lie below VMALLOC_END = 0xff000000, and must not conflict with the 2 MiB reserved region at PCI_IO_VIRT_BASE = 0xfee00000.
Once all SoCs have migrated away from machine_desc.map_io(), more consolidation of DEBUG_<SoC>_<PORT> can be done.

This was derived from the r8a7790 version by Laurent Pinchart.

Signed-off-by Geert Uytterhoeven <[email protected]>

Acked-by Laurent Pinchart <[email protected]>

---
Tested by me on r8a7791/koelsch, r8a7740/armadillo, and sh73a0/kzm9g. TTBOMK tested by Simon or r8a73a4/ape6evm.

Testing on other platforms would still be appreciated, but I think at this point it's already better to have it than not to have it, at least in renesas-devel-*.

v3:
- Add Acked-by,
v2:
- Rename CONFIG_DEBUG_SCIF to CONFIG_DEBUG_RENESAS_SCIF,
- Rename scif.S to renesas-scif.S,
- Add intermediate DEBUG_<SoC>_<PORT> Kconfig symbols in a Kconfig choice statement to select the SoC/port combination for debugging. Use these symbols instead of the SoC type to (a) control the DEBUG_UART_PHYS defaults, and (b) pick the right virtual address. This fixes the case of debugging on an SoC not using machine_desc.map_io(), while platform support for any SoC still using machine_desc.map_io() is enabled.
- Protect SCIF mapping on r8a7779 by #ifdef CONFIG_DEBUG_R8A7779_SCIF2,
- Add renesas-scif.S to the ARM/SHMOBILE section in MAINTAINERS.

Notes:
- On armadillo-multiplatform there may be a period while garbage data is output.
This happens because sh_mobile_i2c_init() enables and disables its clock during probing. As iic0 and scifa1 share the same parent clock, this causes the scifa1 clock to no longer receive clock ticks.
On armadillo-legacy, this is mitigated by the pre-CCF clock driver, which never really disables clocks during boot-up for exactly this reason. Cfr. "One example of this is the handling of the Mackerel serial console output that shares clock with the I2C controller.", in commit 794d78fea51504ba ("drivers: sh: late disabling of clocks V2").
I'm wondering whether this can be fixed in the i2c driver? Does it really have to enable and disable the clock?

---
 MAINTAINERS                            |  1 +
 arch/arm/Kconfig.debug                 | 67 +++++++++++++++++++++++++++++++++-
 arch/arm/include/debug/renesas-scif.S  | 62 +++++++++++++++++++++++++++++++
 arch/arm/mach-shmobile/setup-r8a7779.c |  9 +++++
 4 files changed, 138 insertions(+), 1 deletion(-)
 create mode 100644 arch/arm/include/debug/renesas-scif.S

MAINTAINERS +1 -0

index c931fc99ee84e933..6c16b3da8fe66a7f 100644
@@ -1386,6 +1386,7 @@ F:	arch/arm/configs/lager_defconfig
 F:	arch/arm/configs/mackerel_defconfig
 F:	arch/arm/configs/marzen_defconfig
 F:	arch/arm/configs/shmobile_defconfig
+F:	arch/arm/include/debug/renesas-scif.S
 F:	arch/arm/mach-shmobile/
 F:	drivers/sh/

arch/arm/Kconfig.debug +66 -1

103 lines not on this page · raw message

arch/arm/include/debug/renesas-scif.S new file +62 -0

62 lines not on this page · raw message

arch/arm/mach-shmobile/setup-r8a7779.c +9 -0

index d08e75cceaab3ead..b31032294f478c91 100644
@@ -62,6 +62,15 @@ static struct map_desc r8a7779_io_desc[] __initdata = {
 		.length		= SZ_16M,
 		.type		= MT_DEVICE_NONSHARED
 	},
+#ifdef CONFIG_DEBUG_R8A7779_SCIF2
+	/* 64K entity map for 0xffe40000 (SCIF) */
+	{
+		.virtual	= 0xffe40000,
+		.pfn		= __phys_to_pfn(0xffe40000),
+		.length		= SZ_64K,
+		.type		= MT_DEVICE_NONSHARED
+	},
+#endif /* CONFIG_DEBUG_R8A7779_SCIF2 */
 };
 
 void __init r8a7779_map_io(void)

Signature · 1 line

1.9.1

Arnd Bergmann

2014-11-06 11:53 UTC #1 lore raw

PATCH 1/1 ARM: shmobile: Add early debugging support using SCIF(A)

Quoted 23 lines · On Thursday 06 November 2014 12:38:30 Geert Uytterhoeven wrote:

Add serial port debug macros for the SCIF(A) serial ports. This includes all shmobile SoCs, except for EMEV2.
The configuration logic (both Kconfig and #ifdef) is more complicated than one would expect, for several reasons:
1. Not all SoCs have the same serial ports, and they're not always at the same addresses.
2. There are two different types: SCIF and SCIFA. Fortunately they can easily be distinguished by physical address.
3. Not all boards use the same serial port for the console. The defaults correspond to the boards that are supported in mainline. If you want to use a different serial port, just change the value of CONFIG_DEBUG_UART_PHYS, and the rest will auto-adapt.
4. On SoCs using the legacy machine_desc.map_io(), an identity mapping of the SCIF(A) registers must be set up in .map_io(). On all but r8a7779 this was already done.
On other SoCs, debug_ll_io_init() maps the SCIF(A) registers to a fixed virtual address. 0xfdxxxxxx was chosen, as it should lie below VMALLOC_END = 0xff000000, and must not conflict with the 2 MiB reserved region at PCI_IO_VIRT_BASE = 0xfee00000.
Once all SoCs have migrated away from machine_desc.map_io(), more consolidation of DEBUG_<SoC>_<PORT> can be done.

Quoted 4 lines

This was derived from the r8a7790 version by Laurent Pinchart.
Signed-off-by Geert Uytterhoeven <[email protected]>
Acked-by Laurent Pinchart <[email protected]>

I don't object to this patch going in, but have you looked at doing just earlycon instead? We are still missing some fixmap changes before that works on arm32, but in general, early looks nicer than debug_ll because it doesn't require you to build for a particular machine, with the downside that it's not available as early during boot.

Quoted 4 lines

arch/arm/Kconfig.debug +0 -0

@@ -1039,6 +1047,54 @@ config DEBUG_STI_UART
 	bool
 	depends on ARCH_STI

arch/arm/Kconfig.debug config DEBUG_STI_UART · line 1059

Quoted 10 lines

arch/arm/Kconfig.debug +10 -0

+choice
+	prompt "Renesas SCIF(A) debugging SoC/port"
+	depends on DEBUG_RENESAS_SCIF
+	help
+	  Choose SoC and SCIF(A) debug port combination from the list below.
+
+	  The kernel will most probably only work on the selected SoC.
+	  The selected debug port will be used to provide the default SCIF(A)
+	  physical address; the actual address can still be overridden using
+	  the DEBUG_UART_PHYS option, if needed.

We used to have multi-level choice statements here, but Russell removed them all in cce278d2034 ("ARM: debug: clean up low level kernel debugging selection"). Please follow the same style.

arch/arm/mach-shmobile/setup-r8a7779.c static struct map_desc r8a7779_io_desc[] __initdata = { · line 66

Quoted 10 lines

arch/arm/mach-shmobile/setup-r8a7779.c +2 -0

index d08e75cceaab3ead..b31032294f478c91 100644
@@ -62,6 +62,15 @@ static struct map_desc r8a7779_io_desc[] __initdata = {
 		.length		= SZ_16M,
 		.type		= MT_DEVICE_NONSHARED
 	},
+#ifdef CONFIG_DEBUG_R8A7779_SCIF2
+	/* 64K entity map for 0xffe40000 (SCIF) */

s/entity/identity/ ?

arch/arm/mach-shmobile/setup-r8a7779.c static struct map_desc r8a7779_io_desc[] __initdata = { · line 74

Quoted 8 lines

arch/arm/mach-shmobile/setup-r8a7779.c +7 -0

+	{
+		.virtual	= 0xffe40000,
+		.pfn		= __phys_to_pfn(0xffe40000),
+		.length		= SZ_64K,
+		.type		= MT_DEVICE_NONSHARED
+	},
+#endif /* CONFIG_DEBUG_R8A7779_SCIF2 */
 };

This should be done automatically if you call debug_ll_io_init() or have an empty map_io() callback.

Arnd

Geert Uytterhoeven

2014-11-06 12:33 UTC #2 lore raw

PATCH 1/1 ARM: shmobile: Add early debugging support using SCIF(A)

Hi Arnd,

Thanks for your review!

Quoted 6 lines · Arnd Bergmann

Quoted 2 lines · On Thursday 06 November 2014 12:38:30 Geert Uytterhoeven wrote:
Add serial port debug macros for the SCIF(A) serial ports. This includes all shmobile SoCs, except for EMEV2.
I don't object to this patch going in, but have you looked at doing just earlycon instead? We are still missing some fixmap changes

I was thinking about earlycon support...

Quoted 1 line

before that works on arm32, but in general, early looks nicer than

Good to know there are still some parts missing.

Quoted 3 lines

debug_ll because it doesn't require you to build for a particular machine, with the downside that it's not available as early during boot.

Hence we're gonna need debug_ll anyway?
So far I only needed it for real early issues.

Quoted 4 lines

arch/arm/Kconfig.debug +0 -0

@@ -1039,6 +1047,54 @@ config DEBUG_STI_UART
      bool
      depends on ARCH_STI

Quoted 14 lines

arch/arm/Kconfig.debug config DEBUG_STI_UART · line 1059
Quoted 10 lines
arch/arm/Kconfig.debug +10 -0
+choice
+     prompt "Renesas SCIF(A) debugging SoC/port"
+     depends on DEBUG_RENESAS_SCIF
+     help
+       Choose SoC and SCIF(A) debug port combination from the list below.
+
+       The kernel will most probably only work on the selected SoC.
+       The selected debug port will be used to provide the default SCIF(A)
+       physical address; the actual address can still be overridden using
+       the DEBUG_UART_PHYS option, if needed.
We used to have multi-level choice statements here, but Russell removed them all in cce278d2034 ("ARM: debug: clean up low level kernel debugging selection"). Please follow the same style.

OK, will move it up.

Quoted 4 lines

Quoted 4 lines
arch/arm/mach-shmobile/setup-r8a7779.c +0 -0
index d08e75cceaab3ead..b31032294f478c91 100644

Quoted 8 lines

arch/arm/mach-shmobile/setup-r8a7779.c static struct map_desc r8a7779_io_desc[] __initdata = { · line 66
Quoted 6 lines
arch/arm/mach-shmobile/setup-r8a7779.c +2 -0
@@ -62,6 +62,15 @@ static struct map_desc r8a7779_io_desc[] __initdata = {
              .length         = SZ_16M,
              .type           = MT_DEVICE_NONSHARED
      },
+#ifdef CONFIG_DEBUG_R8A7779_SCIF2
+     /* 64K entity map for 0xffe40000 (SCIF) */
s/entity/identity/ ?

Yep. Never copy swenglish comments.

Quoted 11 lines

arch/arm/mach-shmobile/setup-r8a7779.c static struct map_desc r8a7779_io_desc[] __initdata = { · line 74
Quoted 8 lines
arch/arm/mach-shmobile/setup-r8a7779.c +7 -0
+     {
+             .virtual        = 0xffe40000,
+             .pfn            = __phys_to_pfn(0xffe40000),
+             .length         = SZ_64K,
+             .type           = MT_DEVICE_NONSHARED
+     },
+#endif /* CONFIG_DEBUG_R8A7779_SCIF2 */
 };
This should be done automatically if you call debug_ll_io_init() or have an empty map_io() callback.

On r8a7779 map_io() is not empty, but it didn't cover the SCIF registers, cfr. the commit log:

Quoted 3 lines

Quoted 3 lines
arch/arm/mach-shmobile/setup-r8a7779.c +0 -0
  4. On SoCs using the legacy machine_desc.map_io(), an identity mapping
     of the SCIF(A) registers must be set up in .map_io(). On all but
r8a7779 this was already done.

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at 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

Arnd Bergmann

2014-11-06 13:26 UTC #3 lore raw

PATCH 1/1 ARM: shmobile: Add early debugging support using SCIF(A)

Quoted 13 lines · On Thursday 06 November 2014 13:33:58 Geert Uytterhoeven wrote:

Quoted 6 lines · Arnd Bergmann
Quoted 2 lines · On Thursday 06 November 2014 12:38:30 Geert Uytterhoeven wrote:
Add serial port debug macros for the SCIF(A) serial ports. This includes all shmobile SoCs, except for EMEV2.
I don't object to this patch going in, but have you looked at doing just earlycon instead? We are still missing some fixmap changes
I was thinking about earlycon support...
Quoted 1 line
before that works on arm32, but in general, early looks nicer than

Quoted 8 lines

Good to know there are still some parts missing.
Quoted 3 lines
debug_ll because it doesn't require you to build for a particular machine, with the downside that it's not available as early during boot.
Hence we're gonna need debug_ll anyway?
So far I only needed it for real early issues.

It depends on whether you were missing it in the past. shmobile so far only supports early_platform, which isn't quite as early as earlycon. If early_platform was good enough then earlycon will work too.

Quoted 9 lines

Quoted 8 lines

arch/arm/mach-shmobile/setup-r8a7779.c +7 -0

+     {
+             .virtual        = 0xffe40000,
+             .pfn            = __phys_to_pfn(0xffe40000),
+             .length         = SZ_64K,
+             .type           = MT_DEVICE_NONSHARED
+     },
+#endif /* CONFIG_DEBUG_R8A7779_SCIF2 */
 };

Quoted 9 lines

Quoted 2 lines
This should be done automatically if you call debug_ll_io_init() or have an empty map_io() callback.
On r8a7779 map_io() is not empty, but it didn't cover the SCIF registers, cfr. the commit log:
Quoted 3 lines
Quoted 3 lines
4. On SoCs using the legacy machine_desc.map_io(), an identity mapping of the SCIF(A) registers must be set up in .map_io(). On all but r8a7779 this was already done.

Ok. You can of course just call debug_ll_io_init() manually from the map_io function, it will create the same mapping.

Arnd

Simon Horman

2014-11-10 00:59 UTC #4 lore raw

PATCH 1/1 ARM: shmobile: Add early debugging support using SCIF(A)

Quoted 43 lines · Arnd Bergmann

Quoted 13 lines · On Thursday 06 November 2014 13:33:58 Geert Uytterhoeven wrote:
Quoted 6 lines · Arnd Bergmann
Quoted 2 lines · On Thursday 06 November 2014 12:38:30 Geert Uytterhoeven wrote:
Add serial port debug macros for the SCIF(A) serial ports. This includes all shmobile SoCs, except for EMEV2.
I don't object to this patch going in, but have you looked at doing just earlycon instead? We are still missing some fixmap changes
I was thinking about earlycon support...
Quoted 1 line
before that works on arm32, but in general, early looks nicer than
Quoted 8 lines
Good to know there are still some parts missing.
Quoted 3 lines
debug_ll because it doesn't require you to build for a particular machine, with the downside that it's not available as early during boot.
Hence we're gonna need debug_ll anyway?
So far I only needed it for real early issues.
It depends on whether you were missing it in the past. shmobile so far only supports early_platform, which isn't quite as early as earlycon. If early_platform was good enough then earlycon will work too.
Quoted 15 lines
Quoted 11 lines
arch/arm/mach-shmobile/setup-r8a7779.c static struct map_desc r8a7779_io_desc[] __initdata = { · line 74
Quoted 8 lines
arch/arm/mach-shmobile/setup-r8a7779.c +7 -0
+     {
+             .virtual        = 0xffe40000,
+             .pfn            = __phys_to_pfn(0xffe40000),
+             .length         = SZ_64K,
+             .type           = MT_DEVICE_NONSHARED
+     },
+#endif /* CONFIG_DEBUG_R8A7779_SCIF2 */
 };
This should be done automatically if you call debug_ll_io_init() or have an empty map_io() callback.
On r8a7779 map_io() is not empty, but it didn't cover the SCIF registers, cfr. the commit log:

Quoted 6 lines

Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
4. On SoCs using the legacy machine_desc.map_io(), an identity mapping of the SCIF(A) registers must be set up in .map_io(). On all but r8a7779 this was already done.
Ok. You can of course just call debug_ll_io_init() manually from the map_io function, it will create the same mapping.

This patch feels like is is ready for me to queue up. Is that the consensus here?

Geert Uytterhoeven

2014-11-10 08:03 UTC #5 lore raw

PATCH 1/1 ARM: shmobile: Add early debugging support using SCIF(A)

Quoted 4 lines · Simon Horman

Quoted 3 lines · Arnd Bergmann
Quoted 2 lines · On Thursday 06 November 2014 13:33:58 Geert Uytterhoeven wrote:
Quoted 1 line · Arnd Bergmann
On Thursday 06 November 2014 12:38:30 Geert Uytterhoeven wrote:

Quoted 8 lines

Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 2 lines
Add serial port debug macros for the SCIF(A) serial ports. This includes all shmobile SoCs, except for EMEV2.
I don't object to this patch going in, but have you looked at doing just earlycon instead? We are still missing some fixmap changes
This patch feels like is is ready for me to queue up. Is that the consensus here?

Please wait for v3, which will get rid of the multi-level choice statements, as requested by Arnd.

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at 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

Simon Horman

2014-11-11 00:15 UTC #6 lore raw

PATCH 1/1 ARM: shmobile: Add early debugging support using SCIF(A)

Quoted 8 lines · Geert Uytterhoeven

Quoted 7 lines · Simon Horman
Quoted 6 lines · Arnd Bergmann
Quoted 5 lines · On Thursday 06 November 2014 13:33:58 Geert Uytterhoeven wrote:
Quoted 3 lines · Arnd Bergmann
Quoted 2 lines · On Thursday 06 November 2014 12:38:30 Geert Uytterhoeven wrote:
Add serial port debug macros for the SCIF(A) serial ports. This includes all shmobile SoCs, except for EMEV2.

Quoted 8 lines

Quoted 5 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
I don't object to this patch going in, but have you looked at doing just earlycon instead? We are still missing some fixmap changes
This patch feels like is is ready for me to queue up. Is that the consensus here?
Please wait for v3, which will get rid of the multi-level choice statements, as requested by Arnd.

Sure, will do.

Geert Uytterhoeven

2014-11-10 08:01 UTC #7 lore raw

PATCH 1/1 ARM: shmobile: Add early debugging support using SCIF(A)

In reply to Arnd Bergmann

Quoted 15 lines · Arnd Bergmann

Quoted 15 lines
Quoted 11 lines
arch/arm/mach-shmobile/setup-r8a7779.c static struct map_desc r8a7779_io_desc[] __initdata = { · line 74
Quoted 8 lines
arch/arm/mach-shmobile/setup-r8a7779.c +7 -0
+     {
+             .virtual        = 0xffe40000,
+             .pfn            = __phys_to_pfn(0xffe40000),
+             .length         = SZ_64K,
+             .type           = MT_DEVICE_NONSHARED
+     },
+#endif /* CONFIG_DEBUG_R8A7779_SCIF2 */
 };
This should be done automatically if you call debug_ll_io_init() or have an empty map_io() callback.
On r8a7779 map_io() is not empty, but it didn't cover the SCIF registers, cfr. the commit log:

Quoted 6 lines

Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
4. On SoCs using the legacy machine_desc.map_io(), an identity mapping of the SCIF(A) registers must be set up in .map_io(). On all but r8a7779 this was already done.
Ok. You can of course just call debug_ll_io_init() manually from the map_io function, it will create the same mapping.

Good idea, that would indeed simpify the logic, both in Kconfig and renesas-scif.S.

I gave it a try. On Armadillo it worked, on kzm9g it didn't. Probably the virtual address choice is wrong.

As I can't test it on the other boards using map_io(), I'd like to keep this as-is
for now. We can migrate them gradually later.

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at 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

Simon Horman

2014-11-11 00:14 UTC #8 lore raw

PATCH 1/1 ARM: shmobile: Add early debugging support using SCIF(A)

Quoted 26 lines · Geert Uytterhoeven

Quoted 15 lines · Arnd Bergmann
Quoted 15 lines
Quoted 11 lines
arch/arm/mach-shmobile/setup-r8a7779.c static struct map_desc r8a7779_io_desc[] __initdata = { · line 74
Quoted 8 lines
arch/arm/mach-shmobile/setup-r8a7779.c +7 -0
+     {
+             .virtual        = 0xffe40000,
+             .pfn            = __phys_to_pfn(0xffe40000),
+             .length         = SZ_64K,
+             .type           = MT_DEVICE_NONSHARED
+     },
+#endif /* CONFIG_DEBUG_R8A7779_SCIF2 */
 };
This should be done automatically if you call debug_ll_io_init() or have an empty map_io() callback.
On r8a7779 map_io() is not empty, but it didn't cover the SCIF registers, cfr. the commit log:
Quoted 6 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
4. On SoCs using the legacy machine_desc.map_io(), an identity mapping of the SCIF(A) registers must be set up in .map_io(). On all but r8a7779 this was already done.
Ok. You can of course just call debug_ll_io_init() manually from the map_io function, it will create the same mapping.
Good idea, that would indeed simpify the logic, both in Kconfig and renesas-scif.S.

Quoted 6 lines

I gave it a try. On Armadillo it worked, on kzm9g it didn't. Probably the virtual address choice is wrong.
As I can't test it on the other boards using map_io(), I'd like to keep this as-is
for now. We can migrate them gradually later.

I am happy to assist with testing but I think your incremental approach is a good one: lets merge something that works and then improve on it.