* [patch] video: build fix for drivers/media/video/pvrusb2/ @ 2008-05-11 7:21 Ingo Molnar 2008-05-11 12:34 ` Michael Krufky 0 siblings, 1 reply; 10+ messages in thread From: Ingo Molnar @ 2008-05-11 7:21 UTC (permalink / raw) To: linux-kernel, Mauro Carvalho Chehab Cc: Guennadi Liakhovetski, Michael Krufky, Mike Isely 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [patch] video: build fix for drivers/media/video/pvrusb2/ 2008-05-11 7:21 [patch] video: build fix for drivers/media/video/pvrusb2/ Ingo Molnar @ 2008-05-11 12:34 ` Michael Krufky 2008-05-13 2:54 ` Mauro Carvalho Chehab 0 siblings, 1 reply; 10+ messages in thread From: Michael Krufky @ 2008-05-11 12:34 UTC (permalink / raw) To: Ingo Molnar Cc: linux-kernel, Mauro Carvalho Chehab, Guennadi Liakhovetski, Mike Isely 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [patch] video: build fix for drivers/media/video/pvrusb2/ 2008-05-11 12:34 ` Michael Krufky @ 2008-05-13 2:54 ` Mauro Carvalho Chehab 2008-05-13 4:03 ` Michael Krufky 0 siblings, 1 reply; 10+ messages in thread From: Mauro Carvalho Chehab @ 2008-05-13 2:54 UTC (permalink / raw) To: Michael Krufky Cc: Ingo Molnar, linux-kernel, Guennadi Liakhovetski, Mike Isely 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [patch] video: build fix for drivers/media/video/pvrusb2/ 2008-05-13 2:54 ` Mauro Carvalho Chehab @ 2008-05-13 4:03 ` Michael Krufky 2008-05-13 15:46 ` Mauro Carvalho Chehab 0 siblings, 1 reply; 10+ messages in thread From: Michael Krufky @ 2008-05-13 4:03 UTC (permalink / raw) To: Mauro Carvalho Chehab Cc: Ingo Molnar, linux-kernel, Guennadi Liakhovetski, Mike Isely 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [patch] video: build fix for drivers/media/video/pvrusb2/ 2008-05-13 4:03 ` Michael Krufky @ 2008-05-13 15:46 ` Mauro Carvalho Chehab 2008-05-13 15:54 ` mkrufky 2008-05-13 15:56 ` Mike Isely 0 siblings, 2 replies; 10+ messages in thread From: Mauro Carvalho Chehab @ 2008-05-13 15:46 UTC (permalink / raw) To: Michael Krufky Cc: Ingo Molnar, linux-kernel, Guennadi Liakhovetski, Mike Isely 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [patch] video: build fix for drivers/media/video/pvrusb2/ 2008-05-13 15:46 ` Mauro Carvalho Chehab @ 2008-05-13 15:54 ` mkrufky 2008-05-13 16:30 ` mkrufky 2008-05-13 15:56 ` Mike Isely 1 sibling, 1 reply; 10+ messages in thread From: mkrufky @ 2008-05-13 15:54 UTC (permalink / raw) To: mchehab; +Cc: mingo, linux-kernel, g.liakhovetski, isely 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [patch] video: build fix for drivers/media/video/pvrusb2/ 2008-05-13 15:54 ` mkrufky @ 2008-05-13 16:30 ` mkrufky 0 siblings, 0 replies; 10+ messages in thread From: mkrufky @ 2008-05-13 16:30 UTC (permalink / raw) To: mchehab; +Cc: mingo, linux-kernel, g.liakhovetski, isely 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [patch] video: build fix for drivers/media/video/pvrusb2/ 2008-05-13 15:46 ` Mauro Carvalho Chehab 2008-05-13 15:54 ` mkrufky @ 2008-05-13 15:56 ` Mike Isely 2008-05-13 16:02 ` Mike Isely 1 sibling, 1 reply; 10+ messages in thread From: Mike Isely @ 2008-05-13 15:56 UTC (permalink / raw) To: Mauro Carvalho Chehab Cc: Michael Krufky, Ingo Molnar, linux-kernel, Guennadi Liakhovetski 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [patch] video: build fix for drivers/media/video/pvrusb2/ 2008-05-13 15:56 ` Mike Isely @ 2008-05-13 16:02 ` Mike Isely 2008-05-14 6:01 ` Mauro Carvalho Chehab 0 siblings, 1 reply; 10+ messages in thread From: Mike Isely @ 2008-05-13 16:02 UTC (permalink / raw) To: Mauro Carvalho Chehab, Michael Krufky, Ingo Molnar, Linux Kernel Mailing List, Guennadi Liakhovetski Cc: Mike Isely at pobox 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [patch] video: build fix for drivers/media/video/pvrusb2/ 2008-05-13 16:02 ` Mike Isely @ 2008-05-14 6:01 ` Mauro Carvalho Chehab 0 siblings, 0 replies; 10+ messages in thread From: Mauro Carvalho Chehab @ 2008-05-14 6:01 UTC (permalink / raw) To: Mike Isely Cc: isely, Michael Krufky, Ingo Molnar, Linux Kernel Mailing List, Guennadi Liakhovetski 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2008-05-14 6:01 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2008-05-11 7:21 [patch] video: build fix for drivers/media/video/pvrusb2/ Ingo Molnar 2008-05-11 12:34 ` Michael Krufky 2008-05-13 2:54 ` Mauro Carvalho Chehab 2008-05-13 4:03 ` Michael Krufky 2008-05-13 15:46 ` Mauro Carvalho Chehab 2008-05-13 15:54 ` mkrufky 2008-05-13 16:30 ` mkrufky 2008-05-13 15:56 ` Mike Isely 2008-05-13 16:02 ` Mike Isely 2008-05-14 6:01 ` Mauro Carvalho Chehab
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).