Re: [PATCH] serial: pmac_zilog: don't init if zilog is not available
From: Michael Ellerman <mpe@ellerman.id.au>
Date: 2020-10-22 02:53:18
Also in:
linux-m68k, linux-serial, lkml
Laurent Vivier [off-list ref] writes:
Le 20/10/2020 à 20:32, Greg KH a écrit :quoted
On Tue, Oct 20, 2020 at 08:19:26PM +0200, Laurent Vivier wrote:quoted
Le 20/10/2020 à 19:37, Greg KH a écrit :quoted
On Tue, Oct 20, 2020 at 06:37:41PM +0200, Laurent Vivier wrote:quoted
Le 20/10/2020 à 18:28, Greg KH a écrit :quoted
On Tue, Oct 20, 2020 at 06:23:03PM +0200, Laurent Vivier wrote:quoted
We can avoid to probe for the Zilog device (and generate ugly kernel warning) if kernel is built for Mac but not on a Mac. Signed-off-by: Laurent Vivier <redacted> --- drivers/tty/serial/pmac_zilog.c | 11 +++++++++++ 1 file changed, 11 insertions(+)diff --git a/drivers/tty/serial/pmac_zilog.c b/drivers/tty/serial/pmac_zilog.c index 063484b22523..d1d2e55983c3 100644 --- a/drivers/tty/serial/pmac_zilog.c +++ b/drivers/tty/serial/pmac_zilog.c@@ -1867,6 +1867,12 @@ static struct platform_driver pmz_driver = { static int __init init_pmz(void) { int rc, i; + +#ifdef CONFIG_MAC + if (!MACH_IS_MAC) + return -ENODEV; +#endifWhy is the #ifdef needed? We don't like putting #ifdef in .c files for good reasons. Can you make the api check for this work with and without that #ifdef needed?The #ifdef is needed because this file can be compiled for PowerMac and m68k Mac. For PowerMac, the MACH_IS_MAC is not defined, so we need the #ifdef. We need the MAC_IS_MAC because the same kernel can be used with several m68k machines, so the init_pmz can be called on a m68k machine without the zilog device (it's a multi-targets kernel). You can check it's the good way to do by looking inside: drivers/video/fbdev/valkyriefb.c +317 drivers/macintosh/adb.c +316 That are two files used by both, mac and pmac.Why not fix it to work properly like other arch checks are doneI would be happy to do the same.quoted
Put it in a .h file and do the #ifdef there. Why is this "special"?I don't know. Do you mean something like: drivers/tty/serial/pmac_zilog.h ... #ifndef MACH_IS_MAC #define MACH_IS_MAC (0) #endif ... drivers/tty/serial/pmac_zilog.c ... static int __init pmz_console_init(void) { if (!MACH_IS_MAC) return -ENODEV; ...Yup, that would be a good start, but why is the pmac_zilog.h file responsible for this? Shouldn't this be in some arch-specific file somewhere?For m68k, MACH_IS_MAC is defined in arch/m68k/include/asm/setup.h If I want to define it for any other archs I don't know in which file we can put it. But as m68k mac is only sharing drivers with pmac perhaps we can put this in arch/powerpc/include/asm/setup.h?
It doesn't really belong in there. I'd accept a patch to create arch/powerpc/include/asm/macintosh.h, with MACH_IS_MAC defined in there. cheers