diff mbox

[2/2] include: warn for inconsistent endian config definition

Message ID 1496960243-196898-3-git-send-email-babu.moger@oracle.com
State Not Applicable
Delegated to: David Miller
Headers show

Commit Message

Babu Moger June 8, 2017, 10:17 p.m. UTC 
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 <babu.moger@oracle.com>
Suggested-by: Arnd Bergmann <arnd@arndb.de>
---
 include/linux/byteorder/big_endian.h    |    4 ++++
 include/linux/byteorder/little_endian.h |    4 ++++
 2 files changed, 8 insertions(+), 0 deletions(-)

Comments

Geert Uytterhoeven June 9, 2017, 7:05 a.m. UTC | #1 
On Fri, Jun 9, 2017 at 12:17 AM, Babu Moger <babu.moger@oracle.com> 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 <babu.moger@oracle.com>
> Suggested-by: Arnd Bergmann <arnd@arndb.de>

Hmm, the link above refers to a mail from me? ;-)

Acked-by: Geert Uytterhoeven <geert@linux-m68k.org>

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
--
To unsubscribe from this list: send the line "unsubscribe sparclinux" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Geert Uytterhoeven June 9, 2017, 7:16 a.m. UTC | #2 
Hi Babu,

On Fri, Jun 9, 2017 at 9:05 AM, Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>
>> Here is the original discussion
>> http://www.spinics.net/lists/devicetree/msg178101.html
>>
>> Signed-off-by: Babu Moger <babu.moger@oracle.com>
>> Suggested-by: Arnd Bergmann <arnd@arndb.de>
>
> 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 -- 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
--
To unsubscribe from this list: send the line "unsubscribe sparclinux" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Babu Moger June 9, 2017, 1:55 p.m. UTC | #3 
Geert,

On 6/9/2017 2:16 AM, Geert Uytterhoeven wrote:
> Hi Babu,
>
> On Fri, Jun 9, 2017 at 9:05 AM, Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>>> Here is the original discussion
>>> http://www.spinics.net/lists/devicetree/msg178101.html
>>>
>>> Signed-off-by: Babu Moger <babu.moger@oracle.com>
>>> Suggested-by: Arnd Bergmann <arnd@arndb.de>
>> 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 -- 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

--
To unsubscribe from this list: send the line "unsubscribe sparclinux" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Geert Uytterhoeven June 9, 2017, 2:11 p.m. UTC | #4 
Hi Babu,

On Fri, Jun 9, 2017 at 3:55 PM, Babu Moger <babu.moger@oracle.com> wrote:
> On 6/9/2017 2:16 AM, Geert Uytterhoeven wrote:
>> On Fri, Jun 9, 2017 at 9:05 AM, Geert Uytterhoeven <geert@linux-m68k.org>
>> wrote:
>>>> Here is the original discussion
>>>> http://www.spinics.net/lists/devicetree/msg178101.html
>>>>
>>>> Signed-off-by: Babu Moger <babu.moger@oracle.com>
>>>> Suggested-by: Arnd Bergmann <arnd@arndb.de>

> 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 -- 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
--
To unsubscribe from this list: send the line "unsubscribe sparclinux" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Babu Moger June 9, 2017, 2:39 p.m. UTC | #5 
On 6/9/2017 9:11 AM, Geert Uytterhoeven wrote:
> Hi Babu,
>
> On Fri, Jun 9, 2017 at 3:55 PM, Babu Moger <babu.moger@oracle.com> wrote:
>> On 6/9/2017 2:16 AM, Geert Uytterhoeven wrote:
>>> On Fri, Jun 9, 2017 at 9:05 AM, Geert Uytterhoeven <geert@linux-m68k.org>
>>> wrote:
>>>>> Here is the original discussion
>>>>> http://www.spinics.net/lists/devicetree/msg178101.html
>>>>>
>>>>> Signed-off-by: Babu Moger <babu.moger@oracle.com>
>>>>> Suggested-by: Arnd Bergmann <arnd@arndb.de>
>> 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 -- 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

--
To unsubscribe from this list: send the line "unsubscribe sparclinux" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
kernel test robot June 10, 2017, 2:06 p.m. UTC | #6 
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
diff mbox

Patch

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 */