Hi Patrick,
I'm having issues with some of the recent changes centered around FIP
support and CONFIG_STM32MP15x_STM32IMAGE. and commit f91783edf224 ("arm:
stm32mp: handle the OP-TEE nodes in DT with FIP support")
## Problem description
> +#ifdef CONFIG_STM32MP15x_STM32IMAGE
> + /* only needed for boot with TF-A, witout FIP support */
> firmware {
> optee {
> compatible = "linaro,optee-tz";
> @@ -33,6 +35,7 @@
> no-map;
> };
> };
> +#endif
This removes the "optee" and "reserved-memory" nodes. These nodes are
required by SPL for setting up TZC memory regions, as introduced in
commit 8533263c8512 ("stm32mp1: spl: Configure TrustZone controller for
OP-TEE").
## Further details
We now have several boot flows:
1) BL1 -> SPL -> u-boot
2) BL1 -> SPL -> OP-TEE (this path is now broken)
3) BL1 -> TF-A -> u-boot (use case for STM32IMAGE)
4) BL1 -> TF-A -> OP-TEE (use case for STM32IMAGE)
5) BL1 -> TF-A -> FIP container
So it seems that STM32IMAGE only makes sense for (3) and (4), but
shouldn't affect the others. The fact that OP-TEE is mixed into this is
the first red flag.
> INPUTS-$(CONFIG_STM32MP15x_STM32IMAGE) += u-boot.stm32
> MKIMAGEFLAGS_u-boot.stm32 = -T stm32image ...
This tells me we use _STM32IMAGE just to enable another output image
format. The build could give me both u-boot.img, and u-boot.stm32, and I
would use the correct file. SO in this case, I would expect _STM32IMAGE
to not change the code behavior. This is the second red flag.
> $ git grep -c 'ifdef CONFIG_STM32MP15x_STM32IMAGE'
> arch/arm/mach-stm32mp/cmd_stm32prog/cmd_stm32prog.c:1
> arch/arm/mach-stm32mp/cmd_stm32prog/stm32prog.c:2
> arch/arm/mach-stm32mp/cmd_stm32prog/stm32prog.h:1
> arch/arm/mach-stm32mp/include/mach/stm32prog.h:1
We're actually trying to move away from ifdefs, so this intense reliance
on _STM32IMAGE raises the third red flag.
> board/st/common/stm32mp_mtdparts.c:9
I'm not sure I fully understand why there are so many ifdefs in
mtdparts.c. MTD layout seems to me like something that is intended to be
devicetree-driven. This is the fourth red flag.
Let's take a deeper look at one of those:
> stm32mp_mtdparts.c:#ifdef CONFIG_STM32MP15x_STM32IMAGE
> stm32mp_mtdparts.c: tee = >
stm32prog_get_tee_partitions();
> stm32mp_mtdparts.c-#endif
This makes no sense to me. What does OP-TEE presence have to do with the
image format of u-boot? If TF-A requires a different layout in FIP vs
non-FIP mode, then it's the responsibility of TF-A to supply a corrected
devicetree to u-boot. It's not u-boot's role.
## Proposal
I propose we remove CONFIG_STM32MP15x_STM32IMAGE. Always build
u-boot.stm32, and don't mix it with code behavior.
Alex
