All of lore.kernel.org
 help / color / mirror / Atom feed
* [U-Boot] [PATCH] rockchip: add TPL_TINY_FRAMEWORK support
@ 2018-08-28  9:23 Kever Yang
  2018-08-29 18:58 ` Dr. Philipp Tomsich
  0 siblings, 1 reply; 2+ messages in thread
From: Kever Yang @ 2018-08-28  9:23 UTC (permalink / raw)
  To: u-boot

Rockchip BootRom support load firmware from storage media twice,
one for ddr sdram init code into sram and another time to load
firmware into DDR. For ddr sdram init code(which is TPL in U-Boot
project), it's OK to re-use the stack and vector table. In order
to get more available sram space, we need to skip all the init code
from U-Boot project including start.S, reset.S and framework.

Signed-off-by: Kever Yang <kever.yang@rock-chips.com>
---

 arch/arm/cpu/armv7/start.S                 |   2 +
 arch/arm/cpu/armv8/start.S                 |   2 +
 arch/arm/include/asm/arch-rockchip/boot0.h |  10 ++
 arch/arm/mach-rockchip/Kconfig             |  12 +++
 arch/arm/mach-rockchip/Makefile            |   2 +
 arch/arm/mach-rockchip/tiny_tpl.c          | 106 +++++++++++++++++++++
 6 files changed, 134 insertions(+)
 create mode 100644 arch/arm/mach-rockchip/tiny_tpl.c

diff --git a/arch/arm/cpu/armv7/start.S b/arch/arm/cpu/armv7/start.S
index 81edec01bf..548d9ff23a 100644
--- a/arch/arm/cpu/armv7/start.S
+++ b/arch/arm/cpu/armv7/start.S
@@ -38,7 +38,9 @@
 reset:
 	/* Allow the board to save important registers */
 	b	save_boot_params
+#if !CONFIG_IS_ENABLED(TINY_FRAMEWORK)
 save_boot_params_ret:
+#endif
 #ifdef CONFIG_ARMV7_LPAE
 /*
  * check for Hypervisor support
diff --git a/arch/arm/cpu/armv8/start.S b/arch/arm/cpu/armv8/start.S
index d4db4d044f..e0f8fad10c 100644
--- a/arch/arm/cpu/armv8/start.S
+++ b/arch/arm/cpu/armv8/start.S
@@ -31,6 +31,7 @@ _start:
 	b	reset
 #endif
 
+#if !CONFIG_IS_ENABLED(TINY_FRAMEWORK)
 	.align 3
 
 .globl	_TEXT_BASE
@@ -361,3 +362,4 @@ ENDPROC(c_runtime_cpu_setup)
 WEAK(save_boot_params)
 	b	save_boot_params_ret	/* back to my caller */
 ENDPROC(save_boot_params)
+#endif
diff --git a/arch/arm/include/asm/arch-rockchip/boot0.h b/arch/arm/include/asm/arch-rockchip/boot0.h
index 9ea4708ada..7f00494513 100644
--- a/arch/arm/include/asm/arch-rockchip/boot0.h
+++ b/arch/arm/include/asm/arch-rockchip/boot0.h
@@ -41,8 +41,18 @@ entry_counter:
 
 #if (defined(CONFIG_SPL_BUILD) || defined(CONFIG_ARM64))
 	/* U-Boot proper of armv7 do not need this */
+#if CONFIG_IS_ENABLED(TINY_FRAMEWORK)
+	/* Allow the board to save important registers */
+	b save_boot_params
+.globl	save_boot_params_ret
+.type   save_boot_params_ret, %function
+
+save_boot_params_ret:
+	b board_init_f
+#else
 	b reset
 #endif
+#endif
 
 #if !defined(CONFIG_ARM64)
 	/*
diff --git a/arch/arm/mach-rockchip/Kconfig b/arch/arm/mach-rockchip/Kconfig
index 187aa14184..be23bcc37e 100644
--- a/arch/arm/mach-rockchip/Kconfig
+++ b/arch/arm/mach-rockchip/Kconfig
@@ -306,6 +306,18 @@ config TPL_ROCKCHIP_EARLYRETURN_TO_BROM
 config SPL_MMC_SUPPORT
 	default y if !SPL_ROCKCHIP_BACK_TO_BROM
 
+config TPL_TINY_FRAMEWORK
+	bool "Tiny TPL for DDR init"
+	depends on TPL && !TPL_FRAMEWORK
+	default y
+	help
+	  Rockchip BootRom support load firmware from storage media twice,
+	  one for ddr sdram init code into sram and another time to load
+	  firmware into DDR. For ddr sdram init code(which is TPL in U-Boot
+	  project), it's OK to re-use the stack and vector table. In order
+	  to get more available sram space, we need to skip all the init code
+	  from U-Boot project including start.S, reset.S and framework.
+
 source "arch/arm/mach-rockchip/rk3036/Kconfig"
 source "arch/arm/mach-rockchip/rk3128/Kconfig"
 source "arch/arm/mach-rockchip/rk3188/Kconfig"
diff --git a/arch/arm/mach-rockchip/Makefile b/arch/arm/mach-rockchip/Makefile
index 83afdac99d..c88700f10d 100644
--- a/arch/arm/mach-rockchip/Makefile
+++ b/arch/arm/mach-rockchip/Makefile
@@ -9,6 +9,8 @@
 obj-spl-$(CONFIG_ROCKCHIP_BROM_HELPER) += bootrom.o
 obj-tpl-$(CONFIG_ROCKCHIP_BROM_HELPER) += bootrom.o
 
+obj-tpl-$(CONFIG_TPL_TINY_FRAMEWORK) += tiny_tpl.o
+
 obj-tpl-$(CONFIG_ROCKCHIP_RK3288) += rk3288-board-tpl.o
 obj-tpl-$(CONFIG_ROCKCHIP_RK3368) += rk3368-board-tpl.o
 obj-tpl-$(CONFIG_ROCKCHIP_RK322X) += rk322x-board-tpl.o
diff --git a/arch/arm/mach-rockchip/tiny_tpl.c b/arch/arm/mach-rockchip/tiny_tpl.c
new file mode 100644
index 0000000000..47f15c0af1
--- /dev/null
+++ b/arch/arm/mach-rockchip/tiny_tpl.c
@@ -0,0 +1,106 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * (C) Copyright 2018 Rockchip Electronics Co., Ltd
+ */
+
+#include <common.h>
+#include <debug_uart.h>
+#include <spl.h>
+#include <version.h>
+#include <asm/io.h>
+#include <asm/arch/bootrom.h>
+#include <asm/arch-rockchip/sys_proto.h>
+
+#ifndef CONFIG_TPL_LIBGENERIC_SUPPORT
+#ifdef CONFIG_ARM64
+/* for ARM64,it don't have define timer_init and __udelay except lib/timer.c */
+int __weak timer_init(void)
+{
+	return 0;
+}
+
+void __weak __udelay(unsigned long usec)
+{
+	u64 i, j, count;
+
+	asm volatile ("MRS %0, CNTPCT_EL0" : "=r"(count));
+	i = count;
+	/* usec to count,24MHz */
+	j = usec * 24;
+	i += j;
+	while (1) {
+		asm volatile ("MRS %0, CNTPCT_EL0" : "=r"(count));
+		if (count > i)
+			break;
+	}
+}
+#endif /* CONFIG_ARM64 */
+void udelay(unsigned long usec)
+{
+	__udelay(usec);
+}
+
+void hang(void)
+{
+	for (;;)
+		;
+}
+#endif /* CONFIG_TPL_LIBGENERIC_SUPPORT */
+
+u32 spl_boot_device(void)
+{
+	return BOOT_DEVICE_BOOTROM;
+}
+
+__weak void rockchip_stimer_init(void)
+{
+#ifndef CONFIG_ARM64
+	asm volatile("mcr p15, 0, %0, c14, c0, 0"
+		     : : "r"(COUNTER_FREQUENCY));
+#else
+	/*
+	 * For ARM64,generally initialize CNTFRQ in start.S,
+	 * but if defined CONFIG_TPL_TINY_FRAMEWORK should skip start.S.
+	 * So initialize CNTFRQ to 24MHz here.
+	 */
+	asm volatile("msr CNTFRQ_EL0, %0"
+		     : : "r" (COUNTER_FREQUENCY));
+#endif
+	writel(0, CONFIG_ROCKCHIP_STIMER_BASE + 0x10);
+	writel(0xffffffff, CONFIG_ROCKCHIP_STIMER_BASE);
+	writel(0xffffffff, CONFIG_ROCKCHIP_STIMER_BASE + 4);
+	writel(1, CONFIG_ROCKCHIP_STIMER_BASE + 0x10);
+}
+
+__weak int arch_cpu_init(void)
+{
+	return 0;
+}
+
+void board_init_f(ulong dummy)
+{
+	rockchip_stimer_init();
+	arch_cpu_init();
+#define EARLY_DEBUG
+#ifdef EARLY_DEBUG
+	/*
+	 * Debug UART can be used from here if required:
+	 *
+	 * debug_uart_init();
+	 * printch('a');
+	 * printhex8(0x1234);
+	 * printascii("string");
+	 */
+	debug_uart_init();
+	printascii("\nU-Boot Tiny TPL " PLAIN_VERSION " (" U_BOOT_DATE " - " \
+				U_BOOT_TIME ")\n");
+#endif
+
+	/* Init ARM arch timer */
+	timer_init();
+
+	/* We are not going to use DM framework in tiny TPL */
+	sdram_init();
+
+	back_to_bootrom(BROM_BOOT_NEXTSTAGE);
+}
-- 
2.18.0

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

* [U-Boot] [PATCH] rockchip: add TPL_TINY_FRAMEWORK support
  2018-08-28  9:23 [U-Boot] [PATCH] rockchip: add TPL_TINY_FRAMEWORK support Kever Yang
@ 2018-08-29 18:58 ` Dr. Philipp Tomsich
  0 siblings, 0 replies; 2+ messages in thread
From: Dr. Philipp Tomsich @ 2018-08-29 18:58 UTC (permalink / raw)
  To: u-boot


> On 28 Aug 2018, at 11:23, Kever Yang <kever.yang@rock-chips.com> wrote:
> 
> Rockchip BootRom support load firmware from storage media twice,
> one for ddr sdram init code into sram and another time to load
> firmware into DDR. For ddr sdram init code(which is TPL in U-Boot
> project), it's OK to re-use the stack and vector table. In order
> to get more available sram space, we need to skip all the init code
> from U-Boot project including start.S, reset.S and framework.
> 
> Signed-off-by: Kever Yang <kever.yang@rock-chips.com>

Reviewed-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>

See below for a few recommended changes, a question and one required change.

> ---
> 
> arch/arm/cpu/armv7/start.S                 |   2 +
> arch/arm/cpu/armv8/start.S                 |   2 +
> arch/arm/include/asm/arch-rockchip/boot0.h |  10 ++
> arch/arm/mach-rockchip/Kconfig             |  12 +++
> arch/arm/mach-rockchip/Makefile            |   2 +
> arch/arm/mach-rockchip/tiny_tpl.c          | 106 +++++++++++++++++++++
> 6 files changed, 134 insertions(+)
> create mode 100644 arch/arm/mach-rockchip/tiny_tpl.c
> 
> diff --git a/arch/arm/cpu/armv7/start.S b/arch/arm/cpu/armv7/start.S
> index 81edec01bf..548d9ff23a 100644
> --- a/arch/arm/cpu/armv7/start.S
> +++ b/arch/arm/cpu/armv7/start.S
> @@ -38,7 +38,9 @@
> reset:
> 	/* Allow the board to save important registers */
> 	b	save_boot_params
> +#if !CONFIG_IS_ENABLED(TINY_FRAMEWORK)
> save_boot_params_ret:
> +#endif

Having the label here will not increase code-size, so there’s no point in
making this conditional.

> #ifdef CONFIG_ARMV7_LPAE
> /*
>  * check for Hypervisor support
> diff --git a/arch/arm/cpu/armv8/start.S b/arch/arm/cpu/armv8/start.S
> index d4db4d044f..e0f8fad10c 100644
> --- a/arch/arm/cpu/armv8/start.S
> +++ b/arch/arm/cpu/armv8/start.S
> @@ -31,6 +31,7 @@ _start:
> 	b	reset
> #endif
> 
> +#if !CONFIG_IS_ENABLED(TINY_FRAMEWORK)
> 	.align 3
> 
> .globl	_TEXT_BASE
> @@ -361,3 +362,4 @@ ENDPROC(c_runtime_cpu_setup)
> WEAK(save_boot_params)
> 	b	save_boot_params_ret	/* back to my caller */
> ENDPROC(save_boot_params)
> +#endif
> diff --git a/arch/arm/include/asm/arch-rockchip/boot0.h b/arch/arm/include/asm/arch-rockchip/boot0.h
> index 9ea4708ada..7f00494513 100644
> --- a/arch/arm/include/asm/arch-rockchip/boot0.h
> +++ b/arch/arm/include/asm/arch-rockchip/boot0.h
> @@ -41,8 +41,18 @@ entry_counter:
> 
> #if (defined(CONFIG_SPL_BUILD) || defined(CONFIG_ARM64))
> 	/* U-Boot proper of armv7 do not need this */
> +#if CONFIG_IS_ENABLED(TINY_FRAMEWORK)
> +	/* Allow the board to save important registers */
> +	b save_boot_params
> +.globl	save_boot_params_ret
> +.type   save_boot_params_ret, %function
> +
> +save_boot_params_ret:
> +	b board_init_f
> +#else
> 	b reset
> #endif
> +#endif

I don’t like these nested #if primitives.
Is there a way to do this without nesting?
E.g. 

#if CONFIG_IS_ENABLED(TINY_FRAMEWORK)
	b reset
#elif …
	original code
#endif

> 
> #if !defined(CONFIG_ARM64)
> 	/*
> diff --git a/arch/arm/mach-rockchip/Kconfig b/arch/arm/mach-rockchip/Kconfig
> index 187aa14184..be23bcc37e 100644
> --- a/arch/arm/mach-rockchip/Kconfig
> +++ b/arch/arm/mach-rockchip/Kconfig
> @@ -306,6 +306,18 @@ config TPL_ROCKCHIP_EARLYRETURN_TO_BROM
> config SPL_MMC_SUPPORT
> 	default y if !SPL_ROCKCHIP_BACK_TO_BROM
> 
> +config TPL_TINY_FRAMEWORK
> +	bool "Tiny TPL for DDR init"
> +	depends on TPL && !TPL_FRAMEWORK
> +	default y
> +	help
> +	  Rockchip BootRom support load firmware from storage media twice,
> +	  one for ddr sdram init code into sram and another time to load
> +	  firmware into DDR. For ddr sdram init code(which is TPL in U-Boot
> +	  project), it's OK to re-use the stack and vector table. In order
> +	  to get more available sram space, we need to skip all the init code
> +	  from U-Boot project including start.S, reset.S and framework.
> +
> source "arch/arm/mach-rockchip/rk3036/Kconfig"
> source "arch/arm/mach-rockchip/rk3128/Kconfig"
> source "arch/arm/mach-rockchip/rk3188/Kconfig"
> diff --git a/arch/arm/mach-rockchip/Makefile b/arch/arm/mach-rockchip/Makefile
> index 83afdac99d..c88700f10d 100644
> --- a/arch/arm/mach-rockchip/Makefile
> +++ b/arch/arm/mach-rockchip/Makefile
> @@ -9,6 +9,8 @@
> obj-spl-$(CONFIG_ROCKCHIP_BROM_HELPER) += bootrom.o
> obj-tpl-$(CONFIG_ROCKCHIP_BROM_HELPER) += bootrom.o
> 
> +obj-tpl-$(CONFIG_TPL_TINY_FRAMEWORK) += tiny_tpl.o
> +
> obj-tpl-$(CONFIG_ROCKCHIP_RK3288) += rk3288-board-tpl.o
> obj-tpl-$(CONFIG_ROCKCHIP_RK3368) += rk3368-board-tpl.o
> obj-tpl-$(CONFIG_ROCKCHIP_RK322X) += rk322x-board-tpl.o
> diff --git a/arch/arm/mach-rockchip/tiny_tpl.c b/arch/arm/mach-rockchip/tiny_tpl.c
> new file mode 100644
> index 0000000000..47f15c0af1
> --- /dev/null
> +++ b/arch/arm/mach-rockchip/tiny_tpl.c
> @@ -0,0 +1,106 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * (C) Copyright 2018 Rockchip Electronics Co., Ltd
> + */
> +
> +#include <common.h>
> +#include <debug_uart.h>
> +#include <spl.h>
> +#include <version.h>
> +#include <asm/io.h>
> +#include <asm/arch/bootrom.h>
> +#include <asm/arch-rockchip/sys_proto.h>
> +
> +#ifndef CONFIG_TPL_LIBGENERIC_SUPPORT
> +#ifdef CONFIG_ARM64
> +/* for ARM64,it don't have define timer_init and __udelay except lib/timer.c */
> +int __weak timer_init(void)
> +{
> +	return 0;
> +}
> +
> +void __weak __udelay(unsigned long usec)
> +{
> +	u64 i, j, count;
> +
> +	asm volatile ("MRS %0, CNTPCT_EL0" : "=r"(count));
> +	i = count;
> +	/* usec to count,24MHz */
> +	j = usec * 24;
> +	i += j;
> +	while (1) {
> +		asm volatile ("MRS %0, CNTPCT_EL0" : "=r"(count));
> +		if (count > i)
> +			break;
> +	}
> +}
> +#endif /* CONFIG_ARM64 */
> +void udelay(unsigned long usec)
> +{
> +	__udelay(usec);
> +}
> +
> +void hang(void)
> +{
> +	for (;;)
> +		;
> +}
> +#endif /* CONFIG_TPL_LIBGENERIC_SUPPORT */
> +
> +u32 spl_boot_device(void)
> +{
> +	return BOOT_DEVICE_BOOTROM;
> +}
> +
> +__weak void rockchip_stimer_init(void)
> +{
> +#ifndef CONFIG_ARM64

If we need to keep an #if here, then please have this not inverted.
I.e.
#if defined(CONFIG_ARM)
		armv8 code here
#else
		armv7 code here
#endif

> +	asm volatile("mcr p15, 0, %0, c14, c0, 0"
> +		     : : "r"(COUNTER_FREQUENCY));
> +#else
> +	/*
> +	 * For ARM64,generally initialize CNTFRQ in start.S,
> +	 * but if defined CONFIG_TPL_TINY_FRAMEWORK should skip start.S.
> +	 * So initialize CNTFRQ to 24MHz here.
> +	 */
> +	asm volatile("msr CNTFRQ_EL0, %0"
> +		     : : "r" (COUNTER_FREQUENCY));
> +#endif

Can we do this without having the #ifdef here?
Maybe call an inline function that is implemented differently in the armv7 and armv8 header files?

> +	writel(0, CONFIG_ROCKCHIP_STIMER_BASE + 0x10);
> +	writel(0xffffffff, CONFIG_ROCKCHIP_STIMER_BASE);
> +	writel(0xffffffff, CONFIG_ROCKCHIP_STIMER_BASE + 4);
> +	writel(1, CONFIG_ROCKCHIP_STIMER_BASE + 0x10);

NAK.
Don’t make CONFIG_ROCKCHIP_STIMER_BASE configurable via Kconfig.
This is not a user-configurable option, but a direct consequence of the chip we’re using.
In other words: it’s not a CONFIG-option.

> +}
> +
> +__weak int arch_cpu_init(void)
> +{
> +	return 0;
> +}
> +
> +void board_init_f(ulong dummy)
> +{
> +	rockchip_stimer_init();
> +	arch_cpu_init();
> +#define EARLY_DEBUG
> +#ifdef EARLY_DEBUG
> +	/*
> +	 * Debug UART can be used from here if required:
> +	 *
> +	 * debug_uart_init();
> +	 * printch('a');
> +	 * printhex8(0x1234);
> +	 * printascii("string");
> +	 */
> +	debug_uart_init();
> +	printascii("\nU-Boot Tiny TPL " PLAIN_VERSION " (" U_BOOT_DATE " - " \
> +				U_BOOT_TIME ")\n");
> +#endif
> +
> +	/* Init ARM arch timer */
> +	timer_init();
> +
> +	/* We are not going to use DM framework in tiny TPL */
> +	sdram_init();
> +
> +	back_to_bootrom(BROM_BOOT_NEXTSTAGE);
> +}
> -- 
> 2.18.0
> 

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

end of thread, other threads:[~2018-08-29 18:58 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-08-28  9:23 [U-Boot] [PATCH] rockchip: add TPL_TINY_FRAMEWORK support Kever Yang
2018-08-29 18:58 ` Dr. Philipp Tomsich

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.