[U-Boot] [PATCH] dm: remove pre reloc properties in SPL and TPL device tree

[Simon Glass]

Hi Patrick,

On Mon, 11 Feb 2019 at 19:50, Patrick Delaunay <patrick.delaunay at st.com> wrote:
>
> We can remove the pre reloc property in SPL and TPL device-tree:
> - u-boot,dm-pre-reloc
> - u-boot,dm-spl
> - u-boot,dm-tpl
> As only the needed node are kept by fdtgrep (1st pass).
>
> The associated function (XXX_pre_reloc) are simple for SPL/TPL:
> return always true.
>
>
> Signed-off-by: Patrick Delaunay <patrick.delaunay at st.com>
> ---
>
> remove pre reloc properties in SPL and TPL device tree
>
> Patch created after some remarks on previous
> http://patchwork.ozlabs.org/patch/1035797/
>
> I check the current code and I found a way to reduce the SPL device tree
> size: remove the un-necessary pre reloc parameters in SPL/TPL DT
> (as the node check is already managed by fdtgrep)
>
> But I need to change the DM code to avoid the check on presence
> of this parameters...
>
> On my board stm32mp1-ev1, the SPL device tree is reduced by 790 bytes
> (11149 to 10419) and the SPL binary u-boot-spl.stm32 by 798 bytes
> (64745 to 63947); the boot is not broken and I have the expected
> delta between generated device tree.
>
> But I don't sure that I see all the side impact of the code change
> in SPL/TPL.
>
> PS: I already see a side effect on this patch, because all node are now
>     bounded in SPL/TPL, it is no more needed to TAG all the device tree
>     path but only the last needed children.
>     But in this case phandle for parent node are not keep.

Yes that makes sense. I suppose we could enhance fdtgrep to handle this.

>
>
>  drivers/core/ofnode.c | 16 ++++++++--------
>  drivers/core/util.c   | 31 +++++++++++++++----------------
>  scripts/Makefile.lib  |  1 +
>  3 files changed, 24 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/core/ofnode.c b/drivers/core/ofnode.c
> index 0e584c1..5a109dd 100644
> --- a/drivers/core/ofnode.c
> +++ b/drivers/core/ofnode.c
> @@ -700,18 +700,18 @@ int ofnode_read_simple_size_cells(ofnode node)
>
>  bool ofnode_pre_reloc(ofnode node)
>  {
> +#if defined(CONFIG_SPL_BUILD) || defined(CONFIG_TPL_BUILD)
> +       /* for SPL and TPL the remaining nodes after the fdtgrep 1st pass
> +        * had property dm-pre-reloc or u-boot,dm-spl/tpl.
> +        * They are removed in final dtb (fdtgrep 2nd pass)
> +        */
> +       return true;
> +#else
>         if (ofnode_read_bool(node, "u-boot,dm-pre-reloc"))
>                 return true;
>         if (ofnode_read_bool(node, "u-boot,dm-pre-proper"))
>                 return true;
>
> -#ifdef CONFIG_TPL_BUILD
> -       if (ofnode_read_bool(node, "u-boot,dm-tpl"))
> -               return true;
> -#elif defined(CONFIG_SPL_BUILD)
> -       if (ofnode_read_bool(node, "u-boot,dm-spl"))
> -               return true;
> -#else
>         /*
>          * In regular builds individual spl and tpl handling both
>          * count as handled pre-relocation for later second init.
> @@ -719,9 +719,9 @@ bool ofnode_pre_reloc(ofnode node)
>         if (ofnode_read_bool(node, "u-boot,dm-spl") ||
>             ofnode_read_bool(node, "u-boot,dm-tpl"))
>                 return true;
> -#endif
>
>         return false;
> +#endif
>  }
>
>  int ofnode_read_resource(ofnode node, uint index, struct resource *res)
> diff --git a/drivers/core/util.c b/drivers/core/util.c
> index 27a6848..451f772 100644
> --- a/drivers/core/util.c
> +++ b/drivers/core/util.c
> @@ -33,16 +33,15 @@ int list_count_items(struct list_head *head)
>
>  bool dm_fdt_pre_reloc(const void *blob, int offset)

This function should really be removed. We should be able to use the
ofnode version always.

>  {
> +#if defined(CONFIG_SPL_BUILD) || defined(CONFIG_TPL_BUILD)
> +       /* for SPL and TPL the remaining nodes after the fdtgrep 1st pass
> +        * had property dm-pre-reloc or u-boot,dm-spl/tpl.
> +        * They are removed in final dtb (fdtgrep 2nd pass)
> +        */
> +       return true;
> +#else
>         if (fdt_getprop(blob, offset, "u-boot,dm-pre-reloc", NULL))
>                 return true;
> -
> -#ifdef CONFIG_TPL_BUILD
> -       if (fdt_getprop(blob, offset, "u-boot,dm-tpl", NULL))
> -               return true;
> -#elif defined(CONFIG_SPL_BUILD)
> -       if (fdt_getprop(blob, offset, "u-boot,dm-spl", NULL))
> -               return true;
> -#else
>         /*
>          * In regular builds individual spl and tpl handling both
>          * count as handled pre-relocation for later second init.
> @@ -57,16 +56,16 @@ bool dm_fdt_pre_reloc(const void *blob, int offset)
>
>  bool dm_ofnode_pre_reloc(ofnode node)
>  {
> +#if defined(CONFIG_SPL_BUILD) || defined(CONFIG_TPL_BUILD)
> +       /* for SPL and TPL the remaining nodes after the fdtgrep 1st pass
> +        * had property dm-pre-reloc or u-boot,dm-spl/tpl.
> +        * They are removed in final dtb (fdtgrep 2nd pass)
> +        */
> +       return true;
> +#else
>         if (ofnode_read_bool(node, "u-boot,dm-pre-reloc"))
>                 return true;
>
> -#ifdef CONFIG_TPL_BUILD
> -       if (ofnode_read_bool(node, "u-boot,dm-tpl"))
> -               return true;
> -#elif defined(CONFIG_SPL_BUILD)
> -       if (ofnode_read_bool(node, "u-boot,dm-spl"))
> -               return true;
> -#else
>         /*
>          * In regular builds individual spl and tpl handling both
>          * count as handled pre-relocation for later second init.
> @@ -74,7 +73,7 @@ bool dm_ofnode_pre_reloc(ofnode node)
>         if (ofnode_read_bool(node, "u-boot,dm-spl") ||
>             ofnode_read_bool(node, "u-boot,dm-tpl"))
>                 return true;
> -#endif
>
>         return false;
> +#endif
>  }
> diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
> index a5b57fc..3f4c86e 100644
> --- a/scripts/Makefile.lib
> +++ b/scripts/Makefile.lib
> @@ -524,4 +524,5 @@ quiet_cmd_fdtgrep = FDTGREP $@
>        cmd_fdtgrep = $(objtree)/tools/fdtgrep $(fdtgrep_props) -RT $< \
>                 -n /chosen -n /config -O dtb | \
>         $(objtree)/tools/fdtgrep -r -O dtb - -o $@ \
> +               -P u-boot,dm-pre-reloc -P u-boot,dm-spl -P u-boot,dm-tpl \
>                 $(addprefix -P ,$(subst $\",,$(CONFIG_OF_SPL_REMOVE_PROPS)))
> --
> 2.7.4
>

Is there a documentation update you can do to describe how things now work?

Regards,
Simon