[U-Boot] OMAP (4) boot_params

[Albert ARIBAUD]

Hi Michael,

On Wed, 3 Apr 2013 10:59:23 -0400, Michael Cashwell
<mboards at prograde.net> wrote:

> >>>> ...Making that:
> >>>> 
> >>>> u32 *boot_params_ptr __attribute__ ((section(".data")));
> >> 
> >> Yes, that was my thinking too. Surely clearing data after code has set
> >> it can't be right.
> > 
> > With all due respect, the documentation can with greater legitimately
> > turn your admonition around and ask that you please refrain from
> > setting BSS or data variables when the C runtime environment has not
> > been set. :)
> > 
> > IOW, what is wrong here is writing to a BSS variable before you're
> > allowed to as per the rules under which your code is running.
> 
> I think we're in agreement, but it's not my code doing it. The code,
> as it exists in mainline is writing early to space in bss. My change
> avoids that by moving the variable from the default bss to data:

... except, as I said above, at this point your code should not write at
all, be int in BSS or data, until the C environment is set up. So...

> diff --git a/common/spl/spl.c b/common/spl/spl.c
> index 6715e0d..1d84535
> --- a/common/spl/spl.c
> +++ b/common/spl/spl.c
> @@ -42,7 +42,7 @@ DECLARE_GLOBAL_DATA_PTR;
>  #define CONFIG_SYS_MONITOR_LEN (200 * 1024)
>  #endif
>  
> -u32 *boot_params_ptr = NULL;
> +u32 *boot_params_ptr __attribute__ ((section(".data")));
>  struct spl_image_info spl_image;

... NAK. Place this in a fixed section that you'll map somewhere else
then in BSS or data.

Also: in the future, avoid pasting a diff directly in a mail to the
u-boot list if it is not a real patch submission, as our patchwork
instance at (http://patchwork.ozlabs.org/project/uboot/list/) will get
confused and record your mail as a legitimate patch.

>  /* Define board data structure */
> 
> >> OK, here we have an unfortunate name overloading. The boot_params here
> >> is specifically an OMAP handoff from the CPU's internal boot ROM to SPL
> >> and then from SPL to u-boot. (The same code paths are involved.) It's
> >> totally unrelated to the the boot_params passed to the Linux kernel.
> >> 
> >> Since it's confusing maybe a renaming is called for as well.
> > 
> > Indeed. Plus, if it is shared data, it should definitely be mapped at a
> > fixed memory location or copied from stage to stage (the latter only if
> > the former is impossible)
> 
> Yes, I'm exploring that now. The differences between SPL and U-boot are
> subtle.

Actually not that subtle once you get the hang of it: SPL and U-Boot
are built on the same code base; SPL is the minimal, non-interactive,
early boot stage which can be loaded and run by ROM code, while U-Boot
is the full-featured, interactive, too big to boot directly, stage,
which SPL can chain into.

> Best regards,
> -Mike

Amicalement,
-- 
Albert.