[U-Boot] OMAP (4) boot_params

[Sricharan R]

Hi Mike Cashwell,

On Thursday 04 April 2013 07:48 PM, Michael Cashwell wrote:
> On Apr 4, 2013, at 1:52 AM, Wolfgang Denk <wd at denx.de> wrote:
> 
>> Dear Tom,
>>
>> On Apr 3, 2013, at 11:34 AM, Albert ARIBAUD <albert.u.boot at aribaud.net> wrote:
>>>> ... except, as I said above, at this point your code should not write at
>>>> all, be it in BSS or data, until the C environment is set up. So...
>>>
>>> But we have to save this ROM-passed information before we overwrite it
>>> ourselves (by accident or purpose).
>>
>> Thete are two official places for data storage before the full C
>> runtime environment is available: the stack, and the "global data"
>> structure.
> 
> I thought there were more levels than just pre and post CRT. Specifically,
> the global_data struct's comment says it is intended to be used "until we
> have set up the memory controller so that we can use RAM."
> 
> To me that suggests once we have RAM any further data storage should go there
> instead of bloating global_data. I thought this distinction was embodied in
> the bss/data section difference with the former being zeroed during CRT init
> and the latter not.
> 
> And I'm clearly not the only one who thought this. The change I proposed in
> common/spl/spl.c that Albert doesn't like:
> 
>> -u32 *boot_params_ptr = NULL;
>> +u32 *boot_params_ptr __attribute__ ((section(".data")));
> 
> is already immediately followed by exactly the same pattern:
> 
>> static bd_t bdata __attribute__ ((section(".data")));
> 
> The only reason I can think of to put bdata explicitly in .data instead of
> the default .bss is so it can avoid the CRT zeroing of .bss.
> 
> If that's wrong then why have both sections? How are they different?
> 
>>> ... after that we can talk about moving things that can't be in the BSS
>>> out of the data section and into another section.
>>
>> Adding another section makes things more complicated, but not really
>> better.
> 
> My proposal does not add another section. The needed section already exists
> and seemingly for precisely the purpose under discussion.
> 
>> If you can provide writable storage, then you could also use
>> it in a more regular way, say for a writable data segment, or bigger
>> stack, or malloc space, or ... so it is generally useful instead of
>> only this special case here.
> 
> This is exactly my concern. I see no justification for a special case.
> If never writing to any linker-defined section (.data or .bss) before CRT
> init really is the design rule then there are quite a few other violations
> that need to be fixed. Rolling an ad hoc solution for each can't be the
> right approach.
> 
  Sorry for the late feedback.
  The **only** reason for passing the boot_params from SPL to U-BOOT was
  when somebody uses a CONFIGURATION HEADER + SPL + U-BOOT, which
  was never a case. But the broken code that you pointed was trying to help 
  such a scenario. I guess nobody would have used this combination.

  save_boot_params ideally should not write in to either .data or .bss.
  Because this would break a XIP kind of a boot. The only place where it can
  write is the GD or some reserved SRAM area that is always 'writable'.
  We did not have a XIP in OMAP4/5 and thus this went unnoticed.

  I will post a patch today to address this.

Regards,
 Sricharan