[U-Boot] [PATCH 2/4] arm: make __image_copy_{start, end} compiler-generated

Albert ARIBAUD
2013-05-11T08:02:48Z

Hi Beno?t,

On Sat, 11 May 2013 02:25:02 +0200 (CEST), Beno?t Th?baudeau
<benoit.thebaudeau@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.