On Mon, 21 Mar 2022, Geert Uytterhoeven wrote:
> On Mon, Mar 21, 2022 at 9:29 AM Finn Thain <fth...@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 <l...@intel.com>
> > > > Cc: Michael Ellerman <m...@ellerman.id.au>
> > > > Cc: Geert Uytterhoeven <ge...@linux-m68k.org>
> > > > Cc: Randy Dunlap <rdun...@infradead.org>
> > > > Signed-off-by: Finn Thain <fth...@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,
Perhaps the Kconfig solution is more common. I did look for it but didn't
find it in recent commits. Maybe I didn't look hard enough. Mostly I see
invisible Kconfig getting used for things that are hard to do in any
simpler way.
> and can be confusing.
Kconfig can be confusing too. I don't think your approach reduces
complexity, I think it increases it.
> 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.
>
The same argument applies to Kconfig. "git grep CONFIG_ADB_PMU_EVENT"
doesn't even mention drivers/macintosh/Kconfig.
If you do "git grep ADB_PMU_EVENT" you get:
drivers/macintosh/Kconfig:config ADB_PMU_EVENT
with no mention of the "important surrounding part" that you were
concerned about:
config ADB_PMU_EVENT
def_bool y if ADB_PMU && INPUT=y
Anyway, we don't have to debate this, as drivers/macintosh already has a
maintainer who can decide the question. Both patches are available.
