[PATCH 4/5] mach-snapdragon: support building SPL

[Simon Glass]

Hi Michael,

On 2026-04-03T23:18:18, Michael Srba <michael.srba at seznam.cz> wrote:
> mach-snapdragon: support building SPL
>
> Initially sdm845 support is added, and only usb boot
> is supported for the next stage.
>
> Signed-off-by: Michael Srba <Michael.Srba at seznam.cz>
> ---
>  arch/arm/Kconfig                                   |   6 +-
>  arch/arm/dts/sdm845-u-boot.dtsi                    |  16 +++
>  arch/arm/mach-snapdragon/Kconfig                   |  98 +++++++++++++++-
>  arch/arm/mach-snapdragon/board.c                   |  26 +++++
>  arch/arm/mach-snapdragon/include/mach/boot0.h      |  61 ++--------
>  .../mach-snapdragon/include/mach/msm8916_boot0.h   |  54 +++++++++
>  .../include/mach/sdm845_spl_boot0.h                | 120 +++++++++++++++++++
>  arch/arm/mach-snapdragon/u-boot-spl-elf-sdm845.lds |  25 ++++
>  board/qualcomm/sdm845_spl.env                      |   1 +
>  configs/sdm845_spl_defconfig                       | 130 +++++++++++++++++++++
>  doc/board/qualcomm/index.rst                       |   1 +
>  doc/board/qualcomm/spl.rst                         |  70 +++++++++++
>  12 files changed, 554 insertions(+), 54 deletions(-)

Please can you split this? E.g.

- boot0.h refactoring and msm8916 header extraction
- sdm845-specific boot0.h with the EL3 escalation code
- Kconfig additions for QCOM_SPL and SPL_TARGET_SDM845
- board.c SPL support functions
- defconfig and documentation

> diff --git a/arch/arm/mach-snapdragon/Kconfig b/arch/arm/mach-snapdragon/Kconfig
> @@ -1,6 +1,24 @@
> +config SYS_MALLOC_LEN
> +     default 0x10000000
> +     default $(shell, printf "0x%x\n" "$(dollar)(($(SDM845_OCIMEM_SIZE) / 2))") if SPL_TARGET_SDM845

There are three definitions of SYS_MALLOC_LEN in this file. Only the
first matching default takes effect, so the conditional default for
SPL_TARGET_SDM845 will never be used. Please can you reorder these so
the conditional default comes first, or consolidate them.

> diff --git a/arch/arm/mach-snapdragon/include/mach/boot0.h b/arch/arm/mach-snapdragon/include/mach/boot0.h
> @@ -1,54 +1,11 @@
> +#if defined(CONFIG_SPL_BUILD)
> +#if CONFIG_SPL_TARGET_SDM845

Please can you use defined(CONFIG_SPL_TARGET_SDM845) here for
consistency with the line above and to avoid relying on the value of
an undefined symbol.

> diff --git a/arch/arm/mach-snapdragon/include/mach/sdm845_spl_boot0.h b/arch/arm/mach-snapdragon/include/mach/sdm845_spl_boot0.h
> @@ -0,0 +1,120 @@
> +     movl    x0, CONFIG_SPL_TEXT_BASE
> +     add     x0, x0, el3_ret_point
> +     br      x0

The el3_ret_point label is an absolute address, but you are adding it
to CONFIG_SPL_TEXT_BASE. Don't you want the offset of el3_ret_point
relative to some base, e.g. (el3_ret_point - _start) ? If not, please
can you add a comment?

> diff --git a/arch/arm/mach-snapdragon/include/mach/sdm845_spl_boot0.h b/arch/arm/mach-snapdragon/include/mach/sdm845_spl_boot0.h
> +     /*  */
> +     movl    x0, CONFIG_SPL_TEXT_BASE

Empty comment looks like a leftover - please remove or fill in.

> diff --git a/arch/arm/dts/sdm845-u-boot.dtsi b/arch/arm/dts/sdm845-u-boot.dtsi
> +&rpmhcc: clock-controller {
> +     bootph-all;
> +};

This syntax is incorrect - you cannot redefine a label when
referencing a node. Try:

&rpmhcc {
  bootph-all;
};

> diff --git a/arch/arm/mach-snapdragon/board.c b/arch/arm/mach-snapdragon/board.c
> @@ -749,3 +757,21 @@ void enable_caches(void)
> +__weak void reset_cpu(void)
> +{
> +     /* TODO */
> +     while (1) {
> +             /* loop forever */
> +     };
> +}

This function is not guarded by CONFIG_SPL_BUILD, so it will be
compiled for U-Boot proper as well. Since the defconfig enables
CONFIG_SYSRESET_QCOM_PSHOLD, there could be multiple reset_cpu()
implementations. Please can you wrap this in #ifdef CONFIG_SPL_BUILD
or move it to a separate SPL-specific file.

> diff --git a/arch/arm/mach-snapdragon/board.c b/arch/arm/mach-snapdragon/board.c
> @@ -757,3 +757,21 @@ void enable_caches(void)
> +u32 spl_boot_device(void)
> +{
> +     /* TODO: check boot reason to support UFS and sdcard */
> +     u32 boot_device = BOOT_DEVICE_DFU;
> +
> +     return boot_device;
> +}

Same here - please guard with #ifdef CONFIG_SPL_BUILD

Regards,
Simon