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

[Tom Hebb]

Hi Kever,

On Thu, Nov 14, 2019 at 1:25 AM Kever Yang <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?

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()?

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