From mboxrd@z Thu Jan 1 00:00:00 1970 From: Dr. Philipp Tomsich Date: Wed, 29 Aug 2018 20:58:49 +0200 Subject: [U-Boot] [PATCH] rockchip: add TPL_TINY_FRAMEWORK support In-Reply-To: <20180828092346.17647-1-kever.yang@rock-chips.com> References: <20180828092346.17647-1-kever.yang@rock-chips.com> Message-ID: List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 8bit To: u-boot@lists.denx.de > On 28 Aug 2018, at 11:23, Kever Yang 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 Reviewed-by: Philipp Tomsich 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 > +#include > +#include > +#include > +#include > +#include > +#include > + > +#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 >