Skip to contentSkip to conversation

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

RFC PATCH Superseded Geert Uytterhoeven 8 participants 18 messages Oct 2, 2014 – Oct 7, 2014

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

v1 of 4 Superseded Latest is v4 →

lore
Share & tools
lore

Geert Uytterhoeven

author maintainer
#0 lore raw

RFC 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.

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.

This was derived from the r8a7790 version by Laurent Pinchart.

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

---
Tested on r8a7791/koelsch and r8a7740/armadillo. Testing on other platforms would be highly appreciated.

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?

- While the serial console on r8a7790 was switched from SCIF0 to SCIFA0 recently, I believe r8a7990 still needs to use SCIF0 instead of SCIFA0, as that's what the bootloader has configured.

---
 arch/arm/Kconfig.debug                 | 20 +++++++++++-
 arch/arm/include/debug/scif.S          | 60 ++++++++++++++++++++++++++++++++++
 arch/arm/mach-shmobile/setup-r8a7779.c |  7 ++++
 3 files changed, 86 insertions(+), 1 deletion(-)
 create mode 100644 arch/arm/include/debug/scif.S
arch/arm/Kconfig.debug +19 -1
index b11ad54f8d17fa6b..58ace40b30938e58 100644
@@ -699,6 +699,14 @@ choice
 		  their output to UART 2. The port must have been initialised
 		  by the boot-loader before use.
 
+	config DEBUG_SCIF
+		bool "Kernel low-level debugging messages via SCIF(A) on Renesas ARM SoCs"
+		depends on ARCH_R7S72100 || ARCH_RCAR_GEN1 || ARCH_RCAR_GEN2 || ARCH_RMOBILE
+		help
+		  Say Y here if you want kernel low-level debugging support
+		  on Renesas ARM based platforms using SCIF(A) serial ports
+		  (SH-Mobile, R-Mobile, RZ/A1H, R-Car Gen1, R-Car Gen2).
+
 	config DEBUG_SOCFPGA_UART
 		depends on ARCH_SOCFPGA
 		bool "Use SOCFPGA UART for low-level debug"

@@ -1028,6 +1036,7 @@ config DEBUG_LL_INCLUDE
 				 DEBUG_IMX6SX_UART
 	default "debug/msm.S" if DEBUG_MSM_UART || DEBUG_QCOM_UARTDM
 	default "debug/omap2plus.S" if DEBUG_OMAP2PLUS_UART
+	default "debug/scif.S" if DEBUG_SCIF
 	default "debug/s3c24xx.S" if DEBUG_S3C24XX_UART
 	default "debug/s5pv210.S" if DEBUG_S5PV210_UART
 	default "debug/sirf.S" if DEBUG_SIRFPRIMA2_UART1 || DEBUG_SIRFMARCO_UART1

@@ -1109,6 +1118,12 @@ config DEBUG_UART_PHYS
 	default 0xd4017000 if DEBUG_MMP_UART2
 	default 0xd4018000 if DEBUG_MMP_UART3
 	default 0xe0000000 if ARCH_SPEAR13XX
+	default 0xe6c40000 if DEBUG_SCIF && (ARCH_R8A73A4 || ARCH_SH7372)
+	default 0xe6c50000 if DEBUG_SCIF && ARCH_R8A7740
+	default 0xe6c80000 if DEBUG_SCIF && ARCH_SH73A0
+	default 0xe6e58000 if DEBUG_SCIF && ARCH_R8A7794
+	default 0xe6e60000 if DEBUG_SCIF && (ARCH_R8A7790 || ARCH_R8A7791 || ARCH_R8A7793)
+	default 0xe8008000 if DEBUG_SCIF && ARCH_R7S72100
 	default 0xf0000be0 if ARCH_EBSA110
 	default 0xf1012000 if DEBUG_MVEBU_UART_ALTERNATE
 	default 0xf1012000 if ARCH_DOVE || ARCH_MV78XX0 || \

@@ -1121,12 +1136,15 @@ config DEBUG_UART_PHYS
 	default 0xff690000 if DEBUG_RK32_UART2
 	default 0xffc02000 if DEBUG_SOCFPGA_UART
 	default 0xffd82340 if ARCH_IOP13XX
+	default 0xffe40000 if DEBUG_SCIF && ARCH_R8A7778
+	default 0xffe42000 if DEBUG_SCIF && ARCH_R8A7779
 	default 0xfff36000 if DEBUG_HIGHBANK_UART
 	default 0xfffff700 if ARCH_IOP33X
 	depends on DEBUG_LL_UART_8250 || DEBUG_LL_UART_PL01X || \
 		DEBUG_LL_UART_EFM32 || \
 		DEBUG_UART_8250 || DEBUG_UART_PL01X || \
-		DEBUG_MSM_UART || DEBUG_QCOM_UARTDM || DEBUG_S3C24XX_UART
+		DEBUG_MSM_UART || DEBUG_QCOM_UARTDM || DEBUG_S3C24XX_UART || \
+		DEBUG_SCIF
 
 config DEBUG_UART_VIRT
 	hex "Virtual base address of debug UART"
arch/arm/include/debug/scif.S new file +60 -0

60 lines not on this page · raw message

arch/arm/mach-shmobile/setup-r8a7779.c +7 -0
index 136078ab9407cc2d..24c49f7121d69d28 100644
@@ -66,6 +66,13 @@ static struct map_desc r8a7779_io_desc[] __initdata = {
 		.length		= SZ_16M,
 		.type		= MT_DEVICE_NONSHARED
 	},
+	/* 64K entity map for 0xffe40000 (SCIF0/1) */
+	{
+		.virtual	= 0xffe40000,
+		.pfn		= __phys_to_pfn(0xffe40000),
+		.length		= SZ_64K,
+		.type		= MT_DEVICE_NONSHARED
+	},
 };
 
 void __init r8a7779_map_io(void)
Signature · 1 line
1.9.1
#1 lore raw

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

Hi Geert,

Quoted 12 lines · Geert Uytterhoeven

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

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.

Quoted 6 lines

This was derived from the r8a7790 version by Laurent Pinchart.

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

---
Tested on r8a7791/koelsch and r8a7740/armadillo. Testing on other platforms would be highly appreciated.

Uhm..., I have tried to test your patch with devel branch/renesas repo.
1. CONFIG_DEBUG_SCIF is enabled in shmobile_defconfig.
2. *Need to configure* CONFIG_DEBUG_UART_PHYS for lager/koelsch since 0xe6c50000 (for ARCH_R8A7740) will be selected by default.

Diff of config changed:
-# CONFIG_DEBUG_LL is not set
-CONFIG_DEBUG_LL_INCLUDE="mach/debug-macro.S"
+CONFIG_DEBUG_LL=y
+CONFIG_DEBUG_SCIF=y
+# CONFIG_DEBUG_ICEDCC is not set
+# CONFIG_DEBUG_SEMIHOSTING is not set
+# CONFIG_DEBUG_LL_UART_8250 is not set
+# CONFIG_DEBUG_LL_UART_PL01X is not set
+CONFIG_DEBUG_LL_INCLUDE="debug/scif.S"
# CONFIG_DEBUG_UART_PL01X is not set
# CONFIG_DEBUG_UART_8250 is not set
+CONFIG_DEBUG_UART_PHYS=0xe6e60000
+CONFIG_DEBUG_UNCOMPRESS=y
CONFIG_UNCOMPRESS_INCLUDE="debug/uncompress.h"
+# CONFIG_EARLY_PRINTK is not set
# CONFIG_PID_IN_CONTEXTIDR is not set

3. Boot up in lager/koelsch, kernel log output
to "booting the kernel" then ... nothing else.

Starting kernel ...

Uncompressing Linux... done, booting the kernel.

Could you let me know your test procedure ?

Quoted 169 lines

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?

- While the serial console on r8a7790 was switched from SCIF0 to SCIFA0 recently, I believe r8a7990 still needs to use SCIF0 instead of SCIFA0, as that's what the bootloader has configured.

---
 arch/arm/Kconfig.debug                 | 20 +++++++++++-
 arch/arm/include/debug/scif.S          | 60 ++++++++++++++++++++++++++++++++++
 arch/arm/mach-shmobile/setup-r8a7779.c |  7 ++++
 3 files changed, 86 insertions(+), 1 deletion(-)
 create mode 100644 arch/arm/include/debug/scif.S
arch/arm/Kconfig.debug +19 -1
index b11ad54f8d17fa6b..58ace40b30938e58 100644
@@ -699,6 +699,14 @@ choice
 		  their output to UART 2. The port must have been initialised
 		  by the boot-loader before use.
 
+	config DEBUG_SCIF
+		bool "Kernel low-level debugging messages via SCIF(A) on Renesas ARM SoCs"
+		depends on ARCH_R7S72100 || ARCH_RCAR_GEN1 || ARCH_RCAR_GEN2 || ARCH_RMOBILE
+		help
+		  Say Y here if you want kernel low-level debugging support
+		  on Renesas ARM based platforms using SCIF(A) serial ports
+		  (SH-Mobile, R-Mobile, RZ/A1H, R-Car Gen1, R-Car Gen2).
+
 	config DEBUG_SOCFPGA_UART
 		depends on ARCH_SOCFPGA
 		bool "Use SOCFPGA UART for low-level debug"

@@ -1028,6 +1036,7 @@ config DEBUG_LL_INCLUDE
 				 DEBUG_IMX6SX_UART
 	default "debug/msm.S" if DEBUG_MSM_UART || DEBUG_QCOM_UARTDM
 	default "debug/omap2plus.S" if DEBUG_OMAP2PLUS_UART
+	default "debug/scif.S" if DEBUG_SCIF
 	default "debug/s3c24xx.S" if DEBUG_S3C24XX_UART
 	default "debug/s5pv210.S" if DEBUG_S5PV210_UART
 	default "debug/sirf.S" if DEBUG_SIRFPRIMA2_UART1 || DEBUG_SIRFMARCO_UART1

@@ -1109,6 +1118,12 @@ config DEBUG_UART_PHYS
 	default 0xd4017000 if DEBUG_MMP_UART2
 	default 0xd4018000 if DEBUG_MMP_UART3
 	default 0xe0000000 if ARCH_SPEAR13XX
+	default 0xe6c40000 if DEBUG_SCIF && (ARCH_R8A73A4 || ARCH_SH7372)
+	default 0xe6c50000 if DEBUG_SCIF && ARCH_R8A7740
+	default 0xe6c80000 if DEBUG_SCIF && ARCH_SH73A0
+	default 0xe6e58000 if DEBUG_SCIF && ARCH_R8A7794
+	default 0xe6e60000 if DEBUG_SCIF && (ARCH_R8A7790 || ARCH_R8A7791 || ARCH_R8A7793)
+	default 0xe8008000 if DEBUG_SCIF && ARCH_R7S72100
 	default 0xf0000be0 if ARCH_EBSA110
 	default 0xf1012000 if DEBUG_MVEBU_UART_ALTERNATE
 	default 0xf1012000 if ARCH_DOVE || ARCH_MV78XX0 || \

@@ -1121,12 +1136,15 @@ config DEBUG_UART_PHYS
 	default 0xff690000 if DEBUG_RK32_UART2
 	default 0xffc02000 if DEBUG_SOCFPGA_UART
 	default 0xffd82340 if ARCH_IOP13XX
+	default 0xffe40000 if DEBUG_SCIF && ARCH_R8A7778
+	default 0xffe42000 if DEBUG_SCIF && ARCH_R8A7779
 	default 0xfff36000 if DEBUG_HIGHBANK_UART
 	default 0xfffff700 if ARCH_IOP33X
 	depends on DEBUG_LL_UART_8250 || DEBUG_LL_UART_PL01X || \
 		DEBUG_LL_UART_EFM32 || \
 		DEBUG_UART_8250 || DEBUG_UART_PL01X || \
-		DEBUG_MSM_UART || DEBUG_QCOM_UARTDM || DEBUG_S3C24XX_UART
+		DEBUG_MSM_UART || DEBUG_QCOM_UARTDM || DEBUG_S3C24XX_UART || \
+		DEBUG_SCIF
 
 config DEBUG_UART_VIRT
 	hex "Virtual base address of debug UART"
arch/arm/include/debug/scif.S new file +60 -0
new file mode 100644
index 0000000000000000..9c9b9b2c17f97b4a
@@ -0,0 +1,60 @@
+/*
+ * Renesas SCIF(A) debugging macro include header
+ *
+ * Based on r8a7790.S
+ *
+ * Copyright (C) 2012-2013 Renesas Electronics Corporation
+ * Copyright (C) 1994-1999 Russell King
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#if CONFIG_DEBUG_UART_PHYS < 0xe6e00000
+/* SCIFA */
+#define FTDR		0x20
+#define FSR		0x14
+#else
+/* SCIF */
+#define FTDR		0x0c
+#define FSR		0x10
+#endif
+
+#define SCIF_PHYS	CONFIG_DEBUG_UART_PHYS
+
+#if defined(CONFIG_ARCH_R8A7740) || defined(CONFIG_ARCH_R8A7779) || \
+    defined(CONFIG_ARCH_SH7372) || defined(CONFIG_ARCH_SH73A0)
+/* Legacy machine_desc.map_io() sets up an identity mapping for us */
+#define SCIF_VIRT	SCIF_PHYS
+#else
+/* Relying on debug_ll_io_init(), so we have to select a free gap ourselves */
+#define SCIF_VIRT	((SCIF_PHYS & 0x00ffffff) | 0xfd000000)
+#endif
+
+#define TDFE	(1 << 5)
+#define TEND	(1 << 6)
+
+	.macro	addruart, rp, rv, tmp
+	ldr	\rp, =SCIF_PHYS
+	ldr	\rv, =SCIF_VIRT
+	.endm
+
+	.macro	waituart, rd, rx
+1001:	ldrh	\rd, [\rx, #FSR]
+	tst	\rd, #TDFE
+	beq	1001b
+	.endm
+
+	.macro	senduart, rd, rx
+	strb	\rd, [\rx, #FTDR]
+	ldrh	\rd, [\rx, #FSR]
+	bic	\rd, \rd, #TEND
+	strh	\rd, [\rx, #FSR]
+	.endm
+
+	.macro	busyuart, rd, rx
+1001:	ldrh	\rd, [\rx, #FSR]
+	tst	\rd, #TEND
+	beq	1001b
+	.endm
arch/arm/mach-shmobile/setup-r8a7779.c +7 -0
index 136078ab9407cc2d..24c49f7121d69d28 100644
@@ -66,6 +66,13 @@ static struct map_desc r8a7779_io_desc[] __initdata = {
 		.length		= SZ_16M,
 		.type		= MT_DEVICE_NONSHARED
 	},
+	/* 64K entity map for 0xffe40000 (SCIF0/1) */
+	{
+		.virtual	= 0xffe40000,
+		.pfn		= __phys_to_pfn(0xffe40000),
+		.length		= SZ_64K,
+		.type		= MT_DEVICE_NONSHARED
+	},
 };
 
 void __init r8a7779_map_io(void)
Signature · 2 lines
Best regards,
KHIEM Nguyen
#2 lore raw

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

Hi Khiem-san,

Quoted 20 lines · Khiem Nguyen
Quoted 12 lines · Geert Uytterhoeven

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

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.

Quoted 6 lines

This was derived from the r8a7790 version by Laurent Pinchart.

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

---
Tested on r8a7791/koelsch and r8a7740/armadillo. Testing on other platforms would be highly appreciated.

Quoted 4 lines

Uhm..., I have tried to test your patch with devel branch/renesas repo.
1. CONFIG_DEBUG_SCIF is enabled in shmobile_defconfig.
2. *Need to configure* CONFIG_DEBUG_UART_PHYS for lager/koelsch since 0xe6c50000 (for ARCH_R8A7740) will be selected by default.

DEBUG_LL is meant for low-level debugging, and may not work well if multiple platforms/SoCs are enabled.
If your kernel only has support for r8a7790/r8a7791, the default value of CONFIG_DEBUG_UART_PHYS should be OK.
Else, ... well, you already noticed you can still change the address when configuring.

Quoted 18 lines

Diff of config changed:

(quoted diff) +10 -2
-# CONFIG_DEBUG_LL is not set
-CONFIG_DEBUG_LL_INCLUDE="mach/debug-macro.S"
+CONFIG_DEBUG_LL=y
+CONFIG_DEBUG_SCIF=y
+# CONFIG_DEBUG_ICEDCC is not set
+# CONFIG_DEBUG_SEMIHOSTING is not set
+# CONFIG_DEBUG_LL_UART_8250 is not set
+# CONFIG_DEBUG_LL_UART_PL01X is not set
+CONFIG_DEBUG_LL_INCLUDE="debug/scif.S"
 # CONFIG_DEBUG_UART_PL01X is not set
 # CONFIG_DEBUG_UART_8250 is not set
+CONFIG_DEBUG_UART_PHYS=0xe6e60000
+CONFIG_DEBUG_UNCOMPRESS=y
 CONFIG_UNCOMPRESS_INCLUDE="debug/uncompress.h"
+# CONFIG_EARLY_PRINTK is not set
 # CONFIG_PID_IN_CONTEXTIDR is not set
Quoted 8 lines

3. Boot up in lager/koelsch, kernel log output
to "booting the kernel" then ... nothing else.

Starting kernel ...

Uncompressing Linux... done, booting the kernel.

Could you let me know your test procedure ?

Does enable CONFIG_EARLY_PRINTK help?

Thanks for testing!

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

#3 lore raw

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

Hi Geert,

Quoted 57 lines · Geert Uytterhoeven

Hi Khiem-san,

Quoted 20 lines · Khiem Nguyen
Quoted 12 lines · Geert Uytterhoeven

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

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.

Quoted 6 lines

This was derived from the r8a7790 version by Laurent Pinchart.

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

---
Tested on r8a7791/koelsch and r8a7740/armadillo. Testing on other platforms would be highly appreciated.

Quoted 4 lines

Uhm..., I have tried to test your patch with devel branch/renesas repo.
1. CONFIG_DEBUG_SCIF is enabled in shmobile_defconfig.
2. *Need to configure* CONFIG_DEBUG_UART_PHYS for lager/koelsch since 0xe6c50000 (for ARCH_R8A7740) will be selected by default.

DEBUG_LL is meant for low-level debugging, and may not work well if multiple platforms/SoCs are enabled.
If your kernel only has support for r8a7790/r8a7791, the default value of CONFIG_DEBUG_UART_PHYS should be OK.
Else, ... well, you already noticed you can still change the address when configuring.

Quoted 21 lines

Diff of config changed:

(quoted diff) +10 -2
-# CONFIG_DEBUG_LL is not set
-CONFIG_DEBUG_LL_INCLUDE="mach/debug-macro.S"
+CONFIG_DEBUG_LL=y
+CONFIG_DEBUG_SCIF=y
+# CONFIG_DEBUG_ICEDCC is not set
+# CONFIG_DEBUG_SEMIHOSTING is not set
+# CONFIG_DEBUG_LL_UART_8250 is not set
+# CONFIG_DEBUG_LL_UART_PL01X is not set
+CONFIG_DEBUG_LL_INCLUDE="debug/scif.S"
 # CONFIG_DEBUG_UART_PL01X is not set
 # CONFIG_DEBUG_UART_8250 is not set
+CONFIG_DEBUG_UART_PHYS=0xe6e60000
+CONFIG_DEBUG_UNCOMPRESS=y
 CONFIG_UNCOMPRESS_INCLUDE="debug/uncompress.h"
+# CONFIG_EARLY_PRINTK is not set
 # CONFIG_PID_IN_CONTEXTIDR is not set

3. Boot up in lager/koelsch, kernel log output
to "booting the kernel" then ... nothing else.

Quoted 7 lines
Quoted 5 lines

Starting kernel ...

Uncompressing Linux... done, booting the kernel.

Could you let me know your test procedure ?

Does enable CONFIG_EARLY_PRINTK help?

For multi-platform defconfig, it does not help.

However, I have also tried with legacy defconfig (lager_defconfig), kernel boot is OK.

As legacy defconfig will become deprecated in near future, I think this patch should support multi-platform defconfig.

Quoted 1 line

Thanks for testing!

No problem.

Quoted 11 lines

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

Signature · 2 lines
Best regards,
KHIEM Nguyen
#4 lore raw

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

Hi Khiem-san,

Quoted 3 lines · Khiem Nguyen
Quoted 1 line

Does enable CONFIG_EARLY_PRINTK help?

For multi-platform defconfig, it does not help.

Make sure to also disable CONFIG_ARCH_R8A7740 and CONFIG_ARCH_R8A7779, else arch/arm/include/debug/scif.S will use a wrong value for SCIF_VIRT.

As there's no runtime configuration, enabling support for multiple SoCs that need conflicting parameters may not work.

I've just verified that shmobile_defconfig + CONFIG_DEBUG_LL works as-is for armadillo.
For r8a791/koelsch, I had to disable r8a7740 and r8a7789 support, and either correct DEBUG_UART_PHYS manually, or also disable r8a7794 support.

Quoted 2 lines

However, I have also tried with legacy defconfig (lager_defconfig), kernel boot is OK.

Thanks!

Quoted 2 lines

As legacy defconfig will become deprecated in near future, I think this patch should support multi-platform defconfig.

CONFIG_DEBUG_LL is not to be enabled in any multi-platform defconfig. It's only meant for desperate developers, running a kernel for one specific platform. Cfr. its help text:

config DEBUG_LL
bool "Kernel low-level debugging functions (read help!)" depends on DEBUG_KERNEL
help
Say Y here to include definitions of printascii, printch, printhex in the kernel. This is helpful if you are debugging code that executes before the console is initialized.

Note that selecting this option will limit the kernel to a single UART definition, as specified below. Attempting to boot the kernel image on a different platform *will not work*, so this option should not be enabled for kernels that are intended to be portable.

All defconfigs enabling CONFIG_DEBUG_LL are defconfigs for legacy platforms.
The only exceptions are bcm2835_defconfig, moxart_defconfig, s5pv210_defconfig, and tegra_defconfig, which support only one platform, or a family of very similar platforms.

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

#5 lore raw

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

In reply to Geert Uytterhoeven

Hi Geert,

Thank you for the patch.

It looks like you've beaten Ian Molton (CC'ed) on this, he was planning to send exactly the same thing.

Quoted 19 lines · On Thursday 02 October 2014 21:02:43 Geert Uytterhoeven wrote:

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

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.

This was derived from the r8a7790 version by Laurent Pinchart.

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

---
Tested on r8a7791/koelsch and r8a7740/armadillo. Testing on other platforms would be highly appreciated.

Quoted 15 lines

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?

Do you think it be possible to enable the required clock dynamically in setup code when CONFIG_DEBUG_LL is enabled ?

Quoted 3 lines

- While the serial console on r8a7790 was switched from SCIF0 to SCIFA0 recently, I believe r8a7990 still needs to use SCIF0 instead of SCIFA0, as that's what the bootloader has configured.

Good point. Wolfram, any opinion about this ?

Quoted 12 lines
---
 arch/arm/Kconfig.debug                 | 20 +++++++++++-
 arch/arm/include/debug/scif.S          | 60 +++++++++++++++++++++++++++++++
 arch/arm/mach-shmobile/setup-r8a7779.c |  7 ++++
 3 files changed, 86 insertions(+), 1 deletion(-)
 create mode 100644 arch/arm/include/debug/scif.S
arch/arm/Kconfig.debug +0 -0
index b11ad54f8d17fa6b..58ace40b30938e58 100644

arch/arm/Kconfig.debug choice · line 702

Quoted 4 lines
arch/arm/Kconfig.debug +1 -0
@@ -699,6 +699,14 @@ choice
 		  their output to UART 2. The port must have been initialised
 		  by the boot-loader before use.
 
+	config DEBUG_SCIF

How about calling this DEBUG_RENESAS_SCIF ? Other platforms use the platform name instead of the serial IP core name.

Quoted 13 lines
arch/arm/Kconfig.debug +1 -0
+		bool "Kernel low-level debugging messages via SCIF(A) on Renesas ARM

SoCs"

arch/arm/Kconfig.debug +1 -0
+		depends on ARCH_R7S72100 || ARCH_RCAR_GEN1 || ARCH_RCAR_GEN2 ||

ARCH_RMOBILE

arch/arm/Kconfig.debug +5 -0
+		help
+		  Say Y here if you want kernel low-level debugging support
+		  on Renesas ARM based platforms using SCIF(A) serial ports
+		  (SH-Mobile, R-Mobile, RZ/A1H, R-Car Gen1, R-Car Gen2).
+
 	config DEBUG_SOCFPGA_UART
 		depends on ARCH_SOCFPGA
 		bool "Use SOCFPGA UART for low-level debug"

arch/arm/Kconfig.debug config DEBUG_LL_INCLUDE · line 1039

Quoted 4 lines
arch/arm/Kconfig.debug +1 -0
@@ -1028,6 +1036,7 @@ config DEBUG_LL_INCLUDE
 				 DEBUG_IMX6SX_UART
 	default "debug/msm.S" if DEBUG_MSM_UART || DEBUG_QCOM_UARTDM
 	default "debug/omap2plus.S" if DEBUG_OMAP2PLUS_UART
+	default "debug/scif.S" if DEBUG_SCIF

Similarly, renesas-scif.S ?

Quoted 39 lines
arch/arm/Kconfig.debug +5 -0
 	default "debug/s3c24xx.S" if DEBUG_S3C24XX_UART
 	default "debug/s5pv210.S" if DEBUG_S5PV210_UART
 	default "debug/sirf.S" if DEBUG_SIRFPRIMA2_UART1 || DEBUG_SIRFMARCO_UART1

@@ -1109,6 +1118,12 @@ config DEBUG_UART_PHYS
 	default 0xd4017000 if DEBUG_MMP_UART2
 	default 0xd4018000 if DEBUG_MMP_UART3
 	default 0xe0000000 if ARCH_SPEAR13XX
+	default 0xe6c40000 if DEBUG_SCIF && (ARCH_R8A73A4 || ARCH_SH7372)
+	default 0xe6c50000 if DEBUG_SCIF && ARCH_R8A7740
+	default 0xe6c80000 if DEBUG_SCIF && ARCH_SH73A0
+	default 0xe6e58000 if DEBUG_SCIF && ARCH_R8A7794
+	default 0xe6e60000 if DEBUG_SCIF && (ARCH_R8A7790 || ARCH_R8A7791 ||

ARCH_R8A7793) + default 0xe8008000 if DEBUG_SCIF && ARCH_R7S72100

arch/arm/Kconfig.debug +4 -1
 	default 0xf0000be0 if ARCH_EBSA110
 	default 0xf1012000 if DEBUG_MVEBU_UART_ALTERNATE
 	default 0xf1012000 if ARCH_DOVE || ARCH_MV78XX0 || \

@@ -1121,12 +1136,15 @@ config DEBUG_UART_PHYS
 	default 0xff690000 if DEBUG_RK32_UART2
 	default 0xffc02000 if DEBUG_SOCFPGA_UART
 	default 0xffd82340 if ARCH_IOP13XX
+	default 0xffe40000 if DEBUG_SCIF && ARCH_R8A7778
+	default 0xffe42000 if DEBUG_SCIF && ARCH_R8A7779
 	default 0xfff36000 if DEBUG_HIGHBANK_UART
 	default 0xfffff700 if ARCH_IOP33X
 	depends on DEBUG_LL_UART_8250 || DEBUG_LL_UART_PL01X || \
 		DEBUG_LL_UART_EFM32 || \
 		DEBUG_UART_8250 || DEBUG_UART_PL01X || \
-		DEBUG_MSM_UART || DEBUG_QCOM_UARTDM || DEBUG_S3C24XX_UART
+		DEBUG_MSM_UART || DEBUG_QCOM_UARTDM || DEBUG_S3C24XX_UART || \
+		DEBUG_SCIF
 
 config DEBUG_UART_VIRT
 	hex "Virtual base address of debug UART"
arch/arm/include/debug/scif.S new file +0 -0
new file mode 100644
index 0000000000000000..9c9b9b2c17f97b4a

arch/arm/include/debug/scif.S line 6

Quoted 6 lines
arch/arm/include/debug/scif.S new file +6 -0
@@ -0,0 +1,60 @@
+/*
+ * Renesas SCIF(A) debugging macro include header
+ *
+ * Based on r8a7790.S
+ *
+ * Copyright (C) 2012-2013 Renesas Electronics Corporation

2014 ?

Quoted 61 lines
arch/arm/include/debug/scif.S new file +25 -0
+ * Copyright (C) 1994-1999 Russell King
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#if CONFIG_DEBUG_UART_PHYS < 0xe6e00000
+/* SCIFA */
+#define FTDR		0x20
+#define FSR		0x14
+#else
+/* SCIF */
+#define FTDR		0x0c
+#define FSR		0x10
+#endif
+
+#define SCIF_PHYS	CONFIG_DEBUG_UART_PHYS
+
+#if defined(CONFIG_ARCH_R8A7740) || defined(CONFIG_ARCH_R8A7779) || \
+    defined(CONFIG_ARCH_SH7372) || defined(CONFIG_ARCH_SH73A0)
+/* Legacy machine_desc.map_io() sets up an identity mapping for us */
+#define SCIF_VIRT	SCIF_PHYS
+#else
+/* Relying on debug_ll_io_init(), so we have to select a free gap ourselves

*/

arch/arm/include/debug/scif.S new file +29 -0
+#define SCIF_VIRT	((SCIF_PHYS & 0x00ffffff) | 0xfd000000)
+#endif
+
+#define TDFE	(1 << 5)
+#define TEND	(1 << 6)
+
+	.macro	addruart, rp, rv, tmp
+	ldr	\rp, =SCIF_PHYS
+	ldr	\rv, =SCIF_VIRT
+	.endm
+
+	.macro	waituart, rd, rx
+1001:	ldrh	\rd, [\rx, #FSR]
+	tst	\rd, #TDFE
+	beq	1001b
+	.endm
+
+	.macro	senduart, rd, rx
+	strb	\rd, [\rx, #FTDR]
+	ldrh	\rd, [\rx, #FSR]
+	bic	\rd, \rd, #TEND
+	strh	\rd, [\rx, #FSR]
+	.endm
+
+	.macro	busyuart, rd, rx
+1001:	ldrh	\rd, [\rx, #FSR]
+	tst	\rd, #TEND
+	beq	1001b
+	.endm
(quoted diff) +0 -0

b/arch/arm/mach-shmobile/setup-r8a7779.c index
136078ab9407cc2d..24c49f7121d69d28 100644

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

Quoted 10 lines
arch/arm/mach-shmobile/setup-r8a7779.c +7 -0
@@ -66,6 +66,13 @@ static struct map_desc r8a7779_io_desc[] __initdata = {
 		.length		= SZ_16M,
 		.type		= MT_DEVICE_NONSHARED
 	},
+	/* 64K entity map for 0xffe40000 (SCIF0/1) */
+	{
+		.virtual	= 0xffe40000,
+		.pfn		= __phys_to_pfn(0xffe40000),
+		.length		= SZ_64K,
+		.type		= MT_DEVICE_NONSHARED
+	},

Should we guard that with #ifdef CONFIG_DEBUG_LL ?

Quoted 3 lines
arch/arm/mach-shmobile/setup-r8a7779.c +0 -0
 };
 
 void __init r8a7779_map_io(void)
Signature · 3 lines
Regards,

Laurent Pinchart
#6 lore raw

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

Quoted 6 lines
Quoted 4 lines

- While the serial console on r8a7790 was switched from SCIF0 to SCIFA0 recently, I believe r8a7990 still needs to use SCIF0 instead of SCIFA0, as that's what the bootloader has configured.

Good point. Wolfram, any opinion about this ?

IMO we should only use SCIFA if it uses the same wires as SCIF. This is the case for Lager (7790). Dunno about other boards.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20141006/7c2abe52/attachment.sig>

maintainer
#7 lore raw

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

Hi Wolfram,

Quoted 11 lines · Wolfram Sang
Quoted 6 lines
Quoted 4 lines

- While the serial console on r8a7790 was switched from SCIF0 to SCIFA0 recently, I believe r8a7990 still needs to use SCIF0 instead of SCIFA0, as that's what the bootloader has configured.

Good point. Wolfram, any opinion about this ?

IMO we should only use SCIFA if it uses the same wires as SCIF. This is the case for Lager (7790). Dunno about other boards.

From my side I think we should let our users decide for themselves. So

to me it would make sense to be able to assign various devices like UART, SPI and I2C to whatever pins that the user wants during run time, and use DT to point out what the hardware possibilities look like. Right now we describe one setting in DT, and it smells like a software policy to me. =)

Cheers,

/ magnus

#8 lore raw

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

In reply to Wolfram Sang

Hi Wolfram,

Quoted 8 lines · Wolfram Sang
Quoted 5 lines
Quoted 3 lines

- While the serial console on r8a7790 was switched from SCIF0 to SCIFA0 recently, I believe r8a7990 still needs to use SCIF0 instead of SCIFA0, as that's what the bootloader has configured.

Good point. Wolfram, any opinion about this ?

IMO we should only use SCIFA if it uses the same wires as SCIF. This is the case for Lager (7790). Dunno about other boards.

I agree with your statement about using the same wires. However, having the ability to pinmux both SCIF0 and SCIFA0 to the same wires is not a board property, but an SoC property, right?

Nevertheless, for DEBUG_LL, this is early debug output, so it should use the exact same settings as the bootloader, which has configured SCIF0, and
(b) has setup pinmux for SCIF0. Pinmux setup for SCIFA0 only kicks in when the real SCIFA1 is initialized later.

Please correct me if I'm wrong.

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

#9 lore raw

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

Quoted 4 lines

I agree with your statement about using the same wires. However, having the ability to pinmux both SCIF0 and SCIFA0 to the same wires is not a board property, but an SoC property, right?

Both. The SoC needs to support muxing them to the same wires, yes. But the board must also support it. E.g. SCIFA0 could already be used using some other wires, then it is not possible anymore.

Quoted 3 lines

Nevertheless, for DEBUG_LL, this is early debug output, so it should use the exact same settings as the bootloader, which has configured SCIF0, and
(b) has setup pinmux for SCIF0.

True.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20141006/27f7c7f5/attachment.sig>

#10 lore raw

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

In reply to Laurent Pinchart

Hi Laurent,

Quoted 2 lines · Laurent Pinchart

It looks like you've beaten Ian Molton (CC'ed) on this, he was planning to send exactly the same thing.

Sorry, I desperately needed this for r8a7740 :-)

Quoted 16 lines · On Thursday 02 October 2014 21:02:43 Geert Uytterhoeven wrote:
Quoted 6 lines

- 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.

Quoted 8 lines

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?

Quoted 2 lines

Do you think it be possible to enable the required clock dynamically in setup code when CONFIG_DEBUG_LL is enabled ?

It's definitely possibly, but IMHO that would add complexity.

Quoted 8 lines
Quoted 8 lines
arch/arm/Kconfig.debug +0 -0
index b11ad54f8d17fa6b..58ace40b30938e58 100644
@@ -699,6 +699,14 @@ choice
                their output to UART 2. The port must have been initialised
                by the boot-loader before use.
Quoted 4 lines

arch/arm/Kconfig.debug choice · line 702

Quoted 1 line
arch/arm/Kconfig.debug +1 -0
+     config DEBUG_SCIF

How about calling this DEBUG_RENESAS_SCIF ? Other platforms use the platform name instead of the serial IP core name.

Good point. Will fix.

Quoted 7 lines

arch/arm/Kconfig.debug config DEBUG_LL_INCLUDE · line 1039

Quoted 5 lines
arch/arm/Kconfig.debug +1 -0
@@ -1028,6 +1036,7 @@ config DEBUG_LL_INCLUDE
                               DEBUG_IMX6SX_UART
      default "debug/msm.S" if DEBUG_MSM_UART || DEBUG_QCOM_UARTDM
      default "debug/omap2plus.S" if DEBUG_OMAP2PLUS_UART
+     default "debug/scif.S" if DEBUG_SCIF

Similarly, renesas-scif.S ?

OK.

Quoted 11 lines

arch/arm/include/debug/scif.S line 6

Quoted 9 lines
arch/arm/include/debug/scif.S new file +6 -0
@@ -0,0 +1,60 @@
+/*
+ * Renesas SCIF(A) debugging macro include header
+ *
+ * Based on r8a7790.S
+ *
+ * Copyright (C) 2012-2013 Renesas Electronics Corporation

2014 ?

Based on the date I got r8a7790.S from you, I don't think anyone from Renesas touched it in 2014.

Quoted 7 lines
Quoted 7 lines
arch/arm/mach-shmobile/setup-r8a7779.c +1 -0
@@ -66,6 +66,13 @@ static struct map_desc r8a7779_io_desc[] __initdata = {
              .length         = SZ_16M,
              .type           = MT_DEVICE_NONSHARED
      },
+     /* 64K entity map for 0xffe40000 (SCIF0/1) */
Quoted 8 lines

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

Quoted 6 lines
arch/arm/mach-shmobile/setup-r8a7779.c +6 -0
+     {
+             .virtual        = 0xffe40000,
+             .pfn            = __phys_to_pfn(0xffe40000),
+             .length         = SZ_64K,
+             .type           = MT_DEVICE_NONSHARED
+     },

Should we guard that with #ifdef CONFIG_DEBUG_LL ?

Better, using CONFIG_DEBUG_RENESAS_SCIF.

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

#11 lore raw

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

In reply to Laurent Pinchart

To be fair, my patches were in last month, and awaiting a chance to test on Rcar-H1...

Can we share credit (Signed-off-by:) on the core, lager, 7779, and Kconfig patches please?

Cheers guys :)

(Its been a crap year for me, I've had almost zero chance to mainline anything :( )

-Ian

Quoted 15 lines · On Mon, 06 Oct 2014 01:18 +0300 Laurent Pinchart…

Hi Geert,

Thank you for the patch.

It looks like you've beaten Ian Molton (CC'ed) on this, he was planning to send exactly the same thing.

Quoted 7 lines · On Thursday 02 October 2014 21:02:43 Geert Uytterhoeven wrote:

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

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.

Signature · 1 line
Ian Molton <[email protected]>
#12 lore raw

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

Hi Ian,

Quoted 3 lines · Ian Molton

To be fair, my patches were in last month, and awaiting a chance to test on Rcar-H1...

Can we share credit (Signed-off-by:) on the core, lager, 7779, and Kconfig patches please?

I never saw your patches, and I can't seem to find them? I started from Laurent's version at
http://lists.infradead.org/pipermail/linux-arm-kernel/2013-October/207939.html

Confused...

Quoted 16 lines

Cheers guys :)

(Its been a crap year for me, I've had almost zero chance to mainline anything :( )

-Ian

Quoted 6 lines · On Mon, 06 Oct 2014 01:18 +0300 Laurent Pinchart…

Hi Geert,

Thank you for the patch.

It looks like you've beaten Ian Molton (CC'ed) on this, he was planning to send exactly the same thing.

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

#13 lore raw

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

Quoted 10 lines · Geert Uytterhoeven
Quoted 3 lines · Ian Molton

To be fair, my patches were in last month, and awaiting a chance to test on Rcar-H1...

Can we share credit (Signed-off-by:) on the core, lager, 7779, and Kconfig patches please?

I never saw your patches, and I can't seem to find them? I started from Laurent's version at
http://lists.infradead.org/pipermail/linux-arm-kernel/2013-October/207939.html

Confused...

Ah, found them in http://git.codethink.co.uk/linux.git/log/?h=ian/for_laurent

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

maintainer
#14 lore raw

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

In reply to Geert Uytterhoeven

Hi Geert,

Thanks for your patch! Quick comment from my side below:

Quoted 19 lines · Geert Uytterhoeven

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

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.

This was derived from the r8a7790 version by Laurent Pinchart.

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

---
Tested on r8a7791/koelsch and r8a7740/armadillo. Testing on other platforms would be highly appreciated.

Quoted 15 lines

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?

I've seen this issue on several SoCs actually, and as you describe the main issue seems to be that there are clock topologies that present for devices used during boot (like serial) but this clock topology is not yet managed by software. This does not trigger on all SoCs though.

It is my opinion that it has very little todo with the other devices that happen to be using the same parent clock. The legacy clock framework solved this by not allowing clocks to be disabled until all devices were registered.

Cheers,

/ magnus

#15 lore raw

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

Hi Magnus,

Quoted 15 lines · Magnus Damm
Quoted 6 lines

- 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.

Quoted 8 lines

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?

Quoted 4 lines

I've seen this issue on several SoCs actually, and as you describe the main issue seems to be that there are clock topologies that present for devices used during boot (like serial) but this clock topology is not yet managed by software. This does not trigger on all SoCs though.

To work perfectly, (a) everything must be described correctly in DT, and
(b) this information must be used correctly.
IMHO we can live with the deficiencies for DEBUG_LL.

Quoted 4 lines

It is my opinion that it has very little todo with the other devices that happen to be using the same parent clock. The legacy clock framework solved this by not allowing clocks to be disabled until all devices were registered.

Not allowing clocks to be disabled until all devices were registered has its own problems (some hardware requires disabling clocks for various reasons).

A similar horse is currently being beaten to death in the "simplefb" threads... Read on if you feel bored ;-)

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

#16 lore raw

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

In reply to Geert Uytterhoeven

arch/arm/Kconfig.debug config DEBUG_UART_PHYS · line 1126

Quoted 6 lines · On Thursday 02 October 2014 21:02:43 Geert Uytterhoeven wrote:
arch/arm/Kconfig.debug +6 -0
+       default 0xe6c40000 if DEBUG_SCIF && (ARCH_R8A73A4 || ARCH_SH7372)
+       default 0xe6c50000 if DEBUG_SCIF && ARCH_R8A7740
+       default 0xe6c80000 if DEBUG_SCIF && ARCH_SH73A0
+       default 0xe6e58000 if DEBUG_SCIF && ARCH_R8A7794
+       default 0xe6e60000 if DEBUG_SCIF && (ARCH_R8A7790 || ARCH_R8A7791 || ARCH_R8A7793)
+       default 0xe8008000 if DEBUG_SCIF && ARCH_R7S72100

Just saw this patch. Note that most other entries do not depend on a particular SoC but instead on one of the Kconfig entries, and I would like to eventually change all of them to be the same way.

The main reason for this is that your way is a mix of two methods: if you build a kernel for just one SoC you get the right entry here, but if you build the kernel for multiple SoCs combined, you have to edit the address by hand.

An alternative would be to not define a default here and document the possible addresses in the help text for DEBUG_SCIF, so that users will /always/ have to manually edit these, so at least it would be consistent.

Arnd

#17 lore raw

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

Hi Arnd,

Quoted 7 lines · Arnd Bergmann

arch/arm/Kconfig.debug config DEBUG_UART_PHYS · line 1126

Quoted 6 lines · On Thursday 02 October 2014 21:02:43 Geert Uytterhoeven wrote:
arch/arm/Kconfig.debug +6 -0
+       default 0xe6c40000 if DEBUG_SCIF && (ARCH_R8A73A4 || ARCH_SH7372)
+       default 0xe6c50000 if DEBUG_SCIF && ARCH_R8A7740
+       default 0xe6c80000 if DEBUG_SCIF && ARCH_SH73A0
+       default 0xe6e58000 if DEBUG_SCIF && ARCH_R8A7794
+       default 0xe6e60000 if DEBUG_SCIF && (ARCH_R8A7790 || ARCH_R8A7791 || ARCH_R8A7793)
+       default 0xe8008000 if DEBUG_SCIF && ARCH_R7S72100

There were two (important ones for my point below) more here:

+ default 0xffe40000 if DEBUG_SCIF && ARCH_R8A7778
+ default 0xffe42000 if DEBUG_SCIF && ARCH_R8A7779

Quoted 3 lines

Just saw this patch. Note that most other entries do not depend on a particular SoC but instead on one of the Kconfig entries, and I would like to eventually change all of them to be the same way.

I thought about having separate Kconfig entries for the various serial ports, until I started realizing that
(a) not all serial ports are at the same addresses on all SoCs (most are, though),

    The list above corresponds to these ports and addresses (note that we
    have multiple scif0 and scif2):

        scifa0: serial at e6c40000
        scifa1: serial at e6c50000
        scifa4: serial at e6c80000
        scif2: serial at e6e58000
        scif0: serial at e6e60000
        scif2: serial at e8008000
        scif0: serial at ffe40000
        scif2: serial at ffe42000

(b) this would just add lots of sections with similar lines in Kconfig.debug. E.g. compare

    default 0xe6c50000 if DEBUG_SCIF && ARCH_R8A7740

to

config DEBUG_RENESAS_SCIFA1
bool "Kernel low-level debugging messages via SCIFA1 on
Renesas ARM SoCs"

        depends on DEBUG_RENESAS_SCIF && ARCH_R8A7740
        help
                [ ....]


    default 0xe6c50000 if DEBUG_RENESAS_SCIFA1

and repeat for all serial ports...
Or do you mean something different?

Quoted 4 lines

The main reason for this is that your way is a mix of two methods: if you build a kernel for just one SoC you get the right entry here, but if you build the kernel for multiple SoCs combined, you have to edit the address by hand.

It's not guaranteed to work if you have a kernel with support for multiple SoCs, as some SoCs still use machine_desc.map_io(), while others don't. ARCH_R8A73A4 and ARCH_SH7372 use the same serial port at the same address, but ARCH_SH7372 uses machine_desc.map_io().

Note that also encoding the SoC names/families, and offering a choice option out of

DEBUG_RENESAS_R8A73A4_SCIFA0
DEBUG_RENESAS_SH7372_SCIFA0
DEBUG_RENESAS_R8A7740_SCIFA1
DEBUG_RENESAS_SH73A0_SCIFA4
DEBUG_RENESAS_RCAR_GEN2_SCIF2
DEBUG_RENESAS_RCAR_GEN2_SCIF0
DEBUG_RENESAS_R7S72100_SCIF2
DEBUG_RENESAS_RCAR_GEN1_SCIF0
DEBUG_RENESAS_RCAR_GEN1_SCIF2

could be used to solve the machine_desc.map_io() issue. Then we can have hardcoded addresses based on the above.

Quoted 3 lines

An alternative would be to not define a default here and document the possible addresses in the help text for DEBUG_SCIF, so that users will /always/ have to manually edit these, so at least it would be consistent.

I prefer to optimize the user experience for the common case, i.e. autoselection for the SoC you're using, and manual edit for the less common case.

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