[PATCH] imx8m: set sane default value for SPL_LOAD_FIT_ADDRESS

[Marek Vasut]

On 10/24/24 3:18 PM, Rasmus Villemoes wrote:
> On Thu, Oct 24 2024, Marek Vasut <marex at 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 at 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

Thanks