| Message ID | cb1828050f8c9ef801b2bdf79eccd6c52afed26b.1647663509.git.fthain@linux-m68k.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | macintosh/via-pmu: Fix build failure when CONFIG_INPUT is disabled | expand |
| Context | Check | Description |
|---|---|---|
| snowpatch_ozlabs/github-powerpc_kernel_qemu | success | Successfully ran 24 jobs. |
| snowpatch_ozlabs/github-powerpc_sparse | success | Successfully ran 4 jobs. |
| snowpatch_ozlabs/github-powerpc_clang | success | Successfully ran 7 jobs. |
On 3/18/22 21:18, Finn Thain wrote: > 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> > Signed-off-by: Finn Thain <fthain@linux-m68k.org> Hi Finn, It builds without those reported errors, but I do see these warnings since the robot-supplied .config file has: # CONFIG_PROC_FS is not set CC drivers/macintosh/via-pmu.o ../drivers/macintosh/via-pmu.c:897:12: warning: 'pmu_battery_proc_show' defined but not used [-Wunused-function] 897 | static int pmu_battery_proc_show(struct seq_file *m, void *v) | ^~~~~~~~~~~~~~~~~~~~~ ../drivers/macintosh/via-pmu.c:871:12: warning: 'pmu_irqstats_proc_show' defined but not used [-Wunused-function] 871 | static int pmu_irqstats_proc_show(struct seq_file *m, void *v) | ^~~~~~~~~~~~~~~~~~~~~~ ../drivers/macintosh/via-pmu.c:860:12: warning: 'pmu_info_proc_show' defined but not used [-Wunused-function] 860 | static int pmu_info_proc_show(struct seq_file *m, void *v) | ^~~~~~~~~~~~~~~~~~ > --- > drivers/macintosh/Makefile | 5 ++++- > drivers/macintosh/via-pmu.c | 2 ++ > 2 files changed, 6 insertions(+), 1 deletion(-) > > diff --git a/drivers/macintosh/Makefile b/drivers/macintosh/Makefile > index 49819b1b6f20..eaf28b1c272f 100644 > --- a/drivers/macintosh/Makefile > +++ b/drivers/macintosh/Makefile > @@ -12,7 +12,10 @@ obj-$(CONFIG_MAC_EMUMOUSEBTN) += mac_hid.o > obj-$(CONFIG_INPUT_ADBHID) += adbhid.o > obj-$(CONFIG_ANSLCD) += ans-lcd.o > > -obj-$(CONFIG_ADB_PMU) += via-pmu.o via-pmu-event.o > +obj-$(CONFIG_ADB_PMU) += via-pmu.o > +ifeq ($(CONFIG_INPUT), y) > +obj-$(CONFIG_ADB_PMU) += via-pmu-event.o > +endif > obj-$(CONFIG_ADB_PMU_LED) += via-pmu-led.o > obj-$(CONFIG_PMAC_BACKLIGHT) += via-pmu-backlight.o > obj-$(CONFIG_ADB_CUDA) += via-cuda.o > diff --git a/drivers/macintosh/via-pmu.c b/drivers/macintosh/via-pmu.c > index 4b98bc26a94b..55afa6dfa263 100644 > --- a/drivers/macintosh/via-pmu.c > +++ b/drivers/macintosh/via-pmu.c > @@ -1457,12 +1457,14 @@ pmu_handle_data(unsigned char *data, int len) > if (pmu_battery_count) > query_battery_state(); > pmu_pass_intr(data, len); > +#ifdef CONFIG_INPUT > /* len == 6 is probably a bad check. But how do I > * know what PMU versions send what events here? */ > if (len == 6) { > via_pmu_event(PMU_EVT_POWER, !!(data[1]&8)); > via_pmu_event(PMU_EVT_LID, data[1]&1); > } > +#endif > break; > > default: thanks.
On Fri, 18 Mar 2022, Randy Dunlap wrote: > > Hi Finn, > It builds without those reported errors, but I do see these warnings > since the robot-supplied .config file has: > # CONFIG_PROC_FS is not set > > > CC drivers/macintosh/via-pmu.o > ../drivers/macintosh/via-pmu.c:897:12: warning: 'pmu_battery_proc_show' defined but not used [-Wunused-function] > 897 | static int pmu_battery_proc_show(struct seq_file *m, void *v) > | ^~~~~~~~~~~~~~~~~~~~~ > ../drivers/macintosh/via-pmu.c:871:12: warning: 'pmu_irqstats_proc_show' defined but not used [-Wunused-function] > 871 | static int pmu_irqstats_proc_show(struct seq_file *m, void *v) > | ^~~~~~~~~~~~~~~~~~~~~~ > ../drivers/macintosh/via-pmu.c:860:12: warning: 'pmu_info_proc_show' defined but not used [-Wunused-function] > 860 | static int pmu_info_proc_show(struct seq_file *m, void *v) > | ^~~~~~~~~~~~~~~~~~ > I see. I'll send a separate patch for this issue.
Hi Finn, On Sat, Mar 19, 2022 at 5:23 AM Finn Thain <fthain@linux-m68k.org> wrote: > 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> > Signed-off-by: Finn Thain <fthain@linux-m68k.org> Thanks for your patch! > --- a/drivers/macintosh/Makefile > +++ b/drivers/macintosh/Makefile > @@ -12,7 +12,10 @@ obj-$(CONFIG_MAC_EMUMOUSEBTN) += mac_hid.o > obj-$(CONFIG_INPUT_ADBHID) += adbhid.o > obj-$(CONFIG_ANSLCD) += ans-lcd.o > > -obj-$(CONFIG_ADB_PMU) += via-pmu.o via-pmu-event.o > +obj-$(CONFIG_ADB_PMU) += via-pmu.o > +ifeq ($(CONFIG_INPUT), y) > +obj-$(CONFIG_ADB_PMU) += via-pmu-event.o > +endif Alternatively, you can introduce an invisible Kconfig symbol that is y if ADB_PMU && INPUT, to control the build of via-pmu.o. > obj-$(CONFIG_ADB_PMU_LED) += via-pmu-led.o > obj-$(CONFIG_PMAC_BACKLIGHT) += via-pmu-backlight.o > obj-$(CONFIG_ADB_CUDA) += via-cuda.o > diff --git a/drivers/macintosh/via-pmu.c b/drivers/macintosh/via-pmu.c > index 4b98bc26a94b..55afa6dfa263 100644 > --- a/drivers/macintosh/via-pmu.c > +++ b/drivers/macintosh/via-pmu.c > @@ -1457,12 +1457,14 @@ pmu_handle_data(unsigned char *data, int len) > if (pmu_battery_count) > query_battery_state(); > pmu_pass_intr(data, len); > +#ifdef CONFIG_INPUT > /* len == 6 is probably a bad check. But how do I > * know what PMU versions send what events here? */ > if (len == 6) { > via_pmu_event(PMU_EVT_POWER, !!(data[1]&8)); > via_pmu_event(PMU_EVT_LID, data[1]&1); > } > +#endif Additionally, if that new symbol is not enabled, a dummy via_pmu_event() can be provided, so you don't need to add an #ifdef to the driver anymore. 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
On Sat, 19 Mar 2022, Geert Uytterhoeven wrote: > On Sat, Mar 19, 2022 at 5:23 AM Finn Thain <fthain@linux-m68k.org> wrote: > > 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> > > Signed-off-by: Finn Thain <fthain@linux-m68k.org> > > Thanks for your patch! > Thanks for your review. > > --- a/drivers/macintosh/Makefile > > +++ b/drivers/macintosh/Makefile > > @@ -12,7 +12,10 @@ obj-$(CONFIG_MAC_EMUMOUSEBTN) += mac_hid.o > > obj-$(CONFIG_INPUT_ADBHID) += adbhid.o > > obj-$(CONFIG_ANSLCD) += ans-lcd.o > > > > -obj-$(CONFIG_ADB_PMU) += via-pmu.o via-pmu-event.o > > +obj-$(CONFIG_ADB_PMU) += via-pmu.o > > +ifeq ($(CONFIG_INPUT), y) > > +obj-$(CONFIG_ADB_PMU) += via-pmu-event.o > > +endif > > Alternatively, you can introduce an invisible Kconfig symbol that > is y if ADB_PMU && INPUT, to control the build of via-pmu.o. > There is some precent for that (DVB_AV7110_IR, PINCTRL_FALCON, DVB_AV7110_IR) in recent code but it's a bit of a stretch. Adding the new Kconfig symbol would add complexity and it would only get used in two places. I appreciate the general preference for declarative style over imperative. But is there a stronger argument against conditionals in Makefiles? > > obj-$(CONFIG_ADB_PMU_LED) += via-pmu-led.o > > obj-$(CONFIG_PMAC_BACKLIGHT) += via-pmu-backlight.o > > obj-$(CONFIG_ADB_CUDA) += via-cuda.o > > diff --git a/drivers/macintosh/via-pmu.c b/drivers/macintosh/via-pmu.c > > index 4b98bc26a94b..55afa6dfa263 100644 > > --- a/drivers/macintosh/via-pmu.c > > +++ b/drivers/macintosh/via-pmu.c > > @@ -1457,12 +1457,14 @@ pmu_handle_data(unsigned char *data, int len) > > if (pmu_battery_count) > > query_battery_state(); > > pmu_pass_intr(data, len); > > +#ifdef CONFIG_INPUT > > /* len == 6 is probably a bad check. But how do I > > * know what PMU versions send what events here? */ > > if (len == 6) { > > via_pmu_event(PMU_EVT_POWER, !!(data[1]&8)); > > via_pmu_event(PMU_EVT_LID, data[1]&1); > > } > > +#endif > > Additionally, if that new symbol is not enabled, a dummy via_pmu_event() > can be provided, so you don't need to add an #ifdef to the driver anymore. > Adding a dummy function to be used in only one place seems questionable to me. I'm not expecting new call sites to appear outside of that ifdef. Some of the arguments against ifdefs (reduced test and checker coverage, readability harm) don't really apply in this case.
diff --git a/drivers/macintosh/Makefile b/drivers/macintosh/Makefile index 49819b1b6f20..eaf28b1c272f 100644 --- a/drivers/macintosh/Makefile +++ b/drivers/macintosh/Makefile @@ -12,7 +12,10 @@ obj-$(CONFIG_MAC_EMUMOUSEBTN) += mac_hid.o obj-$(CONFIG_INPUT_ADBHID) += adbhid.o obj-$(CONFIG_ANSLCD) += ans-lcd.o -obj-$(CONFIG_ADB_PMU) += via-pmu.o via-pmu-event.o +obj-$(CONFIG_ADB_PMU) += via-pmu.o +ifeq ($(CONFIG_INPUT), y) +obj-$(CONFIG_ADB_PMU) += via-pmu-event.o +endif obj-$(CONFIG_ADB_PMU_LED) += via-pmu-led.o obj-$(CONFIG_PMAC_BACKLIGHT) += via-pmu-backlight.o obj-$(CONFIG_ADB_CUDA) += via-cuda.o diff --git a/drivers/macintosh/via-pmu.c b/drivers/macintosh/via-pmu.c index 4b98bc26a94b..55afa6dfa263 100644 --- a/drivers/macintosh/via-pmu.c +++ b/drivers/macintosh/via-pmu.c @@ -1457,12 +1457,14 @@ pmu_handle_data(unsigned char *data, int len) if (pmu_battery_count) query_battery_state(); pmu_pass_intr(data, len); +#ifdef CONFIG_INPUT /* len == 6 is probably a bad check. But how do I * know what PMU versions send what events here? */ if (len == 6) { via_pmu_event(PMU_EVT_POWER, !!(data[1]&8)); via_pmu_event(PMU_EVT_LID, data[1]&1); } +#endif break; default:
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> Signed-off-by: Finn Thain <fthain@linux-m68k.org> --- drivers/macintosh/Makefile | 5 ++++- drivers/macintosh/via-pmu.c | 2 ++ 2 files changed, 6 insertions(+), 1 deletion(-)