[U-Boot] kirkwood: CONFIG_SYS_INIT_SP_ADDR wrong?

[Albert ARIBAUD <albert.aribaud@free.fr>]

Le 10/11/2010 16:46, Daniel Hobi a ?crit :
> Hi Heiko,
>
> 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.
>
> 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)
>
> 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
>
> Best regards,
> Daniel

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

(spacing squashed to avoid funny line breaks)

Amicalement,
-- 
Albert.