[PATCH 2/5] rockchip: Improve ARMv7 support for ROCKCHIP_COMMON_STACK_ADDR

[Alex Bee]

Hi Jonas,

> Hi Alex,
>
> On 2025-01-27 19:59, Alex Bee wrote:
>> Hi Jonas,
>>
>> thanks for working on this.  Please see one comment below.
>>
>> Am 27.01.25 um 00:48 schrieb Jonas Karlman:
>>> A few Rockchip ARMv7 SoCs use 0x60000000 as DRAM base address instead of
>>> the more common 0x0 DRAM base address used on AArch64 SoCs.
>>>
>>> Add default options that should work for these ARMv7 SoCs. Same offsets
>>> as before are used, just below 64 MiB. Hex values have also been padded
>>> to improve alignment.
>>>
>>> Signed-off-by: Jonas Karlman <jonas at kwiboo.se>
>>> ---
>>>    arch/arm/mach-rockchip/Kconfig | 35 +++++++++++++++++++++++-----------
>>>    1 file changed, 24 insertions(+), 11 deletions(-)
>>>
>>> diff --git a/arch/arm/mach-rockchip/Kconfig b/arch/arm/mach-rockchip/Kconfig
>>> index 5aa9e0f1af3b..da20d3233fe3 100644
>>> --- a/arch/arm/mach-rockchip/Kconfig
>>> +++ b/arch/arm/mach-rockchip/Kconfig
>>> @@ -637,40 +637,53 @@ source "arch/arm/mach-rockchip/rv1126/Kconfig"
>>>    if ROCKCHIP_COMMON_STACK_ADDR && SPL_SHARES_INIT_SP_ADDR
>>>    
>>>    config CUSTOM_SYS_INIT_SP_ADDR
>>> -	default 0x3f00000
>>> +	default 0x63f00000 if SPL_TEXT_BASE = 0x60000000
>>> +	default 0x03f00000 if SPL_TEXT_BASE = 0x00000000
>>>    
>>>    config SYS_MALLOC_F_LEN
>>> -	default 0x10000 if CUSTOM_SYS_INIT_SP_ADDR = 0x3f00000
>>> +	default 0x10000 if CUSTOM_SYS_INIT_SP_ADDR = 0x63f00000
>>> +	default 0x10000 if CUSTOM_SYS_INIT_SP_ADDR = 0x03f00000
>>>    
>>>    config SPL_SYS_MALLOC_F_LEN
>>> -	default 0x8000 if CUSTOM_SYS_INIT_SP_ADDR = 0x3f00000
>>> +	default 0x8000 if CUSTOM_SYS_INIT_SP_ADDR = 0x63f00000
>>> +	default 0x8000 if CUSTOM_SYS_INIT_SP_ADDR = 0x03f00000
>>>    
>>>    config TPL_SYS_MALLOC_F_LEN
>>> -	default 0x4000 if CUSTOM_SYS_INIT_SP_ADDR = 0x3f00000
>>> +	default 0x0800 if CUSTOM_SYS_INIT_SP_ADDR = 0x63f00000
>>> +	default 0x4000 if CUSTOM_SYS_INIT_SP_ADDR = 0x03f00000
>>>    
>>>    config TEXT_BASE
>>> -	default 0x00200000 if ARM64
>>> +	default 0x60200000 if SPL_TEXT_BASE = 0x60000000
>>> +	default 0x00200000 if SPL_TEXT_BASE = 0x00000000
>>>    
>>>    config SPL_TEXT_BASE
>>> -	default 0x0 if ARM64
>>> +	default 0x60000000 if ROCKCHIP_RK3036 || ROCKCHIP_RK3066 || \
>>> +			      ROCKCHIP_RK3128 || ROCKCHIP_RK3188 || \
>> I'm not sure if adding RK3036 and RK3188 is correct at this point. Both use
>> SPL (without TPL) currently for DRAM initialization and thus need their
>> CFG_IRAM_BASE as SPL_TEXT_BASE.
> I think it should be safe to add these default values here, any value
> defined in board defconfig or soc Kconfig will take precedence.
>
> The RK3036 and RK3188 boards use SPL back-to-brom to load U-Boot proper,
> if someone wants to move these board to use similar TPL+SPL+FIT this
> will prepare for such occasion.
>
> Having them included here also prevents a possible issue in case someone
> in the future where to add a select/imply ROCKCHIP_COMMON_STACK_ADDR for
> these SoCs, else they may fallback to wrongly use 0x0.
>
> Do you want me to drop them in a v2?
>
> Regards,
> Jonas
it is currently "safe" as it would of course be overwritten by the
currently existing defconfigs. But if someone dares to use this symbol for
a downstream/new board for either of these two SoCs, things go south.

How about adding some "extra security" by making it
     depends on TPL || ROCKCHIP_EXTERNAL_TPL

As I mentioned off-list: I'm not sure if any of these
XXX_ROCKCHIP_COMMON_BORD belong here: I understand the goal of syncing
configurations for the different SoCs, but I'm not sure if a symbol called
ROCKCHIP_COMMON_STACK_ADDR is the right place for it (plus the
XXX_ROCKCHIP_COMMON_BORD symbols (should) have no dependency on using
ROCKCHIP_COMMON_STACK_ADDR, neither code-wise nor from semantics).

Regards,
Alex

>> Regards,
>>
>> Alex
>>
>>> +			      ROCKCHIP_RK322X || ROCKCHIP_RV1108
>>> +	default 0x00000000
>>>    
>>>    config SPL_HAS_BSS_LINKER_SECTION
>>>    	default y if ARM64
>>>    
>>>    config SPL_BSS_START_ADDR
>>> -	default 0x3f80000
>>> +	default 0x63f80000 if SPL_TEXT_BASE = 0x60000000
>>> +	default 0x03f80000 if SPL_TEXT_BASE = 0x00000000
>>>    
>>>    config SPL_BSS_MAX_SIZE
>>> -	default 0x8000 if SPL_BSS_START_ADDR = 0x3f80000
>>> +	default 0x8000 if SPL_BSS_START_ADDR = 0x63f80000
>>> +	default 0x8000 if SPL_BSS_START_ADDR = 0x03f80000
>>>    
>>>    config SPL_STACK_R
>>> -	default y if CUSTOM_SYS_INIT_SP_ADDR = 0x3f00000
>>> +	default y if CUSTOM_SYS_INIT_SP_ADDR = 0x63f00000
>>> +	default y if CUSTOM_SYS_INIT_SP_ADDR = 0x03f00000
>>>    
>>>    config SPL_STACK_R_ADDR
>>> -	default 0x3e00000 if CUSTOM_SYS_INIT_SP_ADDR = 0x3f00000
>>> +	default 0x63e00000 if CUSTOM_SYS_INIT_SP_ADDR = 0x63f00000
>>> +	default 0x03e00000 if CUSTOM_SYS_INIT_SP_ADDR = 0x03f00000
>>>    
>>>    config SPL_STACK_R_MALLOC_SIMPLE_LEN
>>> -	default 0x200000 if SPL_STACK_R_ADDR = 0x3e00000
>>> +	default 0x200000 if SPL_STACK_R_ADDR = 0x63e00000
>>> +	default 0x200000 if SPL_STACK_R_ADDR = 0x03e00000
>>>    
>>>    endif
>>>    endif