Hi,
On 5/8/26 18:49, Simon Glass wrote:
Hi Michael,
On 2026-05-08T00:33:28, Michael Srba <[email protected]> wrote:
mach-snapdragon: boot0.h: split out msm8916_boot0.h
Prepare for supporting alternative boot0.h per-SoC by splitting out
the existing msm8916-specific code.
There is now a selection mechanism to choose a specific boot0.h
in the Kconfig. BOOT0_MSM8916_PSCI_WORKAROUND is the only option
right now, but more can be added. The toplevel boot0.h additionally
enables conditionally performing the include only in u-boot proper,
or only in SPL.
Signed-off-by: Michael Srba <[email protected]>
arch/arm/mach-snapdragon/Kconfig | 16 ++++++
arch/arm/mach-snapdragon/include/mach/boot0.h | 60 +++-------------------
.../mach-snapdragon/include/mach/msm8916_boot0.h | 54 +++++++++++++++++++
configs/dragonboard410c_defconfig | 1 +
4 files changed, 79 insertions(+), 52 deletions(-)
Reviewed-by: Simon Glass <[email protected]>
diff --git a/configs/dragonboard410c_defconfig
b/configs/dragonboard410c_defconfig
@@ -11,6 +11,7 @@ CONFIG_ENV_OFFSET=0x0
CONFIG_DEFAULT_DEVICE_TREE='qcom/apq8016-sbc'
CONFIG_OF_LIBFDT_OVERLAY=y
CONFIG_SYS_LOAD_ADDR=0x80080000
+CONFIG_BOOT0_MSM8916_PSCI_WORKAROUND=y
CONFIG_IDENT_STRING="\nQualcomm-DragonBoard 410C"
Do you need to see CONFIG_BOOT0_MSM8916_PSCI_WORKAROUND=y in
hmibsc_defconfig too? It looks like hmibsc_defconfig is also an
apq8016 (msm8916) board with CONFIG_ENABLE_ARM_SOC_BOOT0_HOOK=y, so it
currently relies on the same PSCI workaround and will silently lose it
after this patch. If not, it is worth mentioning in the commit
message.
My bad, didn't realize that, will add it.
diff --git a/arch/arm/mach-snapdragon/include/mach/boot0.h
b/arch/arm/mach-snapdragon/include/mach/boot0.h
@@ -1,54 +1,10 @@
/* SPDX-License-Identifier: GPL-2.0+ */
-/*
- * Workaround for "PSCI bug" on DragonBoard 410c
...
+#if defined(CONFIG_SPL_BUILD)
b reset
-
- .align 3
-el1_system_param:
- .quad 0, 0, 0, 0, 0, 0, 0, 0, 0 /* el1_x0-x8 */
- .quad reset /* el1_elr */
-el1_system_param_end:
+#else
+#if defined(CONFIG_BOOT0_MSM8916_PSCI_WORKAROUND)
+#include 'msm8916_boot0.h'
+#else
+ b reset
+#endif
+#endif
You could flatten this with #elif - which would make it more obvious
that the SPL path is intentionally a stub.
#if defined(CONFIG_SPL_BUILD)
b reset
#elif defined(CONFIG_BOOT0_MSM8916_PSCI_WORKAROUND)
#include 'msm8916_boot0.h'
#else
b reset
#endif
It's not going to be a stub after the followup commit that adds the sdm845
behavior which should only be enabled in SPL though. It's a nested if
(sadly the code style for macros does't really make it obvious),
basically:
```
#if defined(CONFIG_SPL_BUILD)
... spl-only workarounds ...
#else
... u-boot-proper-only workarounds ...
#endif
```
I wonder if using CONFIG_IS_ENABLED would be cleaner, even though
in most cases the config option would only exist either with or without
the SPL_ prefix.
Regards,
Simon
