[U-Boot] [PATCH v2] dm: remove pre reloc properties in SPL and TPL device tree
Simon Glass
sjg at chromium.org
Sun Apr 21 19:42:54 UTC 2019
Hi Patrick,
On Mon, 8 Apr 2019 at 08:18, Patrick Delaunay <patrick.delaunay at st.com> wrote:
>
> We can remove the pre reloc property in SPL and TPL device-tree:
pre-reloc
> - 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 am not 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.
>
>
> Changes in v2:
> - update documentation to describe how the pre-relocation properties
> are used.
>
> doc/README.SPL | 16 ++++++++++++++++
> doc/README.TPL | 4 ++++
> doc/driver-model/README.txt | 4 ++++
> drivers/core/ofnode.c | 16 ++++++++--------
> drivers/core/util.c | 31 +++++++++++++++----------------
> scripts/Makefile.lib | 1 +
> 6 files changed, 48 insertions(+), 24 deletions(-)
Reviewed-by: Simon Glass <sjg at chromium.org>
This is a nice change, but please see suggestions below.
Also do you think it would be possible to add a little test for this,
something like test_ofplatdata? It could use a sample DT (like
test.dts) and check that u-boot-spt.dtb ends up without the properties
you are removing
>
> diff --git a/doc/README.SPL b/doc/README.SPL
> index 7a30fef..6eed83f 100644
> --- a/doc/README.SPL
> +++ b/doc/README.SPL
> @@ -66,6 +66,22 @@ CONFIG_SPL_SPI_LOAD (drivers/mtd/spi/spi_spl_load.o)
> CONFIG_SPL_RAM_DEVICE (common/spl/spl.c)
> CONFIG_SPL_WATCHDOG_SUPPORT (drivers/watchdog/libwatchdog.o)
>
> +Device tree
> +-----------
> +The U-Boot device tree is filtered by the fdtgrep tools during the build
> +process to generate a much smaller device tree used in SPL (spl/u-boot-spl.dtb)
> +with:
> +- the mandatory nodes (/alias, /chosen, /config)
> +- the nodes with one pre-relocation property:
> + 'u-boot,dm-pre-reloc' or 'u-boot,dm-spl'
> +
> +ftgrep is also used to remove:
> +- the properties defined in CONFIG_OF_SPL_REMOVE_PROPS
> +- all the pre-relocation properties
> + ('u-boot,dm-pre-reloc', 'u-boot,dm-spl' and 'u-boot,dm-tpl')
> +
> +All the nodes remaining in the SPL devicetree are bound
> +(see driver-model/README.txt).
>
> Debugging
> ---------
> diff --git a/doc/README.TPL b/doc/README.TPL
> index 980debe..c94129f 100644
> --- a/doc/README.TPL
> +++ b/doc/README.TPL
> @@ -34,6 +34,10 @@ determine which SPL options to choose based on whether CONFIG_TPL_BUILD
> is set. Source files can be compiled for TPL with options choosed in the
> board config file.
>
> +TPL use a small device tree (u-boot-tpl.dtb), containing only the nodes with
> +the pre-relocation properties: 'u-boot,dm-pre-reloc' and 'u-boot,dm-tpl'
> +(see README.SPL for details).
> +
> For example:
>
> spl/Makefile:
> diff --git a/doc/driver-model/README.txt b/doc/driver-model/README.txt
> index 07b120d..532a771 100644
> --- a/doc/driver-model/README.txt
> +++ b/doc/driver-model/README.txt
> @@ -849,6 +849,10 @@ in the device tree node. For U-Boot proper you can use 'u-boot,dm-pre-proper'
> which means that it will be processed (and a driver bound) in U-Boot proper
> prior to relocation, but will not be available in SPL or TPL.
>
> +To reduce the size of SPL and TPL, only the nodes with pre-relocation properties
> +('u-boot,dm-pre-reloc', 'u-boot,dm-spl' or 'u-boot,dm-tpl') are keept in their
> +device trees (see README.SPL for details); the remaining nodes are always bound.
> +
> Then post relocation we throw that away and re-init driver model again.
> For drivers which require some sort of continuity between pre- and
> post-relocation devices, we can provide access to the pre-relocation
> 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)
Can you use if IS_ENABLED(CONFIG_SPL_BUILD) ... instead?
Also below?
> + /* 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)
> {
> +#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 70de9bb..de67677 100644
> --- a/scripts/Makefile.lib
> +++ b/scripts/Makefile.lib
> @@ -525,4 +525,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
>
Regards,
Simon
More information about the U-Boot
mailing list