[patch] video: build fix for drivers/media/video/pvrusb2/

[patch] video: build fix for drivers/media/video/pvrusb2/

[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 <mingo@elte.hu>
---
 drivers/media/video/pvrusb2/Kconfig |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

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

Re: [patch] video: build fix for drivers/media/video/pvrusb2/

[Michael Krufky]

On Sun, May 11, 2008 at 3:21 AM, Ingo Molnar <mingo@elte.hu> wrote:
>
> 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 <mingo@elte.hu>
> ---
>  drivers/media/video/pvrusb2/Kconfig |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> Index: linux/drivers/media/video/pvrusb2/Kconfig
> ===================================================================
> --- linux.orig/drivers/media/video/pvrusb2/Kconfig
> +++ 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
>

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

Re: [patch] video: build fix for drivers/media/video/pvrusb2/

[Mauro Carvalho Chehab]

On Sun, 11 May 2008 08:34:06 -0400
"Michael Krufky" <mkrufky@linuxtv.org> wrote:

> On Sun, May 11, 2008 at 3:21 AM, Ingo Molnar <mingo@elte.hu> wrote:
> >
> > 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 <mingo@elte.hu>
> > ---
> >  drivers/media/video/pvrusb2/Kconfig |    2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > Index: linux/drivers/media/video/pvrusb2/Kconfig
> > ===================================================================
> > --- linux.orig/drivers/media/video/pvrusb2/Kconfig
> > +++ 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
> >
> 
> 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.

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




Cheers,
Mauro

Re: [patch] video: build fix for drivers/media/video/pvrusb2/

[Michael Krufky]

On Mon, May 12, 2008 at 10:54 PM, Mauro Carvalho Chehab
<mchehab@infradead.org> wrote:
>
> On Sun, 11 May 2008 08:34:06 -0400
>  "Michael Krufky" <mkrufky@linuxtv.org> wrote:
>
>  > On Sun, May 11, 2008 at 3:21 AM, Ingo Molnar <mingo@elte.hu> wrote:
>  > >
>  > > 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 <mingo@elte.hu>
>  > > ---
>  > >  drivers/media/video/pvrusb2/Kconfig |    2 +-
>  > >  1 file changed, 1 insertion(+), 1 deletion(-)
>  > >
>  > > Index: linux/drivers/media/video/pvrusb2/Kconfig
>  > > ===================================================================
>  > > --- linux.orig/drivers/media/video/pvrusb2/Kconfig
>  > > +++ 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
>  > >
>  >
>  > 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.
>
>  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

Re: [patch] video: build fix for drivers/media/video/pvrusb2/

[Mauro Carvalho Chehab]

On Tue, 13 May 2008 00:03:02 -0400
"Michael Krufky" <mkrufky@linuxtv.org> wrote:

> On Mon, May 12, 2008 at 10:54 PM, Mauro Carvalho Chehab
> <mchehab@infradead.org> wrote:
> >
> > On Sun, 11 May 2008 08:34:06 -0400
> >  "Michael Krufky" <mkrufky@linuxtv.org> wrote:
> >
> >  > On Sun, May 11, 2008 at 3:21 AM, Ingo Molnar <mingo@elte.hu> wrote:
> >  > >
> >  > > 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 <mingo@elte.hu>
> >  > > ---
> >  > >  drivers/media/video/pvrusb2/Kconfig |    2 +-
> >  > >  1 file changed, 1 insertion(+), 1 deletion(-)
> >  > >
> >  > > Index: linux/drivers/media/video/pvrusb2/Kconfig
> >  > > ===================================================================
> >  > > --- linux.orig/drivers/media/video/pvrusb2/Kconfig
> >  > > +++ 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
> >  > >
> >  >
> >  > 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.
> >
> >  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

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.

> 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

Re: [patch] video: build fix for drivers/media/video/pvrusb2/

[mkrufky]

Mauro Carvalho Chehab wrote:
> On Tue, 13 May 2008 00:03:02 -0400
> "Michael Krufky" <mkrufky@linuxtv.org> wrote:
>
>   
>> On Mon, May 12, 2008 at 10:54 PM, Mauro Carvalho Chehab
>> <mchehab@infradead.org> wrote:
>>     
>>> On Sun, 11 May 2008 08:34:06 -0400
>>>  "Michael Krufky" <mkrufky@linuxtv.org> wrote:
>>>
>>>  > On Sun, May 11, 2008 at 3:21 AM, Ingo Molnar <mingo@elte.hu> wrote:
>>>  > >
>>>  > > 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 <mingo@elte.hu>
>>>  > > ---
>>>  > >  drivers/media/video/pvrusb2/Kconfig |    2 +-
>>>  > >  1 file changed, 1 insertion(+), 1 deletion(-)
>>>  > >
>>>  > > Index: linux/drivers/media/video/pvrusb2/Kconfig
>>>  > > ===================================================================
>>>  > > --- linux.orig/drivers/media/video/pvrusb2/Kconfig
>>>  > > +++ 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
>>>  > >
>>>  >
>>>  > 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.
>>>
>>>  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
>>     
>
> 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.
>
>   
>> 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

Re: [patch] video: build fix for drivers/media/video/pvrusb2/

[Mike Isely]

On Tue, 13 May 2008, Mauro Carvalho Chehab wrote:

> On Tue, 13 May 2008 00:03:02 -0400
> "Michael Krufky" <mkrufky@linuxtv.org> 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"
> > -       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.
> 
> > 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


-- 

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

Re: [patch] video: build fix for drivers/media/video/pvrusb2/

[Mike Isely]

On Tue, 13 May 2008, Mike Isely wrote:

> On Tue, 13 May 2008, Mauro Carvalho Chehab wrote:
> 
> > On Tue, 13 May 2008 00:03:02 -0400
> > "Michael Krufky" <mkrufky@linuxtv.org> 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"
> > > -       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.
> > 
> > > 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

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

  -Mike

-- 

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

Re: [patch] video: build fix for drivers/media/video/pvrusb2/

[mkrufky]

Michael Krufky wrote:
> Mauro Carvalho Chehab wrote:
>> On Tue, 13 May 2008 00:03:02 -0400
>> "Michael Krufky" <mkrufky@linuxtv.org> wrote:
>>
>>  
>>> On Mon, May 12, 2008 at 10:54 PM, Mauro Carvalho Chehab
>>> <mchehab@infradead.org> wrote:
>>>    
>>>> On Sun, 11 May 2008 08:34:06 -0400
>>>>  "Michael Krufky" <mkrufky@linuxtv.org> wrote:
>>>>
>>>>  > On Sun, May 11, 2008 at 3:21 AM, Ingo Molnar <mingo@elte.hu> wrote:
>>>>  > >
>>>>  > > 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 <mingo@elte.hu>
>>>>  > > ---
>>>>  > >  drivers/media/video/pvrusb2/Kconfig |    2 +-
>>>>  > >  1 file changed, 1 insertion(+), 1 deletion(-)
>>>>  > >
>>>>  > > Index: linux/drivers/media/video/pvrusb2/Kconfig
>>>>  > > 
>>>> ===================================================================
>>>>  > > --- linux.orig/drivers/media/video/pvrusb2/Kconfig
>>>>  > > +++ 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
>>>>  > >
>>>>  >
>>>>  > 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.
>>>>
>>>>  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
>>>     
>>
>> 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.
>>
>>  
>>> 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. 
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

Re: [patch] video: build fix for drivers/media/video/pvrusb2/

[Mauro Carvalho Chehab]

On Tue, 13 May 2008 11:02:05 -0500 (CDT)
Mike Isely <isely@isely.net> wrote:

> On Tue, 13 May 2008, Mike Isely wrote:
> 
> > On Tue, 13 May 2008, Mauro Carvalho Chehab wrote:
> > 
> > > On Tue, 13 May 2008 00:03:02 -0400
> > > "Michael Krufky" <mkrufky@linuxtv.org> 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"
> > > > -       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.
> > > 
> > > > 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
> 
> 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