On Thu, Dec 12, 2024 at 08:38:32PM -0700, Simon Glass wrote:
Hi Tom,
On Thu, 12 Dec 2024 at 10:29, Ilias Apalodimas ilias.apalodimas@linaro.org wrote:
On Thu, 12 Dec 2024 at 19:27, Tom Rini trini@konsulko.com wrote:
On Thu, Dec 12, 2024 at 07:08:34PM +0200, Ilias Apalodimas wrote:
On Thu, 12 Dec 2024 at 19:02, Tom Rini trini@konsulko.com wrote:
On Thu, Dec 12, 2024 at 06:34:44PM +0200, Ilias Apalodimas wrote:
Hi Tom,
On Thu, Dec 12, 2024 at 09:11:41AM -0600, Tom Rini wrote: > Introduce an option to control if we expect that a prior stage has > created a bloblist already and thus it is safe to scan the address. We > need to have this be configurable because we do not (and cannot) know if > the address at CONFIG_BLOBLIST_ADDR is in regular DRAM or some special > on-chip memory and so it may or may not be safe to read from this > address this early.
Yes this patch makes instead of making the OF_SEPARATE implicitly take priority
> > Signed-off-by: Tom Rini trini@konsulko.com > --- > Cc: Simon Glass sjg@chromium.org > --- > common/Kconfig | 32 ++++++++++++++++++++++++++++++++ > lib/fdtdec.c | 11 +++-------- > 2 files changed, 35 insertions(+), 8 deletions(-) > > diff --git a/common/Kconfig b/common/Kconfig > index e8d89bf6eb9d..e8febad0f212 100644 > --- a/common/Kconfig > +++ b/common/Kconfig > @@ -1077,6 +1077,14 @@ config BLOBLIST_SIZE > is set up in the first part of U-Boot to run (TPL, SPL or U-Boot > proper), and this sane bloblist is used for subsequent phases. > > +config BLOBLIST_PRIOR_STAGE > + bool "Bloblist was created in a stage prior to U-Boot" > + default y > + depends on BLOBLIST_FIXED > + help > + Select this if the bloblist at CONFIG_BLOBLIST_ADDR was created > + before U-Boot is started. > + > config BLOBLIST_SIZE_RELOC > hex "Size of bloblist after relocation" > default BLOBLIST_SIZE if BLOBLIST_FIXED || BLOBLIST_ALLOC > @@ -1117,6 +1125,14 @@ config SPL_BLOBLIST_ALLOC > > endchoice > > +config SPL_BLOBLIST_PRIOR_STAGE > + bool "Bloblist was created in a stage prior to SPL" > + default y > + depends on SPL_BLOBLIST_FIXED > + help > + Select this if the bloblist at CONFIG_BLOBLIST_ADDR was created before > + U-Boot SPL is started. > + > endif # SPL_BLOBLIST > > if TPL_BLOBLIST > @@ -1146,6 +1162,14 @@ config TPL_BLOBLIST_ALLOC > > endchoice > > +config TPL_BLOBLIST_PRIOR_STAGE > + bool "Bloblist was created in a stage prior to TPL" > + default y > + depends on TPL_BLOBLIST_FIXED > + help > + Select this if the bloblist at CONFIG_BLOBLIST_ADDR was created before > + U-Boot TPL is started. > + > endif # TPL_BLOBLIST > > if VPL_BLOBLIST > @@ -1175,6 +1199,14 @@ config VPL_BLOBLIST_ALLOC > > endchoice > > +config VPL_BLOBLIST_PRIOR_STAGE > + bool "Bloblist was created in a stage prior to VPL" > + default y > + depends on VPL_BLOBLIST_FIXED > + help > + Select this if the bloblist at CONFIG_BLOBLIST_ADDR was created before > + U-Boot VPL is started. > + > endif # VPL_BLOBLIST > > endmenu > diff --git a/lib/fdtdec.c b/lib/fdtdec.c > index b06559880296..29bddab4150c 100644 > --- a/lib/fdtdec.c > +++ b/lib/fdtdec.c > @@ -1669,15 +1669,10 @@ int fdtdec_setup(void) > int ret = -ENOENT; > > /* > - * If allowing a bloblist, check that first. There was discussion about > - * adding an OF_BLOBLIST Kconfig, but this was rejected. > - * > - * The necessary test is whether the previous phase passed a bloblist, > - * not whether this phase creates one. > + * Only scan for a bloblist and then if that bloblist contains a device > + * tree if we have been configured to expect one. > */ > - if (CONFIG_IS_ENABLED(BLOBLIST) && > - (xpl_prev_phase() != PHASE_TPL || > - !IS_ENABLED(CONFIG_TPL_BLOBLIST))) { > + if (CONFIG_IS_ENABLED(BLOBLIST_PRIOR_STAGE)) {
Without wider context I am not sure but if that check can fold in bloblist_maybe_init() and return -X if the config isn't enabled, it's going to be easier to read
Hmm. We'd have: if (CONFIG_IS_ENABLED(BLOBLIST_PRIOR_STAGE) && !(ret = bloblist_maybe_init()) { /* We have a bloblist, see if it has FDT */
I mean move the check inside bloblist_maybe_init()
As we talked about on IRC, no, because the "maybe" in that function means "find or create". So the expected usage here is that we have platforms that create the bloblist in SPL (post DRAM init) and use that with the HANDOFF mechanism to pass information to full U-Boot.
Yes thanks Reviewed-by: Ilias Apalodimas ilias.apalodimas@linaro.org
I am struggling to see how this is different from the patch I sent a year ago
https://patchwork.ozlabs.org/project/uboot/patch/20231226094625.221671-1-sjg...
Is it just a rebase and a rename?
Well, no. I came up with this after debugging the failure on Bob / Kevin via Gitlab. This in fact allows for the device tree to NOT come from the bloblist. The biggest problem with the "OF_BLOBLIST" thread from last year is that your explanation of the problem doesn't match the problem you have.
What's really tragically funny now is that Jonas' add TPL to Bob/Kevin and init DRAM there means that we don't need this series for that platform and only Coral is still doing the likely ill-advised thing of saying the bloblist exists in DRAM without ensuring that DRAM will have been initialized already.