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

[Arnd Bergmann]

On Wed, May 24, 2017 at 11:59 AM, Geert Uytterhoeven
<geert@xxxxxxxxxxxxxx> wrote:
> On Tue, May 23, 2017 at 11:45 PM, Babu Moger <babu.moger@xxxxxxxxxx> 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