Re: Sucky dependencies found in video saa7134 rc

Re: Sucky dependencies found in video saa7134 rc

[Mauro Carvalho Chehab]

Em 10-03-2011 14:37, Steven Rostedt escreveu:
> Running randconfigs with ktest seems to find this crap all over. It's
> not just saa7134, it happens else where too, but this is the one that
> I'm currently looking at.
> 
> I'm also not blaming the saa7134 driver, but I think we need to find a
> way to fix Kconfig to handle this. Maybe it is in the works. I don't
> know.
> 
> Here's the issue:
> 
> 1) in the Makefile in drivers/media/video/saa7134/ we have:
> 
> aa7134-y :=	saa7134-cards.o saa7134-core.o saa7134-i2c.o
> saa7134-y +=	saa7134-ts.o saa7134-tvaudio.o saa7134-vbi.o
> saa7134-y +=	saa7134-video.o
> saa7134-$(CONFIG_VIDEO_SAA7134_RC) += saa7134-input.o
> 
> 2) in the Kconfig of that directory:
> 
> config VIDEO_SAA7134_RC
> 	bool "Philips SAA7134 Remote Controller support"
> 	depends on RC_CORE
> 	depends on VIDEO_SAA7134
> 	default y
> 
> 3) in the Makefile of drivers/media/rc
> 
> obj-$(CONFIG_RC_CORE) += rc-core.o
> 
> 4) in the Kconfig of that directory:
> 
> menuconfig RC_CORE
> 	tristate "Remote Controller adapters"
> 	depends on INPUT
> 	default INPUT
> 
> And finally in the .config produced by randconfig:
> 
> CONFIG_RC_CORE=m
> CONFIG_VIDEO_SAA7134=y
> CONFIG_VIDEO_SAA7134_RC=y
> 
> Thus, RC_CORE is a module and the SAA7134 input file is built in, which
> gives me the following error:
> 
> drivers/built-in.o: In function `saa7134_input_init1':
> /home/rostedt/work/autotest/nobackup/linux-test.git/drivers/media/video/saa7134/saa7134-input.c:737: undefined reference to `rc_allocate_device'
> /home/rostedt/work/autotest/nobackup/linux-test.git/drivers/media/video/saa7134/saa7134-input.c:780: undefined reference to `rc_register_device'
> /home/rostedt/work/autotest/nobackup/linux-test.git/drivers/media/video/saa7134/saa7134-input.c:787: undefined reference to `rc_free_device'
> drivers/built-in.o: In function `saa7134_input_fini':
> /home/rostedt/work/autotest/nobackup/linux-test.git/drivers/media/video/saa7134/saa7134-input.c:799: undefined reference to `rc_unregister_device'
> drivers/built-in.o: In function `ir_raw_decode_timer_end':
> /home/rostedt/work/autotest/nobackup/linux-test.git/drivers/media/video/saa7134/saa7134-input.c:400: undefined reference to `ir_raw_event_handle'
> drivers/built-in.o: In function `build_key':
> /home/rostedt/work/autotest/nobackup/linux-test.git/drivers/media/video/saa7134/saa7134-input.c:93: undefined reference to `rc_keydown_notimeout'
> /home/rostedt/work/autotest/nobackup/linux-test.git/drivers/media/video/saa7134/saa7134-input.c:101: undefined reference to `rc_keydown_notimeout'
> /home/rostedt/work/autotest/nobackup/linux-test.git/drivers/media/video/saa7134/saa7134-input.c:102: undefined reference to `rc_keyup'
> drivers/built-in.o: In function `saa7134_raw_decode_irq':
> /home/rostedt/work/autotest/nobackup/linux-test.git/drivers/media/video/saa7134/saa7134-input.c:920: undefined reference to `ir_raw_event_store_edge'
> make[1]: *** [.tmp_vmlinux1] Error 1
> 
> I know this is just a randconfig and I doubt that people will actually
> run into this problem, but randconfig should always produce a kernel
> that can compile, and preferably boot ;)
> 
> I think the issue here is that we should never allowed that SAA7134_RC
> be set to 'y' when RC_CORE is 'm'. I need to look at the kconfig code
> and see if there's a way to prevent that 'y' if its dependencies have
> 'm'.
> 
> Hmm, but that may not work either :( This may be a Makefile issue, as
> that 'y' is not really a builtin to the kernel but a builtin to a module
> that may or may not be built into the kernel. Which makes this
> dependency agnostic to the Kconfigs. God what a horrible tangled web we
> weave!

One solution would be to convert CONFIG_VIDEO_SAA7134_RC into a module,
and change its Kconfig item to tri-state. This way, everything should work
properly.

It is not that hard to convert it to be a module. Like the approach it
was taken for saa7134-alsa and saa7134-dvb, but this would require to move
the IR init code from saa7134 into saa7134-rc, as the -rc module will be
initialized late, and asynchronous.

Unfortunately, I don't have any time soon for doing that. Perhaps an easier
way would be to disable CONFIG_VIDEO_SAA7134_RC if RC_CORE=m and SAA7134=y.

Cheers,
Mauro

[PATCH] saa7134: Fix strange kconfig dependency on RC_CORE

[Steven Rostedt]

As the code in saa7134-input is not a module, but the config for it is
set as a boolean instead of a tristate, this causes a strange dependency
on RC_CORE.

VIDEO_SAA7134_RC (which determines if saa7134-input.o is built) depends
on RC_CORE and VIDEO_SAA7134. If VIDEO_SAA7134 is compiled as 'y' but
RC_CORE is compiled as 'm' VIDEO_SAA7134_RC can still be set to 'y'
which causes undefined symbols that it needs from RC_CORE.

The simplest solution is to not allow VIDEO_SAA7134_RC be enabled if
RC_CORE compiled as a module (m) and VIDEO_SA7134 is compiled into the
kernel (y).

Suggested-by: Mauro Carvalho Chehab <mchehab@redhat.com>
Cc: Michal Marek <mmarek@suse.cz>
Cc: linux-kbuild <linux-kbuild@vger.kernel.org>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>

---
 drivers/media/video/saa7134/Kconfig |    1 +
 1 file changed, 1 insertion(+)

Index: linux-test.git/drivers/media/video/saa7134/Kconfig
===================================================================
--- linux-test.git.orig/drivers/media/video/saa7134/Kconfig	2011-03-07 22:01:13.046263327 -0500
+++ linux-test.git/drivers/media/video/saa7134/Kconfig	2011-03-10 16:18:26.426709736 -0500
@@ -28,6 +28,7 @@ config VIDEO_SAA7134_RC
 	bool "Philips SAA7134 Remote Controller support"
 	depends on RC_CORE
 	depends on VIDEO_SAA7134
+	depends on !(RC_CORE=m && VIDEO_SAA7134=y)
 	default y
 	---help---
 	  Enables Remote Controller support on saa7134 driver.