[U-Boot] kirkwood: CONFIG_SYS_INIT_SP_ADDR wrong?

[Daniel Hobi]

Hi Albert and Heiko,

On 10.11.2010 17:46, Heiko Schocher wrote:
> Daniel Hobi wrote:
>> 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);

gd (struct global_data) does not require the same alignment as the stack
pointer. If CONFIG_SYS_INIT_SP_ADDR is 4-byte aligned, but not 8-byte
aligned, 4 bytes between these two are unused. If you care about that,
just 8-byte align gd in the assignment above. It makes sense to at least
4-byte align gd anyway.

> 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 ...

We would tell board_init_f where to find the global_data, not the stack.
If other architectures also use the same computation for stack (in
start.S) and global_data (in board_init_f), this change should be
considered.

On 10.11.2010 17:29, Albert ARIBAUD wrote:
> Bad solution IMO: if the symbol is used elsewhere, one would have to 
> always remember that it should be aligned down to a multiple of
> eight. We may not need to make it a generated constant, but we
> definitely need to make sure it is correctly aligned wherever it is
> used, e.g. by defining it as
> 
> #define CONFIG_SYS_INIT_SP_ADDR
>   (0xC8012004-GENERATED_GBL_DATA_SIZE)&~7

I think it's much less error-prone to align CONFIG_SYS_INIT_SP_ADDR
wherever it is used. It should be used only at two places anyway: to
initialize the early stack pointer and the global_data pointer.

As a reminder: it was extremely difficult the find the misaligned stack
pointer (resulting in the patch from 2009-10-06) given the symptoms. And
since you cannot expect board maintainers to spend an equal amount of
time to solve this problem over and over again, we definitely need to
align CONFIG_SYS_INIT_SP_ADDR where it is used, and not where it is defined.

Best regards,
Daniel