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

[Tom Rini <trini@konsulko.com>]

[-- Attachment #1: Type: text/plain, Size: 4613 bytes --]

On Wed, Apr 08, 2026 at 07:03:37PM +0200, Michael Srba wrote:
> 
> 
> On 4/6/26 16:27, Tom Rini wrote:
> > On Sat, Apr 04, 2026 at 01:18:19AM +0200, michael.srba@seznam.cz wrote:
> > 
> > > From: Michael Srba <Michael.Srba@seznam.cz>
> > > 
> > > Initially sdm845 support is added, and only usb boot
> > > is supported for the next stage.
> > > 
> > > Signed-off-by: Michael Srba <Michael.Srba@seznam.cz>
> > [snip]
> > > diff --git a/arch/arm/mach-snapdragon/Kconfig b/arch/arm/mach-snapdragon/Kconfig
> > > index 976c0e35fce..938e6ebd8bf 100644
> > > --- a/arch/arm/mach-snapdragon/Kconfig
> > > +++ b/arch/arm/mach-snapdragon/Kconfig
> > > @@ -1,6 +1,24 @@
> > >   if ARCH_SNAPDRAGON
> > > +# SoC specific SRAM addresses
> > > +
> > > +# sdm845
> > > +SDM845_BOOT_IMEM_BASE := 0x14800000
> > > +SDM845_BOOT_IMEM_SIZE := 0x180000
> > > +# we may not be able to use the whole BOOT_IMEM depending on the whitelisted regions hardcoded in PBL
> > > +# (we could technically relocate ourselves after the fact)
> > > +SDM845_BOOT_IMEM_OFFSET := 0x3f000
> > > +SDM845_BOOT_IMEM_USABLE_SIZE := 0xc1000
> > > +# technically the below would work, except the memory from 0x14833000 to 0x1483F000 gets trashed
> > > +# between the ELF getting loaded and XBL_SEC jumping to our code
> > > +#SDM845_BOOT_IMEM_OFFSET := 0x16000
> > > +#SDM845_BOOT_IMEM_USABLE_SIZE := 0xea000
> > > +SDM845_OCIMEM_BASE := 0x14680000
> > > +SDM845_OCIMEM_SIZE := 0x00040000
> > > +SDM845_OCIMEM_END  := $(shell, printf "0x%x\n" "$(dollar)(($(SDM845_OCIMEM_BASE) + $(SDM845_OCIMEM_SIZE) - 1))")
> > I didn't know Kconfig even allowed this. And I *really* don't like it,
> > and I think it's going in the wrong direction with respect to being able
> > to configure a system. If these are configurable, then they need to be
> > "hex" type "config" options, and then used later on. Otherwise they
> > should just be the evaluated value as "default ... if ..." later on,
> > instead.
> This is per-SoC hardware info. The SRAM is physically located at that
> address with that size. The offset/usable size are further restrictions
> imposed by the bootrom, which are relevant for placing the text section
> (the bootrom will copy the ELF segment precisely where the ELF segment
> says it wants to live, as long as that's in a whitelisted region).
> 
> I really don't like the idea of just hardcoding hex values with either
> no explanation of where they come from, or with a comment that may or
> may not reflect the reality since there's no mechanism that would enforce
> that the value was actually computed according to the comment.
> (also the amount and complexity of such comments would be higher since
> comments can't reference nonexistent intermediate variables)
> 
> What I wanted to do here was more along the lines of:
> ```
> config QCOM_BOOT_IMEM_BASE
>     default 0x14800000 if SPL_TARGET_SDM845
> 
> config QCOM_BOOT_IMEM_SIZE
>     default 0x180000 if SPL_TARGET_SDM845
> 
> config QCOM_BOOT_IMEM_OFFSET
>     default 0x3f000 if SPL_TARGET_SDM845
> 
> config QCOM_BOOT_IMEM_USABLE_SIZE
>     default 0xc1000 if SPL_TARGET_SDM845
> ```
> and then
> ```
> config SPL_STACK
>     default SDM845_OCIMEM_START + SDM845_OCIMEM_SIZE
> 
> config SPL_TEXT_BASE
>     default QCOM_BOOT_IMEM_BASE + QCOM_BOOT_IMEM_OFFSET
> 
> config SPL_SYS_MALLOC_F_LEN
>     QCOM_BOOT_IMEM_SIZE / 2
> ```
> 
> The problem with the above is that Kconfig absolutely doesn't support
> doing that. The only way to do arithmetics is to use the preprocessor
> ("macro language"), and the preprocessor obviously doesn't have access
> to the values of symbols, only to preprocessor variables.
> 
> The current form is the only solution I could come up with that avoids
> hardcoding hex values, and from reading kconfig docs and sources I think
> it's also the only solution possible short of patching kconfig to support
> arithmetics.
> 
> If you consider this more ugly than magic hex values (which afaict is
> the only other option, other than patching kconfig of course),
> I can obviously do that, although it does make me very sad, and I'd prefer
> if we can find a solution that avoids hardcoding. What do you think?

Yeah, I'd rather go with "magic" hex values in appropriate headers, and
should probably be in the CFG_SYS namespace (so CFG_SYS_QCOM_... ) for
things that aren't "how do we work out SPL_SYS_MALLOC_F_LEN" which will
just be (matching everyone else really) just the hex value, and
explained in the board doc where things come from.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]