[U-Boot] [RFC PATCH 6/7] omap: common spl support for OMAP3/4

[Aneesh V <aneesh@ti.com>]

Hi Albert,

On Sunday 03 July 2011 12:26 PM, Albert ARIBAUD wrote:
> Hi Aneesh,
>
> Le 03/07/2011 06:47, Aneesh V a ?crit :
>> Hi Albert,
>>
>> On Saturday 02 July 2011 01:21 AM, Albert ARIBAUD wrote:
>>> Hi Aneesh,
>>>
>>> Le 01/07/2011 13:48, Aneesh V a ?crit :
>>>> Dear Andreas,
>>>>
>>>> On Friday 01 July 2011 03:25 PM, Andreas Bie?mann wrote:
>>>>> Dear Aneesh,
>>>> [snip ..]
>>>>> But the second part is not clear to me. I saw in your linker, that bss
>>>>> is placed in SDRAM. In start.S the boundaries for clear_bss are
>>>>> calculated at compile time to
>>>>>
>>>>> ---8<---
>>>>> _bss_start_ofs:
>>>>> .word __bss_start - _start
>>>>> --->8---
>>>>>
>>>>> Will that also work with e.g. SDRAM adress space is before SRAM, SDRAM
>>>>> addressing is far away (> 4GiB) ... So in you special case it may
>>>>> work,
>>>>> but if this is a blueprint for SPL on arm(v7) we should consider this.
>>>>>
>>>>
>>>> Nice catch. Actually, in my original OMAP4 series I tried to add
>>>> support for disjoint bss to support my case. But now I realize that it
>>>> works only for non-relocation case and that too only when the bss is at
>>>> higher address compared to .text
>>>>
>>>> Basically disjoint bss is not relocation friendly. So here is what I
>>>> propose:
>>>>
>>>> 1. Modify existing clear_bss sub-routine in start.S to take absolute
>>>> addresses.
>>>> 2. In regular u-boot, calculate the relocated bss address and pass to
>>>> this function.
>>>> 3. In SPL don't try to calculate the relocated address and directly
>>>> pass the absolute address.
>>>>
>>>> If this is fine I will make the necessary changes in start.S in the
>>>> next revision.
>>>
>>> So you would compute the BSS location in board_init_f() and pass that to
>>> relocate_code()?
>>
>> I was thinking of doing that in start.S itself. I haven't looked at
>> all the details though.
>>
>> BTW, please note that I am not trying to support disjoint BSS in
>> regular u-boot. I think it becomes complex with relocation + it doesn't
>> seem to be worth when all SDRAM is at our disposal.
>>
>> So:
>> 1. #ifdef CONFIG_PRELOADER part in start.s will just pass __bss_start
>> and __bss_end to the clear_bss function(assumes no relocation).
>> 2. #else part of above will assume that bss follows text and data(or at
>> least that __bss_start > _start), so add relocation offset to
>> __bss_start and __bss_end, and pass them to the clear_bss()
>>
>> Does that sound ok?
>
> (note clear_bss is not a function in arch/arm/cpu/armv7/start.S, it is
> only a label)
>
> So, considering what is already in arch/arm/cpu/armv7/start.S, you would
> just add a conditional variant to the BSS clearing code for the
> preloader case?
>
> If so, and considering that you'll pass an offset of 0, why would you
> need that variant for? With offset=0, the code already does what you
> want, does it not?
>

Yes, I think the following is all we need:

  clear_bss:
+#ifdef CONFIG_PRELOADER
+	/* No relocation for SPL */
+	ldr	r0, =__bss_start
+	ldr	r1, =__bss_end__
+#else
  	ldr	r0, _bss_start_ofs
  	ldr	r1, _bss_end_ofs
  	mov	r4, r6			/* reloc addr */
  	add	r0, r0, r4
  	add	r1, r1, r4
+#endif

I haven't tested this though.

br,
Aneesh