video: build fix for drivers/media/video/pvrusb2/

PATCH In review Ingo Molnar 4 participants 10 messages May 11, 2008 – May 14, 2008 Media (V4L/DVB)

lore

Share & tools

lore

Ingo Molnar

author

2008-05-11 07:25 UTC #0 lore raw

PATCH 1/1 video: build fix for drivers/media/video/pvrusb2/

x86.git testing found the following build failure:

drivers/built-in.o: In function `pvr2_dvb_feed_thread':
pvrusb2-dvb.c:(.text+0x127e78): undefined reference to dvb_dmx_swfilter' drivers/built-in.o: In function pvr2_dvb_adapter_exit':
pvrusb2-dvb.c:(.text+0x128357): undefined reference to dvb_net_release' pvrusb2-dvb.c:(.text+0x12836f): undefined reference to dvb_dmxdev_release' [...]

with this config:

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

the reason for the missing symbols is this combination:

CONFIG_VIDEO_PVRUSB2=y
CONFIG_DVB_CORE=m

i.e. pvrusb2 is built-in, dvb-core is modular.

This patch solves the problem by adding a dependency on DVB_CORE - this is used by other drivers such as au0828 as well. This way the pvrusb2 driver can still be built, but if dvb-core is a module then it will correctly be a module as well and cannot be built-in.

Signed-off-by Ingo Molnar <[email protected]>

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

drivers/media/video/pvrusb2/Kconfig +1 -1

Index: linux/drivers/media/video/pvrusb2/Kconfig
===================================================================
@@ -1,6 +1,6 @@
 config VIDEO_PVRUSB2
 	tristate "Hauppauge WinTV-PVR USB2 support"
-	depends on VIDEO_V4L2 && I2C
+	depends on VIDEO_V4L2 && I2C && DVB_CORE
 	select FW_LOADER
 	select MEDIA_TUNER
 	select VIDEO_TVEEPROM

Michael Krufky

2008-05-11 12:34 UTC #1 lore raw

Quoted 37 lines · Ingo Molnar

x86.git testing found the following build failure:
drivers/built-in.o: In function `pvr2_dvb_feed_thread':
pvrusb2-dvb.c:(.text+0x127e78): undefined reference to dvb_dmx_swfilter' drivers/built-in.o: In function pvr2_dvb_adapter_exit':
pvrusb2-dvb.c:(.text+0x128357): undefined reference to dvb_net_release' pvrusb2-dvb.c:(.text+0x12836f): undefined reference to dvb_dmxdev_release' [...]
with this config:
http://redhat.com/~mingo/misc/config-Sun_May_11_07_06_35_CEST_2008.bad
the reason for the missing symbols is this combination:
CONFIG_VIDEO_PVRUSB2=y
CONFIG_DVB_CORE=m
i.e. pvrusb2 is built-in, dvb-core is modular.
This patch solves the problem by adding a dependency on DVB_CORE - this is used by other drivers such as au0828 as well. This way the pvrusb2 driver can still be built, but if dvb-core is a module then it will correctly be a module as well and cannot be built-in.
Signed-off-by Ingo Molnar <[email protected]>
---
 drivers/media/video/pvrusb2/Kconfig |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Index: linux/drivers/media/video/pvrusb2/Kconfig
===================================================================

drivers/media/video/pvrusb2/Kconfig line 6

Quoted 7 lines

drivers/media/video/pvrusb2/Kconfig +1 -1

@@ -1,6 +1,6 @@
 config VIDEO_PVRUSB2
       tristate "Hauppauge WinTV-PVR USB2 support"
-       depends on VIDEO_V4L2 && I2C
+       depends on VIDEO_V4L2 && I2C && DVB_CORE
       select FW_LOADER
       select MEDIA_TUNER
       select VIDEO_TVEEPROM

Ingo,

VIDEO_PVRUSB2 should not depend on DVB_CORE unless VIDEO_PVRUSB2_DVB is selected, which already depends on DVB_CORE.

For example, if a user has VIDEO_PVRUSB2_DVB not selected, then your patch would generate a false dependency on DVB_CORE.

Regards,

Mike Krufky

Mauro Carvalho Chehab

2008-05-13 02:55 UTC #2 lore raw

Quoted 48 lines · On Sun, 11 May 2008 08:34:06 -0400 "Michael Krufky" <[email protected]> wrote:

Quoted 37 lines · Ingo Molnar
x86.git testing found the following build failure:
drivers/built-in.o: In function `pvr2_dvb_feed_thread':
pvrusb2-dvb.c:(.text+0x127e78): undefined reference to dvb_dmx_swfilter' drivers/built-in.o: In function pvr2_dvb_adapter_exit':
pvrusb2-dvb.c:(.text+0x128357): undefined reference to dvb_net_release' pvrusb2-dvb.c:(.text+0x12836f): undefined reference to dvb_dmxdev_release' [...]
with this config:
http://redhat.com/~mingo/misc/config-Sun_May_11_07_06_35_CEST_2008.bad
the reason for the missing symbols is this combination:
CONFIG_VIDEO_PVRUSB2=y
CONFIG_DVB_CORE=m
i.e. pvrusb2 is built-in, dvb-core is modular.
This patch solves the problem by adding a dependency on DVB_CORE - this is used by other drivers such as au0828 as well. This way the pvrusb2 driver can still be built, but if dvb-core is a module then it will correctly be a module as well and cannot be built-in.
Signed-off-by Ingo Molnar <[email protected]>
---
 drivers/media/video/pvrusb2/Kconfig |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Index: linux/drivers/media/video/pvrusb2/Kconfig
===================================================================
drivers/media/video/pvrusb2/Kconfig line 6
Quoted 7 lines
drivers/media/video/pvrusb2/Kconfig +1 -1
@@ -1,6 +1,6 @@
 config VIDEO_PVRUSB2
       tristate "Hauppauge WinTV-PVR USB2 support"
-       depends on VIDEO_V4L2 && I2C
+       depends on VIDEO_V4L2 && I2C && DVB_CORE
       select FW_LOADER
       select MEDIA_TUNER
       select VIDEO_TVEEPROM
Ingo,

Quoted 5 lines

VIDEO_PVRUSB2 should not depend on DVB_CORE unless VIDEO_PVRUSB2_DVB is selected, which already depends on DVB_CORE.
For example, if a user has VIDEO_PVRUSB2_DVB not selected, then your patch would generate a false dependency on DVB_CORE.

Maybe we can make pvrusb2 dependent of VIDEO_MEDIA. This is a DVB_CORE || VIDEO_DEV. So, pvrusb should be 'm' in this case.

Quoted 4 lines

Regards,
Mike Krufky

Cheers,
Mauro

Michael Krufky

2008-05-13 04:03 UTC #3 lore raw

Quoted 52 lines · Mauro Carvalho Chehab

Quoted 48 lines · On Sun, 11 May 2008 08:34:06 -0400 "Michael Krufky" <[email protected]> wrote:
Quoted 37 lines · Ingo Molnar
x86.git testing found the following build failure:
drivers/built-in.o: In function `pvr2_dvb_feed_thread':
pvrusb2-dvb.c:(.text+0x127e78): undefined reference to dvb_dmx_swfilter' drivers/built-in.o: In function pvr2_dvb_adapter_exit':
pvrusb2-dvb.c:(.text+0x128357): undefined reference to dvb_net_release' pvrusb2-dvb.c:(.text+0x12836f): undefined reference to dvb_dmxdev_release' [...]
with this config:
http://redhat.com/~mingo/misc/config-Sun_May_11_07_06_35_CEST_2008.bad
the reason for the missing symbols is this combination:
CONFIG_VIDEO_PVRUSB2=y
CONFIG_DVB_CORE=m
i.e. pvrusb2 is built-in, dvb-core is modular.
This patch solves the problem by adding a dependency on DVB_CORE - this is used by other drivers such as au0828 as well. This way the pvrusb2 driver can still be built, but if dvb-core is a module then it will correctly be a module as well and cannot be built-in.
Signed-off-by Ingo Molnar <[email protected]>
---
 drivers/media/video/pvrusb2/Kconfig |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Index: linux/drivers/media/video/pvrusb2/Kconfig
===================================================================
drivers/media/video/pvrusb2/Kconfig line 6
Quoted 7 lines
drivers/media/video/pvrusb2/Kconfig +1 -1
@@ -1,6 +1,6 @@
 config VIDEO_PVRUSB2
       tristate "Hauppauge WinTV-PVR USB2 support"
-       depends on VIDEO_V4L2 && I2C
+       depends on VIDEO_V4L2 && I2C && DVB_CORE
       select FW_LOADER
       select MEDIA_TUNER
       select VIDEO_TVEEPROM
Ingo,

Quoted 8 lines

Quoted 5 lines
VIDEO_PVRUSB2 should not depend on DVB_CORE unless VIDEO_PVRUSB2_DVB is selected, which already depends on DVB_CORE.
For example, if a user has VIDEO_PVRUSB2_DVB not selected, then your patch would generate a false dependency on DVB_CORE.
Maybe we can make pvrusb2 dependent of VIDEO_MEDIA. This is a DVB_CORE || VIDEO_DEV. So, pvrusb should be 'm' in this case.

That sounds like it would be OK, although something like this would probably be better:

config VIDEO_PVRUSB2
tristate "Hauppauge WinTV-PVR USB2 support"
- depends on VIDEO_V4L2 && I2C
+ depends on VIDEO_V4L2 && I2C && (DVB_CORE if VIDEO_PVRUSB2_DVB)
select FW_LOADER
select MEDIA_TUNER
select VIDEO_TVEEPROM

I don't know if that syntax works for "depends on" , but it does work for select.

if "depends on FOO if BAR" doesnt work, would adding "select DVB_CORE if VIDEO_PVRUSB2_DVB" solve the problem?

-Mike

Mauro Carvalho Chehab

2008-05-13 15:47 UTC #4 lore raw

Quoted 68 lines · On Tue, 13 May 2008 00:03:02 -0400 "Michael Krufky" <[email protected]> wrote:

Quoted 52 lines · Mauro Carvalho Chehab
Quoted 48 lines · On Sun, 11 May 2008 08:34:06 -0400 "Michael Krufky" <[email protected]> wrote:
Quoted 37 lines · Ingo Molnar
x86.git testing found the following build failure:
drivers/built-in.o: In function `pvr2_dvb_feed_thread':
pvrusb2-dvb.c:(.text+0x127e78): undefined reference to dvb_dmx_swfilter' drivers/built-in.o: In function pvr2_dvb_adapter_exit':
pvrusb2-dvb.c:(.text+0x128357): undefined reference to dvb_net_release' pvrusb2-dvb.c:(.text+0x12836f): undefined reference to dvb_dmxdev_release' [...]
with this config:
http://redhat.com/~mingo/misc/config-Sun_May_11_07_06_35_CEST_2008.bad
the reason for the missing symbols is this combination:
CONFIG_VIDEO_PVRUSB2=y
CONFIG_DVB_CORE=m
i.e. pvrusb2 is built-in, dvb-core is modular.
This patch solves the problem by adding a dependency on DVB_CORE - this is used by other drivers such as au0828 as well. This way the pvrusb2 driver can still be built, but if dvb-core is a module then it will correctly be a module as well and cannot be built-in.
Signed-off-by Ingo Molnar <[email protected]>
---
 drivers/media/video/pvrusb2/Kconfig |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Index: linux/drivers/media/video/pvrusb2/Kconfig
===================================================================
drivers/media/video/pvrusb2/Kconfig line 6
Quoted 7 lines
drivers/media/video/pvrusb2/Kconfig +1 -1
@@ -1,6 +1,6 @@
 config VIDEO_PVRUSB2
       tristate "Hauppauge WinTV-PVR USB2 support"
-       depends on VIDEO_V4L2 && I2C
+       depends on VIDEO_V4L2 && I2C && DVB_CORE
       select FW_LOADER
       select MEDIA_TUNER
       select VIDEO_TVEEPROM
Ingo,
Quoted 8 lines
Quoted 5 lines
VIDEO_PVRUSB2 should not depend on DVB_CORE unless VIDEO_PVRUSB2_DVB is selected, which already depends on DVB_CORE.
For example, if a user has VIDEO_PVRUSB2_DVB not selected, then your patch would generate a false dependency on DVB_CORE.
Maybe we can make pvrusb2 dependent of VIDEO_MEDIA. This is a DVB_CORE || VIDEO_DEV. So, pvrusb should be 'm' in this case.
That sounds like it would be OK, although something like this would probably be better:
config VIDEO_PVRUSB2
tristate "Hauppauge WinTV-PVR USB2 support"

drivers/media/video/pvrusb2/Kconfig line 6

Quoted 5 lines

drivers/media/video/pvrusb2/Kconfig +1 -1

-       depends on VIDEO_V4L2 && I2C
+       depends on VIDEO_V4L2 && I2C && (DVB_CORE if VIDEO_PVRUSB2_DVB)
       select FW_LOADER
       select MEDIA_TUNER
       select VIDEO_TVEEPROM

This doesn't look to be a good idea, since VIDEO_PVRUSB2_DVB depends on VIDEO_PVRUSB2. So, you'll create a circular dependency. The syntax I've proposed seems cleaner. Of course, it needs to be tested. IMO, all hybrid devices should be dependent of VIDEO_MEDIA. This will help to avoid this kind of issue.

Quoted 5 lines

I don't know if that syntax works for "depends on" , but it does work for select.
if "depends on FOO if BAR" doesnt work, would adding "select DVB_CORE if VIDEO_PVRUSB2_DVB" solve the problem?

Also, this leads into a circular reference.

Cheers,
Mauro

[email protected]

2008-05-13 15:54 UTC #5 lore raw

Quoted 10 lines · Mauro Carvalho Chehab

Quoted 6 lines · On Tue, 13 May 2008 00:03:02 -0400 "Michael Krufky" <[email protected]> wrote:
Quoted 3 lines · Mauro Carvalho Chehab
On Sun, 11 May 2008 08:34:06 -0400
"Michael Krufky" <[email protected]> wrote:

Quoted 6 lines

Quoted 6 lines
Quoted 6 lines
Quoted 6 lines · Ingo Molnar
Quoted 5 lines
x86.git testing found the following build failure:
drivers/built-in.o: In function `pvr2_dvb_feed_thread':
pvrusb2-dvb.c:(.text+0x127e78): undefined reference to

`dvb_dmx_swfilter'

Quoted 2 lines

Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
drivers/built-in.o: In function `pvr2_dvb_adapter_exit':
pvrusb2-dvb.c:(.text+0x128357): undefined reference to

`dvb_net_release'

Quoted 1 line

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
pvrusb2-dvb.c:(.text+0x12836f): undefined reference to

`dvb_dmxdev_release'

Quoted 5 lines

Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
[...]
with this config:

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

Quoted 9 lines

Quoted 9 lines
Quoted 9 lines
Quoted 9 lines
Quoted 9 lines
the reason for the missing symbols is this combination:
CONFIG_VIDEO_PVRUSB2=y
CONFIG_DVB_CORE=m
i.e. pvrusb2 is built-in, dvb-core is modular.
This patch solves the problem by adding a dependency on DVB_CORE -

this

Quoted 1 line

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
is used by other drivers such as au0828 as well. This way the

pvrusb2

Quoted 24 lines

Quoted 24 lines
Quoted 24 lines
Quoted 24 lines
Quoted 14 lines
driver can still be built, but if dvb-core is a module then it will correctly be a module as well and cannot be built-in.
Signed-off-by Ingo Molnar <[email protected]>
---
 drivers/media/video/pvrusb2/Kconfig |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Index: linux/drivers/media/video/pvrusb2/Kconfig
===================================================================
drivers/media/video/pvrusb2/Kconfig line 6
Quoted 7 lines
drivers/media/video/pvrusb2/Kconfig +1 -1
@@ -1,6 +1,6 @@
 config VIDEO_PVRUSB2
       tristate "Hauppauge WinTV-PVR USB2 support"
-       depends on VIDEO_V4L2 && I2C
+       depends on VIDEO_V4L2 && I2C && DVB_CORE
       select FW_LOADER
       select MEDIA_TUNER
       select VIDEO_TVEEPROM
Ingo,

Quoted 7 lines

Quoted 7 lines
Quoted 7 lines
Quoted 5 lines
VIDEO_PVRUSB2 should not depend on DVB_CORE unless VIDEO_PVRUSB2_DVB is selected, which already depends on DVB_CORE.
For example, if a user has VIDEO_PVRUSB2_DVB not selected, then your patch would generate a false dependency on DVB_CORE.
Maybe we can make pvrusb2 dependent of VIDEO_MEDIA. This is a DVB_CORE

||
>>>  VIDEO_DEV. So, pvrusb should be 'm' in this case.
>>>

Quoted 12 lines

drivers/media/video/pvrusb2/Kconfig line 6
Quoted 11 lines
That sounds like it would be OK, although something like this would probably be better:
config VIDEO_PVRUSB2
tristate "Hauppauge WinTV-PVR USB2 support"
drivers/media/video/pvrusb2/Kconfig +1 -1
-       depends on VIDEO_V4L2 && I2C
+       depends on VIDEO_V4L2 && I2C && (DVB_CORE if VIDEO_PVRUSB2_DVB)
       select FW_LOADER
       select MEDIA_TUNER
       select VIDEO_TVEEPROM

Quoted 4 lines

This doesn't look to be a good idea, since VIDEO_PVRUSB2_DVB depends on VIDEO_PVRUSB2. So, you'll create a circular dependency. The syntax I've proposed seems cleaner. Of course, it needs to be tested. IMO, all hybrid devices should be dependent of VIDEO_MEDIA. This will help to avoid this

kind

Quoted 11 lines

of issue.
Quoted 6 lines
I don't know if that syntax works for "depends on" , but it does work for select.
if "depends on FOO if BAR" doesnt work, would adding "select DVB_CORE if VIDEO_PVRUSB2_DVB" solve the problem?
Also, this leads into a circular reference.

Mauro,

You're right -- I didn't consider the circular dependency.

Your suggestion (depends on VIDEO_MEDIA) seems like the right way to go.

Regards,

Mike Krufky

[email protected]

2008-05-13 16:31 UTC #6 lore raw

Quoted 107 lines · Michael Krufky

Quoted 106 lines · Mauro Carvalho Chehab
Quoted 80 lines · On Tue, 13 May 2008 00:03:02 -0400 "Michael Krufky" <[email protected]> wrote:
Quoted 63 lines · Mauro Carvalho Chehab
Quoted 6 lines · On Sun, 11 May 2008 08:34:06 -0400 "Michael Krufky" <[email protected]> wrote:
Quoted 5 lines · Ingo Molnar
x86.git testing found the following build failure:
drivers/built-in.o: In function `pvr2_dvb_feed_thread':
pvrusb2-dvb.c:(.text+0x127e78): undefined reference to
`dvb_dmx_swfilter'
Quoted 2 lines
Quoted 2 lines
drivers/built-in.o: In function `pvr2_dvb_adapter_exit':
pvrusb2-dvb.c:(.text+0x128357): undefined reference to
`dvb_net_release'
Quoted 1 line
Quoted 1 line
pvrusb2-dvb.c:(.text+0x12836f): undefined reference to
`dvb_dmxdev_release'
Quoted 5 lines
Quoted 5 lines
[...]
with this config:
http://redhat.com/~mingo/misc/config-Sun_May_11_07_06_35_CEST_2008.bad
Quoted 9 lines
Quoted 9 lines
the reason for the missing symbols is this combination:
CONFIG_VIDEO_PVRUSB2=y
CONFIG_DVB_CORE=m
i.e. pvrusb2 is built-in, dvb-core is modular.
This patch solves the problem by adding a dependency on
DVB_CORE - this
Quoted 1 line
Quoted 1 line
is used by other drivers such as au0828 as well. This way the
pvrusb2
Quoted 1 line
Quoted 1 line
driver can still be built, but if dvb-core is a module then it
will
Quoted 9 lines
Quoted 9 lines
correctly be a module as well and cannot be built-in.
Signed-off-by Ingo Molnar <[email protected]>
---
 drivers/media/video/pvrusb2/Kconfig |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Index: linux/drivers/media/video/pvrusb2/Kconfig
===================================================================
Quoted 14 lines
drivers/media/video/pvrusb2/Kconfig line 6
Quoted 11 lines
drivers/media/video/pvrusb2/Kconfig +1 -1
@@ -1,6 +1,6 @@
 config VIDEO_PVRUSB2
       tristate "Hauppauge WinTV-PVR USB2 support"
-       depends on VIDEO_V4L2 && I2C
+       depends on VIDEO_V4L2 && I2C && DVB_CORE
       select FW_LOADER
       select MEDIA_TUNER
       select VIDEO_TVEEPROM
Ingo,
Quoted 1 line
VIDEO_PVRUSB2 should not depend on DVB_CORE unless
VIDEO_PVRUSB2_DVB
Quoted 2 lines
is selected, which already depends on DVB_CORE.
Quoted 8 lines
Quoted 1 line
For example, if a user has VIDEO_PVRUSB2_DVB not selected, then
your
Quoted 1 line
patch would generate a false dependency on DVB_CORE.
Maybe we can make pvrusb2 dependent of VIDEO_MEDIA. This is a
DVB_CORE ||
VIDEO_DEV. So, pvrusb should be 'm' in this case.
That sounds like it would be OK, although something like this would probably be better:
config VIDEO_PVRUSB2
tristate "Hauppauge WinTV-PVR USB2 support"
drivers/media/video/pvrusb2/Kconfig line 6
Quoted 5 lines
drivers/media/video/pvrusb2/Kconfig +1 -1
-       depends on VIDEO_V4L2 && I2C
+       depends on VIDEO_V4L2 && I2C && (DVB_CORE if VIDEO_PVRUSB2_DVB)
       select FW_LOADER
       select MEDIA_TUNER
       select VIDEO_TVEEPROM
This doesn't look to be a good idea, since VIDEO_PVRUSB2_DVB depends on VIDEO_PVRUSB2. So, you'll create a circular dependency. The syntax I've proposed seems cleaner. Of course, it needs to be tested. IMO, all hybrid
devices should be dependent of VIDEO_MEDIA. This will help to avoid this kind
of issue.
Quoted 6 lines
I don't know if that syntax works for "depends on" , but it does work for select.
if "depends on FOO if BAR" doesnt work, would adding "select DVB_CORE if VIDEO_PVRUSB2_DVB" solve the problem?

Quoted 6 lines

Quoted 1 line
Also, this leads into a circular reference.
Mauro,
You're right -- I didn't consider the circular dependency.
Your suggestion (depends on VIDEO_MEDIA) seems like the right way to go.

Mauro,

It has just occurred to me -- there is yet another issue that is created by the dependency on VIDEO_MEDIA .....

The pvrusb2 driver *always* has a V4L2 interface exposed, and optionally can have a DVB interface.

So, VIDEO_PVRUSB2 will *always* depend on VIDEO_DEV, but conditionally depend on DVB_CORE.

...Sounds like the correct clause should be:

depends on VIDEO_V4L2 && I2C && VIDEO_DEV && VIDEO_MEDIA

...what do you think?

-Mike Krufky

Mike Isely

2008-05-13 15:56 UTC #7 lore raw

In reply to Mauro Carvalho Chehab

Quoted 21 lines · Mauro Carvalho Chehab

drivers/media/video/pvrusb2/Kconfig line 6
Quoted 11 lines · On Tue, 13 May 2008 00:03:02 -0400 "Michael Krufky" <[email protected]> wrote:
That sounds like it would be OK, although something like this would probably be better:
config VIDEO_PVRUSB2
tristate "Hauppauge WinTV-PVR USB2 support"
drivers/media/video/pvrusb2/Kconfig +1 -1
-       depends on VIDEO_V4L2 && I2C
+       depends on VIDEO_V4L2 && I2C && (DVB_CORE if VIDEO_PVRUSB2_DVB)
       select FW_LOADER
       select MEDIA_TUNER
       select VIDEO_TVEEPROM
This doesn't look to be a good idea, since VIDEO_PVRUSB2_DVB depends on VIDEO_PVRUSB2. So, you'll create a circular dependency. The syntax I've proposed seems cleaner. Of course, it needs to be tested. IMO, all hybrid devices should be dependent of VIDEO_MEDIA. This will help to avoid this kind of issue.

Quoted 7 lines

Quoted 5 lines
I don't know if that syntax works for "depends on" , but it does work for select.
if "depends on FOO if BAR" doesnt work, would adding "select DVB_CORE if VIDEO_PVRUSB2_DVB" solve the problem?
Also, this leads into a circular reference.

Mauro:

Where is the loop that results in a circular reference? No part of V4L or DVB depends (or otherwise has any reliance) on VIDEO_PVRUSB2. All Mike Krufky is trying to set up is that VIDEO_PVRUSB2 should only depend on DVB_CORE if the DVB part of the driver (i.e. VIDEO_PVRUSB2_DVB) has been enabled. Without VIDEO_PVRUSB2_DVB selected, then DVB_CORE is not required by the driver.

I've been quiet about this issue because I'm not a kconfig expert, but you've lost me with this conclusion.

-Mike

Signature · 3 lines

Mike Isely
isely @ pobox (dot) com
PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8

Mike Isely

2008-05-13 16:02 UTC #8 lore raw

Quoted 40 lines · Mike Isely

Quoted 21 lines · Mauro Carvalho Chehab
drivers/media/video/pvrusb2/Kconfig line 6
Quoted 11 lines · On Tue, 13 May 2008 00:03:02 -0400 "Michael Krufky" <[email protected]> wrote:
That sounds like it would be OK, although something like this would probably be better:
config VIDEO_PVRUSB2
tristate "Hauppauge WinTV-PVR USB2 support"
drivers/media/video/pvrusb2/Kconfig +1 -1
-       depends on VIDEO_V4L2 && I2C
+       depends on VIDEO_V4L2 && I2C && (DVB_CORE if VIDEO_PVRUSB2_DVB)
       select FW_LOADER
       select MEDIA_TUNER
       select VIDEO_TVEEPROM
This doesn't look to be a good idea, since VIDEO_PVRUSB2_DVB depends on VIDEO_PVRUSB2. So, you'll create a circular dependency. The syntax I've proposed seems cleaner. Of course, it needs to be tested. IMO, all hybrid devices should be dependent of VIDEO_MEDIA. This will help to avoid this kind of issue.
Quoted 7 lines
Quoted 5 lines
I don't know if that syntax works for "depends on" , but it does work for select.
if "depends on FOO if BAR" doesnt work, would adding "select DVB_CORE if VIDEO_PVRUSB2_DVB" solve the problem?
Also, this leads into a circular reference.
Mauro:
Where is the loop that results in a circular reference? No part of V4L or DVB depends (or otherwise has any reliance) on VIDEO_PVRUSB2. All Mike Krufky is trying to set up is that VIDEO_PVRUSB2 should only depend on DVB_CORE if the DVB part of the driver (i.e. VIDEO_PVRUSB2_DVB) has been enabled. Without VIDEO_PVRUSB2_DVB selected, then DVB_CORE is not required by the driver.

Quoted 4 lines

I've been quiet about this issue because I'm not a kconfig expert, but you've lost me with this conclusion.
-Mike

Never mind. I think I see it, and will figure out the rest off-line.

-Mike

Signature · 3 lines

Mike Isely
isely @ pobox (dot) com
PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8

Mauro Carvalho Chehab

2008-05-14 06:01 UTC #9 lore raw

Quoted 42 lines · On Tue, 13 May 2008 11:02:05 -0500 (CDT) Mike Isely <[email protected]> wrote:

Quoted 40 lines · Mike Isely
Quoted 21 lines · Mauro Carvalho Chehab
Review
Quoted 11 lines · On Tue, 13 May 2008 00:03:02 -0400 "Michael Krufky" <[email protected]> wrote:
That sounds like it would be OK, although something like this would probably be better:
config VIDEO_PVRUSB2
tristate "Hauppauge WinTV-PVR USB2 support"
(quoted diff) +1 -1
-       depends on VIDEO_V4L2 && I2C
+       depends on VIDEO_V4L2 && I2C && (DVB_CORE if VIDEO_PVRUSB2_DVB)
       select FW_LOADER
       select MEDIA_TUNER
       select VIDEO_TVEEPROM
This doesn't look to be a good idea, since VIDEO_PVRUSB2_DVB depends on VIDEO_PVRUSB2. So, you'll create a circular dependency. The syntax I've proposed seems cleaner. Of course, it needs to be tested. IMO, all hybrid devices should be dependent of VIDEO_MEDIA. This will help to avoid this kind of issue.
Quoted 7 lines
Quoted 5 lines
I don't know if that syntax works for "depends on" , but it does work for select.
if "depends on FOO if BAR" doesnt work, would adding "select DVB_CORE if VIDEO_PVRUSB2_DVB" solve the problem?
Also, this leads into a circular reference.
Mauro:
Where is the loop that results in a circular reference? No part of V4L or DVB depends (or otherwise has any reliance) on VIDEO_PVRUSB2. All Mike Krufky is trying to set up is that VIDEO_PVRUSB2 should only depend on DVB_CORE if the DVB part of the driver (i.e. VIDEO_PVRUSB2_DVB) has been enabled. Without VIDEO_PVRUSB2_DVB selected, then DVB_CORE is not required by the driver.

Quoted 6 lines

Quoted 4 lines
I've been quiet about this issue because I'm not a kconfig expert, but you've lost me with this conclusion.
-Mike
Never mind. I think I see it, and will figure out the rest off-line.

The solution were not as easy as I've originally imagined. VIDEO_MEDIA logic were broken. So, all tuners were still broken.

I needed to fix VIDEO_MEDIA logic first, to obey to this truth table:

CONFIG_VIDEO_DEV CONFIG_DVB_CORE CONFIG_VIDEO_MEDIA

        N                       N                       N
        N                       M                       M
        N                       Y                       Y
        M                       N                       M
        M                       M                       M
        M                       Y                       M
        Y                       N                       Y
        Y                       M                       M
        Y                       Y                       Y

After this change, a simple dependency add solved the reported issue:

config VIDEO_PVRUSB2

	tristate "Hauppauge WinTV-PVR USB2 support"
	depends on VIDEO_V4L2 && I2C

+ depends on VIDEO_MEDIA # Avoids pvrusb = Y / DVB = M

I've tested the logic. seems fine now.

I did a quick review at the other modules that has hybrid support (cx88, saa7134, em28xx, bttv) and they seemed ok to my eyes. Yet, I didn't test.

---

There are other configurations that seem to be broken. For example, if V4L_CORE=m and DVB_CORE=y, a V4L driver should be 'M'. So, a tuner should also be 'M', otherwise, it will break compilation. a DVB support for that card will also be 'M', at the drivers I've reviewed.

However, a DVB-only driver can be 'Y', but can select a tuner. This will cause a breakage.

I'll try to simulate some of those issues and see what happens.

Anyway, I've added severak Kbuild fixes at my -git tree:

http://www.kernel.org/git/?p=linux/kernel/git/mchehab/v4l-dvb.git

I intend to send the fixes I have later today. Now, I need to sleep ;)

Cheers,
Mauro