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

From: Babu Moger
Date: Wed Jun 07 2017 - 19:08:50 EST



On 5/29/2017 9:56 PM, Michael Ellerman wrote:
Geert Uytterhoeven <geert@xxxxxxxxxxxxxx> writes:

Hi Michael,

On Mon, May 29, 2017 at 2:07 PM, Michael Ellerman <mpe@xxxxxxxxxxxxxx> wrote:
Geert Uytterhoeven <geert@xxxxxxxxxxxxxx> writes:
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:
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.

Thanks for all the responses. I see couple of options here.

1. Fix the c code in include/asm-generic/qrwlock.h using ifdef __BIG_ENDIAN__
This will fix the issue for us.

2. Define CPU_BIG_ENDIAN for all the missing fixed endian architectures. Because the problem is only for fixed big endian archs.

I prefer the option 1. What do you guys think ?

cheers