[U-Boot] [PATCH 1/2] rockchip: allow DRAM init in SPL and fix ordering

[Kever Yang]

Hi Thomas,

On 2019/11/15 上午12:09, Tom Hebb wrote:
> Hi Kever,
>
> On Thu, Nov 14, 2019 at 1:25 AM Kever Yang <kever.yang at rock-chips.com 
> <mailto:kever.yang at rock-chips.com>> wrote:
>
>     Hi Thomas,
>
>     On 2019/11/11 上午12:25, Thomas Hebb wrote:
>     > b7abef2ecbcc ("rockchip: rk3399: Migrate to use common spl board
>     file")
>     > removed SoC-specific code for RK3399's SPL and in the process
>     introduced
>     > two regressions:
>     >
>     >   1. It caused the previously-unconditional DRAM initialization in
>     >      board_init_f() to only happen when compiling a
>     configuration that
>     >      did not support TPL, meaning DRAM would never get
>     initialized if TPL
>     >      was supported but disabled.
>
>
>     If the board support TPL, that means the dram init should be done in
>     TPL, no matter the really
>
>     implement is the U-Boot TPL or use rockchip rkbin instead. In this
>     case,
>     enable DRAM driver in SPL
>
>     make no sense, for dram driver itself would only get dram size
>     which do
>     not have any use other
>
>     than the case CONFIG_SPL_OS_BOOT.
>
>
> I would like to boot my RK3399 board with only an SPL, loaded directly 
> by the.
> BootROM into IRAM. I have no need for a three-stage bootloader, and adding
> only increases boot time and adds complexity. Is there a reason not to 
> support
> this use case?


OK, I understand it now, you update is necessary, thanks.

Well, it will be better to make this patch into two patches.

>
> I do realize that this will add an unnecessary call if someone uses 
> rkbin for DDR
> init and SPL for the second stage. Do any boards actually do that, though?
>
>     >   2. It reordered the DRAM initialization before
>     rockchip_stimer_init(),
>     >      which as far as I can tell causes RK3399 to lock up completely.
>
>
>     The stimer_init should goes before dram_init.
>
>     So please only update the order.
>
>
> Should timer_init() also go before dram_init(), like I have it now? Or 
> should I put
> dram_init() between rockchip_stimer_init() and timer_init()?


timer_init() goes before dram_init, because the dram_init() will use 
udelay().


Thanks,

- Kever

>
>     Thanks,
>
>     - Kever
>
>     >
>     > Fix both of these issues in the common code so that we can again
>     boot
>     > RK3399 without a TPL. This fixes custom configurations that have
>     > disabled TPL, and it should also unbreak the "ficus-rk3399",
>     > "rock960-rk3399", and "chromebook_bob" defconfigs, although since I
>     > don't have any of those devices I can't confirm they're broken now.
>     >
>     > Signed-off-by: Thomas Hebb <tommyhebb at gmail.com
>     <mailto:tommyhebb at gmail.com>>
>     > ---
>     >   arch/arm/mach-rockchip/spl.c | 18 +++++++++---------
>     >   1 file changed, 9 insertions(+), 9 deletions(-)
>     >
>     > diff --git a/arch/arm/mach-rockchip/spl.c
>     b/arch/arm/mach-rockchip/spl.c
>     > index 92102b39e7..089f0a5258 100644
>     > --- a/arch/arm/mach-rockchip/spl.c
>     > +++ b/arch/arm/mach-rockchip/spl.c
>     > @@ -103,7 +103,7 @@ __weak int arch_cpu_init(void)
>     >   void board_init_f(ulong dummy)
>     >   {
>     >       int ret;
>     > -#if !defined(CONFIG_SUPPORT_TPL) || defined(CONFIG_SPL_OS_BOOT)
>     > +#if !defined(CONFIG_TPL) || defined(CONFIG_SPL_OS_BOOT)
>     >       struct udevice *dev;
>     >   #endif
>     >
>     > @@ -128,20 +128,20 @@ void board_init_f(ulong dummy)
>     >               hang();
>     >       }
>     >       arch_cpu_init();
>     > -#if !defined(CONFIG_SUPPORT_TPL) || defined(CONFIG_SPL_OS_BOOT)
>     > -     debug("\nspl:init dram\n");
>     > -     ret = uclass_get_device(UCLASS_RAM, 0, &dev);
>     > -     if (ret) {
>     > -             printf("DRAM init failed: %d\n", ret);
>     > -             return;
>     > -     }
>     > -#endif
>     >   #if !defined(CONFIG_ROCKCHIP_RK3188)
>     >       rockchip_stimer_init();
>     >   #endif
>     >   #ifdef CONFIG_SYS_ARCH_TIMER
>     >       /* Init ARM arch timer in arch/arm/cpu/armv7/arch_timer.c */
>     >       timer_init();
>     > +#endif
>     > +#if !defined(CONFIG_TPL) || defined(CONFIG_SPL_OS_BOOT)
>     > +     debug("\nspl:init dram\n");
>     > +     ret = uclass_get_device(UCLASS_RAM, 0, &dev);
>     > +     if (ret) {
>     > +             printf("DRAM init failed: %d\n", ret);
>     > +             return;
>     > +     }
>     >   #endif
>     >       preloader_console_init();
>     >   }
>
>