Re: [PATCH] imx8m: set sane default value for SPL_LOAD_FIT_ADDRESS

[Tom Rini <trini@konsulko.com>]

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

On Thu, Oct 24, 2024 at 04:06:03PM +0200, Marek Vasut wrote:
> On 10/24/24 3:18 PM, Rasmus Villemoes wrote:
> > On Thu, Oct 24 2024, Marek Vasut <marex@denx.de> wrote:
> > 
> > > On 10/24/24 12:01 PM, Rasmus Villemoes wrote:
> > > > I enabled IMX_HAB on an imx8mp board, but even though I knew about the
> > > > implementation, I forgot that I had to provide a sane value for
> > > > SPL_LOAD_FIT_ADDRESS. The help text for IMX_HAB doesn't mention this
> > > > implicit requirement, and there's no build-time warning; the default
> > > > 0x0 value just ends up being returned from
> > > > board_spl_fit_buffer_addr(), obviously resulting in a non-booting
> > > > board.
> > > > The existing imx8m* board configs that set a non-zero value
> > > > currently
> > > > all use 0x44000000. The actual value doesn't matter too much, but 0 is
> > > > always wrong for imx8m platforms. So just use 0x44000000 as default
> > > > for those platforms.
> > > > Signed-off-by: Rasmus Villemoes <ravi@prevas.dk>
> > > > ---
> > > >    boot/Kconfig | 1 +
> > > >    1 file changed, 1 insertion(+)
> > > > diff --git a/boot/Kconfig b/boot/Kconfig
> > > > index 940389d4882..72d1f69afcd 100644
> > > > --- a/boot/Kconfig
> > > > +++ b/boot/Kconfig
> > > > @@ -231,6 +231,7 @@ config SPL_LOAD_FIT
> > > >    config SPL_LOAD_FIT_ADDRESS
> > > >    	hex "load address of fit image"
> > > >    	depends on SPL_LOAD_FIT
> > > > +	default 0x44000000 if ARCH_IMX8M
> > > This only applies to HAB , for non-HAB the fitImage can be loaded at
> > > arbitrary location, do you need:
> > > 
> > > default 0x44000000 if ARCH_IMX8M && IMX_HAB
> > > 
> > > right ?
> > 
> > On IMX8 without HAB, the value is not used at all AFAICT (otherwise the
> > value of 0x0 would have caused trouble already). I don't see the harm of
> > setting some sane value that's actually within DRAM space independent of
> > HAB.
> > 
> > Making the default depend on IMX_HAB has the UX problem that if I do
> > "make imx8mp_evk_defconfig", then go about tweaking stuff, then do "make
> > menuconfig" and enable IMX_HAB, SPL_LOAD_FIT_ADDRESS already has the
> > bogus 0x0 value and nothing forces or asks that to be changed. Making the
> > default depend only on SOC (or SOC family or whatever IMX8M is in this
> > context) makes that problem go away.
> Looking at this closely, common/spl/spl_ram.c does make use of
> CONFIG_SPL_LOAD_FIT_ADDRESS too, so I think yes, we should have a default
> for this.
> 
> iMX6 should have 0x14000000
> iMX7 should have 0x84000000
> iMX8M should have 0x44000000

These differ, slightly, from the value used in CONFIG_SYS_LOAD_ADDR.
Could that not be used (and the overall option changed to default
SYS_LOAD_ADDR ?

-- 
Tom

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