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