Le 20/10/2020 à 20:32, Greg KH a écrit : > On Tue, Oct 20, 2020 at 08:19:26PM +0200, Laurent Vivier wrote: >> Le 20/10/2020 à 19:37, Greg KH a écrit : >>> On Tue, Oct 20, 2020 at 06:37:41PM +0200, Laurent Vivier wrote: >>>> Le 20/10/2020 à 18:28, Greg KH a écrit : >>>>> On Tue, Oct 20, 2020 at 06:23:03PM +0200, Laurent Vivier wrote: >>>>>> 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 <laurent@xxxxxxxxx> >>>>>> --- >>>>>> 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; >>>>>> +#endif >>>>> >>>>> Why 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 done >> I would be happy to do the same. >> >>> 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? Thanks, Laurent