Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1649398 > unrolled thread

CPU_BIG_ENDIAN in generic code (was: Re: [PATCH v3 3/7] arch/sparc: Define config parameter CPU_BIG_ENDIAN)

Started byGeert Uytterhoeven <geert@linux-m68k.org>
First post2017-05-24 12:00 +0200
Last post2017-05-30 05:00 +0200
Articles 16 — 5 participants

Back to article view | Back to linux.kernel


Contents

  CPU_BIG_ENDIAN in generic code (was: Re: [PATCH v3 3/7] arch/sparc:  Define config parameter CPU_BIG_ENDIAN) Geert Uytterhoeven <geert@linux-m68k.org> - 2017-05-24 12:00 +0200
    Re: CPU_BIG_ENDIAN in generic code (was: Re: [PATCH v3 3/7]  arch/sparc: Define config parameter CPU_BIG_ENDIAN) Arnd Bergmann <arnd@arndb.de> - 2017-05-24 12:20 +0200
      Re: CPU_BIG_ENDIAN in generic code (was: Re: [PATCH v3 3/7]  arch/sparc: Define config parameter CPU_BIG_ENDIAN) Babu Moger <babu.moger@oracle.com> - 2017-05-24 16:50 +0200
        Re: CPU_BIG_ENDIAN in generic code (was: Re: [PATCH v3 3/7]  arch/sparc: Define config parameter CPU_BIG_ENDIAN) Arnd Bergmann <arnd@arndb.de> - 2017-05-24 17:10 +0200
          Re: CPU_BIG_ENDIAN in generic code (was: Re: [PATCH v3 3/7]  arch/sparc: Define config parameter CPU_BIG_ENDIAN) Babu Moger <babu.moger@oracle.com> - 2017-05-24 19:10 +0200
      Re: CPU_BIG_ENDIAN in generic code (was: Re: [PATCH v3 3/7]  arch/sparc: Define config parameter CPU_BIG_ENDIAN) Babu Moger <babu.moger@oracle.com> - 2017-05-25 17:00 +0200
        Re: CPU_BIG_ENDIAN in generic code (was: Re: [PATCH v3 3/7]  arch/sparc: Define config parameter CPU_BIG_ENDIAN) Arnd Bergmann <arnd@arndb.de> - 2017-05-25 22:20 +0200
          Re: CPU_BIG_ENDIAN in generic code (was: Re: [PATCH v3 3/7]  arch/sparc: Define config parameter CPU_BIG_ENDIAN) Babu Moger <babu.moger@oracle.com> - 2017-05-25 22:30 +0200
      Re: CPU_BIG_ENDIAN in generic code (was: Re: [PATCH v3 3/7]  arch/sparc: Define config parameter CPU_BIG_ENDIAN) Max Filippov <jcmvbkbc@gmail.com> - 2017-05-26 00:30 +0200
        Re: CPU_BIG_ENDIAN in generic code (was: Re: [PATCH v3 3/7]  arch/sparc: Define config parameter CPU_BIG_ENDIAN) Babu Moger <babu.moger@oracle.com> - 2017-05-26 00:50 +0200
          Re: CPU_BIG_ENDIAN in generic code (was: Re: [PATCH v3 3/7]  arch/sparc: Define config parameter CPU_BIG_ENDIAN) Max Filippov <jcmvbkbc@gmail.com> - 2017-05-26 01:00 +0200
        Re: CPU_BIG_ENDIAN in generic code (was: Re: [PATCH v3 3/7]  arch/sparc: Define config parameter CPU_BIG_ENDIAN) Max Filippov <jcmvbkbc@gmail.com> - 2017-05-26 00:50 +0200
          Re: CPU_BIG_ENDIAN in generic code (was: Re: [PATCH v3 3/7]  arch/sparc: Define config parameter CPU_BIG_ENDIAN) Arnd Bergmann <arnd@arndb.de> - 2017-05-29 15:00 +0200
    Re: CPU_BIG_ENDIAN in generic code (was: Re: [PATCH v3 3/7] arch/sparc: Define config parameter CPU_BIG_ENDIAN) Michael Ellerman <mpe@ellerman.id.au> - 2017-05-29 14:10 +0200
      Re: CPU_BIG_ENDIAN in generic code (was: Re: [PATCH v3 3/7]  arch/sparc: Define config parameter CPU_BIG_ENDIAN) Geert Uytterhoeven <geert@linux-m68k.org> - 2017-05-29 14:20 +0200
        Re: CPU_BIG_ENDIAN in generic code (was: Re: [PATCH v3 3/7] arch/sparc: Define config parameter CPU_BIG_ENDIAN) Michael Ellerman <mpe@ellerman.id.au> - 2017-05-30 05:00 +0200

#1649398 — CPU_BIG_ENDIAN in generic code (was: Re: [PATCH v3 3/7] arch/sparc: Define config parameter CPU_BIG_ENDIAN)

FromGeert Uytterhoeven <geert@linux-m68k.org>
Date2017-05-24 12:00 +0200
SubjectCPU_BIG_ENDIAN in generic code (was: Re: [PATCH v3 3/7] arch/sparc: Define config parameter CPU_BIG_ENDIAN)
Message-ID<tKBfc-EZ-31@gated-at.bofh.it>
On Tue, May 23, 2017 at 11:45 PM, Babu Moger <babu.moger@oracle.com> wrote:
> Found this problem while enabling queued rwlock on SPARC.
> The parameter CONFIG_CPU_BIG_ENDIAN is used to clear the
> specific byte in qrwlock structure. Without this parameter,
> we clear the wrong byte. Here is the code.
>
> static inline u8 *__qrwlock_write_byte(struct qrwlock *lock)
>  {
>         return (u8 *)lock + 3 * IS_BUILTIN(CONFIG_CPU_BIG_ENDIAN);
>  }
>
> Define CPU_BIG_ENDIAN for SPARC to fix it.

> --- a/arch/sparc/Kconfig
> +++ b/arch/sparc/Kconfig
> @@ -92,6 +92,10 @@ config ARCH_DEFCONFIG
>  config ARCH_PROC_KCORE_TEXT
>         def_bool y
>
> +config CPU_BIG_ENDIAN
> +       bool
> +       default y if SPARC

Nice catch!

Traditionally, CPU_BIG_ENDIAN and CPU_LITTLE_ENDIAN were defined only on
architectures that may support both.  And it was checked in platform code
and drivers only.
Hence the symbol is lacking from most architectures. Heck, even
architectures that support both may default to one endiannes, and declare
only the symbol for the other endianness:

--- arch/alpha ---
--- arch/arc ---
arch/arc/Kconfig:config CPU_BIG_ENDIAN
--- arch/arm ---
arch/arm/mm/Kconfig:config CPU_BIG_ENDIAN
--- arch/arm64 ---
arch/arm64/Kconfig:config CPU_BIG_ENDIAN
--- arch/blackfin ---
--- arch/c6x ---
arch/c6x/Kconfig:config CPU_BIG_ENDIAN
--- arch/cris ---
--- arch/frv ---
--- arch/h8300 ---
--- arch/hexagon ---
--- arch/ia64 ---
--- arch/Kconfig ---
--- arch/m32r ---
arch/m32r/Kconfig:config CPU_LITTLE_ENDIAN
--- arch/m68k ---
--- arch/metag ---
--- arch/microblaze ---
--- arch/mips ---
arch/mips/Kconfig:config CPU_BIG_ENDIAN
arch/mips/Kconfig:config CPU_LITTLE_ENDIAN
--- arch/mn10300 ---
--- arch/nios2 ---
--- arch/openrisc ---
--- arch/parisc ---
--- arch/powerpc ---
arch/powerpc/platforms/Kconfig.cputype:config CPU_BIG_ENDIAN
arch/powerpc/platforms/Kconfig.cputype:config CPU_LITTLE_ENDIAN
--- arch/s390 ---
arch/s390/Kconfig:config CPU_BIG_ENDIAN
--- arch/score ---
--- arch/sh ---
arch/sh/Kconfig.cpu:config CPU_LITTLE_ENDIAN
arch/sh/Kconfig.cpu:config CPU_BIG_ENDIAN
--- arch/sparc ---
--- arch/tile ---
--- arch/um ---
--- arch/unicore32 ---
--- arch/x86 ---
--- arch/xtensa ---

However, there are already a few users in generic code, which are thus
broken on many platforms:

    drivers/of/base.c
    drivers/of/fdt.c
    drivers/tty/serial/earlycon.c
    drivers/tty/serial/serial_core.c

include/asm-generic/qrwlock.h is also generic, but depends on the
architecture to select ARCH_USE_QUEUED_RWLOCKS, which only very few do
(x86, and now sparc).

I guess the time is ripe for adding (both) symbols to all architectures?

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

[toc] | [next] | [standalone]


#1649414 — Re: CPU_BIG_ENDIAN in generic code (was: Re: [PATCH v3 3/7] arch/sparc: Define config parameter CPU_BIG_ENDIAN)

FromArnd Bergmann <arnd@arndb.de>
Date2017-05-24 12:20 +0200
SubjectRe: CPU_BIG_ENDIAN in generic code (was: Re: [PATCH v3 3/7] arch/sparc: Define config parameter CPU_BIG_ENDIAN)
Message-ID<tKByx-11f-1@gated-at.bofh.it>
In reply to#1649398
On Wed, May 24, 2017 at 11:59 AM, Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
> On Tue, May 23, 2017 at 11:45 PM, Babu Moger <babu.moger@oracle.com> wrote:
>> Found this problem while enabling queued rwlock on SPARC.
>> The parameter CONFIG_CPU_BIG_ENDIAN is used to clear the
>> specific byte in qrwlock structure. Without this parameter,
>> we clear the wrong byte. Here is the code.
>>
>> static inline u8 *__qrwlock_write_byte(struct qrwlock *lock)
>>  {
>>         return (u8 *)lock + 3 * IS_BUILTIN(CONFIG_CPU_BIG_ENDIAN);
>>  }
>>
>> Define CPU_BIG_ENDIAN for SPARC to fix it.
>
>> --- a/arch/sparc/Kconfig
>> +++ b/arch/sparc/Kconfig
>> @@ -92,6 +92,10 @@ config ARCH_DEFCONFIG
>>  config ARCH_PROC_KCORE_TEXT
>>         def_bool y
>>
>> +config CPU_BIG_ENDIAN
>> +       bool
>> +       default y if SPARC
>
> Nice catch!
>
> Traditionally, CPU_BIG_ENDIAN and CPU_LITTLE_ENDIAN were defined only on
> architectures that may support both.  And it was checked in platform code
> and drivers only.
> Hence the symbol is lacking from most architectures. Heck, even
> architectures that support both may default to one endiannes, and declare
> only the symbol for the other endianness:
>
> --- arch/alpha ---
> --- arch/arc ---
> arch/arc/Kconfig:config CPU_BIG_ENDIAN
> --- arch/arm ---
> arch/arm/mm/Kconfig:config CPU_BIG_ENDIAN
> --- arch/arm64 ---
> arch/arm64/Kconfig:config CPU_BIG_ENDIAN
> --- arch/blackfin ---
> --- arch/c6x ---
> arch/c6x/Kconfig:config CPU_BIG_ENDIAN
> --- arch/cris ---
> --- arch/frv ---
> --- arch/h8300 ---
> --- arch/hexagon ---
> --- arch/ia64 ---
> --- arch/Kconfig ---
> --- arch/m32r ---
> arch/m32r/Kconfig:config CPU_LITTLE_ENDIAN
> --- arch/m68k ---
> --- arch/metag ---
> --- arch/microblaze ---
> --- arch/mips ---
> arch/mips/Kconfig:config CPU_BIG_ENDIAN
> arch/mips/Kconfig:config CPU_LITTLE_ENDIAN
> --- arch/mn10300 ---
> --- arch/nios2 ---
> --- arch/openrisc ---
> --- arch/parisc ---
> --- arch/powerpc ---
> arch/powerpc/platforms/Kconfig.cputype:config CPU_BIG_ENDIAN
> arch/powerpc/platforms/Kconfig.cputype:config CPU_LITTLE_ENDIAN
> --- arch/s390 ---
> arch/s390/Kconfig:config CPU_BIG_ENDIAN
> --- arch/score ---
> --- arch/sh ---
> arch/sh/Kconfig.cpu:config CPU_LITTLE_ENDIAN
> arch/sh/Kconfig.cpu:config CPU_BIG_ENDIAN
> --- arch/sparc ---
> --- arch/tile ---
> --- arch/um ---
> --- arch/unicore32 ---
> --- arch/x86 ---
> --- arch/xtensa ---
>
> However, there are already a few users in generic code, which are thus
> broken on many platforms:
>
>     drivers/of/base.c
>     drivers/of/fdt.c
>     drivers/tty/serial/earlycon.c
>     drivers/tty/serial/serial_core.c
>
> include/asm-generic/qrwlock.h is also generic, but depends on the
> architecture to select ARCH_USE_QUEUED_RWLOCKS, which only very few do
> (x86, and now sparc).
>
> I guess the time is ripe for adding (both) symbols to all architectures?

Good idea. I think we can do most of this by adding a few lines to
arch/Kconfig:

config CPU_BIG_ENDIAN
        bool

config CPU_LITTLE_ENDIAN
       def_bool !CPU_BIG_ENDIAN

This way, we only need to add 'select CPU_BIG_ENDIAN' to the
architectures that are always big-endian, and we don't need to
change anything for the ones that have a single 'CPU_BIG_ENDIAN'
option.

The three architectures that have a 'choice' statement (mips, ppc and
sh) will have to convert, and m32r will have to replace the
option with the opposite one, which could break 'make oldconfig',
but nobody really cares about m32r any more.

       Arnd

[toc] | [prev] | [next] | [standalone]


#1649677 — Re: CPU_BIG_ENDIAN in generic code (was: Re: [PATCH v3 3/7] arch/sparc: Define config parameter CPU_BIG_ENDIAN)

FromBabu Moger <babu.moger@oracle.com>
Date2017-05-24 16:50 +0200
SubjectRe: CPU_BIG_ENDIAN in generic code (was: Re: [PATCH v3 3/7] arch/sparc: Define config parameter CPU_BIG_ENDIAN)
Message-ID<tKFLP-3z2-1@gated-at.bofh.it>
In reply to#1649414
Arnd,


On 5/24/2017 5:18 AM, Arnd Bergmann wrote:
> On Wed, May 24, 2017 at 11:59 AM, Geert Uytterhoeven
> <geert@linux-m68k.org> wrote:
>> On Tue, May 23, 2017 at 11:45 PM, Babu Moger <babu.moger@oracle.com> wrote:
>>> Found this problem while enabling queued rwlock on SPARC.
>>> The parameter CONFIG_CPU_BIG_ENDIAN is used to clear the
>>> specific byte in qrwlock structure. Without this parameter,
>>> we clear the wrong byte. Here is the code.
>>>
>>> static inline u8 *__qrwlock_write_byte(struct qrwlock *lock)
>>>   {
>>>          return (u8 *)lock + 3 * IS_BUILTIN(CONFIG_CPU_BIG_ENDIAN);
>>>   }
>>>
>>> Define CPU_BIG_ENDIAN for SPARC to fix it.
>>> --- a/arch/sparc/Kconfig
>>> +++ b/arch/sparc/Kconfig
>>> @@ -92,6 +92,10 @@ config ARCH_DEFCONFIG
>>>   config ARCH_PROC_KCORE_TEXT
>>>          def_bool y
>>>
>>> +config CPU_BIG_ENDIAN
>>> +       bool
>>> +       default y if SPARC
>> Nice catch!
>>
>> Traditionally, CPU_BIG_ENDIAN and CPU_LITTLE_ENDIAN were defined only on
>> architectures that may support both.  And it was checked in platform code
>> and drivers only.
>> Hence the symbol is lacking from most architectures. Heck, even
>> architectures that support both may default to one endiannes, and declare
>> only the symbol for the other endianness:
>>
>> --- arch/alpha ---
>> --- arch/arc ---
>> arch/arc/Kconfig:config CPU_BIG_ENDIAN
>> --- arch/arm ---
>> arch/arm/mm/Kconfig:config CPU_BIG_ENDIAN
>> --- arch/arm64 ---
>> arch/arm64/Kconfig:config CPU_BIG_ENDIAN
>> --- arch/blackfin ---
>> --- arch/c6x ---
>> arch/c6x/Kconfig:config CPU_BIG_ENDIAN
>> --- arch/cris ---
>> --- arch/frv ---
>> --- arch/h8300 ---
>> --- arch/hexagon ---
>> --- arch/ia64 ---
>> --- arch/Kconfig ---
>> --- arch/m32r ---
>> arch/m32r/Kconfig:config CPU_LITTLE_ENDIAN
>> --- arch/m68k ---
>> --- arch/metag ---
>> --- arch/microblaze ---
>> --- arch/mips ---
>> arch/mips/Kconfig:config CPU_BIG_ENDIAN
>> arch/mips/Kconfig:config CPU_LITTLE_ENDIAN
>> --- arch/mn10300 ---
>> --- arch/nios2 ---
>> --- arch/openrisc ---
>> --- arch/parisc ---
>> --- arch/powerpc ---
>> arch/powerpc/platforms/Kconfig.cputype:config CPU_BIG_ENDIAN
>> arch/powerpc/platforms/Kconfig.cputype:config CPU_LITTLE_ENDIAN
>> --- arch/s390 ---
>> arch/s390/Kconfig:config CPU_BIG_ENDIAN
>> --- arch/score ---
>> --- arch/sh ---
>> arch/sh/Kconfig.cpu:config CPU_LITTLE_ENDIAN
>> arch/sh/Kconfig.cpu:config CPU_BIG_ENDIAN
>> --- arch/sparc ---
>> --- arch/tile ---
>> --- arch/um ---
>> --- arch/unicore32 ---
>> --- arch/x86 ---
>> --- arch/xtensa ---
>>
>> However, there are already a few users in generic code, which are thus
>> broken on many platforms:
>>
>>      drivers/of/base.c
>>      drivers/of/fdt.c
>>      drivers/tty/serial/earlycon.c
>>      drivers/tty/serial/serial_core.c
>>
>> include/asm-generic/qrwlock.h is also generic, but depends on the
>> architecture to select ARCH_USE_QUEUED_RWLOCKS, which only very few do
>> (x86, and now sparc).
>>
>> I guess the time is ripe for adding (both) symbols to all architectures?
> Good idea. I think we can do most of this by adding a few lines to
> arch/Kconfig:
>
> config CPU_BIG_ENDIAN
>          bool
>
> config CPU_LITTLE_ENDIAN
>         def_bool !CPU_BIG_ENDIAN
I noticed that even x86 does not define CPU_LITTLE_ENDIAN.  Strange.
With this code all the architecture will default to 
CONFIG_CPU_LITTLE_ENDIAN.
I can make it as a separate patch. But I can only test SPARC and little 
bit of x86.
Is that ok?

>
> This way, we only need to add 'select CPU_BIG_ENDIAN' to the
> architectures that are always big-endian, and we don't need to
> change anything for the ones that have a single 'CPU_BIG_ENDIAN'
> option.
>
> The three architectures that have a 'choice' statement (mips, ppc and
> sh) will have to convert, and m32r will have to replace the
> option with the opposite one, which could break 'make oldconfig',
> but nobody really cares about m32r any more.
>
>         Arnd

[toc] | [prev] | [next] | [standalone]


#1649697 — Re: CPU_BIG_ENDIAN in generic code (was: Re: [PATCH v3 3/7] arch/sparc: Define config parameter CPU_BIG_ENDIAN)

FromArnd Bergmann <arnd@arndb.de>
Date2017-05-24 17:10 +0200
SubjectRe: CPU_BIG_ENDIAN in generic code (was: Re: [PATCH v3 3/7] arch/sparc: Define config parameter CPU_BIG_ENDIAN)
Message-ID<tKG5c-3Wx-39@gated-at.bofh.it>
In reply to#1649677
On Wed, May 24, 2017 at 4:45 PM, Babu Moger <babu.moger@oracle.com> wrote:
> On 5/24/2017 5:18 AM, Arnd Bergmann wrote:
>> On Wed, May 24, 2017 at 11:59 AM, Geert Uytterhoeven
>> <geert@linux-m68k.org> wrote:
>>> On Tue, May 23, 2017 at 11:45 PM, Babu Moger <babu.moger@oracle.com>
>>> wrote:
>>> include/asm-generic/qrwlock.h is also generic, but depends on the
>>> architecture to select ARCH_USE_QUEUED_RWLOCKS, which only very few do
>>> (x86, and now sparc).
>>>
>>> I guess the time is ripe for adding (both) symbols to all architectures?
>>
>> Good idea. I think we can do most of this by adding a few lines to
>> arch/Kconfig:
>>
>> config CPU_BIG_ENDIAN
>>          bool
>>
>> config CPU_LITTLE_ENDIAN
>>         def_bool !CPU_BIG_ENDIAN
>
> I noticed that even x86 does not define CPU_LITTLE_ENDIAN.  Strange.

There is no architecture-independent code that tests for
CONFIG_CPU_LITTLE_ENDIAN, unlike CONFIG_CPU_BIG_ENDIAN,
so that's not very suprising.

> With this code all the architecture will default to
> CONFIG_CPU_LITTLE_ENDIAN.

What I meant is that we have to 'select CPU_BIG_ENDIAN' on all architectures
that actually are big-endian:

These are all configurable:
$ git grep -l linux/byteorder/big_endian.h | xargs grep -l
linux/byteorder/little_endian.h
arch/arc/include/uapi/asm/byteorder.h
arch/arm/include/uapi/asm/byteorder.h
arch/arm64/include/uapi/asm/byteorder.h
arch/c6x/include/uapi/asm/byteorder.h
arch/m32r/include/uapi/asm/byteorder.h
arch/microblaze/include/uapi/asm/byteorder.h
arch/mips/include/uapi/asm/byteorder.h
arch/powerpc/include/uapi/asm/byteorder.h
arch/sh/include/uapi/asm/byteorder.h
arch/tile/include/uapi/asm/byteorder.h

These are always big-endian:
$ git grep -l linux/byteorder/big_endian.h | xargs grep -L
linux/byteorder/little_endian.h
arch/avr32/include/uapi/asm/byteorder.h
arch/frv/include/uapi/asm/byteorder.h
arch/m68k/include/uapi/asm/byteorder.h
arch/openrisc/include/uapi/asm/byteorder.h
arch/parisc/include/uapi/asm/byteorder.h
arch/s390/include/uapi/asm/byteorder.h
arch/sparc/include/uapi/asm/byteorder.h

And these are always little-endian:
arch/alpha/include/uapi/asm/byteorder.h
arch/blackfin/include/uapi/asm/byteorder.h
arch/cris/include/uapi/asm/byteorder.h
arch/hexagon/include/uapi/asm/byteorder.h
arch/ia64/include/uapi/asm/byteorder.h
arch/metag/include/uapi/asm/byteorder.h
arch/mn10300/include/uapi/asm/byteorder.h
arch/score/include/uapi/asm/byteorder.h
arch/unicore32/include/uapi/asm/byteorder.h
arch/x86/include/uapi/asm/byteorder.h

So if we 'select CPU_BIG_ENDIAN' from avr32, frv, m68k, openrisc, parisc,
s390 and sparc, this covers all the fixed-endian architectures, and the
other ones are those that already have either CPU_BIG_ENDIAN
as a 'bool' option, or both as a 'choice'.

> I can make it as a separate patch. But I can only test SPARC and little bit
> of x86. Is that ok?

I think that's ok.

        Arnd

[toc] | [prev] | [next] | [standalone]


#1649780 — Re: CPU_BIG_ENDIAN in generic code (was: Re: [PATCH v3 3/7] arch/sparc: Define config parameter CPU_BIG_ENDIAN)

FromBabu Moger <babu.moger@oracle.com>
Date2017-05-24 19:10 +0200
SubjectRe: CPU_BIG_ENDIAN in generic code (was: Re: [PATCH v3 3/7] arch/sparc: Define config parameter CPU_BIG_ENDIAN)
Message-ID<tKHXl-58r-35@gated-at.bofh.it>
In reply to#1649697
On 5/24/2017 10:09 AM, Arnd Bergmann wrote:
> On Wed, May 24, 2017 at 4:45 PM, Babu Moger <babu.moger@oracle.com> wrote:
>> On 5/24/2017 5:18 AM, Arnd Bergmann wrote:
>>> On Wed, May 24, 2017 at 11:59 AM, Geert Uytterhoeven
>>> <geert@linux-m68k.org> wrote:
>>>> On Tue, May 23, 2017 at 11:45 PM, Babu Moger <babu.moger@oracle.com>
>>>> wrote:
>>>> include/asm-generic/qrwlock.h is also generic, but depends on the
>>>> architecture to select ARCH_USE_QUEUED_RWLOCKS, which only very few do
>>>> (x86, and now sparc).
>>>>
>>>> I guess the time is ripe for adding (both) symbols to all architectures?
>>> Good idea. I think we can do most of this by adding a few lines to
>>> arch/Kconfig:
>>>
>>> config CPU_BIG_ENDIAN
>>>           bool
>>>
>>> config CPU_LITTLE_ENDIAN
>>>          def_bool !CPU_BIG_ENDIAN
>> I noticed that even x86 does not define CPU_LITTLE_ENDIAN.  Strange.
> There is no architecture-independent code that tests for
> CONFIG_CPU_LITTLE_ENDIAN, unlike CONFIG_CPU_BIG_ENDIAN,
> so that's not very suprising.

Ok. Thanks
>> With this code all the architecture will default to
>> CONFIG_CPU_LITTLE_ENDIAN.
> What I meant is that we have to 'select CPU_BIG_ENDIAN' on all architectures
> that actually are big-endian:
Ok. Sure.
>
> These are all configurable:
> $ git grep -l linux/byteorder/big_endian.h | xargs grep -l
> linux/byteorder/little_endian.h
> arch/arc/include/uapi/asm/byteorder.h
> arch/arm/include/uapi/asm/byteorder.h
> arch/arm64/include/uapi/asm/byteorder.h
> arch/c6x/include/uapi/asm/byteorder.h
> arch/m32r/include/uapi/asm/byteorder.h
> arch/microblaze/include/uapi/asm/byteorder.h
> arch/mips/include/uapi/asm/byteorder.h
> arch/powerpc/include/uapi/asm/byteorder.h
> arch/sh/include/uapi/asm/byteorder.h
> arch/tile/include/uapi/asm/byteorder.h
>
> These are always big-endian:
> $ git grep -l linux/byteorder/big_endian.h | xargs grep -L
> linux/byteorder/little_endian.h
> arch/avr32/include/uapi/asm/byteorder.h
> arch/frv/include/uapi/asm/byteorder.h
> arch/m68k/include/uapi/asm/byteorder.h
> arch/openrisc/include/uapi/asm/byteorder.h
> arch/parisc/include/uapi/asm/byteorder.h
> arch/s390/include/uapi/asm/byteorder.h
> arch/sparc/include/uapi/asm/byteorder.h
>
> And these are always little-endian:
> arch/alpha/include/uapi/asm/byteorder.h
> arch/blackfin/include/uapi/asm/byteorder.h
> arch/cris/include/uapi/asm/byteorder.h
> arch/hexagon/include/uapi/asm/byteorder.h
> arch/ia64/include/uapi/asm/byteorder.h
> arch/metag/include/uapi/asm/byteorder.h
> arch/mn10300/include/uapi/asm/byteorder.h
> arch/score/include/uapi/asm/byteorder.h
> arch/unicore32/include/uapi/asm/byteorder.h
> arch/x86/include/uapi/asm/byteorder.h
>
> So if we 'select CPU_BIG_ENDIAN' from avr32, frv, m68k, openrisc, parisc,
> s390 and sparc, this covers all the fixed-endian architectures, and the
> other ones are those that already have either CPU_BIG_ENDIAN
> as a 'bool' option, or both as a 'choice'.
Ok.  Great details. I think I have all the details required for the 
first version.  Will post it soon. Thanks

>> I can make it as a separate patch. But I can only test SPARC and little bit
>> of x86. Is that ok?
> I think that's ok.
>
>          Arnd

[toc] | [prev] | [next] | [standalone]


#1650572 — Re: CPU_BIG_ENDIAN in generic code (was: Re: [PATCH v3 3/7] arch/sparc: Define config parameter CPU_BIG_ENDIAN)

FromBabu Moger <babu.moger@oracle.com>
Date2017-05-25 17:00 +0200
SubjectRe: CPU_BIG_ENDIAN in generic code (was: Re: [PATCH v3 3/7] arch/sparc: Define config parameter CPU_BIG_ENDIAN)
Message-ID<tL2p4-13a-19@gated-at.bofh.it>
In reply to#1649414
Hi Arnd,

Here is the draft patch. Some questions below.


On 5/24/2017 5:18 AM, Arnd Bergmann wrote:
> On Wed, May 24, 2017 at 11:59 AM, Geert Uytterhoeven
> <geert@linux-m68k.org> wrote:
>> On Tue, May 23, 2017 at 11:45 PM, Babu Moger <babu.moger@oracle.com> wrote:
>>> Found this problem while enabling queued rwlock on SPARC.
>>> The parameter CONFIG_CPU_BIG_ENDIAN is used to clear the
>>> specific byte in qrwlock structure. Without this parameter,
>>> we clear the wrong byte. Here is the code.
>>>
>>> static inline u8 *__qrwlock_write_byte(struct qrwlock *lock)
>>>   {
>>>          return (u8 *)lock + 3 * IS_BUILTIN(CONFIG_CPU_BIG_ENDIAN);
>>>   }
>>>
>>> Define CPU_BIG_ENDIAN for SPARC to fix it.
>>> --- a/arch/sparc/Kconfig
>>> +++ b/arch/sparc/Kconfig
>>> @@ -92,6 +92,10 @@ config ARCH_DEFCONFIG
>>>   config ARCH_PROC_KCORE_TEXT
>>>          def_bool y
>>>
>>> +config CPU_BIG_ENDIAN
>>> +       bool
>>> +       default y if SPARC
>> Nice catch!
>>
>> Traditionally, CPU_BIG_ENDIAN and CPU_LITTLE_ENDIAN were defined only on
>> architectures that may support both.  And it was checked in platform code
>> and drivers only.
>> Hence the symbol is lacking from most architectures. Heck, even
>> architectures that support both may default to one endiannes, and declare
>> only the symbol for the other endianness:
>>
>> --- arch/alpha ---
>> --- arch/arc ---
>> arch/arc/Kconfig:config CPU_BIG_ENDIAN
>> --- arch/arm ---
>> arch/arm/mm/Kconfig:config CPU_BIG_ENDIAN
>> --- arch/arm64 ---
>> arch/arm64/Kconfig:config CPU_BIG_ENDIAN
>> --- arch/blackfin ---
>> --- arch/c6x ---
>> arch/c6x/Kconfig:config CPU_BIG_ENDIAN
>> --- arch/cris ---
>> --- arch/frv ---
>> --- arch/h8300 ---
>> --- arch/hexagon ---
>> --- arch/ia64 ---
>> --- arch/Kconfig ---
>> --- arch/m32r ---
>> arch/m32r/Kconfig:config CPU_LITTLE_ENDIAN
>> --- arch/m68k ---
>> --- arch/metag ---
>> --- arch/microblaze ---
>> --- arch/mips ---
>> arch/mips/Kconfig:config CPU_BIG_ENDIAN
>> arch/mips/Kconfig:config CPU_LITTLE_ENDIAN
>> --- arch/mn10300 ---
>> --- arch/nios2 ---
>> --- arch/openrisc ---
>> --- arch/parisc ---
>> --- arch/powerpc ---
>> arch/powerpc/platforms/Kconfig.cputype:config CPU_BIG_ENDIAN
>> arch/powerpc/platforms/Kconfig.cputype:config CPU_LITTLE_ENDIAN
>> --- arch/s390 ---
>> arch/s390/Kconfig:config CPU_BIG_ENDIAN
>> --- arch/score ---
>> --- arch/sh ---
>> arch/sh/Kconfig.cpu:config CPU_LITTLE_ENDIAN
>> arch/sh/Kconfig.cpu:config CPU_BIG_ENDIAN
>> --- arch/sparc ---
>> --- arch/tile ---
>> --- arch/um ---
>> --- arch/unicore32 ---
>> --- arch/x86 ---
>> --- arch/xtensa ---
>>
>> However, there are already a few users in generic code, which are thus
>> broken on many platforms:
>>
>>      drivers/of/base.c
>>      drivers/of/fdt.c
>>      drivers/tty/serial/earlycon.c
>>      drivers/tty/serial/serial_core.c
>>
>> include/asm-generic/qrwlock.h is also generic, but depends on the
>> architecture to select ARCH_USE_QUEUED_RWLOCKS, which only very few do
>> (x86, and now sparc).
>>
>> I guess the time is ripe for adding (both) symbols to all architectures?
> Good idea. I think we can do most of this by adding a few lines to
> arch/Kconfig:
>
> config CPU_BIG_ENDIAN
>          bool
>
> config CPU_LITTLE_ENDIAN
>         def_bool !CPU_BIG_ENDIAN
>
> This way, we only need to add 'select CPU_BIG_ENDIAN' to the
> architectures that are always big-endian, and we don't need to
> change anything for the ones that have a single 'CPU_BIG_ENDIAN'
> option.
>
> The three architectures that have a 'choice' statement (mips, ppc and
> sh) will have to convert, and m32r will have to replace the

what to you mean by "(mips, ppc andsh) will have to convert"?  Do you 
expect any changes here?

> option with the opposite one, which could break 'make oldconfig',
> but nobody really cares about m32r any more.
>
>         Arnd
Here is the proposed draft patch
======================================
---
  arch/Kconfig          |    6 ++++++
  arch/frv/Kconfig      |    1 +
  arch/h8300/Kconfig    |    1 +
  arch/m32r/Kconfig     |    6 +++---
  arch/m68k/Kconfig     |    1 +
  arch/openrisc/Kconfig |    1 +
  arch/parisc/Kconfig   |    1 +
  arch/s390/Kconfig     |    1 +
  arch/sparc/Kconfig    |    1 +
  9 files changed, 16 insertions(+), 3 deletions(-)

diff --git a/arch/Kconfig b/arch/Kconfig
index 6c00e5b..19fcafd 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -867,4 +867,10 @@ config STRICT_MODULE_RWX
  config ARCH_WANT_RELAX_ORDER
         bool

+config CPU_BIG_ENDIAN
+        bool
+
+config CPU_LITTLE_ENDIAN
+       def_bool !CPU_BIG_ENDIAN
+
  source "kernel/gcov/Kconfig"
diff --git a/arch/frv/Kconfig b/arch/frv/Kconfig
index eefd9a4..db258a4 100644
--- a/arch/frv/Kconfig
+++ b/arch/frv/Kconfig
@@ -16,6 +16,7 @@ config FRV
         select OLD_SIGACTION
         select HAVE_DEBUG_STACKOVERFLOW
         select ARCH_NO_COHERENT_DMA_MMAP
+       select CPU_BIG_ENDIAN

  config ZONE_DMA
         bool
diff --git a/arch/h8300/Kconfig b/arch/h8300/Kconfig
index 3ae8525..22ebf28 100644
--- a/arch/h8300/Kconfig
+++ b/arch/h8300/Kconfig
@@ -22,6 +22,7 @@ config H8300
         select HAVE_ARCH_KGDB
         select HAVE_ARCH_HASH
         select CPU_NO_EFFICIENT_FFS
+       select CPU_BIG_ENDIAN

  config RWSEM_GENERIC_SPINLOCK
         def_bool y
diff --git a/arch/m32r/Kconfig b/arch/m32r/Kconfig
index 9547446..d57e37b 100644
--- a/arch/m32r/Kconfig
+++ b/arch/m32r/Kconfig
@@ -193,9 +193,9 @@ config TIMER_DIVIDE
         int "Timer divider (integer)"
         default "128"

-config CPU_LITTLE_ENDIAN
-        bool "Generate little endian code"
-       default n
+config CPU_BIG_ENDIAN
+        bool "Generate big endian code"
+       default y

  config MEMORY_START
         hex "Physical memory start address (hex)"
diff --git a/arch/m68k/Kconfig b/arch/m68k/Kconfig
index d140206..b44579b 100644
--- a/arch/m68k/Kconfig
+++ b/arch/m68k/Kconfig
@@ -22,6 +22,7 @@ config M68K
         select MODULES_USE_ELF_RELA
         select OLD_SIGSUSPEND3
         select OLD_SIGACTION
+       select CPU_BIG_ENDIAN

  config RWSEM_GENERIC_SPINLOCK
         bool
diff --git a/arch/openrisc/Kconfig b/arch/openrisc/Kconfig
index 1e95920..d772983 100644
--- a/arch/openrisc/Kconfig
+++ b/arch/openrisc/Kconfig
@@ -28,6 +28,7 @@ config OPENRISC
         select OR1K_PIC
         select CPU_NO_EFFICIENT_FFS if !OPENRISC_HAVE_INST_FF1
         select NO_BOOTMEM
+       select CPU_BIG_ENDIAN

  config MMU
         def_bool y
diff --git a/arch/parisc/Kconfig b/arch/parisc/Kconfig
index 531da9e..7a8ec28 100644
--- a/arch/parisc/Kconfig
+++ b/arch/parisc/Kconfig
@@ -40,6 +40,7 @@ config PARISC
         select GENERIC_CLOCKEVENTS
         select ARCH_NO_COHERENT_DMA_MMAP
         select CPU_NO_EFFICIENT_FFS
+       select CPU_BIG_ENDIAN

         help
           The PA-RISC microprocessor is designed by Hewlett-Packard and 
used
diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig
index e161faf..169e50b 100644
--- a/arch/s390/Kconfig
+++ b/arch/s390/Kconfig
@@ -177,6 +177,7 @@ config S390
         select ARCH_HAS_SCALED_CPUTIME
         select VIRT_TO_BUS
         select HAVE_NMI
+       select CPU_BIG_ENDIAN


  config SCHED_OMIT_FRAME_POINTER
diff --git a/arch/sparc/Kconfig b/arch/sparc/Kconfig
index c5b5a7b..7a28e8a 100644
--- a/arch/sparc/Kconfig
+++ b/arch/sparc/Kconfig
@@ -45,6 +45,7 @@ config SPARC
         select CPU_NO_EFFICIENT_FFS
         select LOCKDEP_SMALL if LOCKDEP
         select ARCH_WANT_RELAX_ORDER
+       select CPU_BIG_ENDIAN

  config SPARC32
         def_bool !64BIT
--
1.7.1

[toc] | [prev] | [next] | [standalone]


#1650796 — Re: CPU_BIG_ENDIAN in generic code (was: Re: [PATCH v3 3/7] arch/sparc: Define config parameter CPU_BIG_ENDIAN)

FromArnd Bergmann <arnd@arndb.de>
Date2017-05-25 22:20 +0200
SubjectRe: CPU_BIG_ENDIAN in generic code (was: Re: [PATCH v3 3/7] arch/sparc: Define config parameter CPU_BIG_ENDIAN)
Message-ID<tL7oJ-4nj-5@gated-at.bofh.it>
In reply to#1650572
On Thu, May 25, 2017 at 4:51 PM, Babu Moger <babu.moger@oracle.com> wrote:
> On 5/24/2017 5:18 AM, Arnd Bergmann wrote:

>>> I guess the time is ripe for adding (both) symbols to all architectures?
>>
>> Good idea. I think we can do most of this by adding a few lines to
>> arch/Kconfig:
>>
>> config CPU_BIG_ENDIAN
>>          bool
>>
>> config CPU_LITTLE_ENDIAN
>>         def_bool !CPU_BIG_ENDIAN
>>
>> This way, we only need to add 'select CPU_BIG_ENDIAN' to the
>> architectures that are always big-endian, and we don't need to
>> change anything for the ones that have a single 'CPU_BIG_ENDIAN'
>> option.
>>
>> The three architectures that have a 'choice' statement (mips, ppc and
>> sh) will have to convert, and m32r will have to replace the
>
>
> what to you mean by "(mips, ppc andsh) will have to convert"?  Do you expect
> any changes here?
>

Kconfig does not allow you to have the same symbol as both a regular
'bool' and also 'bool within choice', so those three have to replace the
choice with a user-visible 'config CPU_BIG_ENDIAN' option like the
other ones have.

I also notice that  for arch/s390/Kconfig you now have both the
'select CPU_BIG_ENDIAN' and the 'config CPU_BIG_ENDIAN
def_bool y', I'd remove the second one in the same patch.

       Arnd

[toc] | [prev] | [next] | [standalone]


#1650799 — Re: CPU_BIG_ENDIAN in generic code (was: Re: [PATCH v3 3/7] arch/sparc: Define config parameter CPU_BIG_ENDIAN)

FromBabu Moger <babu.moger@oracle.com>
Date2017-05-25 22:30 +0200
SubjectRe: CPU_BIG_ENDIAN in generic code (was: Re: [PATCH v3 3/7] arch/sparc: Define config parameter CPU_BIG_ENDIAN)
Message-ID<tL7yp-4sO-3@gated-at.bofh.it>
In reply to#1650796
On 5/25/2017 3:09 PM, Arnd Bergmann wrote:
> On Thu, May 25, 2017 at 4:51 PM, Babu Moger <babu.moger@oracle.com> wrote:
>> On 5/24/2017 5:18 AM, Arnd Bergmann wrote:
>>>> I guess the time is ripe for adding (both) symbols to all architectures?
>>> Good idea. I think we can do most of this by adding a few lines to
>>> arch/Kconfig:
>>>
>>> config CPU_BIG_ENDIAN
>>>           bool
>>>
>>> config CPU_LITTLE_ENDIAN
>>>          def_bool !CPU_BIG_ENDIAN
>>>
>>> This way, we only need to add 'select CPU_BIG_ENDIAN' to the
>>> architectures that are always big-endian, and we don't need to
>>> change anything for the ones that have a single 'CPU_BIG_ENDIAN'
>>> option.
>>>
>>> The three architectures that have a 'choice' statement (mips, ppc and
>>> sh) will have to convert, and m32r will have to replace the
>>
>> what to you mean by "(mips, ppc andsh) will have to convert"?  Do you expect
>> any changes here?
>>
> Kconfig does not allow you to have the same symbol as both a regular
> 'bool' and also 'bool within choice', so those three have to replace the
> choice with a user-visible 'config CPU_BIG_ENDIAN' option like the
> other ones have.

Ok. I will address it in my  next version.  Thanks

>
> I also notice that  for arch/s390/Kconfig you now have both the
> 'select CPU_BIG_ENDIAN' and the 'config CPU_BIG_ENDIAN
> def_bool y', I'd remove the second one in the same patch.

Sure. Will correct it.
>         Arnd

[toc] | [prev] | [next] | [standalone]


#1650936 — Re: CPU_BIG_ENDIAN in generic code (was: Re: [PATCH v3 3/7] arch/sparc: Define config parameter CPU_BIG_ENDIAN)

FromMax Filippov <jcmvbkbc@gmail.com>
Date2017-05-26 00:30 +0200
SubjectRe: CPU_BIG_ENDIAN in generic code (was: Re: [PATCH v3 3/7] arch/sparc: Define config parameter CPU_BIG_ENDIAN)
Message-ID<tL9qy-5KR-15@gated-at.bofh.it>
In reply to#1649414
On Wed, May 24, 2017 at 3:18 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Wed, May 24, 2017 at 11:59 AM, Geert Uytterhoeven
> <geert@linux-m68k.org> wrote:
>> I guess the time is ripe for adding (both) symbols to all architectures?
>
> Good idea. I think we can do most of this by adding a few lines to
> arch/Kconfig:
>
> config CPU_BIG_ENDIAN
>         bool
>
> config CPU_LITTLE_ENDIAN
>        def_bool !CPU_BIG_ENDIAN
>
> This way, we only need to add 'select CPU_BIG_ENDIAN' to the
> architectures that are always big-endian, and we don't need to
> change anything for the ones that have a single 'CPU_BIG_ENDIAN'
> option.
>
> The three architectures that have a 'choice' statement (mips, ppc and
> sh) will have to convert, and m32r will have to replace the
> option with the opposite one, which could break 'make oldconfig',
> but nobody really cares about m32r any more.

Xtensa may have either endianness and for xtensa we define
CONFIG_CPU_BIG_ENDIAN or CONFIG_CPU_LITTLE_ENDIAN
in the arch/xtensa/Makefile based on the value of the compiler builtin
macro.

-- 
Thanks.
-- Max

[toc] | [prev] | [next] | [standalone]


#1650956 — Re: CPU_BIG_ENDIAN in generic code (was: Re: [PATCH v3 3/7] arch/sparc: Define config parameter CPU_BIG_ENDIAN)

FromBabu Moger <babu.moger@oracle.com>
Date2017-05-26 00:50 +0200
SubjectRe: CPU_BIG_ENDIAN in generic code (was: Re: [PATCH v3 3/7] arch/sparc: Define config parameter CPU_BIG_ENDIAN)
Message-ID<tL9JT-5SC-9@gated-at.bofh.it>
In reply to#1650936
On 5/25/2017 5:27 PM, Max Filippov wrote:
> On Wed, May 24, 2017 at 3:18 AM, Arnd Bergmann <arnd@arndb.de> wrote:
>> On Wed, May 24, 2017 at 11:59 AM, Geert Uytterhoeven
>> <geert@linux-m68k.org> wrote:
>>> I guess the time is ripe for adding (both) symbols to all architectures?
>> Good idea. I think we can do most of this by adding a few lines to
>> arch/Kconfig:
>>
>> config CPU_BIG_ENDIAN
>>          bool
>>
>> config CPU_LITTLE_ENDIAN
>>         def_bool !CPU_BIG_ENDIAN
>>
>> This way, we only need to add 'select CPU_BIG_ENDIAN' to the
>> architectures that are always big-endian, and we don't need to
>> change anything for the ones that have a single 'CPU_BIG_ENDIAN'
>> option.
>>
>> The three architectures that have a 'choice' statement (mips, ppc and
>> sh) will have to convert, and m32r will have to replace the
>> option with the opposite one, which could break 'make oldconfig',
>> but nobody really cares about m32r any more.
> Xtensa may have either endianness and for xtensa we define
> CONFIG_CPU_BIG_ENDIAN or CONFIG_CPU_LITTLE_ENDIAN
> in the arch/xtensa/Makefile based on the value of the compiler builtin
> macro.

Hmm.. That means defining  CPU_LITTLE_ENDIAN  based on "def_bool 
!CPU_BIG_ENDIAN" will
  be a problem for Xtensa because menuconfig does not have the knowledge 
of compiler builtin macro.
Is that correct?

[toc] | [prev] | [next] | [standalone]


#1650959 — Re: CPU_BIG_ENDIAN in generic code (was: Re: [PATCH v3 3/7] arch/sparc: Define config parameter CPU_BIG_ENDIAN)

FromMax Filippov <jcmvbkbc@gmail.com>
Date2017-05-26 01:00 +0200
SubjectRe: CPU_BIG_ENDIAN in generic code (was: Re: [PATCH v3 3/7] arch/sparc: Define config parameter CPU_BIG_ENDIAN)
Message-ID<tL9Tz-5Wn-1@gated-at.bofh.it>
In reply to#1650956
On Thu, May 25, 2017 at 3:41 PM, Babu Moger <babu.moger@oracle.com> wrote:
> On 5/25/2017 5:27 PM, Max Filippov wrote:
>> Xtensa may have either endianness and for xtensa we define
>> CONFIG_CPU_BIG_ENDIAN or CONFIG_CPU_LITTLE_ENDIAN
>> in the arch/xtensa/Makefile based on the value of the compiler builtin
>> macro.
>
> Hmm.. That means defining  CPU_LITTLE_ENDIAN  based on "def_bool
> !CPU_BIG_ENDIAN" will
>  be a problem for Xtensa because menuconfig does not have the knowledge of
> compiler builtin macro.
> Is that correct?

I think so. OTOH outside the arch/ CPU_LITTLE_ENDIAN is only used in
two Kconfig files:

drivers/crypto/nx/Kconfig
drivers/isdn/hisax/Kconfig

both of which are irrelevant for xtensa.

-- 
Thanks.
-- Max

[toc] | [prev] | [next] | [standalone]


#1650957 — Re: CPU_BIG_ENDIAN in generic code (was: Re: [PATCH v3 3/7] arch/sparc: Define config parameter CPU_BIG_ENDIAN)

FromMax Filippov <jcmvbkbc@gmail.com>
Date2017-05-26 00:50 +0200
SubjectRe: CPU_BIG_ENDIAN in generic code (was: Re: [PATCH v3 3/7] arch/sparc: Define config parameter CPU_BIG_ENDIAN)
Message-ID<tL9JT-5SC-11@gated-at.bofh.it>
In reply to#1650936
On Thu, May 25, 2017 at 3:27 PM, Max Filippov <jcmvbkbc@gmail.com> wrote:
> On Wed, May 24, 2017 at 3:18 AM, Arnd Bergmann <arnd@arndb.de> wrote:
>> On Wed, May 24, 2017 at 11:59 AM, Geert Uytterhoeven
>> <geert@linux-m68k.org> wrote:
>>> I guess the time is ripe for adding (both) symbols to all architectures?
>>
>> Good idea. I think we can do most of this by adding a few lines to
>> arch/Kconfig:
>>
>> config CPU_BIG_ENDIAN
>>         bool
>>
>> config CPU_LITTLE_ENDIAN
>>        def_bool !CPU_BIG_ENDIAN
>>
>> This way, we only need to add 'select CPU_BIG_ENDIAN' to the
>> architectures that are always big-endian, and we don't need to
>> change anything for the ones that have a single 'CPU_BIG_ENDIAN'
>> option.
>>
>> The three architectures that have a 'choice' statement (mips, ppc and
>> sh) will have to convert, and m32r will have to replace the
>> option with the opposite one, which could break 'make oldconfig',
>> but nobody really cares about m32r any more.
>
> Xtensa may have either endianness and for xtensa we define
> CONFIG_CPU_BIG_ENDIAN or CONFIG_CPU_LITTLE_ENDIAN
> in the arch/xtensa/Makefile based on the value of the compiler builtin
> macro.

Also, in outside the Kconfig files there's much more instances of
__{BIG,LITTLE}_ENDIAN than CONFIG_CPU_{BIG,LITTLE}_ENDIAN:

$ git grep '__\(BIG\|LITTLE\)_ENDIAN' | wc -l
4537
$ git grep 'CONFIG_CPU_\(BIG\|LITTLE\)_ENDIAN' | wc -l
247

My understanding is that CONFIG_CPU_{BIG,LITTLE}_ENDIAN was
intended to be used only in Kconfig files, and perhaps all of its uses
outside should be replaced with __{BIG,LITTLE}_ENDIAN.

-- 
Thanks.
-- Max

[toc] | [prev] | [next] | [standalone]


#1652503 — Re: CPU_BIG_ENDIAN in generic code (was: Re: [PATCH v3 3/7] arch/sparc: Define config parameter CPU_BIG_ENDIAN)

FromArnd Bergmann <arnd@arndb.de>
Date2017-05-29 15:00 +0200
SubjectRe: CPU_BIG_ENDIAN in generic code (was: Re: [PATCH v3 3/7] arch/sparc: Define config parameter CPU_BIG_ENDIAN)
Message-ID<tMsr8-8qg-9@gated-at.bofh.it>
In reply to#1650957
On Fri, May 26, 2017 at 12:43 AM, Max Filippov <jcmvbkbc@gmail.com> wrote:
> On Thu, May 25, 2017 at 3:27 PM, Max Filippov <jcmvbkbc@gmail.com> wrote:
>> On Wed, May 24, 2017 at 3:18 AM, Arnd Bergmann <arnd@arndb.de> wrote:
>>> On Wed, May 24, 2017 at 11:59 AM, Geert Uytterhoeven
>>> <geert@linux-m68k.org> wrote:
>>>> I guess the time is ripe for adding (both) symbols to all architectures?
>>>
>>> Good idea. I think we can do most of this by adding a few lines to
>>> arch/Kconfig:
>>>
>>> config CPU_BIG_ENDIAN
>>>         bool
>>>
>>> config CPU_LITTLE_ENDIAN
>>>        def_bool !CPU_BIG_ENDIAN
>>>
>>> This way, we only need to add 'select CPU_BIG_ENDIAN' to the
>>> architectures that are always big-endian, and we don't need to
>>> change anything for the ones that have a single 'CPU_BIG_ENDIAN'
>>> option.
>>>
>>> The three architectures that have a 'choice' statement (mips, ppc and
>>> sh) will have to convert, and m32r will have to replace the
>>> option with the opposite one, which could break 'make oldconfig',
>>> but nobody really cares about m32r any more.
>>
>> Xtensa may have either endianness and for xtensa we define
>> CONFIG_CPU_BIG_ENDIAN or CONFIG_CPU_LITTLE_ENDIAN
>> in the arch/xtensa/Makefile based on the value of the compiler builtin
>> macro.

I can sort of see why xtensa did it the other way round from everyone
else (letting the toolchain decide what the kernel is, rather than letting
the kernel pass the respective flags to gcc), but I'd argue that it would
be better overall for xtensa to change over so we do it consistently
on all architectures.

> Also, in outside the Kconfig files there's much more instances of
> __{BIG,LITTLE}_ENDIAN than CONFIG_CPU_{BIG,LITTLE}_ENDIAN:
>
> $ git grep '__\(BIG\|LITTLE\)_ENDIAN' | wc -l
> 4537
> $ git grep 'CONFIG_CPU_\(BIG\|LITTLE\)_ENDIAN' | wc -l
> 247
>
> My understanding is that CONFIG_CPU_{BIG,LITTLE}_ENDIAN was
> intended to be used only in Kconfig files, and perhaps all of its uses
> outside should be replaced with __{BIG,LITTLE}_ENDIAN.

Right, but I also think that using the CONFIG_CPU_* symbols in
code makes sense because it is less ambiguous: the way we use
__{BIG,LITTLE}_ENDIAN in the kernel is different from how user
space uses them in glibc, and this confuses everyone when they
try to use them in the kernel after being familiar with the traditional
way. The Kconfig symbols don't have this problem.

       Arnd

[toc] | [prev] | [next] | [standalone]


#1652468 — Re: CPU_BIG_ENDIAN in generic code (was: Re: [PATCH v3 3/7] arch/sparc: Define config parameter CPU_BIG_ENDIAN)

FromMichael Ellerman <mpe@ellerman.id.au>
Date2017-05-29 14:10 +0200
SubjectRe: CPU_BIG_ENDIAN in generic code (was: Re: [PATCH v3 3/7] arch/sparc: Define config parameter CPU_BIG_ENDIAN)
Message-ID<tMrEJ-865-19@gated-at.bofh.it>
In reply to#1649398
Geert Uytterhoeven <geert@linux-m68k.org> writes:

> On Tue, May 23, 2017 at 11:45 PM, Babu Moger <babu.moger@oracle.com> wrote:
>> Found this problem while enabling queued rwlock on SPARC.
>> The parameter CONFIG_CPU_BIG_ENDIAN is used to clear the
>> specific byte in qrwlock structure. Without this parameter,
>> we clear the wrong byte. Here is the code.
>>
>> static inline u8 *__qrwlock_write_byte(struct qrwlock *lock)
>>  {
>>         return (u8 *)lock + 3 * IS_BUILTIN(CONFIG_CPU_BIG_ENDIAN);
>>  }
>>
>> Define CPU_BIG_ENDIAN for SPARC to fix it.
>
>> --- a/arch/sparc/Kconfig
>> +++ b/arch/sparc/Kconfig
>> @@ -92,6 +92,10 @@ config ARCH_DEFCONFIG
>>  config ARCH_PROC_KCORE_TEXT
>>         def_bool y
>>
>> +config CPU_BIG_ENDIAN
>> +       bool
>> +       default y if SPARC
>
> Nice catch!
>
> Traditionally, CPU_BIG_ENDIAN and CPU_LITTLE_ENDIAN were defined only on
> architectures that may support both.  And it was checked in platform code
> and drivers only.
> Hence the symbol is lacking from most architectures. Heck, even
> architectures that support both may default to one endiannes, and declare
> only the symbol for the other endianness:

I guess there's a reason we can't use __BIG_ENDIAN__ / __LITTLE_ENDIAN__ ?

cheers

[toc] | [prev] | [next] | [standalone]


#1652484 — Re: CPU_BIG_ENDIAN in generic code (was: Re: [PATCH v3 3/7] arch/sparc: Define config parameter CPU_BIG_ENDIAN)

FromGeert Uytterhoeven <geert@linux-m68k.org>
Date2017-05-29 14:20 +0200
SubjectRe: CPU_BIG_ENDIAN in generic code (was: Re: [PATCH v3 3/7] arch/sparc: Define config parameter CPU_BIG_ENDIAN)
Message-ID<tMrOq-8a6-15@gated-at.bofh.it>
In reply to#1652468
Hi Michael,

On Mon, May 29, 2017 at 2:07 PM, Michael Ellerman <mpe@ellerman.id.au> wrote:
> Geert Uytterhoeven <geert@linux-m68k.org> writes:
>> On Tue, May 23, 2017 at 11:45 PM, Babu Moger <babu.moger@oracle.com> wrote:
>>> Found this problem while enabling queued rwlock on SPARC.
>>> The parameter CONFIG_CPU_BIG_ENDIAN is used to clear the
>>> specific byte in qrwlock structure. Without this parameter,
>>> we clear the wrong byte. Here is the code.
>>>
>>> static inline u8 *__qrwlock_write_byte(struct qrwlock *lock)
>>>  {
>>>         return (u8 *)lock + 3 * IS_BUILTIN(CONFIG_CPU_BIG_ENDIAN);
>>>  }
>>>
>>> Define CPU_BIG_ENDIAN for SPARC to fix it.
>>
>>> --- a/arch/sparc/Kconfig
>>> +++ b/arch/sparc/Kconfig
>>> @@ -92,6 +92,10 @@ config ARCH_DEFCONFIG
>>>  config ARCH_PROC_KCORE_TEXT
>>>         def_bool y
>>>
>>> +config CPU_BIG_ENDIAN
>>> +       bool
>>> +       default y if SPARC
>>
>> Nice catch!
>>
>> Traditionally, CPU_BIG_ENDIAN and CPU_LITTLE_ENDIAN were defined only on
>> architectures that may support both.  And it was checked in platform code
>> and drivers only.
>> Hence the symbol is lacking from most architectures. Heck, even
>> architectures that support both may default to one endiannes, and declare
>> only the symbol for the other endianness:
>
> I guess there's a reason we can't use __BIG_ENDIAN__ / __LITTLE_ENDIAN__ ?

I (C/asm) code we can, in Kconfig we cannot.

So far we tried always doing that, but a few checks for the semi-existing
Kconfig symbol crept in in generic code. Those could be replaced by the __*__
variants, but consistently having the Kconfig symbols would be useful anyway
(e.g. to avoid building the broken-on-big-endian ISDN drivers).

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

[toc] | [prev] | [next] | [standalone]


#1652784 — Re: CPU_BIG_ENDIAN in generic code (was: Re: [PATCH v3 3/7] arch/sparc: Define config parameter CPU_BIG_ENDIAN)

FromMichael Ellerman <mpe@ellerman.id.au>
Date2017-05-30 05:00 +0200
SubjectRe: CPU_BIG_ENDIAN in generic code (was: Re: [PATCH v3 3/7] arch/sparc: Define config parameter CPU_BIG_ENDIAN)
Message-ID<tMFy2-12b-3@gated-at.bofh.it>
In reply to#1652484
Geert Uytterhoeven <geert@linux-m68k.org> writes:

> Hi Michael,
>
> On Mon, May 29, 2017 at 2:07 PM, Michael Ellerman <mpe@ellerman.id.au> wrote:
>> Geert Uytterhoeven <geert@linux-m68k.org> writes:
>>> On Tue, May 23, 2017 at 11:45 PM, Babu Moger <babu.moger@oracle.com> wrote:
>>>> Found this problem while enabling queued rwlock on SPARC.
>>>> The parameter CONFIG_CPU_BIG_ENDIAN is used to clear the
>>>> specific byte in qrwlock structure. Without this parameter,
>>>> we clear the wrong byte. Here is the code.
>>>>
>>>> static inline u8 *__qrwlock_write_byte(struct qrwlock *lock)
>>>>  {
>>>>         return (u8 *)lock + 3 * IS_BUILTIN(CONFIG_CPU_BIG_ENDIAN);
>>>>  }
>>>>
>>>> Define CPU_BIG_ENDIAN for SPARC to fix it.
>>>
>>>> --- a/arch/sparc/Kconfig
>>>> +++ b/arch/sparc/Kconfig
>>>> @@ -92,6 +92,10 @@ config ARCH_DEFCONFIG
>>>>  config ARCH_PROC_KCORE_TEXT
>>>>         def_bool y
>>>>
>>>> +config CPU_BIG_ENDIAN
>>>> +       bool
>>>> +       default y if SPARC
>>>
>>> Nice catch!
>>>
>>> Traditionally, CPU_BIG_ENDIAN and CPU_LITTLE_ENDIAN were defined only on
>>> architectures that may support both.  And it was checked in platform code
>>> and drivers only.
>>> Hence the symbol is lacking from most architectures. Heck, even
>>> architectures that support both may default to one endiannes, and declare
>>> only the symbol for the other endianness:
>>
>> I guess there's a reason we can't use __BIG_ENDIAN__ / __LITTLE_ENDIAN__ ?
>
> I (C/asm) code we can, in Kconfig we cannot.
>
> So far we tried always doing that, but a few checks for the semi-existing
> Kconfig symbol crept in in generic code. Those could be replaced by the __*__
> variants, but consistently having the Kconfig symbols would be useful anyway
> (e.g. to avoid building the broken-on-big-endian ISDN drivers).

Ah OK, the original mail was citing C code, but yeah I guess it would be
handy in Makefiles etc.

cheers

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web