Hi Benoît,
On Sat, 11 May 2013 02:25:02 +0200 (CEST), Benoît Thébaudeau
<benoit.thebaud...@advansee.com> wrote:
> Hi Albert,
> > diff --git a/arch/arm/cpu/arm1136/start.S b/arch/arm/cpu/arm1136/start.S
> > index ccea2d5..ab8fd56 100644
> > --- a/arch/arm/cpu/arm1136/start.S
> > +++ b/arch/arm/cpu/arm1136/start.S
> > @@ -104,10 +104,6 @@ _TEXT_BASE:
> > _bss_start_ofs:
> > .word __bss_start - _start
> >
> > -.globl _image_copy_end_ofs
>
> Wasn't _image_copy_end_ofs used outside of start.S? Same question for all the
> start.S files.
No -- and the build would have failed if it was. The only user of
__image_copy_end_ofs is the start.S which defines it, and only for the
purpose of preventing the assembler from generating an R_ARM_ABS32
relocation to __image_copy_end.
> > diff --git a/arch/arm/cpu/arm1136/u-boot-spl.lds
> > b/arch/arm/cpu/arm1136/u-boot-spl.lds
> > index 8296e5d..04fc881 100644
> > --- a/arch/arm/cpu/arm1136/u-boot-spl.lds
> > +++ b/arch/arm/cpu/arm1136/u-boot-spl.lds
> > @@ -37,7 +37,6 @@ SECTIONS
> > {
> > .text :
> > {
> > - __start = .;
> > arch/arm/cpu/arm1136/start.o (.text*)
> > *(.text*)
> > } >.sram
> > @@ -48,7 +47,9 @@ SECTIONS
> > . = ALIGN(4);
> > .data : { *(SORT_BY_ALIGNMENT(.data*)) } >.sram
> > . = ALIGN(4);
> > +
> > __image_copy_end = .;
>
> Why aren't all linker scripts treated equally?
>
> Here, start.S is still used, so '*(.__image_copy_end)' and the related stuff
> should be like what you did for arch/arm/cpu/u-boot.lds below. Or am I missing
> something?
>
> Same question for several other linker scripts below.
Not all SPLs use relocation -- actually, most SPLs do not use
relocation, and thus do not need image and relocaton section symbols.
> > diff --git a/arch/arm/cpu/u-boot.lds b/arch/arm/cpu/u-boot.lds
> > index d9bbee3..5b43621 100644
> > --- a/arch/arm/cpu/u-boot.lds
> > +++ b/arch/arm/cpu/u-boot.lds
> > @@ -33,7 +33,7 @@ SECTIONS
> > . = ALIGN(4);
> > .text :
> > {
> > - __image_copy_start = .;
> > + *(.__image_copy_start)
>
> Are there any users of __image_copy_start?
(see below)
> > diff --git a/arch/arm/lib/sections.c b/arch/arm/lib/sections.c
> > index 99eda59..80a0c38 100644
> > --- a/arch/arm/lib/sections.c
> +++ b/arch/arm/lib/sections.c
> > @@ -37,3 +37,5 @@
> >
> > char __bss_start[0] __attribute__((used, section(".__bss_start")));
> > char __bss_end[0] __attribute__((used, section(".__bss_end")));
> > +char __image_copy_start[0] __attribute__((used,
> > section(".__image_copy_start")));
>
> Ditto.
The only user of __image_copy_start is the relocation routine which
uses _start but should actually use __image_copy_start (will fix in V2),
to match with the semantics introduced when fixing CONFIG_SPL_MAX_SIZE
semantics in 6ebc3461.
> Best regards,
> Benoît
Thanks for your review!
Amicalement,
--
Albert.
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
