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

[Tom Rini]

On Sun, Feb 09, 2025 at 07:26:38AM -0700, Simon Glass wrote:
> 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.

Please note that if you're going to revisit the "XPL" series I rejected
you need to do what I asked instead or there's just going to be a larger
gap between mainline and your downstream fork.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20250210/cab696b2/attachment.sig>