diff mbox series

am65x build issues

Message ID 04446bbd-6858-b8ba-0328-ba7c10291d68@web.de
State New
Headers show
Series am65x build issues | expand

Commit Message

Jan Kiszka April 30, 2020, 7:03 p.m. UTC 
Hi all,

I've noticed that building am65x_evm_a53_defconfig causes the dtbs to be
built twice, once for the main u-boot and once for the spl. This is
because of an extra dependency in mach-k3/config_secure.mk added by
508369672ca3. Why should the produced dtbs depend on the target that
produces them? Why not simply this?



Disclaimer: I didn't actually test the HS path, just the unsigned build.

But also with this change, make -j remains broken for this target:

[...]
  COPY    u-boot.dtb
  MKIMAGE u-boot-dtb.img
  MKIMAGE u-boot.img
  CAT     u-boot-dtb.bin
  COPY    u-boot.bin
  CC      spl/./lib/asm-offsets.s
  CC      spl/./arch/arm/lib/asm-offsets.s
../tools/k3_fit_atf.sh \
spl/dts/k3-am654-base-board.dtb > u-boot-spl-k3.its
  LDS     spl/u-boot-spl.lds
  FDTGREP spl/dts/k3-am654-base-board.dtb
Usage: fdtgrep - extract portions from device tree
[...]

Error: Cannot open output file
make[2]: *** [../scripts/Makefile.spl:465: spl/dts/k3-am654-base-board.dtb] Error 1
make[2]: *** Waiting for unfinished jobs....


Something is fishy with dependencies here. Any ideas?

Thanks,
Jan

Comments

Andrew Davis April 30, 2020, 9:16 p.m. UTC | #1 
On 4/30/20 3:03 PM, Jan Kiszka wrote:
> Hi all,
> 
> I've noticed that building am65x_evm_a53_defconfig causes the dtbs to be
> built twice, once for the main u-boot and once for the spl. This is
> because of an extra dependency in mach-k3/config_secure.mk added by
> 508369672ca3. Why should the produced dtbs depend on the target that
> produces them? Why not simply this?
> 
> diff --git a/arch/arm/mach-k3/config_secure.mk b/arch/arm/mach-k3/config_secure.mk
> index 6d63c57665..cbe9b684fb 100644
> --- a/arch/arm/mach-k3/config_secure.mk
> +++ b/arch/arm/mach-k3/config_secure.mk
> @@ -34,11 +34,8 @@ MKIMAGEFLAGS_u-boot.img_HS = -f auto -A $(ARCH) -T firmware -C none -O u-boot \
>  	-n "U-Boot $(UBOOTRELEASE) for $(BOARD) board" -E \
>  	$(patsubst %,-b arch/$(ARCH)/dts/%.dtb_HS,$(subst ",,$(CONFIG_OF_LIST)))
> 
> -OF_LIST_TARGETS = $(patsubst %,arch/$(ARCH)/dts/%.dtb,$(subst ",,$(CONFIG_OF_LIST)))
> -$(OF_LIST_TARGETS): dtbs
> -
>  u-boot-nodtb.bin_HS: u-boot-nodtb.bin FORCE
>  	$(call if_changed,k3secureimg)
> 
> -u-boot.img_HS: u-boot-nodtb.bin_HS u-boot.img $(patsubst %.dtb,%.dtb_HS,$(OF_LIST_TARGETS)) FORCE
> +u-boot.img_HS: u-boot-nodtb.bin_HS u-boot.img dtbs FORCE
>  	$(call if_changed,mkimage)
> 


Because then no one will depend on the _HS versions of the .dtb files,
so they wont get built.

I agree it's a huge mess right now, the "correct" thing to do would be
for the %.dtb_HS files to depend on their corresponding %.dtb files from
which they are generated. Which they do, kinda. But Make doesn't seem
smart enough to know that at the start, it checks for the .dtb files and
fails instantly during the first round of parsing, due to the .dtb files
not existing and no rules existing for them. The .dtb files are not
being generated by a rule with their name so Make doesn't understand
they will be generated later, because U-Boot uses scripting for dtbs and
uses target "dtbs". So we are stuck depending on that until someone does
some major rework of the U-Boot/Linux makefiles.. Feel free :D

Andrew


> 
> Disclaimer: I didn't actually test the HS path, just the unsigned build.
> 
> But also with this change, make -j remains broken for this target:
> 
> [...]
>   COPY    u-boot.dtb
>   MKIMAGE u-boot-dtb.img
>   MKIMAGE u-boot.img
>   CAT     u-boot-dtb.bin
>   COPY    u-boot.bin
>   CC      spl/./lib/asm-offsets.s
>   CC      spl/./arch/arm/lib/asm-offsets.s
> ../tools/k3_fit_atf.sh \
> spl/dts/k3-am654-base-board.dtb > u-boot-spl-k3.its
>   LDS     spl/u-boot-spl.lds
>   FDTGREP spl/dts/k3-am654-base-board.dtb
> Usage: fdtgrep - extract portions from device tree
> [...]
> 
> Error: Cannot open output file
> make[2]: *** [../scripts/Makefile.spl:465: spl/dts/k3-am654-base-board.dtb] Error 1
> make[2]: *** Waiting for unfinished jobs....
> 
> 
> Something is fishy with dependencies here. Any ideas?
> 
> Thanks,
> Jan
>
Jan Kiszka May 1, 2020, 8:31 a.m. UTC | #2 
On 30.04.20 23:16, Andrew F. Davis wrote:
> On 4/30/20 3:03 PM, Jan Kiszka wrote:
>> Hi all,
>>
>> I've noticed that building am65x_evm_a53_defconfig causes the dtbs to be
>> built twice, once for the main u-boot and once for the spl. This is
>> because of an extra dependency in mach-k3/config_secure.mk added by
>> 508369672ca3. Why should the produced dtbs depend on the target that
>> produces them? Why not simply this?
>>
>> diff --git a/arch/arm/mach-k3/config_secure.mk b/arch/arm/mach-k3/config_secure.mk
>> index 6d63c57665..cbe9b684fb 100644
>> --- a/arch/arm/mach-k3/config_secure.mk
>> +++ b/arch/arm/mach-k3/config_secure.mk
>> @@ -34,11 +34,8 @@ MKIMAGEFLAGS_u-boot.img_HS = -f auto -A $(ARCH) -T firmware -C none -O u-boot \
>>   	-n "U-Boot $(UBOOTRELEASE) for $(BOARD) board" -E \
>>   	$(patsubst %,-b arch/$(ARCH)/dts/%.dtb_HS,$(subst ",,$(CONFIG_OF_LIST)))
>>
>> -OF_LIST_TARGETS = $(patsubst %,arch/$(ARCH)/dts/%.dtb,$(subst ",,$(CONFIG_OF_LIST)))
>> -$(OF_LIST_TARGETS): dtbs
>> -
>>   u-boot-nodtb.bin_HS: u-boot-nodtb.bin FORCE
>>   	$(call if_changed,k3secureimg)
>>
>> -u-boot.img_HS: u-boot-nodtb.bin_HS u-boot.img $(patsubst %.dtb,%.dtb_HS,$(OF_LIST_TARGETS)) FORCE
>> +u-boot.img_HS: u-boot-nodtb.bin_HS u-boot.img dtbs FORCE
>>   	$(call if_changed,mkimage)
>>
>
>
> Because then no one will depend on the _HS versions of the .dtb files,
> so they wont get built.

Indeed, missed that.

>
> I agree it's a huge mess right now, the "correct" thing to do would be
> for the %.dtb_HS files to depend on their corresponding %.dtb files from
> which they are generated. Which they do, kinda. But Make doesn't seem
> smart enough to know that at the start, it checks for the .dtb files and
> fails instantly during the first round of parsing, due to the .dtb files
> not existing and no rules existing for them. The .dtb files are not
> being generated by a rule with their name so Make doesn't understand
> they will be generated later, because U-Boot uses scripting for dtbs and
> uses target "dtbs". So we are stuck depending on that until someone does
> some major rework of the U-Boot/Linux makefiles.. Feel free :D

loop_cmd = $(echo-cmd) $(cmd_$(1)) || exit;

dtbs_HS: dtbs FORCE
	$(foreach dtb, ..., $(call, loop_cmd, ...))

This avoids per-dtb rules and dependencies. But it will require some
variant of k3secureimg that takes the source as a parameter and not from
the rule's dependency list. This is the same pattern I currently play
with for injecting the public key into dtbs for fit image verification.
I'll see if I can get something building with am65x_hs_evm_a53_defconfig
to validate a concrete fix proposal.

Again my question: What can be the other broken dependency that causes
"make -j" failures?

Jan
diff mbox series

Patch

diff --git a/arch/arm/mach-k3/config_secure.mk b/arch/arm/mach-k3/config_secure.mk
index 6d63c57665..cbe9b684fb 100644
--- a/arch/arm/mach-k3/config_secure.mk
+++ b/arch/arm/mach-k3/config_secure.mk
@@ -34,11 +34,8 @@  MKIMAGEFLAGS_u-boot.img_HS = -f auto -A $(ARCH) -T firmware -C none -O u-boot \
 	-n "U-Boot $(UBOOTRELEASE) for $(BOARD) board" -E \
 	$(patsubst %,-b arch/$(ARCH)/dts/%.dtb_HS,$(subst ",,$(CONFIG_OF_LIST)))

-OF_LIST_TARGETS = $(patsubst %,arch/$(ARCH)/dts/%.dtb,$(subst ",,$(CONFIG_OF_LIST)))
-$(OF_LIST_TARGETS): dtbs
-
 u-boot-nodtb.bin_HS: u-boot-nodtb.bin FORCE
 	$(call if_changed,k3secureimg)

-u-boot.img_HS: u-boot-nodtb.bin_HS u-boot.img $(patsubst %.dtb,%.dtb_HS,$(OF_LIST_TARGETS)) FORCE
+u-boot.img_HS: u-boot-nodtb.bin_HS u-boot.img dtbs FORCE
 	$(call if_changed,mkimage)