Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <d987663bbed18d7dbf106db6066a759040b4e57a.1647837028.git.fthain@linux-m68k.org>
 <5291a177-9a67-b23a-515f-7faa3cc887e8@csgroup.eu> <e8398356-8ba3-e9de-8da0-3591907340b5@linux-m68k.org>
In-Reply-To: <e8398356-8ba3-e9de-8da0-3591907340b5@linux-m68k.org>
From:   Geert Uytterhoeven <geert@linux-m68k.org>
Date:   Mon, 21 Mar 2022 09:47:01 +0100
Message-ID: <CAMuHMdXqzWo1+gAfEjOQEZHeprbPEzWyCaUqLKx0_4nrDG+5Uw@mail.gmail.com>
Subject: Re: [PATCH] macintosh/via-pmu: Fix build failure when CONFIG_INPUT is disabled
To:     Finn Thain <fthain@linux-m68k.org>
Cc:     Christophe Leroy <christophe.leroy@csgroup.eu>,
        Benjamin Herrenschmidt <benh@kernel.crashing.org>,
        Randy Dunlap <rdunlap@infradead.org>,
        "linuxppc-dev@lists.ozlabs.org" <linuxppc-dev@lists.ozlabs.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 8BIT
Precedence: bulk

Hi Finn,

On Mon, Mar 21, 2022 at 9:29 AM Finn Thain <fthain@linux-m68k.org> wrote:
> On Mon, 21 Mar 2022, Christophe Leroy wrote:
> > Le 21/03/2022 à 05:30, Finn Thain a écrit :
> > > drivers/macintosh/via-pmu-event.o: In function `via_pmu_event':
> > > via-pmu-event.c:(.text+0x44): undefined reference to `input_event'
> > > via-pmu-event.c:(.text+0x68): undefined reference to `input_event'
> > > via-pmu-event.c:(.text+0x94): undefined reference to `input_event'
> > > via-pmu-event.c:(.text+0xb8): undefined reference to `input_event'
> > > drivers/macintosh/via-pmu-event.o: In function `via_pmu_event_init':
> > > via-pmu-event.c:(.init.text+0x20): undefined reference to `input_allocate_device'
> > > via-pmu-event.c:(.init.text+0xc4): undefined reference to `input_register_device'
> > > via-pmu-event.c:(.init.text+0xd4): undefined reference to `input_free_device'
> > > make[1]: *** [Makefile:1155: vmlinux] Error 1
> > > make: *** [Makefile:350: __build_one_by_one] Error 2
> > >
> > > Don't call into the input subsystem unless CONFIG_INPUT is built-in.
> > >
> > > Reported-by: kernel test robot <lkp@intel.com>
> > > Cc: Michael Ellerman <mpe@ellerman.id.au>
> > > Cc: Geert Uytterhoeven <geert@linux-m68k.org>
> > > Cc: Randy Dunlap <rdunlap@infradead.org>
> > > Signed-off-by: Finn Thain <fthain@linux-m68k.org>
> > > ---
> > > This is equivalent to the patch I sent a couple of days ago. This one
> > > is slightly longer and adds a new symbol so that Kconfig logic can been
> > > used instead of Makefile logic in case reviewers prefer that.
> > > ---
> > >   drivers/macintosh/Kconfig   | 5 +++++
> > >   drivers/macintosh/Makefile  | 3 ++-
> > >   drivers/macintosh/via-pmu.c | 2 ++
> > >   3 files changed, 9 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/macintosh/Kconfig b/drivers/macintosh/Kconfig
> > > index 5cdc361da37c..b9102f051bbb 100644
> > > --- a/drivers/macintosh/Kconfig
> > > +++ b/drivers/macintosh/Kconfig
> > > @@ -67,6 +67,11 @@ config ADB_PMU
> > >       this device; you should do so if your machine is one of those
> > >       mentioned above.
> > >
> > > +config ADB_PMU_EVENT
> > > +   bool
> > > +   depends on ADB_PMU && INPUT=y
> > > +   default y
> >
> > Could be reduced to
> >
> > config ADB_PMU_EVENT
> >       def_bool y if ADB_PMU && INPUT=y
>
> That's great but my question remains unanswered: why the aversion to
> conditionals in Makefiles, when that would be simpler (no new symbol)?

While conditionals in Makefiles do exist, they are far less common, and
can be confusing.  They're also harder to grep for.
E.g. "git grep via-pmu-event.o" after your alternative patch would
give:

    obj-$(CONFIG_ADB_PMU)          += via-pmu-event.o

but would miss the important surrounding part:

    ifeq ($(CONFIG_INPUT), y)
    obj-$(CONFIG_ADB_PMU)          += via-pmu-event.o
    endif

Keeping configuration logic in a single place (the Kconfig file)
avoids that.  The extra symbol is invisible, so it doesn't hurt much.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds