Patchwork [U-Boot] Add LDFLAGS-u-boot variable and move some linker option to this

login
register
mail settings
Submitter Nobuhiro Iwamatsu
Date Dec. 27, 2010, 7 a.m.
Message ID <1293433224-17341-1-git-send-email-iwamatsu@nigauri.org>
Download mbox | patch
Permalink /patch/76758/
State Changes Requested
Headers show

Comments

Nobuhiro Iwamatsu - Dec. 27, 2010, 7 a.m.
This move linker option used by the last of u-boot in LDFLAGS_u-boot variable.
And the option to use in ld uses LDFLAGS variable.

Signed-off-by: Nobuhiro Iwamatsu <iwamatsu@nigauri.org>
---
 Makefile                  |    2 +-
 arch/blackfin/config.mk   |    3 ++-
 arch/i386/config.mk       |    3 ++-
 arch/nios2/config.mk      |    2 +-
 arch/powerpc/config.mk    |    4 ++--
 arch/sh/config.mk         |    2 +-
 arch/sh/cpu/sh2/config.mk |    4 +++-
 config.mk                 |    8 +++++---
 8 files changed, 17 insertions(+), 11 deletions(-)
Wolfgang Denk - Dec. 27, 2010, 10:47 a.m.
Dear Nobuhiro Iwamatsu,

In message <1293433224-17341-1-git-send-email-iwamatsu@nigauri.org> you wrote:
> This move linker option used by the last of u-boot in LDFLAGS_u-boot variable.
> And the option to use in ld uses LDFLAGS variable.

Can you please explain why this would be needed?

> -LDFLAGS += --gc-sections -m elf32bfin
> +LDFLAGS_u-boot += --gc-sections

The name "LDFLAGS_u-boot" may actually work here, but iut is
inconsistent with other such variables names, and using '-' in a name
is probably not a good idea either.



Best regards,

Wolfgang Denk
Mike Frysinger - Dec. 27, 2010, 4:46 p.m.
On Monday, December 27, 2010 05:47:54 Wolfgang Denk wrote:
> Nobuhiro Iwamatsu wrote:
> > This move linker option used by the last of u-boot in LDFLAGS_u-boot
> > variable. And the option to use in ld uses LDFLAGS variable.
> 
> Can you please explain why this would be needed?

he explained in the previous thread why we need to split things.  there are 
flags that are needed for all linker options and there are flags needed just 
for the final u-boot link.

> > -LDFLAGS += --gc-sections -m elf32bfin
> > +LDFLAGS_u-boot += --gc-sections
> 
> The name "LDFLAGS_u-boot" may actually work here, but iut is
> inconsistent with other such variables names, and using '-' in a name
> is probably not a good idea either.

it isnt inconsistent.  the convention is $(XFLAGS_$(@F)) and we just happen to 
be creating a file here which is named "u-boot".  dashes in variable names 
isnt a problem for make.  otherwise you're basically banning creating files in 
the u-boot tree with dashes in their filenames.
-mike
Wolfgang Denk - Dec. 27, 2010, 6:58 p.m.
Dear Mike Frysinger,

In message <201012271146.22205.vapier@gentoo.org> you wrote:
>
> > Nobuhiro Iwamatsu wrote:
> > > This move linker option used by the last of u-boot in LDFLAGS_u-boot
> > > variable. And the option to use in ld uses LDFLAGS variable.
> > 
> > Can you please explain why this would be needed?
>
> he explained in the previous thread why we need to split things.  there are 
> flags that are needed for all linker options and there are flags needed just 
> for the final u-boot link.

Such an explanation belongs into the commit message.

> > > -LDFLAGS += --gc-sections -m elf32bfin
> > > +LDFLAGS_u-boot += --gc-sections
> > 
> > The name "LDFLAGS_u-boot" may actually work here, but iut is
> > inconsistent with other such variables names, and using '-' in a name
> > is probably not a good idea either.
>
> it isnt inconsistent.  the convention is $(XFLAGS_$(@F)) and we just happen to 

Can you please point me to an example where this has been used in
U-Boot before?  Or why do you call this a convention?

I fail to see a reason this is needed or even useful here.

Best regards,

Wolfgang Denk
Mike Frysinger - Dec. 27, 2010, 8:11 p.m.
On Monday, December 27, 2010 13:58:08 Wolfgang Denk wrote:
> Mike Frysinger wrote:
> > > Nobuhiro Iwamatsu wrote:
> > > > This move linker option used by the last of u-boot in LDFLAGS_u-boot
> > > > variable. And the option to use in ld uses LDFLAGS variable.
> > > 
> > > Can you please explain why this would be needed?
> > 
> > he explained in the previous thread why we need to split things.  there
> > are flags that are needed for all linker options and there are flags
> > needed just for the final u-boot link.
> 
> Such an explanation belongs into the commit message.

yes, but that isnt what you asked

> > > > -LDFLAGS += --gc-sections -m elf32bfin
> > > > +LDFLAGS_u-boot += --gc-sections
> > > 
> > > The name "LDFLAGS_u-boot" may actually work here, but iut is
> > > inconsistent with other such variables names, and using '-' in a name
> > > is probably not a good idea either.
> > 
> > it isnt inconsistent.  the convention is $(XFLAGS_$(@F)) and we just
> > happen to
> 
> Can you please point me to an example where this has been used in
> U-Boot before?  Or why do you call this a convention?

(1) it's what's used in Linux and every build system based on that (kbuild)
(2) u-boot is slowly moving to the conventions already in use by Linux
(3) u-boot already uses this specific convention for every .c/.s/.S file -- 
simply look at the bottom of config.mk

it makes perfect sense to keep the existing syntax and extend LDFLAGS behavior 
to it rather than coming up with some new specific variable that only applies 
to the final link of u-boot.  otherwise every other final link we have in u-
boot will need its own random style (examples, standalone, spl, ...).
-mike
Wolfgang Denk - Dec. 27, 2010, 11:23 p.m.
Dear Mike Frysinger,

In message <201012271511.54110.vapier@gentoo.org> you wrote:
>
> > > > Nobuhiro Iwamatsu wrote:
> > > > > This move linker option used by the last of u-boot in LDFLAGS_u-boot
> > > > > variable. And the option to use in ld uses LDFLAGS variable.
> > > > 
> > > > Can you please explain why this would be needed?
> > > 
> > > he explained in the previous thread why we need to split things.  there
> > > are flags that are needed for all linker options and there are flags
> > > needed just for the final u-boot link.
> > 
> > Such an explanation belongs into the commit message.
>
> yes, but that isnt what you asked

This is just your quibblerish interpretation.

> > > it isnt inconsistent.  the convention is $(XFLAGS_$(@F)) and we just
> > > happen to
> > 
> > Can you please point me to an example where this has been used in
> > U-Boot before?  Or why do you call this a convention?
>
> (1) it's what's used in Linux and every build system based on that (kbuild)

So let's out on record that this is NOT an accepted convention in U-Boot.

> (2) u-boot is slowly moving to the conventions already in use by Linux

Oh, is it?

> (3) u-boot already uses this specific convention for every .c/.s/.S file -->  
> simply look at the bottom of config.mk

Well, applying your strict interpretation I see this:

...
258 #########################################################################
259
260 # If the list of objects to link is empty, just create an empty built-in.o
261 cmd_link_o_target = $(if $(strip $1),\
262                       $(LD) -r -o $@ $1 ,\
263                       rm -f $@; $(AR) rcs $@ )
264
265 #########################################################################

And I see no trace of any $(XFLAGS_$(@F)) there.

Actually I see no trace of any $(XFLAGS_$(@F)) anywhere; the closest we have is this:

245 ALL_AFLAGS = $(AFLAGS) $(AFLAGS_$(BCURDIR)/$(@F)) $(AFLAGS_$(BCURDIR))
246 ALL_CFLAGS = $(CFLAGS) $(CFLAGS_$(BCURDIR)/$(@F)) $(CFLAGS_$(BCURDIR))


> it makes perfect sense to keep the existing syntax and extend LDFLAGS behavior 
> to it rather than coming up with some new specific variable that only applies 
> to the final link of u-boot.  otherwise every other final link we have in u-
> boot will need its own random style (examples, standalone, spl, ...).

Maybe you a are even right. But you might find it easier to get your
arguments considered if you'd use a more constructive way to present
it.


And in any way, my statementt was and is that such explanations must
be included with the commit message.   Period.

Best regards,

Wolfgang Denk
Mike Frysinger - Dec. 28, 2010, 1 a.m.
On Monday, December 27, 2010 18:23:17 Wolfgang Denk wrote:
> And I see no trace of any $(XFLAGS_$(@F)) there.

it's fairly obvious that the "X" is for you to replace as a matching char
-mike
Mike Frysinger - Dec. 30, 2010, 2:02 a.m.
On Monday, December 27, 2010 02:00:24 Nobuhiro Iwamatsu wrote:
> This move linker option used by the last of u-boot in LDFLAGS_u-boot
> variable. And the option to use in ld uses LDFLAGS variable.

Nobuhiro: could you repost with an updated changelog please ?  talk about how 
the the linker needs to use the proper endian/bfd flags even when doing 
partial linking.
-mike
Nobuhiro Iwamatsu - Jan. 5, 2011, 6:52 a.m.
Hi,
On Wed, Dec 29, 2010 at 09:02:33PM -0500, Mike Frysinger wrote:
> On Monday, December 27, 2010 02:00:24 Nobuhiro Iwamatsu wrote:
> > This move linker option used by the last of u-boot in LDFLAGS_u-boot
> > variable. And the option to use in ld uses LDFLAGS variable.
> 
> Nobuhiro: could you repost with an updated changelog please ?  talk about how 
> the the linker needs to use the proper endian/bfd flags even when doing 
> partial linking.

Sorry, I had long vacation.
I am going to report new patch.

Thanks,
  Nobuhiro

Patch

diff --git a/Makefile b/Makefile
index 9055028..d6884ea 100644
--- a/Makefile
+++ b/Makefile
@@ -369,7 +369,7 @@  $(obj)u-boot.dis:	$(obj)u-boot
 GEN_UBOOT = \
 		UNDEF_SYM=`$(OBJDUMP) -x $(LIBBOARD) $(LIBS) | \
 		sed  -n -e 's/.*\($(SYM_PREFIX)__u_boot_cmd_.*\)/-u\1/p'|sort|uniq`;\
-		cd $(LNDIR) && $(LD) $(LDFLAGS) $$UNDEF_SYM $(__OBJS) \
+		cd $(LNDIR) && $(LD) $(LDFLAGS) $(LDFLAGS_$(@F)) $$UNDEF_SYM $(__OBJS) \
 			--start-group $(__LIBS) --end-group $(PLATFORM_LIBS) \
 			-Map u-boot.map -o u-boot
 $(obj)u-boot:	depend \
diff --git a/arch/blackfin/config.mk b/arch/blackfin/config.mk
index ab117ca..0cba294 100644
--- a/arch/blackfin/config.mk
+++ b/arch/blackfin/config.mk
@@ -30,7 +30,8 @@  CONFIG_BFIN_BOOT_MODE := $(strip $(subst ",,$(CONFIG_BFIN_BOOT_MODE)))
 PLATFORM_RELFLAGS += -ffixed-P3 -fomit-frame-pointer -mno-fdpic
 PLATFORM_CPPFLAGS += -DCONFIG_BLACKFIN
 
-LDFLAGS += --gc-sections -m elf32bfin
+LDFLAGS_u-boot += --gc-sections
+LDFLAGS += -m elf32bfin
 PLATFORM_RELFLAGS += -ffunction-sections -fdata-sections
 
 PLATFORM_CPPFLAGS += -DBFIN_CPU='"$(CONFIG_BFIN_CPU)"'
diff --git a/arch/i386/config.mk b/arch/i386/config.mk
index 8743f1a..2da1666 100644
--- a/arch/i386/config.mk
+++ b/arch/i386/config.mk
@@ -35,5 +35,6 @@  PLATFORM_CPPFLAGS += $(call cc-option, -fno-stack-protector)
 PLATFORM_CPPFLAGS += $(call cc-option, -mpreferred-stack-boundary=2)
 PLATFORM_CPPFLAGS += -DCONFIG_I386 -D__I386__
 
-LDFLAGS += --cref --gc-sections
+LDFLAGS += --cref 
+LDFLAGS_u-boot += --gc-sections
 PLATFORM_RELFLAGS += -ffunction-sections
diff --git a/arch/nios2/config.mk b/arch/nios2/config.mk
index aba96b3..fa93180 100644
--- a/arch/nios2/config.mk
+++ b/arch/nios2/config.mk
@@ -31,5 +31,5 @@  PLATFORM_CPPFLAGS += -G0
 
 LDSCRIPT ?= $(SRCTREE)/$(CPUDIR)/u-boot.lds
 
-LDFLAGS += --gc-sections
+LDFLAGS_u-boot += --gc-sections
 PLATFORM_RELFLAGS += -ffunction-sections -fdata-sections
diff --git a/arch/powerpc/config.mk b/arch/powerpc/config.mk
index 2912604..64191c7 100644
--- a/arch/powerpc/config.mk
+++ b/arch/powerpc/config.mk
@@ -24,10 +24,10 @@ 
 CROSS_COMPILE ?= ppc_8xx-
 
 STANDALONE_LOAD_ADDR = 0x40000
-
+LDFLAGS_u-boot = --gc-sections
 PLATFORM_RELFLAGS += -mrelocatable -ffunction-sections -fdata-sections
 PLATFORM_CPPFLAGS += -DCONFIG_PPC -D__powerpc__
-PLATFORM_LDFLAGS  += -n --gc-sections
+PLATFORM_LDFLAGS  += -n
 
 ifdef CONFIG_SYS_LDSCRIPT
 # need to strip off double quotes
diff --git a/arch/sh/config.mk b/arch/sh/config.mk
index 415c949..4ef85e3 100644
--- a/arch/sh/config.mk
+++ b/arch/sh/config.mk
@@ -30,5 +30,5 @@  endif
 
 PLATFORM_CPPFLAGS += -DCONFIG_SH -D__SH__
 PLATFORM_LDFLAGS += -e $(CONFIG_SYS_TEXT_BASE) --defsym reloc_dst=$(CONFIG_SYS_TEXT_BASE)
-
+LDFLAGS_u-boot = --gc-sections
 LDSCRIPT := $(SRCTREE)/$(CPUDIR)/u-boot.lds
diff --git a/arch/sh/cpu/sh2/config.mk b/arch/sh/cpu/sh2/config.mk
index 52d5a0f..f2d40aa 100644
--- a/arch/sh/cpu/sh2/config.mk
+++ b/arch/sh/cpu/sh2/config.mk
@@ -21,6 +21,8 @@ 
 # MA 02111-1307 USA
 #
 #
+ENDIANNESS += -EB
+
 PLATFORM_CPPFLAGS += -m3e -mb
 PLATFORM_RELFLAGS += -ffixed-r13
-PLATFORM_LDFLAGS += -EB
+PLATFORM_LDFLAGS += $(ENDIANNESS)
diff --git a/config.mk b/config.mk
index c6d6f7b..1666a48 100644
--- a/config.mk
+++ b/config.mk
@@ -204,9 +204,11 @@  endif
 
 AFLAGS := $(AFLAGS_DEBUG) -D__ASSEMBLY__ $(CPPFLAGS)
 
-LDFLAGS += -Bstatic -T $(obj)u-boot.lds $(PLATFORM_LDFLAGS)
+LDFLAGS += $(PLATFORM_LDFLAGS)
+
+LDFLAGS_u-boot += -Bstatic -T $(obj)u-boot.lds $(PLATFORM_LDFLAGS)
 ifneq ($(CONFIG_SYS_TEXT_BASE),)
-LDFLAGS += -Ttext $(CONFIG_SYS_TEXT_BASE)
+LDFLAGS_u-boot += -Ttext $(CONFIG_SYS_TEXT_BASE)
 endif
 
 # Location of a usable BFD library, where we define "usable" as
@@ -262,7 +264,7 @@  $(obj)%.s:	%.c
 
 # If the list of objects to link is empty, just create an empty built-in.o
 cmd_link_o_target = $(if $(strip $1),\
-		      $(LD) -r -o $@ $1 ,\
+		      $(LD) $(LDFLAGS) -r -o $@ $1,\
 		      rm -f $@; $(AR) rcs $@ )
 
 #########################################################################