Hi Alper, On Sat, 24 Oct 2020 at 16:29, Alper Nebi Yasak <[email protected]> wrote: > > On 24/10/2020 03:05, Simon Glass wrote: > > Hi Alper, > > > > On Thu, 22 Oct 2020 at 14:37, Alper Nebi Yasak <[email protected]> > > wrote: > >> > >> It's possible to chainload U-Boot proper from the vendor firmware in > >> rk3399 chromebooks, but the way the vendor firmware sets up clocks is > >> somehow different than what U-Boot expects. This causes the display to > >> stay devoid of content even though vidconsole claims to work (with > >> patches in process of being upstreamed). > >> > >> This is meant to be a rk3399 version of commit d3cb46aa8c41 ("rockchip: > >> Init clocks again when chain-loading") which can detect the discrepancy, > >> but this patch can not so it always re-inits. > >> > >> Signed-off-by: Alper Nebi Yasak <[email protected]> > >> --- > >> The rk3288 version has rockchip_vop_set_clk in #ifndef CONFIG_BUILD_SPL, > >> and checks if that setup is already done before that's called. I think I > >> can do the #ifndef to avoid unnecessary re-inits for rk3399 as well, but > >> the vop clocks are set differently for each chip so I don't know how > >> correct that'd be. > >> > >> The pmucru setup is #ifndef CONFIG_BUILD_SPL on rk3399, so I think I can > >> also check for that, but that's technically in a different driver and I > >> don't know how best to do that. > >> > >> drivers/clk/rockchip/clk_rk3399.c | 7 +------ > >> 1 file changed, 1 insertion(+), 6 deletions(-) > > > > Could we make this conditional on SPL not running? We could check that > > by enabling CONFIG_HANDOFF, for example. > > Using CONFIG_HANDOFF in the probe function like this works on my side: > > static int rk3399_clk_probe(struct udevice *dev) > { > struct rk3399_clk_priv *priv = dev_get_priv(dev); > bool init_clocks = false; > > #if CONFIG_IS_ENABLED(OF_PLATDATA) > struct rk3399_clk_plat *plat = dev_get_platdata(dev); > > priv->cru = map_sysmem(plat->dtd.reg[0], plat->dtd.reg[1]); > #endif > > #if defined(CONFIG_SPL_BUILD) > init_clocks = true; > #elif CONFIG_IS_ENABLED(HANDOFF) > if (!(gd->spl_handoff)) > init_clocks = true; > #endif > > if (init_clocks) > rkclk_init(priv->cru); > > return 0; > } > > The clk_rk3288 code also checks for GD_FLG_RELOC, adding that didn't > look like it did anything good/bad for me. Should I add that too? I > mean: > > #elif CONFIG_IS_ENABLED(HANDOFF) > if (!(gd->flags & GD_FLG_RELOC)) { > if (!(gd->spl_handoff)) > init_clocks = true; > } > #endif > > Also, HANDOFF depends on BLOBLIST (maybe it shouldn't?), so I added > these values from chromebook_coral to my config, are they OK? > > CONFIG_BLOBLIST=y > CONFIG_BLOBLIST_SIZE=0x30000 > CONFIG_BLOBLIST_ADDR=0x100000
Yes that's right. The handoff struct is stored in the blblist. It probably only needs to be small though. Regards, Simon
