[PATCH 1/2] rockchip: spl: do full dram_init instead of only probing

[PATCH 1/2] rockchip: spl: do full dram_init instead of only probing

[Heiko Stuebner]

From: Heiko Stuebner <heiko.stuebner@theobroma-systems.com>

Parts of later SPL may need RAM information as well, so do full
dram_init() call, which includes the existing dram probing but also
initializes the ram information in gd.

All Rockchip SoCs use a TPL+SPL combination now, so that's ok for all
of them.

Signed-off-by: Heiko Stuebner <heiko.stuebner@theobroma-systems.com>
Change-Id: I2c7496f2d88d65a9f80f74d2139bf307bb4b442b
---
 arch/arm/mach-rockchip/spl.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mach-rockchip/spl.c b/arch/arm/mach-rockchip/spl.c
index 0b76af6080..0eda2c3485 100644
--- a/arch/arm/mach-rockchip/spl.c
+++ b/arch/arm/mach-rockchip/spl.c
@@ -135,13 +135,15 @@ void board_init_f(ulong dummy)
 	/* Init ARM arch timer in arch/arm/cpu/armv7/arch_timer.c */
 	timer_init();
 #endif
-#if !defined(CONFIG_TPL) || defined(CONFIG_SPL_OS_BOOT)
+#if !defined(CONFIG_TPL) || defined(CONFIG_SPL_RAM)
 	debug("\nspl:init dram\n");
-	ret = uclass_get_device(UCLASS_RAM, 0, &dev);
+	ret = dram_init();
 	if (ret) {
 		printf("DRAM init failed: %d\n", ret);
 		return;
 	}
+	gd->ram_top = gd->ram_base + get_effective_memsize();
+	gd->ram_top = board_get_usable_ram_top(gd->ram_size);
 #endif
 	preloader_console_init();
 }
-- 
2.25.1

[PATCH 2/2] spl: add fixed memory node in target fdt also when loading ATF

[Heiko Stuebner]

From: Heiko Stuebner <heiko.stuebner@theobroma-systems.com>

In a loading chain SPL -> ATF (->OP-TEE) -> U-Boot, ATF and a subsequent
OP-TEE will re-use the same fdt as the U-Boot target and may need the
information about usable memory ranges.

Especially OP-TEE needs this to initialize dynamic shared memory
(the only type U-Boot implements when talking to OP-TEE).

So allow spl_fixup_fdt() to take a fdt_blob argument, falling back to
the existing CONFIG_SYS_SPL_ARGS_ADDR if needed and call it from the
ATF path as well.

Signed-off-by: Heiko Stuebner <heiko.stuebner@theobroma-systems.com>
Change-Id: I95d555e170534ad7fd03b183773e0b1f2d90d486
---
 common/spl/spl.c | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/common/spl/spl.c b/common/spl/spl.c
index b0f0e1557b..90d8bfd058 100644
--- a/common/spl/spl.c
+++ b/common/spl/spl.c
@@ -58,7 +58,8 @@ static bd_t bdata __attribute__ ((section(".data")));
  */
 __weak void show_boot_progress(int val) {}
 
-#if defined(CONFIG_SPL_OS_BOOT) || CONFIG_IS_ENABLED(HANDOFF)
+#if defined(CONFIG_SPL_OS_BOOT) || CONFIG_IS_ENABLED(HANDOFF) || \
+    defined(CONFIG_SPL_ATF)
 /* weak, default platform-specific function to initialize dram banks */
 __weak int dram_init_banksize(void)
 {
@@ -100,12 +101,14 @@ void __weak spl_perform_fixups(struct spl_image_info *spl_image)
 {
 }
 
-void spl_fixup_fdt(void)
+void spl_fixup_fdt(void *fdt_blob)
 {
-#if defined(CONFIG_SPL_OF_LIBFDT) && defined(CONFIG_SYS_SPL_ARGS_ADDR)
-	void *fdt_blob = (void *)CONFIG_SYS_SPL_ARGS_ADDR;
+#if defined(CONFIG_SPL_OF_LIBFDT)
 	int err;
 
+	if (!fdt_blob)
+		return;
+
 	err = fdt_check_header(fdt_blob);
 	if (err < 0) {
 		printf("fdt_root: %s\n", fdt_strerror(err));
@@ -638,7 +641,8 @@ void board_init_r(gd_t *dummy1, ulong dummy2)
 	initr_watchdog();
 #endif
 
-	if (IS_ENABLED(CONFIG_SPL_OS_BOOT) || CONFIG_IS_ENABLED(HANDOFF))
+	if (IS_ENABLED(CONFIG_SPL_OS_BOOT) || CONFIG_IS_ENABLED(HANDOFF) ||
+	    IS_ENABLED(CONFIG_SPL_ATF))
 		dram_init_banksize();
 
 	bootcount_inc();
@@ -680,6 +684,7 @@ void board_init_r(gd_t *dummy1, ulong dummy2)
 #if CONFIG_IS_ENABLED(ATF)
 	case IH_OS_ARM_TRUSTED_FIRMWARE:
 		debug("Jumping to U-Boot via ARM Trusted Firmware\n");
+		spl_fixup_fdt(spl_image.fdt_addr);
 		spl_invoke_atf(&spl_image);
 		break;
 #endif
@@ -699,7 +704,9 @@ void board_init_r(gd_t *dummy1, ulong dummy2)
 #ifdef CONFIG_SPL_OS_BOOT
 	case IH_OS_LINUX:
 		debug("Jumping to Linux\n");
-		spl_fixup_fdt();
+#if defined(CONFIG_SYS_SPL_ARGS_ADDR)
+		spl_fixup_fdt((void *)CONFIG_SYS_SPL_ARGS_ADDR);
+#endif
 		spl_board_prepare_for_linux();
 		jump_to_image_linux(&spl_image);
 #endif
-- 
2.25.1

[PATCH 1/2] rockchip: spl: do full dram_init instead of only probing

[Kever Yang]

On 2020/5/12 ??6:34, Heiko Stuebner wrote:
> From: Heiko Stuebner <heiko.stuebner@theobroma-systems.com>
>
> Parts of later SPL may need RAM information as well, so do full
> dram_init() call, which includes the existing dram probing but also
> initializes the ram information in gd.

NAK.

I would prefer to use DM interface and leave the decision of ram 
initialization to the ram driver.

>
> All Rockchip SoCs use a TPL+SPL combination now, so that's ok for all
> of them.
Still not all of the board use the combination.
>
> Signed-off-by: Heiko Stuebner <heiko.stuebner@theobroma-systems.com>
> Change-Id: I2c7496f2d88d65a9f80f74d2139bf307bb4b442b

Please remove the Change-Id next time.

Thanks,

- Kever

> ---
>   arch/arm/mach-rockchip/spl.c | 6 ++++--
>   1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm/mach-rockchip/spl.c b/arch/arm/mach-rockchip/spl.c
> index 0b76af6080..0eda2c3485 100644
> --- a/arch/arm/mach-rockchip/spl.c
> +++ b/arch/arm/mach-rockchip/spl.c
> @@ -135,13 +135,15 @@ void board_init_f(ulong dummy)
>   	/* Init ARM arch timer in arch/arm/cpu/armv7/arch_timer.c */
>   	timer_init();
>   #endif
> -#if !defined(CONFIG_TPL) || defined(CONFIG_SPL_OS_BOOT)
> +#if !defined(CONFIG_TPL) || defined(CONFIG_SPL_RAM)
>   	debug("\nspl:init dram\n");
> -	ret = uclass_get_device(UCLASS_RAM, 0, &dev);
> +	ret = dram_init();
>   	if (ret) {
>   		printf("DRAM init failed: %d\n", ret);
>   		return;
>   	}
> +	gd->ram_top = gd->ram_base + get_effective_memsize();
> +	gd->ram_top = board_get_usable_ram_top(gd->ram_size);
>   #endif
>   	preloader_console_init();
>   }

[PATCH 2/2] spl: add fixed memory node in target fdt also when loading ATF

[Kever Yang]

On 2020/5/12 ??6:34, Heiko Stuebner wrote:
> From: Heiko Stuebner <heiko.stuebner@theobroma-systems.com>
>
> In a loading chain SPL -> ATF (->OP-TEE) -> U-Boot, ATF and a subsequent
> OP-TEE will re-use the same fdt as the U-Boot target and may need the
> information about usable memory ranges.
>
> Especially OP-TEE needs this to initialize dynamic shared memory
> (the only type U-Boot implements when talking to OP-TEE).
>
> So allow spl_fixup_fdt() to take a fdt_blob argument, falling back to
> the existing CONFIG_SYS_SPL_ARGS_ADDR if needed and call it from the
> ATF path as well.
>
> Signed-off-by: Heiko Stuebner <heiko.stuebner@theobroma-systems.com>

> Change-Id: I95d555e170534ad7fd03b183773e0b1f2d90d486

Remove this Change-Id.

For other parts:

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

Thanks,
- Kever

> ---
>   common/spl/spl.c | 19 +++++++++++++------
>   1 file changed, 13 insertions(+), 6 deletions(-)
>
> diff --git a/common/spl/spl.c b/common/spl/spl.c
> index b0f0e1557b..90d8bfd058 100644
> --- a/common/spl/spl.c
> +++ b/common/spl/spl.c
> @@ -58,7 +58,8 @@ static bd_t bdata __attribute__ ((section(".data")));
>    */
>   __weak void show_boot_progress(int val) {}
>   
> -#if defined(CONFIG_SPL_OS_BOOT) || CONFIG_IS_ENABLED(HANDOFF)
> +#if defined(CONFIG_SPL_OS_BOOT) || CONFIG_IS_ENABLED(HANDOFF) || \
> +    defined(CONFIG_SPL_ATF)
>   /* weak, default platform-specific function to initialize dram banks */
>   __weak int dram_init_banksize(void)
>   {
> @@ -100,12 +101,14 @@ void __weak spl_perform_fixups(struct spl_image_info *spl_image)
>   {
>   }
>   
> -void spl_fixup_fdt(void)
> +void spl_fixup_fdt(void *fdt_blob)
>   {
> -#if defined(CONFIG_SPL_OF_LIBFDT) && defined(CONFIG_SYS_SPL_ARGS_ADDR)
> -	void *fdt_blob = (void *)CONFIG_SYS_SPL_ARGS_ADDR;
> +#if defined(CONFIG_SPL_OF_LIBFDT)
>   	int err;
>   
> +	if (!fdt_blob)
> +		return;
> +
>   	err = fdt_check_header(fdt_blob);
>   	if (err < 0) {
>   		printf("fdt_root: %s\n", fdt_strerror(err));
> @@ -638,7 +641,8 @@ void board_init_r(gd_t *dummy1, ulong dummy2)
>   	initr_watchdog();
>   #endif
>   
> -	if (IS_ENABLED(CONFIG_SPL_OS_BOOT) || CONFIG_IS_ENABLED(HANDOFF))
> +	if (IS_ENABLED(CONFIG_SPL_OS_BOOT) || CONFIG_IS_ENABLED(HANDOFF) ||
> +	    IS_ENABLED(CONFIG_SPL_ATF))
>   		dram_init_banksize();
>   
>   	bootcount_inc();
> @@ -680,6 +684,7 @@ void board_init_r(gd_t *dummy1, ulong dummy2)
>   #if CONFIG_IS_ENABLED(ATF)
>   	case IH_OS_ARM_TRUSTED_FIRMWARE:
>   		debug("Jumping to U-Boot via ARM Trusted Firmware\n");
> +		spl_fixup_fdt(spl_image.fdt_addr);
>   		spl_invoke_atf(&spl_image);
>   		break;
>   #endif
> @@ -699,7 +704,9 @@ void board_init_r(gd_t *dummy1, ulong dummy2)
>   #ifdef CONFIG_SPL_OS_BOOT
>   	case IH_OS_LINUX:
>   		debug("Jumping to Linux\n");
> -		spl_fixup_fdt();
> +#if defined(CONFIG_SYS_SPL_ARGS_ADDR)
> +		spl_fixup_fdt((void *)CONFIG_SYS_SPL_ARGS_ADDR);
> +#endif
>   		spl_board_prepare_for_linux();
>   		jump_to_image_linux(&spl_image);
>   #endif

[PATCH 1/2] rockchip: spl: do full dram_init instead of only probing

[Heiko Stübner]

Hi Kever,

Am Freitag, 15. Mai 2020, 04:07:41 CEST schrieb Kever Yang:
> 
> On 2020/5/12 ??6:34, Heiko Stuebner wrote:
> > From: Heiko Stuebner <heiko.stuebner@theobroma-systems.com>
> >
> > Parts of later SPL may need RAM information as well, so do full
> > dram_init() call, which includes the existing dram probing but also
> > initializes the ram information in gd.
> 
> NAK.
> 
> I would prefer to use DM interface and leave the decision of ram 
> initialization to the ram driver.

With CONFIG_SPL_RAM, we get dram_init from mach-rockchip/sdram.c
not some board-specific variant and the in the code change below you
can see that it is guarded by such a defined(CONFIG_SPL_RAM).

That dram_init() function does only:
- uclass_get_device(UCLASS_RAM, 0, &dev);
- ram_get_info(dev, &ram);
- gd->ram_size = ram.size;


So the only difference between the old code and my change is that it
will store information about usable ram in the gd struct, so that other
parts of the SPL can access it - and we of course keep using the DM
interface as before ;-) .


Heiko


> > All Rockchip SoCs use a TPL+SPL combination now, so that's ok for all
> > of them.
> Still not all of the board use the combination.
> >
> > Signed-off-by: Heiko Stuebner <heiko.stuebner@theobroma-systems.com>
> > Change-Id: I2c7496f2d88d65a9f80f74d2139bf307bb4b442b
> 
> Please remove the Change-Id next time.
> 
> Thanks,
> 
> - Kever
> 
> > ---
> >   arch/arm/mach-rockchip/spl.c | 6 ++++--
> >   1 file changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/arm/mach-rockchip/spl.c b/arch/arm/mach-rockchip/spl.c
> > index 0b76af6080..0eda2c3485 100644
> > --- a/arch/arm/mach-rockchip/spl.c
> > +++ b/arch/arm/mach-rockchip/spl.c
> > @@ -135,13 +135,15 @@ void board_init_f(ulong dummy)
> >   	/* Init ARM arch timer in arch/arm/cpu/armv7/arch_timer.c */
> >   	timer_init();
> >   #endif
> > -#if !defined(CONFIG_TPL) || defined(CONFIG_SPL_OS_BOOT)
> > +#if !defined(CONFIG_TPL) || defined(CONFIG_SPL_RAM)
> >   	debug("\nspl:init dram\n");
> > -	ret = uclass_get_device(UCLASS_RAM, 0, &dev);
> > +	ret = dram_init();
> >   	if (ret) {
> >   		printf("DRAM init failed: %d\n", ret);
> >   		return;
> >   	}
> > +	gd->ram_top = gd->ram_base + get_effective_memsize();
> > +	gd->ram_top = board_get_usable_ram_top(gd->ram_size);
> >   #endif
> >   	preloader_console_init();
> >   }
> 
> 
>