[PATCH v2] dm: core: Report bootph-pre-ram/sram node as pre-reloc after relocation : regression

Mon Oct 2 03:16:39 CEST 2023

Hi Massimo,

On Sun, 1 Oct 2023 at 07:29, Massimo Pegorer
<massimo.pegorer+oss at gmail.com> wrote:
>
> Il giorno ven 29 set 2023 alle ore 13:23 Roger Quadros
> <rogerq at kernel.org> ha scritto:
> >
> >
> >
> > On 28/09/2023 22:18, Jonas Karlman wrote:
> > > Hi Roger,
> > >
> > > On 2023-09-28 14:59, Roger Quadros wrote:
> > >> Hi,
> > >>
> > >> On 21/08/2023 01:03, Jonas Karlman wrote:
> > >>> Nodes with bootph-pre-sram/ram props are bound in multiple phases:
> > >>> 1. At TPL (bootph-pre-sram) or SPL (bootph-pre-ram) phase
> > >>> 2. At U-Boot proper pre-relocation phase
> > >>> 3. At U-Boot proper normal phase
> > >>>
> > >>> However the binding and U-Boot Driver Model documentation indicate that
> > >>> only nodes marked with bootph-all or bootph-some-ram should be bound in
> > >>> the U-Boot proper pre-relocation phase.
> > >>>
> > >>> Change ofnode_pre_reloc to report a node with bootph-pre-ram/sram prop
> > >>> with a pre-reloc status only after U-Boot proper pre-relocation phase.
> > >>> Also update the ofnode_pre_reloc documentation to closer reflect the
> > >>> binding and driver model documentation.
> > >>>
> > >>> This changes behavior of what nodes are bound in the U-Boot proper
> > >>> pre-relocation phase. Change to bootph-all or add bootph-some-ram prop
> > >>> to restore prior behavior.
> > >>>
> > >>> Signed-off-by: Jonas Karlman <jonas at kwiboo.se>
> > >>> Reviewed-by: Simon Glass <sjg at chromium.org>
> > >>> ---
> > >>> Changes in v2:
> > >>> - Drop use of !! to convert into bool
> > >>> - Update documentation for ofnode_pre_reloc
> > >>> - Rewrite commit message
> > >>> - Collect r-b tag
> > >>
> > >> This patch breaks boot on AM642-EVM. Boot log at the end.
> > >
> > > From what I can tell your board use a lot of bootph-pre-ram.
> > > https://source.denx.de/u-boot/u-boot/-/blob/master/arch/arm/dts/k3-am642-evm-u-boot.dtsi
> > >
> > > What happens if you change to bootph-all or add bootph-some-ram next to
> > > bootph-pre-ram on devices that is also needed in U-Boot proper
> > > pre-relocation phase in addition to SPL phase?
> >
> > Then it works.
> >
> > BTW, AM62-SK is broken as well and I suppose most K3 TI boards would be broken.
> >
> > Nishanth / Tom,
> >
> > What approach to take here?
> > Replacing bootph-pre-ram to bootph-all in *-u-boot.dtsi would be a quick fix.
>
> An exact quick fix is to add bootph-some-ram next to bootph-pre-ram:
> this will have the same effects of bootph-pre-ram before the patch.
> Instead, replacing bootph-pre-ram with bootph-all will affect also
> other boot phases (e.g. TPL) if any.

Yes.

Since this is in -next we have plenty of time to get things figured
out before it ends up in a release.

Regards,
Simon

>
> Regards,
> Massimo
>
> > Then we need to mark nodes required only for SPL as bootph-pre-ram.
> >
> > Meanwhile I will suggest to revert the $subject patch till this is sorted out
> > and gets a Ack from board maintainers.
> >
> > cheers,
> > -roger
> >
> > >
> > > Regards,
> > > Jonas
> > >
> > >>
> > >>>
> > >>>  drivers/core/ofnode.c | 2 +-
> > >>>  include/dm/ofnode.h   | 8 ++++----
> > >>>  2 files changed, 5 insertions(+), 5 deletions(-)
> > >>>
> > >>> diff --git a/drivers/core/ofnode.c b/drivers/core/ofnode.c
> > >>> index 8df16e56af5c..b1e94b2d60df 100644
> > >>> --- a/drivers/core/ofnode.c
> > >>> +++ b/drivers/core/ofnode.c
> > >>> @@ -1353,7 +1353,7 @@ bool ofnode_pre_reloc(ofnode node)
> > >>>      */
> > >>>     if (ofnode_read_bool(node, "bootph-pre-ram") ||
> > >>>         ofnode_read_bool(node, "bootph-pre-sram"))
> > >>> -           return true;
> > >>> +           return gd->flags & GD_FLG_RELOC;
> > >>>
> > >>>     if (IS_ENABLED(CONFIG_OF_TAG_MIGRATE)) {
> > >>>             /* detect and handle old tags */
> > >>> diff --git a/include/dm/ofnode.h b/include/dm/ofnode.h
> > >>> index 0f38b3e736de..13700f8266d7 100644
> > >>> --- a/include/dm/ofnode.h
> > >>> +++ b/include/dm/ofnode.h
> > >>> @@ -1198,15 +1198,15 @@ int ofnode_read_simple_size_cells(ofnode node);
> > >>>   * determine if a node was bound in one of SPL/TPL stages.
> > >>>   *
> > >>>   * There are 4 settings currently in use
> > >>> - * - bootph-some-ram: U-Boot proper pre-relocation only
> > >>> + * - bootph-some-ram: U-Boot proper pre-relocation phase
> > >>>   * - bootph-all: all phases
> > >>>   * Existing platforms only use it to indicate nodes needed in
> > >>>   * SPL. Should probably be replaced by bootph-pre-ram for new platforms.
> > >>> - * - bootph-pre-ram: SPL and U-Boot pre-relocation
> > >>> - * - bootph-pre-sram: TPL and U-Boot pre-relocation
> > >>> + * - bootph-pre-ram: SPL phase
> > >>> + * - bootph-pre-sram: TPL phase
> > >>>   *
> > >>>   * @node: node to check
> > >>> - * Return: true if node is needed in SPL/TL, false otherwise
> > >>> + * Return: true if node should be or was bound, false otherwise
> > >>>   */
> > >>>  bool ofnode_pre_reloc(ofnode node);
> > >>>
> > >>
> > >> ---boot log---
> > >>
> > >> U-Boot SPL 2023.10-rc4-00480-g9e644284ab (Sep 28 2023 - 15:53:09 +0300)
> > >> Resetting on cold boot to workaround ErrataID:i2331
> > >> Please resend tiboot3.bin in case of UART/DFU boot
> > >> resetting ...
> > >>
> > >> U-Boot SPL 2023.10-rc4-00480-g9e644284ab (Sep 28 2023 - 15:53:09 +0300)
> > >> SYSFW ABI: 3.1 (firmware rev 0x0009 '9.0.5--v09.00.05 (Kool Koala)')
> > >> SPL initial stack usage: 13408 bytes
> > >> Trying to boot from MMC2
> > >> Warning: Detected image signing certificate on GP device. Skipping certificate to prevent boot failure. This will fail if the image was also encrypted
> > >> Warning: Detected image signing certificate on GP device. Skipping certificate to prevent boot failure. This will fail if the image was also encrypted
> > >> Warning: Detected image signing certificate on GP device. Skipping certificate to prevent boot failure. This will fail if the image was also encrypted
> > >> Warning: Detected image signing certificate on GP device. Skipping certificate to prevent boot failure. This will fail if the image was also encrypted
> > >> Loading Environment from MMC... *** Warning - No MMC card found, using default environment
> > >>
> > >> Starting ATF on ARM64 core...
> > >>
> > >> NOTICE:  BL31: v2.8(release):v2.8-226-g2fcd408bb3-dirty
> > >> NOTICE:  BL31: Built : 00:42:57, Jan 13 2023
> > >> I/TC:
> > >> I/TC: OP-TEE version: 3.20.0 (gcc version 11.3.0 (GCC)) #1 Fri Jan 20 15:42:54 UTC 2023 aarch64
> > >> I/TC: WARNING: This OP-TEE configuration might be insecure!
> > >> I/TC: WARNING: Please check https://optee.readthedocs.io/en/latest/architecture/porting_guidelines.html
> > >> I/TC: Primary CPU initializing
> > >> I/TC: SYSFW ABI: 3.1 (firmware rev 0x0009 '9.0.5--v09.00.05 (Kool Koala)')
> > >> I/TC: HUK Initialized
> > >> I/TC: Activated SA2UL device
> > >> I/TC: Fixing SA2UL firewall owner for GP device
> > >> I/TC: Enabled firewalls for SA2UL TRNG device
> > >> I/TC: SA2UL TRNG initialized
> > >> I/TC: SA2UL Drivers initialized
> > >> I/TC: Primary CPU switching to normal world boot
> > >>
> > >> U-Boot SPL 2023.10-rc4-00480-g9e644284ab (Sep 28 2023 - 15:54:37 +0300)
> > >> SYSFW ABI: 3.1 (firmware rev 0x0009 '9.0.5--v09.00.05 (Kool Koala)')
> > >> Trying to boot from MMC2
> > >> Warning: Detected image signing certificate on GP device. Skipping certificate to prevent boot failure. This will fail if the image was also encrypted
> > >> Warning: Detected image signing certificate on GP device. Skipping certificate to prevent boot failure. This will fail if the image was also encrypted
> > >>
> > >> ---boot log end---
> > >>
> > >> we expect U-Boot banner to be printed next but nothing is printed.
> > >>
> > >
> >