diff mbox

[U-Boot] MIPS: fix endianess handling

Message ID 1322487768-19439-1-git-send-email-daniel.schwierzeck@googlemail.com
State Superseded
Headers show

Commit Message

Daniel Schwierzeck Nov. 28, 2011, 1:42 p.m. UTC
Make endianess of target CPU configurable. Use the new config
option for dbau1550_el and pb1000 boards.

Adapt linking of standalone applications to pass through
endianess options to LD.

Build tested with:
 - ELDK 4 mips_4KC- and mips4KCle
 - Sourcery CodeBench Lite 2011.03-93

Signed-off-by: Daniel Schwierzeck <daniel.schwierzeck@googlemail.com>
Cc: Thomas Lange <thomas@corelatus.se>
Cc: Mike Frysinger <vapier@gentoo.org>
Cc: Wolfgang Denk <wd@denx.de>
---
Another try to finally fix this originated by discussion:
[1] http://thread.gmane.org/gmane.comp.boot-loaders.u-boot/118111/focus=118122

Related discussions:
[2] http://thread.gmane.org/gmane.comp.boot-loaders.u-boot/81572
[3] http://thread.gmane.org/gmane.comp.boot-loaders.u-boot/45404

 README                         |    6 ++++++
 arch/mips/cpu/mips32/config.mk |   21 +++++++++++++++------
 boards.cfg                     |    4 ++--
 examples/standalone/Makefile   |    6 +++++-
 4 files changed, 28 insertions(+), 9 deletions(-)

Comments

Shinya Kuribayashi Dec. 4, 2011, 2:32 p.m. UTC | #1
On 12/4/11 9:02 PM, Daniel Schwierzeck wrote:
>> diff --git a/boards.cfg b/boards.cfg
>> index c83d861..2cd917e 100644
>> --- a/boards.cfg
>> +++ b/boards.cfg
>> @@ -324,13 +324,13 @@ dbau1000                     mips        mips32      dbau1x00            -
>>    dbau1100                     mips        mips32      dbau1x00            -              au1x00      dbau1x00:DBAU1100
>>    dbau1500                     mips        mips32      dbau1x00            -              au1x00      dbau1x00:DBAU1500
>>    dbau1550                     mips        mips32      dbau1x00            -              au1x00      dbau1x00:DBAU1550
>> -dbau1550_el                  mips        mips32      dbau1x00            -              au1x00      dbau1x00:DBAU1550
>> +dbau1550_el                  mips        mips32      dbau1x00            -              au1x00      dbau1x00:DBAU1550,SYS_LITTLE_ENDIAN

This looks Ok.

>>    gth2                         mips        mips32      -                   -              au1x00
>>    incaip                       mips        mips32      incaip              -              incaip
>>    incaip_100MHz                mips        mips32      incaip              -              incaip      incaip:CPU_CLOCK_RATE=100000000
>>    incaip_133MHz                mips        mips32      incaip              -              incaip      incaip:CPU_CLOCK_RATE=133000000
>>    incaip_150MHz                mips        mips32      incaip              -              incaip      incaip:CPU_CLOCK_RATE=150000000
>> -pb1000                       mips        mips32      pb1x00              -              au1x00      pb1x00:PB1000
>> +pb1000                       mips        mips32      pb1x00              -              au1x00      pb1x00:PB1000,SYS_LITTLE_ENDIAN
>>    qemu_mips                    mips        mips32      qemu-mips           -              -           qemu-mips
>>    tb0229                       mips        mips32
>>    vct_premium                  mips        mips32      vct                 micronas       -           vct:VCT_PREMIUM

But I don't see any reason CONFIG_SYS_LITTLE_ENDIAN is specified in
boards.cfg.  Just putting it in configs/pb1000.h is enough, isn't it?

> Shinya, do you have an opinion about this?

No, if it works for you, I'm fine.  Thanks for tackling this issue.
Does anyone disagree with this change?  If not, I'll pick this up.

>> diff --git a/examples/standalone/Makefile b/examples/standalone/Makefile
>> index e23865b..eab23b4 100644
>> --- a/examples/standalone/Makefile
>> +++ b/examples/standalone/Makefile
>> @@ -88,6 +88,10 @@ endif
>>    CFLAGS_NTR := $(call cc-option,-fno-toplevel-reorder)
>>    CFLAGS += $(CFLAGS_NTR)
>>
>> +# Pass through endianess settings in LDFLAGS to LD

s/endianess/endianness/
Wolfgang Denk Dec. 4, 2011, 3:06 p.m. UTC | #2
Dear Daniel Schwierzeck,

In message <4EDB5F86.40605@googlemail.com> you wrote:
>
> Wolfgang, is there any reason why standalone apps are linked without 
> LDFLAGS (especially PLATFORM_LDFLAGS)?

None that I know, at least.

In general, standalone applications are a rarely used special feature,
and the few people who actually ever used this for a purpose probably
used custom code and custom linker scripts, so the mainline code is
eventually not that good supproted.

> This patch fixes a general issue with standalone apps on archs 
> supporting both endianess types when running MAKEALL. I guess this could 
> be relevant for ARM too if U-Boot has support for big-endian ARM boards.

It does, but I doubt anybody ever used standalone applications on
these.

Best regards,

Wolfgang Denk
Wolfgang Denk Dec. 4, 2011, 3:08 p.m. UTC | #3
Dear Shinya Kuribayashi,

In message <4EDB8475.1080305@pobox.com> you wrote:
>
> >> -pb1000                       mips        mips32      pb1x00              -              au1x00      pb1x00:PB1000
> >> +pb1000                       mips        mips32      pb1x00              -              au1x00      pb1x00:PB1000,SYS_LITTLE_ENDIAN
...
> But I don't see any reason CONFIG_SYS_LITTLE_ENDIAN is specified in
> boards.cfg.  Just putting it in configs/pb1000.h is enough, isn't it?

Agreed.  In almost all cases this should be done in the board config
file only.  Only if there are cases where a board suppors both LE and
BE configurations (is there any such board??) then we should need a
SYS_*_ENDIAN setting in boards.cfg (but even then, only one - not
two).

Best regards,

Wolfgang Denk
Daniel Schwierzeck Dec. 5, 2011, 12:04 a.m. UTC | #4
On 12/04/2011 04:06 PM, Wolfgang Denk wrote:
> Dear Daniel Schwierzeck,
>
> In message<4EDB5F86.40605@googlemail.com>  you wrote:
>>
>> Wolfgang, is there any reason why standalone apps are linked without
>> LDFLAGS (especially PLATFORM_LDFLAGS)?
>
> None that I know, at least.
>
> In general, standalone applications are a rarely used special feature,
> and the few people who actually ever used this for a purpose probably
> used custom code and custom linker scripts, so the mainline code is
> eventually not that good supproted.

the problem is those apps are always compiled. Maybe we should introduce 
a config option to enable building only if someone really needs them?

>
>> This patch fixes a general issue with standalone apps on archs
>> supporting both endianess types when running MAKEALL. I guess this could
>> be relevant for ARM too if U-Boot has support for big-endian ARM boards.
>
> It does, but I doubt anybody ever used standalone applications on
> these.
>
> Best regards,
>
> Wolfgang Denk
>
Daniel Schwierzeck Dec. 5, 2011, 12:10 a.m. UTC | #5
On 12/04/2011 04:08 PM, Wolfgang Denk wrote:
> Dear Shinya Kuribayashi,
>
> In message<4EDB8475.1080305@pobox.com>  you wrote:
>>
>>>> -pb1000                       mips        mips32      pb1x00              -              au1x00      pb1x00:PB1000
>>>> +pb1000                       mips        mips32      pb1x00              -              au1x00      pb1x00:PB1000,SYS_LITTLE_ENDIAN
> ...
>> But I don't see any reason CONFIG_SYS_LITTLE_ENDIAN is specified in
>> boards.cfg.  Just putting it in configs/pb1000.h is enough, isn't it?
>
> Agreed.  In almost all cases this should be done in the board config
> file only.  Only if there are cases where a board suppors both LE and
> BE configurations (is there any such board??) then we should need a
> SYS_*_ENDIAN setting in boards.cfg (but even then, only one - not
> two).

I did this because of dbau1550 and dbau1550_el. For the pb1000 I'll
move it to the board config.

>
> Best regards,
>
> Wolfgang Denk
>
Wolfgang Denk Dec. 5, 2011, 10:05 a.m. UTC | #6
Dear Daniel Schwierzeck,

In message <4EDC0A99.6090106@googlemail.com> you wrote:
>
> > In general, standalone applications are a rarely used special feature,
> > and the few people who actually ever used this for a purpose probably
> > used custom code and custom linker scripts, so the mainline code is
> > eventually not that good supproted.
> 
> the problem is those apps are always compiled. Maybe we should introduce 
> a config option to enable building only if someone really needs them?

As is, they are rarely used, so they tend to collect bit-rot.

If we stop to even compile them, they wil rot much faster.  This is
not a good idea.


Best regards,

Wolfgang Denk
Marek Vasut March 31, 2012, 8:54 p.m. UTC | #7
Dear Daniel Schwierzeck,

> On 12/04/2011 04:08 PM, Wolfgang Denk wrote:
> > Dear Shinya Kuribayashi,
> > 
> > In message<4EDB8475.1080305@pobox.com>  you wrote:
> >>>> -pb1000                       mips        mips32      pb1x00          
> >>>>    -              au1x00      pb1x00:PB1000 +pb1000                  
> >>>>     mips        mips32      pb1x00              -              au1x00
> >>>>      pb1x00:PB1000,SYS_LITTLE_ENDIAN
> > 
> > ...
> > 
> >> But I don't see any reason CONFIG_SYS_LITTLE_ENDIAN is specified in
> >> boards.cfg.  Just putting it in configs/pb1000.h is enough, isn't it?
> > 
> > Agreed.  In almost all cases this should be done in the board config
> > file only.  Only if there are cases where a board suppors both LE and
> > BE configurations (is there any such board??) then we should need a
> > SYS_*_ENDIAN setting in boards.cfg (but even then, only one - not
> > two).
> 
> I did this because of dbau1550 and dbau1550_el. For the pb1000 I'll
> move it to the board config.
> 
> > Best regards,
> > 
> > Wolfgang Denk

Will we see V2 of this patch please?

Best regards,
Marek Vasut
diff mbox

Patch

diff --git a/README b/README
index 07f1d11..468bfdf 100644
--- a/README
+++ b/README
@@ -374,6 +374,12 @@  The following options need to be configured:
 		Defines the string to utilize when trying to match PCIe device
 		tree nodes for the given platform.
 
+- Generic CPU options:
+		CONFIG_SYS_BIG_ENDIAN, CONFIG_SYS_LITTLE_ENDIAN
+
+		Defines the endianess of the CPU. Implementation of those
+		values is arch specific.
+
 - Intel Monahans options:
 		CONFIG_SYS_MONAHANS_RUN_MODE_OSC_RATIO
 
diff --git a/arch/mips/cpu/mips32/config.mk b/arch/mips/cpu/mips32/config.mk
index 4d1b273..a1cd590 100644
--- a/arch/mips/cpu/mips32/config.mk
+++ b/arch/mips/cpu/mips32/config.mk
@@ -27,14 +27,23 @@ 
 # Note: Toolchains with binutils prior to v2.16
 # are no longer supported by U-Boot MIPS tree!
 #
-MIPSFLAGS = -march=mips32r2
+MIPSFLAGS := -march=mips32r2
 
+# Handle special prefix in ELDK 4.0 toolchain
 ifneq (,$(findstring 4KCle,$(CROSS_COMPILE)))
-ENDIANNESS = -EL
-else
-ENDIANNESS = -EB
+ENDIANNESS := -EL
 endif
 
-MIPSFLAGS += $(ENDIANNESS)
+ifdef CONFIG_SYS_LITTLE_ENDIAN
+ENDIANNESS := -EL
+endif
+
+ifdef CONFIG_SYS_BIG_ENDIAN
+ENDIANNESS := -EB
+endif
+
+# Default to EB if no endianess is configured
+ENDIANNESS ?= -EB
 
-PLATFORM_CPPFLAGS += $(MIPSFLAGS)
+PLATFORM_CPPFLAGS += $(MIPSFLAGS) $(ENDIANNESS)
+PLATFORM_LDFLAGS += $(ENDIANNESS)
diff --git a/boards.cfg b/boards.cfg
index c83d861..2cd917e 100644
--- a/boards.cfg
+++ b/boards.cfg
@@ -324,13 +324,13 @@  dbau1000                     mips        mips32      dbau1x00            -
 dbau1100                     mips        mips32      dbau1x00            -              au1x00      dbau1x00:DBAU1100
 dbau1500                     mips        mips32      dbau1x00            -              au1x00      dbau1x00:DBAU1500
 dbau1550                     mips        mips32      dbau1x00            -              au1x00      dbau1x00:DBAU1550
-dbau1550_el                  mips        mips32      dbau1x00            -              au1x00      dbau1x00:DBAU1550
+dbau1550_el                  mips        mips32      dbau1x00            -              au1x00      dbau1x00:DBAU1550,SYS_LITTLE_ENDIAN
 gth2                         mips        mips32      -                   -              au1x00
 incaip                       mips        mips32      incaip              -              incaip
 incaip_100MHz                mips        mips32      incaip              -              incaip      incaip:CPU_CLOCK_RATE=100000000
 incaip_133MHz                mips        mips32      incaip              -              incaip      incaip:CPU_CLOCK_RATE=133000000
 incaip_150MHz                mips        mips32      incaip              -              incaip      incaip:CPU_CLOCK_RATE=150000000
-pb1000                       mips        mips32      pb1x00              -              au1x00      pb1x00:PB1000
+pb1000                       mips        mips32      pb1x00              -              au1x00      pb1x00:PB1000,SYS_LITTLE_ENDIAN
 qemu_mips                    mips        mips32      qemu-mips           -              -           qemu-mips
 tb0229                       mips        mips32
 vct_premium                  mips        mips32      vct                 micronas       -           vct:VCT_PREMIUM
diff --git a/examples/standalone/Makefile b/examples/standalone/Makefile
index e23865b..eab23b4 100644
--- a/examples/standalone/Makefile
+++ b/examples/standalone/Makefile
@@ -88,6 +88,10 @@  endif
 CFLAGS_NTR := $(call cc-option,-fno-toplevel-reorder)
 CFLAGS += $(CFLAGS_NTR)
 
+# Pass through endianess settings in LDFLAGS to LD
+LDFLAGS_ENDIAN += $(filter -EB,$(LDFLAGS))
+LDFLAGS_ENDIAN += $(filter -EL,$(LDFLAGS))
+
 all:	$(obj).depend $(OBJS) $(LIB) $(SREC) $(BIN) $(ELF)
 
 #########################################################################
@@ -96,7 +100,7 @@  $(LIB):	$(obj).depend $(LIBOBJS)
 
 $(ELF):
 $(obj)%:	$(obj)%.o $(LIB)
-		$(LD) -g -Ttext $(CONFIG_STANDALONE_LOAD_ADDR) \
+		$(LD) $(LDFLAGS_ENDIAN) -g -Ttext $(CONFIG_STANDALONE_LOAD_ADDR) \
 			-o $@ -e $(SYM_PREFIX)$(notdir $(<:.o=)) $< $(LIB) \
 			-L$(gcclibdir) -lgcc