[PATCH] scripts/Makefile.lib: do not include CONFIG_DEVICE_TREE_INCLUDES in dtsi_include_list

[PATCH] scripts/Makefile.lib: do not include CONFIG_DEVICE_TREE_INCLUDES in dtsi_include_list

[Rasmus Villemoes]

The commit mentioned in Fixes broke the
CONFIG_DEVICE_TREE_INCLUDES feature, with the result that any board
setting any non-empty value for that fails to build.

The parent of the mentioned commit refactoring a bit by introducing
the dtsi_include_list variable and changing cmd_dtc to loop over that
was fine.

However, the .dtsi files mentioned in CONFIG_DEVICE_TREE_INCLUDES are
not supposed to be generated via the build system. They are meant for
e.g. including a public key for verified boot (generated with the
key2dtsi script), or for injecting some stuff to the /config
node (say, a bootcmd or a load-environment setting or things like
that). The files can either live in-tree in a private branch or
completely outside, e.g. in some Yocto metadata.

But regardless, U-Boot's build system will never know anything about
them, so when the mentioned commit did

dtsi_include_list_deps = $(addprefix $(obj)/,$(subst $(quote),,$(dtsi_include_list)))

things broke, because if CONFIG_DEVICE_TREE_INCLUDES is for example
"/path/to/public_key.dtsi", this would add a dependency on
$(obj)//path/to/public_key.dtsi to each $(obj)/*.dtb target, yielding

make[3]: *** No rule to make target 'arch/arm/dts/imx6dl-aristainetos2c_7.dtb', needed by 'dtbs'.  Stop.

To fix that while preserving the introduced
CONFIG_EFI_CAPSULE_ESL_FILE behaviour, disentangle
CONFIG_DEVICE_TREE_INCLUDES from dtsi_include_list from which
dtsi_include_list_deps is built, and instead just add the items
directly to the $(foreach) loop.

Fixes: a958988b62 ("scripts/Makefile.lib: Add dtsi include files as deps for building DTB")
Signed-off-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
---

Aside: The later change $(obj) -> $(u_boot_dtsi_loc) (commit df6fb77c98aa)
makes no difference wrt. this bug, but I do think it used to be wrong
to have the -u-boot.dtsi file in dtsi_include_list_deps with an
$(obj)/ prefix - but probably our VPATH setting meant that it didn't
actually break.

 scripts/Makefile.lib | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
index df754d1d9f0..5dfb4b6e46f 100644
--- a/scripts/Makefile.lib
+++ b/scripts/Makefile.lib
@@ -185,9 +185,6 @@ endif
 dtsi_include_list = $(strip $(u_boot_dtsi_options_debug) \
 	$(notdir $(firstword $(u_boot_dtsi_options))))
 
-# The CONFIG_DEVICE_TREE_INCLUDES also need to be included
-dtsi_include_list += $(CONFIG_DEVICE_TREE_INCLUDES)
-
 # Modified for U-Boot
 upstream_dtsi_include = $(addprefix -I, $(srctree)/dts/upstream/src/ \
 	$(sort $(dir $(wildcard $(srctree)/dts/upstream/src/$(ARCH)/*/*))) \
@@ -350,7 +347,7 @@ quiet_cmd_dtc = DTC     $@
 # And finally any custom .dtsi fragments specified with CONFIG_DEVICE_TREE_INCLUDES
 cmd_dtc = mkdir -p $(dir ${dtc-tmp}) ; \
 	(cat $< > $(pre-tmp)); \
-	$(foreach f,$(subst $(quote),,$(dtsi_include_list)), \
+	$(foreach f,$(subst $(quote),,$(dtsi_include_list) $(CONFIG_DEVICE_TREE_INCLUDES)), \
 	  echo '$(pound)include "$(f)"' >> $(pre-tmp);) \
 	$(HOSTCC) -E $(dtc_cpp_flags) -I$(obj) -x assembler-with-cpp -o $(dtc-tmp) $(pre-tmp) ; \
 	$(DTC) -O dtb -o $@ -b 0 \
-- 
2.46.0

Re: [PATCH] scripts/Makefile.lib: do not include CONFIG_DEVICE_TREE_INCLUDES in dtsi_include_list

[Emil Kronborg]

On Tue, Sep 03, 2024 at 23:59 GMT, Rasmus Villemoes wrote:
>  scripts/Makefile.lib | 5 +----
>  1 file changed, 1 insertion(+), 4 deletions(-)
> 
> diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
> index df754d1d9f0..5dfb4b6e46f 100644
> --- a/scripts/Makefile.lib
> +++ b/scripts/Makefile.lib
> @@ -185,9 +185,6 @@ endif
>  dtsi_include_list = $(strip $(u_boot_dtsi_options_debug) \
>  	$(notdir $(firstword $(u_boot_dtsi_options))))
> 
> -# The CONFIG_DEVICE_TREE_INCLUDES also need to be included
> -dtsi_include_list += $(CONFIG_DEVICE_TREE_INCLUDES)
> -
>  # Modified for U-Boot
>  upstream_dtsi_include = $(addprefix -I, $(srctree)/dts/upstream/src/ \
>  	$(sort $(dir $(wildcard $(srctree)/dts/upstream/src/$(ARCH)/*/*))) \
> @@ -350,7 +347,7 @@ quiet_cmd_dtc = DTC     $@
>  # And finally any custom .dtsi fragments specified with CONFIG_DEVICE_TREE_INCLUDES
>  cmd_dtc = mkdir -p $(dir ${dtc-tmp}) ; \
>  	(cat $< > $(pre-tmp)); \
> -	$(foreach f,$(subst $(quote),,$(dtsi_include_list)), \
> +	$(foreach f,$(subst $(quote),,$(dtsi_include_list) $(CONFIG_DEVICE_TREE_INCLUDES)), \
>  	  echo '$(pound)include "$(f)"' >> $(pre-tmp);) \
>  	$(HOSTCC) -E $(dtc_cpp_flags) -I$(obj) -x assembler-with-cpp -o $(dtc-tmp) $(pre-tmp) ; \
>  	$(DTC) -O dtb -o $@ -b 0 \
> --
> 2.46.0

Tested-by: Emil Kronborg <emil.kronborg@protonmail.com>

-- 
Emil Kronborg

Re: [PATCH] scripts/Makefile.lib: do not include CONFIG_DEVICE_TREE_INCLUDES in dtsi_include_list

[Rasmus Villemoes]

Rasmus Villemoes <rasmus.villemoes@prevas.dk> writes:

> The commit mentioned in Fixes broke the
> CONFIG_DEVICE_TREE_INCLUDES feature, with the result that any board
> setting any non-empty value for that fails to build.
>

FWIW, it's quite easy to reproduce, though I don't know if it makes
sense to pollute, say, sandbox_defconfig with this to keep it from
regressing. Anyway, just add include/h2g2.dtsi containing

/ {
        config {
                question = "Life, the Universe and Everything";
                answer = <42>;
        };
};

and add "h2g2.dtsi" to CONFIG_DEVICE_TREE_INCLUDES. Or put the file in
/tmp and add the absolute path "/tmp/h2g2.dtsi" to
CONFIG_DEVICE_TREE_INCLUDES.

I know we're close to release date, but I'd really appreciate if this
could make it into 2024.10. Yes, it's my fault for not noticing and
reporting sooner; the timing just happened to mean I had several
customers end up using 2023.10 (making use of
CONFIG_DEVICE_TREE_INCLUDES), the breakage happened in 2024.01, and I
hadn't had occasion to use this with newer u-boots since, until just
recently.

Rasmus

Re: [PATCH] scripts/Makefile.lib: do not include CONFIG_DEVICE_TREE_INCLUDES in dtsi_include_list

[Tom Rini]

On Tue, 03 Sep 2024 23:59:36 +0200, Rasmus Villemoes wrote:

> The commit mentioned in Fixes broke the
> CONFIG_DEVICE_TREE_INCLUDES feature, with the result that any board
> setting any non-empty value for that fails to build.
> 
> The parent of the mentioned commit refactoring a bit by introducing
> the dtsi_include_list variable and changing cmd_dtc to loop over that
> was fine.
> 
> [...]

Applied to u-boot/master, thanks!

-- 
Tom