Hi Finn,
On 31/7/19 3:36 pm, Finn Thain wrote:
On Mon, 29 Jul 2019,gerg@xxxxxxxxxx wrote:
From: Greg Ungerer<gerg@xxxxxxxxxxxxxx>
Commit d3b41b6bb49e ("m68k: Dispatch nvram_ops calls to Atari
or Mac functions") causes a number of compile time warnings
to be generated if compiling for a ColdFire MMU enabled target:
In file included from ./arch/m68k/include/asm/atarihw.h:25:0,
from arch/m68k/kernel/setup_mm.c:41,
from arch/m68k/kernel/setup.c:3:
./arch/m68k/include/asm/raw_io.h:39:0: warning: "__raw_readb" redefined
#define __raw_readb in_8
^
In file included from ./arch/m68k/include/asm/io.h:6:0,
from arch/m68k/kernel/setup_mm.c:36,
from arch/m68k/kernel/setup.c:3:
./arch/m68k/include/asm/io_no.h:16:0: note: this is the location of the previous definition
#define __raw_readb(addr) \
^
...
It appears that I neglected to build test that patch for coldfire. Sorry
about that.
:-)
The most strait forward fix is to conditionaly include only
those headers actually required, and to only check for
machine types that are configured/enabled into this build.
Signed-off-by: Greg Ungerer<gerg@xxxxxxxxxxxxxx>
---
arch/m68k/kernel/setup_mm.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/arch/m68k/kernel/setup_mm.c b/arch/m68k/kernel/setup_mm.c
index 528484feff80..04853f68f7a8 100644
--- a/arch/m68k/kernel/setup_mm.c
+++ b/arch/m68k/kernel/setup_mm.c
@@ -38,14 +38,16 @@
#ifdef CONFIG_AMIGA
#include <asm/amigahw.h>
#endif
-#include <asm/atarihw.h>
#ifdef CONFIG_ATARI
+#include <asm/atarihw.h>
#include <asm/atari_stram.h>
#endif
Is that change not sufficient to avoid the new warnings?
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);
^
#ifdef CONFIG_SUN3X
#include <asm/dvma.h>
#endif
+#ifdef CONFIG_MAC
#include <asm/macintosh.h>
+#endif
#include <asm/natfeat.h>
#if !FPSTATESIZE || !NR_IRQS
Can we avoid this ifdef?
This ifdef fixes the warnings I listed above.
@@ -602,10 +604,14 @@ static long m68k_nvram_initialize(void)
static ssize_t m68k_nvram_get_size(void)
{
+#ifdef CONFIG_ATARI
if (MACH_IS_ATARI)
return atari_nvram_get_size();
- else if (MACH_IS_MAC)
+#endif
+#ifdef CONFIG_MAC
+ if (MACH_IS_MAC)
return mac_pram_get_size();
+#endif
The MACH_IS_ATARI and MACH_IS_MAC macros already appear unconditionally in
this file in m68k_parse_bootinfo(). Can we avoid these ifdefs too?
If the MACH_IS_* macros can no longer be used unconditionally, would it
not be better to find a way to allow this?
It is not the presence of the macros that ends up being the problem.
It is the functions, atari_nvram_get_size() and mac_pram_get_size().
Without these ifdefs here if you try to compile for the mac_defconfig
you get the following errors at compile time:
CC arch/m68k/kernel/setup.o
In file included from arch/m68k/kernel/setup.c:3:0:
arch/m68k/kernel/setup_mm.c: In function ‘m68k_nvram_get_size’:
arch/m68k/kernel/setup_mm.c:608:10: error: implicit declaration of function ‘atari_nvram_get_size’ [-Werror=implicit-function-declaration]
return atari_nvram_get_size();
^
cc1: some warnings being treated as errors
scripts/Makefile.build:273: recipe for target 'arch/m68k/kernel/setup.o' failed
make[1]: *** [arch/m68k/kernel/setup.o] Error 1
The MACH_IS_ATARI is not guaranteed to be compile time constant,
depending on what target options you have configured.
Regards
Greg
