Thread (5 messages) 5 messages, 3 authors, 2022-03-21

Re: [PATCH] macintosh/via-pmu: Fix build failure when CONFIG_INPUT is disabled

From: Finn Thain <fthain@linux-m68k.org>
Date: 2022-03-21 09:09:42
Also in: lkml

On Mon, 21 Mar 2022, Geert Uytterhoeven wrote:
On Mon, Mar 21, 2022 at 9:29 AM Finn Thain [off-list ref] wrote:
quoted
On Mon, 21 Mar 2022, Christophe Leroy wrote:
quoted
Le 21/03/2022 à 05:30, Finn Thain a écrit :
quoted
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 <redacted>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: Randy Dunlap <redacted>
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, 
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.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help