Re: [PATCH -rc2-mm2] BUG FIX - v4l broken hybrid dvb inclusion

2005-07-12 Thread Johannes Stezenbach
Michael Krufky wrote:
> diff -upr linux-2.6.13-rc2-mm2.orig/drivers/media/video/cx88/cx88-dvb.c 
> linux/drivers/media/video/cx88/cx88-dvb.c
> --- linux-2.6.13-rc2-mm2.orig/drivers/media/video/cx88/cx88-dvb.c 
> 2005-07-12 08:56:58.0 +
> +++ linux/drivers/media/video/cx88/cx88-dvb.c 2005-07-12 09:01:13.0 
> +
> @@ -30,6 +30,11 @@
>  #include 
>  #include 
>  
> +#define CONFIG_DVB_MT352 1
> +#define CONFIG_DVB_CX22702 1
> +#define CONFIG_DVB_OR51132 1
> +#define CONFIG_DVB_LGDT3302 1
> +
>  #include "cx88.h"
>  #include "dvb-pll.h"
>  
> diff -upr linux-2.6.13-rc2-mm2.orig/drivers/media/video/saa7134/saa7134-dvb.c 
> linux/drivers/media/video/saa7134/saa7134-dvb.c
> --- linux-2.6.13-rc2-mm2.orig/drivers/media/video/saa7134/saa7134-dvb.c   
> 2005-07-12 08:56:59.0 +
> +++ linux/drivers/media/video/saa7134/saa7134-dvb.c   2005-07-12 
> 09:01:55.0 +
> @@ -30,6 +30,9 @@
>  #include 
>  #include 
>  
> +#define CONFIG_DVB_MT352 1
> +#define CONFIG_DVB_TDA1004X 1
> +
>  #include "saa7134-reg.h"
>  #include "saa7134.h"
>  


A better fix would be to remove the #ifdefs which use this defines.
They are now pointless as Kconfig selects those DVB frontends. As
Gerd Knorr pointed out they were originally introduced as a
workaround because the DVB maintainer was too slow merging
sending patches which the saa7134-dvb.c and cx88-dvb.c
drivers depended on.

Johannes
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH -rc2-mm2] BUG FIX - v4l broken hybrid dvb inclusion

2005-07-12 Thread Michael Krufky

Alexey Dobriyan wrote:


On Tuesday 12 July 2005 21:19, Michael Krufky wrote:
 


Alexey Dobriyan wrote:
   


On Tuesday 12 July 2005 19:06, Michael Krufky wrote:


I had tested this change against 2.6.13-rc2-mm1, and it worked perfectly as
expected, but it caused problems in today's 2.6.13-rc2-mm2 release.  For
some reason, the symbols don't get set properly.


What I meant was the CONFIG_DVB_LGDT3302 , etc flags

Previous patch removed the #define's that you see below... This should 
have worked, since these should be set instead from kconfig, but it 
didn't work as expected (even though the modules ARE selected by 
kconfig),
   


Strange... I did allyesconfig and preprocessed source shows lgdt3302.h,
or51132.h et al. are included. What's your .config?

and the #ifdef's return false (I don't know why it worked  
in my test against 2.6.13-rc2-mm1, but it doesn't work in -mm2, and it 
must be fixed) Breaks all hybrid v4l/dvb boards.
   

Everything does get built, just as you say... However, there is code 
inside cx88-dvb.c and saa7134-dvb.c that is enclosed within #ifdef's 
...  This code is NOT included during the compile.  For some reason the 
#ifdef's are turning up as false during compile time... In -mm1 this 
didn't happen.  For now, I am just setting these to true at the top of 
the *-dvb.c files... In the future, we (v4l) will either provide a 
better solution, or just remove the #define AND #ifdef's alltogether...  
I am including an excerp from cx88-dvb.c to illustrate what I am talking 
about:


#if CONFIG_DVB_MT352
# include "mt352.h"
# include "mt352_priv.h"
#endif
#if CONFIG_DVB_CX22702
# include "cx22702.h"
#endif
#if CONFIG_DVB_OR51132
# include "or51132.h"
#endif
#if CONFIG_DVB_LGDT3302
# include "lgdt3302.h"
#endif



#if CONFIG_DVB_MT352
static int dvico_fusionhdtv_demod_init(struct dvb_frontend* fe)
{
static u8 clock_config []  = { CLOCK_CTL,  0x38, 0x39 };
static u8 reset [] = { RESET,  0x80 };
static u8 adc_ctl_1_cfg [] = { ADC_CTL_1,  0x40 };
static u8 agc_cfg []   = { AGC_TARGET, 0x24, 0x20 };



static struct mt352_config dntv_live_dvbt_config = {
.demod_address = 0x0f,
.demod_init= dntv_live_dvbt_demod_init,
.pll_set   = mt352_pll_set,
};
#endif



This should be enough to explain what I am talking about... Of course this is 
just a small snippet...
There is more in cx88-dvb.c that does this, as well as saa7134-dvb.c...

In -mm2, the code inside the #if is NOT compiled... This is a problem, and this 
is why I sumbitted the patch.

--
Michael Krufky


-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH -rc2-mm2] BUG FIX - v4l broken hybrid dvb inclusion

2005-07-12 Thread Alexey Dobriyan
On Tuesday 12 July 2005 21:19, Michael Krufky wrote:
> Alexey Dobriyan wrote:
> >On Tuesday 12 July 2005 19:06, Michael Krufky wrote:
> >  
> >
> >>v4l-saa7134-hybrid-dvb.patch
> >>v4l-cx88-update.patch
> >>
> >>The specific change that caused this problem is:
> >>
> >>- Let Kconfig decide whether to include frontend-specific code.
> >>
> >>I had tested this change against 2.6.13-rc2-mm1, and it worked perfectly as
> >>expected, but it caused problems in today's 2.6.13-rc2-mm2 release.  For
> >>some reason, the symbols don't get set properly.
> >>
> >>
> >What symbols? What error messages do you see?
> >
> Alexey-
> 
> Maybe symbols was the wrong terminology... What I meant was the 
> CONFIG_DVB_LGDT3302 , etc flags
> 
> Previous patch removed the #define's that you see below... This should 
> have worked, since these should be set instead from kconfig, but it 
> didn't work as expected (even though the modules ARE selected by 
> kconfig),

Strange... I did allyesconfig and preprocessed source shows lgdt3302.h,
or51132.h et al. are included. What's your .config?

> and the #ifdef's return false (I don't know why it worked  
> in my test against 2.6.13-rc2-mm1, but it doesn't work in -mm2, and it 
> must be fixed) Breaks all hybrid v4l/dvb boards.
> 
> >>--- linux-2.6.13-rc2-mm2.orig/drivers/media/video/cx88/cx88-dvb.c
> >>+++ linux/drivers/media/video/cx88/cx88-dvb.c
> >>
> >>
> >>+#define CONFIG_DVB_MT352 1
> >>+#define CONFIG_DVB_CX22702 1
> >>+#define CONFIG_DVB_OR51132 1
> >>+#define CONFIG_DVB_LGDT3302 1
> >>
> >>
> >>--- linux-2.6.13-rc2-mm2.orig/drivers/media/video/saa7134/saa7134-dvb.c
> >>+++ linux/drivers/media/video/saa7134/saa7134-dvb.c
> >>
> >>
> >>+#define CONFIG_DVB_MT352 1
> >>+#define CONFIG_DVB_TDA1004X 1
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH -rc2-mm2] BUG FIX - v4l broken hybrid dvb inclusion

2005-07-12 Thread Michael Krufky

Alexey Dobriyan wrote:


On Tuesday 12 July 2005 19:06, Michael Krufky wrote:
 


v4l-saa7134-hybrid-dvb.patch
v4l-cx88-update.patch

The specific change that caused this problem is:

- Let Kconfig decide whether to include frontend-specific code.

I had tested this change against 2.6.13-rc2-mm1, and it worked perfectly as
expected, but it caused problems in today's 2.6.13-rc2-mm2 release.  For
some reason, the symbols don't get set properly.
   


What symbols? What error messages do you see?


Alexey-

Maybe symbols was the wrong terminology... What I meant was the 
CONFIG_DVB_LGDT3302 , etc flags


Previous patch removed the #define's that you see below... This should 
have worked, since these should be set instead from kconfig, but it 
didn't work as expected (even though the modules ARE selected by 
kconfig), and the #ifdef's return false (I don't know why it worked 
in my test against 2.6.13-rc2-mm1, but it doesn't work in -mm2, and it 
must be fixed) Breaks all hybrid v4l/dvb boards.



--- linux-2.6.13-rc2-mm2.orig/drivers/media/video/cx88/cx88-dvb.c
+++ linux/drivers/media/video/cx88/cx88-dvb.c
   


+#define CONFIG_DVB_MT352 1
+#define CONFIG_DVB_CX22702 1
+#define CONFIG_DVB_OR51132 1
+#define CONFIG_DVB_LGDT3302 1
   


--- linux-2.6.13-rc2-mm2.orig/drivers/media/video/saa7134/saa7134-dvb.c
+++ linux/drivers/media/video/saa7134/saa7134-dvb.c
   


+#define CONFIG_DVB_MT352 1
+#define CONFIG_DVB_TDA1004X 1
   



Looks band-aidly.
 

Yes, it does LOOK like a band-aid, but this is actually only reverting a 
previous change.  I admit that something better needs to be done.  Gerd 
Korr says to remove the #ifdef's alltogether.  Instead, I just returned 
the #define's .  We can remove the #ifdef's in a future patch, but I 
want to discuss this with the other v4l developers before I would make a 
change like that.  THIS patch, however, is safe to apply, and I've 
already committed it into video4linux cvs.


Andrew, please apply this and don't merge the 
[v4l-saa7134-hybrid-dvb.patch & v4l-cx88-update.patch] patches to Linus' 
tree without this patch as well.


Thank you.

--
Michael Krufky


-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH -rc2-mm2] BUG FIX - v4l broken hybrid dvb inclusion

2005-07-12 Thread Alexey Dobriyan
On Tuesday 12 July 2005 19:06, Michael Krufky wrote:
> v4l-saa7134-hybrid-dvb.patch
> v4l-cx88-update.patch
> 
> The specific change that caused this problem is:
> 
> - Let Kconfig decide whether to include frontend-specific code.
> 
> I had tested this change against 2.6.13-rc2-mm1, and it worked perfectly as
> expected, but it caused problems in today's 2.6.13-rc2-mm2 release.  For
> some reason, the symbols don't get set properly.

What symbols? What error messages do you see?

> --- linux-2.6.13-rc2-mm2.orig/drivers/media/video/cx88/cx88-dvb.c
> +++ linux/drivers/media/video/cx88/cx88-dvb.c

> +#define CONFIG_DVB_MT352 1
> +#define CONFIG_DVB_CX22702 1
> +#define CONFIG_DVB_OR51132 1
> +#define CONFIG_DVB_LGDT3302 1

> --- linux-2.6.13-rc2-mm2.orig/drivers/media/video/saa7134/saa7134-dvb.c
> +++ linux/drivers/media/video/saa7134/saa7134-dvb.c

> +#define CONFIG_DVB_MT352 1
> +#define CONFIG_DVB_TDA1004X 1

Looks band-aidly.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH -rc2-mm2] BUG FIX - v4l broken hybrid dvb inclusion

2005-07-12 Thread Johannes Stezenbach
Michael Krufky wrote:
 diff -upr linux-2.6.13-rc2-mm2.orig/drivers/media/video/cx88/cx88-dvb.c 
 linux/drivers/media/video/cx88/cx88-dvb.c
 --- linux-2.6.13-rc2-mm2.orig/drivers/media/video/cx88/cx88-dvb.c 
 2005-07-12 08:56:58.0 +
 +++ linux/drivers/media/video/cx88/cx88-dvb.c 2005-07-12 09:01:13.0 
 +
 @@ -30,6 +30,11 @@
  #include linux/file.h
  #include linux/suspend.h
  
 +#define CONFIG_DVB_MT352 1
 +#define CONFIG_DVB_CX22702 1
 +#define CONFIG_DVB_OR51132 1
 +#define CONFIG_DVB_LGDT3302 1
 +
  #include cx88.h
  #include dvb-pll.h
  
 diff -upr linux-2.6.13-rc2-mm2.orig/drivers/media/video/saa7134/saa7134-dvb.c 
 linux/drivers/media/video/saa7134/saa7134-dvb.c
 --- linux-2.6.13-rc2-mm2.orig/drivers/media/video/saa7134/saa7134-dvb.c   
 2005-07-12 08:56:59.0 +
 +++ linux/drivers/media/video/saa7134/saa7134-dvb.c   2005-07-12 
 09:01:55.0 +
 @@ -30,6 +30,9 @@
  #include linux/kthread.h
  #include linux/suspend.h
  
 +#define CONFIG_DVB_MT352 1
 +#define CONFIG_DVB_TDA1004X 1
 +
  #include saa7134-reg.h
  #include saa7134.h
  


A better fix would be to remove the #ifdefs which use this defines.
They are now pointless as Kconfig selects those DVB frontends. As
Gerd Knorr pointed out they were originally introduced as a
workaround because the DVB maintainer was too slow merging
sending patches which the saa7134-dvb.c and cx88-dvb.c
drivers depended on.

Johannes
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH -rc2-mm2] BUG FIX - v4l broken hybrid dvb inclusion

2005-07-12 Thread Alexey Dobriyan
On Tuesday 12 July 2005 19:06, Michael Krufky wrote:
 v4l-saa7134-hybrid-dvb.patch
 v4l-cx88-update.patch
 
 The specific change that caused this problem is:
 
 - Let Kconfig decide whether to include frontend-specific code.
 
 I had tested this change against 2.6.13-rc2-mm1, and it worked perfectly as
 expected, but it caused problems in today's 2.6.13-rc2-mm2 release.  For
 some reason, the symbols don't get set properly.

What symbols? What error messages do you see?

 --- linux-2.6.13-rc2-mm2.orig/drivers/media/video/cx88/cx88-dvb.c
 +++ linux/drivers/media/video/cx88/cx88-dvb.c

 +#define CONFIG_DVB_MT352 1
 +#define CONFIG_DVB_CX22702 1
 +#define CONFIG_DVB_OR51132 1
 +#define CONFIG_DVB_LGDT3302 1

 --- linux-2.6.13-rc2-mm2.orig/drivers/media/video/saa7134/saa7134-dvb.c
 +++ linux/drivers/media/video/saa7134/saa7134-dvb.c

 +#define CONFIG_DVB_MT352 1
 +#define CONFIG_DVB_TDA1004X 1

Looks band-aidly.
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH -rc2-mm2] BUG FIX - v4l broken hybrid dvb inclusion

2005-07-12 Thread Michael Krufky

Alexey Dobriyan wrote:


On Tuesday 12 July 2005 19:06, Michael Krufky wrote:
 


v4l-saa7134-hybrid-dvb.patch
v4l-cx88-update.patch

The specific change that caused this problem is:

- Let Kconfig decide whether to include frontend-specific code.

I had tested this change against 2.6.13-rc2-mm1, and it worked perfectly as
expected, but it caused problems in today's 2.6.13-rc2-mm2 release.  For
some reason, the symbols don't get set properly.
   


What symbols? What error messages do you see?


Alexey-

Maybe symbols was the wrong terminology... What I meant was the 
CONFIG_DVB_LGDT3302 , etc flags


Previous patch removed the #define's that you see below... This should 
have worked, since these should be set instead from kconfig, but it 
didn't work as expected (even though the modules ARE selected by 
kconfig), and the #ifdef's return false (I don't know why it worked 
in my test against 2.6.13-rc2-mm1, but it doesn't work in -mm2, and it 
must be fixed) Breaks all hybrid v4l/dvb boards.



--- linux-2.6.13-rc2-mm2.orig/drivers/media/video/cx88/cx88-dvb.c
+++ linux/drivers/media/video/cx88/cx88-dvb.c
   


+#define CONFIG_DVB_MT352 1
+#define CONFIG_DVB_CX22702 1
+#define CONFIG_DVB_OR51132 1
+#define CONFIG_DVB_LGDT3302 1
   


--- linux-2.6.13-rc2-mm2.orig/drivers/media/video/saa7134/saa7134-dvb.c
+++ linux/drivers/media/video/saa7134/saa7134-dvb.c
   


+#define CONFIG_DVB_MT352 1
+#define CONFIG_DVB_TDA1004X 1
   



Looks band-aidly.
 

Yes, it does LOOK like a band-aid, but this is actually only reverting a 
previous change.  I admit that something better needs to be done.  Gerd 
Korr says to remove the #ifdef's alltogether.  Instead, I just returned 
the #define's .  We can remove the #ifdef's in a future patch, but I 
want to discuss this with the other v4l developers before I would make a 
change like that.  THIS patch, however, is safe to apply, and I've 
already committed it into video4linux cvs.


Andrew, please apply this and don't merge the 
[v4l-saa7134-hybrid-dvb.patch  v4l-cx88-update.patch] patches to Linus' 
tree without this patch as well.


Thank you.

--
Michael Krufky


-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH -rc2-mm2] BUG FIX - v4l broken hybrid dvb inclusion

2005-07-12 Thread Alexey Dobriyan
On Tuesday 12 July 2005 21:19, Michael Krufky wrote:
 Alexey Dobriyan wrote:
 On Tuesday 12 July 2005 19:06, Michael Krufky wrote:
   
 
 v4l-saa7134-hybrid-dvb.patch
 v4l-cx88-update.patch
 
 The specific change that caused this problem is:
 
 - Let Kconfig decide whether to include frontend-specific code.
 
 I had tested this change against 2.6.13-rc2-mm1, and it worked perfectly as
 expected, but it caused problems in today's 2.6.13-rc2-mm2 release.  For
 some reason, the symbols don't get set properly.
 
 
 What symbols? What error messages do you see?
 
 Alexey-
 
 Maybe symbols was the wrong terminology... What I meant was the 
 CONFIG_DVB_LGDT3302 , etc flags
 
 Previous patch removed the #define's that you see below... This should 
 have worked, since these should be set instead from kconfig, but it 
 didn't work as expected (even though the modules ARE selected by 
 kconfig),

Strange... I did allyesconfig and preprocessed source shows lgdt3302.h,
or51132.h et al. are included. What's your .config?

 and the #ifdef's return false (I don't know why it worked  
 in my test against 2.6.13-rc2-mm1, but it doesn't work in -mm2, and it 
 must be fixed) Breaks all hybrid v4l/dvb boards.
 
 --- linux-2.6.13-rc2-mm2.orig/drivers/media/video/cx88/cx88-dvb.c
 +++ linux/drivers/media/video/cx88/cx88-dvb.c
 
 
 +#define CONFIG_DVB_MT352 1
 +#define CONFIG_DVB_CX22702 1
 +#define CONFIG_DVB_OR51132 1
 +#define CONFIG_DVB_LGDT3302 1
 
 
 --- linux-2.6.13-rc2-mm2.orig/drivers/media/video/saa7134/saa7134-dvb.c
 +++ linux/drivers/media/video/saa7134/saa7134-dvb.c
 
 
 +#define CONFIG_DVB_MT352 1
 +#define CONFIG_DVB_TDA1004X 1
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH -rc2-mm2] BUG FIX - v4l broken hybrid dvb inclusion

2005-07-12 Thread Michael Krufky

Alexey Dobriyan wrote:


On Tuesday 12 July 2005 21:19, Michael Krufky wrote:
 


Alexey Dobriyan wrote:
   


On Tuesday 12 July 2005 19:06, Michael Krufky wrote:


I had tested this change against 2.6.13-rc2-mm1, and it worked perfectly as
expected, but it caused problems in today's 2.6.13-rc2-mm2 release.  For
some reason, the symbols don't get set properly.


What I meant was the CONFIG_DVB_LGDT3302 , etc flags

Previous patch removed the #define's that you see below... This should 
have worked, since these should be set instead from kconfig, but it 
didn't work as expected (even though the modules ARE selected by 
kconfig),
   


Strange... I did allyesconfig and preprocessed source shows lgdt3302.h,
or51132.h et al. are included. What's your .config?

and the #ifdef's return false (I don't know why it worked  
in my test against 2.6.13-rc2-mm1, but it doesn't work in -mm2, and it 
must be fixed) Breaks all hybrid v4l/dvb boards.
   

Everything does get built, just as you say... However, there is code 
inside cx88-dvb.c and saa7134-dvb.c that is enclosed within #ifdef's 
...  This code is NOT included during the compile.  For some reason the 
#ifdef's are turning up as false during compile time... In -mm1 this 
didn't happen.  For now, I am just setting these to true at the top of 
the *-dvb.c files... In the future, we (v4l) will either provide a 
better solution, or just remove the #define AND #ifdef's alltogether...  
I am including an excerp from cx88-dvb.c to illustrate what I am talking 
about:


#if CONFIG_DVB_MT352
# include mt352.h
# include mt352_priv.h
#endif
#if CONFIG_DVB_CX22702
# include cx22702.h
#endif
#if CONFIG_DVB_OR51132
# include or51132.h
#endif
#if CONFIG_DVB_LGDT3302
# include lgdt3302.h
#endif

snip

#if CONFIG_DVB_MT352
static int dvico_fusionhdtv_demod_init(struct dvb_frontend* fe)
{
static u8 clock_config []  = { CLOCK_CTL,  0x38, 0x39 };
static u8 reset [] = { RESET,  0x80 };
static u8 adc_ctl_1_cfg [] = { ADC_CTL_1,  0x40 };
static u8 agc_cfg []   = { AGC_TARGET, 0x24, 0x20 };

snip

static struct mt352_config dntv_live_dvbt_config = {
.demod_address = 0x0f,
.demod_init= dntv_live_dvbt_demod_init,
.pll_set   = mt352_pll_set,
};
#endif

snip

This should be enough to explain what I am talking about... Of course this is 
just a small snippet...
There is more in cx88-dvb.c that does this, as well as saa7134-dvb.c...

In -mm2, the code inside the #if is NOT compiled... This is a problem, and this 
is why I sumbitted the patch.

--
Michael Krufky


-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/