[2.6 patch] cx88/saa7134: remove unused -DHAVE_VIDEO_BUF_DVB

[2.6 patch] cx88/saa7134: remove unused -DHAVE_VIDEO_BUF_DVB

[Adrian Bunk]

This patch removes the unused HAVE_VIDEO_BUF_DVB define.

Signed-off-by: Adrian Bunk <bunk@stusta.de>

--- linux-2.6.19-rc6-mm2/drivers/media/video/cx88/Makefile.old	2006-12-07 15:04:11.000000000 +0100
+++ linux-2.6.19-rc6-mm2/drivers/media/video/cx88/Makefile	2006-12-07 15:05:04.000000000 +0100
@@ -13,7 +13,6 @@
 EXTRA_CFLAGS += -Idrivers/media/dvb/dvb-core
 EXTRA_CFLAGS += -Idrivers/media/dvb/frontends
 
-extra-cflags-$(CONFIG_VIDEO_BUF_DVB) += -DHAVE_VIDEO_BUF_DVB=1
 extra-cflags-$(CONFIG_VIDEO_CX88_VP3054)+= -DHAVE_VP3054_I2C=1
 
 EXTRA_CFLAGS += $(extra-cflags-y) $(extra-cflags-m)
--- linux-2.6.19-rc6-mm2/drivers/media/video/saa7134/Makefile.old	2006-12-07 15:04:45.000000000 +0100
+++ linux-2.6.19-rc6-mm2/drivers/media/video/saa7134/Makefile	2006-12-07 15:04:58.000000000 +0100
@@ -15,6 +15,3 @@
 EXTRA_CFLAGS += -Idrivers/media/dvb/dvb-core
 EXTRA_CFLAGS += -Idrivers/media/dvb/frontends
 
-extra-cflags-$(CONFIG_VIDEO_BUF_DVB) += -DHAVE_VIDEO_BUF_DVB=1
-
-EXTRA_CFLAGS += $(extra-cflags-y) $(extra-cflags-m)

Re: [v4l-dvb-maintainer] [2.6 patch] cx88/saa7134: remove unused -DHAVE_VIDEO_BUF_DVB

[Michael Krufky]

Adrian Bunk wrote:
> This patch removes the unused HAVE_VIDEO_BUF_DVB define.
> 
> Signed-off-by: Adrian Bunk <bunk@stusta.de>
> 
> --- linux-2.6.19-rc6-mm2/drivers/media/video/cx88/Makefile.old	2006-12-07 15:04:11.000000000 +0100
> +++ linux-2.6.19-rc6-mm2/drivers/media/video/cx88/Makefile	2006-12-07 15:05:04.000000000 +0100
> @@ -13,7 +13,6 @@
>  EXTRA_CFLAGS += -Idrivers/media/dvb/dvb-core
>  EXTRA_CFLAGS += -Idrivers/media/dvb/frontends
>  
> -extra-cflags-$(CONFIG_VIDEO_BUF_DVB) += -DHAVE_VIDEO_BUF_DVB=1
>  extra-cflags-$(CONFIG_VIDEO_CX88_VP3054)+= -DHAVE_VP3054_I2C=1
>  
>  EXTRA_CFLAGS += $(extra-cflags-y) $(extra-cflags-m)
> --- linux-2.6.19-rc6-mm2/drivers/media/video/saa7134/Makefile.old	2006-12-07 15:04:45.000000000 +0100
> +++ linux-2.6.19-rc6-mm2/drivers/media/video/saa7134/Makefile	2006-12-07 15:04:58.000000000 +0100
> @@ -15,6 +15,3 @@
>  EXTRA_CFLAGS += -Idrivers/media/dvb/dvb-core
>  EXTRA_CFLAGS += -Idrivers/media/dvb/frontends
>  
> -extra-cflags-$(CONFIG_VIDEO_BUF_DVB) += -DHAVE_VIDEO_BUF_DVB=1
> -
> -EXTRA_CFLAGS += $(extra-cflags-y) $(extra-cflags-m)

NACK.

HAVE_VIDEO_BUF_DVB is not "unused" ... This symbol is tested for in the
following locations:

cx88.h:34:#ifdef HAVE_VIDEO_BUF_DVB
cx88.h:327:#ifdef HAVE_VIDEO_BUF_DVB
cx88.h:503:#ifdef HAVE_VIDEO_BUF_DVB
cx88-i2c.c:157:#ifdef HAVE_VIDEO_BUF_DVB
saa7134.h:51:#ifdef HAVE_VIDEO_BUF_DVB
saa7134.h:569:#ifdef HAVE_VIDEO_BUF_DVB

...We need this in order to allow compilation of the cx88 / saa7134 modules
without DVB support. (analog only)

If you want to convert the HAVE_VIDEO_BUF_DVB to a kconfig #ifdef test
for CONFIG_VIDEO_BUF_DVB, that would be acceptable.

Regards,

Michael Krufky

Re: [v4l-dvb-maintainer] [2.6 patch] cx88/saa7134: remove unused -DHAVE_VIDEO_BUF_DVB

[Adrian Bunk]

On Thu, Dec 07, 2006 at 10:36:01AM -0500, Michael Krufky wrote:
> Adrian Bunk wrote:
> > This patch removes the unused HAVE_VIDEO_BUF_DVB define.
> > 
> > Signed-off-by: Adrian Bunk <bunk@stusta.de>
> > 
> > --- linux-2.6.19-rc6-mm2/drivers/media/video/cx88/Makefile.old	2006-12-07 15:04:11.000000000 +0100
> > +++ linux-2.6.19-rc6-mm2/drivers/media/video/cx88/Makefile	2006-12-07 15:05:04.000000000 +0100
> > @@ -13,7 +13,6 @@
> >  EXTRA_CFLAGS += -Idrivers/media/dvb/dvb-core
> >  EXTRA_CFLAGS += -Idrivers/media/dvb/frontends
> >  
> > -extra-cflags-$(CONFIG_VIDEO_BUF_DVB) += -DHAVE_VIDEO_BUF_DVB=1
> >  extra-cflags-$(CONFIG_VIDEO_CX88_VP3054)+= -DHAVE_VP3054_I2C=1
> >  
> >  EXTRA_CFLAGS += $(extra-cflags-y) $(extra-cflags-m)
> > --- linux-2.6.19-rc6-mm2/drivers/media/video/saa7134/Makefile.old	2006-12-07 15:04:45.000000000 +0100
> > +++ linux-2.6.19-rc6-mm2/drivers/media/video/saa7134/Makefile	2006-12-07 15:04:58.000000000 +0100
> > @@ -15,6 +15,3 @@
> >  EXTRA_CFLAGS += -Idrivers/media/dvb/dvb-core
> >  EXTRA_CFLAGS += -Idrivers/media/dvb/frontends
> >  
> > -extra-cflags-$(CONFIG_VIDEO_BUF_DVB) += -DHAVE_VIDEO_BUF_DVB=1
> > -
> > -EXTRA_CFLAGS += $(extra-cflags-y) $(extra-cflags-m)
> 
> NACK.
> 
> HAVE_VIDEO_BUF_DVB is not "unused" ... This symbol is tested for in the
> following locations:
> 
> cx88.h:34:#ifdef HAVE_VIDEO_BUF_DVB
> cx88.h:327:#ifdef HAVE_VIDEO_BUF_DVB
> cx88.h:503:#ifdef HAVE_VIDEO_BUF_DVB
> cx88-i2c.c:157:#ifdef HAVE_VIDEO_BUF_DVB
> saa7134.h:51:#ifdef HAVE_VIDEO_BUF_DVB
> saa7134.h:569:#ifdef HAVE_VIDEO_BUF_DVB
> 
> ...We need this in order to allow compilation of the cx88 / saa7134 modules
> without DVB support. (analog only)
>...
 
Ah, you added them in v4l-dvb last year.

But they are neither in Linus' tree nor in the v4l-dvb git tree that is 
in the latest -mm.

Compilation of cx88 and saa7134 without DVB works fine in these trees, 
so what's the story behind this?

> Regards,
> 
> Michael Krufky

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

Re: [v4l-dvb-maintainer] [2.6 patch] cx88/saa7134: remove unused -DHAVE_VIDEO_BUF_DVB

[Michael Krufky]

Adrian Bunk wrote:
> On Thu, Dec 07, 2006 at 10:36:01AM -0500, Michael Krufky wrote:
>> Adrian Bunk wrote:
>>> This patch removes the unused HAVE_VIDEO_BUF_DVB define.

[snip]

>> ...We need this in order to allow compilation of the cx88 / saa7134 modules
>> without DVB support. (analog only)
>  
> Ah, you added them in v4l-dvb last year.

You are correct:  Tue Oct 11 20:11:34 2005 +0000 (14 months ago)

http://linuxtv.org/hg/v4l-dvb?cmd=changeset;node=56cf49b544f0

> But they are neither in Linus' tree nor in the v4l-dvb git tree that is 
> in the latest -mm.

hmm... looks like some changesets never made it over to git from our hg tree.

> Compilation of cx88 and saa7134 without DVB works fine in these trees, 
> so what's the story behind this?

It's a bug -- looks like CONFIG_VIDEO_BUF_DVB is being enabled, regardless
of whether or not it is selected -- otherwise we'd get other compiler errors,
because both cx88 and saa7134 have dependencies on video-buf-dvb.

VIDEO_BUF_DVB is being build without it's dependency, DVB_CORE -- that is
the only reason why the build is still working, but it sounds unstable to me.

CONFIG_VIDEO_BUF_DVB is set inside drivers/media/Kconfig, with zero
dependencies... In fact, VIDEO_BUF_DVB "depends on DVB_CORE" , but
this is not being reflected in Kconfig.

Hmm... looks like a bit of a mess.

The story is much clearer now... Looks like we should in fact apply your
patch, Adrian, but we will also have to make the following additional
changes:

- add "depends on DVB_CORE" to the Kconfig entry for VIDEO_BUF_DVB
- convert the #ifdef tests in the hg repository for HAVE_VIDEO_BUF_DVB
	to look for the CONFIG_VIDEO_BUF_DVB instead
- generate a patch against Linus' tree that add's these #ifdefs to the
	cx88 and saa7134 drivers.

If you dont mind, I'd like to take care of this stuff myself.  I will prepare
these patches tomorrow, and I'll have them applied to both our v4l-dvb.hg
repository on linuxtv.org, and I'll also ask Mauro to merge them into his git
tree before his next pull request to Linus.

Thanks, Adrian, for pointing out this inconsistency.


Cheers,

Michael Krufky

Re: [v4l-dvb-maintainer] [2.6 patch] cx88/saa7134: remove unused -DHAVE_VIDEO_BUF_DVB

[Adrian Bunk]

On Thu, Dec 07, 2006 at 12:13:59PM -0500, Michael Krufky wrote:
> Adrian Bunk wrote:
> > On Thu, Dec 07, 2006 at 10:36:01AM -0500, Michael Krufky wrote:
> >> Adrian Bunk wrote:
> >>> This patch removes the unused HAVE_VIDEO_BUF_DVB define.
> 
> [snip]
> 
> >> ...We need this in order to allow compilation of the cx88 / saa7134 modules
> >> without DVB support. (analog only)
> >  
> > Ah, you added them in v4l-dvb last year.
> 
> You are correct:  Tue Oct 11 20:11:34 2005 +0000 (14 months ago)
> 
> http://linuxtv.org/hg/v4l-dvb?cmd=changeset;node=56cf49b544f0
> 
> > But they are neither in Linus' tree nor in the v4l-dvb git tree that is 
> > in the latest -mm.
> 
> hmm... looks like some changesets never made it over to git from our hg tree.
> 
> > Compilation of cx88 and saa7134 without DVB works fine in these trees, 
> > so what's the story behind this?
> 
> It's a bug -- looks like CONFIG_VIDEO_BUF_DVB is being enabled, regardless
> of whether or not it is selected -- otherwise we'd get other compiler errors,
> because both cx88 and saa7134 have dependencies on video-buf-dvb.


No, the configuration

  CONFIG_VIDEO_SAA7134=y
  CONFIG_VIDEO_SAA7134_DVB=n
  CONFIG_VIDEO_BUF_DVB=n

builds fine in 2.6.19.


Only the saa7134-dvb and cx88-dvb modules require video-buf-dvb, and the 
config options for them select VIDEO_BUF_DVB.


> VIDEO_BUF_DVB is being build without it's dependency, DVB_CORE -- that is
> the only reason why the build is still working, but it sounds unstable to me.


No, that's already handled correctly by both VIDEO_CX88_DVB and 
VIDEO_SAA7134_DVB depending on DVB_CORE.


> CONFIG_VIDEO_BUF_DVB is set inside drivers/media/Kconfig, with zero
> dependencies... In fact, VIDEO_BUF_DVB "depends on DVB_CORE" , but
> this is not being reflected in Kconfig.
> 
> Hmm... looks like a bit of a mess.
> 
> The story is much clearer now... Looks like we should in fact apply your
> patch, Adrian, but we will also have to make the following additional
> changes:
> 
> - add "depends on DVB_CORE" to the Kconfig entry for VIDEO_BUF_DVB


That would be a noop since VIDEO_BUF_DVB is a not user visible option 
that gets select'ed.


> - convert the #ifdef tests in the hg repository for HAVE_VIDEO_BUF_DVB
> 	to look for the CONFIG_VIDEO_BUF_DVB instead
> - generate a patch against Linus' tree that add's these #ifdefs to the
> 	cx88 and saa7134 drivers.


There is no problem these #ifdef's would solve in Linus' tree...


> If you dont mind, I'd like to take care of this stuff myself.  I will prepare
> these patches tomorrow, and I'll have them applied to both our v4l-dvb.hg
> repository on linuxtv.org, and I'll also ask Mauro to merge them into his git
> tree before his next pull request to Linus.
> 
> Thanks, Adrian, for pointing out this inconsistency.
> 
> Cheers,
> 
> Michael Krufky


cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

Re: [v4l-dvb-maintainer] [2.6 patch] cx88/saa7134: remove unused -DHAVE_VIDEO_BUF_DVB

[Mauro Carvalho Chehab]

Hi Mkrufky,

Em Qui, 2006-12-07 às 18:26 +0100, Adrian Bunk escreveu:

> No, the configuration
> 
>   CONFIG_VIDEO_SAA7134=y
>   CONFIG_VIDEO_SAA7134_DVB=n
>   CONFIG_VIDEO_BUF_DVB=n
> 
> builds fine in 2.6.19.

> > Thanks, Adrian, for pointing out this inconsistency.

The point here, seemed to be related to the old v4l-dvb building system
and some conflicts with /boot/config. Previously, if /boot/config have a
symbol (for example) CONFIG_VIDEO_BUF_DVB=Y, it would define this symbol
for cx88, saa7134, etc, but it won't compile the required module,
generating some mess. Our current building system were improved in a way
that it will work fine, undefining such symbols.

In other words, just replacing all HAVE_foo to the proper CONFIG_foo
should work fine.

Anyway, I think it is better if you can take a look on it and do some
tests, before cleaning those legacy defines. There's no rush for this to
kernel window, since it would be just a trivial cleanup patch.

Cheers, 
Mauro.