Re: [PATCH v5 26/26] fdt: Don't call board_fdt_blob_setup() without OF_BOARD

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

Hi Simon,

As I said here [1], this is moving on an entirely different direction I had
in mind. I'd much prefer starting the discussions for a solution that
allows us to scale. 
FWIW I think the current code is still not clean for my taste.  Commit
3b595da441cf ("fdtdec: allow board to provide fdt for CONFIG_OF_SEPARATE")
allowed this function to be used regardless of the config options.  IMHO we
should have 2 clear options:
- U-Boot provides the DTB 
- It's somehow passed over to U-Boot 

On Mon, Oct 25, 2021 at 06:23:44PM -0600, Simon Glass wrote:
> At present this override function is called even when OF_BOARD Is not
> enabled. This makes it impossible to disable this feature and in fact
> makes the OF_BOARD option useless.
> 
> Reinstate its intended purpose, so that it is possible to switch between
> the appended devicetree and one provided by the board's custom function.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
> 
> Changes in v5:
> - Add new patches to clean up fdtdec_setup() and surrounds
> 
>  include/fdtdec.h |  7 +++++--
>  lib/fdtdec.c     | 17 +++++++++++------
>  2 files changed, 16 insertions(+), 8 deletions(-)
> 
> diff --git a/include/fdtdec.h b/include/fdtdec.h
> index 386f6611294..b2faa84008e 100644
> --- a/include/fdtdec.h
> +++ b/include/fdtdec.h
> @@ -1170,8 +1170,11 @@ int fdtdec_resetup(int *rescan);
>  
>  /**
>   * Board-specific FDT initialization. Returns the address to a device tree blob.
> - * Called when CONFIG_OF_BOARD is defined, or if CONFIG_OF_SEPARATE is defined
> - * and the board implements it.
> + * Called when CONFIG_OF_BOARD is defined.
> + *
> + * The existing devicetree is available at gd->fdt_blob
> + *
> + * @returns new devicetree blob pointer
>   */
>  void *board_fdt_blob_setup(void);
>  
> diff --git a/lib/fdtdec.c b/lib/fdtdec.c
> index 067c27d0aa3..da36dffec62 100644
> --- a/lib/fdtdec.c
> +++ b/lib/fdtdec.c
> @@ -1203,11 +1203,12 @@ static int uncompress_blob(const void *src, ulong sz_src, void **dstp)
>  	return 0;
>  }
>  
> -/*
> - * For CONFIG_OF_SEPARATE, the board may optionally implement this to
> - * provide and/or fixup the fdt.
> +/**
> + * fdt_find_separate() - Find a devicetree at the end of the image
> + *
> + * @return pointer to FDT blob
>   */
> -__weak void *board_fdt_blob_setup(void)
> +static void *fdt_find_separate(void)
>  {
>  	void *fdt_blob = NULL;
>  #ifdef CONFIG_SPL_BUILD
> @@ -1623,11 +1624,15 @@ int fdtdec_setup(void)
>  	int ret;
>  
>  	/* The devicetree is typically appended to U-Boot */
> -	if (IS_ENABLED(CONFIG_OF_SEPARATE) || IS_ENABLED(CONFIG_OF_BOARD))
> -		gd->fdt_blob = board_fdt_blob_setup();
> +	if (IS_ENABLED(CONFIG_OF_SEPARATE))
> +		gd->fdt_blob = fdt_find_separate();
>  	else /* embed dtb in ELF file for testing / development */
>  		gd->fdt_blob = dtb_dt_embedded();
>  
> +	/* Allow the board to override the fdt address. */
> +	if (IS_ENABLED(CONFIG_OF_BOARD))
> +		gd->fdt_blob = board_fdt_blob_setup();
> +
>  	if (!IS_ENABLED(CONFIG_SPL_BUILD)) {
>  		/* Allow the early environment to override the fdt address */
>  		gd->fdt_blob = map_sysmem(env_get_ulong("fdtcontroladdr", 16,
> -- 
> 2.33.0.1079.g6e70778dc9-goog
> 

[1] https://lore.kernel.org/u-boot/YXekTkeL73NM0UOU@apalos.home/

Regards
/Ilias