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

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@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(-)

Hi Jean-Jacques,

On 5 October 2018 at 10:45, Jean-Jacques Hiblot <jjhiblot@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@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