Patchwork [U-Boot,1/2,for,2010.12] Add support PLATFORM_LDFLAGS to cmd_link_o_target

login
register
mail settings
Submitter Nobuhiro Iwamatsu
Date Dec. 25, 2010, 11:02 p.m.
Message ID <1293318147-15653-1-git-send-email-iwamatsu@nigauri.org>
Download mbox | patch
Permalink /patch/76709/
State Changes Requested
Headers show

Comments

Nobuhiro Iwamatsu - Dec. 25, 2010, 11:02 p.m.
Current cmd_link_o_target function in config.mk does not support the set
of the endian.

Some architecture is bi-endian (e.g. mips and sh).
Therefore, there is case supporting big endian and little endian
with one toolchain.
For example, when user builds target of big endian in host of little endian,
they need set endian.

Signed-off-by: Nobuhiro Iwamatsu <iwamatsu@nigauri.org>
---
 config.mk |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
Mike Frysinger - Dec. 26, 2010, 6:54 p.m.
On Saturday, December 25, 2010 18:02:26 Nobuhiro Iwamatsu wrote:
> Current cmd_link_o_target function in config.mk does not support the set
> of the endian.
> 
> Some architecture is bi-endian (e.g. mips and sh).
> Therefore, there is case supporting big endian and little endian
> with one toolchain.
> For example, when user builds target of big endian in host of little
> endian, they need set endian.
> 
>  # 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) $(ENDIANNESS) -r -o $@ $1 ,\

i dont think we should start declaring random new variables with specific 
purposes.  better to split the "u-boot final" LDFLAGS out into their own 
variable (LDFLAGS_u-boot) and keep LDFLAGS as a "these are the flags that need 
to be used with $(LD)".
-mike
Nobuhiro Iwamatsu - Dec. 27, 2010, 4:17 a.m.
Hi, mike.

2010/12/27 Mike Frysinger <vapier@gentoo.org>:
> On Saturday, December 25, 2010 18:02:26 Nobuhiro Iwamatsu wrote:
>> Current cmd_link_o_target function in config.mk does not support the set
>> of the endian.
>>
>> Some architecture is bi-endian (e.g. mips and sh).
>> Therefore, there is case supporting big endian and little endian
>> with one toolchain.
>> For example, when user builds target of big endian in host of little
>> endian, they need set endian.
>>
>>  # 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) $(ENDIANNESS) -r -o $@ $1 ,\
>
> i dont think we should start declaring random new variables with specific
> purposes.

I agree. But....

> better to split the "u-boot final" LDFLAGS out into their own
> variable (LDFLAGS_u-boot) and keep LDFLAGS as a "these are the flags that need
> to be used with $(LD)".

cmd_link_o_target is not used in the last of u-boot  (u-boot final) .
But this is used in the middle of build.
For example, when we make libstubs.o in examples/standalone/Makefile.

: examples/standalone/Makefile
-----
 89
 90 all:    $(obj).depend $(OBJS) $(LIB) $(SREC) $(BIN) $(ELF)
 91
 92 #########################################################################
 93 $(LIB): $(obj).depend $(LIBOBJS)
 94     $(call cmd_link_o_target, $(LIBOBJS))
 95
 96 $(ELF):
-----

Therefore, I think that we have to add a new variable to this.

Best regards,
  Nobuhiro
Mike Frysinger - Dec. 27, 2010, 4:51 a.m.
On Sunday, December 26, 2010 23:17:39 Nobuhiro Iwamatsu wrote:
> 2010/12/27 Mike Frysinger <vapier@gentoo.org>:
> > On Saturday, December 25, 2010 18:02:26 Nobuhiro Iwamatsu wrote:
> >> Current cmd_link_o_target function in config.mk does not support the set
> >> of the endian.
> >> 
> >> Some architecture is bi-endian (e.g. mips and sh).
> >> Therefore, there is case supporting big endian and little endian
> >> with one toolchain.
> >> For example, when user builds target of big endian in host of little
> >> endian, they need set endian.
> >> 
> >>  # 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) $(ENDIANNESS) -r -o $@ $1 ,\
> > 
> > i dont think we should start declaring random new variables with specific
> > purposes.
> 
> I agree. But....
> 
> > better to split the "u-boot final" LDFLAGS out into their own
> > variable (LDFLAGS_u-boot) and keep LDFLAGS as a "these are the flags that
> > need to be used with $(LD)".
> 
> cmd_link_o_target is not used in the last of u-boot  (u-boot final) .

this is irrelevant to what i suggested

> But this is used in the middle of build.
> For example, when we make libstubs.o in examples/standalone/Makefile.
> 
> : examples/standalone/Makefile
> 
> -----
>  89
>  90 all:    $(obj).depend $(OBJS) $(LIB) $(SREC) $(BIN) $(ELF)
>  91
>  92
> #########################################################################
> 93 $(LIB): $(obj).depend $(LIBOBJS)
>  94     $(call cmd_link_o_target, $(LIBOBJS))
>  95
>  96 $(ELF):
> -----
> 
> Therefore, I think that we have to add a new variable to this.

no, we dont.  do as i suggested:
 - add $(LDFLAGS_$(@F)) to Makefile:GEN_UBOOT
 - move -T/-B flags from Makefile:LDFLAGS to Makefile:LDFLAGS_u-boot
 - move --gc-sections from LDFLAGS to LDFLAGS_u-boot in arch/*/config.mk
 - add $(LDFLAGS) to cmd_link_o_target

your patch doesnt scale to address all problems
-mike
Nobuhiro Iwamatsu - Dec. 27, 2010, 6:29 a.m.
2010/12/27 Mike Frysinger <vapier@gentoo.org>:
> On Sunday, December 26, 2010 23:17:39 Nobuhiro Iwamatsu wrote:
>> 2010/12/27 Mike Frysinger <vapier@gentoo.org>:
>> > On Saturday, December 25, 2010 18:02:26 Nobuhiro Iwamatsu wrote:
>> >> Current cmd_link_o_target function in config.mk does not support the set
>> >> of the endian.
>> >>
>> >> Some architecture is bi-endian (e.g. mips and sh).
>> >> Therefore, there is case supporting big endian and little endian
>> >> with one toolchain.
>> >> For example, when user builds target of big endian in host of little
>> >> endian, they need set endian.
>> >>
>> >>  # 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) $(ENDIANNESS) -r -o $@ $1 ,\
>> >
>> > i dont think we should start declaring random new variables with specific
>> > purposes.
>>
>> I agree. But....
>>
>> > better to split the "u-boot final" LDFLAGS out into their own
>> > variable (LDFLAGS_u-boot) and keep LDFLAGS as a "these are the flags that
>> > need to be used with $(LD)".
>>
>> cmd_link_o_target is not used in the last of u-boot  (u-boot final) .
>
> this is irrelevant to what i suggested
>
>> But this is used in the middle of build.
>> For example, when we make libstubs.o in examples/standalone/Makefile.
>>
>> : examples/standalone/Makefile
>>
>> -----
>>  89
>>  90 all:    $(obj).depend $(OBJS) $(LIB) $(SREC) $(BIN) $(ELF)
>>  91
>>  92
>> #########################################################################
>> 93 $(LIB): $(obj).depend $(LIBOBJS)
>>  94     $(call cmd_link_o_target, $(LIBOBJS))
>>  95
>>  96 $(ELF):
>> -----
>>
>> Therefore, I think that we have to add a new variable to this.
>
> no, we dont.  do as i suggested:
>  - add $(LDFLAGS_$(@F)) to Makefile:GEN_UBOOT
>  - move -T/-B flags from Makefile:LDFLAGS to Makefile:LDFLAGS_u-boot
>  - move --gc-sections from LDFLAGS to LDFLAGS_u-boot in arch/*/config.mk
>  - add $(LDFLAGS) to cmd_link_o_target
>
> your patch doesnt scale to address all problems

OK, I understood your indication.
I will send new patch.

Thanks!

Nobuhiro

Patch

diff --git a/config.mk b/config.mk
index c6d6f7b..bbcefbe 100644
--- a/config.mk
+++ b/config.mk
@@ -262,7 +262,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) $(ENDIANNESS) -r -o $@ $1 ,\
 		      rm -f $@; $(AR) rcs $@ )
 
 #########################################################################