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

Albert ARIBAUD
2011-07-03T06:56:58Z

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?

> br,
> Aneesh

Amicalement,
--
Albert.