Re: [PATCH 1/1] ARM: i.MX31: Add support for mx31moboard board

["Philippe Rétornaz" <philippe.retornaz@epfl.ch>]

Le 28/02/2014 09:37, Sascha Hauer a écrit :
> On Fri, Feb 28, 2014 at 09:21:40AM +0100, Philippe Rétornaz wrote:
>> Le 27/02/2014 21:36, Sascha Hauer a écrit :
>>> On Thu, Feb 27, 2014 at 03:03:45PM +0100, Philippe Rétornaz wrote:
>>>> +void __bare_init __naked barebox_arm_reset_vector(void)
>>>> +{
>>>> +	uint32_t r;
>>>> +
>>>> +	arm_cpu_lowlevel_init();
>>>> +
>>>> +	/* Enable IPU Display interface */
>>>> +	writel(1 << 6, MX31_IPU_CTRL_BASE_ADDR);
>>>> +
>>>> +	writel(0x074B0BF5, MX31_CCM_BASE_ADDR + MX31_CCM_CCMR);
>>>> +
>>>> +	asm volatile("1:\n\t"
>>>> +			"SUBS %0, %0, #1	\n\t"
>>>> +			"BNE  1b		\n\t"
>>>> +			: : "r" (0x4000) : "cc");
>>>
>>> You can write a delay loop in c with:
>>>
>>> 	volatile int c;
>>>
>>> 	for (c = 0; c < 0x4000; c++)
>>
>> Well, no, at least not on my toolchain. Because the volatile ask gcc to
>> not optimize the variable, it then put it on the stack. But the stack
>> pointer is not yet initialized, so it crashes. I've tried with a
>> barrier() instead of the volatile, but it leads to the same assembly
>> (which is not surprising). Here is the compiled code with your suggestion:
>>
>>          ldr     r2, .L9+8
>>          b       .L2
>> .L3:
>>          ldr     r3, [sp, #4]
>>          add     r3, r3, #1
>>          str     r3, [sp, #4]
>> .L2:
>>          ldr     r3, [sp, #4]
>>          cmp     r3, r2
>>          ble     .L3
>>
>> With L9+8:
>>          .word   16383
>>
>> But the stack pointer is initialised only in barebox_arm_entry() which
>> is called later. So I decided that a two instructions assembly loop was
>> the simplest solution.
>
> This may happen because the function gets too complex and gcc starts
> using the stack in this case.
>
> Try rewriting the lowlevel stuff as:
>
> static void __noinline mx31_moboard_startup(void)
> {
> 	/* Put setup here */
> }
>
> void __bare_init __naked barebox_arm_reset_vector(void)
> {
> 	 arm_cpu_lowlevel_init();
>
> 	 arm_setup_stack(MX31_IRAM_BASE_ADDR + MX31_IRAM_SIZE - 12);
>
> 	 mx31_moboard_startup();
> }
>
> With this you can use the stack and should be on the safe side.

Ok, I tried and it works fine so I will do this.
BTW, any reason for the -12, why not -8 ?

Thanks,

Philippe



_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox