diff mbox series

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

Message ID 20200512103446.1533277-1-heiko@sntech.de
State Rejected
Delegated to: Kever Yang
Headers show
Series [1/2] rockchip: spl: do full dram_init instead of only probing | expand

Commit Message

Heiko Stübner May 12, 2020, 10:34 a.m. UTC
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(-)

Comments

Kever Yang May 15, 2020, 2:07 a.m. UTC | #1
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();
>   }
Heiko Stübner May 15, 2020, 10:33 a.m. UTC | #2
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();
> >   }
> 
> 
>
diff mbox series

Patch

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();
 }