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

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

[Finn Thain]

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
+
 config ADB_PMU_LED
 	bool "Support for the Power/iBook front LED"
 	depends on PPC_PMAC && ADB_PMU
diff --git a/drivers/macintosh/Makefile b/drivers/macintosh/Makefile
index 49819b1b6f20..712edcb3e0b0 100644
--- a/drivers/macintosh/Makefile
+++ b/drivers/macintosh/Makefile
@@ -12,7 +12,8 @@ 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
+obj-$(CONFIG_ADB_PMU_EVENT)	+= via-pmu-event.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 b1859e5340b3..022e2fd4397b 100644
--- a/drivers/macintosh/via-pmu.c
+++ b/drivers/macintosh/via-pmu.c
@@ -1468,12 +1468,14 @@ pmu_handle_data(unsigned char *data, int len)
 		if (pmu_battery_count)
 			query_battery_state();
 		pmu_pass_intr(data, len);
+#ifdef CONFIG_ADB_PMU_EVENT
 		/* 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:
-- 
2.32.0

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

[Christophe Leroy]

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

> +
>   config ADB_PMU_LED
>   	bool "Support for the Power/iBook front LED"
>   	depends on PPC_PMAC && ADB_PMU
> diff --git a/drivers/macintosh/Makefile b/drivers/macintosh/Makefile
> index 49819b1b6f20..712edcb3e0b0 100644
> --- a/drivers/macintosh/Makefile
> +++ b/drivers/macintosh/Makefile
> @@ -12,7 +12,8 @@ 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
> +obj-$(CONFIG_ADB_PMU_EVENT)	+= via-pmu-event.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 b1859e5340b3..022e2fd4397b 100644
> --- a/drivers/macintosh/via-pmu.c
> +++ b/drivers/macintosh/via-pmu.c
> @@ -1468,12 +1468,14 @@ pmu_handle_data(unsigned char *data, int len)
>   		if (pmu_battery_count)
>   			query_battery_state();
>   		pmu_pass_intr(data, len);
> +#ifdef CONFIG_ADB_PMU_EVENT
>   		/* len == 6 is probably a bad check. But how do I
>   		 * know what PMU versions send what events here? */
>   		if (len == 6) {

Can you replace that #ifdef stuff by

		if (IS_ENABLED(CONFIG_ADB_PMU_EVENT) && len == 6) {

>   			via_pmu_event(PMU_EVT_POWER, !!(data[1]&8));
>   			via_pmu_event(PMU_EVT_LID, data[1]&1);
>   		}
> +#endif
>   		break;
>   
>   	default:

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

[Finn Thain]

[-- Attachment #1: Type: text/plain, Size: 2337 bytes --]

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)?

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

[Geert Uytterhoeven]

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

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

[Finn Thain]

[-- Attachment #1: Type: text/plain, Size: 4110 bytes --]

On Mon, 21 Mar 2022, Geert Uytterhoeven wrote:

> 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, 

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.

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

[Finn Thain]

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(-)

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:
-- 
2.32.0

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

[Randy Dunlap]

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.
-- 
~Randy

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

[Finn Thain]

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.

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

[Geert Uytterhoeven]

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

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

[Finn Thain]

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.