Hello,
On 06.03.25 11:18, Quentin Schulz wrote:
Hi Hendrik,
On 3/5/25 7:35 PM, Hendrik Donner wrote:
[You don't often get email from h...@os-cillation.de. Learn why this is
important at https://aka.ms/LearnAboutSenderIdentification ]
The CONFIG_*PL_BUILD defines are currently not defined when
preprocessing the dts files, leading to build problems with binman. Set
the defines based on the related CONFIG_*PL values.
Tested-by: Oliver Graute <oliver.gra...@kococonnector.com>
Please have this person publicly answer they tested it instead of having
it already in the v1 of the patch, too easy to impersonate someone and
give a false sense of trust into a patch that may not actually have been
tested.
Signed-off-by: Hendrik Donner <h...@os-cillation.de>
---
scripts/Makefile.lib | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
index 54403040f00..dd2c6363224 100644
--- a/scripts/Makefile.lib
+++ b/scripts/Makefile.lib
@@ -217,6 +217,16 @@ dtc_cpp_flags = -Wp,-MD,$(depfile).pre.tmp -
nostdinc \
-
D__ASSEMBLY__ \
-undef -D__DTS__
+ifeq ($(CONFIG_SPL),y)
+dtc_cpp_flags += -DCONFIG_XPL_BUILD -DCONFIG_SPL_BUILD
+endif
+ifeq ($(CONFIG_TPL),y)
+dtc_cpp_flags += -DCONFIG_XPL_BUILD -DCONFIG_TPL_BUILD
+endif
+ifeq ($(CONFIG_VPL),y)
+dtc_cpp_flags += -DCONFIG_XPL_BUILD -DCONFIG_VPL_BUILD
+endif
+
You don't actually explain what you're trying to fix here and why this
fixes it?
Why would you need those symbols for the DT?
well they are already in use in tree ([1], [2]), which breaks the build.
The history of both files is that the guards were introduced as
CONFIG_SPL, which builds, to CONFIG_SPL_BUILD, which should break the
build, haven't tested, and somewhat recently to CONFIG_XPL_BUILD,
which is when i noticed that it breaks the build for me.
Now the guards are around SPL specific nodes, so i didn't question it.
My understanding was that those are needed not necessarily during the
SPL phase, but only if SPL is defined at all.
If i understand you correctly none of it should have been introduced
like that in the first place?
I noticed it's broken in the uboot CI run too, but since the boards
need firmware blobs the CI build is erroring out in the first place, so
i guess no one noticed.
For removing nodes in xPL, you can use one of the bootph- properties.
The DT is supposed to represent the HW, so having a different DT between
stages is very likely wrong. Note that having a subset of the full DT in
xPL stages is a bit of an exception here, for size purposes (usually
because of limited SRAM) or boot time purposes (you don't need to enable
everything in the DT in the first stage after BootROM simply to init
UART and DRAM :) ).
As for binman, why would it even need to be run during the xPL build
stages? Can you provide more context on this?
We have some big nasty ifdefery in arch/arm/dts/rockchip-u-boot.dtsi for
example, which has some logic on whether some symbols are defined, but
they are defined regardless of the stage and I assume binman gets run
only once, and not in xPL build phase? Is this something not applicable
for (I assume that's the user of that) imx8?
The arch/arm/dts/rockchip-u-boot.dtsi ifdefery is based directly on
CONFIG_SPL, not CONFIG_SPL_BUILD nor CONFIG_XPL_BUILD, so maybe the 2
dtsi i mentioned should have kept using CONFIG_SPL in the first place.
So move those back to using CONFIG_SPL?
Regards,
Hendrik
[1]
https://source.denx.de/u-boot/u-boot/-/blob/master/arch/arm/dts/imx8qm-u-boot.dtsi#L13
[2]
https://source.denx.de/u-boot/u-boot/-/blob/master/arch/arm/dts/imx8qxp-u-boot.dtsi#L13
Cheers,
Quentin
