Hi Simon, On 2024-06-26 17:59, Simon Glass wrote: > The code here is confusing due to large blocks which are #ifdefed out. > Add a function phase_sdram_init() which returns whether SDRAM init > should happen in the current phase, using that as needed to control the > code flow. > > This increases code size by about 500 bytes in SPL when the cache is on, > since it must call the rather large rockchip_sdram_size() function. > > - Drop the non-dcache optimisation, since the cache should normally be on > > Signed-off-by: Simon Glass <s...@chromium.org> > --- > > Changes in v5: > - Move setting of pmugrf into the probe() function > > Changes in v3: > - Split out the refactoring into a separate patch > > drivers/ram/rockchip/sdram_rk3399.c | 33 ++++++++++++++--------------- > 1 file changed, 16 insertions(+), 17 deletions(-) > > diff --git a/drivers/ram/rockchip/sdram_rk3399.c > b/drivers/ram/rockchip/sdram_rk3399.c > index 3c4e20f4e80..b259e8e3dd6 100644 > --- a/drivers/ram/rockchip/sdram_rk3399.c > +++ b/drivers/ram/rockchip/sdram_rk3399.c
[snip] > +/** > + * phase_sdram_init() - Check if this is the phase where SDRAM init happens > + * > + * Returns: true to do SDRAM init in this phase, false to not > + */ > +static bool phase_sdram_init(void) > +{ > + return spl_phase() == PHASE_TPL || > + (!IS_ENABLED(CONFIG_TPL) && !spl_in_proper()); This still need to check for !IS_ENABLED(CONFIG_ROCKCHIP_EXTERNAL_TPL) to be correct, else you must build with both CONFIG_TPL and CONFIG_ROCKCHIP_EXTERNAL_TPL or the generated SPL will incorrectly try to initialize DRAM. DRAM should only be initialized in TPL or ROCKCHIP_EXTERNAL_TPL, or if none of them are defined in SPL. All other phases should just read and report dram size. Regards, Jonas > +} > + [snip]