[PATCH 05/29] rockchip: Allow RAM init to happen in SPL on rk3399

[Simon Glass]

Hi Jonas,

On Wed, 5 Feb 2025 at 15:18, Jonas Karlman <jonas at kwiboo.se> wrote:
>
> Hi Simon,
>
> On 2025-02-05 02:54, Simon Glass wrote:
> > TPL runs before VPL. The earliest updatable phase with VBE is SPL. We
> > want to be able to update the RAM-init code in the field.
>
> If I understand your description here, you mean that both TPL and VPL
> are meant to be "burned" onto the board and not intended to be updatable?

Yes, that's right. If you look at [1] and click on 'vbe-intro' there
is a diagram that shows the flow.

>
> Is this a typo or do I misunderstand something? Else I do not understand
> why you would need TPL->VPL->SPL, please explain.
>
> >
> > So when VPL is being used, init the RAM later, in SPL.
> >
> > Signed-off-by: Simon Glass <sjg at chromium.org>
> > ---
> >
> >  drivers/ram/rockchip/sdram_rk3399.c | 6 ++++--
> >  1 file changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/ram/rockchip/sdram_rk3399.c b/drivers/ram/rockchip/sdram_rk3399.c
> > index 6fa8f268770..9ac16dfdc71 100644
> > --- a/drivers/ram/rockchip/sdram_rk3399.c
> > +++ b/drivers/ram/rockchip/sdram_rk3399.c
> > @@ -194,6 +194,7 @@ struct io_setting {
> >  static bool phase_sdram_init(void)
> >  {
> >       return xpl_phase() == PHASE_TPL ||
> > +             (IS_ENABLED(CONFIG_VPL) && xpl_phase() == PHASE_SPL) ||
> >               (!IS_ENABLED(CONFIG_TPL) &&
> >                !IS_ENABLED(CONFIG_ROCKCHIP_EXTERNAL_TPL) &&
> >                !not_xpl());
> > @@ -3195,8 +3196,9 @@ U_BOOT_DRIVER(dmc_rk3399) = {
> >       .of_to_plat = rk3399_dmc_of_to_plat,
> >       .probe = rk3399_dmc_probe,
> >       .priv_auto      = sizeof(struct dram_info),
> > -#if defined(CONFIG_TPL_BUILD) || \
> > -     (!defined(CONFIG_TPL) && defined(CONFIG_XPL_BUILD))
> > +#if defined(CONFIG_VPL) && defined(CONFIG_SPL_BUILD) || \
> > +     !defined(CONFIG_VPL) && defined(CONFIG_TPL_BUILD) || \
> > +     !defined(CONFIG_TPL) && defined(CONFIG_SPL_BUILD)
> >       .plat_auto      = sizeof(struct rockchip_dmc_plat),
> >  #endif
> >  };
>
> These two conditions are starting to get ridiculous.
>
> With "[PATCH 24/29] rockchip: Allow SPL to set up SDRAM" you are
> depending on TPL_RAM to decide if RAM should be initialized. Maybe that
> is something we can use here to simplify?

The problem is that TPL_RAM means that the ram driver is present,
which isn't the same thing as actually doing the RAM init in that
phase. If we had a Kconfig value for 'phase' then we could perhaps
have CONFIG_PHASE_RAM=tpl or something like that. I plan to get back
to my rejected XPL series soon and that is something I could
incorporate when that goes in.

>
> Basically we will have 3 possible DRAM init scenarios to support:
> - ROCKCHIP_EXTERNA_TPL=y -> DRAM init happen in external TPL and SPL
>   should only read DRAM size
> - VPL=y or TPL=n -> both DRAM init and read DRAM size happen in SPL
> - TPL=y -> DRAM init in TPL and SPL should only read DRAM size

Yes that's my understanding too. For the last one, for clarity:

 - TPL=y -> DRAM init in TPL; SPL should only read DRAM size

Regards,
Simon

[1] https://docs.u-boot.org/en/latest/develop/vbe.html#verified-boot-for-embedded-vbe