All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/5] arm: mvebu: turris_omnia: Fix hangup in debug UART
@ 2022-05-06  9:05 Pali Rohár
  2022-05-06  9:05 ` [PATCH 1/5] arm: Add new config option ARCH_VERY_EARLY_INIT Pali Rohár
                   ` (8 more replies)
  0 siblings, 9 replies; 32+ messages in thread
From: Pali Rohár @ 2022-05-06  9:05 UTC (permalink / raw)
  To: Stefan Roese, Marek Behun; +Cc: u-boot

If proper U-Boot on Turris Omnia tries to print something on debug UART
then CPU hangs. Reason is that debug UART in proper U-Boot for Turris
Omnia has incorrect configuration of base register. Base register is
different in SPL and also in different stages of proper U-Boot.

Fix all these issues. Introduce new arch_very_early_init() ARM function
which will be called at very early stage prior initialization of debug
UART. Move code which moves internal mvebu registers to this function.
And split configuration of DEBUG_UART_BASE into SPL and proper U-Boot
to allow setting different values which matches current HW status.

With all these changes, it is possible call "printf" function in
Turris Omnia's board_early_init_f() function in both SPL and proper
U-Boot.

Pali Rohár (5):
  arm: Add new config option ARCH_VERY_EARLY_INIT
  arm: mvebu: Move internal registers in arch_very_early_init() function
  serial: Add new config option SPL_DEBUG_UART_BASE
  serial: ns16550: Add support for SPL_DEBUG_UART_BASE
  arm: mvebu: turris_omnia: Fix DEBUG_UART_BASE

 arch/arm/Kconfig               |  6 ++++++
 arch/arm/lib/crt0.S            |  5 +++++
 arch/arm/mach-mvebu/Kconfig    |  1 +
 arch/arm/mach-mvebu/Makefile   |  1 +
 arch/arm/mach-mvebu/cpu.c      | 31 -------------------------------
 arch/arm/mach-mvebu/lowlevel.S | 27 +++++++++++++++++++++++++++
 configs/turris_omnia_defconfig |  3 ++-
 drivers/serial/Kconfig         |  7 +++++++
 drivers/serial/ns16550.c       |  4 ++--
 9 files changed, 51 insertions(+), 34 deletions(-)
 create mode 100644 arch/arm/mach-mvebu/lowlevel.S

-- 
2.20.1


^ permalink raw reply	[flat|nested] 32+ messages in thread

* [PATCH 1/5] arm: Add new config option ARCH_VERY_EARLY_INIT
  2022-05-06  9:05 [PATCH 0/5] arm: mvebu: turris_omnia: Fix hangup in debug UART Pali Rohár
@ 2022-05-06  9:05 ` Pali Rohár
  2022-05-16  6:37   ` Stefan Roese
  2022-05-06  9:05 ` [PATCH 2/5] arm: mvebu: Move internal registers in arch_very_early_init() function Pali Rohár
                   ` (7 subsequent siblings)
  8 siblings, 1 reply; 32+ messages in thread
From: Pali Rohár @ 2022-05-06  9:05 UTC (permalink / raw)
  To: Stefan Roese, Marek Behun; +Cc: u-boot

When this option is set then ARM _main() function would call
arch_very_early_init() function at the beginning. It would be before
calling any other functions like debug_uart_init() and also before
initializing C runtime environment.

Signed-off-by: Pali Rohár <pali@kernel.org>
---
 arch/arm/Kconfig    | 6 ++++++
 arch/arm/lib/crt0.S | 5 +++++
 2 files changed, 11 insertions(+)

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 0afec5155b1b..9898c7d68e1b 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -401,6 +401,12 @@ config SYS_ARM_CACHE_WRITEALLOC
 	  write is performed.
 endchoice
 
+config ARCH_VERY_EARLY_INIT
+	bool
+
+config SPL_ARCH_VERY_EARLY_INIT
+	bool
+
 config ARCH_CPU_INIT
 	bool "Enable ARCH_CPU_INIT"
 	help
diff --git a/arch/arm/lib/crt0.S b/arch/arm/lib/crt0.S
index ba312901f331..612a2d5b698e 100644
--- a/arch/arm/lib/crt0.S
+++ b/arch/arm/lib/crt0.S
@@ -90,6 +90,11 @@ clbss_l:cmp	r0, r1			/* while not at end of BSS */
 
 ENTRY(_main)
 
+/* Call arch_very_early_init before initializing C runtime environment. */
+#if CONFIG_IS_ENABLED(ARCH_VERY_EARLY_INIT)
+	bl	arch_very_early_init
+#endif
+
 /*
  * Set up initial C runtime environment and call board_init_f(0).
  */
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH 2/5] arm: mvebu: Move internal registers in arch_very_early_init() function
  2022-05-06  9:05 [PATCH 0/5] arm: mvebu: turris_omnia: Fix hangup in debug UART Pali Rohár
  2022-05-06  9:05 ` [PATCH 1/5] arm: Add new config option ARCH_VERY_EARLY_INIT Pali Rohár
@ 2022-05-06  9:05 ` Pali Rohár
  2022-05-06 12:04   ` Stefan Roese
  2022-05-16  6:37   ` [PATCH 2/5] arm: mvebu: Move internal registers in arch_very_early_init() function Stefan Roese
  2022-05-06  9:05 ` [PATCH 3/5] serial: Add new config option SPL_DEBUG_UART_BASE Pali Rohár
                   ` (6 subsequent siblings)
  8 siblings, 2 replies; 32+ messages in thread
From: Pali Rohár @ 2022-05-06  9:05 UTC (permalink / raw)
  To: Stefan Roese, Marek Behun; +Cc: u-boot

Moving of internal registers from INTREG_BASE_ADDR_REG to SOC_REGS_PHY_BASE
needs to be done very early, prior calling any function which may touch
internal registers, like debug_uart_init().

So do it earlier in arch_very_early_init() instead of arch_cpu_init().

Movement is done in proper U-Boot, not in SPL. SPL may return to bootrom
and bootrom requires internal registers at (old) expected location.

Signed-off-by: Pali Rohár <pali@kernel.org>
---
 arch/arm/mach-mvebu/Kconfig    |  1 +
 arch/arm/mach-mvebu/Makefile   |  1 +
 arch/arm/mach-mvebu/cpu.c      | 31 -------------------------------
 arch/arm/mach-mvebu/lowlevel.S | 27 +++++++++++++++++++++++++++
 4 files changed, 29 insertions(+), 31 deletions(-)
 create mode 100644 arch/arm/mach-mvebu/lowlevel.S

diff --git a/arch/arm/mach-mvebu/Kconfig b/arch/arm/mach-mvebu/Kconfig
index a3f273f4f949..98b13d1e336d 100644
--- a/arch/arm/mach-mvebu/Kconfig
+++ b/arch/arm/mach-mvebu/Kconfig
@@ -16,6 +16,7 @@ config ARMADA_32BIT
 	select SUPPORT_SPL
 	select TRANSLATION_OFFSET
 	select SPL_SYS_NO_VECTOR_TABLE if SPL
+	select ARCH_VERY_EARLY_INIT
 
 config ARMADA_64BIT
 	bool
diff --git a/arch/arm/mach-mvebu/Makefile b/arch/arm/mach-mvebu/Makefile
index 1b451889d242..8bd2246325ca 100644
--- a/arch/arm/mach-mvebu/Makefile
+++ b/arch/arm/mach-mvebu/Makefile
@@ -21,6 +21,7 @@ else # CONFIG_ARCH_KIRKWOOD
 
 obj-y	= cpu.o
 obj-y	+= dram.o
+obj-y	+= lowlevel.o
 obj-$(CONFIG_DM_RESET) += system-controller.o
 ifndef CONFIG_SPL_BUILD
 obj-$(CONFIG_ARMADA_375) += ../../../drivers/ddr/marvell/axp/xor.o
diff --git a/arch/arm/mach-mvebu/cpu.c b/arch/arm/mach-mvebu/cpu.c
index 1e893777b292..173d95a760a3 100644
--- a/arch/arm/mach-mvebu/cpu.c
+++ b/arch/arm/mach-mvebu/cpu.c
@@ -413,20 +413,7 @@ static void update_sdram_window_sizes(void)
 	}
 }
 
-void mmu_disable(void)
-{
-	asm volatile(
-		"mrc p15, 0, r0, c1, c0, 0\n"
-		"bic r0, #1\n"
-		"mcr p15, 0, r0, c1, c0, 0\n");
-}
-
 #ifdef CONFIG_ARCH_CPU_INIT
-static void set_cbar(u32 addr)
-{
-	asm("mcr p15, 4, %0, c15, c0" : : "r" (addr));
-}
-
 #define MV_USB_PHY_BASE			(MVEBU_AXP_USB_BASE + 0x800)
 #define MV_USB_PHY_PLL_REG(reg)		(MV_USB_PHY_BASE | (((reg) & 0xF) << 2))
 #define MV_USB_X3_BASE(addr)		(MVEBU_AXP_USB_BASE | BIT(11) | \
@@ -476,24 +463,6 @@ int arch_cpu_init(void)
 	struct pl310_regs *const pl310 =
 		(struct pl310_regs *)CONFIG_SYS_PL310_BASE;
 
-	/*
-	 * Only with disabled MMU its possible to switch the base
-	 * register address on Armada 38x. Without this the SDRAM
-	 * located at >= 0x4000.0000 is also not accessible, as its
-	 * still locked to cache.
-	 */
-	mmu_disable();
-
-	/* Linux expects the internal registers to be at 0xf1000000 */
-	writel(SOC_REGS_PHY_BASE, INTREG_BASE_ADDR_REG);
-	set_cbar(SOC_REGS_PHY_BASE + 0xC000);
-
-	/*
-	 * From this stage on, the SoC detection is working. As we have
-	 * configured the internal register base to the value used
-	 * in the macros / defines in the U-Boot header (soc.h).
-	 */
-
 	if (mvebu_soc_family() == MVEBU_SOC_A38X) {
 		/*
 		 * To fully release / unlock this area from cache, we need
diff --git a/arch/arm/mach-mvebu/lowlevel.S b/arch/arm/mach-mvebu/lowlevel.S
new file mode 100644
index 000000000000..2491310eb0c1
--- /dev/null
+++ b/arch/arm/mach-mvebu/lowlevel.S
@@ -0,0 +1,27 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+
+#include <config.h>
+#include <linux/linkage.h>
+
+ENTRY(arch_very_early_init)
+#ifdef CONFIG_ARMADA_38X
+	/*
+	 * Only with disabled MMU its possible to switch the base
+	 * register address on Armada 38x. Without this the SDRAM
+	 * located at >= 0x4000.0000 is also not accessible, as its
+	 * still locked to cache.
+	 */
+	mrc	p15, 0, r0, c1, c0, 0
+	bic	r0, #1
+	mcr	p15, 0, r0, c1, c0, 0
+#endif
+
+	/* Move internal registers from INTREG_BASE_ADDR_REG to SOC_REGS_PHY_BASE */
+	ldr	r0, =SOC_REGS_PHY_BASE
+	ldr	r1, =INTREG_BASE_ADDR_REG
+	str	r0, [r1]
+	add	r0, r0, #0xC000
+	mcr	p15, 4, r0, c15, c0
+
+	bx lr
+ENDPROC(arch_very_early_init)
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH 3/5] serial: Add new config option SPL_DEBUG_UART_BASE
  2022-05-06  9:05 [PATCH 0/5] arm: mvebu: turris_omnia: Fix hangup in debug UART Pali Rohár
  2022-05-06  9:05 ` [PATCH 1/5] arm: Add new config option ARCH_VERY_EARLY_INIT Pali Rohár
  2022-05-06  9:05 ` [PATCH 2/5] arm: mvebu: Move internal registers in arch_very_early_init() function Pali Rohár
@ 2022-05-06  9:05 ` Pali Rohár
  2022-05-16  6:37   ` Stefan Roese
  2022-05-06  9:05 ` [PATCH 4/5] serial: ns16550: Add support for SPL_DEBUG_UART_BASE Pali Rohár
                   ` (5 subsequent siblings)
  8 siblings, 1 reply; 32+ messages in thread
From: Pali Rohár @ 2022-05-06  9:05 UTC (permalink / raw)
  To: Stefan Roese, Marek Behun; +Cc: u-boot

SPL_DEBUG_UART_BASE is same as DEBUG_UART_BASE, but applies only for SPL.

In some cases base address of UART is different in SPL and proper U-Boot.

Signed-off-by: Pali Rohár <pali@kernel.org>
---
 drivers/serial/Kconfig | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/serial/Kconfig b/drivers/serial/Kconfig
index 26fa498bbbb7..46726d76b162 100644
--- a/drivers/serial/Kconfig
+++ b/drivers/serial/Kconfig
@@ -508,6 +508,13 @@ config DEBUG_UART_BASE
 	  A default should be provided by your board, but if not you will need
 	  to use the correct value here.
 
+config SPL_DEBUG_UART_BASE
+	hex "Base address of UART for SPL"
+	depends on SPL && DEBUG_UART
+	default DEBUG_UART_BASE
+	help
+	  This is the base address of your UART for memory-mapped UARTs for SPL.
+
 config DEBUG_UART_CLOCK
 	int "UART input clock"
 	depends on DEBUG_UART
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH 4/5] serial: ns16550: Add support for SPL_DEBUG_UART_BASE
  2022-05-06  9:05 [PATCH 0/5] arm: mvebu: turris_omnia: Fix hangup in debug UART Pali Rohár
                   ` (2 preceding siblings ...)
  2022-05-06  9:05 ` [PATCH 3/5] serial: Add new config option SPL_DEBUG_UART_BASE Pali Rohár
@ 2022-05-06  9:05 ` Pali Rohár
  2022-05-16  6:38   ` Stefan Roese
  2022-05-06  9:05 ` [PATCH 5/5] arm: mvebu: turris_omnia: Fix DEBUG_UART_BASE Pali Rohár
                   ` (4 subsequent siblings)
  8 siblings, 1 reply; 32+ messages in thread
From: Pali Rohár @ 2022-05-06  9:05 UTC (permalink / raw)
  To: Stefan Roese, Marek Behun; +Cc: u-boot

Use CONFIG_VAL(DEBUG_UART_BASE) instead of CONFIG_DEBUG_UART_BASE, so
proper config value (CONFIG_DEBUG_UART_BASE or CONFIG_SPL_DEBUG_UART_BASE)
is used based on building target.

Signed-off-by: Pali Rohár <pali@kernel.org>
---
 drivers/serial/ns16550.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/serial/ns16550.c b/drivers/serial/ns16550.c
index a4220fd0ae2f..78bfe6281ce3 100644
--- a/drivers/serial/ns16550.c
+++ b/drivers/serial/ns16550.c
@@ -325,7 +325,7 @@ int ns16550_tstc(struct ns16550 *com_port)
 
 static inline void _debug_uart_init(void)
 {
-	struct ns16550 *com_port = (struct ns16550 *)CONFIG_DEBUG_UART_BASE;
+	struct ns16550 *com_port = (struct ns16550 *)CONFIG_VAL(DEBUG_UART_BASE);
 	int baud_divisor;
 
 	/*
@@ -360,7 +360,7 @@ static inline int NS16550_read_baud_divisor(struct ns16550 *com_port)
 
 static inline void _debug_uart_putc(int ch)
 {
-	struct ns16550 *com_port = (struct ns16550 *)CONFIG_DEBUG_UART_BASE;
+	struct ns16550 *com_port = (struct ns16550 *)CONFIG_VAL(DEBUG_UART_BASE);
 
 	while (!(serial_din(&com_port->lsr) & UART_LSR_THRE)) {
 #ifdef CONFIG_DEBUG_UART_NS16550_CHECK_ENABLED
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH 5/5] arm: mvebu: turris_omnia: Fix DEBUG_UART_BASE
  2022-05-06  9:05 [PATCH 0/5] arm: mvebu: turris_omnia: Fix hangup in debug UART Pali Rohár
                   ` (3 preceding siblings ...)
  2022-05-06  9:05 ` [PATCH 4/5] serial: ns16550: Add support for SPL_DEBUG_UART_BASE Pali Rohár
@ 2022-05-06  9:05 ` Pali Rohár
  2022-05-06 12:21   ` Stefan Roese
  2022-05-16  6:38   ` Stefan Roese
  2022-05-09 18:17 ` [PATCH 6/5] arm: mvebu: Fix DEBUG_UART_BASE for all 32-bit boards Pali Rohár
                   ` (3 subsequent siblings)
  8 siblings, 2 replies; 32+ messages in thread
From: Pali Rohár @ 2022-05-06  9:05 UTC (permalink / raw)
  To: Stefan Roese, Marek Behun; +Cc: u-boot

Internal registers in SPL are at address 0xd0000000 and in proper U-Boot at
address 0xf1000000. UART base address is located in internal registers.

Fix DEBUG_UART_BASE option to correct value for both SPL and proper U-Boot.

This change fixes hangup of proper U-Boot when it is trying to print
something via debug UART.

Signed-off-by: Pali Rohár <pali@kernel.org>
---
 configs/turris_omnia_defconfig | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/configs/turris_omnia_defconfig b/configs/turris_omnia_defconfig
index 217e2603fdfb..62c9be29c86b 100644
--- a/configs/turris_omnia_defconfig
+++ b/configs/turris_omnia_defconfig
@@ -18,7 +18,7 @@ CONFIG_DEFAULT_DEVICE_TREE="armada-385-turris-omnia"
 CONFIG_SPL_TEXT_BASE=0x40000030
 CONFIG_SPL_SERIAL=y
 CONFIG_SPL=y
-CONFIG_DEBUG_UART_BASE=0xd0012000
+CONFIG_DEBUG_UART_BASE=0xf1012000
 CONFIG_DEBUG_UART_CLOCK=250000000
 CONFIG_SYS_LOAD_ADDR=0x800000
 CONFIG_DEBUG_UART=y
@@ -93,6 +93,7 @@ CONFIG_PCI_MVEBU=y
 CONFIG_DM_RTC=y
 CONFIG_RTC_ARMADA38X=y
 CONFIG_SCSI=y
+CONFIG_SPL_DEBUG_UART_BASE=0xd0012000
 CONFIG_DEBUG_UART_SHIFT=2
 CONFIG_SYS_NS16550=y
 CONFIG_KIRKWOOD_SPI=y
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* Re: [PATCH 2/5] arm: mvebu: Move internal registers in arch_very_early_init() function
  2022-05-06  9:05 ` [PATCH 2/5] arm: mvebu: Move internal registers in arch_very_early_init() function Pali Rohár
@ 2022-05-06 12:04   ` Stefan Roese
  2022-05-06 12:09     ` Pali Rohár
  2022-05-16  6:37   ` [PATCH 2/5] arm: mvebu: Move internal registers in arch_very_early_init() function Stefan Roese
  1 sibling, 1 reply; 32+ messages in thread
From: Stefan Roese @ 2022-05-06 12:04 UTC (permalink / raw)
  To: Pali Rohár, Marek Behun; +Cc: u-boot

On 06.05.22 11:05, Pali Rohár wrote:
> Moving of internal registers from INTREG_BASE_ADDR_REG to SOC_REGS_PHY_BASE
> needs to be done very early, prior calling any function which may touch
> internal registers, like debug_uart_init().
> 
> So do it earlier in arch_very_early_init() instead of arch_cpu_init().
> 
> Movement is done in proper U-Boot, not in SPL. SPL may return to bootrom
> and bootrom requires internal registers at (old) expected location.
> 
> Signed-off-by: Pali Rohár <pali@kernel.org>
> ---
>   arch/arm/mach-mvebu/Kconfig    |  1 +
>   arch/arm/mach-mvebu/Makefile   |  1 +
>   arch/arm/mach-mvebu/cpu.c      | 31 -------------------------------
>   arch/arm/mach-mvebu/lowlevel.S | 27 +++++++++++++++++++++++++++
>   4 files changed, 29 insertions(+), 31 deletions(-)
>   create mode 100644 arch/arm/mach-mvebu/lowlevel.S
> 
> diff --git a/arch/arm/mach-mvebu/Kconfig b/arch/arm/mach-mvebu/Kconfig
> index a3f273f4f949..98b13d1e336d 100644
> --- a/arch/arm/mach-mvebu/Kconfig
> +++ b/arch/arm/mach-mvebu/Kconfig
> @@ -16,6 +16,7 @@ config ARMADA_32BIT
>   	select SUPPORT_SPL
>   	select TRANSLATION_OFFSET
>   	select SPL_SYS_NO_VECTOR_TABLE if SPL
> +	select ARCH_VERY_EARLY_INIT
>   
>   config ARMADA_64BIT
>   	bool
> diff --git a/arch/arm/mach-mvebu/Makefile b/arch/arm/mach-mvebu/Makefile
> index 1b451889d242..8bd2246325ca 100644
> --- a/arch/arm/mach-mvebu/Makefile
> +++ b/arch/arm/mach-mvebu/Makefile
> @@ -21,6 +21,7 @@ else # CONFIG_ARCH_KIRKWOOD
>   
>   obj-y	= cpu.o
>   obj-y	+= dram.o
> +obj-y	+= lowlevel.o
>   obj-$(CONFIG_DM_RESET) += system-controller.o
>   ifndef CONFIG_SPL_BUILD
>   obj-$(CONFIG_ARMADA_375) += ../../../drivers/ddr/marvell/axp/xor.o
> diff --git a/arch/arm/mach-mvebu/cpu.c b/arch/arm/mach-mvebu/cpu.c
> index 1e893777b292..173d95a760a3 100644
> --- a/arch/arm/mach-mvebu/cpu.c
> +++ b/arch/arm/mach-mvebu/cpu.c
> @@ -413,20 +413,7 @@ static void update_sdram_window_sizes(void)
>   	}
>   }
>   
> -void mmu_disable(void)
> -{
> -	asm volatile(
> -		"mrc p15, 0, r0, c1, c0, 0\n"
> -		"bic r0, #1\n"
> -		"mcr p15, 0, r0, c1, c0, 0\n");
> -}
> -
>   #ifdef CONFIG_ARCH_CPU_INIT
> -static void set_cbar(u32 addr)
> -{
> -	asm("mcr p15, 4, %0, c15, c0" : : "r" (addr));
> -}
> -
>   #define MV_USB_PHY_BASE			(MVEBU_AXP_USB_BASE + 0x800)
>   #define MV_USB_PHY_PLL_REG(reg)		(MV_USB_PHY_BASE | (((reg) & 0xF) << 2))
>   #define MV_USB_X3_BASE(addr)		(MVEBU_AXP_USB_BASE | BIT(11) | \
> @@ -476,24 +463,6 @@ int arch_cpu_init(void)
>   	struct pl310_regs *const pl310 =
>   		(struct pl310_regs *)CONFIG_SYS_PL310_BASE;
>   
> -	/*
> -	 * Only with disabled MMU its possible to switch the base
> -	 * register address on Armada 38x. Without this the SDRAM
> -	 * located at >= 0x4000.0000 is also not accessible, as its
> -	 * still locked to cache.
> -	 */
> -	mmu_disable();
> -
> -	/* Linux expects the internal registers to be at 0xf1000000 */
> -	writel(SOC_REGS_PHY_BASE, INTREG_BASE_ADDR_REG);
> -	set_cbar(SOC_REGS_PHY_BASE + 0xC000);
> -
> -	/*
> -	 * From this stage on, the SoC detection is working. As we have
> -	 * configured the internal register base to the value used
> -	 * in the macros / defines in the U-Boot header (soc.h).
> -	 */
> -
>   	if (mvebu_soc_family() == MVEBU_SOC_A38X) {
>   		/*
>   		 * To fully release / unlock this area from cache, we need
> diff --git a/arch/arm/mach-mvebu/lowlevel.S b/arch/arm/mach-mvebu/lowlevel.S
> new file mode 100644
> index 000000000000..2491310eb0c1
> --- /dev/null
> +++ b/arch/arm/mach-mvebu/lowlevel.S
> @@ -0,0 +1,27 @@
> +/* SPDX-License-Identifier: GPL-2.0+ */
> +
> +#include <config.h>
> +#include <linux/linkage.h>
> +
> +ENTRY(arch_very_early_init)
> +#ifdef CONFIG_ARMADA_38X
> +	/*
> +	 * Only with disabled MMU its possible to switch the base
> +	 * register address on Armada 38x. Without this the SDRAM
> +	 * located at >= 0x4000.0000 is also not accessible, as its
> +	 * still locked to cache.
> +	 */
> +	mrc	p15, 0, r0, c1, c0, 0
> +	bic	r0, #1
> +	mcr	p15, 0, r0, c1, c0, 0
> +#endif

You are disabling the MMU here only for CONFIG_ARMADA_38X. In the
non-asm version removed above, it's disabled in all cases in
arch_cpu_init(). Is this correct? I might be missing something - it's
been quite some time, since I worked on this code.

Thanks,
Stefan

> +
> +	/* Move internal registers from INTREG_BASE_ADDR_REG to SOC_REGS_PHY_BASE */
> +	ldr	r0, =SOC_REGS_PHY_BASE
> +	ldr	r1, =INTREG_BASE_ADDR_REG
> +	str	r0, [r1]
> +	add	r0, r0, #0xC000
> +	mcr	p15, 4, r0, c15, c0
> +
> +	bx lr
> +ENDPROC(arch_very_early_init)

Viele Grüße,
Stefan Roese

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr@denx.de

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 2/5] arm: mvebu: Move internal registers in arch_very_early_init() function
  2022-05-06 12:04   ` Stefan Roese
@ 2022-05-06 12:09     ` Pali Rohár
  2022-05-06 12:16       ` Stefan Roese
  0 siblings, 1 reply; 32+ messages in thread
From: Pali Rohár @ 2022-05-06 12:09 UTC (permalink / raw)
  To: Stefan Roese; +Cc: Marek Behun, u-boot

On Friday 06 May 2022 14:04:53 Stefan Roese wrote:
> On 06.05.22 11:05, Pali Rohár wrote:
> > Moving of internal registers from INTREG_BASE_ADDR_REG to SOC_REGS_PHY_BASE
> > needs to be done very early, prior calling any function which may touch
> > internal registers, like debug_uart_init().
> > 
> > So do it earlier in arch_very_early_init() instead of arch_cpu_init().
> > 
> > Movement is done in proper U-Boot, not in SPL. SPL may return to bootrom
> > and bootrom requires internal registers at (old) expected location.
> > 
> > Signed-off-by: Pali Rohár <pali@kernel.org>
> > ---
> >   arch/arm/mach-mvebu/Kconfig    |  1 +
> >   arch/arm/mach-mvebu/Makefile   |  1 +
> >   arch/arm/mach-mvebu/cpu.c      | 31 -------------------------------
> >   arch/arm/mach-mvebu/lowlevel.S | 27 +++++++++++++++++++++++++++
> >   4 files changed, 29 insertions(+), 31 deletions(-)
> >   create mode 100644 arch/arm/mach-mvebu/lowlevel.S
> > 
> > diff --git a/arch/arm/mach-mvebu/Kconfig b/arch/arm/mach-mvebu/Kconfig
> > index a3f273f4f949..98b13d1e336d 100644
> > --- a/arch/arm/mach-mvebu/Kconfig
> > +++ b/arch/arm/mach-mvebu/Kconfig
> > @@ -16,6 +16,7 @@ config ARMADA_32BIT
> >   	select SUPPORT_SPL
> >   	select TRANSLATION_OFFSET
> >   	select SPL_SYS_NO_VECTOR_TABLE if SPL
> > +	select ARCH_VERY_EARLY_INIT
> >   config ARMADA_64BIT
> >   	bool
> > diff --git a/arch/arm/mach-mvebu/Makefile b/arch/arm/mach-mvebu/Makefile
> > index 1b451889d242..8bd2246325ca 100644
> > --- a/arch/arm/mach-mvebu/Makefile
> > +++ b/arch/arm/mach-mvebu/Makefile
> > @@ -21,6 +21,7 @@ else # CONFIG_ARCH_KIRKWOOD
> >   obj-y	= cpu.o
> >   obj-y	+= dram.o
> > +obj-y	+= lowlevel.o
> >   obj-$(CONFIG_DM_RESET) += system-controller.o
> >   ifndef CONFIG_SPL_BUILD
> >   obj-$(CONFIG_ARMADA_375) += ../../../drivers/ddr/marvell/axp/xor.o
> > diff --git a/arch/arm/mach-mvebu/cpu.c b/arch/arm/mach-mvebu/cpu.c
> > index 1e893777b292..173d95a760a3 100644
> > --- a/arch/arm/mach-mvebu/cpu.c
> > +++ b/arch/arm/mach-mvebu/cpu.c
> > @@ -413,20 +413,7 @@ static void update_sdram_window_sizes(void)
> >   	}
> >   }
> > -void mmu_disable(void)
> > -{
> > -	asm volatile(
> > -		"mrc p15, 0, r0, c1, c0, 0\n"
> > -		"bic r0, #1\n"
> > -		"mcr p15, 0, r0, c1, c0, 0\n");
> > -}
> > -
> >   #ifdef CONFIG_ARCH_CPU_INIT
> > -static void set_cbar(u32 addr)
> > -{
> > -	asm("mcr p15, 4, %0, c15, c0" : : "r" (addr));
> > -}
> > -
> >   #define MV_USB_PHY_BASE			(MVEBU_AXP_USB_BASE + 0x800)
> >   #define MV_USB_PHY_PLL_REG(reg)		(MV_USB_PHY_BASE | (((reg) & 0xF) << 2))
> >   #define MV_USB_X3_BASE(addr)		(MVEBU_AXP_USB_BASE | BIT(11) | \
> > @@ -476,24 +463,6 @@ int arch_cpu_init(void)
> >   	struct pl310_regs *const pl310 =
> >   		(struct pl310_regs *)CONFIG_SYS_PL310_BASE;
> > -	/*
> > -	 * Only with disabled MMU its possible to switch the base
> > -	 * register address on Armada 38x. Without this the SDRAM
> > -	 * located at >= 0x4000.0000 is also not accessible, as its
> > -	 * still locked to cache.
> > -	 */
> > -	mmu_disable();
> > -
> > -	/* Linux expects the internal registers to be at 0xf1000000 */
> > -	writel(SOC_REGS_PHY_BASE, INTREG_BASE_ADDR_REG);
> > -	set_cbar(SOC_REGS_PHY_BASE + 0xC000);
> > -
> > -	/*
> > -	 * From this stage on, the SoC detection is working. As we have
> > -	 * configured the internal register base to the value used
> > -	 * in the macros / defines in the U-Boot header (soc.h).
> > -	 */
> > -
> >   	if (mvebu_soc_family() == MVEBU_SOC_A38X) {
> >   		/*
> >   		 * To fully release / unlock this area from cache, we need
> > diff --git a/arch/arm/mach-mvebu/lowlevel.S b/arch/arm/mach-mvebu/lowlevel.S
> > new file mode 100644
> > index 000000000000..2491310eb0c1
> > --- /dev/null
> > +++ b/arch/arm/mach-mvebu/lowlevel.S
> > @@ -0,0 +1,27 @@
> > +/* SPDX-License-Identifier: GPL-2.0+ */
> > +
> > +#include <config.h>
> > +#include <linux/linkage.h>
> > +
> > +ENTRY(arch_very_early_init)
> > +#ifdef CONFIG_ARMADA_38X
> > +	/*
> > +	 * Only with disabled MMU its possible to switch the base
> > +	 * register address on Armada 38x. Without this the SDRAM
> > +	 * located at >= 0x4000.0000 is also not accessible, as its
> > +	 * still locked to cache.
> > +	 */
> > +	mrc	p15, 0, r0, c1, c0, 0
> > +	bic	r0, #1
> > +	mcr	p15, 0, r0, c1, c0, 0
> > +#endif
> 
> You are disabling the MMU here only for CONFIG_ARMADA_38X. In the
> non-asm version removed above, it's disabled in all cases in
> arch_cpu_init(). Is this correct? I might be missing something - it's
> been quite some time, since I worked on this code.

I'm not sure. I was in impression from that comment that disabling MMU
is needed only for A38x. But runtime detection of the board was not
possible until registers are relocated to the new location, so code
disabling MMU was called always.

So, this change should be tested on some AXP, A370 or A375 board to
verify that everything is working correctly.

I tested it only on A385 Turris Omnia.

> Thanks,
> Stefan
> 
> > +
> > +	/* Move internal registers from INTREG_BASE_ADDR_REG to SOC_REGS_PHY_BASE */
> > +	ldr	r0, =SOC_REGS_PHY_BASE
> > +	ldr	r1, =INTREG_BASE_ADDR_REG
> > +	str	r0, [r1]
> > +	add	r0, r0, #0xC000
> > +	mcr	p15, 4, r0, c15, c0
> > +
> > +	bx lr
> > +ENDPROC(arch_very_early_init)
> 
> Viele Grüße,
> Stefan Roese
> 
> -- 
> DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr@denx.de

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 2/5] arm: mvebu: Move internal registers in arch_very_early_init() function
  2022-05-06 12:09     ` Pali Rohár
@ 2022-05-06 12:16       ` Stefan Roese
  2022-05-06 12:35         ` Stefan Roese
  0 siblings, 1 reply; 32+ messages in thread
From: Stefan Roese @ 2022-05-06 12:16 UTC (permalink / raw)
  To: Pali Rohár; +Cc: Marek Behun, u-boot

On 06.05.22 14:09, Pali Rohár wrote:
> On Friday 06 May 2022 14:04:53 Stefan Roese wrote:
>> On 06.05.22 11:05, Pali Rohár wrote:
>>> Moving of internal registers from INTREG_BASE_ADDR_REG to SOC_REGS_PHY_BASE
>>> needs to be done very early, prior calling any function which may touch
>>> internal registers, like debug_uart_init().
>>>
>>> So do it earlier in arch_very_early_init() instead of arch_cpu_init().
>>>
>>> Movement is done in proper U-Boot, not in SPL. SPL may return to bootrom
>>> and bootrom requires internal registers at (old) expected location.
>>>
>>> Signed-off-by: Pali Rohár <pali@kernel.org>
>>> ---
>>>    arch/arm/mach-mvebu/Kconfig    |  1 +
>>>    arch/arm/mach-mvebu/Makefile   |  1 +
>>>    arch/arm/mach-mvebu/cpu.c      | 31 -------------------------------
>>>    arch/arm/mach-mvebu/lowlevel.S | 27 +++++++++++++++++++++++++++
>>>    4 files changed, 29 insertions(+), 31 deletions(-)
>>>    create mode 100644 arch/arm/mach-mvebu/lowlevel.S
>>>
>>> diff --git a/arch/arm/mach-mvebu/Kconfig b/arch/arm/mach-mvebu/Kconfig
>>> index a3f273f4f949..98b13d1e336d 100644
>>> --- a/arch/arm/mach-mvebu/Kconfig
>>> +++ b/arch/arm/mach-mvebu/Kconfig
>>> @@ -16,6 +16,7 @@ config ARMADA_32BIT
>>>    	select SUPPORT_SPL
>>>    	select TRANSLATION_OFFSET
>>>    	select SPL_SYS_NO_VECTOR_TABLE if SPL
>>> +	select ARCH_VERY_EARLY_INIT
>>>    config ARMADA_64BIT
>>>    	bool
>>> diff --git a/arch/arm/mach-mvebu/Makefile b/arch/arm/mach-mvebu/Makefile
>>> index 1b451889d242..8bd2246325ca 100644
>>> --- a/arch/arm/mach-mvebu/Makefile
>>> +++ b/arch/arm/mach-mvebu/Makefile
>>> @@ -21,6 +21,7 @@ else # CONFIG_ARCH_KIRKWOOD
>>>    obj-y	= cpu.o
>>>    obj-y	+= dram.o
>>> +obj-y	+= lowlevel.o
>>>    obj-$(CONFIG_DM_RESET) += system-controller.o
>>>    ifndef CONFIG_SPL_BUILD
>>>    obj-$(CONFIG_ARMADA_375) += ../../../drivers/ddr/marvell/axp/xor.o
>>> diff --git a/arch/arm/mach-mvebu/cpu.c b/arch/arm/mach-mvebu/cpu.c
>>> index 1e893777b292..173d95a760a3 100644
>>> --- a/arch/arm/mach-mvebu/cpu.c
>>> +++ b/arch/arm/mach-mvebu/cpu.c
>>> @@ -413,20 +413,7 @@ static void update_sdram_window_sizes(void)
>>>    	}
>>>    }
>>> -void mmu_disable(void)
>>> -{
>>> -	asm volatile(
>>> -		"mrc p15, 0, r0, c1, c0, 0\n"
>>> -		"bic r0, #1\n"
>>> -		"mcr p15, 0, r0, c1, c0, 0\n");
>>> -}
>>> -
>>>    #ifdef CONFIG_ARCH_CPU_INIT
>>> -static void set_cbar(u32 addr)
>>> -{
>>> -	asm("mcr p15, 4, %0, c15, c0" : : "r" (addr));
>>> -}
>>> -
>>>    #define MV_USB_PHY_BASE			(MVEBU_AXP_USB_BASE + 0x800)
>>>    #define MV_USB_PHY_PLL_REG(reg)		(MV_USB_PHY_BASE | (((reg) & 0xF) << 2))
>>>    #define MV_USB_X3_BASE(addr)		(MVEBU_AXP_USB_BASE | BIT(11) | \
>>> @@ -476,24 +463,6 @@ int arch_cpu_init(void)
>>>    	struct pl310_regs *const pl310 =
>>>    		(struct pl310_regs *)CONFIG_SYS_PL310_BASE;
>>> -	/*
>>> -	 * Only with disabled MMU its possible to switch the base
>>> -	 * register address on Armada 38x. Without this the SDRAM
>>> -	 * located at >= 0x4000.0000 is also not accessible, as its
>>> -	 * still locked to cache.
>>> -	 */
>>> -	mmu_disable();
>>> -
>>> -	/* Linux expects the internal registers to be at 0xf1000000 */
>>> -	writel(SOC_REGS_PHY_BASE, INTREG_BASE_ADDR_REG);
>>> -	set_cbar(SOC_REGS_PHY_BASE + 0xC000);
>>> -
>>> -	/*
>>> -	 * From this stage on, the SoC detection is working. As we have
>>> -	 * configured the internal register base to the value used
>>> -	 * in the macros / defines in the U-Boot header (soc.h).
>>> -	 */
>>> -
>>>    	if (mvebu_soc_family() == MVEBU_SOC_A38X) {
>>>    		/*
>>>    		 * To fully release / unlock this area from cache, we need
>>> diff --git a/arch/arm/mach-mvebu/lowlevel.S b/arch/arm/mach-mvebu/lowlevel.S
>>> new file mode 100644
>>> index 000000000000..2491310eb0c1
>>> --- /dev/null
>>> +++ b/arch/arm/mach-mvebu/lowlevel.S
>>> @@ -0,0 +1,27 @@
>>> +/* SPDX-License-Identifier: GPL-2.0+ */
>>> +
>>> +#include <config.h>
>>> +#include <linux/linkage.h>
>>> +
>>> +ENTRY(arch_very_early_init)
>>> +#ifdef CONFIG_ARMADA_38X
>>> +	/*
>>> +	 * Only with disabled MMU its possible to switch the base
>>> +	 * register address on Armada 38x. Without this the SDRAM
>>> +	 * located at >= 0x4000.0000 is also not accessible, as its
>>> +	 * still locked to cache.
>>> +	 */
>>> +	mrc	p15, 0, r0, c1, c0, 0
>>> +	bic	r0, #1
>>> +	mcr	p15, 0, r0, c1, c0, 0
>>> +#endif
>>
>> You are disabling the MMU here only for CONFIG_ARMADA_38X. In the
>> non-asm version removed above, it's disabled in all cases in
>> arch_cpu_init(). Is this correct? I might be missing something - it's
>> been quite some time, since I worked on this code.
> 
> I'm not sure. I was in impression from that comment that disabling MMU
> is needed only for A38x. But runtime detection of the board was not
> possible until registers are relocated to the new location, so code
> disabling MMU was called always.
> 
> So, this change should be tested on some AXP, A370 or A375 board to
> verify that everything is working correctly.

Okay, understood. I can do some testing on AXP.

Thanks,
Stefan

> I tested it only on A385 Turris Omnia.
> 
>> Thanks,
>> Stefan
>>
>>> +
>>> +	/* Move internal registers from INTREG_BASE_ADDR_REG to SOC_REGS_PHY_BASE */
>>> +	ldr	r0, =SOC_REGS_PHY_BASE
>>> +	ldr	r1, =INTREG_BASE_ADDR_REG
>>> +	str	r0, [r1]
>>> +	add	r0, r0, #0xC000
>>> +	mcr	p15, 4, r0, c15, c0
>>> +
>>> +	bx lr
>>> +ENDPROC(arch_very_early_init)
>>
>> Viele Grüße,
>> Stefan Roese
>>
>> -- 
>> DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
>> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
>> Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr@denx.de

Viele Grüße,
Stefan Roese

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr@denx.de

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 5/5] arm: mvebu: turris_omnia: Fix DEBUG_UART_BASE
  2022-05-06  9:05 ` [PATCH 5/5] arm: mvebu: turris_omnia: Fix DEBUG_UART_BASE Pali Rohár
@ 2022-05-06 12:21   ` Stefan Roese
  2022-05-06 12:30     ` Pali Rohár
  2022-05-16  6:38   ` Stefan Roese
  1 sibling, 1 reply; 32+ messages in thread
From: Stefan Roese @ 2022-05-06 12:21 UTC (permalink / raw)
  To: Pali Rohár, Marek Behun; +Cc: u-boot

On 06.05.22 11:05, Pali Rohár wrote:
> Internal registers in SPL are at address 0xd0000000 and in proper U-Boot at
> address 0xf1000000. UART base address is located in internal registers.
> 
> Fix DEBUG_UART_BASE option to correct value for both SPL and proper U-Boot.
> 
> This change fixes hangup of proper U-Boot when it is trying to print
> something via debug UART.
> 
> Signed-off-by: Pali Rohár <pali@kernel.org>
> ---
>   configs/turris_omnia_defconfig | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/configs/turris_omnia_defconfig b/configs/turris_omnia_defconfig
> index 217e2603fdfb..62c9be29c86b 100644
> --- a/configs/turris_omnia_defconfig
> +++ b/configs/turris_omnia_defconfig
> @@ -18,7 +18,7 @@ CONFIG_DEFAULT_DEVICE_TREE="armada-385-turris-omnia"
>   CONFIG_SPL_TEXT_BASE=0x40000030
>   CONFIG_SPL_SERIAL=y
>   CONFIG_SPL=y
> -CONFIG_DEBUG_UART_BASE=0xd0012000
> +CONFIG_DEBUG_UART_BASE=0xf1012000
>   CONFIG_DEBUG_UART_CLOCK=250000000
>   CONFIG_SYS_LOAD_ADDR=0x800000
>   CONFIG_DEBUG_UART=y
> @@ -93,6 +93,7 @@ CONFIG_PCI_MVEBU=y
>   CONFIG_DM_RTC=y
>   CONFIG_RTC_ARMADA38X=y
>   CONFIG_SCSI=y
> +CONFIG_SPL_DEBUG_UART_BASE=0xd0012000
>   CONFIG_DEBUG_UART_SHIFT=2
>   CONFIG_SYS_NS16550=y
>   CONFIG_KIRKWOOD_SPI=y

Isn't such a change now needed for all 32bit Armada MVEBU targets?

Viele Grüße,
Stefan Roese

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr@denx.de

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 5/5] arm: mvebu: turris_omnia: Fix DEBUG_UART_BASE
  2022-05-06 12:21   ` Stefan Roese
@ 2022-05-06 12:30     ` Pali Rohár
  2022-05-09 18:18       ` Pali Rohár
  0 siblings, 1 reply; 32+ messages in thread
From: Pali Rohár @ 2022-05-06 12:30 UTC (permalink / raw)
  To: Stefan Roese; +Cc: Marek Behun, u-boot

On Friday 06 May 2022 14:21:02 Stefan Roese wrote:
> On 06.05.22 11:05, Pali Rohár wrote:
> > Internal registers in SPL are at address 0xd0000000 and in proper U-Boot at
> > address 0xf1000000. UART base address is located in internal registers.
> > 
> > Fix DEBUG_UART_BASE option to correct value for both SPL and proper U-Boot.
> > 
> > This change fixes hangup of proper U-Boot when it is trying to print
> > something via debug UART.
> > 
> > Signed-off-by: Pali Rohár <pali@kernel.org>
> > ---
> >   configs/turris_omnia_defconfig | 3 ++-
> >   1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/configs/turris_omnia_defconfig b/configs/turris_omnia_defconfig
> > index 217e2603fdfb..62c9be29c86b 100644
> > --- a/configs/turris_omnia_defconfig
> > +++ b/configs/turris_omnia_defconfig
> > @@ -18,7 +18,7 @@ CONFIG_DEFAULT_DEVICE_TREE="armada-385-turris-omnia"
> >   CONFIG_SPL_TEXT_BASE=0x40000030
> >   CONFIG_SPL_SERIAL=y
> >   CONFIG_SPL=y
> > -CONFIG_DEBUG_UART_BASE=0xd0012000
> > +CONFIG_DEBUG_UART_BASE=0xf1012000
> >   CONFIG_DEBUG_UART_CLOCK=250000000
> >   CONFIG_SYS_LOAD_ADDR=0x800000
> >   CONFIG_DEBUG_UART=y
> > @@ -93,6 +93,7 @@ CONFIG_PCI_MVEBU=y
> >   CONFIG_DM_RTC=y
> >   CONFIG_RTC_ARMADA38X=y
> >   CONFIG_SCSI=y
> > +CONFIG_SPL_DEBUG_UART_BASE=0xd0012000
> >   CONFIG_DEBUG_UART_SHIFT=2
> >   CONFIG_SYS_NS16550=y
> >   CONFIG_KIRKWOOD_SPI=y
> 
> Isn't such a change now needed for all 32bit Armada MVEBU targets?

This is a good question. It should be fixed for all 32bit mvebu targets.

> Viele Grüße,
> Stefan Roese
> 
> -- 
> DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr@denx.de

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 2/5] arm: mvebu: Move internal registers in arch_very_early_init() function
  2022-05-06 12:16       ` Stefan Roese
@ 2022-05-06 12:35         ` Stefan Roese
  2022-05-06 12:44           ` Pali Rohár
  0 siblings, 1 reply; 32+ messages in thread
From: Stefan Roese @ 2022-05-06 12:35 UTC (permalink / raw)
  To: Pali Rohár; +Cc: Marek Behun, u-boot

On 06.05.22 14:16, Stefan Roese wrote:
> On 06.05.22 14:09, Pali Rohár wrote:
>> On Friday 06 May 2022 14:04:53 Stefan Roese wrote:
>>> On 06.05.22 11:05, Pali Rohár wrote:
>>>> Moving of internal registers from INTREG_BASE_ADDR_REG to 
>>>> SOC_REGS_PHY_BASE
>>>> needs to be done very early, prior calling any function which may touch
>>>> internal registers, like debug_uart_init().
>>>>
>>>> So do it earlier in arch_very_early_init() instead of arch_cpu_init().
>>>>
>>>> Movement is done in proper U-Boot, not in SPL. SPL may return to 
>>>> bootrom
>>>> and bootrom requires internal registers at (old) expected location.
>>>>
>>>> Signed-off-by: Pali Rohár <pali@kernel.org>
>>>> ---
>>>>    arch/arm/mach-mvebu/Kconfig    |  1 +
>>>>    arch/arm/mach-mvebu/Makefile   |  1 +
>>>>    arch/arm/mach-mvebu/cpu.c      | 31 -------------------------------
>>>>    arch/arm/mach-mvebu/lowlevel.S | 27 +++++++++++++++++++++++++++
>>>>    4 files changed, 29 insertions(+), 31 deletions(-)
>>>>    create mode 100644 arch/arm/mach-mvebu/lowlevel.S
>>>>
>>>> diff --git a/arch/arm/mach-mvebu/Kconfig b/arch/arm/mach-mvebu/Kconfig
>>>> index a3f273f4f949..98b13d1e336d 100644
>>>> --- a/arch/arm/mach-mvebu/Kconfig
>>>> +++ b/arch/arm/mach-mvebu/Kconfig
>>>> @@ -16,6 +16,7 @@ config ARMADA_32BIT
>>>>        select SUPPORT_SPL
>>>>        select TRANSLATION_OFFSET
>>>>        select SPL_SYS_NO_VECTOR_TABLE if SPL
>>>> +    select ARCH_VERY_EARLY_INIT
>>>>    config ARMADA_64BIT
>>>>        bool
>>>> diff --git a/arch/arm/mach-mvebu/Makefile 
>>>> b/arch/arm/mach-mvebu/Makefile
>>>> index 1b451889d242..8bd2246325ca 100644
>>>> --- a/arch/arm/mach-mvebu/Makefile
>>>> +++ b/arch/arm/mach-mvebu/Makefile
>>>> @@ -21,6 +21,7 @@ else # CONFIG_ARCH_KIRKWOOD
>>>>    obj-y    = cpu.o
>>>>    obj-y    += dram.o
>>>> +obj-y    += lowlevel.o
>>>>    obj-$(CONFIG_DM_RESET) += system-controller.o
>>>>    ifndef CONFIG_SPL_BUILD
>>>>    obj-$(CONFIG_ARMADA_375) += ../../../drivers/ddr/marvell/axp/xor.o
>>>> diff --git a/arch/arm/mach-mvebu/cpu.c b/arch/arm/mach-mvebu/cpu.c
>>>> index 1e893777b292..173d95a760a3 100644
>>>> --- a/arch/arm/mach-mvebu/cpu.c
>>>> +++ b/arch/arm/mach-mvebu/cpu.c
>>>> @@ -413,20 +413,7 @@ static void update_sdram_window_sizes(void)
>>>>        }
>>>>    }
>>>> -void mmu_disable(void)
>>>> -{
>>>> -    asm volatile(
>>>> -        "mrc p15, 0, r0, c1, c0, 0\n"
>>>> -        "bic r0, #1\n"
>>>> -        "mcr p15, 0, r0, c1, c0, 0\n");
>>>> -}
>>>> -
>>>>    #ifdef CONFIG_ARCH_CPU_INIT
>>>> -static void set_cbar(u32 addr)
>>>> -{
>>>> -    asm("mcr p15, 4, %0, c15, c0" : : "r" (addr));
>>>> -}
>>>> -
>>>>    #define MV_USB_PHY_BASE            (MVEBU_AXP_USB_BASE + 0x800)
>>>>    #define MV_USB_PHY_PLL_REG(reg)        (MV_USB_PHY_BASE | (((reg) 
>>>> & 0xF) << 2))
>>>>    #define MV_USB_X3_BASE(addr)        (MVEBU_AXP_USB_BASE | BIT(11) 
>>>> | \
>>>> @@ -476,24 +463,6 @@ int arch_cpu_init(void)
>>>>        struct pl310_regs *const pl310 =
>>>>            (struct pl310_regs *)CONFIG_SYS_PL310_BASE;
>>>> -    /*
>>>> -     * Only with disabled MMU its possible to switch the base
>>>> -     * register address on Armada 38x. Without this the SDRAM
>>>> -     * located at >= 0x4000.0000 is also not accessible, as its
>>>> -     * still locked to cache.
>>>> -     */
>>>> -    mmu_disable();
>>>> -
>>>> -    /* Linux expects the internal registers to be at 0xf1000000 */
>>>> -    writel(SOC_REGS_PHY_BASE, INTREG_BASE_ADDR_REG);
>>>> -    set_cbar(SOC_REGS_PHY_BASE + 0xC000);
>>>> -
>>>> -    /*
>>>> -     * From this stage on, the SoC detection is working. As we have
>>>> -     * configured the internal register base to the value used
>>>> -     * in the macros / defines in the U-Boot header (soc.h).
>>>> -     */
>>>> -
>>>>        if (mvebu_soc_family() == MVEBU_SOC_A38X) {
>>>>            /*
>>>>             * To fully release / unlock this area from cache, we need
>>>> diff --git a/arch/arm/mach-mvebu/lowlevel.S 
>>>> b/arch/arm/mach-mvebu/lowlevel.S
>>>> new file mode 100644
>>>> index 000000000000..2491310eb0c1
>>>> --- /dev/null
>>>> +++ b/arch/arm/mach-mvebu/lowlevel.S
>>>> @@ -0,0 +1,27 @@
>>>> +/* SPDX-License-Identifier: GPL-2.0+ */
>>>> +
>>>> +#include <config.h>
>>>> +#include <linux/linkage.h>
>>>> +
>>>> +ENTRY(arch_very_early_init)
>>>> +#ifdef CONFIG_ARMADA_38X
>>>> +    /*
>>>> +     * Only with disabled MMU its possible to switch the base
>>>> +     * register address on Armada 38x. Without this the SDRAM
>>>> +     * located at >= 0x4000.0000 is also not accessible, as its
>>>> +     * still locked to cache.
>>>> +     */
>>>> +    mrc    p15, 0, r0, c1, c0, 0
>>>> +    bic    r0, #1
>>>> +    mcr    p15, 0, r0, c1, c0, 0
>>>> +#endif
>>>
>>> You are disabling the MMU here only for CONFIG_ARMADA_38X. In the
>>> non-asm version removed above, it's disabled in all cases in
>>> arch_cpu_init(). Is this correct? I might be missing something - it's
>>> been quite some time, since I worked on this code.
>>
>> I'm not sure. I was in impression from that comment that disabling MMU
>> is needed only for A38x. But runtime detection of the board was not
>> possible until registers are relocated to the new location, so code
>> disabling MMU was called always.
>>
>> So, this change should be tested on some AXP, A370 or A375 board to
>> verify that everything is working correctly.
> 
> Okay, understood. I can do some testing on AXP.

Some quick tests on theadorable_debug (AXP) have shown, that it makes
no difference, if the MMU is disabled or not AFAICT.

While doing this I noticed though, that kwboot UART booting only worked
in roughly 1 out of 2 cases. With no progress after this line:

Sending boot message. Please reboot the target...\

IIRC, this worked more reliable a few weeks ago. Any idea, what might
have caused this regression - if I am correct here?

Thanks,
Stefan

> Thanks,
> Stefan
> 
>> I tested it only on A385 Turris Omnia.
>>
>>> Thanks,
>>> Stefan
>>>
>>>> +
>>>> +    /* Move internal registers from INTREG_BASE_ADDR_REG to 
>>>> SOC_REGS_PHY_BASE */
>>>> +    ldr    r0, =SOC_REGS_PHY_BASE
>>>> +    ldr    r1, =INTREG_BASE_ADDR_REG
>>>> +    str    r0, [r1]
>>>> +    add    r0, r0, #0xC000
>>>> +    mcr    p15, 4, r0, c15, c0
>>>> +
>>>> +    bx lr
>>>> +ENDPROC(arch_very_early_init)
>>>
>>> Viele Grüße,
>>> Stefan Roese
>>>
>>> -- 
>>> DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
>>> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
>>> Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr@denx.de
> 
> Viele Grüße,
> Stefan Roese
> 

Viele Grüße,
Stefan Roese

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr@denx.de

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 2/5] arm: mvebu: Move internal registers in arch_very_early_init() function
  2022-05-06 12:35         ` Stefan Roese
@ 2022-05-06 12:44           ` Pali Rohár
  2022-06-01 10:27             ` Pali Rohár
  0 siblings, 1 reply; 32+ messages in thread
From: Pali Rohár @ 2022-05-06 12:44 UTC (permalink / raw)
  To: Stefan Roese; +Cc: Marek Behun, u-boot

On Friday 06 May 2022 14:35:55 Stefan Roese wrote:
> On 06.05.22 14:16, Stefan Roese wrote:
> > On 06.05.22 14:09, Pali Rohár wrote:
> > > On Friday 06 May 2022 14:04:53 Stefan Roese wrote:
> > > > On 06.05.22 11:05, Pali Rohár wrote:
> > > > > Moving of internal registers from INTREG_BASE_ADDR_REG to
> > > > > SOC_REGS_PHY_BASE
> > > > > needs to be done very early, prior calling any function which may touch
> > > > > internal registers, like debug_uart_init().
> > > > > 
> > > > > So do it earlier in arch_very_early_init() instead of arch_cpu_init().
> > > > > 
> > > > > Movement is done in proper U-Boot, not in SPL. SPL may
> > > > > return to bootrom
> > > > > and bootrom requires internal registers at (old) expected location.
> > > > > 
> > > > > Signed-off-by: Pali Rohár <pali@kernel.org>
> > > > > ---
> > > > >    arch/arm/mach-mvebu/Kconfig    |  1 +
> > > > >    arch/arm/mach-mvebu/Makefile   |  1 +
> > > > >    arch/arm/mach-mvebu/cpu.c      | 31 -------------------------------
> > > > >    arch/arm/mach-mvebu/lowlevel.S | 27 +++++++++++++++++++++++++++
> > > > >    4 files changed, 29 insertions(+), 31 deletions(-)
> > > > >    create mode 100644 arch/arm/mach-mvebu/lowlevel.S
> > > > > 
> > > > > diff --git a/arch/arm/mach-mvebu/Kconfig b/arch/arm/mach-mvebu/Kconfig
> > > > > index a3f273f4f949..98b13d1e336d 100644
> > > > > --- a/arch/arm/mach-mvebu/Kconfig
> > > > > +++ b/arch/arm/mach-mvebu/Kconfig
> > > > > @@ -16,6 +16,7 @@ config ARMADA_32BIT
> > > > >        select SUPPORT_SPL
> > > > >        select TRANSLATION_OFFSET
> > > > >        select SPL_SYS_NO_VECTOR_TABLE if SPL
> > > > > +    select ARCH_VERY_EARLY_INIT
> > > > >    config ARMADA_64BIT
> > > > >        bool
> > > > > diff --git a/arch/arm/mach-mvebu/Makefile
> > > > > b/arch/arm/mach-mvebu/Makefile
> > > > > index 1b451889d242..8bd2246325ca 100644
> > > > > --- a/arch/arm/mach-mvebu/Makefile
> > > > > +++ b/arch/arm/mach-mvebu/Makefile
> > > > > @@ -21,6 +21,7 @@ else # CONFIG_ARCH_KIRKWOOD
> > > > >    obj-y    = cpu.o
> > > > >    obj-y    += dram.o
> > > > > +obj-y    += lowlevel.o
> > > > >    obj-$(CONFIG_DM_RESET) += system-controller.o
> > > > >    ifndef CONFIG_SPL_BUILD
> > > > >    obj-$(CONFIG_ARMADA_375) += ../../../drivers/ddr/marvell/axp/xor.o
> > > > > diff --git a/arch/arm/mach-mvebu/cpu.c b/arch/arm/mach-mvebu/cpu.c
> > > > > index 1e893777b292..173d95a760a3 100644
> > > > > --- a/arch/arm/mach-mvebu/cpu.c
> > > > > +++ b/arch/arm/mach-mvebu/cpu.c
> > > > > @@ -413,20 +413,7 @@ static void update_sdram_window_sizes(void)
> > > > >        }
> > > > >    }
> > > > > -void mmu_disable(void)
> > > > > -{
> > > > > -    asm volatile(
> > > > > -        "mrc p15, 0, r0, c1, c0, 0\n"
> > > > > -        "bic r0, #1\n"
> > > > > -        "mcr p15, 0, r0, c1, c0, 0\n");
> > > > > -}
> > > > > -
> > > > >    #ifdef CONFIG_ARCH_CPU_INIT
> > > > > -static void set_cbar(u32 addr)
> > > > > -{
> > > > > -    asm("mcr p15, 4, %0, c15, c0" : : "r" (addr));
> > > > > -}
> > > > > -
> > > > >    #define MV_USB_PHY_BASE            (MVEBU_AXP_USB_BASE + 0x800)
> > > > >    #define MV_USB_PHY_PLL_REG(reg)        (MV_USB_PHY_BASE |
> > > > > (((reg) & 0xF) << 2))
> > > > >    #define MV_USB_X3_BASE(addr)        (MVEBU_AXP_USB_BASE |
> > > > > BIT(11) | \
> > > > > @@ -476,24 +463,6 @@ int arch_cpu_init(void)
> > > > >        struct pl310_regs *const pl310 =
> > > > >            (struct pl310_regs *)CONFIG_SYS_PL310_BASE;
> > > > > -    /*
> > > > > -     * Only with disabled MMU its possible to switch the base
> > > > > -     * register address on Armada 38x. Without this the SDRAM
> > > > > -     * located at >= 0x4000.0000 is also not accessible, as its
> > > > > -     * still locked to cache.
> > > > > -     */
> > > > > -    mmu_disable();
> > > > > -
> > > > > -    /* Linux expects the internal registers to be at 0xf1000000 */
> > > > > -    writel(SOC_REGS_PHY_BASE, INTREG_BASE_ADDR_REG);
> > > > > -    set_cbar(SOC_REGS_PHY_BASE + 0xC000);
> > > > > -
> > > > > -    /*
> > > > > -     * From this stage on, the SoC detection is working. As we have
> > > > > -     * configured the internal register base to the value used
> > > > > -     * in the macros / defines in the U-Boot header (soc.h).
> > > > > -     */
> > > > > -
> > > > >        if (mvebu_soc_family() == MVEBU_SOC_A38X) {
> > > > >            /*
> > > > >             * To fully release / unlock this area from cache, we need
> > > > > diff --git a/arch/arm/mach-mvebu/lowlevel.S
> > > > > b/arch/arm/mach-mvebu/lowlevel.S
> > > > > new file mode 100644
> > > > > index 000000000000..2491310eb0c1
> > > > > --- /dev/null
> > > > > +++ b/arch/arm/mach-mvebu/lowlevel.S
> > > > > @@ -0,0 +1,27 @@
> > > > > +/* SPDX-License-Identifier: GPL-2.0+ */
> > > > > +
> > > > > +#include <config.h>
> > > > > +#include <linux/linkage.h>
> > > > > +
> > > > > +ENTRY(arch_very_early_init)
> > > > > +#ifdef CONFIG_ARMADA_38X
> > > > > +    /*
> > > > > +     * Only with disabled MMU its possible to switch the base
> > > > > +     * register address on Armada 38x. Without this the SDRAM
> > > > > +     * located at >= 0x4000.0000 is also not accessible, as its
> > > > > +     * still locked to cache.
> > > > > +     */
> > > > > +    mrc    p15, 0, r0, c1, c0, 0
> > > > > +    bic    r0, #1
> > > > > +    mcr    p15, 0, r0, c1, c0, 0
> > > > > +#endif
> > > > 
> > > > You are disabling the MMU here only for CONFIG_ARMADA_38X. In the
> > > > non-asm version removed above, it's disabled in all cases in
> > > > arch_cpu_init(). Is this correct? I might be missing something - it's
> > > > been quite some time, since I worked on this code.
> > > 
> > > I'm not sure. I was in impression from that comment that disabling MMU
> > > is needed only for A38x. But runtime detection of the board was not
> > > possible until registers are relocated to the new location, so code
> > > disabling MMU was called always.
> > > 
> > > So, this change should be tested on some AXP, A370 or A375 board to
> > > verify that everything is working correctly.
> > 
> > Okay, understood. I can do some testing on AXP.
> 
> Some quick tests on theadorable_debug (AXP) have shown, that it makes
> no difference, if the MMU is disabled or not AFAICT.

Ok!

> While doing this I noticed though, that kwboot UART booting only worked
> in roughly 1 out of 2 cases. With no progress after this line:
> 
> Sending boot message. Please reboot the target...\
> 
> IIRC, this worked more reliable a few weeks ago. Any idea, what might
> have caused this regression - if I am correct here?

There were changes in handling of bootrom boot message. There is option
-s which can change default timeout option. And plus there is option -a
which sets this timeout option to AXP value.

Default value is 50 and for AXP default value is 1000.

Maybe it is needed to tune AXP value... Try to call with -s option
between 50 and 1000...

> Thanks,
> Stefan
> 
> > Thanks,
> > Stefan
> > 
> > > I tested it only on A385 Turris Omnia.
> > > 
> > > > Thanks,
> > > > Stefan
> > > > 
> > > > > +
> > > > > +    /* Move internal registers from INTREG_BASE_ADDR_REG to
> > > > > SOC_REGS_PHY_BASE */
> > > > > +    ldr    r0, =SOC_REGS_PHY_BASE
> > > > > +    ldr    r1, =INTREG_BASE_ADDR_REG
> > > > > +    str    r0, [r1]
> > > > > +    add    r0, r0, #0xC000
> > > > > +    mcr    p15, 4, r0, c15, c0
> > > > > +
> > > > > +    bx lr
> > > > > +ENDPROC(arch_very_early_init)
> > > > 
> > > > Viele Grüße,
> > > > Stefan Roese
> > > > 
> > > > -- 
> > > > DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
> > > > HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> > > > Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr@denx.de
> > 
> > Viele Grüße,
> > Stefan Roese
> > 
> 
> Viele Grüße,
> Stefan Roese
> 
> -- 
> DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr@denx.de

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [PATCH 6/5] arm: mvebu: Fix DEBUG_UART_BASE for all 32-bit boards
  2022-05-06  9:05 [PATCH 0/5] arm: mvebu: turris_omnia: Fix hangup in debug UART Pali Rohár
                   ` (4 preceding siblings ...)
  2022-05-06  9:05 ` [PATCH 5/5] arm: mvebu: turris_omnia: Fix DEBUG_UART_BASE Pali Rohár
@ 2022-05-09 18:17 ` Pali Rohár
  2022-05-16  6:39   ` Stefan Roese
  2022-05-16 14:52 ` [PATCH 0/5] arm: mvebu: turris_omnia: Fix hangup in debug UART Stefan Roese
                   ` (2 subsequent siblings)
  8 siblings, 1 reply; 32+ messages in thread
From: Pali Rohár @ 2022-05-09 18:17 UTC (permalink / raw)
  To: Stefan Roese, Marek Behun; +Cc: u-boot

UART base address is located in internal registers.

Internal registers for 32-bit mvebu boards in SPL are at address 0xd0000000
and in proper U-Boot at address 0xf1000000.

Fix DEBUG_UART_BASE option for all 32-bit mvebu boards.

Signed-off-by: Pali Rohár <pali@kernel.org>
---
 configs/clearfog_defconfig          | 3 ++-
 configs/controlcenterdc_defconfig   | 3 ++-
 configs/db-88f6820-amc_defconfig    | 3 ++-
 configs/db-88f6820-gp_defconfig     | 3 ++-
 configs/db-mv784mp-gp_defconfig     | 3 ++-
 configs/ds414_defconfig             | 3 ++-
 configs/helios4_defconfig           | 3 ++-
 configs/maxbcm_defconfig            | 3 ++-
 configs/theadorable_debug_defconfig | 3 ++-
 configs/x530_defconfig              | 3 ++-
 10 files changed, 20 insertions(+), 10 deletions(-)

diff --git a/configs/clearfog_defconfig b/configs/clearfog_defconfig
index a7b6508a5c6d..9f744d0caf96 100644
--- a/configs/clearfog_defconfig
+++ b/configs/clearfog_defconfig
@@ -13,7 +13,7 @@ CONFIG_DEFAULT_DEVICE_TREE="armada-388-clearfog"
 CONFIG_SPL_TEXT_BASE=0x40000030
 CONFIG_SPL_SERIAL=y
 CONFIG_SPL=y
-CONFIG_DEBUG_UART_BASE=0xd0012000
+CONFIG_DEBUG_UART_BASE=0xf1012000
 CONFIG_DEBUG_UART_CLOCK=250000000
 CONFIG_SYS_LOAD_ADDR=0x800000
 CONFIG_DEBUG_UART=y
@@ -66,6 +66,7 @@ CONFIG_MVMDIO=y
 CONFIG_PCI=y
 CONFIG_PCI_MVEBU=y
 CONFIG_SCSI=y
+CONFIG_SPL_DEBUG_UART_BASE=0xd0012000
 CONFIG_DEBUG_UART_SHIFT=2
 CONFIG_SYS_NS16550=y
 CONFIG_KIRKWOOD_SPI=y
diff --git a/configs/controlcenterdc_defconfig b/configs/controlcenterdc_defconfig
index df38b2c54f08..c366db40e6fa 100644
--- a/configs/controlcenterdc_defconfig
+++ b/configs/controlcenterdc_defconfig
@@ -16,7 +16,7 @@ CONFIG_DEFAULT_DEVICE_TREE="armada-38x-controlcenterdc"
 CONFIG_SPL_TEXT_BASE=0x40000030
 CONFIG_SPL_SERIAL=y
 CONFIG_SPL=y
-CONFIG_DEBUG_UART_BASE=0xd0012000
+CONFIG_DEBUG_UART_BASE=0xf1012000
 CONFIG_DEBUG_UART_CLOCK=250000000
 CONFIG_SYS_LOAD_ADDR=0x800000
 CONFIG_DEBUG_UART=y
@@ -84,6 +84,7 @@ CONFIG_PCI_MVEBU=y
 CONFIG_SCSI=y
 CONFIG_SCSI_AHCI_PLAT=y
 CONFIG_SYS_SCSI_MAX_SCSI_ID=2
+CONFIG_SPL_DEBUG_UART_BASE=0xd0012000
 CONFIG_DEBUG_UART_SHIFT=2
 CONFIG_SYS_NS16550=y
 CONFIG_KIRKWOOD_SPI=y
diff --git a/configs/db-88f6820-amc_defconfig b/configs/db-88f6820-amc_defconfig
index 9b77b4a5f057..be4ee7913513 100644
--- a/configs/db-88f6820-amc_defconfig
+++ b/configs/db-88f6820-amc_defconfig
@@ -13,7 +13,7 @@ CONFIG_DEFAULT_DEVICE_TREE="armada-385-db-88f6820-amc"
 CONFIG_SPL_TEXT_BASE=0x40000030
 CONFIG_SPL_SERIAL=y
 CONFIG_SPL=y
-CONFIG_DEBUG_UART_BASE=0xd0012000
+CONFIG_DEBUG_UART_BASE=0xf1012000
 CONFIG_DEBUG_UART_CLOCK=200000000
 CONFIG_SYS_LOAD_ADDR=0x800000
 CONFIG_DEBUG_UART=y
@@ -70,6 +70,7 @@ CONFIG_MII=y
 CONFIG_MVMDIO=y
 CONFIG_PCI=y
 CONFIG_PCI_MVEBU=y
+CONFIG_SPL_DEBUG_UART_BASE=0xd0012000
 CONFIG_DEBUG_UART_SHIFT=2
 CONFIG_SYS_NS16550=y
 CONFIG_KIRKWOOD_SPI=y
diff --git a/configs/db-88f6820-gp_defconfig b/configs/db-88f6820-gp_defconfig
index f56d1fbf2530..55ebb57c692e 100644
--- a/configs/db-88f6820-gp_defconfig
+++ b/configs/db-88f6820-gp_defconfig
@@ -13,7 +13,7 @@ CONFIG_DEFAULT_DEVICE_TREE="armada-388-gp"
 CONFIG_SPL_TEXT_BASE=0x40000030
 CONFIG_SPL_SERIAL=y
 CONFIG_SPL=y
-CONFIG_DEBUG_UART_BASE=0xd0012000
+CONFIG_DEBUG_UART_BASE=0xf1012000
 CONFIG_DEBUG_UART_CLOCK=250000000
 CONFIG_SYS_LOAD_ADDR=0x800000
 CONFIG_DEBUG_UART=y
@@ -66,6 +66,7 @@ CONFIG_MVMDIO=y
 CONFIG_PCI=y
 CONFIG_PCI_MVEBU=y
 CONFIG_SCSI=y
+CONFIG_SPL_DEBUG_UART_BASE=0xd0012000
 CONFIG_DEBUG_UART_SHIFT=2
 CONFIG_SYS_NS16550=y
 CONFIG_KIRKWOOD_SPI=y
diff --git a/configs/db-mv784mp-gp_defconfig b/configs/db-mv784mp-gp_defconfig
index 5683f118365f..2c1d3b464d85 100644
--- a/configs/db-mv784mp-gp_defconfig
+++ b/configs/db-mv784mp-gp_defconfig
@@ -13,7 +13,7 @@ CONFIG_DEFAULT_DEVICE_TREE="armada-xp-gp"
 CONFIG_SPL_TEXT_BASE=0x40004030
 CONFIG_SPL_SERIAL=y
 CONFIG_SPL=y
-CONFIG_DEBUG_UART_BASE=0xd0012000
+CONFIG_DEBUG_UART_BASE=0xf1012000
 CONFIG_DEBUG_UART_CLOCK=250000000
 CONFIG_SYS_LOAD_ADDR=0x800000
 CONFIG_DEBUG_UART=y
@@ -68,6 +68,7 @@ CONFIG_MII=y
 CONFIG_MVMDIO=y
 CONFIG_PCI=y
 CONFIG_PCI_MVEBU=y
+CONFIG_SPL_DEBUG_UART_BASE=0xd0012000
 CONFIG_DEBUG_UART_SHIFT=2
 CONFIG_SYS_NS16550=y
 CONFIG_KIRKWOOD_SPI=y
diff --git a/configs/ds414_defconfig b/configs/ds414_defconfig
index a83fe079b3a1..81a767cc0ea8 100644
--- a/configs/ds414_defconfig
+++ b/configs/ds414_defconfig
@@ -19,7 +19,7 @@ CONFIG_DEFAULT_DEVICE_TREE="armada-xp-synology-ds414"
 CONFIG_SPL_TEXT_BASE=0x40004030
 CONFIG_SPL_SERIAL=y
 CONFIG_SPL=y
-CONFIG_DEBUG_UART_BASE=0xd0012000
+CONFIG_DEBUG_UART_BASE=0xf1012000
 CONFIG_DEBUG_UART_CLOCK=250000000
 CONFIG_SYS_LOAD_ADDR=0x800000
 CONFIG_DEBUG_UART=y
@@ -68,6 +68,7 @@ CONFIG_MII=y
 CONFIG_MVMDIO=y
 CONFIG_PCI=y
 CONFIG_PCI_MVEBU=y
+CONFIG_SPL_DEBUG_UART_BASE=0xd0012000
 CONFIG_DEBUG_UART_SHIFT=2
 CONFIG_SYS_NS16550=y
 CONFIG_KIRKWOOD_SPI=y
diff --git a/configs/helios4_defconfig b/configs/helios4_defconfig
index c2130bacb4b4..4dc9e3bc58dd 100644
--- a/configs/helios4_defconfig
+++ b/configs/helios4_defconfig
@@ -13,7 +13,7 @@ CONFIG_DEFAULT_DEVICE_TREE="armada-388-helios4"
 CONFIG_SPL_TEXT_BASE=0x40000030
 CONFIG_SPL_SERIAL=y
 CONFIG_SPL=y
-CONFIG_DEBUG_UART_BASE=0xd0012000
+CONFIG_DEBUG_UART_BASE=0xf1012000
 CONFIG_DEBUG_UART_CLOCK=250000000
 CONFIG_SYS_LOAD_ADDR=0x800000
 CONFIG_DEBUG_UART=y
@@ -67,6 +67,7 @@ CONFIG_MVMDIO=y
 CONFIG_PCI=y
 CONFIG_PCI_MVEBU=y
 CONFIG_SCSI=y
+CONFIG_SPL_DEBUG_UART_BASE=0xd0012000
 CONFIG_DEBUG_UART_SHIFT=2
 CONFIG_SYS_NS16550=y
 CONFIG_KIRKWOOD_SPI=y
diff --git a/configs/maxbcm_defconfig b/configs/maxbcm_defconfig
index 40f79d47ea98..c325dd7cb272 100644
--- a/configs/maxbcm_defconfig
+++ b/configs/maxbcm_defconfig
@@ -13,7 +13,7 @@ CONFIG_DEFAULT_DEVICE_TREE="armada-xp-maxbcm"
 CONFIG_SPL_TEXT_BASE=0x40004030
 CONFIG_SPL_SERIAL=y
 CONFIG_SPL=y
-CONFIG_DEBUG_UART_BASE=0xd0012000
+CONFIG_DEBUG_UART_BASE=0xf1012000
 CONFIG_DEBUG_UART_CLOCK=250000000
 CONFIG_SYS_LOAD_ADDR=0x800000
 CONFIG_DEBUG_UART=y
@@ -48,6 +48,7 @@ CONFIG_PHY_GIGE=y
 CONFIG_MVNETA=y
 CONFIG_MII=y
 CONFIG_MVMDIO=y
+CONFIG_SPL_DEBUG_UART_BASE=0xd0012000
 CONFIG_DEBUG_UART_SHIFT=2
 CONFIG_SYS_NS16550=y
 CONFIG_KIRKWOOD_SPI=y
diff --git a/configs/theadorable_debug_defconfig b/configs/theadorable_debug_defconfig
index 111392d9a941..9e8523d94145 100644
--- a/configs/theadorable_debug_defconfig
+++ b/configs/theadorable_debug_defconfig
@@ -14,7 +14,7 @@ CONFIG_DEFAULT_DEVICE_TREE="armada-xp-theadorable"
 CONFIG_SPL_TEXT_BASE=0x40004030
 CONFIG_SPL_SERIAL=y
 CONFIG_SPL=y
-CONFIG_DEBUG_UART_BASE=0xd0012000
+CONFIG_DEBUG_UART_BASE=0xf1012000
 CONFIG_DEBUG_UART_CLOCK=250000000
 CONFIG_SYS_MEM_TOP_HIDE=0x80000
 CONFIG_SYS_LOAD_ADDR=0x800000
@@ -74,6 +74,7 @@ CONFIG_MVMDIO=y
 CONFIG_PCI=y
 CONFIG_DM_PCI_COMPAT=y
 CONFIG_PCI_MVEBU=y
+CONFIG_SPL_DEBUG_UART_BASE=0xd0012000
 CONFIG_DEBUG_UART_SHIFT=2
 CONFIG_SYS_NS16550=y
 CONFIG_KIRKWOOD_SPI=y
diff --git a/configs/x530_defconfig b/configs/x530_defconfig
index 77d224942764..860cb2207139 100644
--- a/configs/x530_defconfig
+++ b/configs/x530_defconfig
@@ -14,7 +14,7 @@ CONFIG_DEFAULT_DEVICE_TREE="armada-385-atl-x530"
 CONFIG_SPL_TEXT_BASE=0x40000030
 CONFIG_SPL_SERIAL=y
 CONFIG_SPL=y
-CONFIG_DEBUG_UART_BASE=0xd0012000
+CONFIG_DEBUG_UART_BASE=0xf1012000
 CONFIG_DEBUG_UART_CLOCK=250000000
 CONFIG_SYS_LOAD_ADDR=0x1000000
 CONFIG_ENV_ADDR=0x100000
@@ -72,6 +72,7 @@ CONFIG_PCI_MVEBU=y
 CONFIG_DM_RTC=y
 CONFIG_RTC_DS1307=y
 CONFIG_SPECIFY_CONSOLE_INDEX=y
+CONFIG_SPL_DEBUG_UART_BASE=0xd0012000
 CONFIG_DEBUG_UART_SHIFT=2
 CONFIG_SYS_NS16550=y
 CONFIG_KIRKWOOD_SPI=y
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* Re: [PATCH 5/5] arm: mvebu: turris_omnia: Fix DEBUG_UART_BASE
  2022-05-06 12:30     ` Pali Rohár
@ 2022-05-09 18:18       ` Pali Rohár
  0 siblings, 0 replies; 32+ messages in thread
From: Pali Rohár @ 2022-05-09 18:18 UTC (permalink / raw)
  To: Stefan Roese; +Cc: Marek Behun, u-boot

On Friday 06 May 2022 14:30:22 Pali Rohár wrote:
> On Friday 06 May 2022 14:21:02 Stefan Roese wrote:
> > On 06.05.22 11:05, Pali Rohár wrote:
> > > Internal registers in SPL are at address 0xd0000000 and in proper U-Boot at
> > > address 0xf1000000. UART base address is located in internal registers.
> > > 
> > > Fix DEBUG_UART_BASE option to correct value for both SPL and proper U-Boot.
> > > 
> > > This change fixes hangup of proper U-Boot when it is trying to print
> > > something via debug UART.
> > > 
> > > Signed-off-by: Pali Rohár <pali@kernel.org>
> > > ---
> > >   configs/turris_omnia_defconfig | 3 ++-
> > >   1 file changed, 2 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/configs/turris_omnia_defconfig b/configs/turris_omnia_defconfig
> > > index 217e2603fdfb..62c9be29c86b 100644
> > > --- a/configs/turris_omnia_defconfig
> > > +++ b/configs/turris_omnia_defconfig
> > > @@ -18,7 +18,7 @@ CONFIG_DEFAULT_DEVICE_TREE="armada-385-turris-omnia"
> > >   CONFIG_SPL_TEXT_BASE=0x40000030
> > >   CONFIG_SPL_SERIAL=y
> > >   CONFIG_SPL=y
> > > -CONFIG_DEBUG_UART_BASE=0xd0012000
> > > +CONFIG_DEBUG_UART_BASE=0xf1012000
> > >   CONFIG_DEBUG_UART_CLOCK=250000000
> > >   CONFIG_SYS_LOAD_ADDR=0x800000
> > >   CONFIG_DEBUG_UART=y
> > > @@ -93,6 +93,7 @@ CONFIG_PCI_MVEBU=y
> > >   CONFIG_DM_RTC=y
> > >   CONFIG_RTC_ARMADA38X=y
> > >   CONFIG_SCSI=y
> > > +CONFIG_SPL_DEBUG_UART_BASE=0xd0012000
> > >   CONFIG_DEBUG_UART_SHIFT=2
> > >   CONFIG_SYS_NS16550=y
> > >   CONFIG_KIRKWOOD_SPI=y
> > 
> > Isn't such a change now needed for all 32bit Armada MVEBU targets?
> 
> This is a good question. It should be fixed for all 32bit mvebu targets.

Now I sent additional patch which fixes this for all other 32bit mvebu
platforms which have CONFIG_DEBUG_UART_BASE in defconfig file.

> > Viele Grüße,
> > Stefan Roese
> > 
> > -- 
> > DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
> > HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> > Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr@denx.de

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 1/5] arm: Add new config option ARCH_VERY_EARLY_INIT
  2022-05-06  9:05 ` [PATCH 1/5] arm: Add new config option ARCH_VERY_EARLY_INIT Pali Rohár
@ 2022-05-16  6:37   ` Stefan Roese
  0 siblings, 0 replies; 32+ messages in thread
From: Stefan Roese @ 2022-05-16  6:37 UTC (permalink / raw)
  To: Pali Rohár, Marek Behun; +Cc: u-boot

On 06.05.22 11:05, Pali Rohár wrote:
> When this option is set then ARM _main() function would call
> arch_very_early_init() function at the beginning. It would be before
> calling any other functions like debug_uart_init() and also before
> initializing C runtime environment.
> 
> Signed-off-by: Pali Rohár <pali@kernel.org>

Reviewed-by: Stefan Roese <sr@denx.de>

Thanks,
Stefan

> ---
>   arch/arm/Kconfig    | 6 ++++++
>   arch/arm/lib/crt0.S | 5 +++++
>   2 files changed, 11 insertions(+)
> 
> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> index 0afec5155b1b..9898c7d68e1b 100644
> --- a/arch/arm/Kconfig
> +++ b/arch/arm/Kconfig
> @@ -401,6 +401,12 @@ config SYS_ARM_CACHE_WRITEALLOC
>   	  write is performed.
>   endchoice
>   
> +config ARCH_VERY_EARLY_INIT
> +	bool
> +
> +config SPL_ARCH_VERY_EARLY_INIT
> +	bool
> +
>   config ARCH_CPU_INIT
>   	bool "Enable ARCH_CPU_INIT"
>   	help
> diff --git a/arch/arm/lib/crt0.S b/arch/arm/lib/crt0.S
> index ba312901f331..612a2d5b698e 100644
> --- a/arch/arm/lib/crt0.S
> +++ b/arch/arm/lib/crt0.S
> @@ -90,6 +90,11 @@ clbss_l:cmp	r0, r1			/* while not at end of BSS */
>   
>   ENTRY(_main)
>   
> +/* Call arch_very_early_init before initializing C runtime environment. */
> +#if CONFIG_IS_ENABLED(ARCH_VERY_EARLY_INIT)
> +	bl	arch_very_early_init
> +#endif
> +
>   /*
>    * Set up initial C runtime environment and call board_init_f(0).
>    */

Viele Grüße,
Stefan Roese

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr@denx.de

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 2/5] arm: mvebu: Move internal registers in arch_very_early_init() function
  2022-05-06  9:05 ` [PATCH 2/5] arm: mvebu: Move internal registers in arch_very_early_init() function Pali Rohár
  2022-05-06 12:04   ` Stefan Roese
@ 2022-05-16  6:37   ` Stefan Roese
  1 sibling, 0 replies; 32+ messages in thread
From: Stefan Roese @ 2022-05-16  6:37 UTC (permalink / raw)
  To: Pali Rohár, Marek Behun; +Cc: u-boot

On 06.05.22 11:05, Pali Rohár wrote:
> Moving of internal registers from INTREG_BASE_ADDR_REG to SOC_REGS_PHY_BASE
> needs to be done very early, prior calling any function which may touch
> internal registers, like debug_uart_init().
> 
> So do it earlier in arch_very_early_init() instead of arch_cpu_init().
> 
> Movement is done in proper U-Boot, not in SPL. SPL may return to bootrom
> and bootrom requires internal registers at (old) expected location.
> 
> Signed-off-by: Pali Rohár <pali@kernel.org>

Reviewed-by: Stefan Roese <sr@denx.de>

Thanks,
Stefan

> ---
>   arch/arm/mach-mvebu/Kconfig    |  1 +
>   arch/arm/mach-mvebu/Makefile   |  1 +
>   arch/arm/mach-mvebu/cpu.c      | 31 -------------------------------
>   arch/arm/mach-mvebu/lowlevel.S | 27 +++++++++++++++++++++++++++
>   4 files changed, 29 insertions(+), 31 deletions(-)
>   create mode 100644 arch/arm/mach-mvebu/lowlevel.S
> 
> diff --git a/arch/arm/mach-mvebu/Kconfig b/arch/arm/mach-mvebu/Kconfig
> index a3f273f4f949..98b13d1e336d 100644
> --- a/arch/arm/mach-mvebu/Kconfig
> +++ b/arch/arm/mach-mvebu/Kconfig
> @@ -16,6 +16,7 @@ config ARMADA_32BIT
>   	select SUPPORT_SPL
>   	select TRANSLATION_OFFSET
>   	select SPL_SYS_NO_VECTOR_TABLE if SPL
> +	select ARCH_VERY_EARLY_INIT
>   
>   config ARMADA_64BIT
>   	bool
> diff --git a/arch/arm/mach-mvebu/Makefile b/arch/arm/mach-mvebu/Makefile
> index 1b451889d242..8bd2246325ca 100644
> --- a/arch/arm/mach-mvebu/Makefile
> +++ b/arch/arm/mach-mvebu/Makefile
> @@ -21,6 +21,7 @@ else # CONFIG_ARCH_KIRKWOOD
>   
>   obj-y	= cpu.o
>   obj-y	+= dram.o
> +obj-y	+= lowlevel.o
>   obj-$(CONFIG_DM_RESET) += system-controller.o
>   ifndef CONFIG_SPL_BUILD
>   obj-$(CONFIG_ARMADA_375) += ../../../drivers/ddr/marvell/axp/xor.o
> diff --git a/arch/arm/mach-mvebu/cpu.c b/arch/arm/mach-mvebu/cpu.c
> index 1e893777b292..173d95a760a3 100644
> --- a/arch/arm/mach-mvebu/cpu.c
> +++ b/arch/arm/mach-mvebu/cpu.c
> @@ -413,20 +413,7 @@ static void update_sdram_window_sizes(void)
>   	}
>   }
>   
> -void mmu_disable(void)
> -{
> -	asm volatile(
> -		"mrc p15, 0, r0, c1, c0, 0\n"
> -		"bic r0, #1\n"
> -		"mcr p15, 0, r0, c1, c0, 0\n");
> -}
> -
>   #ifdef CONFIG_ARCH_CPU_INIT
> -static void set_cbar(u32 addr)
> -{
> -	asm("mcr p15, 4, %0, c15, c0" : : "r" (addr));
> -}
> -
>   #define MV_USB_PHY_BASE			(MVEBU_AXP_USB_BASE + 0x800)
>   #define MV_USB_PHY_PLL_REG(reg)		(MV_USB_PHY_BASE | (((reg) & 0xF) << 2))
>   #define MV_USB_X3_BASE(addr)		(MVEBU_AXP_USB_BASE | BIT(11) | \
> @@ -476,24 +463,6 @@ int arch_cpu_init(void)
>   	struct pl310_regs *const pl310 =
>   		(struct pl310_regs *)CONFIG_SYS_PL310_BASE;
>   
> -	/*
> -	 * Only with disabled MMU its possible to switch the base
> -	 * register address on Armada 38x. Without this the SDRAM
> -	 * located at >= 0x4000.0000 is also not accessible, as its
> -	 * still locked to cache.
> -	 */
> -	mmu_disable();
> -
> -	/* Linux expects the internal registers to be at 0xf1000000 */
> -	writel(SOC_REGS_PHY_BASE, INTREG_BASE_ADDR_REG);
> -	set_cbar(SOC_REGS_PHY_BASE + 0xC000);
> -
> -	/*
> -	 * From this stage on, the SoC detection is working. As we have
> -	 * configured the internal register base to the value used
> -	 * in the macros / defines in the U-Boot header (soc.h).
> -	 */
> -
>   	if (mvebu_soc_family() == MVEBU_SOC_A38X) {
>   		/*
>   		 * To fully release / unlock this area from cache, we need
> diff --git a/arch/arm/mach-mvebu/lowlevel.S b/arch/arm/mach-mvebu/lowlevel.S
> new file mode 100644
> index 000000000000..2491310eb0c1
> --- /dev/null
> +++ b/arch/arm/mach-mvebu/lowlevel.S
> @@ -0,0 +1,27 @@
> +/* SPDX-License-Identifier: GPL-2.0+ */
> +
> +#include <config.h>
> +#include <linux/linkage.h>
> +
> +ENTRY(arch_very_early_init)
> +#ifdef CONFIG_ARMADA_38X
> +	/*
> +	 * Only with disabled MMU its possible to switch the base
> +	 * register address on Armada 38x. Without this the SDRAM
> +	 * located at >= 0x4000.0000 is also not accessible, as its
> +	 * still locked to cache.
> +	 */
> +	mrc	p15, 0, r0, c1, c0, 0
> +	bic	r0, #1
> +	mcr	p15, 0, r0, c1, c0, 0
> +#endif
> +
> +	/* Move internal registers from INTREG_BASE_ADDR_REG to SOC_REGS_PHY_BASE */
> +	ldr	r0, =SOC_REGS_PHY_BASE
> +	ldr	r1, =INTREG_BASE_ADDR_REG
> +	str	r0, [r1]
> +	add	r0, r0, #0xC000
> +	mcr	p15, 4, r0, c15, c0
> +
> +	bx lr
> +ENDPROC(arch_very_early_init)

Viele Grüße,
Stefan Roese

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr@denx.de

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 3/5] serial: Add new config option SPL_DEBUG_UART_BASE
  2022-05-06  9:05 ` [PATCH 3/5] serial: Add new config option SPL_DEBUG_UART_BASE Pali Rohár
@ 2022-05-16  6:37   ` Stefan Roese
  0 siblings, 0 replies; 32+ messages in thread
From: Stefan Roese @ 2022-05-16  6:37 UTC (permalink / raw)
  To: Pali Rohár, Marek Behun; +Cc: u-boot

On 06.05.22 11:05, Pali Rohár wrote:
> SPL_DEBUG_UART_BASE is same as DEBUG_UART_BASE, but applies only for SPL.
> 
> In some cases base address of UART is different in SPL and proper U-Boot.
> 
> Signed-off-by: Pali Rohár <pali@kernel.org>

Reviewed-by: Stefan Roese <sr@denx.de>

Thanks,
Stefan

> ---
>   drivers/serial/Kconfig | 7 +++++++
>   1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/serial/Kconfig b/drivers/serial/Kconfig
> index 26fa498bbbb7..46726d76b162 100644
> --- a/drivers/serial/Kconfig
> +++ b/drivers/serial/Kconfig
> @@ -508,6 +508,13 @@ config DEBUG_UART_BASE
>   	  A default should be provided by your board, but if not you will need
>   	  to use the correct value here.
>   
> +config SPL_DEBUG_UART_BASE
> +	hex "Base address of UART for SPL"
> +	depends on SPL && DEBUG_UART
> +	default DEBUG_UART_BASE
> +	help
> +	  This is the base address of your UART for memory-mapped UARTs for SPL.
> +
>   config DEBUG_UART_CLOCK
>   	int "UART input clock"
>   	depends on DEBUG_UART

Viele Grüße,
Stefan Roese

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr@denx.de

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 4/5] serial: ns16550: Add support for SPL_DEBUG_UART_BASE
  2022-05-06  9:05 ` [PATCH 4/5] serial: ns16550: Add support for SPL_DEBUG_UART_BASE Pali Rohár
@ 2022-05-16  6:38   ` Stefan Roese
  0 siblings, 0 replies; 32+ messages in thread
From: Stefan Roese @ 2022-05-16  6:38 UTC (permalink / raw)
  To: Pali Rohár, Marek Behun; +Cc: u-boot

On 06.05.22 11:05, Pali Rohár wrote:
> Use CONFIG_VAL(DEBUG_UART_BASE) instead of CONFIG_DEBUG_UART_BASE, so
> proper config value (CONFIG_DEBUG_UART_BASE or CONFIG_SPL_DEBUG_UART_BASE)
> is used based on building target.
> 
> Signed-off-by: Pali Rohár <pali@kernel.org>

Reviewed-by: Stefan Roese <sr@denx.de>

Thanks,
Stefan

> ---
>   drivers/serial/ns16550.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/serial/ns16550.c b/drivers/serial/ns16550.c
> index a4220fd0ae2f..78bfe6281ce3 100644
> --- a/drivers/serial/ns16550.c
> +++ b/drivers/serial/ns16550.c
> @@ -325,7 +325,7 @@ int ns16550_tstc(struct ns16550 *com_port)
>   
>   static inline void _debug_uart_init(void)
>   {
> -	struct ns16550 *com_port = (struct ns16550 *)CONFIG_DEBUG_UART_BASE;
> +	struct ns16550 *com_port = (struct ns16550 *)CONFIG_VAL(DEBUG_UART_BASE);
>   	int baud_divisor;
>   
>   	/*
> @@ -360,7 +360,7 @@ static inline int NS16550_read_baud_divisor(struct ns16550 *com_port)
>   
>   static inline void _debug_uart_putc(int ch)
>   {
> -	struct ns16550 *com_port = (struct ns16550 *)CONFIG_DEBUG_UART_BASE;
> +	struct ns16550 *com_port = (struct ns16550 *)CONFIG_VAL(DEBUG_UART_BASE);
>   
>   	while (!(serial_din(&com_port->lsr) & UART_LSR_THRE)) {
>   #ifdef CONFIG_DEBUG_UART_NS16550_CHECK_ENABLED

Viele Grüße,
Stefan Roese

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr@denx.de

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 5/5] arm: mvebu: turris_omnia: Fix DEBUG_UART_BASE
  2022-05-06  9:05 ` [PATCH 5/5] arm: mvebu: turris_omnia: Fix DEBUG_UART_BASE Pali Rohár
  2022-05-06 12:21   ` Stefan Roese
@ 2022-05-16  6:38   ` Stefan Roese
  1 sibling, 0 replies; 32+ messages in thread
From: Stefan Roese @ 2022-05-16  6:38 UTC (permalink / raw)
  To: Pali Rohár, Marek Behun; +Cc: u-boot

On 06.05.22 11:05, Pali Rohár wrote:
> Internal registers in SPL are at address 0xd0000000 and in proper U-Boot at
> address 0xf1000000. UART base address is located in internal registers.
> 
> Fix DEBUG_UART_BASE option to correct value for both SPL and proper U-Boot.
> 
> This change fixes hangup of proper U-Boot when it is trying to print
> something via debug UART.
> 
> Signed-off-by: Pali Rohár <pali@kernel.org>

Reviewed-by: Stefan Roese <sr@denx.de>

Thanks,
Stefan

> ---
>   configs/turris_omnia_defconfig | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/configs/turris_omnia_defconfig b/configs/turris_omnia_defconfig
> index 217e2603fdfb..62c9be29c86b 100644
> --- a/configs/turris_omnia_defconfig
> +++ b/configs/turris_omnia_defconfig
> @@ -18,7 +18,7 @@ CONFIG_DEFAULT_DEVICE_TREE="armada-385-turris-omnia"
>   CONFIG_SPL_TEXT_BASE=0x40000030
>   CONFIG_SPL_SERIAL=y
>   CONFIG_SPL=y
> -CONFIG_DEBUG_UART_BASE=0xd0012000
> +CONFIG_DEBUG_UART_BASE=0xf1012000
>   CONFIG_DEBUG_UART_CLOCK=250000000
>   CONFIG_SYS_LOAD_ADDR=0x800000
>   CONFIG_DEBUG_UART=y
> @@ -93,6 +93,7 @@ CONFIG_PCI_MVEBU=y
>   CONFIG_DM_RTC=y
>   CONFIG_RTC_ARMADA38X=y
>   CONFIG_SCSI=y
> +CONFIG_SPL_DEBUG_UART_BASE=0xd0012000
>   CONFIG_DEBUG_UART_SHIFT=2
>   CONFIG_SYS_NS16550=y
>   CONFIG_KIRKWOOD_SPI=y

Viele Grüße,
Stefan Roese

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr@denx.de

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 6/5] arm: mvebu: Fix DEBUG_UART_BASE for all 32-bit boards
  2022-05-09 18:17 ` [PATCH 6/5] arm: mvebu: Fix DEBUG_UART_BASE for all 32-bit boards Pali Rohár
@ 2022-05-16  6:39   ` Stefan Roese
  0 siblings, 0 replies; 32+ messages in thread
From: Stefan Roese @ 2022-05-16  6:39 UTC (permalink / raw)
  To: Pali Rohár, Marek Behun; +Cc: u-boot

On 09.05.22 20:17, Pali Rohár wrote:
> UART base address is located in internal registers.
> 
> Internal registers for 32-bit mvebu boards in SPL are at address 0xd0000000
> and in proper U-Boot at address 0xf1000000.
> 
> Fix DEBUG_UART_BASE option for all 32-bit mvebu boards.
> 
> Signed-off-by: Pali Rohár <pali@kernel.org>

Reviewed-by: Stefan Roese <sr@denx.de>

Thanks,
Stefan

> ---
>   configs/clearfog_defconfig          | 3 ++-
>   configs/controlcenterdc_defconfig   | 3 ++-
>   configs/db-88f6820-amc_defconfig    | 3 ++-
>   configs/db-88f6820-gp_defconfig     | 3 ++-
>   configs/db-mv784mp-gp_defconfig     | 3 ++-
>   configs/ds414_defconfig             | 3 ++-
>   configs/helios4_defconfig           | 3 ++-
>   configs/maxbcm_defconfig            | 3 ++-
>   configs/theadorable_debug_defconfig | 3 ++-
>   configs/x530_defconfig              | 3 ++-
>   10 files changed, 20 insertions(+), 10 deletions(-)
> 
> diff --git a/configs/clearfog_defconfig b/configs/clearfog_defconfig
> index a7b6508a5c6d..9f744d0caf96 100644
> --- a/configs/clearfog_defconfig
> +++ b/configs/clearfog_defconfig
> @@ -13,7 +13,7 @@ CONFIG_DEFAULT_DEVICE_TREE="armada-388-clearfog"
>   CONFIG_SPL_TEXT_BASE=0x40000030
>   CONFIG_SPL_SERIAL=y
>   CONFIG_SPL=y
> -CONFIG_DEBUG_UART_BASE=0xd0012000
> +CONFIG_DEBUG_UART_BASE=0xf1012000
>   CONFIG_DEBUG_UART_CLOCK=250000000
>   CONFIG_SYS_LOAD_ADDR=0x800000
>   CONFIG_DEBUG_UART=y
> @@ -66,6 +66,7 @@ CONFIG_MVMDIO=y
>   CONFIG_PCI=y
>   CONFIG_PCI_MVEBU=y
>   CONFIG_SCSI=y
> +CONFIG_SPL_DEBUG_UART_BASE=0xd0012000
>   CONFIG_DEBUG_UART_SHIFT=2
>   CONFIG_SYS_NS16550=y
>   CONFIG_KIRKWOOD_SPI=y
> diff --git a/configs/controlcenterdc_defconfig b/configs/controlcenterdc_defconfig
> index df38b2c54f08..c366db40e6fa 100644
> --- a/configs/controlcenterdc_defconfig
> +++ b/configs/controlcenterdc_defconfig
> @@ -16,7 +16,7 @@ CONFIG_DEFAULT_DEVICE_TREE="armada-38x-controlcenterdc"
>   CONFIG_SPL_TEXT_BASE=0x40000030
>   CONFIG_SPL_SERIAL=y
>   CONFIG_SPL=y
> -CONFIG_DEBUG_UART_BASE=0xd0012000
> +CONFIG_DEBUG_UART_BASE=0xf1012000
>   CONFIG_DEBUG_UART_CLOCK=250000000
>   CONFIG_SYS_LOAD_ADDR=0x800000
>   CONFIG_DEBUG_UART=y
> @@ -84,6 +84,7 @@ CONFIG_PCI_MVEBU=y
>   CONFIG_SCSI=y
>   CONFIG_SCSI_AHCI_PLAT=y
>   CONFIG_SYS_SCSI_MAX_SCSI_ID=2
> +CONFIG_SPL_DEBUG_UART_BASE=0xd0012000
>   CONFIG_DEBUG_UART_SHIFT=2
>   CONFIG_SYS_NS16550=y
>   CONFIG_KIRKWOOD_SPI=y
> diff --git a/configs/db-88f6820-amc_defconfig b/configs/db-88f6820-amc_defconfig
> index 9b77b4a5f057..be4ee7913513 100644
> --- a/configs/db-88f6820-amc_defconfig
> +++ b/configs/db-88f6820-amc_defconfig
> @@ -13,7 +13,7 @@ CONFIG_DEFAULT_DEVICE_TREE="armada-385-db-88f6820-amc"
>   CONFIG_SPL_TEXT_BASE=0x40000030
>   CONFIG_SPL_SERIAL=y
>   CONFIG_SPL=y
> -CONFIG_DEBUG_UART_BASE=0xd0012000
> +CONFIG_DEBUG_UART_BASE=0xf1012000
>   CONFIG_DEBUG_UART_CLOCK=200000000
>   CONFIG_SYS_LOAD_ADDR=0x800000
>   CONFIG_DEBUG_UART=y
> @@ -70,6 +70,7 @@ CONFIG_MII=y
>   CONFIG_MVMDIO=y
>   CONFIG_PCI=y
>   CONFIG_PCI_MVEBU=y
> +CONFIG_SPL_DEBUG_UART_BASE=0xd0012000
>   CONFIG_DEBUG_UART_SHIFT=2
>   CONFIG_SYS_NS16550=y
>   CONFIG_KIRKWOOD_SPI=y
> diff --git a/configs/db-88f6820-gp_defconfig b/configs/db-88f6820-gp_defconfig
> index f56d1fbf2530..55ebb57c692e 100644
> --- a/configs/db-88f6820-gp_defconfig
> +++ b/configs/db-88f6820-gp_defconfig
> @@ -13,7 +13,7 @@ CONFIG_DEFAULT_DEVICE_TREE="armada-388-gp"
>   CONFIG_SPL_TEXT_BASE=0x40000030
>   CONFIG_SPL_SERIAL=y
>   CONFIG_SPL=y
> -CONFIG_DEBUG_UART_BASE=0xd0012000
> +CONFIG_DEBUG_UART_BASE=0xf1012000
>   CONFIG_DEBUG_UART_CLOCK=250000000
>   CONFIG_SYS_LOAD_ADDR=0x800000
>   CONFIG_DEBUG_UART=y
> @@ -66,6 +66,7 @@ CONFIG_MVMDIO=y
>   CONFIG_PCI=y
>   CONFIG_PCI_MVEBU=y
>   CONFIG_SCSI=y
> +CONFIG_SPL_DEBUG_UART_BASE=0xd0012000
>   CONFIG_DEBUG_UART_SHIFT=2
>   CONFIG_SYS_NS16550=y
>   CONFIG_KIRKWOOD_SPI=y
> diff --git a/configs/db-mv784mp-gp_defconfig b/configs/db-mv784mp-gp_defconfig
> index 5683f118365f..2c1d3b464d85 100644
> --- a/configs/db-mv784mp-gp_defconfig
> +++ b/configs/db-mv784mp-gp_defconfig
> @@ -13,7 +13,7 @@ CONFIG_DEFAULT_DEVICE_TREE="armada-xp-gp"
>   CONFIG_SPL_TEXT_BASE=0x40004030
>   CONFIG_SPL_SERIAL=y
>   CONFIG_SPL=y
> -CONFIG_DEBUG_UART_BASE=0xd0012000
> +CONFIG_DEBUG_UART_BASE=0xf1012000
>   CONFIG_DEBUG_UART_CLOCK=250000000
>   CONFIG_SYS_LOAD_ADDR=0x800000
>   CONFIG_DEBUG_UART=y
> @@ -68,6 +68,7 @@ CONFIG_MII=y
>   CONFIG_MVMDIO=y
>   CONFIG_PCI=y
>   CONFIG_PCI_MVEBU=y
> +CONFIG_SPL_DEBUG_UART_BASE=0xd0012000
>   CONFIG_DEBUG_UART_SHIFT=2
>   CONFIG_SYS_NS16550=y
>   CONFIG_KIRKWOOD_SPI=y
> diff --git a/configs/ds414_defconfig b/configs/ds414_defconfig
> index a83fe079b3a1..81a767cc0ea8 100644
> --- a/configs/ds414_defconfig
> +++ b/configs/ds414_defconfig
> @@ -19,7 +19,7 @@ CONFIG_DEFAULT_DEVICE_TREE="armada-xp-synology-ds414"
>   CONFIG_SPL_TEXT_BASE=0x40004030
>   CONFIG_SPL_SERIAL=y
>   CONFIG_SPL=y
> -CONFIG_DEBUG_UART_BASE=0xd0012000
> +CONFIG_DEBUG_UART_BASE=0xf1012000
>   CONFIG_DEBUG_UART_CLOCK=250000000
>   CONFIG_SYS_LOAD_ADDR=0x800000
>   CONFIG_DEBUG_UART=y
> @@ -68,6 +68,7 @@ CONFIG_MII=y
>   CONFIG_MVMDIO=y
>   CONFIG_PCI=y
>   CONFIG_PCI_MVEBU=y
> +CONFIG_SPL_DEBUG_UART_BASE=0xd0012000
>   CONFIG_DEBUG_UART_SHIFT=2
>   CONFIG_SYS_NS16550=y
>   CONFIG_KIRKWOOD_SPI=y
> diff --git a/configs/helios4_defconfig b/configs/helios4_defconfig
> index c2130bacb4b4..4dc9e3bc58dd 100644
> --- a/configs/helios4_defconfig
> +++ b/configs/helios4_defconfig
> @@ -13,7 +13,7 @@ CONFIG_DEFAULT_DEVICE_TREE="armada-388-helios4"
>   CONFIG_SPL_TEXT_BASE=0x40000030
>   CONFIG_SPL_SERIAL=y
>   CONFIG_SPL=y
> -CONFIG_DEBUG_UART_BASE=0xd0012000
> +CONFIG_DEBUG_UART_BASE=0xf1012000
>   CONFIG_DEBUG_UART_CLOCK=250000000
>   CONFIG_SYS_LOAD_ADDR=0x800000
>   CONFIG_DEBUG_UART=y
> @@ -67,6 +67,7 @@ CONFIG_MVMDIO=y
>   CONFIG_PCI=y
>   CONFIG_PCI_MVEBU=y
>   CONFIG_SCSI=y
> +CONFIG_SPL_DEBUG_UART_BASE=0xd0012000
>   CONFIG_DEBUG_UART_SHIFT=2
>   CONFIG_SYS_NS16550=y
>   CONFIG_KIRKWOOD_SPI=y
> diff --git a/configs/maxbcm_defconfig b/configs/maxbcm_defconfig
> index 40f79d47ea98..c325dd7cb272 100644
> --- a/configs/maxbcm_defconfig
> +++ b/configs/maxbcm_defconfig
> @@ -13,7 +13,7 @@ CONFIG_DEFAULT_DEVICE_TREE="armada-xp-maxbcm"
>   CONFIG_SPL_TEXT_BASE=0x40004030
>   CONFIG_SPL_SERIAL=y
>   CONFIG_SPL=y
> -CONFIG_DEBUG_UART_BASE=0xd0012000
> +CONFIG_DEBUG_UART_BASE=0xf1012000
>   CONFIG_DEBUG_UART_CLOCK=250000000
>   CONFIG_SYS_LOAD_ADDR=0x800000
>   CONFIG_DEBUG_UART=y
> @@ -48,6 +48,7 @@ CONFIG_PHY_GIGE=y
>   CONFIG_MVNETA=y
>   CONFIG_MII=y
>   CONFIG_MVMDIO=y
> +CONFIG_SPL_DEBUG_UART_BASE=0xd0012000
>   CONFIG_DEBUG_UART_SHIFT=2
>   CONFIG_SYS_NS16550=y
>   CONFIG_KIRKWOOD_SPI=y
> diff --git a/configs/theadorable_debug_defconfig b/configs/theadorable_debug_defconfig
> index 111392d9a941..9e8523d94145 100644
> --- a/configs/theadorable_debug_defconfig
> +++ b/configs/theadorable_debug_defconfig
> @@ -14,7 +14,7 @@ CONFIG_DEFAULT_DEVICE_TREE="armada-xp-theadorable"
>   CONFIG_SPL_TEXT_BASE=0x40004030
>   CONFIG_SPL_SERIAL=y
>   CONFIG_SPL=y
> -CONFIG_DEBUG_UART_BASE=0xd0012000
> +CONFIG_DEBUG_UART_BASE=0xf1012000
>   CONFIG_DEBUG_UART_CLOCK=250000000
>   CONFIG_SYS_MEM_TOP_HIDE=0x80000
>   CONFIG_SYS_LOAD_ADDR=0x800000
> @@ -74,6 +74,7 @@ CONFIG_MVMDIO=y
>   CONFIG_PCI=y
>   CONFIG_DM_PCI_COMPAT=y
>   CONFIG_PCI_MVEBU=y
> +CONFIG_SPL_DEBUG_UART_BASE=0xd0012000
>   CONFIG_DEBUG_UART_SHIFT=2
>   CONFIG_SYS_NS16550=y
>   CONFIG_KIRKWOOD_SPI=y
> diff --git a/configs/x530_defconfig b/configs/x530_defconfig
> index 77d224942764..860cb2207139 100644
> --- a/configs/x530_defconfig
> +++ b/configs/x530_defconfig
> @@ -14,7 +14,7 @@ CONFIG_DEFAULT_DEVICE_TREE="armada-385-atl-x530"
>   CONFIG_SPL_TEXT_BASE=0x40000030
>   CONFIG_SPL_SERIAL=y
>   CONFIG_SPL=y
> -CONFIG_DEBUG_UART_BASE=0xd0012000
> +CONFIG_DEBUG_UART_BASE=0xf1012000
>   CONFIG_DEBUG_UART_CLOCK=250000000
>   CONFIG_SYS_LOAD_ADDR=0x1000000
>   CONFIG_ENV_ADDR=0x100000
> @@ -72,6 +72,7 @@ CONFIG_PCI_MVEBU=y
>   CONFIG_DM_RTC=y
>   CONFIG_RTC_DS1307=y
>   CONFIG_SPECIFY_CONSOLE_INDEX=y
> +CONFIG_SPL_DEBUG_UART_BASE=0xd0012000
>   CONFIG_DEBUG_UART_SHIFT=2
>   CONFIG_SYS_NS16550=y
>   CONFIG_KIRKWOOD_SPI=y

Viele Grüße,
Stefan Roese

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr@denx.de

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 0/5] arm: mvebu: turris_omnia: Fix hangup in debug UART
  2022-05-06  9:05 [PATCH 0/5] arm: mvebu: turris_omnia: Fix hangup in debug UART Pali Rohár
                   ` (5 preceding siblings ...)
  2022-05-09 18:17 ` [PATCH 6/5] arm: mvebu: Fix DEBUG_UART_BASE for all 32-bit boards Pali Rohár
@ 2022-05-16 14:52 ` Stefan Roese
  2022-05-16 16:50   ` Pali Rohár
  2022-05-16 16:49 ` [PATCH 7/5] serial: Add new config option TPL_DEBUG_UART_BASE Pali Rohár
  2022-05-17  7:53 ` [PATCH 0/5] arm: mvebu: turris_omnia: Fix hangup in debug UART Stefan Roese
  8 siblings, 1 reply; 32+ messages in thread
From: Stefan Roese @ 2022-05-16 14:52 UTC (permalink / raw)
  To: Pali Rohár, Marek Behun; +Cc: u-boot

Hi Pali,

On 06.05.22 11:05, Pali Rohár wrote:
> If proper U-Boot on Turris Omnia tries to print something on debug UART
> then CPU hangs. Reason is that debug UART in proper U-Boot for Turris
> Omnia has incorrect configuration of base register. Base register is
> different in SPL and also in different stages of proper U-Boot.
> 
> Fix all these issues. Introduce new arch_very_early_init() ARM function
> which will be called at very early stage prior initialization of debug
> UART. Move code which moves internal mvebu registers to this function.
> And split configuration of DEBUG_UART_BASE into SPL and proper U-Boot
> to allow setting different values which matches current HW status.
> 
> With all these changes, it is possible call "printf" function in
> Turris Omnia's board_early_init_f() function in both SPL and proper
> U-Boot.
> 
> Pali Rohár (5):
>    arm: Add new config option ARCH_VERY_EARLY_INIT
>    arm: mvebu: Move internal registers in arch_very_early_init() function
>    serial: Add new config option SPL_DEBUG_UART_BASE
>    serial: ns16550: Add support for SPL_DEBUG_UART_BASE
>    arm: mvebu: turris_omnia: Fix DEBUG_UART_BASE
> 
>   arch/arm/Kconfig               |  6 ++++++
>   arch/arm/lib/crt0.S            |  5 +++++
>   arch/arm/mach-mvebu/Kconfig    |  1 +
>   arch/arm/mach-mvebu/Makefile   |  1 +
>   arch/arm/mach-mvebu/cpu.c      | 31 -------------------------------
>   arch/arm/mach-mvebu/lowlevel.S | 27 +++++++++++++++++++++++++++
>   configs/turris_omnia_defconfig |  3 ++-
>   drivers/serial/Kconfig         |  7 +++++++
>   drivers/serial/ns16550.c       |  4 ++--
>   9 files changed, 51 insertions(+), 34 deletions(-)
>   create mode 100644 arch/arm/mach-mvebu/lowlevel.S

This patchset seems to introduce problems on some non-MVEBU platforms
like x86, chromebook_coral_defconfig:

drivers/serial/ns16550.c: In function ‘_debug_uart_init’:
././include/linux/kconfig.h:51:32: error: ‘CONFIG_TPL_DEBUG_UART_BASE’ 
undeclared (first use in this function); did you mean 
‘CONFIG_SPL_DEBUG_UART_BASE’?
    51 | #define __config_val(pfx, cfg) CONFIG_ ## pfx ## cfg
       |                                ^~~~~~~
././include/linux/kconfig.h:50:32: note: in expansion of macro 
‘__config_val’
    50 | #define  _config_val(pfx, cfg) __config_val(pfx, cfg)
       |                                ^~~~~~~~~~~~
././include/linux/kconfig.h:49:33: note: in expansion of macro ‘_config_val’
    49 | #define   config_val(cfg)       _config_val(_CONFIG_PREFIX, cfg)
       |                                 ^~~~~~~~~~~
././include/linux/kconfig.h:61:29: note: in expansion of macro ‘config_val’
    61 | #define CONFIG_VAL(option)  config_val(option)
       |                             ^~~~~~~~~~
drivers/serial/ns16550.c:328:54: note: in expansion of macro ‘CONFIG_VAL’
   328 |         struct ns16550 *com_port = (struct ns16550 
*)CONFIG_VAL(DEBUG_UART_BASE);
       |                                                      ^~~~~~~~~~
././include/linux/kconfig.h:51:32: note: each undeclared identifier is 
reported only once for each function it appears in
    51 | #define __config_val(pfx, cfg) CONFIG_ ## pfx ## cfg
       |                                ^~~~~~~
././include/linux/kconfig.h:50:32: note: in expansion of macro 
‘__config_val’
    50 | #define  _config_val(pfx, cfg) __config_val(pfx, cfg)
       |                                ^~~~~~~~~~~~
././include/linux/kconfig.h:49:33: note: in expansion of macro ‘_config_val’
    49 | #define   config_val(cfg)       _config_val(_CONFIG_PREFIX, cfg)
       |                                 ^~~~~~~~~~~
././include/linux/kconfig.h:61:29: note: in expansion of macro ‘config_val’
    61 | #define CONFIG_VAL(option)  config_val(option)
       |                             ^~~~~~~~~~
drivers/serial/ns16550.c:328:54: note: in expansion of macro ‘CONFIG_VAL’
   328 |         struct ns16550 *com_port = (struct ns16550 
*)CONFIG_VAL(DEBUG_UART_BASE);
       |                                                      ^~~~~~~~~~
drivers/serial/ns16550.c: In function ‘_debug_uart_putc’:
././include/linux/kconfig.h:51:32: error: ‘CONFIG_TPL_DEBUG_UART_BASE’ 
undeclared (first use in this function); did you mean 
‘CONFIG_SPL_DEBUG_UART_BASE’?
    51 | #define __config_val(pfx, cfg) CONFIG_ ## pfx ## cfg
       |                                ^~~~~~~
././include/linux/kconfig.h:50:32: note: in expansion of macro 
‘__config_val’
    50 | #define  _config_val(pfx, cfg) __config_val(pfx, cfg)
       |                                ^~~~~~~~~~~~
././include/linux/kconfig.h:49:33: note: in expansion of macro ‘_config_val’
    49 | #define   config_val(cfg)       _config_val(_CONFIG_PREFIX, cfg)
       |                                 ^~~~~~~~~~~
././include/linux/kconfig.h:61:29: note: in expansion of macro ‘config_val’
    61 | #define CONFIG_VAL(option)  config_val(option)
       |                             ^~~~~~~~~~
drivers/serial/ns16550.c:363:54: note: in expansion of macro ‘CONFIG_VAL’
   363 |         struct ns16550 *com_port = (struct ns16550 
*)CONFIG_VAL(DEBUG_UART_BASE);
       |                                                      ^~~~~~~~~~
make[3]: *** [scripts/Makefile.build:258: tpl/drivers/serial/ns16550.o] 
Error 1

Could you please take a look?

Thanks,
Stefan

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [PATCH 7/5] serial: Add new config option TPL_DEBUG_UART_BASE
  2022-05-06  9:05 [PATCH 0/5] arm: mvebu: turris_omnia: Fix hangup in debug UART Pali Rohár
                   ` (6 preceding siblings ...)
  2022-05-16 14:52 ` [PATCH 0/5] arm: mvebu: turris_omnia: Fix hangup in debug UART Stefan Roese
@ 2022-05-16 16:49 ` Pali Rohár
  2022-05-17  7:51   ` Stefan Roese
  2022-05-17  7:53 ` [PATCH 0/5] arm: mvebu: turris_omnia: Fix hangup in debug UART Stefan Roese
  8 siblings, 1 reply; 32+ messages in thread
From: Pali Rohár @ 2022-05-16 16:49 UTC (permalink / raw)
  To: Stefan Roese, Marek Behun; +Cc: u-boot

TPL_DEBUG_UART_BASE is same as DEBUG_UART_BASE, but applies only for TPL.

Signed-off-by: Pali Rohár <pali@kernel.org>
---
 drivers/serial/Kconfig | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/serial/Kconfig b/drivers/serial/Kconfig
index 46726d76b162..f6425a530e57 100644
--- a/drivers/serial/Kconfig
+++ b/drivers/serial/Kconfig
@@ -515,6 +515,13 @@ config SPL_DEBUG_UART_BASE
 	help
 	  This is the base address of your UART for memory-mapped UARTs for SPL.
 
+config TPL_DEBUG_UART_BASE
+	hex "Base address of UART for TPL"
+	depends on TPL && DEBUG_UART
+	default DEBUG_UART_BASE
+	help
+	  This is the base address of your UART for memory-mapped UARTs for TPL.
+
 config DEBUG_UART_CLOCK
 	int "UART input clock"
 	depends on DEBUG_UART
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* Re: [PATCH 0/5] arm: mvebu: turris_omnia: Fix hangup in debug UART
  2022-05-16 14:52 ` [PATCH 0/5] arm: mvebu: turris_omnia: Fix hangup in debug UART Stefan Roese
@ 2022-05-16 16:50   ` Pali Rohár
  0 siblings, 0 replies; 32+ messages in thread
From: Pali Rohár @ 2022-05-16 16:50 UTC (permalink / raw)
  To: Stefan Roese; +Cc: Marek Behun, u-boot

On Monday 16 May 2022 16:52:06 Stefan Roese wrote:
> Hi Pali,
> 
> On 06.05.22 11:05, Pali Rohár wrote:
> > If proper U-Boot on Turris Omnia tries to print something on debug UART
> > then CPU hangs. Reason is that debug UART in proper U-Boot for Turris
> > Omnia has incorrect configuration of base register. Base register is
> > different in SPL and also in different stages of proper U-Boot.
> > 
> > Fix all these issues. Introduce new arch_very_early_init() ARM function
> > which will be called at very early stage prior initialization of debug
> > UART. Move code which moves internal mvebu registers to this function.
> > And split configuration of DEBUG_UART_BASE into SPL and proper U-Boot
> > to allow setting different values which matches current HW status.
> > 
> > With all these changes, it is possible call "printf" function in
> > Turris Omnia's board_early_init_f() function in both SPL and proper
> > U-Boot.
> > 
> > Pali Rohár (5):
> >    arm: Add new config option ARCH_VERY_EARLY_INIT
> >    arm: mvebu: Move internal registers in arch_very_early_init() function
> >    serial: Add new config option SPL_DEBUG_UART_BASE
> >    serial: ns16550: Add support for SPL_DEBUG_UART_BASE
> >    arm: mvebu: turris_omnia: Fix DEBUG_UART_BASE
> > 
> >   arch/arm/Kconfig               |  6 ++++++
> >   arch/arm/lib/crt0.S            |  5 +++++
> >   arch/arm/mach-mvebu/Kconfig    |  1 +
> >   arch/arm/mach-mvebu/Makefile   |  1 +
> >   arch/arm/mach-mvebu/cpu.c      | 31 -------------------------------
> >   arch/arm/mach-mvebu/lowlevel.S | 27 +++++++++++++++++++++++++++
> >   configs/turris_omnia_defconfig |  3 ++-
> >   drivers/serial/Kconfig         |  7 +++++++
> >   drivers/serial/ns16550.c       |  4 ++--
> >   9 files changed, 51 insertions(+), 34 deletions(-)
> >   create mode 100644 arch/arm/mach-mvebu/lowlevel.S
> 
> This patchset seems to introduce problems on some non-MVEBU platforms
> like x86, chromebook_coral_defconfig:
> 
> drivers/serial/ns16550.c: In function ‘_debug_uart_init’:
> ././include/linux/kconfig.h:51:32: error: ‘CONFIG_TPL_DEBUG_UART_BASE’
> undeclared (first use in this function); did you mean
> ‘CONFIG_SPL_DEBUG_UART_BASE’?
>    51 | #define __config_val(pfx, cfg) CONFIG_ ## pfx ## cfg
>       |                                ^~~~~~~
> ././include/linux/kconfig.h:50:32: note: in expansion of macro
> ‘__config_val’
>    50 | #define  _config_val(pfx, cfg) __config_val(pfx, cfg)
>       |                                ^~~~~~~~~~~~
> ././include/linux/kconfig.h:49:33: note: in expansion of macro ‘_config_val’
>    49 | #define   config_val(cfg)       _config_val(_CONFIG_PREFIX, cfg)
>       |                                 ^~~~~~~~~~~
> ././include/linux/kconfig.h:61:29: note: in expansion of macro ‘config_val’
>    61 | #define CONFIG_VAL(option)  config_val(option)
>       |                             ^~~~~~~~~~
> drivers/serial/ns16550.c:328:54: note: in expansion of macro ‘CONFIG_VAL’
>   328 |         struct ns16550 *com_port = (struct ns16550
> *)CONFIG_VAL(DEBUG_UART_BASE);
>       |                                                      ^~~~~~~~~~
> ././include/linux/kconfig.h:51:32: note: each undeclared identifier is
> reported only once for each function it appears in
>    51 | #define __config_val(pfx, cfg) CONFIG_ ## pfx ## cfg
>       |                                ^~~~~~~
> ././include/linux/kconfig.h:50:32: note: in expansion of macro
> ‘__config_val’
>    50 | #define  _config_val(pfx, cfg) __config_val(pfx, cfg)
>       |                                ^~~~~~~~~~~~
> ././include/linux/kconfig.h:49:33: note: in expansion of macro ‘_config_val’
>    49 | #define   config_val(cfg)       _config_val(_CONFIG_PREFIX, cfg)
>       |                                 ^~~~~~~~~~~
> ././include/linux/kconfig.h:61:29: note: in expansion of macro ‘config_val’
>    61 | #define CONFIG_VAL(option)  config_val(option)
>       |                             ^~~~~~~~~~
> drivers/serial/ns16550.c:328:54: note: in expansion of macro ‘CONFIG_VAL’
>   328 |         struct ns16550 *com_port = (struct ns16550
> *)CONFIG_VAL(DEBUG_UART_BASE);
>       |                                                      ^~~~~~~~~~
> drivers/serial/ns16550.c: In function ‘_debug_uart_putc’:
> ././include/linux/kconfig.h:51:32: error: ‘CONFIG_TPL_DEBUG_UART_BASE’
> undeclared (first use in this function); did you mean
> ‘CONFIG_SPL_DEBUG_UART_BASE’?
>    51 | #define __config_val(pfx, cfg) CONFIG_ ## pfx ## cfg
>       |                                ^~~~~~~
> ././include/linux/kconfig.h:50:32: note: in expansion of macro
> ‘__config_val’
>    50 | #define  _config_val(pfx, cfg) __config_val(pfx, cfg)
>       |                                ^~~~~~~~~~~~
> ././include/linux/kconfig.h:49:33: note: in expansion of macro ‘_config_val’
>    49 | #define   config_val(cfg)       _config_val(_CONFIG_PREFIX, cfg)
>       |                                 ^~~~~~~~~~~
> ././include/linux/kconfig.h:61:29: note: in expansion of macro ‘config_val’
>    61 | #define CONFIG_VAL(option)  config_val(option)
>       |                             ^~~~~~~~~~
> drivers/serial/ns16550.c:363:54: note: in expansion of macro ‘CONFIG_VAL’
>   363 |         struct ns16550 *com_port = (struct ns16550
> *)CONFIG_VAL(DEBUG_UART_BASE);
>       |                                                      ^~~~~~~~~~
> make[3]: *** [scripts/Makefile.build:258: tpl/drivers/serial/ns16550.o]
> Error 1
> 
> Could you please take a look?
> 
> Thanks,
> Stefan

I totally forgot for TPL. Fixed in additional patch "serial: Add new
config option TPL_DEBUG_UART_BASE" which I sent recently.

With this patch chromebook_coral_defconfig with x86 compiler builds
fine.

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 7/5] serial: Add new config option TPL_DEBUG_UART_BASE
  2022-05-16 16:49 ` [PATCH 7/5] serial: Add new config option TPL_DEBUG_UART_BASE Pali Rohár
@ 2022-05-17  7:51   ` Stefan Roese
  0 siblings, 0 replies; 32+ messages in thread
From: Stefan Roese @ 2022-05-17  7:51 UTC (permalink / raw)
  To: Pali Rohár, Marek Behun; +Cc: u-boot

On 16.05.22 18:49, Pali Rohár wrote:
> TPL_DEBUG_UART_BASE is same as DEBUG_UART_BASE, but applies only for TPL.
> 
> Signed-off-by: Pali Rohár <pali@kernel.org>

Reviewed-by: Stefan Roese <sr@denx.de>

Thanks,
Stefan

> ---
>   drivers/serial/Kconfig | 7 +++++++
>   1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/serial/Kconfig b/drivers/serial/Kconfig
> index 46726d76b162..f6425a530e57 100644
> --- a/drivers/serial/Kconfig
> +++ b/drivers/serial/Kconfig
> @@ -515,6 +515,13 @@ config SPL_DEBUG_UART_BASE
>   	help
>   	  This is the base address of your UART for memory-mapped UARTs for SPL.
>   
> +config TPL_DEBUG_UART_BASE
> +	hex "Base address of UART for TPL"
> +	depends on TPL && DEBUG_UART
> +	default DEBUG_UART_BASE
> +	help
> +	  This is the base address of your UART for memory-mapped UARTs for TPL.
> +
>   config DEBUG_UART_CLOCK
>   	int "UART input clock"
>   	depends on DEBUG_UART

Viele Grüße,
Stefan Roese

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr@denx.de

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 0/5] arm: mvebu: turris_omnia: Fix hangup in debug UART
  2022-05-06  9:05 [PATCH 0/5] arm: mvebu: turris_omnia: Fix hangup in debug UART Pali Rohár
                   ` (7 preceding siblings ...)
  2022-05-16 16:49 ` [PATCH 7/5] serial: Add new config option TPL_DEBUG_UART_BASE Pali Rohár
@ 2022-05-17  7:53 ` Stefan Roese
  8 siblings, 0 replies; 32+ messages in thread
From: Stefan Roese @ 2022-05-17  7:53 UTC (permalink / raw)
  To: Pali Rohár, Marek Behun; +Cc: u-boot

On 06.05.22 11:05, Pali Rohár wrote:
> If proper U-Boot on Turris Omnia tries to print something on debug UART
> then CPU hangs. Reason is that debug UART in proper U-Boot for Turris
> Omnia has incorrect configuration of base register. Base register is
> different in SPL and also in different stages of proper U-Boot.
> 
> Fix all these issues. Introduce new arch_very_early_init() ARM function
> which will be called at very early stage prior initialization of debug
> UART. Move code which moves internal mvebu registers to this function.
> And split configuration of DEBUG_UART_BASE into SPL and proper U-Boot
> to allow setting different values which matches current HW status.
> 
> With all these changes, it is possible call "printf" function in
> Turris Omnia's board_early_init_f() function in both SPL and proper
> U-Boot.
> 
> Pali Rohár (5):
>    arm: Add new config option ARCH_VERY_EARLY_INIT
>    arm: mvebu: Move internal registers in arch_very_early_init() function
>    serial: Add new config option SPL_DEBUG_UART_BASE
>    serial: ns16550: Add support for SPL_DEBUG_UART_BASE
>    arm: mvebu: turris_omnia: Fix DEBUG_UART_BASE
> 
>   arch/arm/Kconfig               |  6 ++++++
>   arch/arm/lib/crt0.S            |  5 +++++
>   arch/arm/mach-mvebu/Kconfig    |  1 +
>   arch/arm/mach-mvebu/Makefile   |  1 +
>   arch/arm/mach-mvebu/cpu.c      | 31 -------------------------------
>   arch/arm/mach-mvebu/lowlevel.S | 27 +++++++++++++++++++++++++++
>   configs/turris_omnia_defconfig |  3 ++-
>   drivers/serial/Kconfig         |  7 +++++++
>   drivers/serial/ns16550.c       |  4 ++--
>   9 files changed, 51 insertions(+), 34 deletions(-)
>   create mode 100644 arch/arm/mach-mvebu/lowlevel.S
> 

Applied with the corresponding additional patches to
u-boot-marvell/master

Thanks,
Stefan

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 2/5] arm: mvebu: Move internal registers in arch_very_early_init() function
  2022-05-06 12:44           ` Pali Rohár
@ 2022-06-01 10:27             ` Pali Rohár
  2022-06-01 10:44               ` Stefan Roese
  0 siblings, 1 reply; 32+ messages in thread
From: Pali Rohár @ 2022-06-01 10:27 UTC (permalink / raw)
  To: Stefan Roese; +Cc: Marek Behun, u-boot

On Friday 06 May 2022 14:44:48 Pali Rohár wrote:
> On Friday 06 May 2022 14:35:55 Stefan Roese wrote:
> > While doing this I noticed though, that kwboot UART booting only worked
> > in roughly 1 out of 2 cases. With no progress after this line:
> > 
> > Sending boot message. Please reboot the target...\
> > 
> > IIRC, this worked more reliable a few weeks ago. Any idea, what might
> > have caused this regression - if I am correct here?
> 
> There were changes in handling of bootrom boot message. There is option
> -s which can change default timeout option. And plus there is option -a
> which sets this timeout option to AXP value.
> 
> Default value is 50 and for AXP default value is 1000.
> 
> Maybe it is needed to tune AXP value... Try to call with -s option
> between 50 and 1000...

Hello Stefan! Have you tried to tune kwboot's -s or -a option for AXP board?

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 2/5] arm: mvebu: Move internal registers in arch_very_early_init() function
  2022-06-01 10:27             ` Pali Rohár
@ 2022-06-01 10:44               ` Stefan Roese
  2022-06-01 10:54                 ` Pali Rohár
  0 siblings, 1 reply; 32+ messages in thread
From: Stefan Roese @ 2022-06-01 10:44 UTC (permalink / raw)
  To: Pali Rohár; +Cc: Marek Behun, u-boot

Hi Pali,

On 01.06.22 12:27, Pali Rohár wrote:
> On Friday 06 May 2022 14:44:48 Pali Rohár wrote:
>> On Friday 06 May 2022 14:35:55 Stefan Roese wrote:
>>> While doing this I noticed though, that kwboot UART booting only worked
>>> in roughly 1 out of 2 cases. With no progress after this line:
>>>
>>> Sending boot message. Please reboot the target...\
>>>
>>> IIRC, this worked more reliable a few weeks ago. Any idea, what might
>>> have caused this regression - if I am correct here?
>>
>> There were changes in handling of bootrom boot message. There is option
>> -s which can change default timeout option. And plus there is option -a
>> which sets this timeout option to AXP value.
>>
>> Default value is 50 and for AXP default value is 1000.
>>
>> Maybe it is needed to tune AXP value... Try to call with -s option
>> between 50 and 1000...
> 
> Hello Stefan! Have you tried to tune kwboot's -s or -a option for AXP board?

Thanks for getting back on this. I totally forgot about it.

'-a' makes it actually worse. Zero success with 5 tries. '-s 10' worked
5 out of 5 times. So I'll stick with this version I think.

Thanks,
Stefan

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 2/5] arm: mvebu: Move internal registers in arch_very_early_init() function
  2022-06-01 10:44               ` Stefan Roese
@ 2022-06-01 10:54                 ` Pali Rohár
  2022-08-16 18:41                   ` Fix KWBOOT_MSG_RSP_TIMEO_AXP in kwboot for Armada XP Pali Rohár
  0 siblings, 1 reply; 32+ messages in thread
From: Pali Rohár @ 2022-06-01 10:54 UTC (permalink / raw)
  To: Stefan Roese; +Cc: Marek Behun, u-boot

On Wednesday 01 June 2022 12:44:01 Stefan Roese wrote:
> Hi Pali,
> 
> On 01.06.22 12:27, Pali Rohár wrote:
> > On Friday 06 May 2022 14:44:48 Pali Rohár wrote:
> > > On Friday 06 May 2022 14:35:55 Stefan Roese wrote:
> > > > While doing this I noticed though, that kwboot UART booting only worked
> > > > in roughly 1 out of 2 cases. With no progress after this line:
> > > > 
> > > > Sending boot message. Please reboot the target...\
> > > > 
> > > > IIRC, this worked more reliable a few weeks ago. Any idea, what might
> > > > have caused this regression - if I am correct here?
> > > 
> > > There were changes in handling of bootrom boot message. There is option
> > > -s which can change default timeout option. And plus there is option -a
> > > which sets this timeout option to AXP value.
> > > 
> > > Default value is 50 and for AXP default value is 1000.
> > > 
> > > Maybe it is needed to tune AXP value... Try to call with -s option
> > > between 50 and 1000...
> > 
> > Hello Stefan! Have you tried to tune kwboot's -s or -a option for AXP board?
> 
> Thanks for getting back on this. I totally forgot about it.
> 
> '-a' makes it actually worse. Zero success with 5 tries. '-s 10' worked
> 5 out of 5 times. So I'll stick with this version I think.
> 
> Thanks,
> Stefan

Ok! So I think that you could change KWBOOT_MSG_RSP_TIMEO_AXP value in
tools/kwboot.c to 10 if it works fine for your AXP board. So '-a' option
would work better.

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Fix KWBOOT_MSG_RSP_TIMEO_AXP in kwboot for Armada XP
  2022-06-01 10:54                 ` Pali Rohár
@ 2022-08-16 18:41                   ` Pali Rohár
  2022-08-19  7:30                     ` Stefan Roese
  0 siblings, 1 reply; 32+ messages in thread
From: Pali Rohár @ 2022-08-16 18:41 UTC (permalink / raw)
  To: Stefan Roese; +Cc: u-boot

On Wednesday 01 June 2022 12:54:28 Pali Rohár wrote:
> On Wednesday 01 June 2022 12:44:01 Stefan Roese wrote:
> > Hi Pali,
> > 
> > On 01.06.22 12:27, Pali Rohár wrote:
> > > On Friday 06 May 2022 14:44:48 Pali Rohár wrote:
> > > > On Friday 06 May 2022 14:35:55 Stefan Roese wrote:
> > > > > While doing this I noticed though, that kwboot UART booting only worked
> > > > > in roughly 1 out of 2 cases. With no progress after this line:
> > > > > 
> > > > > Sending boot message. Please reboot the target...\
> > > > > 
> > > > > IIRC, this worked more reliable a few weeks ago. Any idea, what might
> > > > > have caused this regression - if I am correct here?
> > > > 
> > > > There were changes in handling of bootrom boot message. There is option
> > > > -s which can change default timeout option. And plus there is option -a
> > > > which sets this timeout option to AXP value.
> > > > 
> > > > Default value is 50 and for AXP default value is 1000.
> > > > 
> > > > Maybe it is needed to tune AXP value... Try to call with -s option
> > > > between 50 and 1000...
> > > 
> > > Hello Stefan! Have you tried to tune kwboot's -s or -a option for AXP board?
> > 
> > Thanks for getting back on this. I totally forgot about it.
> > 
> > '-a' makes it actually worse. Zero success with 5 tries. '-s 10' worked
> > 5 out of 5 times. So I'll stick with this version I think.
> > 
> > Thanks,
> > Stefan
> 
> Ok! So I think that you could change KWBOOT_MSG_RSP_TIMEO_AXP value in
> tools/kwboot.c to 10 if it works fine for your AXP board. So '-a' option
> would work better.

Hello! I would like to remind this issue. Could you look at it, so we do
not have broken '-a' option in kwboot?

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: Fix KWBOOT_MSG_RSP_TIMEO_AXP in kwboot for Armada XP
  2022-08-16 18:41                   ` Fix KWBOOT_MSG_RSP_TIMEO_AXP in kwboot for Armada XP Pali Rohár
@ 2022-08-19  7:30                     ` Stefan Roese
  2022-08-19  7:34                       ` Pali Rohár
  0 siblings, 1 reply; 32+ messages in thread
From: Stefan Roese @ 2022-08-19  7:30 UTC (permalink / raw)
  To: Pali Rohár; +Cc: u-boot

Hi Pali,

On 16.08.22 20:41, Pali Rohár wrote:
> On Wednesday 01 June 2022 12:54:28 Pali Rohár wrote:
>> On Wednesday 01 June 2022 12:44:01 Stefan Roese wrote:
>>> Hi Pali,
>>>
>>> On 01.06.22 12:27, Pali Rohár wrote:
>>>> On Friday 06 May 2022 14:44:48 Pali Rohár wrote:
>>>>> On Friday 06 May 2022 14:35:55 Stefan Roese wrote:
>>>>>> While doing this I noticed though, that kwboot UART booting only worked
>>>>>> in roughly 1 out of 2 cases. With no progress after this line:
>>>>>>
>>>>>> Sending boot message. Please reboot the target...\
>>>>>>
>>>>>> IIRC, this worked more reliable a few weeks ago. Any idea, what might
>>>>>> have caused this regression - if I am correct here?
>>>>>
>>>>> There were changes in handling of bootrom boot message. There is option
>>>>> -s which can change default timeout option. And plus there is option -a
>>>>> which sets this timeout option to AXP value.
>>>>>
>>>>> Default value is 50 and for AXP default value is 1000.
>>>>>
>>>>> Maybe it is needed to tune AXP value... Try to call with -s option
>>>>> between 50 and 1000...
>>>>
>>>> Hello Stefan! Have you tried to tune kwboot's -s or -a option for AXP board?
>>>
>>> Thanks for getting back on this. I totally forgot about it.
>>>
>>> '-a' makes it actually worse. Zero success with 5 tries. '-s 10' worked
>>> 5 out of 5 times. So I'll stick with this version I think.
>>>
>>> Thanks,
>>> Stefan
>>
>> Ok! So I think that you could change KWBOOT_MSG_RSP_TIMEO_AXP value in
>> tools/kwboot.c to 10 if it works fine for your AXP board. So '-a' option
>> would work better.
> 
> Hello! I would like to remind this issue. Could you look at it, so we do
> not have broken '-a' option in kwboot?

Thanks for the reminder. I've checked again and "-a" is not working
reliable on my AXP platform (any more?). Using "-s 10" seem to be much
better.

So we should either remove this -a option completely or change the
define to 10 instead of 1000 IMHO.

Thanks,
Stefan

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: Fix KWBOOT_MSG_RSP_TIMEO_AXP in kwboot for Armada XP
  2022-08-19  7:30                     ` Stefan Roese
@ 2022-08-19  7:34                       ` Pali Rohár
  0 siblings, 0 replies; 32+ messages in thread
From: Pali Rohár @ 2022-08-19  7:34 UTC (permalink / raw)
  To: Stefan Roese; +Cc: u-boot

On Friday 19 August 2022 09:30:55 Stefan Roese wrote:
> Hi Pali,
> 
> On 16.08.22 20:41, Pali Rohár wrote:
> > On Wednesday 01 June 2022 12:54:28 Pali Rohár wrote:
> > > On Wednesday 01 June 2022 12:44:01 Stefan Roese wrote:
> > > > Hi Pali,
> > > > 
> > > > On 01.06.22 12:27, Pali Rohár wrote:
> > > > > On Friday 06 May 2022 14:44:48 Pali Rohár wrote:
> > > > > > On Friday 06 May 2022 14:35:55 Stefan Roese wrote:
> > > > > > > While doing this I noticed though, that kwboot UART booting only worked
> > > > > > > in roughly 1 out of 2 cases. With no progress after this line:
> > > > > > > 
> > > > > > > Sending boot message. Please reboot the target...\
> > > > > > > 
> > > > > > > IIRC, this worked more reliable a few weeks ago. Any idea, what might
> > > > > > > have caused this regression - if I am correct here?
> > > > > > 
> > > > > > There were changes in handling of bootrom boot message. There is option
> > > > > > -s which can change default timeout option. And plus there is option -a
> > > > > > which sets this timeout option to AXP value.
> > > > > > 
> > > > > > Default value is 50 and for AXP default value is 1000.
> > > > > > 
> > > > > > Maybe it is needed to tune AXP value... Try to call with -s option
> > > > > > between 50 and 1000...
> > > > > 
> > > > > Hello Stefan! Have you tried to tune kwboot's -s or -a option for AXP board?
> > > > 
> > > > Thanks for getting back on this. I totally forgot about it.
> > > > 
> > > > '-a' makes it actually worse. Zero success with 5 tries. '-s 10' worked
> > > > 5 out of 5 times. So I'll stick with this version I think.
> > > > 
> > > > Thanks,
> > > > Stefan
> > > 
> > > Ok! So I think that you could change KWBOOT_MSG_RSP_TIMEO_AXP value in
> > > tools/kwboot.c to 10 if it works fine for your AXP board. So '-a' option
> > > would work better.
> > 
> > Hello! I would like to remind this issue. Could you look at it, so we do
> > not have broken '-a' option in kwboot?
> 
> Thanks for the reminder. I've checked again and "-a" is not working
> reliable on my AXP platform (any more?). Using "-s 10" seem to be much
> better.
> 
> So we should either remove this -a option completely or change the
> define to 10 instead of 1000 IMHO.
> 
> Thanks,
> Stefan

Yes, please check also if kwboot is reliable without any -a or -s option
on AXP platform. If it works, then remove/deprecate -a option. If it
does not work and requires specific -s 10 option, then change
KWBOOT_MSG_RSP_TIMEO_AXP macro to 10.

^ permalink raw reply	[flat|nested] 32+ messages in thread

end of thread, other threads:[~2022-08-19  7:35 UTC | newest]

Thread overview: 32+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-06  9:05 [PATCH 0/5] arm: mvebu: turris_omnia: Fix hangup in debug UART Pali Rohár
2022-05-06  9:05 ` [PATCH 1/5] arm: Add new config option ARCH_VERY_EARLY_INIT Pali Rohár
2022-05-16  6:37   ` Stefan Roese
2022-05-06  9:05 ` [PATCH 2/5] arm: mvebu: Move internal registers in arch_very_early_init() function Pali Rohár
2022-05-06 12:04   ` Stefan Roese
2022-05-06 12:09     ` Pali Rohár
2022-05-06 12:16       ` Stefan Roese
2022-05-06 12:35         ` Stefan Roese
2022-05-06 12:44           ` Pali Rohár
2022-06-01 10:27             ` Pali Rohár
2022-06-01 10:44               ` Stefan Roese
2022-06-01 10:54                 ` Pali Rohár
2022-08-16 18:41                   ` Fix KWBOOT_MSG_RSP_TIMEO_AXP in kwboot for Armada XP Pali Rohár
2022-08-19  7:30                     ` Stefan Roese
2022-08-19  7:34                       ` Pali Rohár
2022-05-16  6:37   ` [PATCH 2/5] arm: mvebu: Move internal registers in arch_very_early_init() function Stefan Roese
2022-05-06  9:05 ` [PATCH 3/5] serial: Add new config option SPL_DEBUG_UART_BASE Pali Rohár
2022-05-16  6:37   ` Stefan Roese
2022-05-06  9:05 ` [PATCH 4/5] serial: ns16550: Add support for SPL_DEBUG_UART_BASE Pali Rohár
2022-05-16  6:38   ` Stefan Roese
2022-05-06  9:05 ` [PATCH 5/5] arm: mvebu: turris_omnia: Fix DEBUG_UART_BASE Pali Rohár
2022-05-06 12:21   ` Stefan Roese
2022-05-06 12:30     ` Pali Rohár
2022-05-09 18:18       ` Pali Rohár
2022-05-16  6:38   ` Stefan Roese
2022-05-09 18:17 ` [PATCH 6/5] arm: mvebu: Fix DEBUG_UART_BASE for all 32-bit boards Pali Rohár
2022-05-16  6:39   ` Stefan Roese
2022-05-16 14:52 ` [PATCH 0/5] arm: mvebu: turris_omnia: Fix hangup in debug UART Stefan Roese
2022-05-16 16:50   ` Pali Rohár
2022-05-16 16:49 ` [PATCH 7/5] serial: Add new config option TPL_DEBUG_UART_BASE Pali Rohár
2022-05-17  7:51   ` Stefan Roese
2022-05-17  7:53 ` [PATCH 0/5] arm: mvebu: turris_omnia: Fix hangup in debug UART Stefan Roese

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.