[U-Boot] cross compiling fw_printenv on big endian

[U-Boot] cross compiling fw_printenv on big endian

[Jeff Angielski]

I seem to be having a problem with fw_printenv on my PowerPC 85xx target 
whereby I constantly get CRC32 errors:

# fw_printenv
Warning: Bad CRC, using default environment

I tracked this down to the fact that u-boot/lib_generic/crc32.c is 
getting compiled with the __LITTLE_ENDIAN defined even though I have 
CROSS_COMPILE setup correctly [e.g. I can build u-boot.bin just fine].

Is there a special way to cross compile fw_printenv so that the 
__LITTLE_ENDIAN is not defined?  Are there other PowerPC users that are 
building fw_printenv?

I'd rather do the "proper" thing than resort of hacking the crc32.c file.

-- 
Jeff Angielski
The PTR Group
www.theptrgroup.com

[U-Boot] cross compiling fw_printenv on big endian

[Detlev Zundel]

Hi Jeff,

> I seem to be having a problem with fw_printenv on my PowerPC 85xx target 
> whereby I constantly get CRC32 errors:
>
> # fw_printenv
> Warning: Bad CRC, using default environment
>
> I tracked this down to the fact that u-boot/lib_generic/crc32.c is 
> getting compiled with the __LITTLE_ENDIAN defined even though I have 
> CROSS_COMPILE setup correctly [e.g. I can build u-boot.bin just fine].

This makes little sense to me, as then the crc version used in U-Boot
would also use a wrong endianness.  Are you sure this happens?  Las time
I used the tool it worked for me without any such hacks.

Cheers
  Detlev

-- 
The limits of my language stand for the limits of my world.
                                        -- Ludwig Wittgenstein
--
DENX Software Engineering GmbH,      MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich,  Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: dzu at denx.de

[U-Boot] cross compiling fw_printenv on big endian

[Joakim Tjernlund]

>
> Hi Jeff,
>
> > I seem to be having a problem with fw_printenv on my PowerPC 85xx target
> > whereby I constantly get CRC32 errors:
> >
> > # fw_printenv
> > Warning: Bad CRC, using default environment
> >
> > I tracked this down to the fact that u-boot/lib_generic/crc32.c is
> > getting compiled with the __LITTLE_ENDIAN defined even though I have
> > CROSS_COMPILE setup correctly [e.g. I can build u-boot.bin just fine].
>
> This makes little sense to me, as then the crc version used in U-Boot
> would also use a wrong endianness.  Are you sure this happens?  Las time
> I used the tool it worked for me without any such hacks.

hmm, I recently discovered that normal user space headers always define
both __LITTLE_ENDIAN and __BIG_ENDIAN so therefore a
# ifdef __LITTLE_ENDIAN
#  define DO_CRC(x) crc = tab[(crc ^ (x)) & 255] ^ (crc >> 8)
# else
#  define DO_CRC(x) crc = tab[((crc >> 24) ^ (x)) & 255] ^ (crc << 8)
# endif

Wont work. One have to use
 #if __BYTE_ORDER == __LITTLE_ENDIAN
instead.

Problem is that I don't think u-boot #defines __BYTE_ORDER so
that would have to be added too for all archs:
#define __BYTE_ORDER __LITTLE_ENDIAN
or
#define __BYTE_ORDER __BIG_ENDIAN

[U-Boot] cross compiling fw_printenv on big endian

[Wolfgang Denk]

Dear Joakim Tjernlund,

In message <OFF4AB0804.BE309218-ONC12576E9.003A69DF-C12576E9.003B4D86@transmode.se> you wrote:
>
> hmm, I recently discovered that normal user space headers always define
> both __LITTLE_ENDIAN and __BIG_ENDIAN so therefore a
> # ifdef __LITTLE_ENDIAN
> #  define DO_CRC(x) crc = tab[(crc ^ (x)) & 255] ^ (crc >> 8)
> # else
> #  define DO_CRC(x) crc = tab[((crc >> 24) ^ (x)) & 255] ^ (crc << 8)
> # endif
> 
> Wont work. One have to use
>  #if __BYTE_ORDER == __LITTLE_ENDIAN
> instead.

Wenn...

3ee8c120 (Joakim Tjernlund                 2009-11-19 13:44:16 +0100 166) # ifdef __LITTLE_ENDIAN
3ee8c120 (Joakim Tjernlund                 2009-11-19 13:44:16 +0100 167) #  define DO_CRC(x) crc = tab[(crc ^ (x)) & 255] ^ (crc >> 8)
3ee8c120 (Joakim Tjernlund                 2009-11-19 13:44:16 +0100 168) # else
3ee8c120 (Joakim Tjernlund                 2009-11-19 13:44:16 +0100 169) #  define DO_CRC(x) crc = tab[((crc >> 24) ^ (x)) & 255] ^ (crc << 8)
3ee8c120 (Joakim Tjernlund                 2009-11-19 13:44:16 +0100 170) # endif


commit 3ee8c12071f0e3bdda25125b63c9d3fd54a7c9d8
Author: Joakim Tjernlund <Joakim.Tjernlund@transmode.se>
Date:   Thu Nov 19 13:44:16 2009 +0100

    crc32: Impl. linux optimized crc32()
    
    Ported over the more efficient linux crc32() function.
    A quick comparsion on ppc:
    After changing the old crc32 to do 4 bytes in the
    inner loop to be able to compare with new version one can note:
    - old inner loop has 61 insn, new has 19 insn.
    - new crc32 does one 32 bit load of data to crc while
      the old does four 8 bits loads.
    - size is bit bigger for the new crc32:
      1392(old) 1416(new) of text. The is because the new version
      shares code with crc32_no_comp() instead of duplicating code.
    - about 33% faster on ppc:
      New > crc 0 0xfffffff -> 39 secs
      Old > crc 0 0xfffffff -> 60 secs
    
    Signed-off-by: Joakim Tjernlund <Joakim.Tjernlund@transmode.se>


Looks as if this were your very own commit. Do you have a fix in the
works?

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
The manager will be continually amazed that policies he took for com-
mon knowledge are totally unknown by some member of his  team.  Since
his fundamental job is to keep everybody going in the same direction,
his chief daily task will be communication, not decision-making.
                              - Fred Brooks, "The Mythical Man Month"

[U-Boot] cross compiling fw_printenv on big endian

[Joakim Tjernlund]

Wolfgang Denk <wd@denx.de> wrote on 2010/03/17 12:57:31:
>
> Dear Joakim Tjernlund,
>
> In message <OFF4AB0804.BE309218-ONC12576E9.003A69DF-C12576E9.
> 003B4D86 at transmode.se> you wrote:
> >
> > hmm, I recently discovered that normal user space headers always define
> > both __LITTLE_ENDIAN and __BIG_ENDIAN so therefore a
> > # ifdef __LITTLE_ENDIAN
> > #  define DO_CRC(x) crc = tab[(crc ^ (x)) & 255] ^ (crc >> 8)
> > # else
> > #  define DO_CRC(x) crc = tab[((crc >> 24) ^ (x)) & 255] ^ (crc << 8)
> > # endif
> >
> > Wont work. One have to use
> >  #if __BYTE_ORDER == __LITTLE_ENDIAN
> > instead.
>
> Wenn...

>
>     Signed-off-by: Joakim Tjernlund <Joakim.Tjernlund@transmode.se>
>
>
> Looks as if this were your very own commit. Do you have a fix in the
> works?

I know, but I don't have anything ATM. I am too busy debugging serious
customer problems.

     Jocke

[U-Boot] cross compiling fw_printenv on big endian

[Jeff Angielski]

Joakim Tjernlund wrote:
> 
> Wolfgang Denk <wd@denx.de> wrote on 2010/03/17 12:57:31:
>> Dear Joakim Tjernlund,
>>
>> In message <OFF4AB0804.BE309218-ONC12576E9.003A69DF-C12576E9.
>> 003B4D86 at transmode.se> you wrote:
>>> hmm, I recently discovered that normal user space headers always define
>>> both __LITTLE_ENDIAN and __BIG_ENDIAN so therefore a
>>> # ifdef __LITTLE_ENDIAN
>>> #  define DO_CRC(x) crc = tab[(crc ^ (x)) & 255] ^ (crc >> 8)
>>> # else
>>> #  define DO_CRC(x) crc = tab[((crc >> 24) ^ (x)) & 255] ^ (crc << 8)
>>> # endif
>>>
>>> Wont work. One have to use
>>>  #if __BYTE_ORDER == __LITTLE_ENDIAN
>>> instead.
>> Wenn...
> 
>>     Signed-off-by: Joakim Tjernlund <Joakim.Tjernlund@transmode.se>
>>
>>
>> Looks as if this were your very own commit. Do you have a fix in the
>> works?
> 
> I know, but I don't have anything ATM. I am too busy debugging serious
> customer problems.
> 
>      Jocke

This appears to work for me on my big endian PowerPC target.  Perhaps 
somebody with a little endian target can verify it does not break their 
env tools.

diff --git a/lib_generic/crc32.c b/lib_generic/crc32.c
index 468b397..27335a3 100644
--- a/lib_generic/crc32.c
+++ b/lib_generic/crc32.c
@@ -163,7 +163,7 @@ const uint32_t * ZEXPORT get_crc_table()
  #endif

  /* 
========================================================================= */
-# ifdef __LITTLE_ENDIAN
+# if __BYTE_ORDER == __LITTLE_ENDIAN
  #  define DO_CRC(x) crc = tab[(crc ^ (x)) & 255] ^ (crc >> 8)
  # else
  #  define DO_CRC(x) crc = tab[((crc >> 24) ^ (x)) & 255] ^ (crc << 8)



Jeff Angielski
The PTR Group
www.theptrgroup.com

[U-Boot] cross compiling fw_printenv on big endian

[Joakim Tjernlund]

Jeff Angielski <jeff@theptrgroup.com> wrote on 2010/03/17 16:10:50:
>
> Joakim Tjernlund wrote:
> >
> > Wolfgang Denk <wd@denx.de> wrote on 2010/03/17 12:57:31:
> >> Dear Joakim Tjernlund,
> >>
> >> In message <OFF4AB0804.BE309218-ONC12576E9.003A69DF-C12576E9.
> >> 003B4D86 at transmode.se> you wrote:
> >>> hmm, I recently discovered that normal user space headers always define
> >>> both __LITTLE_ENDIAN and __BIG_ENDIAN so therefore a
> >>> # ifdef __LITTLE_ENDIAN
> >>> #  define DO_CRC(x) crc = tab[(crc ^ (x)) & 255] ^ (crc >> 8)
> >>> # else
> >>> #  define DO_CRC(x) crc = tab[((crc >> 24) ^ (x)) & 255] ^ (crc << 8)
> >>> # endif
> >>>
> >>> Wont work. One have to use
> >>>  #if __BYTE_ORDER == __LITTLE_ENDIAN
> >>> instead.
> >> Wenn...
> >
> >>     Signed-off-by: Joakim Tjernlund <Joakim.Tjernlund@transmode.se>
> >>
> >>
> >> Looks as if this were your very own commit. Do you have a fix in the
> >> works?
> >
> > I know, but I don't have anything ATM. I am too busy debugging serious
> > customer problems.
> >
> >      Jocke
>
> This appears to work for me on my big endian PowerPC target.  Perhaps
> somebody with a little endian target can verify it does not break their
> env tools.
>
> diff --git a/lib_generic/crc32.c b/lib_generic/crc32.c
> index 468b397..27335a3 100644
> --- a/lib_generic/crc32.c
> +++ b/lib_generic/crc32.c
> @@ -163,7 +163,7 @@ const uint32_t * ZEXPORT get_crc_table()
>   #endif
>
>   /*
> ========================================================================= */
> -# ifdef __LITTLE_ENDIAN
> +# if __BYTE_ORDER == __LITTLE_ENDIAN
>   #  define DO_CRC(x) crc = tab[(crc ^ (x)) & 255] ^ (crc >> 8)
>   # else
>   #  define DO_CRC(x) crc = tab[((crc >> 24) ^ (x)) & 255] ^ (crc << 8)


I THINK this will work. Looking@include/linux/byteorder/big_endian.h it says:
#define	__BYTE_ORDER	__BIG_ENDIAN
so it seems like the __BYTE_ORDER logic is in place in u-boot.
Someone with a LE CPU should confirm this.

Mind doing a proper patch?

       Jocke

[U-Boot] cross compiling fw_printenv on big endian

[Jeff Angielski]

Joakim Tjernlund wrote:
> Jeff Angielski <jeff@theptrgroup.com> wrote on 2010/03/17 16:10:50:
>> Joakim Tjernlund wrote:
>>> Wolfgang Denk <wd@denx.de> wrote on 2010/03/17 12:57:31:
>>>> Dear Joakim Tjernlund,
>>>>
>>>> In message <OFF4AB0804.BE309218-ONC12576E9.003A69DF-C12576E9.
>>>> 003B4D86 at transmode.se> you wrote:
>>>>> hmm, I recently discovered that normal user space headers always define
>>>>> both __LITTLE_ENDIAN and __BIG_ENDIAN so therefore a
>>>>> # ifdef __LITTLE_ENDIAN
>>>>> #  define DO_CRC(x) crc = tab[(crc ^ (x)) & 255] ^ (crc >> 8)
>>>>> # else
>>>>> #  define DO_CRC(x) crc = tab[((crc >> 24) ^ (x)) & 255] ^ (crc << 8)
>>>>> # endif
>>>>>
>>>>> Wont work. One have to use
>>>>>  #if __BYTE_ORDER == __LITTLE_ENDIAN
>>>>> instead.
>>>> Wenn...
>>>>     Signed-off-by: Joakim Tjernlund <Joakim.Tjernlund@transmode.se>
>>>>
>>>>
>>>> Looks as if this were your very own commit. Do you have a fix in the
>>>> works?
>>> I know, but I don't have anything ATM. I am too busy debugging serious
>>> customer problems.
>>>
>>>      Jocke
>> This appears to work for me on my big endian PowerPC target.  Perhaps
>> somebody with a little endian target can verify it does not break their
>> env tools.
>>
>> diff --git a/lib_generic/crc32.c b/lib_generic/crc32.c
>> index 468b397..27335a3 100644
>> --- a/lib_generic/crc32.c
>> +++ b/lib_generic/crc32.c
>> @@ -163,7 +163,7 @@ const uint32_t * ZEXPORT get_crc_table()
>>   #endif
>>
>>   /*
>> ========================================================================= */
>> -# ifdef __LITTLE_ENDIAN
>> +# if __BYTE_ORDER == __LITTLE_ENDIAN
>>   #  define DO_CRC(x) crc = tab[(crc ^ (x)) & 255] ^ (crc >> 8)
>>   # else
>>   #  define DO_CRC(x) crc = tab[((crc >> 24) ^ (x)) & 255] ^ (crc << 8)
> 
> 
> I THINK this will work. Looking at include/linux/byteorder/big_endian.h it says:
> #define	__BYTE_ORDER	__BIG_ENDIAN
> so it seems like the __BYTE_ORDER logic is in place in u-boot.
> Someone with a LE CPU should confirm this.
> 
> Mind doing a proper patch?
>

See attached file.




-- 
Jeff Angielski
The PTR Group
www.theptrgroup.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-env-fix-endian-ordering-in-crc-table.patch
Type: text/x-patch
Size: 0 bytes
Desc: not available
Url : http://lists.denx.de/pipermail/u-boot/attachments/20100317/27c25dd9/attachment.bin 

[U-Boot] cross compiling fw_printenv on big endian

[Wolfgang Denk]

Dear Jeff Angielski,

In message <4BA12AF2.4080506@theptrgroup.com> you wrote:
>
> Subject: [PATCH] env: fix endian ordering in crc table
> 
> The crc table was being built as little endian for big endian
> targets.  This would cause fw_printenv to always fail with
> "Warning: Bad CRC, using default environment" messages.
> Signed-off-by: Jeff Angielski <jeff@theptrgroup.com>
> ---
>  lib_generic/crc32.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)

Applied, thanks.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Mirrors should reflect a little before throwing back images.
- Jean Cocteau