[U-Boot] kirkwood: CONFIG_SYS_INIT_SP_ADDR wrong?

[Heiko Schocher]

Hello Daniel,

Daniel Hobi wrote:
> On 10.11.2010 16:40, Heiko Schocher wrote:
>> Daniel Hobi wrote:
>>> But you also added assembly code to setup the initial stack pointer in
>>> arch/arm/cpu/*/start.S (ie commit ab86f72c for arm926ejf) which reads:
>>>
>>> /* Set stackpointer in internal RAM to call board_init_f */
>>> call_board_init_f:
>>> 	ldr	sp, =(CONFIG_SYS_INIT_SP_ADDR)
>>>
>>> CONFIG_SYS_INIT_SP_ADDR may not be aligned properly, especially with
>>> your latest patch to km_arm.h:
>>>
>>> #define CONFIG_SYS_INIT_SP_ADDR	(0xC8012000 - GENERATED_GBL_DATA_SIZE)
>> Ah, good catch.
>>
>> Then we should add this alignment into the generation of
>> GENERATED_GBL_DATA_SIZE.

looked in the generation of GENERATED_GBL_DATA_SIZE:

./lib/asm-offsets.c

        /* Round up to make sure size gives nice stack alignment */
        DEFINE(GENERATED_GBL_DATA_SIZE,
                (sizeof(struct global_data)+15) & ~15);

this is ok.

> Hm? The stack pointer needs alignment, not GENERATED_GBL_DATA_SIZE. What
> happens if I define:
> 
> #define CONFIG_SYS_INIT_SP_ADDR	(0xC8012004 - GENERATED_GBL_DATA_SIZE)

Indeed, that would be bad ... don;t do it ;-)

> Why not add alignment to start.S?
> 
>  /* Set stackpointer in internal RAM to call board_init_f */
>  call_board_init_f:
>  	ldr	sp, =(CONFIG_SYS_INIT_SP_ADDR)
> +	bic	sp, sp, #7

Hmm.. because we do in board_init_f:

 gd = (gd_t *) (CONFIG_SYS_INIT_SP_ADDR);

Hmm.. maybe we should call board_init_f with the info, where to
find the stack, but this would be a change for all architectures,
not only arm ...

opinions?

bye,
Heiko
-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany