Skip to contentSkip to conversation

irda: fix !PNP support in drivers/net/irda/nsc-ircc.c

PATCH In review Ingo Molnar 2 participants 4 messages May 3, 2008 – May 5, 2008 Network drivers

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 irda: fix !PNP support in drivers/net/irda/nsc-ircc.c

x86.git testing found the following build failure in latest -git:

drivers/built-in.o: In function `nsc_ircc_pnp_probe':
nsc-ircc.c:(.text+0xdf1b6): undefined reference to pnp_get_resource' nsc-ircc.c:(.text+0xdf1d4): undefined reference to pnp_get_resource' nsc-ircc.c:(.text+0xdf1ee): undefined reference to pnp_get_resource' nsc-ircc.c:(.text+0xdf237): undefined reference to pnp_get_resource' nsc-ircc.c:(.text+0xdf24c): undefined reference to pnp_get_resource' drivers/built-in.o:nsc-ircc.c:(.text+0xdf266): more undefined references to pnp_get_resource' follow
make: *** [.tmp_vmlinux1] Error 1

triggered via this config:

http://redhat.com/~mingo/misc/config-Sat_May__3_20_53_13_CEST_2008.bad

while generally most users will have PNP enabled, drivers can support non-PNP build mode too - and most drivers implement it. That is typically done by providing a dummy pnp_driver structure that will not probe anything.

The fallback routines in the driver will handle this dumber mode of operation too.

This patch implements that. I have not tested whether this actually works on real hardware so take care. It does resolve the build bug.

[ Another solution that is used by a few drivers is to exclude the driver
in the Kconfig if PNP is disabled, via "depends on PNP", but this would limit the availability of the driver needlessly. ]

---
 drivers/net/irda/nsc-ircc.c |    6 ++++++
 1 file changed, 6 insertions(+)
drivers/net/irda/nsc-ircc.c +6 -0
Index: linux/drivers/net/irda/nsc-ircc.c
===================================================================
@@ -100,7 +100,9 @@ static int nsc_ircc_probe_39x(nsc_chip_t
 static int nsc_ircc_init_108(nsc_chip_t *chip, chipio_t *info);
 static int nsc_ircc_init_338(nsc_chip_t *chip, chipio_t *info);
 static int nsc_ircc_init_39x(nsc_chip_t *chip, chipio_t *info);
+#ifdef CONFIG_PNP
 static int nsc_ircc_pnp_probe(struct pnp_dev *dev, const struct pnp_device_id *id);
+#endif
 
 /* These are the known NSC chips */
 static nsc_chip_t chips[] = {
@@ -156,9 +158,11 @@ static const struct pnp_device_id nsc_ir
 MODULE_DEVICE_TABLE(pnp, nsc_ircc_pnp_table);
 
 static struct pnp_driver nsc_ircc_pnp_driver = {
+#ifdef CONFIG_PNP
 	.name = "nsc-ircc",
 	.id_table = nsc_ircc_pnp_table,
 	.probe = nsc_ircc_pnp_probe,
+#endif
 };
 
 /* Some prototypes */
@@ -916,6 +920,7 @@ static int nsc_ircc_probe_39x(nsc_chip_t
 	return 0;
 }
 
+#ifdef CONFIG_PNP
 /* PNP probing */
 static int nsc_ircc_pnp_probe(struct pnp_dev *dev, const struct pnp_device_id *id)
 {
@@ -952,6 +957,7 @@ static int nsc_ircc_pnp_probe(struct pnp
 
 	return 0;
 }
+#endif
 
 /*
  * Function nsc_ircc_setup (info)
maintainer
#1 lore raw

From: Ingo Molnar <[email protected]>
Date: Sat, 3 May 2008 21:32:23 +0200

Quoted 11 lines

x86.git testing found the following build failure in latest -git:

drivers/built-in.o: In function `nsc_ircc_pnp_probe':
nsc-ircc.c:(.text+0xdf1b6): undefined reference to pnp_get_resource' nsc-ircc.c:(.text+0xdf1d4): undefined reference to pnp_get_resource' nsc-ircc.c:(.text+0xdf1ee): undefined reference to pnp_get_resource' nsc-ircc.c:(.text+0xdf237): undefined reference to pnp_get_resource' nsc-ircc.c:(.text+0xdf24c): undefined reference to pnp_get_resource' drivers/built-in.o:nsc-ircc.c:(.text+0xdf266): more undefined references to pnp_get_resource' follow
make: *** [.tmp_vmlinux1] Error 1

I feel there is a better way to fix this and the other IRDA pnp build failure.

linux/pnp.h provides erroring inlines when PNP is disabled, but it does not provide a full set. In order to be fully consistent it should provide similar stub implementations for things like pnp_get_flags() (which is what triggers the reference to pnp_get_resource here), for example.

Either the whole API is provided with stubs when PNP is disabled, or none should be. If only some interfaces get the treatment, we get messy ifdefs such as those seen in these patches we are discussing.

#2 lore raw
Quoted 24 lines · David Miller

From: Ingo Molnar <[email protected]>
Date: Sat, 3 May 2008 21:32:23 +0200

Quoted 11 lines

x86.git testing found the following build failure in latest -git:

drivers/built-in.o: In function `nsc_ircc_pnp_probe':
nsc-ircc.c:(.text+0xdf1b6): undefined reference to pnp_get_resource' nsc-ircc.c:(.text+0xdf1d4): undefined reference to pnp_get_resource' nsc-ircc.c:(.text+0xdf1ee): undefined reference to pnp_get_resource' nsc-ircc.c:(.text+0xdf237): undefined reference to pnp_get_resource' nsc-ircc.c:(.text+0xdf24c): undefined reference to pnp_get_resource' drivers/built-in.o:nsc-ircc.c:(.text+0xdf266): more undefined references to pnp_get_resource' follow
make: *** [.tmp_vmlinux1] Error 1

I feel there is a better way to fix this and the other IRDA pnp build failure.

linux/pnp.h provides erroring inlines when PNP is disabled, but it does not provide a full set. In order to be fully consistent it should provide similar stub implementations for things like pnp_get_flags() (which is what triggers the reference to pnp_get_resource here), for example.

Quoted 3 lines

Either the whole API is provided with stubs when PNP is disabled, or none should be. If only some interfaces get the treatment, we get messy ifdefs such as those seen in these patches we are discussing.

i just used the existing precedent of doing the #ifdef: it's not _that_ bad as it surrounds a single function most of the time so it's rather apparent what is happening - and that is what is done in some other drivers as well.

btw., when i fixed this i almost implemented it the way that you suggested, but there's also a code size argument to when the suspend callbacks are not used: the extra dummy function takes up text space, and people turn off PNP when they build some specialized thing and try to reduce the size of the kernel.

but in any case, i have no strong feelings in either direction.

Ingo

maintainer
#3 lore raw

From: Ingo Molnar <[email protected]>
Date: Mon, 5 May 2008 09:58:23 +0200

Quoted 5 lines

i just used the existing precedent of doing the #ifdef: it's not _that_ bad as it surrounds a single function most of the time so it's rather apparent what is happening - and that is what is done in some other drivers as well.

Quoted 7 lines

btw., when i fixed this i almost implemented it the way that you suggested, but there's also a code size argument to when the suspend callbacks are not used: the extra dummy function takes up text space, and people turn off PNP when they build some specialized thing and try to reduce the size of the kernel.

but in any case, i have no strong feelings in either direction.

The existing practice is ugly :-)

But I see arguments for either side, just like you.

Meanwhile let's fix this build error, I'll toss your patches into my net-2.6 tree.

Thanks.