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 <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.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
signature.asc
Description: PGP signature
