All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Alex G." <mr.nuke.me@gmail.com>
To: Patrick DELAUNAY <patrick.delaunay@foss.st.com>,
	"u-boot@lists.denx.de" <u-boot@lists.denx.de>
Cc: Marek Vasut <marex@denx.de>
Subject: Massive stm32mp1 breakage with v2021.10-rc2
Date: Tue, 24 Aug 2021 15:30:03 -0500	[thread overview]
Message-ID: <966fc974-8331-aeac-ba15-5b2ab19c0eaf@gmail.com> (raw)

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



             reply	other threads:[~2021-08-24 20:30 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-24 20:30 Alex G. [this message]
2021-08-26 21:47 ` [RFC PATCH] stm32mp1: Replace STM32IMAGE config with TFABOOT_FIP Alexandru Gagniuc
2021-08-31 14:54   ` Patrick DELAUNAY
2021-08-31 16:42     ` Marek Vasut
2021-09-01  9:07       ` Patrick DELAUNAY
2021-09-03 15:32         ` Marek Vasut
2021-09-03 17:47           ` Alex G.
2021-09-06  6:16             ` Etienne Carriere

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=966fc974-8331-aeac-ba15-5b2ab19c0eaf@gmail.com \
    --to=mr.nuke.me@gmail.com \
    --cc=marex@denx.de \
    --cc=patrick.delaunay@foss.st.com \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.