[patch, -git] media/video/sound build fix, TEA5761/TEA5767

[patch, -git] media/video/sound build fix, TEA5761/TEA5767

[Ingo Molnar]

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.

Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
 drivers/media/common/tuners/Kconfig |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Index: linux/drivers/media/common/tuners/Kconfig
===================================================================
--- linux.orig/drivers/media/common/tuners/Kconfig
+++ 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

Re: [patch, -git] media/video/sound build fix, TEA5761/TEA5767

[Takashi Iwai]

At Wed, 30 Apr 2008 13:01:15 +0200,
Ingo Molnar wrote:
> 
> 
> 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.

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.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---

diff --git a/sound/pci/Kconfig b/sound/pci/Kconfig
index 581debf..7e47421 100644
--- a/sound/pci/Kconfig
+++ b/sound/pci/Kconfig
@@ -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"

Re: [patch, -git] media/video/sound build fix, TEA5761/TEA5767

[Ingo Molnar]

* Ingo Molnar <mingo@elte.hu> wrote:

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

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 <mingo@elte.hu>
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.

Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
 drivers/media/common/tuners/Kconfig |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Index: linux/drivers/media/common/tuners/Kconfig
===================================================================
--- linux.orig/drivers/media/common/tuners/Kconfig
+++ 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

Re: [patch, -git] media/video/sound build fix, TEA5761/TEA5767

[Ingo Molnar]

* Takashi Iwai <tiwai@suse.de> wrote:

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

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

	Ingo

Re: [patch, -git] media/video/sound build fix, TEA5761/TEA5767

[David Miller]

From: Takashi Iwai <tiwai@suse.de>
Date: Wed, 30 Apr 2008 13:11:31 +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?

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. :(

Re: [patch, -git] media/video/sound build fix, TEA5761/TEA5767

[Ingo Molnar]

* Ingo Molnar <mingo@elte.hu> wrote:

> > 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?
> 
> thanks, i've added it to the mix, instead of my patch.

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

Re: [patch, -git] media/video/sound build fix, TEA5761/TEA5767

[Ingo Molnar]

* Ingo Molnar <mingo@elte.hu> wrote:

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

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 <mingo@elte.hu>
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.

Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
 drivers/media/video/Kconfig |    2 ++
 1 file changed, 2 insertions(+)

Index: linux/drivers/media/video/Kconfig
===================================================================
--- linux.orig/drivers/media/video/Kconfig
+++ 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 <mingo@elte.hu>
Date: Mon Apr 28 23:22:13 CEST 2008

Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
 drivers/media/video/cx23885/Kconfig |    2 ++
 1 file changed, 2 insertions(+)

Index: linux/drivers/media/video/cx23885/Kconfig
===================================================================
--- linux.orig/drivers/media/video/cx23885/Kconfig
+++ 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 <mingo@elte.hu>
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)

Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
 drivers/media/Makefile |    2 ++
 1 file changed, 2 insertions(+)

Index: linux/drivers/media/Makefile
===================================================================
--- linux.orig/drivers/media/Makefile
+++ 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 <tiwai@suse.de>
Date: Wed, 30 Apr 2008 13:11:31 +0200

At Wed, 30 Apr 2008 13:01:15 +0200,
Ingo Molnar wrote:
>
>
> 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.

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.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
---
 sound/pci/Kconfig |    5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

Index: linux/sound/pci/Kconfig
===================================================================
--- linux.orig/sound/pci/Kconfig
+++ 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 <mingo@elte.hu>
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").

Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
 drivers/media/common/Makefile |    1 +
 1 file changed, 1 insertion(+)

Index: linux/drivers/media/common/Makefile
===================================================================
--- linux.orig/drivers/media/common/Makefile
+++ 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 <mingo@elte.hu>
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.

Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
 sound/pci/Kconfig |    1 +
 1 file changed, 1 insertion(+)

Index: linux/sound/pci/Kconfig
===================================================================
--- linux.orig/sound/pci/Kconfig
+++ 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 <mingo@elte.hu>
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.

Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
 drivers/media/common/tuners/Kconfig |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Index: linux/drivers/media/common/tuners/Kconfig
===================================================================
--- linux.orig/drivers/media/common/tuners/Kconfig
+++ 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

Re: [patch, -git] media/video/sound build fix, TEA5761/TEA5767

[Sam Ravnborg]

On Wed, Apr 30, 2008 at 04:17:03AM -0700, David Miller wrote:
> From: Takashi Iwai <tiwai@suse.de>
> Date: Wed, 30 Apr 2008 13:11:31 +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?
> 
> 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?

[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

Re: [patch, -git] media/video/sound build fix, TEA5761/TEA5767

[Takashi Iwai]

At Wed, 30 Apr 2008 04:17:03 -0700 (PDT),
David Miller wrote:
> 
> From: Takashi Iwai <tiwai@suse.de>
> Date: Wed, 30 Apr 2008 13:11:31 +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?
> 
> 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.

Yes, from the usability POV, select is better.

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

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

Re: [patch, -git] media/video/sound build fix, TEA5761/TEA5767

[Takashi Iwai]

At Wed, 30 Apr 2008 13:21:38 +0200,
Ingo Molnar wrote:
> 
> Subject: sound/video: sound/i2c/other/tea575x-tuner.c build fix
> From: Ingo Molnar <mingo@elte.hu>
> 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.
> 
> Signed-off-by: Ingo Molnar <mingo@elte.hu>
> ---
>  sound/pci/Kconfig |    1 +
>  1 file changed, 1 insertion(+)
> 
> Index: linux/sound/pci/Kconfig
> ===================================================================
> --- linux.orig/sound/pci/Kconfig
> +++ 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"

This one shouldn't be needed with my patch.


Takashi

Re: [patch, -git] media/video/sound build fix, TEA5761/TEA5767

[Takashi Iwai]

At Wed, 30 Apr 2008 13:18:03 +0200,
Ingo Molnar wrote:
> 
> 
> * Ingo Molnar <mingo@elte.hu> wrote:
> 
> > > 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?
> > 
> > thanks, i've added it to the mix, instead of my patch.
> 
> it now fails with another randconfig:
> 
>    http://redhat.com/~mingo/misc/config-Wed_Apr_30_13_13_26_CEST_2008.bad

What error did you get with this?

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

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

Re: [patch, -git] media/video/sound build fix, TEA5761/TEA5767

[Takashi Iwai]

At Wed, 30 Apr 2008 13:13:37 +0200,
Ingo Molnar wrote:
> 
> 
> * Ingo Molnar <mingo@elte.hu> wrote:
> 
> > x86.git randconfig testing found the following new build error in 
> > latest -git:
> 
> 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 <mingo@elte.hu>
> 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.
> 
> Signed-off-by: Ingo Molnar <mingo@elte.hu>

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

> ---
>  drivers/media/common/tuners/Kconfig |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> Index: linux/drivers/media/common/tuners/Kconfig
> ===================================================================
> --- linux.orig/drivers/media/common/tuners/Kconfig
> +++ 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
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
> 

Re: [patch, -git] media/video/sound build fix, TEA5761/TEA5767

[Ingo Molnar]

* Takashi Iwai <tiwai@suse.de> wrote:

> >  	default SND_FM801
> > +	select VIDEO_V4L1_COMPAT
> >  
> >  config SND_HDA_INTEL
> >  	tristate "Intel HD Audio"
> 
> This one shouldn't be needed with my patch.

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 <mingo@elte.hu>
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.

Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
 drivers/media/video/Kconfig      |    1 +
 drivers/media/video/stk-webcam.c |    1 +
 2 files changed, 2 insertions(+)

Index: linux/drivers/media/video/Kconfig
===================================================================
--- linux.orig/drivers/media/video/Kconfig
+++ 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
===================================================================
--- linux.orig/drivers/media/video/stk-webcam.c
+++ 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>

Re: [patch, -git] media/video/sound build fix, TEA5761/TEA5767

[Takashi Iwai]

At Wed, 30 Apr 2008 13:29:59 +0200,
Sam Ravnborg wrote:
> 
> On Wed, Apr 30, 2008 at 04:17:03AM -0700, David Miller wrote:
> > From: Takashi Iwai <tiwai@suse.de>
> > Date: Wed, 30 Apr 2008 13:11:31 +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?
> > 
> > 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?
> 
> [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.

This looks promising.

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

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

Re: [patch, -git] media/video/sound build fix, TEA5761/TEA5767

[Ingo Molnar]

* Sam Ravnborg <sam@ravnborg.org> wrote:

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

Re: [v4l-dvb-maintainer] [patch, -git] media/video/sound build fix, TEA5761/TEA5767

[Michael Krufky]

On Wed, Apr 30, 2008 at 7:21 AM, Ingo Molnar <mingo@elte.hu> wrote:
>  Subject: dvb: fix3
>  From: Ingo Molnar <mingo@elte.hu>
>  Date: Mon Apr 28 23:22:13 CEST 2008
>
>  Signed-off-by: Ingo Molnar <mingo@elte.hu>
>  ---
>   drivers/media/video/cx23885/Kconfig |    2 ++
>   1 file changed, 2 insertions(+)
>
>  Index: linux/drivers/media/video/cx23885/Kconfig
>  ===================================================================
>  --- linux.orig/drivers/media/video/cx23885/Kconfig
>  +++ 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


Ingo,

It should actually be as follows:



[PATCH] cx23885: fix kbuild dependencies

Thanks to Ingo Molnar for finding this.

Signed-off-by: Michael Krufky <mkrufky@linuxtv.org>

--- linux.orig/drivers/media/video/cx23885/Kconfig
+++ 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 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

Re: [patch, -git] media/video/sound build fix, TEA5761/TEA5767

[Sam Ravnborg]

On Wed, Apr 30, 2008 at 02:18:54PM +0200, Ingo Molnar wrote:
> 
> * Sam Ravnborg <sam@ravnborg.org> wrote:
> 
> > [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 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).

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.

> 
> 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.
Would be nice.

	Sam

Re: [patch, -git] media/video/sound build fix, TEA5761/TEA5767

[Mauro Carvalho Chehab]

> 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

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
mchehab@infradead.org