[U-Boot] [PATCH v2 16/19] lib: fdtdec: Add function re-setup the fdt more effeciently

[Simon Glass]

Hi Jean-Jacques,

On 5 October 2018 at 10:45, Jean-Jacques Hiblot <jjhiblot at ti.com> wrote:
> In some cases it may be useful to be able to change the fdt we have been
> using and use another one instead. For example, the TI platforms uses an
> EEPROM to store board information and, based on the type of board,
> different dtbs are used by the SPL. When DM_I2C is used, a first dtb must
> be used before the I2C is initialized and only then the final dtb can be
> selected.
> To speed up the process and reduce memory usage, introduce a new function
> fdtdec_setup_best_match() that re-use the DTBs loaded in memory by
> fdtdec_setup() to select the best match.
>
> Signed-off-by: Jean-Jacques Hiblot <jjhiblot at ti.com>
> ---
>
> Changes in v2: None
>
>  include/asm-generic/global_data.h |  4 ++++
>  include/fdtdec.h                  | 17 +++++++++++++++++
>  lib/fdtdec.c                      | 38 +++++++++++++++++++++++++++++++++++++-
>  3 files changed, 58 insertions(+), 1 deletion(-)
>
> diff --git a/include/asm-generic/global_data.h b/include/asm-generic/global_data.h
> index c83fc01..49287fa 100644
> --- a/include/asm-generic/global_data.h
> +++ b/include/asm-generic/global_data.h
> @@ -77,6 +77,10 @@ typedef struct global_data {
>  #ifdef CONFIG_OF_LIVE
>         struct device_node *of_root;
>  #endif
> +
> +#if CONFIG_IS_ENABLED(MULTI_DTB_FIT)
> +       const void *multi_dtb_fit;

comment please

> +#endif
>         struct jt_funcs *jt;            /* jump table */
>         char env_buf[32];               /* buffer for env_get() before reloc. */
>  #ifdef CONFIG_TRACE
> diff --git a/include/fdtdec.h b/include/fdtdec.h
> index 83be064..2cb3da2 100644
> --- a/include/fdtdec.h
> +++ b/include/fdtdec.h
> @@ -996,6 +996,23 @@ int fdtdec_setup_memory_banksize(void);
>   */
>  int fdtdec_setup(void);
>
> +#if CONFIG_IS_ENABLED(MULTI_DTB_FIT)
> +/**
> + * fdtdec_resetup()  - Set up the device tree ready for use again.

Please remove .

> + *
> + * The main difference with fdtdec_setup() is that it returns if the fdt has
> + * changed because a better match has been found.
> + * This is typically used for boards that rely on a DM driver to detect the
> + * board type.
> + *
> + * @param rescan Return a flag indicating that fdt has changed and rescaning the
> + *               fdt is required.

This should be bool I think.

also 'rescanning'

Who actually calls this function and when is it permitted to call it?
I think then needs a bit more info in the comment.

Also I think you should mention it in the docs, perhaps README.fdt-control

Finally this calls either fdtdec_setup() or fdtdec_prepare() but I am
not not why it calls one or the other. Please add more docs/comments.

> + *
> + * @return 0 if OK, -ve on error
> + */
> +int fdtdec_resetup(int *rescan);
> +#endif
> +
>  /**
>   * 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
> diff --git a/lib/fdtdec.c b/lib/fdtdec.c
> index 74196ce..3b80bd4 100644
> --- a/lib/fdtdec.c
> +++ b/lib/fdtdec.c
> @@ -1353,14 +1353,50 @@ int fdtdec_setup(void)
>          * If so, pick the most relevant
>          */
>         fdt_blob = locate_dtb_in_fit(gd->fdt_blob);
> -       if (fdt_blob)
> +       if (fdt_blob) {
> +               gd->multi_dtb_fit = gd->fdt_blob;
>                 gd->fdt_blob = fdt_blob;
> +       }
> +
>  # endif
>  #endif
>
>         return fdtdec_prepare_fdt();
>  }
>
> +#if CONFIG_IS_ENABLED(MULTI_DTB_FIT)
> +int fdtdec_resetup(int *rescan)
> +{
> +       void *fdt_blob;
> +
> +       /*
> +        * If the current DTB is part of a compressed FIT image,
> +        * try to locate the best match from the uncompressed
> +        * FIT image stillpresent there. Save the time and space

is still present ? This doesn't make sense.

> +        * required to uncompress it again.
> +        */
> +       if (gd->multi_dtb_fit) {
> +               fdt_blob = locate_dtb_in_fit(gd->multi_dtb_fit);
> +
> +               if (fdt_blob == gd->fdt_blob) {
> +                       /*
> +                        * The best match did not change. no need to tear down
> +                        * the DM and rescan the fdt.
> +                        */
> +                       *rescan = 0;
> +                       return 0;
> +               }
> +
> +               *rescan = 1;
> +               gd->fdt_blob = fdt_blob;
> +               return fdtdec_prepare_fdt();
> +       }
> +
> +       *rescan = 1;
> +       return fdtdec_setup();
> +}
> +#endif
> +
>  #ifdef CONFIG_NR_DRAM_BANKS
>  int fdtdec_decode_ram_size(const void *blob, const char *area, int board_id,
>                            phys_addr_t *basep, phys_size_t *sizep, bd_t *bd)
> --
> 2.7.4
>

Regards,
Simon