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);
}
Here is the previous discussion.
http://www.spinics.net/lists/devicetree/msg178101.html
Based on the discussion, it was decided to add CONFIG_CPU_BIG_ENDIAN
for all the fixed big endian architecture(frv, h8300, m68k, openrisc,
parisc and sparc). And warn if there are inconsistencies in this definition.
Babu Moger (2):
arch: Define CPU_BIG_ENDIAN for all fixed big endian archs
include: warn for inconsistent endian config definition
arch/frv/Kconfig | 3 +++
arch/h8300/Kconfig | 3 +++
arch/m68k/Kconfig | 3 +++
arch/openrisc/Kconfig | 3 +++
arch/parisc/Kconfig | 3 +++
arch/sparc/Kconfig | 3 +++
include/linux/byteorder/big_endian.h | 4 ++++
include/linux/byteorder/little_endian.h | 4 ++++
8 files changed, 26 insertions(+), 0 deletions(-)
Display warning if CPU_BIG_ENDIAN is not defined on big endian
architecture and also warn if it defined on little endian architectures.
We have seen some generic code(for example code include/asm-generic/qrwlock.h)
uses CONFIG_CPU_BIG_ENDIAN to decide the endianess.
Here is the original discussion
http://www.spinics.net/lists/devicetree/msg178101.html
Signed-off-by: Babu Moger <[email protected]>
Suggested-by: Arnd Bergmann <[email protected]>
---
include/linux/byteorder/big_endian.h | 4 ++++
include/linux/byteorder/little_endian.h | 4 ++++
2 files changed, 8 insertions(+), 0 deletions(-)
diff --git a/include/linux/byteorder/big_endian.h b/include/linux/byteorder/big_endian.h
index 3920414..ffd2159 100644
--- a/include/linux/byteorder/big_endian.h
+++ b/include/linux/byteorder/big_endian.h
@@ -3,5 +3,9 @@
#include <uapi/linux/byteorder/big_endian.h>
+#ifndef CONFIG_CPU_BIG_ENDIAN
+#warning inconsistent configuration, needs CONFIG_CPU_BIG_ENDIAN
+#endif
+
#include <linux/byteorder/generic.h>
#endif /* _LINUX_BYTEORDER_BIG_ENDIAN_H */
diff --git a/include/linux/byteorder/little_endian.h b/include/linux/byteorder/little_endian.h
index 0805737..ba910bb 100644
--- a/include/linux/byteorder/little_endian.h
+++ b/include/linux/byteorder/little_endian.h
@@ -3,5 +3,9 @@
#include <uapi/linux/byteorder/little_endian.h>
+#ifdef CONFIG_CPU_BIG_ENDIAN
+#warning inconsistent configuration, CONFIG_CPU_BIG_ENDIAN is set
+#endif
+
#include <linux/byteorder/generic.h>
#endif /* _LINUX_BYTEORDER_LITTLE_ENDIAN_H */
--
1.7.1
While working on enabling queued rwlock on SPARC, found
this following code in include/asm-generic/qrwlock.h
which uses CONFIG_CPU_BIG_ENDIAN to clear a byte.
static inline u8 *__qrwlock_write_byte(struct qrwlock *lock)
{
return (u8 *)lock + 3 * IS_BUILTIN(CONFIG_CPU_BIG_ENDIAN);
}
Problem is many of the fixed big endian architectures dont define
CPU_BIG_ENDIAN and clears the wrong byte.
Define CPU_BIG_ENDIAN for all the fixed big endian architecture.
Here is the orinal discussion
http://www.spinics.net/lists/devicetree/msg178101.html
Signed-off-by: Babu Moger <[email protected]>
Suggested-by: Arnd Bergmann <[email protected]>
---
arch/frv/Kconfig | 3 +++
arch/h8300/Kconfig | 3 +++
arch/m68k/Kconfig | 3 +++
arch/openrisc/Kconfig | 3 +++
arch/parisc/Kconfig | 3 +++
arch/sparc/Kconfig | 3 +++
6 files changed, 18 insertions(+), 0 deletions(-)
diff --git a/arch/frv/Kconfig b/arch/frv/Kconfig
index eefd9a4..1cce824 100644
--- a/arch/frv/Kconfig
+++ b/arch/frv/Kconfig
@@ -17,6 +17,9 @@ config FRV
select HAVE_DEBUG_STACKOVERFLOW
select ARCH_NO_COHERENT_DMA_MMAP
+config CPU_BIG_ENDIAN
+ def_bool y
+
config ZONE_DMA
bool
default y
diff --git a/arch/h8300/Kconfig b/arch/h8300/Kconfig
index 3ae8525..5380ac8 100644
--- a/arch/h8300/Kconfig
+++ b/arch/h8300/Kconfig
@@ -23,6 +23,9 @@ config H8300
select HAVE_ARCH_HASH
select CPU_NO_EFFICIENT_FFS
+config CPU_BIG_ENDIAN
+ def_bool y
+
config RWSEM_GENERIC_SPINLOCK
def_bool y
diff --git a/arch/m68k/Kconfig b/arch/m68k/Kconfig
index d140206..029a58b 100644
--- a/arch/m68k/Kconfig
+++ b/arch/m68k/Kconfig
@@ -23,6 +23,9 @@ config M68K
select OLD_SIGSUSPEND3
select OLD_SIGACTION
+config CPU_BIG_ENDIAN
+ def_bool y
+
config RWSEM_GENERIC_SPINLOCK
bool
default y
diff --git a/arch/openrisc/Kconfig b/arch/openrisc/Kconfig
index 1e95920..a0f2e4a 100644
--- a/arch/openrisc/Kconfig
+++ b/arch/openrisc/Kconfig
@@ -29,6 +29,9 @@ config OPENRISC
select CPU_NO_EFFICIENT_FFS if !OPENRISC_HAVE_INST_FF1
select NO_BOOTMEM
+config CPU_BIG_ENDIAN
+ def_bool y
+
config MMU
def_bool y
diff --git a/arch/parisc/Kconfig b/arch/parisc/Kconfig
index 531da9e..dda1f55 100644
--- a/arch/parisc/Kconfig
+++ b/arch/parisc/Kconfig
@@ -47,6 +47,9 @@ config PARISC
and later HP3000 series). The PA-RISC Linux project home page is
at <http://www.parisc-linux.org/>.
+config CPU_BIG_ENDIAN
+ def_bool y
+
config MMU
def_bool y
diff --git a/arch/sparc/Kconfig b/arch/sparc/Kconfig
index 58243b0..eb213b5 100644
--- a/arch/sparc/Kconfig
+++ b/arch/sparc/Kconfig
@@ -92,6 +92,9 @@ config ARCH_DEFCONFIG
config ARCH_PROC_KCORE_TEXT
def_bool y
+config CPU_BIG_ENDIAN
+ def_bool y
+
config ARCH_ATU
bool
default y if SPARC64
--
1.7.1
On Fri, Jun 9, 2017 at 12:17 AM, Babu Moger <[email protected]> wrote:
> While working on enabling queued rwlock on SPARC, found
> this following code in include/asm-generic/qrwlock.h
> which uses CONFIG_CPU_BIG_ENDIAN to clear a byte.
>
> static inline u8 *__qrwlock_write_byte(struct qrwlock *lock)
> {
> return (u8 *)lock + 3 * IS_BUILTIN(CONFIG_CPU_BIG_ENDIAN);
> }
>
> Problem is many of the fixed big endian architectures dont define
> CPU_BIG_ENDIAN and clears the wrong byte.
>
> Define CPU_BIG_ENDIAN for all the fixed big endian architecture.
>
> Here is the orinal discussion
> http://www.spinics.net/lists/devicetree/msg178101.html
>
> Signed-off-by: Babu Moger <[email protected]>
> Suggested-by: Arnd Bergmann <[email protected]>
Hmm, the link above refers to a mail from me? ;-)
Acked-by: Geert Uytterhoeven <[email protected]>
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
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
On Fri, Jun 9, 2017 at 12:17 AM, Babu Moger <[email protected]> wrote:
> Display warning if CPU_BIG_ENDIAN is not defined on big endian
> architecture and also warn if it defined on little endian architectures.
>
> We have seen some generic code(for example code include/asm-generic/qrwlock.h)
> uses CONFIG_CPU_BIG_ENDIAN to decide the endianess.
That example is IMHO the least harmful, as qrwlock must be selected explicitly
by the architecture.
The uses in
drivers/of/base.c
drivers/of/fdt.c
drivers/tty/serial/earlycon.c
drivers/tty/serial/serial_core.c
are more dangerous, and may have bitten people already.
In addition, people may have worked around them in DT, so this series may
actually introduce regressions.
> Here is the original discussion
> http://www.spinics.net/lists/devicetree/msg178101.html
>
> Signed-off-by: Babu Moger <[email protected]>
> Suggested-by: Arnd Bergmann <[email protected]>
Hmm, the link above refers to a mail from me? ;-)
Acked-by: Geert Uytterhoeven <[email protected]>
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
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
Hi Babu,
On Fri, Jun 9, 2017 at 9:05 AM, Geert Uytterhoeven <[email protected]> wrote:
>
>> Here is the original discussion
>> http://www.spinics.net/lists/devicetree/msg178101.html
>>
>> Signed-off-by: Babu Moger <[email protected]>
>> Suggested-by: Arnd Bergmann <[email protected]>
>
> Hmm, the link above refers to a mail from me? ;-)
Please ignore that comment. I accidentally copied one line too much
from the other reply.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
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
Geert,
On 6/9/2017 2:16 AM, Geert Uytterhoeven wrote:
> Hi Babu,
>
> On Fri, Jun 9, 2017 at 9:05 AM, Geert Uytterhoeven <[email protected]> wrote:
>>> Here is the original discussion
>>> http://www.spinics.net/lists/devicetree/msg178101.html
>>>
>>> Signed-off-by: Babu Moger <[email protected]>
>>> Suggested-by: Arnd Bergmann <[email protected]>
>> Hmm, the link above refers to a mail from me? ;-)
> Please ignore that comment. I accidentally copied one line too much
> from the other reply.
Yes. Got it. So patch #1 is fine. But, patch #2 might cause
regressions. Should I drop patch 2.
>
> Gr{oetje,eeting}s,
>
> Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
>
> 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
Hi Babu,
On Fri, Jun 9, 2017 at 3:55 PM, Babu Moger <[email protected]> wrote:
> On 6/9/2017 2:16 AM, Geert Uytterhoeven wrote:
>> On Fri, Jun 9, 2017 at 9:05 AM, Geert Uytterhoeven <[email protected]>
>> wrote:
>>>> Here is the original discussion
>>>> http://www.spinics.net/lists/devicetree/msg178101.html
>>>>
>>>> Signed-off-by: Babu Moger <[email protected]>
>>>> Suggested-by: Arnd Bergmann <[email protected]>
> Yes. Got it. So patch #1 is fine. But, patch #2 might cause regressions.
> Should I drop patch 2.
No, it should be applied, and regressions should be fixed.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
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
On 6/9/2017 9:11 AM, Geert Uytterhoeven wrote:
> Hi Babu,
>
> On Fri, Jun 9, 2017 at 3:55 PM, Babu Moger <[email protected]> wrote:
>> On 6/9/2017 2:16 AM, Geert Uytterhoeven wrote:
>>> On Fri, Jun 9, 2017 at 9:05 AM, Geert Uytterhoeven <[email protected]>
>>> wrote:
>>>>> Here is the original discussion
>>>>> http://www.spinics.net/lists/devicetree/msg178101.html
>>>>>
>>>>> Signed-off-by: Babu Moger <[email protected]>
>>>>> Suggested-by: Arnd Bergmann <[email protected]>
>> Yes. Got it. So patch #1 is fine. But, patch #2 might cause regressions.
>> Should I drop patch 2.
> No, it should be applied, and regressions should be fixed.
Geert, Ok. Sure. I will resubmit the patch mentioning all the
files(base.c, fdt.c etc..) that are affected by this change.
thanks
> Gr{oetje,eeting}s,
>
> Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
>
> 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
On 6/9/2017 2:03 AM, Geert Uytterhoeven wrote:
> On Fri, Jun 9, 2017 at 12:17 AM, Babu Moger <[email protected]> wrote:
>> While working on enabling queued rwlock on SPARC, found
>> this following code in include/asm-generic/qrwlock.h
>> which uses CONFIG_CPU_BIG_ENDIAN to clear a byte.
>>
>> static inline u8 *__qrwlock_write_byte(struct qrwlock *lock)
>> {
>> return (u8 *)lock + 3 * IS_BUILTIN(CONFIG_CPU_BIG_ENDIAN);
>> }
>>
>> Problem is many of the fixed big endian architectures dont define
>> CPU_BIG_ENDIAN and clears the wrong byte.
>>
>> Define CPU_BIG_ENDIAN for all the fixed big endian architecture.
>>
>> Here is the orinal discussion
>> http://www.spinics.net/lists/devicetree/msg178101.html
>>
>> Signed-off-by: Babu Moger <[email protected]>
>> Suggested-by: Arnd Bergmann <[email protected]>
> Hmm, the link above refers to a mail from me? ;-)
>
> Acked-by: Geert Uytterhoeven <[email protected]>
One more question before I resubmit.
Dave,
I have added CONFIG_CPU_BIG_ENDIAN for sparc via my earlier patch.
https://patchwork.ozlabs.org/patch/766735/
Should I exclude sparc here?
Thanks
Babu
>
> Gr{oetje,eeting}s,
>
> Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
>
> 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
From: Babu Moger <[email protected]>
Date: Thu, 8 Jun 2017 15:17:22 -0700
> While working on enabling queued rwlock on SPARC, found
> this following code in include/asm-generic/qrwlock.h
> which uses CONFIG_CPU_BIG_ENDIAN to clear a byte.
>
> static inline u8 *__qrwlock_write_byte(struct qrwlock *lock)
> {
> return (u8 *)lock + 3 * IS_BUILTIN(CONFIG_CPU_BIG_ENDIAN);
> }
>
> Problem is many of the fixed big endian architectures dont define
> CPU_BIG_ENDIAN and clears the wrong byte.
>
> Define CPU_BIG_ENDIAN for all the fixed big endian architecture.
>
> Here is the orinal discussion
> http://www.spinics.net/lists/devicetree/msg178101.html
>
> Signed-off-by: Babu Moger <[email protected]>
> Suggested-by: Arnd Bergmann <[email protected]>
Acked-by: David S. Miller <[email protected]>
Hi Babu,
[auto build test WARNING on linus/master]
[also build test WARNING on v4.12-rc4 next-20170609]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Babu-Moger/Define-CPU_BIG_ENDIAN-or-warn-for-inconsistencies/20170610-200424
config: microblaze-mmu_defconfig (attached as .config)
compiler: microblaze-linux-gcc (GCC) 6.2.0
reproduce:
wget https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=microblaze
All warnings (new ones prefixed by >>):
In file included from arch/microblaze/include/uapi/asm/byteorder.h:7:0,
from include/asm-generic/bitops/le.h:5,
from include/asm-generic/bitops.h:34,
from arch/microblaze/include/asm/bitops.h:1,
from include/linux/bitops.h:36,
from include/linux/kernel.h:10,
from include/asm-generic/bug.h:15,
from arch/microblaze/include/asm/bug.h:1,
from include/linux/bug.h:4,
from include/linux/page-flags.h:9,
from kernel/bounds.c:9:
>> include/linux/byteorder/big_endian.h:7:2: warning: #warning inconsistent configuration, needs CONFIG_CPU_BIG_ENDIAN [-Wcpp]
#warning inconsistent configuration, needs CONFIG_CPU_BIG_ENDIAN
^~~~~~~
--
In file included from arch/microblaze/include/uapi/asm/byteorder.h:7:0,
from include/asm-generic/bitops/le.h:5,
from include/asm-generic/bitops.h:34,
from arch/microblaze/include/asm/bitops.h:1,
from include/linux/bitops.h:36,
from include/linux/kernel.h:10,
from include/asm-generic/bug.h:15,
from arch/microblaze/include/asm/bug.h:1,
from include/linux/bug.h:4,
from include/linux/page-flags.h:9,
from kernel/bounds.c:9:
>> include/linux/byteorder/big_endian.h:7:2: warning: #warning inconsistent configuration, needs CONFIG_CPU_BIG_ENDIAN [-Wcpp]
#warning inconsistent configuration, needs CONFIG_CPU_BIG_ENDIAN
^~~~~~~
In file included from arch/microblaze/include/uapi/asm/byteorder.h:7:0,
from include/asm-generic/bitops/le.h:5,
from include/asm-generic/bitops.h:34,
from arch/microblaze/include/asm/bitops.h:1,
from include/linux/bitops.h:36,
from include/linux/kernel.h:10,
from include/linux/list.h:8,
from include/linux/rculist.h:9,
from include/linux/pid.h:4,
from include/linux/sched.h:13,
from arch/microblaze/kernel/asm-offsets.c:13:
>> include/linux/byteorder/big_endian.h:7:2: warning: #warning inconsistent configuration, needs CONFIG_CPU_BIG_ENDIAN [-Wcpp]
#warning inconsistent configuration, needs CONFIG_CPU_BIG_ENDIAN
^~~~~~~
<stdin>:1326:2: warning: #warning syscall statx not implemented [-Wcpp]
vim +7 include/linux/byteorder/big_endian.h
1 #ifndef _LINUX_BYTEORDER_BIG_ENDIAN_H
2 #define _LINUX_BYTEORDER_BIG_ENDIAN_H
3
4 #include <uapi/linux/byteorder/big_endian.h>
5
6 #ifndef CONFIG_CPU_BIG_ENDIAN
> 7 #warning inconsistent configuration, needs CONFIG_CPU_BIG_ENDIAN
8 #endif
9
10 #include <linux/byteorder/generic.h>
11 #endif /* _LINUX_BYTEORDER_BIG_ENDIAN_H */
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
Hi All,
On 6/10/2017 9:06 AM, kbuild test robot wrote:
> Hi Babu,
>
> [auto build test WARNING on linus/master]
> [also build test WARNING on v4.12-rc4 next-20170609]
> [if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
>
> url: https://github.com/0day-ci/linux/commits/Babu-Moger/Define-CPU_BIG_ENDIAN-or-warn-for-inconsistencies/20170610-200424
> config: microblaze-mmu_defconfig (attached as .config)
> compiler: microblaze-linux-gcc (GCC) 6.2.0
> reproduce:
> wget https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
> chmod +x ~/bin/make.cross
> # save the attached .config to linux build tree
> make.cross ARCH=microblaze
>
> All warnings (new ones prefixed by >>):
>
> In file included from arch/microblaze/include/uapi/asm/byteorder.h:7:0,
> from include/asm-generic/bitops/le.h:5,
> from include/asm-generic/bitops.h:34,
> from arch/microblaze/include/asm/bitops.h:1,
> from include/linux/bitops.h:36,
> from include/linux/kernel.h:10,
> from include/asm-generic/bug.h:15,
> from arch/microblaze/include/asm/bug.h:1,
> from include/linux/bug.h:4,
> from include/linux/page-flags.h:9,
> from kernel/bounds.c:9:
>>> include/linux/byteorder/big_endian.h:7:2: warning: #warning inconsistent configuration, needs CONFIG_CPU_BIG_ENDIAN [-Wcpp]
> #warning inconsistent configuration, needs CONFIG_CPU_BIG_ENDIAN
> ^~~~~~~
> --
> In file included from arch/microblaze/include/uapi/asm/byteorder.h:7:0,
> from include/asm-generic/bitops/le.h:5,
> from include/asm-generic/bitops.h:34,
> from arch/microblaze/include/asm/bitops.h:1,
> from include/linux/bitops.h:36,
> from include/linux/kernel.h:10,
> from include/asm-generic/bug.h:15,
> from arch/microblaze/include/asm/bug.h:1,
> from include/linux/bug.h:4,
> from include/linux/page-flags.h:9,
> from kernel/bounds.c:9:
>>> include/linux/byteorder/big_endian.h:7:2: warning: #warning inconsistent configuration, needs CONFIG_CPU_BIG_ENDIAN [-Wcpp]
> #warning inconsistent configuration, needs CONFIG_CPU_BIG_ENDIAN
> ^~~~~~~
> In file included from arch/microblaze/include/uapi/asm/byteorder.h:7:0,
> from include/asm-generic/bitops/le.h:5,
> from include/asm-generic/bitops.h:34,
> from arch/microblaze/include/asm/bitops.h:1,
> from include/linux/bitops.h:36,
> from include/linux/kernel.h:10,
> from include/linux/list.h:8,
> from include/linux/rculist.h:9,
> from include/linux/pid.h:4,
> from include/linux/sched.h:13,
> from arch/microblaze/kernel/asm-offsets.c:13:
>>> include/linux/byteorder/big_endian.h:7:2: warning: #warning inconsistent configuration, needs CONFIG_CPU_BIG_ENDIAN [-Wcpp]
> #warning inconsistent configuration, needs CONFIG_CPU_BIG_ENDIAN
> ^~~~~~~
> <stdin>:1326:2: warning: #warning syscall statx not implemented [-Wcpp]
>
> vim +7 include/linux/byteorder/big_endian.h
>
> 1 #ifndef _LINUX_BYTEORDER_BIG_ENDIAN_H
> 2 #define _LINUX_BYTEORDER_BIG_ENDIAN_H
> 3
> 4 #include <uapi/linux/byteorder/big_endian.h>
> 5
> 6 #ifndef CONFIG_CPU_BIG_ENDIAN
> > 7 #warning inconsistent configuration, needs CONFIG_CPU_BIG_ENDIAN
> 8 #endif
> 9
> 10 #include <linux/byteorder/generic.h>
> 11 #endif /* _LINUX_BYTEORDER_BIG_ENDIAN_H */
>
> ---
> 0-DAY kernel test infrastructure Open Source Technology Center
> https://lists.01.org/pipermail/kbuild-all Intel Corporation
Looks like microblaze can be configured to either little or big endian
formats. How about
adding a choice statement to address this.
Here is my proposed patch.
=======================================
diff --git a/arch/microblaze/Kconfig b/arch/microblaze/Kconfig
index 85885a5..74aa5de 100644
--- a/arch/microblaze/Kconfig
+++ b/arch/microblaze/Kconfig
@@ -35,6 +35,22 @@ config MICROBLAZE
select VIRT_TO_BUS
select CPU_NO_EFFICIENT_FFS
+# Endianness selection
+choice
+ prompt "Endianness selection"
+ default CPU_BIG_ENDIAN
+ help
+ microblaze architectures can be configured for either little or
+ big endian formats. Be sure to select the appropriate mode.
+
+config CPU_BIG_ENDIAN
+ bool "Big endian"
+
+config CPU_LITTLE_ENDIAN
+ bool "Little endian"
+
+endchoice
+
config SWAP
def_bool n
On Mon, Jun 12, 2017 at 10:30 PM, Babu Moger <[email protected]> wrote:
>
> Looks like microblaze can be configured to either little or big endian
> formats. How about
> adding a choice statement to address this.
> Here is my proposed patch.
Hi Babu,
This part looks fine, but I think we also need this one:
diff --git a/arch/microblaze/Makefile b/arch/microblaze/Makefile
index 740f2b82a182..1f6c486826a0 100644
--- a/arch/microblaze/Makefile
+++ b/arch/microblaze/Makefile
@@ -35,6 +35,8 @@ endif
CPUFLAGS-$(CONFIG_XILINX_MICROBLAZE0_USE_DIV) += -mno-xl-soft-div
CPUFLAGS-$(CONFIG_XILINX_MICROBLAZE0_USE_BARREL) += -mxl-barrel-shift
CPUFLAGS-$(CONFIG_XILINX_MICROBLAZE0_USE_PCMP_INSTR) += -mxl-pattern-compare
+CPUFLAGS-$(CONFIG_BIG_ENDIAN) += -mbig-endian
+CPUFLAGS-$(CONFIG_LITTLE_ENDIAN) += -mlittle-endian
CPUFLAGS-1 += $(call cc-option,-mcpu=v$(CPU_VER))
That way, we don't have to guess what the toolchain does, but rather
tell it to do whatever is configured, like we do for most other architectures.
Unfortunately we can't do the same thing on xtensa, as that no longer
supports the -mbig-endian/-mbig-endian flags in any recent gcc version
(a long time ago it had them, but they were removed along with many other
options).
Arnd
On Mon, Jun 12, 2017 at 1:51 PM, Arnd Bergmann <[email protected]> wrote:
> That way, we don't have to guess what the toolchain does, but rather
> tell it to do whatever is configured, like we do for most other architectures.
>
> Unfortunately we can't do the same thing on xtensa, as that no longer
> supports the -mbig-endian/-mbig-endian flags in any recent gcc version
> (a long time ago it had them, but they were removed along with many other
> options).
For xtensa we probably need to generate Kconfig fragment that would go
in with the variant subdirectory. That will solve this, and clean up other
options that we currently have for manual selection for xtensa, but there's
actually no choice, i.e. the option has to be selected correctly, there's only
one correct choice and otherwise the kernel either won't build or won't work.
I'll look into it.
--
Thanks.
-- Max
On 6/12/2017 3:51 PM, Arnd Bergmann wrote:
> On Mon, Jun 12, 2017 at 10:30 PM, Babu Moger <[email protected]> wrote:
>> Looks like microblaze can be configured to either little or big endian
>> formats. How about
>> adding a choice statement to address this.
>> Here is my proposed patch.
> Hi Babu,
>
> This part looks fine, but I think we also need this one:
>
> diff --git a/arch/microblaze/Makefile b/arch/microblaze/Makefile
> index 740f2b82a182..1f6c486826a0 100644
> --- a/arch/microblaze/Makefile
> +++ b/arch/microblaze/Makefile
> @@ -35,6 +35,8 @@ endif
> CPUFLAGS-$(CONFIG_XILINX_MICROBLAZE0_USE_DIV) += -mno-xl-soft-div
> CPUFLAGS-$(CONFIG_XILINX_MICROBLAZE0_USE_BARREL) += -mxl-barrel-shift
> CPUFLAGS-$(CONFIG_XILINX_MICROBLAZE0_USE_PCMP_INSTR) += -mxl-pattern-compare
> +CPUFLAGS-$(CONFIG_BIG_ENDIAN) += -mbig-endian
> +CPUFLAGS-$(CONFIG_LITTLE_ENDIAN) += -mlittle-endian
>
> CPUFLAGS-1 += $(call cc-option,-mcpu=v$(CPU_VER))
>
>
> That way, we don't have to guess what the toolchain does, but rather
> tell it to do whatever is configured, like we do for most other architectures.
Ok. Thanks. Arnd. Will update and resend the series.
>
> Unfortunately we can't do the same thing on xtensa, as that no longer
> supports the -mbig-endian/-mbig-endian flags in any recent gcc version
> (a long time ago it had them, but they were removed along with many other
> options).
>
> Arnd
On 6/12/2017 3:58 PM, Max Filippov wrote:
> On Mon, Jun 12, 2017 at 1:51 PM, Arnd Bergmann <[email protected]> wrote:
>> That way, we don't have to guess what the toolchain does, but rather
>> tell it to do whatever is configured, like we do for most other architectures.
>>
>> Unfortunately we can't do the same thing on xtensa, as that no longer
>> supports the -mbig-endian/-mbig-endian flags in any recent gcc version
>> (a long time ago it had them, but they were removed along with many other
>> options).
> For xtensa we probably need to generate Kconfig fragment that would go
> in with the variant subdirectory. That will solve this, and clean up other
> options that we currently have for manual selection for xtensa, but there's
> actually no choice, i.e. the option has to be selected correctly, there's only
> one correct choice and otherwise the kernel either won't build or won't work.
> I'll look into it.
Max. Thanks. Please update us when you are done.
>