Hi Simon, On 2024-07-31 16:49, 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.
I am guessing this size increase only is related to kevin/bob since all other RK3399 boards already use rockchip_sdram_size() in SPL. > > - Drop the non-dcache optimisation, since the cache should normally be on Is this referencing to something that is still done in this patch? > > Signed-off-by: Simon Glass <s...@chromium.org> > --- > > (no changes since v5) > > Changes in v5: > - Move setting of pmugrf into the probe() function > - Drop patches previously applied > > Changes in v4: > - Fix 'stating' typo > - Move Binman size feature to a separate series > > Changes in v3: > - Split out the refactoring into a separate patch > > Changes in v2: > - Drop patch "regulator: rk8xx: Fix incorrect parameter" > - Rewrite boneblack patch to onstead drop the target and update docs > > 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 bc79c034808..8a7cbb1849f 100644 > --- a/drivers/ram/rockchip/sdram_rk3399.c > +++ b/drivers/ram/rockchip/sdram_rk3399.c > @@ -13,6 +13,7 @@ > #include <log.h> > #include <ram.h> > #include <regmap.h> > +#include <spl.h> > #include <syscon.h> > #include <asm/arch-rockchip/clock.h> > #include <asm/arch-rockchip/cru.h> > @@ -63,8 +64,6 @@ struct chan_info { > }; > > struct dram_info { > -#if defined(CONFIG_TPL_BUILD) || \ > - (!defined(CONFIG_TPL) && defined(CONFIG_SPL_BUILD)) > u32 pwrup_srefresh_exit[2]; > struct chan_info chan[2]; > struct clk ddr_clk; > @@ -75,7 +74,6 @@ struct dram_info { > struct rk3399_pmusgrf_regs *pmusgrf; > struct rk3399_ddr_cic_regs *cic; > const struct sdram_rk3399_ops *ops; > -#endif > struct ram_info info; > struct rk3399_pmugrf_regs *pmugrf; > }; > @@ -92,9 +90,6 @@ struct sdram_rk3399_ops { > struct rk3399_sdram_params *params); > }; > > -#if defined(CONFIG_TPL_BUILD) || \ > - (!defined(CONFIG_TPL) && defined(CONFIG_SPL_BUILD)) > - > struct rockchip_dmc_plat { > #if CONFIG_IS_ENABLED(OF_PLATDATA) > struct dtd_rockchip_rk3399_dmc dtplat; > @@ -191,6 +186,17 @@ struct io_setting { > }, > }; > > +/** > + * 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 is still wrong, please consider ROCKCHIP_EXTERNAL_TPL or this function fail to do what you document it to do. See our prior thread about this: https://lore.kernel.org/u-boot/caflszthg5xohr-9agsmhvrw5dp_tn39nc7rq1z4zmnqoqqj...@mail.gmail.com/ Regards, Jonas > +} > + > static struct io_setting * > lpddr4_get_io_settings(const struct rk3399_sdram_params *params, u32 mr5) > { > @@ -3024,7 +3030,7 @@ static int rk3399_dmc_of_to_plat(struct udevice *dev) > struct rockchip_dmc_plat *plat = dev_get_plat(dev); > int ret; > > - if (!CONFIG_IS_ENABLED(OF_REAL)) > + if (!CONFIG_IS_ENABLED(OF_REAL) || !phase_sdram_init()) > return 0; > > ret = dev_read_u32_array(dev, "rockchip,sdram-params", > @@ -3093,7 +3099,6 @@ static int rk3399_dmc_init(struct udevice *dev) > priv->cic = syscon_get_first_range(ROCKCHIP_SYSCON_CIC); > priv->grf = syscon_get_first_range(ROCKCHIP_SYSCON_GRF); > priv->pmu = syscon_get_first_range(ROCKCHIP_SYSCON_PMU); > - priv->pmugrf = syscon_get_first_range(ROCKCHIP_SYSCON_PMUGRF); > priv->pmusgrf = syscon_get_first_range(ROCKCHIP_SYSCON_PMUSGRF); > priv->pmucru = rockchip_get_pmucru(); > priv->cru = rockchip_get_cru(); > @@ -3138,19 +3143,16 @@ static int rk3399_dmc_init(struct udevice *dev) > > return 0; > } > -#endif > > static int rk3399_dmc_probe(struct udevice *dev) > { > struct dram_info *priv = dev_get_priv(dev); > > -#if defined(CONFIG_TPL_BUILD) || \ > - (!defined(CONFIG_TPL) && defined(CONFIG_SPL_BUILD)) > - if (rk3399_dmc_init(dev)) > - return 0; > -#endif > priv->pmugrf = syscon_get_first_range(ROCKCHIP_SYSCON_PMUGRF); > debug("%s: pmugrf = %p\n", __func__, priv->pmugrf); > + if (phase_sdram_init() && rk3399_dmc_init(dev)) > + return 0; > + > priv->info.base = CFG_SYS_SDRAM_BASE; > priv->info.size = > rockchip_sdram_size((phys_addr_t)&priv->pmugrf->os_reg2); > @@ -3181,10 +3183,7 @@ U_BOOT_DRIVER(dmc_rk3399) = { > .id = UCLASS_RAM, > .of_match = rk3399_dmc_ids, > .ops = &rk3399_dmc_ops, > -#if defined(CONFIG_TPL_BUILD) || \ > - (!defined(CONFIG_TPL) && defined(CONFIG_SPL_BUILD)) > .of_to_plat = rk3399_dmc_of_to_plat, > -#endif > .probe = rk3399_dmc_probe, > .priv_auto = sizeof(struct dram_info), > #if defined(CONFIG_TPL_BUILD) || \