BUG FIX - v4l broken hybrid dvb inclusion
PATCH 1/1 BUG FIX - v4l broken hybrid dvb inclusion
There was a change made in 2.6.13-rc2-mm2 within both of the following patches:
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. The following patch corrects this problem and restores previous behavior. We will eventually have to remove these symbols alltogether when we find a better solution, but this will fix the bug until then.
Always include dvb frontend code for hybrid cx88 and saa7134 boards.
cx88/cx88-dvb.c | 5 +++++ saa7134/saa7134-dvb.c | 3 +++ 2 files changed, 8 insertions(+)
drivers/media/video/cx88/cx88-dvb.c +5 -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"
drivers/media/video/saa7134/saa7134-dvb.c +3 -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"
Quoted 10 lines · Michael Krufky
v4l-saa7134-hybrid-dvb.patch
v4l-cx88-update.patchThe specific change that caused this problem is:
- Let
Kconfigdecide 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?
drivers/media/video/saa7134/saa7134-dvb.c line 34
Quoted 13 lines
drivers/media/video/cx88/cx88-dvb.c +4
-0+#define CONFIG_DVB_MT352 1 +#define CONFIG_DVB_CX22702 1 +#define CONFIG_DVB_OR51132 1 +#define CONFIG_DVB_LGDT3302 1drivers/media/video/saa7134/saa7134-dvb.c +2
-0+#define CONFIG_DVB_MT352 1 +#define CONFIG_DVB_TDA1004X 1
Looks band-aidly.
Quoted 8 lines · Alexey Dobriyan
Quoted 5 lines · Michael Krufky
v4l-saa7134-hybrid-dvb.patch
v4l-cx88-update.patchThe specific change that caused this problem is:
Quoted 9 lines
Quoted 7 lines
- Let
Kconfigdecide 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.
Quoted 14 lines
Quoted 14 lines
drivers/media/video/cx88/cx88-dvb.c +4
-0+#define CONFIG_DVB_MT352 1 +#define CONFIG_DVB_CX22702 1 +#define CONFIG_DVB_OR51132 1 +#define CONFIG_DVB_LGDT3302 1drivers/media/video/saa7134/saa7134-dvb.c +0
-0
Quoted 8 lines
drivers/media/video/saa7134/saa7134-dvb.c line 34
Quoted 4 lines
drivers/media/video/saa7134/saa7134-dvb.c +2
-0+#define CONFIG_DVB_MT352 1 +#define CONFIG_DVB_TDA1004X 1Looks 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.
Signature · 1 line
Michael Krufky
Quoted 20 lines · Michael Krufky
Quoted 8 lines · Alexey Dobriyan
Quoted 5 lines · Michael Krufky
v4l-saa7134-hybrid-dvb.patch
v4l-cx88-update.patchThe specific change that caused this problem is:
Quoted 9 lines
Quoted 7 lines
- Let
Kconfigdecide 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-
Quoted 7 lines
Maybe symbols was the wrong terminology... What I meant was the
CONFIG_DVB_LGDT3302, etc flagsPrevious 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?
Quoted 20 lines
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.
Quoted 16 lines
Quoted 16 lines
drivers/media/video/cx88/cx88-dvb.c +4
-0+#define CONFIG_DVB_MT352 1 +#define CONFIG_DVB_CX22702 1 +#define CONFIG_DVB_OR51132 1 +#define CONFIG_DVB_LGDT3302 1drivers/media/video/saa7134/saa7134-dvb.c +2
-0+#define CONFIG_DVB_MT352 1 +#define CONFIG_DVB_TDA1004X 1
Quoted 12 lines · Alexey Dobriyan
Quoted 9 lines · Michael Krufky
Quoted 6 lines · Alexey Dobriyan
Quoted 4 lines · Michael Krufky
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.
Quoted 11 lines
Quoted 5 lines
What I meant was the
CONFIG_DVB_LGDT3302, etc flagsPrevious 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.het al. are included. What's your .config?Quoted 3 lines
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.
Signature · 1 line
Michael Krufky
Quoted 19 lines · Michael Krufky
diff -upr
linux-2.6.13-rc2-mm2.orig/drivers/media/video/cx88/cx88-dvb.clinux/drivers/media/video/cx88/cx88-dvb.cdrivers/media/video/cx88/cx88-dvb.c +5
-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.clinux/drivers/media/video/saa7134/saa7134-dvb.c
drivers/media/video/saa7134/saa7134-dvb.c line 38
Quoted 9 lines
drivers/media/video/saa7134/saa7134-dvb.c +3
-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