Re: [PATCH] m68k: fix ColdFire with MMU compile warnings

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, 31 Jul 2019, Geert Uytterhoeven wrote:

On Wed, Jul 31, 2019 at 9:47 AM Finn Thain <fthain@xxxxxxxxxxxxxxxxxxx> wrote:
On Wed, 31 Jul 2019, Greg Ungerer wrote:
No, not sufficient. You still get the following warnings after
just moving that include of atarihw.h:

  CC      arch/m68k/kernel/setup.o
In file included from arch/m68k/kernel/setup_mm.c:48:0,
                 from arch/m68k/kernel/setup.c:3:
./arch/m68k/include/asm/macintosh.h:19:35: warning: 'struct irq_data'
declared inside parameter list
 extern void mac_irq_enable(struct irq_data *data);
                                   ^
./arch/m68k/include/asm/macintosh.h:19:35: warning: its scope is only this
definition or declaration, which is probably not what you want
./arch/m68k/include/asm/macintosh.h:20:36: warning: 'struct irq_data'
declared inside parameter list
 extern void mac_irq_disable(struct irq_data *data);


The warning can be resolved with,

diff --git a/arch/m68k/include/asm/macintosh.h b/arch/m68k/include/asm/macintosh.h
index d9a08bed4b12..f653b60f2afc 100644
--- a/arch/m68k/include/asm/macintosh.h
+++ b/arch/m68k/include/asm/macintosh.h
@@ -4,6 +4,7 @@

 #include <linux/seq_file.h>
 #include <linux/interrupt.h>
+#include <linux/irq.h>

 #include <asm/bootinfo-mac.h>

so that macintosh.h could be unconditionally included to avoid some
#ifdefs. That's just BTW. I don't object to your solution.


The MACH_IS_ATARI is not guaranteed to be compile time constant,
depending on what target options you have configured.


Actually, MACH_IS_ATARI is a compile-time constant when CONFIG_ATARI=n.

When I build that file with mac_defconfig and gcc 6.4, the C preprocessor
generates this:

   if ((0))
    unknown = amiga_parse_bootinfo(record);
   else if ((0))
    unknown = atari_parse_bootinfo(record);
   else if ((1))
    unknown = mac_parse_bootinfo(record);
   else if ((0))
    unknown = q40_parse_bootinfo(record);
   else if ((0))
    unknown = bvme6000_parse_bootinfo(record);
   else if ((0))
    unknown = mvme16x_parse_bootinfo(record);
   else if ((0))
    unknown = mvme147_parse_bootinfo(record);
   else if ((0))
    unknown = hp300_parse_bootinfo(record);
   else if ((0))
    unknown = apollo_parse_bootinfo(record);
   else
    unknown = 1;

We don't get that "implicit declaration" warning because the function
prototypes are all declared unconditionally at the top of the same file.

Anyway, the warning we were discussing was this one:

arch/m68k/kernel/setup_mm.c: In function 'm68k_nvram_get_size':
arch/m68k/kernel/setup_mm.c:605:10: error: implicit declaration of
function 'atari_nvram_get_size' [-Werror=implicit-function-declaration]
   return atari_nvram_get_size();

This warning is the reason why commit d3b41b6bb49e ("m68k: Dispatch
nvram_ops calls to Atari or Mac functions") unconditionally included
atarihw.h.

It's annoying that we can't unconditionally include atarihw.h but I don't
have a solution for that.

The real issue is <asm/amigahw.h> including <asm/raw_io.h>, right?


I take it you meant to write <asm/atarihw.h> not <asm/amigahw.h>.

At first sight, the only reason for that is:

    #define atari_readb   raw_inb
    #define atari_writeb  raw_outb

    #define atari_inb_p   raw_inb
    #define atari_outb_p  raw_outb

Note that the first definition is unused, and the other 3 have only a handful
users.

At second sight, the <asm/raw_io.h> include can just be removed, an Atari
kernel still builds fine...

Would that fix the issue?


Yes, it seems so. Thanks.

Here's the patch I tested.

diff --git a/arch/m68k/include/asm/atarihw.h b/arch/m68k/include/asm/atarihw.h
index 533008262b69..ba1889c1a933 100644
--- a/arch/m68k/include/asm/atarihw.h
+++ b/arch/m68k/include/asm/atarihw.h
@@ -22,7 +22,6 @@
 
 #include <linux/types.h>
 #include <asm/bootinfo-atari.h>
-#include <asm/raw_io.h>
 #include <asm/kmap.h>
 
 extern u_long atari_mch_cookie;
diff --git a/arch/m68k/include/asm/macintosh.h b/arch/m68k/include/asm/macintosh.h
index d9a08bed4b12..f653b60f2afc 100644
--- a/arch/m68k/include/asm/macintosh.h
+++ b/arch/m68k/include/asm/macintosh.h
@@ -4,6 +4,7 @@
 
 #include <linux/seq_file.h>
 #include <linux/interrupt.h>
+#include <linux/irq.h>
 
 #include <asm/bootinfo-mac.h>
 
I built 4 configurations - coldfire (mmu), atari, mac, atari + mac.

It's hard to imagine that removing those extra #defines would be a problem 
given that doing so doesn't lead to compiler warnings or errors. The atari 
build booted up okay in aranym.

-- 

Thanks!

Gr{oetje,eeting}s,

                        Geert





[Index of Archives]     [Video for Linux]     [Yosemite News]     [Linux S/390]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux