Hi Jonas,
On Wed, 5 Feb 2025 at 15:18, Jonas Karlman <jo...@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 <s...@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
