[U-Boot] [PATCH v9 18/30] nand: mxc: Switch NAND SPL to generic SPL

[Albert ARIBAUD <albert.u.boot@aribaud.net>]

On Wed, 3 Apr 2013 10:05:57 +0200, Albert ARIBAUD
<albert.u.boot@aribaud.net> wrote:

> > The resulting HEAD is identical to yours, except for
> > board/freescale/mx31ads/u-boot.lds in which you had removed the following
> > unrelated line for 30/30:
> >         __bss_end = .;
> > 
> > Regarding this line, it is also present in a few other .lds, as well as the
> > following ones:
> >         KEEP(*(.__bss_start));
> >         KEEP(*(__bss_end));
> > 
> > However, the end section is named .__bss_end in arch/arm/lib/bss.c, so there is
> > perhaps something wrong here, unless you did that on purpose because of the
> > __bss_end test at the end of the linker script. I had noticed that, but I let
> > you handle it. If something needs to be changed here, please do it after my
> > series if possible in order to avoid a v11 because of newer rebase conflicts. ;)
> 
> Normally, defining __bss_end symbols in the ARM lds files has become 
> unnecessary as arch/arm/lib/bss.c now defines then (and in so doing
> avoids generating ugly R_ARM_ABS32 relocation records) so yes, the line
> removal was intentional, but indeed unrelated to your series.
> 
> As for the missing period in the .__bss_end input section specification,
> thanks for bringing this to my attention. I'll give it a look at once
> as this could lead to tricky bugs popping up...

Alright, now that the coffee shot has reached full effect, I think I
understand what is going on and indeed that has to be fixed although it
most certainly did not cause any issue.

In my commit 3ebd1cbc, where the whole bss.c system was put in place,
the .__bss_end__ input section specification had the leading period and
was thus placed correctly, as was the compiler-generated __bss_end__
symbol from bss.c.

Later, when Tom's commit 0ce033d2 manually merged Simon's work turning
all __bss_end__ occurrences into __bss_end, two things happened:

1) both the bss.c file and ARM *.lds files then defined __bss_end.

2) the leading period in .__bss_end__ was lost in the conversion.

This led to the compiler-generated __bss_end section being mapped away
from the whole BSS area -- *before* it, actually, which would have been
catastrophic... if the definition for __bss_end from the lds files had
not taken precedence over that from bss.c, thus preserving the correct
value and semantics for the symbol.

In other words, the first issue actually countered the second one, and
both stayed hidden as the code continued running as expected.

But this has to be fixed, because it causes __bss_end references to
generate R_ARM_ABS32 relocation records again, which must be averted
(it might be a good idea to add a build-time check to avoid any new
R_ARM_ABS32 from creeping up in the future).

I'll send out a patch to be taken in 2013.04, in which I'll reintroduce
the leading periods in .__bss_end sections, and rename the linker
symbols used for overlay mapping (and make them non-exported so that
the code won't mistakenly reference them).

... and indeed I won't ask for a V10 rebase for it :) -- either V10
gets in first, or if my patch gets in first, I'll handle rebasing
V10 myself.

Thanks Beno?t for spotting this!

Amicalement,
-- 
Albert.