Re: [PATCH 1/2] bloblist: Introduce BLOBLIST_PRIOR_STAGE options

[Ilias Apalodimas <ilias.apalodimas@linaro.org>]

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

Thanks
/Ilias
>  		ret = bloblist_maybe_init();
>  		if (!ret) {
>  			gd->fdt_blob = bloblist_find(BLOBLISTT_CONTROL_FDT, 0);
> --
> 2.43.0
>