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. If the convention is to put #ifdef around code like that then I guess that's what we should do here too... Reviewed-by: Finn Thain <fthain@xxxxxxxxxxxxxxxxxxx> --
Regards Greg