media/video/sound build fix, TEA5761/TEA5767

x86.git randconfig testing found the following new build error in latest -git:

drivers/built-in.o: In function `v4l2_i2c_drv_attach_legacy':
tuner-core.c:(.text+0x1a829d): undefined reference to v4l2_i2c_attach' drivers/built-in.o: In function tuner_command':
tuner-core.c:(.text+0x1a971c): undefined reference to `v4l_printk_ioctl'

the reason appears to be that the TEA5761/TEA5767 tuner code is dependent on (legacy?) V4L infrastructure like v4l2_i2c_attach, by virtue of drivers/media/video/tuner-core.c including media/v4l2-i2c-drv-legacy.h and ./sound/i2c/other/Makefile doing:

snd-tea575x-tuner-objs := tea575x-tuner.o

for now i solved this via adding a VIDEO_V4L2_COMMON dependency to MEDIA_TUNER - which solves the build problem by excluding these drivers
- but i suspect there might be a better fix as well that does not restrict the selectability of these drivers.

---
 drivers/media/common/tuners/Kconfig |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Index: linux/drivers/media/common/tuners/Kconfig
===================================================================
@@ -20,7 +20,7 @@ config MEDIA_ATTACH
 config MEDIA_TUNER
 	tristate
 	default DVB_CORE || VIDEO_DEV
-	depends on DVB_CORE || VIDEO_DEV
+	depends on (DVB_CORE || VIDEO_DEV) && VIDEO_V4L2_COMMON
 	select MEDIA_TUNER_XC2028 if !MEDIA_TUNER_CUSTOMIZE
 	select MEDIA_TUNER_XC5000 if !MEDIA_TUNER_CUSTOMIZE
 	select MEDIA_TUNER_MT20XX if !MEDIA_TUNER_CUSTOMIZE

At Wed, 30 Apr 2008 13:01:15 +0200,

IMO, it's the reverse selection from sound to V4L that makes things complicated. I believe it's better to fix it as a normal dependency.

How about the patch below?

Takashi

Fix kconfig dependency mess of fm801-tea575x

FM801-tea575x tuner has a reverse selection to V4L1 and this causes dependency problem. The patch simplifies the dependency with a normal depend on VIDEO_V4L1.

---

index 581debf..7e47421 100644
@@ -515,19 +515,16 @@ config SND_FM801
 config SND_FM801_TEA575X_BOOL
 	bool "ForteMedia FM801 + TEA5757 tuner"
 	depends on SND_FM801
+	depends on VIDEO_V4L1=y || VIDEO_V4L1=SND_FM801
 	help
 	  Say Y here to include support for soundcards based on the ForteMedia
 	  FM801 chip with a TEA5757 tuner connected to GPIO1-3 pins (Media
 	  Forte SF256-PCS-02) into the snd-fm801 driver.
 
-	  This will enable support for the old V4L1 API.
-
 config SND_FM801_TEA575X
 	tristate
 	depends on SND_FM801_TEA575X_BOOL
 	default SND_FM801
-	select VIDEO_V4L1
-	select VIDEO_DEV
 
 config SND_HDA_INTEL
 	tristate "Intel HD Audio"

thanks, i've added it to the mix, instead of my patch.

Ingo

it now fails with another randconfig:

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

i've added back my hack to keep things rolling. (Note: i've got some media drivers hacks in this tree too so maybe one of them has a side-effect)

Ingo

FYI, below are the 7 fixes in this area that i've queued up so far. Let me know if i should drop any of them.

Ingo

----------------------->
Subject: fix SOC_CAMERA_MT9M001 build bug
From: Ingo Molnar <[email protected]>
Date: Mon Apr 28 13:51:58 CEST 2008

with this randconfig:

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

CONFIG_SOC_CAMERA_MT9M001=y
CONFIG_SOC_CAMERA_MT9V022=y
# CONFIG_I2C is not set

the build fails:

drivers/built-in.o: In function `reg_read':
mt9m001.c:(.text+0x140a87): undefined reference to i2c_smbus_read_word_data' drivers/built-in.o: In function reg_write':
mt9m001.c:(.text+0x140b6e): undefined reference to i2c_smbus_write_word_data' drivers/built-in.o: In function reg_read':
mt9v022.c:(.text+0x1413a7): undefined reference to i2c_smbus_read_word_data' drivers/built-in.o: In function reg_write':
mt9v022.c:(.text+0x1414ce): undefined reference to i2c_smbus_write_word_data' drivers/built-in.o: In function mt9m001_mod_init':
mt9m001.c:(.init.text+0x158a8): undefined reference to i2c_register_driver' drivers/built-in.o: In function mt9v022_mod_init':
mt9v022.c:(.init.text+0x158b8): undefined reference to i2c_register_driver' drivers/built-in.o: In function mt9m001_mod_exit':
mt9m001.c:(.exit.text+0x1656): undefined reference to i2c_del_driver' drivers/built-in.o: In function mt9v022_mod_exit':
mt9v022.c:(.exit.text+0x1666): undefined reference to `i2c_del_driver'

due to missing I2C dependency.

---
 drivers/media/video/Kconfig |    2 ++
 1 file changed, 2 insertions(+)

Index: linux/drivers/media/video/Kconfig
===================================================================
@@ -908,6 +908,7 @@ config SOC_CAMERA
 config SOC_CAMERA_MT9M001
 	tristate "mt9m001 support"
 	depends on SOC_CAMERA
+	depends on I2C
 	select GPIO_PCA953X if MT9M001_PCA9536_SWITCH
 	help
 	  This driver supports MT9M001 cameras from Micron, monochrome

@@ -923,6 +924,7 @@ config MT9M001_PCA9536_SWITCH
 config SOC_CAMERA_MT9V022
 	tristate "mt9v022 support"
 	depends on SOC_CAMERA
+	depends on I2C
 	select GPIO_PCA953X if MT9V022_PCA9536_SWITCH
 	help
 	  This driver supports MT9V022 cameras from Micron

Subject: dvb: fix3
From: Ingo Molnar <[email protected]>
Date: Mon Apr 28 23:22:13 CEST 2008

---
 drivers/media/video/cx23885/Kconfig |    2 ++
 1 file changed, 2 insertions(+)

Index: linux/drivers/media/video/cx23885/Kconfig
===================================================================
@@ -9,6 +9,8 @@ config VIDEO_CX23885
 	select VIDEO_IR
 	select VIDEOBUF_DVB
 	select VIDEO_CX25840
+	select VIDEO_CX2341X
+	select DVB_DIB7000P
 	select MEDIA_TUNER_MT2131 if !DVB_FE_CUSTOMISE
 	select DVB_S5H1409 if !DVB_FE_CUSTOMISE
 	select DVB_LGDT330X if !DVB_FE_CUSTOMISE

Subject: drivers/media build fix for modular builds
From: Ingo Molnar <[email protected]>
Date: Wed Apr 30 09:36:42 CEST 2008

fix build bug:

LD kernel/built-in.o
LD drivers/built-in.o
ld: drivers/media/built-in.o: No such file: No such file or directory

which happens if all media drivers are modular:

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

In that case there's no obj-y rule connecting all the built-in.o files and the link tree breaks.

the fix is to add a guaranteed obj-y rule for the core vmlinux to build. (which results in an empty object file if all media drivers are modular)

---
 drivers/media/Makefile |    2 ++
 1 file changed, 2 insertions(+)

Index: linux/drivers/media/Makefile
===================================================================
@@ -2,6 +2,8 @@
 # Makefile for the kernel multimedia device drivers.
 #
 
+obj-y := common/
+
 obj-$(CONFIG_VIDEO_MEDIA) += common/
 
 # Since hybrid devices are here, should be compiled if DVB and/or V4L

Subject: Re: media/video/sound build fix, TEA5761/TEA5767
From: Takashi Iwai <[email protected]>
Date: Wed, 30 Apr 2008 13:11:31 +0200

At Wed, 30 Apr 2008 13:01:15 +0200,

IMO, it's the reverse selection from sound to V4L that makes things complicated. I believe it's better to fix it as a normal dependency.

How about the patch below?

Takashi

Fix kconfig dependency mess of fm801-tea575x

FM801-tea575x tuner has a reverse selection to V4L1 and this causes dependency problem. The patch simplifies the dependency with a normal depend on VIDEO_V4L1.

---

---
 sound/pci/Kconfig |    5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

Index: linux/sound/pci/Kconfig
===================================================================
@@ -515,21 +515,18 @@ config SND_FM801
 config SND_FM801_TEA575X_BOOL
 	bool "ForteMedia FM801 + TEA5757 tuner"
 	depends on SND_FM801
+	depends on VIDEO_V4L1=y || VIDEO_V4L1=SND_FM801
 	help
 	  Say Y here to include support for soundcards based on the ForteMedia
 	  FM801 chip with a TEA5757 tuner connected to GPIO1-3 pins (Media
 	  Forte SF256-PCS-02) into the snd-fm801 driver.
 
-	  This will enable support for the old V4L1 API.
-
 config SND_FM801_TEA575X
 	# depends on 0
 	# VIDEO_DEV build bug re-reported on Fri Feb  1 13:03:44 CET 2008
 	tristate
 	depends on SND_FM801_TEA575X_BOOL
 	default SND_FM801
-	select VIDEO_V4L1
-	select VIDEO_DEV
 
 config SND_HDA_INTEL
 	tristate "Intel HD Audio"

Subject: video: fix3
From: Ingo Molnar <[email protected]>
Date: Wed Apr 30 11:44:58 CEST 2008

x86.git randconfig testing found a build failure on latest -git:

drivers/built-in.o: In function `set_type':
tuner-core.c:(.text+0x2a9a26): undefined reference to tea5761_attach' tuner-core.c:(.text+0x2a9d05): undefined reference to tda9887_attach' tuner-core.c:(.text+0x2a9d51): undefined reference to xc2028_attach' tuner-core.c:(.text+0x2a9e22): undefined reference to tda829x_attach' tuner-core.c:(.text+0x2a9e3f): undefined reference to microtune_attach' drivers/built-in.o: In function tuner_probe':
tuner-core.c:(.text+0x2aa18a): undefined reference to tda829x_probe' tuner-core.c:(.text+0x2aa302): undefined reference to tea5761_autodetection'

with the following config:

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

the problem is caused by the drivers/media/common/tuners/ subdirectory not being part of the kbuild hierarchy anymore, due to commit 7c91f0624 ("V4L/DVB(7767): Move tuners to common/tuners").

---
 drivers/media/common/Makefile |    1 +
 1 file changed, 1 insertion(+)

Index: linux/drivers/media/common/Makefile
===================================================================
@@ -2,6 +2,7 @@ saa7146-objs    := saa7146_i2c.o saa7146
 saa7146_vv-objs := saa7146_fops.o saa7146_video.o saa7146_hlp.o saa7146_vbi.o
 ir-common-objs  := ir-functions.o ir-keymaps.o
 
+obj-y += tuners/
 obj-$(CONFIG_VIDEO_SAA7146) += saa7146.o
 obj-$(CONFIG_VIDEO_SAA7146_VV) += saa7146_vv.o
 obj-$(CONFIG_VIDEO_IR) += ir-common.o

Subject: sound/video: sound/i2c/other/tea575x-tuner.c build fix
From: Ingo Molnar <[email protected]>
Date: Tue Apr 29 22:40:31 CEST 2008

fix build failure triggered by this config:

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

sound/i2c/other/tea575x-tuner.c: In function 'snd_tea575x_ioctl':
sound/i2c/other/tea575x-tuner.c:95: error: 'VIDIOCGCAP' undeclared (first use in this function) sound/i2c/other/tea575x-tuner.c:95: error: (Each undeclared identifier is reported only once sound/i2c/other/tea575x-tuner.c:95: error: for each function it appears in.) sound/i2c/other/tea575x-tuner.c:97: error: storage size of 'v' isn't known

that code still depends on VIDEO_V4L1_COMPAT APIs.

---
 sound/pci/Kconfig |    1 +
 1 file changed, 1 insertion(+)

Index: linux/sound/pci/Kconfig
===================================================================
@@ -527,6 +527,7 @@ config SND_FM801_TEA575X
 	tristate
 	depends on SND_FM801_TEA575X_BOOL
 	default SND_FM801
+	select VIDEO_V4L1_COMPAT
 
 config SND_HDA_INTEL
 	tristate "Intel HD Audio"

Subject: media/video/sound build fix, TEA5761/TEA5767
From: Ingo Molnar <[email protected]>
Date: Wed Apr 30 12:44:02 CEST 2008

x86.git randconfig testing found the following build error:

drivers/built-in.o: In function `v4l2_i2c_drv_attach_legacy':
tuner-core.c:(.text+0x1a829d): undefined reference to v4l2_i2c_attach' drivers/built-in.o: In function tuner_command':
tuner-core.c:(.text+0x1a971c): undefined reference to `v4l_printk_ioctl'

the reason is that the TEA5761/TEA5767 tuner code is dependent on (legacy?) V4L infrastructure like v4l2_i2c_attach, by virtue of drivers/media/video/tuner-core.c including media/v4l2-i2c-drv-legacy.h and ./sound/i2c/other/Makefile doing:

snd-tea575x-tuner-objs := tea575x-tuner.o

for now i solved this via adding a VIDEO_V4L2_COMMON dependency to MEDIA_TUNER - but i suspect there might be a better fix as well.

---
 drivers/media/common/tuners/Kconfig |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Index: linux/drivers/media/common/tuners/Kconfig
===================================================================
@@ -20,7 +20,7 @@ config MEDIA_ATTACH
 config MEDIA_TUNER
 	tristate
 	default DVB_CORE || VIDEO_DEV
-	depends on DVB_CORE || VIDEO_DEV
+	depends on (DVB_CORE || VIDEO_DEV) && VIDEO_V4L2_COMMON && I2C
 	select MEDIA_TUNER_XC2028 if !MEDIA_TUNER_CUSTOMIZE
 	select MEDIA_TUNER_XC5000 if !MEDIA_TUNER_CUSTOMIZE
 	select MEDIA_TUNER_MT20XX if !MEDIA_TUNER_CUSTOMIZE

At Wed, 30 Apr 2008 13:21:38 +0200,

ok, removed it, we'll see.

I also have the one below, of similar pattern. (so it's 8 fixlets, otherwise it's a vanilla kernel in this area)

Ingo

--------------->
Subject: video: STKWEBCAM fixes
From: Ingo Molnar <[email protected]>
Date: Tue Apr 29 15:11:29 CEST 2008

x86.git randconfig testing found these build bugs in -git:

drivers/media/video/stk-webcam.c: In function 'stk_create_sysfs_files':
drivers/media/video/stk-webcam.c:340: error: implicit declaration of function 'video_device_create_file' drivers/media/video/stk-webcam.c: In function 'stk_remove_sysfs_files':
[...]
drivers/media/video/stk-webcam.c: In function 'v4l_stk_mmap':
drivers/media/video/stk-webcam.c:846: error: 'VM_WRITE' undeclared (first use in this function) [...]

config at:

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

the reason was a missing include file, and a dependency on a deprecated (but still in use) VIDEO_V4L1_COMPAT API.

---
 drivers/media/video/Kconfig      |    1 +
 drivers/media/video/stk-webcam.c |    1 +
 2 files changed, 2 insertions(+)

Index: linux/drivers/media/video/Kconfig
===================================================================
@@ -883,6 +883,7 @@ config USB_ZR364XX
 config USB_STKWEBCAM
 	tristate "USB Syntek DC1125 Camera support"
 	depends on VIDEO_V4L2 && EXPERIMENTAL
+	select VIDEO_V4L1_COMPAT
 	---help---
 	  Say Y here if you want to use this type of camera.
 	  Supported devices are typically found in some Asus laptops,

Index: linux/drivers/media/video/stk-webcam.c
===================================================================
@@ -28,6 +28,7 @@
 #include <linux/errno.h>
 #include <linux/slab.h>
 #include <linux/kref.h>
+#include <linux/mm.h>
 
 #include <linux/usb.h>
 #include <linux/vmalloc.h>

Ingo,

It should actually be as follows:

[PATCH] cx23885: fix kbuild dependencies

Thanks to Ingo Molnar for finding this.

@@ -9,6 +9,8 @@ config VIDEO_CX23885
 	select VIDEO_IR
 	select VIDEOBUF_DVB
 	select VIDEO_CX25840
+	select VIDEO_CX2341X
+	select DVB_DIB7000P if !DVB_FE_CUSTOMISE
 	select MEDIA_TUNER_MT2131 if !DVB_FE_CUSTOMISE
 	select DVB_S5H1409 if !DVB_FE_CUSTOMISE
 	select DVB_LGDT330X if !DVB_FE_CUSTOMISE

--
video4linux-list mailing list
Unsubscribe mailto:[email protected]?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

At Wed, 30 Apr 2008 13:18:03 +0200,

What error did you get with this?

Taking a quick look,

CONFIG_VIDEO_DEV=y
CONFIG_VIDEO_V4L2_COMMON=y
# CONFIG_VIDEO_ALLOW_V4L1 is not set
CONFIG_VIDEO_V4L1_COMPAT=y

seems strange. VIDEO_ALLOW_V4L1 should have been selected since drivers/media/video/Kconfig contains the following:

	config VIDEO_V4L1
		tristate
		depends on VIDEO_DEV && VIDEO_V4L2_COMMON && VIDEO_ALLOW_V4L1
		default VIDEO_DEV && VIDEO_V4L2_COMMON && VIDEO_ALLOW_V4L1

I guess the remaining reverse-selection caused this.

thanks,

Takashi

From: Takashi Iwai <[email protected]>
Date: Wed, 30 Apr 2008 13:11:31 +0200

The question that remains is what does this mean for users?

How does a user, who wants to enable this 'sound' driver, learn that they must enable the v4l subsystem in order to do so?

This is typically why select constructs are used, to not bother the user with such messy details.

But like we see in this case, it can cause serious implementation problems if not used properly. :(

[OT to the actual problem]

I have envisioned something like a "requires tag" that would list what a config symbols needs.

Like in this case it would have been:

requires V4L

This should in the frontends then if the user selects a symbol where the 'requires' are not satisfied with a window listing the menuentries for the symbol that the user needs to enable to satisfy what the original symbol requires.

This is the only way to do this in a way so the
user is actually aware that enabling a webcam also enables USB. Or at least this is my best suggestion.

But sorry - I have not implemented it.
And it is likely more complicated than I foresee.

Sam

At Wed, 30 Apr 2008 13:29:59 +0200,

This looks promising.

Yeah, in some cases, you can't easily turn on the item depends on a complex dependency. Distros have similar problems regarding package maintenance system.

Takashi

but auto-selecting USB is the obvious thing that most users expect...

I.e. they expect a static hierarchy of 'every component' that exists and that can be selected. [yes there are complications like multiple-choice, but that's the minority]

Users want to browse the hierarchy and pick their things - and _at most_ the tool should warn if there's side side-effects of a selection - users will likely not mind that enabling a webcam also enables USB (in the overwhelming majority of the cases they couldnt care less about that).

But otherwise it should all be automatic.

And i dont even think there should be any additional Kconfig complication. Kconfig should be made _simpler_, not more complex. Maintainers list dependencies and that's it. No "select" needed at all! The stuff that needs to be selected automatically derives from the "depends on" tree that we code.

in fact even dependencies between modules of source code should be auto-discovered most of the time.

The symbol dependency tree from an allyesconfig kernel could give a 99% accurate map of all the dependencies that exist in the code. Why burden humans with that? We've got better things to do than to figure out dependencies between source code as the dependencies can be totally non-obvious.

Humans might want to _optimize_ the dependencies after the fact and get rid of unnecessary dependencies (so that the kernel gets smaller, etc.), but there should be no correctness ramifications of it visible to developers or users.

So the whole thing is being thought about exactly upside down i believe.

Ingo

Which is exactly what my "require SYMBOL2 would give us. A webcam would not depends on USB, but would "require USB". If the decide to include the webcam she will optionally be prompted so she can decide if it is OK to include USB support too.

But again - have not thought this to the end.

Would be nice.

Sam

This would be a nice feature. In fact, some SELECT's were wrongly added in the past, at V4L/DVB Kconfig's as people understood that this would be the behaviour of SELECT.

IMO, I think it is preferred that "SELECT" could act as you've described: Check if all dependencies for the selected symbol are satisfied. If not, auto-selects or prompt to the users.

The auto-select feature, without prompting could be very helpful to allow testing kconfig items, since, instead of running all randomconfig's, subsystem maintainers can use scripts that will do something like allnoconfig, and then select just the symbols requested by each driver (*).

(*) Something like:
make select="DRIVER_FOO"
and
make select="DRIVER_FOO_MODULE"

could just do this: marks NO to everything and Y (or M) to the symbol, and just the required dependencies for that symbol to compile.

Cheers,
Mauro Carvalho Chehab
http://linuxtv.org
[email protected]

--
video4linux-list mailing list
Unsubscribe mailto:[email protected]?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

At Wed, 30 Apr 2008 04:17:03 -0700 (PDT),

Yes, from the usability POV, select is better.

Indeed. The problem is that the select doesn't (can't?) resolve the dependecy in a reverse way. For example, select VIDEO_V4L1 won't turn on the dependent items such as VIDEO_DEV, VIDEO_V4L2_COMMON, VIDEO_ALLOW_V4L1, so and so.

Thus, when another dependency is added to the selected target, it can easily break the depdency chain like this case...

Takashi

updated fix below - another randconfig also pointed it out that there's an I2C dependency. (I cannot possibly do a full analysis of the dependencies of this code so i keep fixing up dependencies as they occur.)

Ingo

--------------------->
Subject: media/video/sound build fix, TEA5761/TEA5767
From: Ingo Molnar <[email protected]>
Date: Wed Apr 30 12:44:02 CEST 2008

x86.git randconfig testing found the following build error:

drivers/built-in.o: In function `v4l2_i2c_drv_attach_legacy':
tuner-core.c:(.text+0x1a829d): undefined reference to v4l2_i2c_attach' drivers/built-in.o: In function tuner_command':
tuner-core.c:(.text+0x1a971c): undefined reference to `v4l_printk_ioctl'

the reason is that the TEA5761/TEA5767 tuner code is dependent on (legacy?) V4L infrastructure like v4l2_i2c_attach, by virtue of drivers/media/video/tuner-core.c including media/v4l2-i2c-drv-legacy.h and ./sound/i2c/other/Makefile doing:

snd-tea575x-tuner-objs := tea575x-tuner.o

for now i solved this via adding a VIDEO_V4L2_COMMON dependency to MEDIA_TUNER - but i suspect there might be a better fix as well.

---
 drivers/media/common/tuners/Kconfig |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Index: linux/drivers/media/common/tuners/Kconfig
===================================================================
@@ -20,7 +20,7 @@ config MEDIA_ATTACH
 config MEDIA_TUNER
 	tristate
 	default DVB_CORE || VIDEO_DEV
-	depends on DVB_CORE || VIDEO_DEV
+	depends on (DVB_CORE || VIDEO_DEV) && VIDEO_V4L2_COMMON && I2C
 	select MEDIA_TUNER_XC2028 if !MEDIA_TUNER_CUSTOMIZE
 	select MEDIA_TUNER_XC5000 if !MEDIA_TUNER_CUSTOMIZE
 	select MEDIA_TUNER_MT20XX if !MEDIA_TUNER_CUSTOMIZE

At Wed, 30 Apr 2008 13:13:37 +0200,

I guess this patch is also unnecessary if the reverse selection from fm801-tea575x is replaced with depend by my patch. Then VIDEO_V4L2_COMMON and I2C have to be turned on as the dependency of VIDEO_V4L1.

thanks,

Takashi