Hi Mike Cashwell,
On Thursday 04 April 2013 07:48 PM, Michael Cashwell wrote:
> On Apr 4, 2013, at 1:52 AM, Wolfgang Denk <w...@denx.de> wrote:
>
>> Dear Tom,
>>
>> On Apr 3, 2013, at 11:34 AM, Albert ARIBAUD <albert.u.b...@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
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
