login
Header Space

 
 

Re: [patch] irda: fix !PNP support in drivers/net/irda/nsc-ircc.c

Previous thread: [PATCH] tiny mq_open optimization by Ulrich Drepper on Saturday, May 3, 2008 - 3:28 pm. (1 message)

Next thread: Re: Remove Asus EEE UDMA/33 limitation. by Robert Hancock on Saturday, May 3, 2008 - 3:34 pm. (1 message)
To: <linux-kernel@...>
Cc: <samuel@...>, Dmitry Torokhov <dtor_core@...>
Date: Saturday, May 3, 2008 - 3:32 pm

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. ]

Signed-off-by: Ingo Molnar &lt;mingo@elte.hu&gt;
---
 drivers/net/irda/nsc-ircc.c |    6 ++++++
 1 file changed, 6 insertions(+)

Index: linux/drivers/net/irda/nsc-ircc.c
===================================================================
--- linux.orig/drivers/net/irda/nsc-ircc.c
+++ 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...
To: <mingo@...>
Cc: <linux-kernel@...>, <samuel@...>, <dtor_core@...>
Date: Monday, May 5, 2008 - 3:50 am

From: Ingo Molnar &lt;mingo@elte.hu&gt;

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.
--
To: David Miller <davem@...>
Cc: <linux-kernel@...>, <samuel@...>, <dtor_core@...>
Date: Monday, May 5, 2008 - 3:58 am

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
--
To: <mingo@...>
Cc: <linux-kernel@...>, <samuel@...>, <dtor_core@...>
Date: Monday, May 5, 2008 - 4:02 am

From: Ingo Molnar &lt;mingo@elte.hu&gt;

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.
--
Previous thread: [PATCH] tiny mq_open optimization by Ulrich Drepper on Saturday, May 3, 2008 - 3:28 pm. (1 message)

Next thread: Re: Remove Asus EEE UDMA/33 limitation. by Robert Hancock on Saturday, May 3, 2008 - 3:34 pm. (1 message)
speck-geostationary