[U-Boot] [PATCH] Introduce a new linker flag LDFLAGS_FINAL

[Scott Wood]

On Mon, 31 Jan 2011 20:33:09 +0100
Wolfgang Denk <wd at denx.de> wrote:

> Dear Haiying.Wang at freescale.com,
> 
> In message <1296498767-26408-1-git-send-email-Haiying.Wang at freescale.com> you wrote:
> > From: Haiying Wang <Haiying.Wang at freescale.com>
> > 
> > commit 8aba9dceebb14144e07d19593111ee3a999c37fc
> > Divides variable of linker flags to LDFLAGS-u-boot and LDFLAGS
> > 
> > breaks the usage of --gc-section to build nand_spl. We still need linker option
> > --gc-section for every uboot image, not only the main one. LDFLAGS_FINAL passes
> > the --gc-sections to each uboot image.
> 
> If I understand the intention of the LDFLAGS_u-boot setting
> corrrectly, then you would have to add a "LDFLAGS_nand_spl" setting.
> 
> If you introduce a new LDFLAGS_FINAL instead, then why do we have to
> keep LDFLAGS_u-boot - isn't LDFLAGS_u-boot also for "final" linking of
> the U-Boot image?
> 
> [Btw: "final" is probably not a technically correct term for all the
> use cases I see below.]

I meant final as compared to partial links, not anything to do with spl
versus tpl versus the main image.

Do you have a better wording?

> > diff --git a/config.mk b/config.mk
> > index 5147c35..caa6221 100644
> > --- a/config.mk
> > +++ b/config.mk
> > @@ -205,8 +205,9 @@ endif
> >  AFLAGS := $(AFLAGS_DEBUG) -D__ASSEMBLY__ $(CPPFLAGS)
> >  
> >  LDFLAGS += $(PLATFORM_LDFLAGS)
> > +LDFLAGS_FINAL += -Bstatic $(LDFLAGS)
> >  
> > -LDFLAGS_u-boot += -Bstatic -T $(obj)u-boot.lds $(PLATFORM_LDFLAGS)
> > +LDFLAGS_u-boot += -T $(obj)u-boot.lds $(LDFLAGS_FINAL)
> 
> Is it intentional that you change PLATFORM_LDFLAGS into LDFLAGS here?
>
> Are you sure that this change is correct for all affected boards?
> 
> How has this change been tested?

As I understand it, it has only been limited to PLATFORM_LDFLAGS since
the LDFLAGS_u-boot commit.  Was that change intentional, and widely
tested?

> > -LDFLAGS	= -Bstatic -T $(nandobj)u-boot.lds -Ttext $(CONFIG_SYS_TEXT_BASE) $(PLATFORM_LDFLAGS)
> > +LDFLAGS_spl := -T $(nandobj)u-boot.lds -Ttext $(CONFIG_SYS_TEXT_BASE) $(LDFLAGS_FINAL)
> 
> 
> Arghhh...  Here you introduce yet another setting, LDFLAGS_spl ?
> 
> This is not mentioned in the commit message.  And why do we need it?
> Isn't LDFLAGS_FINAL enough?

No, it's not enough.  The whole point is to separate options which
should apply to all final (non-partial) links, versus options which are
specific to a particular image.

This is the point where we add in the options that are specific to the
spl image.

With :=, we could preserve the LDFLAGS name, but it seemed better to
avoid muddying up what LDFLAGS means, if it's now supposed to mean
options that are used in partial linking -- and it makes things look
more consistent with what's done in the main image.

Or we could not use a variable for this and stick it on the ld command
line, but is that really an improvement?

> Will I soon see patches to also add LDFLAGS_tpl?

Of course. :-)

> This is becoming a mess.  We need to find a simple, clean way to solve
> this.  I'm on the verge of reverting the LDFLAGS_u-boot commit.

What's the alternative solution, when we have some options that need to
be set during partial link, others which cannot be set during
partial link but must be set during final link, and a couple more
(text address and linker script) which are specific to the individual
image?

I think introducing the additional variables makes things less messy,
because it means each variable has a specific, unambiguous purpose.

-Scott