af9035 test needed!

DISCUSSION Antti Palosaari 7 participants 24 messages Jan 11, 2013 – Feb 15, 2013

Subsystems (3)

Media (V4L/DVB) DVB_USB_AF9035 media driver MXL5007T media driver

lore

Share & tools

lore

Antti Palosaari

author

2013-01-11 18:38 UTC #0 lore raw

Hello Jose and Gianluca

Could you test that (tda18218 & mxl5007t):
http://git.linuxtv.org/anttip/media_tree.git/shortlog/refs/heads/it9135_tuner

I wonder if ADC config logic still works for superheterodyne tuners (tuner having IF). I changed it to adc / 2 always due to IT9135 tuner. That makes me wonder it possible breaks tuners having IF, as ADC was clocked just over 20MHz and if it is half then it is 10MHz. For BB that is enough, but I think that having IF, which is 4MHz at least for 8MHz BW it is too less.

F*ck I hate to maintain driver without a hardware! Any idea where I can get AF9035 device having tda18218 or mxl5007t?

regards
Antti

Signature · 1 line

http://palosaari.fi/

Jose Alberto Reguero

2013-01-11 23:45 UTC #1 lore raw

Quoted 13 lines · On Viernes, 11 de enero de 2013 20:38:01 Antti Palosaari escribió:

Hello Jose and Gianluca
Could you test that (tda18218 & mxl5007t):
http://git.linuxtv.org/anttip/media_tree.git/shortlog/refs/heads/it9135_tune
r
I wonder if ADC config logic still works for superheterodyne tuners (tuner having IF). I changed it to adc / 2 always due to IT9135 tuner. That makes me wonder it possible breaks tuners having IF, as ADC was clocked just over 20MHz and if it is half then it is 10MHz. For BB that is enough, but I think that having IF, which is 4MHz at least for 8MHz BW it is too less.

Quoted 5 lines

F*ck I hate to maintain driver without a hardware! Any idea where I can get AF9035 device having tda18218 or mxl5007t?
regards
Antti

Still pending the changes for mxl5007t. Attached is a patch for that.

Changes to make work Avermedia Twinstar with the af9035 driver.

Signed-off-by Jose Alberto Reguero <[email protected]>

Jose Alberto

diff -upr linux/drivers/media/tuners/mxl5007t.c linux.new/drivers/media/tuners/mxl5007t.c
--- linux/drivers/media/tuners/mxl5007t.c 2012-08-14 05:45:22.000000000 +0200
+++ linux.new/drivers/media/tuners/mxl5007t.c 2013-01-10 19:23:09.247556275
+0100
@@ -374,7 +374,6 @@ static struct reg_pair_t *mxl5007t_calc_
mxl5007t_set_if_freq_bits(state, cfg->if_freq_hz, cfg->invert_if); mxl5007t_set_xtal_freq_bits(state, cfg->xtal_freq_hz);

- set_reg_bits(state->tab_init, 0x04, 0x01, cfg->loop_thru_enable);
set_reg_bits(state->tab_init, 0x03, 0x08, cfg->clk_out_enable << 3); set_reg_bits(state->tab_init, 0x03, 0x07, cfg->clk_out_amp);

@@ -531,9 +530,12 @@ static int mxl5007t_tuner_init(struct mx
struct reg_pair_t *init_regs;
int ret;

- ret = mxl5007t_soft_reset(state);
- if (mxl_fail(ret))
+ if (!state->config->no_reset) {
+ ret = mxl5007t_soft_reset(state);
+ if (mxl_fail(ret))
goto fail;
+ }
+

/* calculate initialization reg array */ init_regs = mxl5007t_calc_init_regs(state, mode);
@@ -887,7 +889,12 @@ struct dvb_frontend *mxl5007t_attach(str
if (fe->ops.i2c_gate_ctrl)
fe->ops.i2c_gate_ctrl(fe, 1);

- ret = mxl5007t_get_chip_id(state);
+ if (!state->config->no_probe)
+ ret = mxl5007t_get_chip_id(state);
+
+ ret = mxl5007t_write_reg(state, 0x04,
+ state->config->loop_thru_enable);
+

if (fe->ops.i2c_gate_ctrl)
fe->ops.i2c_gate_ctrl(fe, 0);
diff -upr linux/drivers/media/tuners/mxl5007t.h linux.new/drivers/media/tuners/mxl5007t.h
--- linux/drivers/media/tuners/mxl5007t.h 2012-08-14 05:45:22.000000000 +0200
+++ linux.new/drivers/media/tuners/mxl5007t.h 2013-01-10 19:19:11.204379581
+0100
@@ -73,8 +73,10 @@ struct mxl5007t_config {
enum mxl5007t_xtal_freq xtal_freq_hz; enum mxl5007t_if_freq if_freq_hz; unsigned int invert_if:1;
- unsigned int loop_thru_enable:1;
+ unsigned int loop_thru_enable:3;
unsigned int clk_out_enable:1;
+ unsigned int no_probe:1;
+ unsigned int no_reset:1;
};

#if defined(CONFIG_MEDIA_TUNER_MXL5007T) ||
(defined(CONFIG_MEDIA_TUNER_MXL5007T_MODULE) && defined(MODULE)) diff -upr linux/drivers/media/usb/dvb-usb-v2/af9035.c linux.new/drivers/media/usb/dvb-usb-v2/af9035.c
--- linux/drivers/media/usb/dvb-usb-v2/af9035.c 2013-01-07 05:45:57.000000000
+0100
+++ linux.new/drivers/media/usb/dvb-usb-v2/af9035.c 2013-01-12
00:30:57.557310465 +0100
@@ -886,13 +886,17 @@ static struct mxl5007t_config af9035_mxl
.loop_thru_enable = 0,
.clk_out_enable = 0,
.clk_out_amp = MxL_CLKOUT_AMP_0_94V,
+ .no_probe = 1,
+ .no_reset = 1,
}, {
.xtal_freq_hz = MxL_XTAL_24_MHZ, .if_freq_hz = MxL_IF_4_57_MHZ, .invert_if = 0,
- .loop_thru_enable = 1,
+ .loop_thru_enable = 3,
.clk_out_enable = 1,
.clk_out_amp = MxL_CLKOUT_AMP_0_94V,
+ .no_probe = 1,
+ .no_reset = 1,
}
};

Antti Palosaari

author

2013-01-11 23:49 UTC #2 lore raw

Quoted 17 lines · Jose Alberto Reguero

Quoted 16 lines · On Viernes, 11 de enero de 2013 20:38:01 Antti Palosaari escribió:
Hello Jose and Gianluca
Could you test that (tda18218 & mxl5007t):
http://git.linuxtv.org/anttip/media_tree.git/shortlog/refs/heads/it9135_tune
r
I wonder if ADC config logic still works for superheterodyne tuners (tuner having IF). I changed it to adc / 2 always due to IT9135 tuner. That makes me wonder it possible breaks tuners having IF, as ADC was clocked just over 20MHz and if it is half then it is 10MHz. For BB that is enough, but I think that having IF, which is 4MHz at least for 8MHz BW it is too less.
F*ck I hate to maintain driver without a hardware! Any idea where I can get AF9035 device having tda18218 or mxl5007t?

Quoted 8 lines

Quoted 2 lines
regards
Antti
Still pending the changes for mxl5007t. Attached is a patch for that.
Changes to make work Avermedia Twinstar with the af9035 driver.
Signed-off-by Jose Alberto Reguero <[email protected]>

I cannot do much about this as it changes mxl5007t driver which is not maintained by me. :)

regards
Antti

Quoted 91 lines

Jose Alberto

diff -upr linux/drivers/media/tuners/mxl5007t.c
linux.new/drivers/media/tuners/mxl5007t.c

drivers/media/tuners/mxl5007t.c +12 -4

+0100

@@ -374,7 +374,6 @@ static struct reg_pair_t *mxl5007t_calc_
  	mxl5007t_set_if_freq_bits(state, cfg->if_freq_hz, cfg->invert_if);
  	mxl5007t_set_xtal_freq_bits(state, cfg->xtal_freq_hz);
 
-	set_reg_bits(state->tab_init, 0x04, 0x01, cfg->loop_thru_enable);
  	set_reg_bits(state->tab_init, 0x03, 0x08, cfg->clk_out_enable << 3);
  	set_reg_bits(state->tab_init, 0x03, 0x07, cfg->clk_out_amp);

@@ -531,9 +530,12 @@ static int mxl5007t_tuner_init(struct mx
  	struct reg_pair_t *init_regs;
  	int ret;
 
-	ret = mxl5007t_soft_reset(state);
-	if (mxl_fail(ret))
+	if (!state->config->no_reset) {
+		ret = mxl5007t_soft_reset(state);
+		if (mxl_fail(ret))
  		goto fail;
+	}
+
 
  	/* calculate initialization reg array */
  	init_regs = mxl5007t_calc_init_regs(state, mode);

@@ -887,7 +889,12 @@ struct dvb_frontend *mxl5007t_attach(str
  		if (fe->ops.i2c_gate_ctrl)
  			fe->ops.i2c_gate_ctrl(fe, 1);
 
-		ret = mxl5007t_get_chip_id(state);
+		if (!state->config->no_probe)
+			ret = mxl5007t_get_chip_id(state);
+
+		ret = mxl5007t_write_reg(state, 0x04,
+			state->config->loop_thru_enable);
+
 
  		if (fe->ops.i2c_gate_ctrl)
  			fe->ops.i2c_gate_ctrl(fe, 0);

diff -upr linux/drivers/media/tuners/mxl5007t.h
linux.new/drivers/media/tuners/mxl5007t.h

drivers/media/tuners/mxl5007t.h +4 -1

+0100

@@ -73,8 +73,10 @@ struct mxl5007t_config {
  	enum mxl5007t_xtal_freq xtal_freq_hz;
  	enum mxl5007t_if_freq if_freq_hz;
  	unsigned int invert_if:1;
-	unsigned int loop_thru_enable:1;
+	unsigned int loop_thru_enable:3;
  	unsigned int clk_out_enable:1;
+	unsigned int no_probe:1;
+	unsigned int no_reset:1;
  };
 
  #if defined(CONFIG_MEDIA_TUNER_MXL5007T) ||

(defined(CONFIG_MEDIA_TUNER_MXL5007T_MODULE) && defined(MODULE)) diff -upr linux/drivers/media/usb/dvb-usb-v2/af9035.c linux.new/drivers/media/usb/dvb-usb-v2/af9035.c
--- linux/drivers/media/usb/dvb-usb-v2/af9035.c 2013-01-07 05:45:57.000000000
+0100
+++ linux.new/drivers/media/usb/dvb-usb-v2/af9035.c 2013-01-12
00:30:57.557310465 +0100

drivers/media/tuners/mxl5007t.h +5 -1

@@ -886,13 +886,17 @@ static struct mxl5007t_config af9035_mxl
  		.loop_thru_enable = 0,
  		.clk_out_enable = 0,
  		.clk_out_amp = MxL_CLKOUT_AMP_0_94V,
+		.no_probe = 1,
+		.no_reset = 1,
  	}, {
  		.xtal_freq_hz = MxL_XTAL_24_MHZ,
  		.if_freq_hz = MxL_IF_4_57_MHZ,
  		.invert_if = 0,
-		.loop_thru_enable = 1,
+		.loop_thru_enable = 3,
  		.clk_out_enable = 1,
  		.clk_out_amp = MxL_CLKOUT_AMP_0_94V,
+		.no_probe = 1,
+		.no_reset = 1,
  	}
  };

Signature · 1 line

http://palosaari.fi/

Jose Alberto Reguero

2013-01-12 21:14 UTC #3 lore raw

Quoted 25 lines · On Sábado, 12 de enero de 2013 01:49:21 Antti Palosaari escribió:

Quoted 24 lines · Jose Alberto Reguero
Quoted 13 lines · On Viernes, 11 de enero de 2013 20:38:01 Antti Palosaari escribió:
Hello Jose and Gianluca
Could you test that (tda18218 & mxl5007t):
http://git.linuxtv.org/anttip/media_tree.git/shortlog/refs/heads/it9135_t
une r
I wonder if ADC config logic still works for superheterodyne tuners (tuner having IF). I changed it to adc / 2 always due to IT9135 tuner. That makes me wonder it possible breaks tuners having IF, as ADC was clocked just over 20MHz and if it is half then it is 10MHz. For BB that is enough, but I think that having IF, which is 4MHz at least for 8MHz BW it is too less.
Quoted 5 lines
F*ck I hate to maintain driver without a hardware! Any idea where I can get AF9035 device having tda18218 or mxl5007t?
regards
Antti
Still pending the changes for mxl5007t. Attached is a patch for that.
Changes to make work Avermedia Twinstar with the af9035 driver.

Quoted 8 lines

Quoted 1 line
Signed-off-by Jose Alberto Reguero <[email protected]>
I cannot do much about this as it changes mxl5007t driver which is not maintained by me. :)
regards
Antti

Adding CC to Michael Krufky because it is the maintainer of mxl5007t driver. Michael, any chance to get this patch merged?

Jose Alberto

Quoted 111 lines

Jose Alberto

(quoted diff) +0 -1

@@ -374,7 +374,6 @@ static struct reg_pair_t *mxl5007t_calc_
 
  	mxl5007t_set_if_freq_bits(state, cfg->if_freq_hz, cfg->invert_if);
  	mxl5007t_set_xtal_freq_bits(state, cfg->xtal_freq_hz);
 
-	set_reg_bits(state->tab_init, 0x04, 0x01, cfg->loop_thru_enable);
 
  	set_reg_bits(state->tab_init, 0x03, 0x08, cfg->clk_out_enable << 3);

  	set_reg_bits(state->tab_init, 0x03, 0x07, cfg->clk_out_amp);

(quoted diff) +5 -2

@@ -531,9 +530,12 @@ static int mxl5007t_tuner_init(struct mx
 
  	struct reg_pair_t *init_regs;
  	int ret;
 
-	ret = mxl5007t_soft_reset(state);
-	if (mxl_fail(ret))
+	if (!state->config->no_reset) {
+		ret = mxl5007t_soft_reset(state);
+		if (mxl_fail(ret))
 
  		goto fail;
 
+	}
+

  	/* calculate initialization reg array */
  	init_regs = mxl5007t_calc_init_regs(state, mode);

(quoted diff) +6 -1

@@ -887,7 +889,12 @@ struct dvb_frontend *mxl5007t_attach(str
 
  		if (fe->ops.i2c_gate_ctrl)
 
  			fe->ops.i2c_gate_ctrl(fe, 1);
 
-		ret = mxl5007t_get_chip_id(state);
+		if (!state->config->no_probe)
+			ret = mxl5007t_get_chip_id(state);
+
+		ret = mxl5007t_write_reg(state, 0x04,
+			state->config->loop_thru_enable);
+

if (fe->ops.i2c_gate_ctrl)

  			fe->ops.i2c_gate_ctrl(fe, 0);

diff -upr linux/drivers/media/tuners/mxl5007t.h
linux.new/drivers/media/tuners/mxl5007t.h
--- linux/drivers/media/tuners/mxl5007t.h 2012-08-14 05:45:22.000000000
+0200 +++ linux.new/drivers/media/tuners/mxl5007t.h 2013-01-10
19:19:11.204379581 +0100

(quoted diff) +3 -1

@@ -73,8 +73,10 @@ struct mxl5007t_config {
 
  	enum mxl5007t_xtal_freq xtal_freq_hz;
  	enum mxl5007t_if_freq if_freq_hz;
  	unsigned int invert_if:1;
 
-	unsigned int loop_thru_enable:1;
+	unsigned int loop_thru_enable:3;
 
  	unsigned int clk_out_enable:1;
 
+	unsigned int no_probe:1;
+	unsigned int no_reset:1;
 
  };
 
  #if defined(CONFIG_MEDIA_TUNER_MXL5007T) ||

(defined(CONFIG_MEDIA_TUNER_MXL5007T_MODULE) && defined(MODULE)) diff -upr linux/drivers/media/usb/dvb-usb-v2/af9035.c linux.new/drivers/media/usb/dvb-usb-v2/af9035.c
--- linux/drivers/media/usb/dvb-usb-v2/af9035.c 2013-01-07
05:45:57.000000000 +0100
+++ linux.new/drivers/media/usb/dvb-usb-v2/af9035.c 2013-01-12
00:30:57.557310465 +0100

(quoted diff) +5 -1

@@ -886,13 +886,17 @@ static struct mxl5007t_config af9035_mxl
 
  		.loop_thru_enable = 0,
  		.clk_out_enable = 0,
  		.clk_out_amp = MxL_CLKOUT_AMP_0_94V,
 
+		.no_probe = 1,
+		.no_reset = 1,
 
  	}, {
 
  		.xtal_freq_hz = MxL_XTAL_24_MHZ,
  		.if_freq_hz = MxL_IF_4_57_MHZ,
  		.invert_if = 0,
 
-		.loop_thru_enable = 1,
+		.loop_thru_enable = 3,
 
  		.clk_out_enable = 1,
  		.clk_out_amp = MxL_CLKOUT_AMP_0_94V,
 
+		.no_probe = 1,
+		.no_reset = 1,
 
  	}
 
  };

Jose Alberto Reguero

2013-01-13 20:19 UTC #4 lore raw

Quoted 145 lines · On Sábado, 12 de enero de 2013 22:14:07 Jose Alberto Reguero escribió:

Quoted 25 lines · On Sábado, 12 de enero de 2013 01:49:21 Antti Palosaari escribió:
Quoted 24 lines · Jose Alberto Reguero
Quoted 13 lines · On Viernes, 11 de enero de 2013 20:38:01 Antti Palosaari escribió:
Hello Jose and Gianluca
Could you test that (tda18218 & mxl5007t):
http://git.linuxtv.org/anttip/media_tree.git/shortlog/refs/heads/it9135_t
une r
I wonder if ADC config logic still works for superheterodyne tuners (tuner having IF). I changed it to adc / 2 always due to IT9135 tuner. That makes me wonder it possible breaks tuners having IF, as ADC was clocked just over 20MHz and if it is half then it is 10MHz. For BB that is enough, but I think that having IF, which is 4MHz at least for 8MHz BW it is too less.
Quoted 5 lines
F*ck I hate to maintain driver without a hardware! Any idea where I can get AF9035 device having tda18218 or mxl5007t?
regards
Antti
Still pending the changes for mxl5007t. Attached is a patch for that.
Changes to make work Avermedia Twinstar with the af9035 driver.
Quoted 8 lines
Quoted 1 line
Signed-off-by Jose Alberto Reguero <[email protected]>
I cannot do much about this as it changes mxl5007t driver which is not maintained by me. :)
regards
Antti
Adding CC to Michael Krufky because it is the maintainer of mxl5007t driver. Michael, any chance to get this patch merged?
Jose Alberto
Quoted 105 lines
Quoted 105 lines
Jose Alberto
diff -upr linux/drivers/media/tuners/mxl5007t.c
linux.new/drivers/media/tuners/mxl5007t.c
--- linux/drivers/media/tuners/mxl5007t.c 2012-08-14 05:45:22.000000000
+0200 +++ linux.new/drivers/media/tuners/mxl5007t.c 2013-01-10
19:23:09.247556275 +0100
(quoted diff) +0 -1
@@ -374,7 +374,6 @@ static struct reg_pair_t *mxl5007t_calc_
 
  	mxl5007t_set_if_freq_bits(state, cfg->if_freq_hz, cfg->invert_if);
  	mxl5007t_set_xtal_freq_bits(state, cfg->xtal_freq_hz);
 
-	set_reg_bits(state->tab_init, 0x04, 0x01, cfg->loop_thru_enable);
 
  	set_reg_bits(state->tab_init, 0x03, 0x08, cfg->clk_out_enable << 3);
  	set_reg_bits(state->tab_init, 0x03, 0x07, cfg->clk_out_amp);
(quoted diff) +5 -2
@@ -531,9 +530,12 @@ static int mxl5007t_tuner_init(struct mx
 
  	struct reg_pair_t *init_regs;
  	int ret;
 
-	ret = mxl5007t_soft_reset(state);
-	if (mxl_fail(ret))
+	if (!state->config->no_reset) {
+		ret = mxl5007t_soft_reset(state);
+		if (mxl_fail(ret))
 
  		goto fail;
 
+	}
+
  	/* calculate initialization reg array */
  	init_regs = mxl5007t_calc_init_regs(state, mode);
(quoted diff) +6 -1
@@ -887,7 +889,12 @@ struct dvb_frontend *mxl5007t_attach(str
 
  		if (fe->ops.i2c_gate_ctrl)
 
  			fe->ops.i2c_gate_ctrl(fe, 1);
 
-		ret = mxl5007t_get_chip_id(state);
+		if (!state->config->no_probe)
+			ret = mxl5007t_get_chip_id(state);
+
+		ret = mxl5007t_write_reg(state, 0x04,
+			state->config->loop_thru_enable);
+
 
if (fe->ops.i2c_gate_ctrl)
  			fe->ops.i2c_gate_ctrl(fe, 0);
diff -upr linux/drivers/media/tuners/mxl5007t.h
linux.new/drivers/media/tuners/mxl5007t.h
--- linux/drivers/media/tuners/mxl5007t.h 2012-08-14 05:45:22.000000000
+0200 +++ linux.new/drivers/media/tuners/mxl5007t.h 2013-01-10
19:19:11.204379581 +0100
(quoted diff) +3 -1
@@ -73,8 +73,10 @@ struct mxl5007t_config {
 
  	enum mxl5007t_xtal_freq xtal_freq_hz;
  	enum mxl5007t_if_freq if_freq_hz;
  	unsigned int invert_if:1;
 
-	unsigned int loop_thru_enable:1;
+	unsigned int loop_thru_enable:3;
 
  	unsigned int clk_out_enable:1;
 
+	unsigned int no_probe:1;
+	unsigned int no_reset:1;
 
  };
 
  #if defined(CONFIG_MEDIA_TUNER_MXL5007T) ||
(defined(CONFIG_MEDIA_TUNER_MXL5007T_MODULE) && defined(MODULE)) diff -upr linux/drivers/media/usb/dvb-usb-v2/af9035.c linux.new/drivers/media/usb/dvb-usb-v2/af9035.c
--- linux/drivers/media/usb/dvb-usb-v2/af9035.c 2013-01-07
05:45:57.000000000 +0100
+++ linux.new/drivers/media/usb/dvb-usb-v2/af9035.c 2013-01-12
00:30:57.557310465 +0100
(quoted diff) +3 -1
@@ -886,13 +886,17 @@ static struct mxl5007t_config af9035_mxl
 
  		.loop_thru_enable = 0,
  		.clk_out_enable = 0,
  		.clk_out_amp = MxL_CLKOUT_AMP_0_94V,
 
+		.no_probe = 1,
+		.no_reset = 1,
 
  	}, {
 
  		.xtal_freq_hz = MxL_XTAL_24_MHZ,
  		.if_freq_hz = MxL_IF_4_57_MHZ,
  		.invert_if = 0,
 
-		.loop_thru_enable = 1,
+		.loop_thru_enable = 3,
 
  		.clk_out_enable = 1,
  		.clk_out_amp = MxL_CLKOUT_AMP_0_94V,

Quoted 7 lines

Quoted 6 lines
Review
Quoted 6 lines
(quoted diff) +2 -0
+		.no_probe = 1,
+		.no_reset = 1,
 
  	}
 
  };
--

Sending again because some lines are mangled.

Changes to make work Avermedia Twinstar with the af9035 driver.

Signed-off-by Jose Alberto Reguero <[email protected]>

drivers/media/tuners/mxl5007t.c +11 -4

diff -upr linux/drivers/media/tuners/mxl5007t.c linux.new/drivers/media/tuners/mxl5007t.c
@@ -374,7 +374,6 @@ static struct reg_pair_t *mxl5007t_calc_
 	mxl5007t_set_if_freq_bits(state, cfg->if_freq_hz, cfg->invert_if);
 	mxl5007t_set_xtal_freq_bits(state, cfg->xtal_freq_hz);
 
-	set_reg_bits(state->tab_init, 0x04, 0x01, cfg->loop_thru_enable);
 	set_reg_bits(state->tab_init, 0x03, 0x08, cfg->clk_out_enable << 3);
 	set_reg_bits(state->tab_init, 0x03, 0x07, cfg->clk_out_amp);

@@ -531,9 +530,12 @@ static int mxl5007t_tuner_init(struct mx
 	struct reg_pair_t *init_regs;
 	int ret;
 
-	ret = mxl5007t_soft_reset(state);
-	if (mxl_fail(ret))
+	if (!state->config->no_reset) {
+		ret = mxl5007t_soft_reset(state);
+		if (mxl_fail(ret))
 		goto fail;
+	}
+
 
 	/* calculate initialization reg array */
 	init_regs = mxl5007t_calc_init_regs(state, mode);

@@ -887,7 +889,12 @@ struct dvb_frontend *mxl5007t_attach(str
 		if (fe->ops.i2c_gate_ctrl)
 			fe->ops.i2c_gate_ctrl(fe, 1);
 
-		ret = mxl5007t_get_chip_id(state);
+		if (!state->config->no_probe)
+			ret = mxl5007t_get_chip_id(state);
+
+		ret = mxl5007t_write_reg(state, 0x04,
+			state->config->loop_thru_enable);
+
 
 		if (fe->ops.i2c_gate_ctrl)
 			fe->ops.i2c_gate_ctrl(fe, 0);

drivers/media/tuners/mxl5007t.h +3 -1

diff -upr linux/drivers/media/tuners/mxl5007t.h linux.new/drivers/media/tuners/mxl5007t.h
@@ -73,8 +73,10 @@ struct mxl5007t_config {
 	enum mxl5007t_xtal_freq xtal_freq_hz;
 	enum mxl5007t_if_freq if_freq_hz;
 	unsigned int invert_if:1;
-	unsigned int loop_thru_enable:1;
+	unsigned int loop_thru_enable:3;
 	unsigned int clk_out_enable:1;
+	unsigned int no_probe:1;
+	unsigned int no_reset:1;
 };
 
 #if defined(CONFIG_MEDIA_TUNER_MXL5007T) || (defined(CONFIG_MEDIA_TUNER_MXL5007T_MODULE) && defined(MODULE))

drivers/media/usb/dvb-usb-v2/af9035.c +5 -1

diff -upr linux/drivers/media/usb/dvb-usb-v2/af9035.c linux.new/drivers/media/usb/dvb-usb-v2/af9035.c
@@ -886,13 +886,17 @@ static struct mxl5007t_config af9035_mxl
 		.loop_thru_enable = 0,
 		.clk_out_enable = 0,
 		.clk_out_amp = MxL_CLKOUT_AMP_0_94V,
+		.no_probe = 1,
+		.no_reset = 1,
 	}, {
 		.xtal_freq_hz = MxL_XTAL_24_MHZ,
 		.if_freq_hz = MxL_IF_4_57_MHZ,
 		.invert_if = 0,
-		.loop_thru_enable = 1,
+		.loop_thru_enable = 3,
 		.clk_out_enable = 1,
 		.clk_out_amp = MxL_CLKOUT_AMP_0_94V,
+		.no_probe = 1,
+		.no_reset = 1,
 	}
 };

Michael Krufky

maintainer

2013-02-03 04:00 UTC #5 lore raw

In reply to Jose Alberto Reguero

Quoted 34 lines · Jose Alberto Reguero

Quoted 13 lines · On Viernes, 11 de enero de 2013 20:38:01 Antti Palosaari escribió:
Hello Jose and Gianluca
Could you test that (tda18218 & mxl5007t):
http://git.linuxtv.org/anttip/media_tree.git/shortlog/refs/heads/it9135_tune
r
I wonder if ADC config logic still works for superheterodyne tuners (tuner having IF). I changed it to adc / 2 always due to IT9135 tuner. That makes me wonder it possible breaks tuners having IF, as ADC was clocked just over 20MHz and if it is half then it is 10MHz. For BB that is enough, but I think that having IF, which is 4MHz at least for 8MHz BW it is too less.
Quoted 5 lines
F*ck I hate to maintain driver without a hardware! Any idea where I can get AF9035 device having tda18218 or mxl5007t?
regards
Antti
Still pending the changes for mxl5007t. Attached is a patch for that.
Changes to make work Avermedia Twinstar with the af9035 driver.
Signed-off-by Jose Alberto Reguero <[email protected]>
Jose Alberto
diff -upr linux/drivers/media/tuners/mxl5007t.c
linux.new/drivers/media/tuners/mxl5007t.c
drivers/media/tuners/mxl5007t.c 2013-01-10 19:23:09.247556275 renamed from drivers/media/tuners/mxl5007t.c 2012-08-14 05:45:22.000000000 +0200 +1 -0
+0100

drivers/media/tuners/mxl5007t.c 2013-01-10 19:23:09.247556275 static struct reg_pair_t *mxl5007t_calc_ · line 379

Quoted 7 lines

drivers/media/tuners/mxl5007t.c 2013-01-10 19:23:09.247556275 renamed from drivers/media/tuners/mxl5007t.c 2012-08-14 05:45:22.000000000 +0200 +0 -1

@@ -374,7 +374,6 @@ static struct reg_pair_t *mxl5007t_calc_
        mxl5007t_set_if_freq_bits(state, cfg->if_freq_hz, cfg->invert_if);
        mxl5007t_set_xtal_freq_bits(state, cfg->xtal_freq_hz);
 
-       set_reg_bits(state->tab_init, 0x04, 0x01, cfg->loop_thru_enable);
        set_reg_bits(state->tab_init, 0x03, 0x08, cfg->clk_out_enable << 3);
        set_reg_bits(state->tab_init, 0x03, 0x07, cfg->clk_out_amp);

This is a configurable option - it should not be removed, just configure your glue code to not use that option if you dont want it.... unless there's some other reason why you're removing this?

drivers/media/tuners/mxl5007t.c 2013-01-10 19:23:09.247556275 static int mxl5007t_tuner_init(struct mx · line 538

Quoted 12 lines

drivers/media/tuners/mxl5007t.c 2013-01-10 19:23:09.247556275 renamed from drivers/media/tuners/mxl5007t.c 2012-08-14 05:45:22.000000000 +0200 +5 -2

@@ -531,9 +530,12 @@ static int mxl5007t_tuner_init(struct mx
        struct reg_pair_t *init_regs;
        int ret;
 
-       ret = mxl5007t_soft_reset(state);
-       if (mxl_fail(ret))
+       if (!state->config->no_reset) {
+               ret = mxl5007t_soft_reset(state);
+               if (mxl_fail(ret))
                goto fail;
+       }
+

this seems wrong to me. why would you want to prevent the driver from doing a soft reset?

Quoted 5 lines

drivers/media/tuners/mxl5007t.c 2013-01-10 19:23:09.247556275 renamed from drivers/media/tuners/mxl5007t.c 2012-08-14 05:45:22.000000000 +0200 +0 -0
        /* calculate initialization reg array */
        init_regs = mxl5007t_calc_init_regs(state, mode);
@@ -887,7 +889,12 @@ struct dvb_frontend *mxl5007t_attach(str
                if (fe->ops.i2c_gate_ctrl)

drivers/media/tuners/mxl5007t.c 2013-01-10 19:23:09.247556275 struct dvb_frontend *mxl5007t_attach(str · line 898

Quoted 10 lines

drivers/media/tuners/mxl5007t.c 2013-01-10 19:23:09.247556275 renamed from drivers/media/tuners/mxl5007t.c 2012-08-14 05:45:22.000000000 +0200 +6 -1

                        fe->ops.i2c_gate_ctrl(fe, 1);
 
-               ret = mxl5007t_get_chip_id(state);
+               if (!state->config->no_probe)
+                       ret = mxl5007t_get_chip_id(state);
+
+               ret = mxl5007t_write_reg(state, 0x04,
+                       state->config->loop_thru_enable);
+

Can you explain why this change was made? ^^

drivers/media/tuners/mxl5007t.h 2013-01-10 19:19:11.204379581 struct mxl5007t_config { · line 76

Quoted 13 lines

drivers/media/tuners/mxl5007t.c 2013-01-10 19:23:09.247556275 renamed from drivers/media/tuners/mxl5007t.c 2012-08-14 05:45:22.000000000 +0200 +0 -0
                if (fe->ops.i2c_gate_ctrl)
                        fe->ops.i2c_gate_ctrl(fe, 0);
diff -upr linux/drivers/media/tuners/mxl5007t.h
linux.new/drivers/media/tuners/mxl5007t.h
drivers/media/tuners/mxl5007t.h 2013-01-10 19:19:11.204379581 renamed from drivers/media/tuners/mxl5007t.h 2012-08-14 05:45:22.000000000 +0200 +2 -1
+0100
@@ -73,8 +73,10 @@ struct mxl5007t_config {
        enum mxl5007t_xtal_freq xtal_freq_hz;
        enum mxl5007t_if_freq if_freq_hz;
        unsigned int invert_if:1;
-       unsigned int loop_thru_enable:1;
+       unsigned int loop_thru_enable:3;

Why widen this boolean to three bits?

Quoted 22 lines

drivers/media/tuners/mxl5007t.h 2013-01-10 19:19:11.204379581 renamed from drivers/media/tuners/mxl5007t.h 2012-08-14 05:45:22.000000000 +0200 +2 -0
        unsigned int clk_out_enable:1;
+       unsigned int no_probe:1;
+       unsigned int no_reset:1;
 };
 
 #if defined(CONFIG_MEDIA_TUNER_MXL5007T) ||
(defined(CONFIG_MEDIA_TUNER_MXL5007T_MODULE) && defined(MODULE)) diff -upr linux/drivers/media/usb/dvb-usb-v2/af9035.c linux.new/drivers/media/usb/dvb-usb-v2/af9035.c
--- linux/drivers/media/usb/dvb-usb-v2/af9035.c 2013-01-07 05:45:57.000000000 +0100
+++ linux.new/drivers/media/usb/dvb-usb-v2/af9035.c 2013-01-12
00:30:57.557310465 +0100
drivers/media/tuners/mxl5007t.h 2013-01-10 19:19:11.204379581 renamed from drivers/media/tuners/mxl5007t.h 2012-08-14 05:45:22.000000000 +0200 +2 -0
@@ -886,13 +886,17 @@ static struct mxl5007t_config af9035_mxl
                .loop_thru_enable = 0,
                .clk_out_enable = 0,
                .clk_out_amp = MxL_CLKOUT_AMP_0_94V,
+               .no_probe = 1,
+               .no_reset = 1,
        }, {
                .xtal_freq_hz = MxL_XTAL_24_MHZ,
                .if_freq_hz = MxL_IF_4_57_MHZ,

drivers/media/tuners/mxl5007t.h 2013-01-10 19:19:11.204379581 static struct mxl5007t_config af9035_mxl · line 902

Quoted 10 lines

drivers/media/tuners/mxl5007t.h 2013-01-10 19:19:11.204379581 renamed from drivers/media/tuners/mxl5007t.h 2012-08-14 05:45:22.000000000 +0200 +3 -1

                .invert_if = 0,
-               .loop_thru_enable = 1,
+               .loop_thru_enable = 3,
                .clk_out_enable = 1,
                .clk_out_amp = MxL_CLKOUT_AMP_0_94V,
+               .no_probe = 1,
+               .no_reset = 1,
        }
 };

This patch cannot be merged as-is. I'm sorry. If you could explain why each change was made, then perhaps I would be able to advise better how to make this work on your device without breaking others.

-Mike Krufky

Jose Alberto Reguero

2013-02-03 12:04 UTC #6 lore raw

Quoted 43 lines · On Sábado, 2 de febrero de 2013 23:00:45 Michael Krufky escribió:

On Fri, Jan 11, 2013 at 6:45 PM, Jose Alberto Reguero
Quoted 40 lines · <[email protected]> wrote:
Quoted 13 lines · On Viernes, 11 de enero de 2013 20:38:01 Antti Palosaari escribió:
Hello Jose and Gianluca
Could you test that (tda18218 & mxl5007t):
http://git.linuxtv.org/anttip/media_tree.git/shortlog/refs/heads/it9135_t
une r
I wonder if ADC config logic still works for superheterodyne tuners (tuner having IF). I changed it to adc / 2 always due to IT9135 tuner. That makes me wonder it possible breaks tuners having IF, as ADC was clocked just over 20MHz and if it is half then it is 10MHz. For BB that is enough, but I think that having IF, which is 4MHz at least for 8MHz BW it is too less.
Quoted 5 lines
F*ck I hate to maintain driver without a hardware! Any idea where I can get AF9035 device having tda18218 or mxl5007t?
regards
Antti
Still pending the changes for mxl5007t. Attached is a patch for that.
Changes to make work Avermedia Twinstar with the af9035 driver.
Signed-off-by Jose Alberto Reguero <[email protected]>
Jose Alberto
diff -upr linux/drivers/media/tuners/mxl5007t.c linux.new/drivers/media/tuners/mxl5007t.c
--- linux/drivers/media/tuners/mxl5007t.c 2012-08-14
05:45:22.000000000 +0200 +++ linux.new/drivers/media/tuners/mxl5007t.c
2013-01-10 19:23:09.247556275 +0100
(quoted diff) +0 -1
@@ -374,7 +374,6 @@ static struct reg_pair_t *mxl5007t_calc_
 
        mxl5007t_set_if_freq_bits(state, cfg->if_freq_hz, cfg->invert_if);
        mxl5007t_set_xtal_freq_bits(state, cfg->xtal_freq_hz);
 
-       set_reg_bits(state->tab_init, 0x04, 0x01, cfg->loop_thru_enable);
 

Quoted 8 lines

Quoted 3 lines
(quoted diff) +0 -0
        set_reg_bits(state->tab_init, 0x03, 0x08, cfg->clk_out_enable <<
        3);
        set_reg_bits(state->tab_init, 0x03, 0x07, cfg->clk_out_amp);
This is a configurable option - it should not be removed, just configure your glue code to not use that option if you dont want it.... unless there's some other reason why you're removing this?

I just move the code to a mxl5007t_attach because with dual tuner until the code is executed, the other tuner don't work. It can be left here also.

Quoted 11 lines

(quoted diff) +3 -2

@@ -531,9 +530,12 @@ static int mxl5007t_tuner_init(struct mx
 
        struct reg_pair_t *init_regs;
        int ret;
 
-       ret = mxl5007t_soft_reset(state);
-       if (mxl_fail(ret))
+       if (!state->config->no_reset) {
+               ret = mxl5007t_soft_reset(state);
+               if (mxl_fail(ret))

Quoted 8 lines

Review static int mxl5007t_tuner_init(struct mx · line 541
Quoted 4 lines
(quoted diff) +2 -0
                goto fail;
 
+       }
+
this seems wrong to me. why would you want to prevent the driver from doing a soft reset?

That is because with my hardware and dual tuner, when one tuner do reset, the other one is perturbed, and the stream has errors.

Quoted 9 lines

        /* calculate initialization reg array */
        init_regs = mxl5007t_calc_init_regs(state, mode);

(quoted diff) +0 -0

@@ -887,7 +889,12 @@ struct dvb_frontend *mxl5007t_attach(str
 
                if (fe->ops.i2c_gate_ctrl)
 
                        fe->ops.i2c_gate_ctrl(fe, 1);

Quoted 10 lines

Review struct dvb_frontend *mxl5007t_attach(str · line 899
Quoted 7 lines
(quoted diff) +6 -1
-               ret = mxl5007t_get_chip_id(state);
+               if (!state->config->no_probe)
+                       ret = mxl5007t_get_chip_id(state);
+
+               ret = mxl5007t_write_reg(state, 0x04,
+                       state->config->loop_thru_enable);
+
Can you explain why this change was made? ^^

mxl5007t_get_chip_id has a read, and with the hardware I have, after the read operation is made, communication with the chip don't work.

Quoted 11 lines

Quoted 11 lines
(quoted diff) +0 -0
                if (fe->ops.i2c_gate_ctrl)
                
                        fe->ops.i2c_gate_ctrl(fe, 0);
diff -upr linux/drivers/media/tuners/mxl5007t.h linux.new/drivers/media/tuners/mxl5007t.h
--- linux/drivers/media/tuners/mxl5007t.h 2012-08-14
05:45:22.000000000 +0200 +++ linux.new/drivers/media/tuners/mxl5007t.h
2013-01-10 19:19:11.204379581 +0100
(quoted diff) +0 -0
@@ -73,8 +73,10 @@ struct mxl5007t_config {
 

Quoted 9 lines

Review struct mxl5007t_config { · line 78

Quoted 6 lines

(quoted diff) +1 -1

        enum mxl5007t_xtal_freq xtal_freq_hz;
        enum mxl5007t_if_freq if_freq_hz;
        unsigned int invert_if:1;
 
-       unsigned int loop_thru_enable:1;
+       unsigned int loop_thru_enable:3;

Why widen this boolean to three bits?

I just use the value 3 for this option(taken from windows driver) and it works well.

Thanks for review the code.

Jose Alberto

Quoted 49 lines

Quoted 33 lines
(quoted diff) +2 -0
        unsigned int clk_out_enable:1;
 
+       unsigned int no_probe:1;
+       unsigned int no_reset:1;
};
#if defined(CONFIG_MEDIA_TUNER_MXL5007T) ||
(defined(CONFIG_MEDIA_TUNER_MXL5007T_MODULE) && defined(MODULE)) diff -upr linux/drivers/media/usb/dvb-usb-v2/af9035.c linux.new/drivers/media/usb/dvb-usb-v2/af9035.c
--- linux/drivers/media/usb/dvb-usb-v2/af9035.c 2013-01-07 05:45:57.000000000 +0100
+++ linux.new/drivers/media/usb/dvb-usb-v2/af9035.c 2013-01-12
00:30:57.557310465 +0100
(quoted diff) +3 -1
@@ -886,13 +886,17 @@ static struct mxl5007t_config af9035_mxl
 
                .loop_thru_enable = 0,
                .clk_out_enable = 0,
                .clk_out_amp = MxL_CLKOUT_AMP_0_94V,
 
+               .no_probe = 1,
+               .no_reset = 1,
 
        }, {
 
                .xtal_freq_hz = MxL_XTAL_24_MHZ,
                .if_freq_hz = MxL_IF_4_57_MHZ,
                .invert_if = 0,
 
-               .loop_thru_enable = 1,
+               .loop_thru_enable = 3,
Review static struct mxl5007t_config af9035_mxl
Quoted 10 lines
(quoted diff) +2 -0
 
                .clk_out_enable = 1,
                .clk_out_amp = MxL_CLKOUT_AMP_0_94V,
 
+               .no_probe = 1,
+               .no_reset = 1,
 
        }
 
 };
This patch cannot be merged as-is. I'm sorry. If you could explain why each change was made, then perhaps I would be able to advise better how to make this work on your device without breaking others.
-Mike Krufky

Antti Palosaari

author

2013-02-03 13:22 UTC #7 lore raw

Quoted 123 lines · Jose Alberto Reguero

Quoted 43 lines · On Sábado, 2 de febrero de 2013 23:00:45 Michael Krufky escribió:
On Fri, Jan 11, 2013 at 6:45 PM, Jose Alberto Reguero
Quoted 40 lines · <[email protected]> wrote:
Quoted 13 lines · On Viernes, 11 de enero de 2013 20:38:01 Antti Palosaari escribió:
Hello Jose and Gianluca
Could you test that (tda18218 & mxl5007t):
http://git.linuxtv.org/anttip/media_tree.git/shortlog/refs/heads/it9135_t
une r
I wonder if ADC config logic still works for superheterodyne tuners (tuner having IF). I changed it to adc / 2 always due to IT9135 tuner. That makes me wonder it possible breaks tuners having IF, as ADC was clocked just over 20MHz and if it is half then it is 10MHz. For BB that is enough, but I think that having IF, which is 4MHz at least for 8MHz BW it is too less.
Quoted 5 lines
F*ck I hate to maintain driver without a hardware! Any idea where I can get AF9035 device having tda18218 or mxl5007t?
regards
Antti
Still pending the changes for mxl5007t. Attached is a patch for that.
Changes to make work Avermedia Twinstar with the af9035 driver.
Signed-off-by Jose Alberto Reguero <[email protected]>
Jose Alberto
diff -upr linux/drivers/media/tuners/mxl5007t.c linux.new/drivers/media/tuners/mxl5007t.c
--- linux/drivers/media/tuners/mxl5007t.c 2012-08-14
05:45:22.000000000 +0200 +++ linux.new/drivers/media/tuners/mxl5007t.c
2013-01-10 19:23:09.247556275 +0100
(quoted diff) +0 -1
@@ -374,7 +374,6 @@ static struct reg_pair_t *mxl5007t_calc_
 
         mxl5007t_set_if_freq_bits(state, cfg->if_freq_hz, cfg->invert_if);
         mxl5007t_set_xtal_freq_bits(state, cfg->xtal_freq_hz);
 
-       set_reg_bits(state->tab_init, 0x04, 0x01, cfg->loop_thru_enable);
 
Quoted 8 lines
Quoted 3 lines
(quoted diff) +0 -0
         set_reg_bits(state->tab_init, 0x03, 0x08, cfg->clk_out_enable <<
         3);
         set_reg_bits(state->tab_init, 0x03, 0x07, cfg->clk_out_amp);
This is a configurable option - it should not be removed, just configure your glue code to not use that option if you dont want it.... unless there's some other reason why you're removing this?
I just move the code to a mxl5007t_attach because with dual tuner until the code is executed, the other tuner don't work. It can be left here also.
Quoted 11 lines
Quoted 11 lines
(quoted diff) +3 -2
@@ -531,9 +530,12 @@ static int mxl5007t_tuner_init(struct mx
 
         struct reg_pair_t *init_regs;
         int ret;
 
-       ret = mxl5007t_soft_reset(state);
-       if (mxl_fail(ret))
+       if (!state->config->no_reset) {
+               ret = mxl5007t_soft_reset(state);
+               if (mxl_fail(ret))
 
Quoted 8 lines
Review static int mxl5007t_tuner_init(struct mx · line 541
Quoted 4 lines
(quoted diff) +2 -0
                 goto fail;
 
+       }
+
this seems wrong to me. why would you want to prevent the driver from doing a soft reset?
That is because with my hardware and dual tuner, when one tuner do reset, the other one is perturbed, and the stream has errors.
Quoted 9 lines
Quoted 9 lines
         /* calculate initialization reg array */
         init_regs = mxl5007t_calc_init_regs(state, mode);
(quoted diff) +0 -0
@@ -887,7 +889,12 @@ struct dvb_frontend *mxl5007t_attach(str
 
                 if (fe->ops.i2c_gate_ctrl)
 
                         fe->ops.i2c_gate_ctrl(fe, 1);
 
Quoted 10 lines
Review struct dvb_frontend *mxl5007t_attach(str · line 899
Quoted 7 lines
(quoted diff) +6 -1
-               ret = mxl5007t_get_chip_id(state);
+               if (!state->config->no_probe)
+                       ret = mxl5007t_get_chip_id(state);
+
+               ret = mxl5007t_write_reg(state, 0x04,
+                       state->config->loop_thru_enable);
+
Can you explain why this change was made? ^^
mxl5007t_get_chip_id has a read, and with the hardware I have, after the read operation is made, communication with the chip don't work.
Quoted 11 lines
Quoted 11 lines
(quoted diff) +0 -0
                 if (fe->ops.i2c_gate_ctrl)
                         fe->ops.i2c_gate_ctrl(fe, 0);
diff -upr linux/drivers/media/tuners/mxl5007t.h linux.new/drivers/media/tuners/mxl5007t.h
--- linux/drivers/media/tuners/mxl5007t.h 2012-08-14
05:45:22.000000000 +0200 +++ linux.new/drivers/media/tuners/mxl5007t.h
2013-01-10 19:19:11.204379581 +0100
(quoted diff) +0 -0
@@ -73,8 +73,10 @@ struct mxl5007t_config {
 
Quoted 9 lines
Review struct mxl5007t_config { · line 78
Quoted 6 lines
(quoted diff) +1 -1
         enum mxl5007t_xtal_freq xtal_freq_hz;
         enum mxl5007t_if_freq if_freq_hz;
         unsigned int invert_if:1;
 
-       unsigned int loop_thru_enable:1;
+       unsigned int loop_thru_enable:3;
Why widen this boolean to three bits?

Quoted 57 lines

I just use the value 3 for this option(taken from windows driver) and it works well.
Thanks for review the code.
Jose Alberto
Quoted 49 lines
Quoted 33 lines
(quoted diff) +2 -0
         unsigned int clk_out_enable:1;
 
+       unsigned int no_probe:1;
+       unsigned int no_reset:1;
  };

  #if defined(CONFIG_MEDIA_TUNER_MXL5007T) ||
(defined(CONFIG_MEDIA_TUNER_MXL5007T_MODULE) && defined(MODULE)) diff -upr linux/drivers/media/usb/dvb-usb-v2/af9035.c linux.new/drivers/media/usb/dvb-usb-v2/af9035.c
--- linux/drivers/media/usb/dvb-usb-v2/af9035.c 2013-01-07 05:45:57.000000000 +0100
+++ linux.new/drivers/media/usb/dvb-usb-v2/af9035.c 2013-01-12
00:30:57.557310465 +0100
(quoted diff) +3 -1
@@ -886,13 +886,17 @@ static struct mxl5007t_config af9035_mxl
 
                 .loop_thru_enable = 0,
                 .clk_out_enable = 0,
                 .clk_out_amp = MxL_CLKOUT_AMP_0_94V,
 
+               .no_probe = 1,
+               .no_reset = 1,
 
         }, {
 
                 .xtal_freq_hz = MxL_XTAL_24_MHZ,
                 .if_freq_hz = MxL_IF_4_57_MHZ,
                 .invert_if = 0,
 
-               .loop_thru_enable = 1,
+               .loop_thru_enable = 3,
Review static struct mxl5007t_config af9035_mxl
Quoted 10 lines
(quoted diff) +2 -0
 
                 .clk_out_enable = 1,
                 .clk_out_amp = MxL_CLKOUT_AMP_0_94V,
 
+               .no_probe = 1,
+               .no_reset = 1,
 
         }
 
  };
This patch cannot be merged as-is. I'm sorry. If you could explain why each change was made, then perhaps I would be able to advise better how to make this work on your device without breaking others.
-Mike Krufky

Mike, Jose,
I think there is multiple bugs - both existing and new ones added by Jose.

First existing MxL5007t driver bugs, which are tried to resolve by Jose just wrongly.

You configure clock output and RF loop-through only after first tune and during every tune.

Control flow of mxl5007t init seems to be totally broken logically and cannot work if there is slave tuner used.

Relevant control flow bug is that:

mxl5007t_set_params() {
mxl5007t_tuner_init() {
mxl5007t_soft_reset()
-- configure clock output
-- configure RF loop-through
}
}

RF loop-through as well clock output should be configured on attach() that you could trust slave tuner is operational just after all is attached().

Also, there is another bug, which likely causes disturbance to picture Jose mentioned. MxL5007t soft reset resets all registers to default state. You do that *every* time when set_params() is called. Could you guess what happens to clock output and RF loop-through slave tuner is using?

I made month or two back patch series fixing similar problems of FC0012 driver. See example from there. Jose's hacks are not mostly relevant after that MxL5007t driver is fixed to offer clock and loop-through correctly to the slave tuner.

************************

Another issue is that register 04, which contains loop-though and some other bits. Jose changes its value from 1 to 3. Is that change really needed? What happens if you leave it just 1? You should not change options like that just for fun without need.

regards
Antti

Signature · 1 line

http://palosaari.fi/

Michael Krufky

maintainer

2013-02-03 19:27 UTC #8 lore raw

Quoted 238 lines · Antti Palosaari

Quoted 139 lines · Jose Alberto Reguero
Quoted 50 lines · On Sábado, 2 de febrero de 2013 23:00:45 Michael Krufky escribió:
On Fri, Jan 11, 2013 at 6:45 PM, Jose Alberto Reguero
Quoted 46 lines · <[email protected]> wrote:
Quoted 15 lines · On Viernes, 11 de enero de 2013 20:38:01 Antti Palosaari escribió:
Hello Jose and Gianluca
Could you test that (tda18218 & mxl5007t):
http://git.linuxtv.org/anttip/media_tree.git/shortlog/refs/heads/it9135_t
une r
I wonder if ADC config logic still works for superheterodyne tuners (tuner having IF). I changed it to adc / 2 always due to IT9135 tuner. That makes me wonder it possible breaks tuners having IF, as ADC was clocked just over 20MHz and if it is half then it is 10MHz. For BB that is enough, but I think that having IF, which is 4MHz at least for 8MHz BW it is too less.
Quoted 5 lines
F*ck I hate to maintain driver without a hardware! Any idea where I can get AF9035 device having tda18218 or mxl5007t?
regards
Antti
Still pending the changes for mxl5007t. Attached is a patch for that.
Changes to make work Avermedia Twinstar with the af9035 driver.
Signed-off-by Jose Alberto Reguero <[email protected]>
Jose Alberto
diff -upr linux/drivers/media/tuners/mxl5007t.c linux.new/drivers/media/tuners/mxl5007t.c
--- linux/drivers/media/tuners/mxl5007t.c 2012-08-14
05:45:22.000000000 +0200 +++ linux.new/drivers/media/tuners/mxl5007t.c
2013-01-10 19:23:09.247556275 +0100
(quoted diff) +0 -1
@@ -374,7 +374,6 @@ static struct reg_pair_t *mxl5007t_calc_
 
         mxl5007t_set_if_freq_bits(state, cfg->if_freq_hz,
 cfg->invert_if);
         mxl5007t_set_xtal_freq_bits(state, cfg->xtal_freq_hz);
 
-       set_reg_bits(state->tab_init, 0x04, 0x01,
 cfg->loop_thru_enable);
Quoted 10 lines
Quoted 4 lines
         set_reg_bits(state->tab_init, 0x03, 0x08, cfg->clk_out_enable
<<
         3);
         set_reg_bits(state->tab_init, 0x03, 0x07, cfg->clk_out_amp);
This is a configurable option - it should not be removed, just configure your glue code to not use that option if you dont want it.... unless there's some other reason why you're removing this?
I just move the code to a mxl5007t_attach because with dual tuner until the
code is executed, the other tuner don't work. It can be left here also.
Quoted 11 lines
Quoted 11 lines
(quoted diff) +3 -2
@@ -531,9 +530,12 @@ static int mxl5007t_tuner_init(struct mx
 
         struct reg_pair_t *init_regs;
         int ret;
 
-       ret = mxl5007t_soft_reset(state);
-       if (mxl_fail(ret))
+       if (!state->config->no_reset) {
+               ret = mxl5007t_soft_reset(state);
+               if (mxl_fail(ret))
 
Quoted 9 lines
Review static int mxl5007t_tuner_init(struct mx · line 541
Quoted 4 lines
(quoted diff) +2 -0
                 goto fail;
 
+       }
+
this seems wrong to me. why would you want to prevent the driver from doing a soft reset?
That is because with my hardware and dual tuner, when one tuner do reset, the
other one is perturbed, and the stream has errors.
Quoted 9 lines
Quoted 9 lines
         /* calculate initialization reg array */
         init_regs = mxl5007t_calc_init_regs(state, mode);
(quoted diff) +0 -0
@@ -887,7 +889,12 @@ struct dvb_frontend *mxl5007t_attach(str
 
                 if (fe->ops.i2c_gate_ctrl)
 
                         fe->ops.i2c_gate_ctrl(fe, 1);
 
Quoted 11 lines
Review struct dvb_frontend *mxl5007t_attach(str · line 899
Quoted 7 lines
(quoted diff) +6 -1
-               ret = mxl5007t_get_chip_id(state);
+               if (!state->config->no_probe)
+                       ret = mxl5007t_get_chip_id(state);
+
+               ret = mxl5007t_write_reg(state, 0x04,
+                       state->config->loop_thru_enable);
+
Can you explain why this change was made? ^^
mxl5007t_get_chip_id has a read, and with the hardware I have, after the read
operation is made, communication with the chip don't work.
Quoted 15 lines
Quoted 15 lines
(quoted diff) +0 -0
                 if (fe->ops.i2c_gate_ctrl)
                         fe->ops.i2c_gate_ctrl(fe, 0);
diff -upr linux/drivers/media/tuners/mxl5007t.h linux.new/drivers/media/tuners/mxl5007t.h
--- linux/drivers/media/tuners/mxl5007t.h 2012-08-14
05:45:22.000000000 +0200 +++ linux.new/drivers/media/tuners/mxl5007t.h
2013-01-10 19:19:11.204379581 +0100
(quoted diff) +0 -0
@@ -73,8 +73,10 @@ struct mxl5007t_config {
 
         enum mxl5007t_xtal_freq xtal_freq_hz;
         enum mxl5007t_if_freq if_freq_hz;
         unsigned int invert_if:1;
 
Quoted 6 lines
Review struct mxl5007t_config { · line 78
Quoted 2 lines
(quoted diff) +1 -1
-       unsigned int loop_thru_enable:1;
+       unsigned int loop_thru_enable:3;
Why widen this boolean to three bits?
Quoted 59 lines
I just use the value 3 for this option(taken from windows driver) and it works
well.
Thanks for review the code.
Jose Alberto
Quoted 50 lines
Quoted 33 lines
(quoted diff) +2 -0
         unsigned int clk_out_enable:1;
 
+       unsigned int no_probe:1;
+       unsigned int no_reset:1;
  };

  #if defined(CONFIG_MEDIA_TUNER_MXL5007T) ||
(defined(CONFIG_MEDIA_TUNER_MXL5007T_MODULE) && defined(MODULE)) diff -upr linux/drivers/media/usb/dvb-usb-v2/af9035.c linux.new/drivers/media/usb/dvb-usb-v2/af9035.c
--- linux/drivers/media/usb/dvb-usb-v2/af9035.c 2013-01-07 05:45:57.000000000 +0100
+++ linux.new/drivers/media/usb/dvb-usb-v2/af9035.c 2013-01-12
00:30:57.557310465 +0100
(quoted diff) +3 -1
@@ -886,13 +886,17 @@ static struct mxl5007t_config af9035_mxl
 
                 .loop_thru_enable = 0,
                 .clk_out_enable = 0,
                 .clk_out_amp = MxL_CLKOUT_AMP_0_94V,
 
+               .no_probe = 1,
+               .no_reset = 1,
 
         }, {
 
                 .xtal_freq_hz = MxL_XTAL_24_MHZ,
                 .if_freq_hz = MxL_IF_4_57_MHZ,
                 .invert_if = 0,
 
-               .loop_thru_enable = 1,
+               .loop_thru_enable = 3,
Review static struct mxl5007t_config af9035_mxl
Quoted 10 lines
(quoted diff) +2 -0
 
                 .clk_out_enable = 1,
                 .clk_out_amp = MxL_CLKOUT_AMP_0_94V,
 
+               .no_probe = 1,
+               .no_reset = 1,
 
         }
 
  };
This patch cannot be merged as-is. I'm sorry. If you could explain why each change was made, then perhaps I would be able to advise better how to make this work on your device without breaking others.
-Mike Krufky
Mike, Jose,
I think there is multiple bugs - both existing and new ones added by Jose.
First existing MxL5007t driver bugs, which are tried to resolve by Jose just wrongly.
You configure clock output and RF loop-through only after first tune and during every tune.
Control flow of mxl5007t init seems to be totally broken logically and cannot work if there is slave tuner used.
Relevant control flow bug is that:
mxl5007t_set_params() {
mxl5007t_tuner_init() {
mxl5007t_soft_reset()
-- configure clock output
-- configure RF loop-through
}
}
RF loop-through as well clock output should be configured on attach() that you could trust slave tuner is operational just after all is attached().
Also, there is another bug, which likely causes disturbance to picture Jose mentioned. MxL5007t soft reset resets all registers to default state. You do that *every* time when set_params() is called. Could you guess what happens to clock output and RF loop-through slave tuner is using?
I made month or two back patch series fixing similar problems of FC0012 driver. See example from there. Jose's hacks are not mostly relevant after that MxL5007t driver is fixed to offer clock and loop-through correctly to the slave tuner.
************************

Quoted 8 lines

Another issue is that register 04, which contains loop-though and some other bits. Jose changes its value from 1 to 3. Is that change really needed? What happens if you leave it just 1? You should not change options like that just for fun without need.
regards
Antti

Due to the complexity of the situation, I would ask that the changes against mxl5007t be resubmitted as a patch series, where each patch only makes a single change, and each patch description should describe that change and the reason for why it is being made. (this should be the gold standard for all patches, anyway)

I don't mean to make things complicated, its just that the patch in question seems to make many changes all at once and lacks clarity for the reasons why those changes are being made.

Please break this up into a patch series of smaller changer so that we can analyze each change separately, we will merge those which do not cause issues with other hardware right away, leaving only the bits that need further discussion unmerged.

Consider the loop thru change from 1 to 3 NACK'd for now, until a better explanation can be provided. Please leave that off the series unless it is absolutely necessary, or you can provide better documentation of it.

Best regards,

Mike Krufky

Antti Palosaari

author

2013-02-03 19:37 UTC #9 lore raw

Quoted 264 lines · Michael Krufky

Quoted 238 lines · Antti Palosaari
Quoted 139 lines · Jose Alberto Reguero
Quoted 50 lines · On Sábado, 2 de febrero de 2013 23:00:45 Michael Krufky escribió:
On Fri, Jan 11, 2013 at 6:45 PM, Jose Alberto Reguero
Quoted 46 lines · <[email protected]> wrote:
Quoted 15 lines · On Viernes, 11 de enero de 2013 20:38:01 Antti Palosaari escribió:
Hello Jose and Gianluca
Could you test that (tda18218 & mxl5007t):
http://git.linuxtv.org/anttip/media_tree.git/shortlog/refs/heads/it9135_t
une r
I wonder if ADC config logic still works for superheterodyne tuners (tuner having IF). I changed it to adc / 2 always due to IT9135 tuner. That makes me wonder it possible breaks tuners having IF, as ADC was clocked just over 20MHz and if it is half then it is 10MHz. For BB that is enough, but I think that having IF, which is 4MHz at least for 8MHz BW it is too less.
Quoted 5 lines
F*ck I hate to maintain driver without a hardware! Any idea where I can get AF9035 device having tda18218 or mxl5007t?
regards
Antti
Still pending the changes for mxl5007t. Attached is a patch for that.
Changes to make work Avermedia Twinstar with the af9035 driver.
Signed-off-by Jose Alberto Reguero <[email protected]>
Jose Alberto
diff -upr linux/drivers/media/tuners/mxl5007t.c linux.new/drivers/media/tuners/mxl5007t.c
--- linux/drivers/media/tuners/mxl5007t.c 2012-08-14
05:45:22.000000000 +0200 +++ linux.new/drivers/media/tuners/mxl5007t.c
2013-01-10 19:23:09.247556275 +0100
(quoted diff) +0 -1
@@ -374,7 +374,6 @@ static struct reg_pair_t *mxl5007t_calc_
 
          mxl5007t_set_if_freq_bits(state, cfg->if_freq_hz,
 cfg->invert_if);
          mxl5007t_set_xtal_freq_bits(state, cfg->xtal_freq_hz);
 
-       set_reg_bits(state->tab_init, 0x04, 0x01,
 cfg->loop_thru_enable);
Quoted 10 lines
Quoted 4 lines
          set_reg_bits(state->tab_init, 0x03, 0x08, cfg->clk_out_enable
<<
          3);
          set_reg_bits(state->tab_init, 0x03, 0x07, cfg->clk_out_amp);
This is a configurable option - it should not be removed, just configure your glue code to not use that option if you dont want it.... unless there's some other reason why you're removing this?
I just move the code to a mxl5007t_attach because with dual tuner until the
code is executed, the other tuner don't work. It can be left here also.
Quoted 11 lines
Quoted 11 lines
(quoted diff) +3 -2
@@ -531,9 +530,12 @@ static int mxl5007t_tuner_init(struct mx
 
          struct reg_pair_t *init_regs;
          int ret;
 
-       ret = mxl5007t_soft_reset(state);
-       if (mxl_fail(ret))
+       if (!state->config->no_reset) {
+               ret = mxl5007t_soft_reset(state);
+               if (mxl_fail(ret))
 
Quoted 9 lines
Review static int mxl5007t_tuner_init(struct mx · line 541
Quoted 4 lines
(quoted diff) +2 -0
                  goto fail;
 
+       }
+
this seems wrong to me. why would you want to prevent the driver from doing a soft reset?
That is because with my hardware and dual tuner, when one tuner do reset, the
other one is perturbed, and the stream has errors.
Quoted 9 lines
Quoted 9 lines
          /* calculate initialization reg array */
          init_regs = mxl5007t_calc_init_regs(state, mode);
(quoted diff) +0 -0
@@ -887,7 +889,12 @@ struct dvb_frontend *mxl5007t_attach(str
 
                  if (fe->ops.i2c_gate_ctrl)
 
                          fe->ops.i2c_gate_ctrl(fe, 1);
 
Quoted 11 lines
Review struct dvb_frontend *mxl5007t_attach(str · line 899
Quoted 7 lines
(quoted diff) +6 -1
-               ret = mxl5007t_get_chip_id(state);
+               if (!state->config->no_probe)
+                       ret = mxl5007t_get_chip_id(state);
+
+               ret = mxl5007t_write_reg(state, 0x04,
+                       state->config->loop_thru_enable);
+
Can you explain why this change was made? ^^
mxl5007t_get_chip_id has a read, and with the hardware I have, after the read
operation is made, communication with the chip don't work.
Quoted 15 lines
Quoted 15 lines
(quoted diff) +0 -0
                  if (fe->ops.i2c_gate_ctrl)
                          fe->ops.i2c_gate_ctrl(fe, 0);
diff -upr linux/drivers/media/tuners/mxl5007t.h linux.new/drivers/media/tuners/mxl5007t.h
--- linux/drivers/media/tuners/mxl5007t.h 2012-08-14
05:45:22.000000000 +0200 +++ linux.new/drivers/media/tuners/mxl5007t.h
2013-01-10 19:19:11.204379581 +0100
(quoted diff) +0 -0
@@ -73,8 +73,10 @@ struct mxl5007t_config {
 
          enum mxl5007t_xtal_freq xtal_freq_hz;
          enum mxl5007t_if_freq if_freq_hz;
          unsigned int invert_if:1;
 
Quoted 6 lines
Review struct mxl5007t_config { · line 78
Quoted 2 lines
(quoted diff) +1 -1
-       unsigned int loop_thru_enable:1;
+       unsigned int loop_thru_enable:3;
Why widen this boolean to three bits?
Quoted 59 lines
I just use the value 3 for this option(taken from windows driver) and it works
well.
Thanks for review the code.
Jose Alberto
Quoted 50 lines
Quoted 33 lines
(quoted diff) +2 -0
          unsigned int clk_out_enable:1;
 
+       unsigned int no_probe:1;
+       unsigned int no_reset:1;
   };

   #if defined(CONFIG_MEDIA_TUNER_MXL5007T) ||
(defined(CONFIG_MEDIA_TUNER_MXL5007T_MODULE) && defined(MODULE)) diff -upr linux/drivers/media/usb/dvb-usb-v2/af9035.c linux.new/drivers/media/usb/dvb-usb-v2/af9035.c
--- linux/drivers/media/usb/dvb-usb-v2/af9035.c 2013-01-07 05:45:57.000000000 +0100
+++ linux.new/drivers/media/usb/dvb-usb-v2/af9035.c 2013-01-12
00:30:57.557310465 +0100
(quoted diff) +3 -1
@@ -886,13 +886,17 @@ static struct mxl5007t_config af9035_mxl
 
                  .loop_thru_enable = 0,
                  .clk_out_enable = 0,
                  .clk_out_amp = MxL_CLKOUT_AMP_0_94V,
 
+               .no_probe = 1,
+               .no_reset = 1,
 
          }, {
 
                  .xtal_freq_hz = MxL_XTAL_24_MHZ,
                  .if_freq_hz = MxL_IF_4_57_MHZ,
                  .invert_if = 0,
 
-               .loop_thru_enable = 1,
+               .loop_thru_enable = 3,
Review static struct mxl5007t_config af9035_mxl
Quoted 10 lines
(quoted diff) +2 -0
 
                  .clk_out_enable = 1,
                  .clk_out_amp = MxL_CLKOUT_AMP_0_94V,
 
+               .no_probe = 1,
+               .no_reset = 1,
 
          }
 
   };
This patch cannot be merged as-is. I'm sorry. If you could explain why each change was made, then perhaps I would be able to advise better how to make this work on your device without breaking others.
-Mike Krufky
Mike, Jose,
I think there is multiple bugs - both existing and new ones added by Jose.
First existing MxL5007t driver bugs, which are tried to resolve by Jose just wrongly.
You configure clock output and RF loop-through only after first tune and during every tune.
Control flow of mxl5007t init seems to be totally broken logically and cannot work if there is slave tuner used.
Relevant control flow bug is that:
mxl5007t_set_params() {
   mxl5007t_tuner_init() {
     mxl5007t_soft_reset()
     -- configure clock output
     -- configure RF loop-through
   }
}
RF loop-through as well clock output should be configured on attach() that you could trust slave tuner is operational just after all is attached().
Also, there is another bug, which likely causes disturbance to picture Jose mentioned. MxL5007t soft reset resets all registers to default state. You do that *every* time when set_params() is called. Could you guess what happens to clock output and RF loop-through slave tuner is using?
I made month or two back patch series fixing similar problems of FC0012 driver. See example from there. Jose's hacks are not mostly relevant after that MxL5007t driver is fixed to offer clock and loop-through correctly to the slave tuner.
************************
Quoted 8 lines
Another issue is that register 04, which contains loop-though and some other bits. Jose changes its value from 1 to 3. Is that change really needed? What happens if you leave it just 1? You should not change options like that just for fun without need.
regards
Antti
Due to the complexity of the situation, I would ask that the changes against mxl5007t be resubmitted as a patch series, where each patch only makes a single change, and each patch description should describe that change and the reason for why it is being made. (this should be the gold standard for all patches, anyway)
I don't mean to make things complicated, its just that the patch in question seems to make many changes all at once and lacks clarity for the reasons why those changes are being made.
Please break this up into a patch series of smaller changer so that we can analyze each change separately, we will merge those which do not cause issues with other hardware right away, leaving only the bits that need further discussion unmerged.

Quoted 9 lines

Consider the loop thru change from 1 to 3 NACK'd for now, until a better explanation can be provided. Please leave that off the series unless it is absolutely necessary, or you can provide better documentation of it.
Best regards,
Mike Krufky

I agree that it should be split multiple patches.

1) soft reset should be moved to attach() (it could not be on init() nor set_parameters() as it stops clock out and loop-through in few ms or so causing slave tuner errors)

2) clock out and loop-through must be set on attach() and not touch after that

3) no_probe option should not be added unless it is really needed. If chip ID reading fails with some I/O error then there is two possibilities a) block reads like now b) add glue to AF9035 brain-dead I2C adapter to handle / fake such case

4) loop_thru_enable to 3 bit wide should not be done unless really needed. What happens if it is left as it is?

These are the four logical changes that should be sent as own patch. Jose, we are waiting for you :)

regards
Antti

Signature · 1 line

http://palosaari.fi/

Michael Krufky

maintainer

2013-02-03 19:53 UTC #10 lore raw

(history chopped cuz it got messy)

quoting Antti with my responses inline.

<<
I agree that it should be split multiple patches.

KRUFKY: YES.

1) soft reset should be moved to attach() (it could not be on init() nor set_parameters() as it stops clock out and loop-through in few ms or so causing slave tuner errors)

KRUFKY: NO. This is not the solution. If there is a bug in the driver, then we fix the bug. Moving the soft reset to a one time only call during attach can cause worse problems. If you feel strongly about this, then submit it in a separate patch and we can work on that issue separately. The soft reset needs to be done each time the tuner is programmed for good reason - if we are screwing up some registers, then it means that there is a bug - lets fix the bug.

2) clock out and loop-through must be set on attach() and not touch after that

KRUFKY: NO. attach() is called once, ever. I admit that the current
code may be buggy but doing this would cause unpredicable behavior after low-power states... If this needs to be fixed then it needs to be fixed in a thorough way, not by moving the code away into the attach function where it will only be called once. Clearly this issue is directly related to issue number 1, so I understand if these two items might be the focus of future discussion :-/

KRUFKY: I agree -- this may be required in order to work around some questionable hardware implementations. If the problem is really in the i2c adapter, then the hack belongs there, not in the tuner driver.

4) loop_thru_enable to 3 bit wide should not be done unless really needed. What happens if it is left as it is?

KRUFKY: Agreed. We don't make a change just because you saw something in 'the windows driver' As per the current Linux driver, the loop thru setting is 1 bit wide. If this is wrong, please provide a better explanation of those bits.

These are the four logical changes that should be sent as own patch. Jose, we are waiting for you :)

Quoted 1 line

Quoted 1 line

-Mike

Antti Palosaari

author

2013-02-03 20:30 UTC #11 lore raw

Quoted 13 lines · Michael Krufky

(history chopped cuz it got messy)
quoting Antti with my responses inline.
<<
I agree that it should be split multiple patches.
KRUFKY: YES.
1) soft reset should be moved to attach() (it could not be on init() nor set_parameters() as it stops clock out and loop-through in few ms or so causing slave tuner errors)

Quoted 7 lines

KRUFKY: NO. This is not the solution. If there is a bug in the driver, then we fix the bug. Moving the soft reset to a one time only call during attach can cause worse problems. If you feel strongly about this, then submit it in a separate patch and we can work on that issue separately. The soft reset needs to be done each time the tuner is programmed for good reason - if we are screwing up some registers, then it means that there is a bug - lets fix the bug.

You cannot do soft reset all the time. MxL5007t soft reset looks like just jump instruction to chip reset vector, it simply clears all the registers to the default state (I think just same state as power on reset).

That means you taint clock output and loop-through every time you call that soft reset. Why the hell there is such outputs offered by the chip if those are aimed to shut off frequently by soft resetting chip? Such outputs are useless. Due to that analogy, there will be only one conclusion: soft reset is not aimed to be called for every tuning attempt.

It is just easy way to ensure chip is known default state on attach(). For example you warm boot from windows to linux and wish to ensure chip is known state after attach(). It is driver bug if soft resetting all the registers to default is needed frequently in order to operate normally!

Quoted 9 lines

2) clock out and loop-through must be set on attach() and not touch after that
KRUFKY: NO. attach() is called once, ever. I admit that the current
code may be buggy but doing this would cause unpredicable behavior after low-power states... If this needs to be fixed then it needs to be fixed in a thorough way, not by moving the code away into the attach function where it will only be called once. Clearly this issue is directly related to issue number 1, so I understand if these two items might be the focus of future discussion :-/

Shutting down clock output when not needed surely saves few mA from the current drain. But currently there is no DVB framework support for it, so better to leave clock out enabled always. It is relative small amount of current you will save - there is a lot of bigger power management issues about all the drivers currently.

Quoted 8 lines

3) no_probe option should not be added unless it is really needed. If chip ID reading fails with some I/O error then there is two possibilities a) block reads like now b) add glue to AF9035 brain-dead I2C adapter to handle / fake such case
KRUFKY: I agree -- this may be required in order to work around some questionable hardware implementations. If the problem is really in the i2c adapter, then the hack belongs there, not in the tuner driver.

The one thing what I think I has already mentioned for Jose - test some other tuner IDs. There is many tuners supported by AF9035 FW and about all of those uses register reads. So telling wrong tuner ID to AF9035 just before attach tuner could do the trick. And after successful tuner attach just tell AF9035 FW that MXL5007T tuner id.

Quoted 8 lines

4) loop_thru_enable to 3 bit wide should not be done unless really needed. What happens if it is left as it is?
KRUFKY: Agreed. We don't make a change just because you saw something in 'the windows driver' As per the current Linux driver, the loop thru setting is 1 bit wide. If this is wrong, please provide a better explanation of those bits.

Quoted 6 lines

These are the four logical changes that should be sent as own patch. Jose, we are waiting for you :)
Quoted 1 line
Quoted 1 line
-Mike

Antti

Signature · 1 line

http://palosaari.fi/

Michael Krufky

maintainer

2013-02-03 21:02 UTC #12 lore raw

Quoted 91 lines · Antti Palosaari

Quoted 14 lines · Michael Krufky
(history chopped cuz it got messy)
quoting Antti with my responses inline.
<<
I agree that it should be split multiple patches.
KRUFKY: YES.
1) soft reset should be moved to attach() (it could not be on init() nor set_parameters() as it stops clock out and loop-through in few ms or so causing slave tuner errors)
Quoted 7 lines
KRUFKY: NO. This is not the solution. If there is a bug in the driver, then we fix the bug. Moving the soft reset to a one time only call during attach can cause worse problems. If you feel strongly about this, then submit it in a separate patch and we can work on that issue separately. The soft reset needs to be done each time the tuner is programmed for good reason - if we are screwing up some registers, then it means that there is a bug - lets fix the bug.
You cannot do soft reset all the time. MxL5007t soft reset looks like just jump instruction to chip reset vector, it simply clears all the registers to the default state (I think just same state as power on reset).
That means you taint clock output and loop-through every time you call that soft reset. Why the hell there is such outputs offered by the chip if those are aimed to shut off frequently by soft resetting chip? Such outputs are useless. Due to that analogy, there will be only one conclusion: soft reset is not aimed to be called for every tuning attempt.
It is just easy way to ensure chip is known default state on attach(). For example you warm boot from windows to linux and wish to ensure chip is known state after attach(). It is driver bug if soft resetting all the registers to default is needed frequently in order to operate normally!
Quoted 10 lines
2) clock out and loop-through must be set on attach() and not touch after that
KRUFKY: NO. attach() is called once, ever. I admit that the current
code may be buggy but doing this would cause unpredicable behavior after low-power states... If this needs to be fixed then it needs to be fixed in a thorough way, not by moving the code away into the attach function where it will only be called once. Clearly this issue is directly related to issue number 1, so I understand if these two items might be the focus of future discussion :-/
Shutting down clock output when not needed surely saves few mA from the current drain. But currently there is no DVB framework support for it, so better to leave clock out enabled always. It is relative small amount of current you will save - there is a lot of bigger power management issues about all the drivers currently.
Quoted 8 lines
3) no_probe option should not be added unless it is really needed. If chip ID reading fails with some I/O error then there is two possibilities a) block reads like now b) add glue to AF9035 brain-dead I2C adapter to handle / fake such case
KRUFKY: I agree -- this may be required in order to work around some questionable hardware implementations. If the problem is really in the i2c adapter, then the hack belongs there, not in the tuner driver.
The one thing what I think I has already mentioned for Jose - test some other tuner IDs. There is many tuners supported by AF9035 FW and about all of those uses register reads. So telling wrong tuner ID to AF9035 just before attach tuner could do the trick. And after successful tuner attach just tell AF9035 FW that MXL5007T tuner id.
Quoted 13 lines
4) loop_thru_enable to 3 bit wide should not be done unless really needed. What happens if it is left as it is?
KRUFKY: Agreed. We don't make a change just because you saw something in 'the windows driver' As per the current Linux driver, the loop thru setting is 1 bit wide. If this is wrong, please provide a better explanation of those bits.
These are the four logical changes that should be sent as own patch. Jose, we are waiting for you :)
Quoted 2 lines
Quoted 2 lines

Quoted 4 lines

Quoted 2 lines
-Mike
Antti

I was just thinking and I realize fault in my own arguments where I said "No." ... Coincidentally you sent this email just as I was thinking of doing the same.

I retract my "No" s :-)

Let's just see a patch series and we'll evaluate each patch individually.

Cheers,

Mike

Gianluca Gennari

2013-01-18 09:19 UTC #13 lore raw

In reply to Antti Palosaari

Il 11/01/2013 19:38, Antti Palosaari ha scritto:

Quoted 6 lines

Hello Jose and Gianluca
Could you test that (tda18218 & mxl5007t):
http://git.linuxtv.org/anttip/media_tree.git/shortlog/refs/heads/it9135_tuner

Quoted 6 lines

I wonder if ADC config logic still works for superheterodyne tuners (tuner having IF). I changed it to adc / 2 always due to IT9135 tuner. That makes me wonder it possible breaks tuners having IF, as ADC was clocked just over 20MHz and if it is half then it is 10MHz. For BB that is enough, but I think that having IF, which is 4MHz at least for 8MHz BW it is too less.

Hi Antti,
I tested your latest tree and both the Avermedia A835 (af9035+tda18218) and the A867 (af9035+mxl5007t) work perfectly fine. I could not find any change in the behavior of the two devices.

BTW, there are reports on several Italian forums (like this:
http://www.vuplus-community.net/board/threads/varie-prove-fatte-su-vu-uno-e-digital-key-a867-led-blu.9930/)
that a new revision of the A867 stick (marked as "A867-DP7") does not work with the current af9035 driver, but it works perfectly fine when Jose's patch for the mxl5007t tuner is applied. I have an older "DP5" revision that always worked fine with the af9035 driver, so I cannot test the new "DP7" revision directly.

Quoted 3 lines

F*ck I hate to maintain driver without a hardware! Any idea where I can get AF9035 device having tda18218 or mxl5007t?

Here are a couple of links to buy cheap af9035 sticks.

Avermedia A835/B835: af9035 + tda18218, also known ad "Avermedia Volar HD" or "Avermedia Volar HD Pro" or "Avermedia Volar Green HD". The only difference between the models seems to be the presence of the remote controller and the IR sensor:

http://www.amazon.it/Avermedia-Avertv-Volar-Green-Hd/dp/B003GXAMEM/ref=sr_1_2?ie=UTF8&qid=1358499827&sr=8-2

http://www.amazon.it/Avermedia-Avertv-Volar-Hd-Pro/dp/B003ACL1ZI/ref=sr_1_1?ie=UTF8&qid=1358499827&sr=8-1

Avermedia A867: af9035 + mxl5007t, also known as "Aver Media AVerTV 3D" or "Sky Digital Key with blue led". You can buy them very cheap on Ebay Italia because Sky Italia is giving away them almost for free to its subscribers, to add DVB-T support to the Skyboxes. You can find dozens of items looking for "Sky Digital Key Blu Led" on the Italian Ebay:

http://www.ebay.it/sch/i.html?_trksid=p3902.m570.l1313&_nkw=sky+digital+key+blu&_sacat=0&_from=R40

or you can buy the retail model:

http://www.amazon.it/Avermedia-AverTV-Volar-Entertainment-Pack/dp/B003TPW5WO/ref=sr_1_2?s=electronics&ie=UTF8&qid=1358499900&sr=1-2

As an alternative, I could ask a friend if he is willing to lend you the 2 sticks for all the time you need. He has both the A835 and the A867 "DP7" revision. Let me know if you are interested.

Regards,
Gianluca

Quoted 4 lines

regards
Antti

Andre Heider

2013-01-31 13:04 UTC #14 lore raw

In reply to Antti Palosaari

Hi,

Quoted 2 lines · Antti Palosaari

Could you test that (tda18218 & mxl5007t):
http://git.linuxtv.org/anttip/media_tree.git/shortlog/refs/heads/it9135_tuner

I got a 'TerraTec Cinergy T Stick Dual RC (rev. 2)', which is fixed by this series.
Any chance to get this into 3.9 (I guess its too late for the USB VID/PID 'fix' for 3.8)?

Regards,
Andre

Michael Krufky

maintainer

2013-01-31 13:46 UTC #15 lore raw

Hey guys... somehow this email slipped through my filters :-( I see it now, and I'll give a look over the patch this weekend.

-Mike

Quoted 17 lines · Andre Heider

Hi,
Quoted 2 lines · Antti Palosaari
Could you test that (tda18218 & mxl5007t):
http://git.linuxtv.org/anttip/media_tree.git/shortlog/refs/heads/it9135_tuner
I got a 'TerraTec Cinergy T Stick Dual RC (rev. 2)', which is fixed by this series.
Any chance to get this into 3.9 (I guess its too late for the USB VID/PID 'fix' for 3.8)?
Regards,
Andre
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Andre Heider

2013-02-07 12:45 UTC #16 lore raw

Hi,

Quoted 2 lines · Michael Krufky

Hey guys... somehow this email slipped through my filters :-( I see it now, and I'll give a look over the patch this weekend.

I suspect the merge window opens soon, so... *poke* ;) Any chance for 3.9?

Thanks,
Andre

Antti Palosaari

author

2013-01-31 14:00 UTC #17 lore raw

In reply to Andre Heider

Quoted 10 lines · Andre Heider

Hi,
Quoted 2 lines · Antti Palosaari
Could you test that (tda18218 & mxl5007t):
http://git.linuxtv.org/anttip/media_tree.git/shortlog/refs/heads/it9135_tuner
I got a 'TerraTec Cinergy T Stick Dual RC (rev. 2)', which is fixed by this series.
Any chance to get this into 3.9 (I guess its too late for the USB VID/PID 'fix' for 3.8)?

Thank you for the report! There was someone else who reported it working too. Do you want to your name as tester for the changelog?

I just yesterday got that TerraTec device too and I am going to add dual tuner support. Also, for some reason IT9135 v2 devices are not working - only v1. That is one thing I should fix before merge that stuff.

regards
Antti

Signature · 1 line

http://palosaari.fi/

Andre Heider

Tested-by

2013-01-31 17:38 UTC #18 lore raw

Hi,

Quoted 2 lines · Antti Palosaari

Thank you for the report! There was someone else who reported it working too. Do you want to your name as tester for the changelog?

if I didn't mess up my way of testing feel free to add

Tested-by Andre Heider <[email protected]>

to these patches:
af9035: merge af9035 and it9135 eeprom read routines
af9035: USB1.1 support (== PID filters)
af9035: constify clock tables
af9035: [0ccd:0099] TerraTec Cinergy T Stick Dual RC (rev. 2)
af9015: reject device TerraTec Cinergy T Stick Dual RC (rev. 2)
af9035: fix af9033 demod sampling frequency
af9035: add auto configuration heuristic for it9135
af9035: add support for 1st gen it9135
af9033: support for it913x tuners
ITE IT913X silicon tuner driver

I didn't use any media trees before, and the whole media_build.git shebang seems a little, well, unusual...
So I rebased media_tree.git/staging/for_v3.9 on Linus' master and then cherry-picked the patches mentioned above.

That gives me:
usb 2-1.5: new high-speed USB device number 3 using ehci-pci
usb 2-1.5: New USB device found, idVendor=0ccd, idProduct=0099
usb 2-1.5: New USB device strings: Mfr=1, Product=2, SerialNumber=0 usb 2-1.5: Product: DVB-T TV Stick
usb 2-1.5: Manufacturer: ITE Technologies, Inc.
input: ITE Technologies, Inc. DVB-T TV Stick as /devices/pci0000:00/0000:00:1d.0/usb2/2-1/2-1.5/2-1.5:1.1/input/input20 usb 2-1.5: af9035_identify_state: prechip_version=83 chip_version=01 chip_type=9135
hid-generic 0003:0CCD:0099.0007: input,hidraw4: USB HID v1.01 Keyboard [ITE Technologies, Inc. DVB-T TV Stick] on usb-0000:00:1d.0-1.5/input1 usb 2-1.5: dvb_usb_v2: found a 'TerraTec Cinergy T Stick Dual RC (rev. 2)' in cold state
usb 2-1.5: dvb_usb_v2: downloading firmware from file 'dvb-usb-it9135-01.fw' usb 2-1.5: dvb_usb_af9035: firmware version=12.54.14.0
usb 2-1.5: dvb_usb_v2: found a 'TerraTec Cinergy T Stick Dual RC (rev. 2)' in warm state
usb 2-1.5: dvb_usb_af9035: driver does not support 2nd tuner and will disable it usb 2-1.5: dvb_usb_v2: will pass the complete MPEG2 transport stream to the software demuxer
DVB: registering new adapter (TerraTec Cinergy T Stick Dual RC (rev. 2)) i2c i2c-18: af9033: firmware version: LINK=255.255.255.255 OFDM=2.47.14.0 usb 2-1.5: DVB: registering adapter 0 frontend 0 (Afatech AF9033 (DVB-T))... Tuner LNA type :38
it913x: ITE Tech IT913X attached
usb 2-1.5: dvb_usb_v2: 'TerraTec Cinergy T Stick Dual RC (rev. 2)' successfully initialized and connected

Quoted 3 lines

I just yesterday got that TerraTec device too and I am going to add dual tuner support. Also, for some reason IT9135 v2 devices are not working - only v1. That is one thing I should fix before merge that stuff.

Nice, feel free to CC me if you need any testing.

Regards,
Andre

Andre Heider

2013-01-31 18:40 UTC #19 lore raw

In reply to Antti Palosaari

Hey,

Quoted 3 lines · Antti Palosaari

Quoted 3 lines · Antti Palosaari
Quoted 2 lines
Could you test that (tda18218 & mxl5007t):

only now I see you mentioned mxl5007t too, and with the same tree as I used for my 'TerraTec Cinergy T Stick Dual RC (rev. 2)', a 'AVerMedia HD Volar (A867)' with a mxl5007t (and an unkown rev) works too:

usb 3-3.1.4: new high-speed USB device number 7 using xhci_hcd usb 3-3.1.4: New USB device found, idVendor=07ca, idProduct=1867 usb 3-3.1.4: New USB device strings: Mfr=1, Product=2, SerialNumber=3 usb 3-3.1.4: Product: A867
usb 3-3.1.4: Manufacturer: AVerMedia TECHNOLOGIES, Inc
usb 3-3.1.4: SerialNumber: 0305770200261
usb 3-3.1.4: af9035_identify_state: prechip_version=00 chip_version=03 chip_type=3802
usb 3-3.1.4: dvb_usb_v2: found a 'AVerMedia HD Volar (A867)' in cold state usb 3-3.1.4: dvb_usb_v2: downloading firmware from file 'dvb-usb-af9035-02.fw' usb 3-3.1.4: dvb_usb_af9035: firmware version=11.5.9.0
usb 3-3.1.4: dvb_usb_v2: found a 'AVerMedia HD Volar (A867)' in warm state usb 3-3.1.4: dvb_usb_v2: will pass the complete MPEG2 transport stream to the software demuxer
DVB: registering new adapter (AVerMedia HD Volar (A867))
i2c i2c-19: af9033: firmware version: LINK=11.5.9.0 OFDM=5.17.9.1 usb 3-3.1.4: DVB: registering adapter 1 frontend 0 (Afatech AF9033 (DVB-T))... mxl5007t 19-0060: creating new instance
mxl5007t_get_chip_id: unknown rev (3f)
mxl5007t_get_chip_id: MxL5007T detected @ 19-0060
Registered IR keymap rc-empty
input: AVerMedia HD Volar (A867) as /devices/pci0000:00/0000:00:14.0/usb3/3-3/3-3.1/3-3.1.4/rc/rc5/input29
rc5: AVerMedia HD Volar (A867) as /devices/pci0000:00/0000:00:14.0/usb3/3-3/3-3.1/3-3.1.4/rc/rc5 usb 3-3.1.4: dvb_usb_v2: schedule remote query interval to 500 msecs usb 3-3.1.4: dvb_usb_v2: 'AVerMedia HD Volar (A867)' successfully initialized and connected

Regards,
Andre

Antti Palosaari

author

2013-01-31 18:53 UTC #20 lore raw

Jose, Gianluca,

Quoted 11 lines · Andre Heider

Hey,
Quoted 3 lines · Antti Palosaari
Quoted 3 lines · Antti Palosaari
Quoted 2 lines
Could you test that (tda18218 & mxl5007t):
only now I see you mentioned mxl5007t too, and with the same tree as I used for my 'TerraTec Cinergy T Stick Dual RC (rev. 2)', a 'AVerMedia HD Volar (A867)' with a mxl5007t (and an unkown rev) works too:

Quoted 8 lines

usb 3-3.1.4: new high-speed USB device number 7 using xhci_hcd usb 3-3.1.4: New USB device found, idVendor=07ca, idProduct=1867 usb 3-3.1.4: New USB device strings: Mfr=1, Product=2, SerialNumber=3 usb 3-3.1.4: Product: A867
usb 3-3.1.4: Manufacturer: AVerMedia TECHNOLOGIES, Inc
usb 3-3.1.4: SerialNumber: 0305770200261
usb 3-3.1.4: af9035_identify_state: prechip_version=00 chip_version=03 chip_type=3802

Who one as able to test with non-working AF9035 + MxL5007T combination. Does it report different chip versions? Same firmware used?

Quoted 12 lines

usb 3-3.1.4: dvb_usb_v2: found a 'AVerMedia HD Volar (A867)' in cold state usb 3-3.1.4: dvb_usb_v2: downloading firmware from file 'dvb-usb-af9035-02.fw' usb 3-3.1.4: dvb_usb_af9035: firmware version=11.5.9.0
usb 3-3.1.4: dvb_usb_v2: found a 'AVerMedia HD Volar (A867)' in warm state usb 3-3.1.4: dvb_usb_v2: will pass the complete MPEG2 transport stream to the software demuxer
DVB: registering new adapter (AVerMedia HD Volar (A867))
i2c i2c-19: af9033: firmware version: LINK=11.5.9.0 OFDM=5.17.9.1 usb 3-3.1.4: DVB: registering adapter 1 frontend 0 (Afatech AF9033 (DVB-T))... mxl5007t 19-0060: creating new instance
mxl5007t_get_chip_id: unknown rev (3f)
mxl5007t_get_chip_id: MxL5007T detected @ 19-0060

Quoted 8 lines

Registered IR keymap rc-empty
input: AVerMedia HD Volar (A867) as /devices/pci0000:00/0000:00:14.0/usb3/3-3/3-3.1/3-3.1.4/rc/rc5/input29
rc5: AVerMedia HD Volar (A867) as /devices/pci0000:00/0000:00:14.0/usb3/3-3/3-3.1/3-3.1.4/rc/rc5 usb 3-3.1.4: dvb_usb_v2: schedule remote query interval to 500 msecs usb 3-3.1.4: dvb_usb_v2: 'AVerMedia HD Volar (A867)' successfully initialized and connected

regards
Antti

Signature · 1 line

http://palosaari.fi/

Gianluca Gennari

2013-02-09 14:17 UTC #21 lore raw

Il 31/01/2013 19:52, Antti Palosaari ha scritto:

Quoted 23 lines

Jose, Gianluca,
Quoted 11 lines · Andre Heider
Hey,
Quoted 3 lines · Antti Palosaari
Quoted 3 lines · Antti Palosaari
Quoted 2 lines
Could you test that (tda18218 & mxl5007t):
only now I see you mentioned mxl5007t too, and with the same tree as I used for my 'TerraTec Cinergy T Stick Dual RC (rev. 2)', a 'AVerMedia HD Volar (A867)' with a mxl5007t (and an unkown rev) works too:
Quoted 8 lines
usb 3-3.1.4: new high-speed USB device number 7 using xhci_hcd usb 3-3.1.4: New USB device found, idVendor=07ca, idProduct=1867 usb 3-3.1.4: New USB device strings: Mfr=1, Product=2, SerialNumber=3 usb 3-3.1.4: Product: A867
usb 3-3.1.4: Manufacturer: AVerMedia TECHNOLOGIES, Inc
usb 3-3.1.4: SerialNumber: 0305770200261
usb 3-3.1.4: af9035_identify_state: prechip_version=00 chip_version=03 chip_type=3802

Quoted 2 lines

Who one as able to test with non-working AF9035 + MxL5007T combination. Does it report different chip versions? Same firmware used?

Hi Antti,
I finally found a friend with the Avermedia A867 (AF9035 + MxL5007T) non working revision (A867-DP7):
http://forum.ubuntu-it.org/viewtopic.php?f=9&t=516182&start=60#p4301226

Apparently, there is no difference in the log file about the chip version:

[   90.047319] usb 1-1.3: New USB device found, idVendor=07ca,
idProduct=a867
[   90.047325] usb 1-1.3: New USB device strings: Mfr=1, Product=2,
SerialNumber=3
[   90.047330] usb 1-1.3: Product: A867
[   90.047334] usb 1-1.3: Manufacturer: AVerMedia TECHNOLOGIES, Inc
[   90.047337] usb 1-1.3: SerialNumber: 5037944035440
[   90.142796] usbcore: registered new interface driver dvb_usb_af9035
[   90.143779] usb 1-1.3: af9035_identify_state: prechip_version=00
chip_version=03 chip_type=3802
[   90.144178] usb 1-1.3: dvb_usb_v2: found a 'AVerMedia HD Volar
(A867)' in cold state
[   90.170437] usb 1-1.3: dvb_usb_v2: downloading firmware from file
'dvb-usb-af9035-02.fw'
[   90.495461] usb 1-1.3: dvb_usb_af9035: firmware version=12.13.15.0
[   90.495483] usb 1-1.3: dvb_usb_v2: found a 'AVerMedia HD Volar
(A867)' in warm state
[   90.498004] usb 1-1.3: dvb_usb_v2: will pass the complete MPEG2
transport stream to the software demuxer
[   90.498046] DVB: registering new adapter (AVerMedia HD Volar (A867))
[   90.498401] DVB: register adapter0/demux0 @ minor: 0 (0x00)
[   90.498476] DVB: register adapter0/dvr0 @ minor: 1 (0x01)
[   90.498543] DVB: register adapter0/net0 @ minor: 2 (0x02)
[   90.549788] i2c i2c-8: af9033: firmware version: LINK=12.13.15.0
OFDM=6.20.15.0
[   90.549798] usb 1-1.3: DVB: registering adapter 0 frontend 0 (Afatech
AF9033 (DVB-T))...
[   90.549903] DVB: register adapter0/frontend0 @ minor: 3 (0x03)
[   90.913945] mxl5007t 8-0060: creating new instance
[   90.929824] Registered IR keymap rc-empty
[   90.929937] input: AVerMedia HD Volar (A867) as
/devices/pci0000:00/0000:00:1a.0/usb1/1-1/1-1.3/rc/rc0/input13
[   90.930019] rc0: AVerMedia HD Volar (A867) as
/devices/pci0000:00/0000:00:1a.0/usb1/1-1/1-1.3/rc/rc0
[   90.930027] usb 1-1.3: dvb_usb_v2: schedule remote query interval to
500 msecs
[   90.930032] usb 1-1.3: dvb_usb_v2: 'AVerMedia HD Volar (A867)'

successfully initialized and connected

Also, the stick works fine with Jose's patch, independently from the firmware file used.

Regards,
Gianluca

Quoted 30 lines

Quoted 18 lines
usb 3-3.1.4: dvb_usb_v2: found a 'AVerMedia HD Volar (A867)' in cold state
usb 3-3.1.4: dvb_usb_v2: downloading firmware from file 'dvb-usb-af9035-02.fw'
usb 3-3.1.4: dvb_usb_af9035: firmware version=11.5.9.0
usb 3-3.1.4: dvb_usb_v2: found a 'AVerMedia HD Volar (A867)' in warm state
usb 3-3.1.4: dvb_usb_v2: will pass the complete MPEG2 transport stream to the software demuxer
DVB: registering new adapter (AVerMedia HD Volar (A867)) i2c i2c-19: af9033: firmware version: LINK=11.5.9.0 OFDM=5.17.9.1 usb 3-3.1.4: DVB: registering adapter 1 frontend 0 (Afatech AF9033 (DVB-T))...
mxl5007t 19-0060: creating new instance
mxl5007t_get_chip_id: unknown rev (3f)
mxl5007t_get_chip_id: MxL5007T detected @ 19-0060
Quoted 8 lines
Registered IR keymap rc-empty
input: AVerMedia HD Volar (A867) as /devices/pci0000:00/0000:00:14.0/usb3/3-3/3-3.1/3-3.1.4/rc/rc5/input29
rc5: AVerMedia HD Volar (A867) as /devices/pci0000:00/0000:00:14.0/usb3/3-3/3-3.1/3-3.1.4/rc/rc5 usb 3-3.1.4: dvb_usb_v2: schedule remote query interval to 500 msecs usb 3-3.1.4: dvb_usb_v2: 'AVerMedia HD Volar (A867)' successfully initialized and connected
regards
Antti

pierigno

2013-02-15 15:48 UTC #22 lore raw

Hello,

I've tested my AverMedia Twinstar (A825) against the latest antti's git repository, branch it9135_tuner. The following patches make the stick works in dual mode with all the currenty available firmware,that is:

dvb-usb-af9035-02.fw_0184ba128bee3befe2fc49f144a8dbab_12.5.13.0_6.8.13.bin dvb-usb-af9035-02.fw_085e676dc50addf538b6cb18f4ca233a_12.13.15.0_6.20.15.bin dvb-usb-af9035-02.fw_14ae2b81bac90ff5c6b490c225265267_12.13.15.0_6.20.15.bin dvb-usb-af9035-02.fw_3735d499d945a6bb873a7f3ad5c701fa_12.13.15.0_6.20.15.bin dvb-usb-af9035-02.fw_7cdc1e3aba54f3a9ad052dc6a29603fd_11.10.10.0_5.33.10.bin dvb-usb-af9035-02.fw_f71efe295151ba76cac2280680b69f3f_11.5.9.0_5.17.9.bin

general output of dmesg:

[112271.422441] usb 2-1.2.4: new high-speed USB device number 18 using ehci_hcd
[112271.503855] usb 2-1.2.4: New USB device found, idVendor=07ca, idProduct=0825
[112271.503867] usb 2-1.2.4: New USB device strings: Mfr=1, Product=2,
SerialNumber=3
[112271.503873] usb 2-1.2.4: Product: A825
[112271.503879] usb 2-1.2.4: Manufacturer: AVerMedia TECHNOLOGIES, Inc.
[112271.503884] usb 2-1.2.4: SerialNumber: 3018704000300
[112271.506973] usb 2-1.2.4: af9035_identify_state: prechip_version=00
chip_version=03 chip_type=3802
[112271.507348] usb 2-1.2.4: dvb_usb_v2: found a 'AVerMedia Twinstar
(A825)' in cold state
[112271.507491] usb 2-1.2.4: dvb_usb_v2: downloading firmware from
file 'dvb-usb-af9035-02.fw'
[112274.148675] usb 2-1.2.4: dvb_usb_af9035: firmware version=12.5.13.0
[112274.148714] usb 2-1.2.4: dvb_usb_v2: found a 'AVerMedia Twinstar
(A825)' in warm state
[112274.152673] usb 2-1.2.4: dvb_usb_v2: will pass the complete MPEG2
transport stream to the software demuxer
[112274.152723] DVB: registering new adapter (AVerMedia Twinstar (A825))
[112274.154013] 909: i2c i2c-7: af9033_attach:
[112274.154864] i2c i2c-7: af9033: firmware version: LINK=12.5.13.0
OFDM=6.8.13.0
[112274.154876] usb 2-1.2.4: DVB: registering adapter 0 frontend 0
(Afatech AF9033 (DVB-T))...
[112274.490178] mxl5007t 7-0060: creating new instance
[112274.490888] mxl5007t_get_chip_id: unknown rev (3f)
[112274.490894] mxl5007t_get_chip_id: MxL5007T detected @ 7-0060
[112274.490906] usb 2-1.2.4: dvb_usb_v2: will pass the complete MPEG2
transport stream to the software demuxer
[112274.490955] DVB: registering new adapter (AVerMedia Twinstar (A825))
[112274.491655] 909: i2c i2c-7: af9033_attach:
[112274.503395] i2c i2c-7: af9033: firmware version: LINK=12.5.13.0
OFDM=6.8.13.0
[112274.503415] usb 2-1.2.4: DVB: registering adapter 1 frontend 0
(Afatech AF9033 (DVB-T))...
[112274.503631] mxl5007t 7-00e0: creating new instance
[112274.509382] mxl5007t_get_chip_id: unknown rev (3f)
[112274.509394] mxl5007t_get_chip_id: MxL5007T detected @ 7-00e0
[112274.520989] usb 2-1.2.4: dvb_usb_v2: 'AVerMedia Twinstar (A825)'

successfully initialized and connected

The two adapters works quite reliably, adapters can perform scanning independently and at the same time, and all channels get recocgnized by both of them.

However, there are some sporadic image glitches while watching a channel on adapter 0 and at the same time performing a scanning on adapater 1. After the scan, though, glitches do not appear anymore. On the other hand, there are no image glithces at all watching a channel on adapter 1 and performing at the same time a scan on adapter0.

(quoted diff) +0 -0

b/drivers/media/usb/dvb-usb-v2/af9035.c
index a1e953a..c051083 100644

drivers/media/usb/dvb-usb-v2/af9035.c +3 -0

@@ -628,6 +628,7 @@ static int af9035_read_config(struct dvb_usb_device *d)
 		if (ret < 0)
 			goto err;
 
+		state->af9033_config[1].adc_multiplier = AF9033_ADC_MULTIPLIER_2X;
 		state->af9033_config[1].i2c_addr = tmp;
 		dev_dbg(&d->udev->dev, "%s: 2nd demod I2C addr=%02x\n",
 				__func__, tmp);

@@ -673,6 +674,8 @@ static int af9035_read_config(struct dvb_usb_device *d)
 			switch (tmp) {
 			case AF9033_TUNER_FC0012:
 				break;
+			case AF9033_TUNER_MXL5007T:
+				break;
 			default:
 				state->dual_mode = false;
 				dev_info(&d->udev->dev,

Malcolm Priestley

Acked-by

2013-01-31 20:29 UTC #23 lore raw

In reply to Antti Palosaari

Quoted 12 lines · Antti Palosaari

Quoted 10 lines · Andre Heider
Hi,
Quoted 2 lines · Antti Palosaari
Could you test that (tda18218 & mxl5007t):
http://git.linuxtv.org/anttip/media_tree.git/shortlog/refs/heads/it9135_tuner
I got a 'TerraTec Cinergy T Stick Dual RC (rev. 2)', which is fixed by this series.
Any chance to get this into 3.9 (I guess its too late for the USB VID/PID 'fix' for 3.8)?

Quoted 6 lines

Thank you for the report! There was someone else who reported it working too. Do you want to your name as tester for the changelog?
I just yesterday got that TerraTec device too and I am going to add dual tuner support. Also, for some reason IT9135 v2 devices are not working - only v1. That is one thing I should fix before merge that stuff.

Hi Antti,

I am going to acknowledge this development, so you are free to copy any code from the it913x and it913x-fe driver to get this working.

My time is very limited at the moment, so I will try to do testing when possible.

Once everything is stable enough the dvb-usb-it913x and it913x-fe modules can be removed.

Acked-by Malcolm Priestley <[email protected]>

Regards

Malcolm