af9035 test needed!

af9035 test needed!

[Antti Palosaari]

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

-- 
http://palosaari.fi/

Re: af9035 test needed!

[Jose Alberto Reguero]

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?
> 
> 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 <jareguero@telefonica.net>

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,
 	}
 };
 

 

Re: af9035 test needed!

[Antti Palosaari]

On 01/12/2013 01:45 AM, Jose Alberto Reguero wrote:
> 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?
>>
>> 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 <jareguero@telefonica.net>

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

regards
Antti

>
> 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,
>   	}
>   };
>
>
>
>


-- 
http://palosaari.fi/

Re: af9035 test needed!

[Jose Alberto Reguero]

On Sábado, 12 de enero de 2013 01:49:21 Antti Palosaari escribió:
> On 01/12/2013 01:45 AM, Jose Alberto Reguero wrote:
> > 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.
> >> 
> >> 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 <jareguero@telefonica.net>
> 
> 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
  
> > 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,
> > 
> >   	}
> >   
> >   };

Re: af9035 test needed!

[Jose Alberto Reguero]

On Sábado, 12 de enero de 2013 22:14:07 Jose Alberto Reguero escribió:
> On Sábado, 12 de enero de 2013 01:49:21 Antti Palosaari escribió:
> > On 01/12/2013 01:45 AM, Jose Alberto Reguero wrote:
> > > 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.
> > >> 
> > >> 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 <jareguero@telefonica.net>
> > 
> > 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
>   
> > > 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,
> > > 
> > >   	}
> > >   
> > >   };
> --

Sending again because some lines are mangled.

Changes  to make work Avermedia Twinstar with the af9035 driver.

Signed-off-by: Jose Alberto Reguero <jareguero@telefonica.net>

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,
 	}
 };
 




 

Re: af9035 test needed!

[Gianluca Gennari]

Il 11/01/2013 19:38, Antti Palosaari ha scritto:
> 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.

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.

> 
> 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

> 
> regards
> Antti
> 

Re: af9035 test needed!

[Andre Heider]

Hi,

On Fri, Jan 11, 2013 at 7:38 PM, Antti Palosaari <crope@iki.fi> wrote:
> 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

Re: af9035 test needed!

[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.

-Mike

On Thu, Jan 31, 2013 at 8:04 AM, Andre Heider <a.heider@gmail.com> wrote:
> Hi,
>
> On Fri, Jan 11, 2013 at 7:38 PM, Antti Palosaari <crope@iki.fi> wrote:
>> 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 majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: af9035 test needed!

[Antti Palosaari]

On 01/31/2013 03:04 PM, Andre Heider wrote:
> Hi,
>
> On Fri, Jan 11, 2013 at 7:38 PM, Antti Palosaari <crope@iki.fi> wrote:
>> 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

-- 
http://palosaari.fi/

Re: af9035 test needed!

[Andre Heider]

Hi,

On Thu, Jan 31, 2013 at 2:59 PM, Antti Palosaari <crope@iki.fi> wrote:
> 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 <a.heider@gmail.com>

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

> 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

Re: af9035 test needed!

[Andre Heider]

Hey,

On Thu, Jan 31, 2013 at 2:59 PM, Antti Palosaari <crope@iki.fi> wrote:
>> On Fri, Jan 11, 2013 at 7:38 PM, Antti Palosaari <crope@iki.fi> wrote:
>>>
>>> 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

Re: af9035 test needed!

[Antti Palosaari]

Jose, Gianluca,

On 01/31/2013 08:40 PM, Andre Heider wrote:
> Hey,
>
> On Thu, Jan 31, 2013 at 2:59 PM, Antti Palosaari <crope@iki.fi> wrote:
>>> On Fri, Jan 11, 2013 at 7:38 PM, Antti Palosaari <crope@iki.fi> wrote:
>>>>
>>>> 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

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


> 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
Antti

-- 
http://palosaari.fi/

Re: af9035 test needed!

[Malcolm Priestley]

On Thu, 2013-01-31 at 15:59 +0200, Antti Palosaari wrote:
> On 01/31/2013 03:04 PM, Andre Heider wrote:
> > Hi,
> >
> > On Fri, Jan 11, 2013 at 7:38 PM, Antti Palosaari <crope@iki.fi> wrote:
> >> 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.
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 <tvboxspy@gmail.com>

Regards


Malcolm

Re: af9035 test needed!

[Michael Krufky]

On Fri, Jan 11, 2013 at 6:45 PM, Jose Alberto Reguero
<jareguero@telefonica.net> wrote:
> 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?
>>
>> 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 <jareguero@telefonica.net>
>
> 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);
>

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?

> @@ -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?

>
>         /* 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);
> +
>

Can you explain why this change was made?  ^^

>                 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;

Why widen this boolean to three bits?

>         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,
>         }
>  };
>



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

Re: af9035 test needed!

[Jose Alberto Reguero]

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
> 
> <jareguero@telefonica.net> wrote:
> > 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.
> >> 
> >> 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 <jareguero@telefonica.net>
> > 
> > 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);
> 
> 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.

> > @@ -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?
>

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

> >         /* 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);
> > +
> 
> 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.
 
> >                 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;
> 
> 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

> >         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,
> > 
> >         }
> >  
> >  };
> 
> 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

Re: af9035 test needed!

[Antti Palosaari]

On 02/03/2013 02:04 PM, Jose Alberto Reguero wrote:
> 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
>>
>> <jareguero@telefonica.net> wrote:
>>> 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.
>>>>
>>>> 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 <jareguero@telefonica.net>
>>>
>>> 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);
>>
>> 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.
>
>>> @@ -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?
>>
>
> That is because with my hardware and dual tuner, when one tuner do reset, the
> other one is perturbed, and the stream has errors.
>
>>>          /* 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);
>>> +
>>
>> 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.
>
>>>                  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;
>>
>> 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
>
>>>          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,
>>>
>>>          }
>>>
>>>   };
>>
>> 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

-- 
http://palosaari.fi/

Re: af9035 test needed!

[Michael Krufky]

On Sun, Feb 3, 2013 at 8:21 AM, Antti Palosaari <crope@iki.fi> wrote:
> On 02/03/2013 02:04 PM, Jose Alberto Reguero wrote:
>>
>> 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
>>>
>>> <jareguero@telefonica.net> wrote:
>>>>
>>>> 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.
>>>>>
>>>>> 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 <jareguero@telefonica.net>
>>>>
>>>> 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);
>>>
>>>
>>> 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.
>>
>>>> @@ -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?
>>>
>>
>> That is because with my hardware and dual tuner, when one tuner do reset,
>> the
>> other one is perturbed, and the stream has errors.
>>
>>>>          /* 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);
>>>> +
>>>
>>>
>>> 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.
>>
>>>>                  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;
>>>
>>>
>>> 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
>>
>>>>          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,
>>>>
>>>>          }
>>>>
>>>>   };
>>>
>>>
>>> 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


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

Re: af9035 test needed!

[Antti Palosaari]

On 02/03/2013 09:27 PM, Michael Krufky wrote:
> On Sun, Feb 3, 2013 at 8:21 AM, Antti Palosaari <crope@iki.fi> wrote:
>> On 02/03/2013 02:04 PM, Jose Alberto Reguero wrote:
>>>
>>> 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
>>>>
>>>> <jareguero@telefonica.net> wrote:
>>>>>
>>>>> 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.
>>>>>>
>>>>>> 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 <jareguero@telefonica.net>
>>>>>
>>>>> 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);
>>>>
>>>>
>>>> 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.
>>>
>>>>> @@ -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?
>>>>
>>>
>>> That is because with my hardware and dual tuner, when one tuner do reset,
>>> the
>>> other one is perturbed, and the stream has errors.
>>>
>>>>>           /* 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);
>>>>> +
>>>>
>>>>
>>>> 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.
>>>
>>>>>                   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;
>>>>
>>>>
>>>> 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
>>>
>>>>>           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,
>>>>>
>>>>>           }
>>>>>
>>>>>    };
>>>>
>>>>
>>>> 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
>
>
> 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
>

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

-- 
http://palosaari.fi/

Re: af9035 test needed!

[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)

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 :-/

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.

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 :)
>>

-Mike

Re: af9035 test needed!

[Antti Palosaari]

On 02/03/2013 09:53 PM, Michael Krufky wrote:
> (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.

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!


> 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.


> 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.

> 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 :)
>>>
>
> -Mike
>

Antti

-- 
http://palosaari.fi/

Re: af9035 test needed!

[Michael Krufky]

On Sun, Feb 3, 2013 at 3:29 PM, Antti Palosaari <crope@iki.fi> wrote:
> On 02/03/2013 09:53 PM, Michael Krufky wrote:
>>
>> (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.
>
>
> 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!
>
>
>
>> 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.
>
>
>
>> 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.
>
>
>> 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 :)
>>>>
>>>>
>>
>> -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

Re: af9035 test needed!

[Andre Heider]

Hi,

On Thu, Jan 31, 2013 at 2:46 PM, Michael Krufky <mkrufky@linuxtv.org> wrote:
> 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

Re: af9035 test needed!

[Gianluca Gennari]

Il 31/01/2013 19:52, Antti Palosaari ha scritto:
> Jose, Gianluca,
> 
> On 01/31/2013 08:40 PM, Andre Heider wrote:
>> Hey,
>>
>> On Thu, Jan 31, 2013 at 2:59 PM, Antti Palosaari <crope@iki.fi> wrote:
>>>> On Fri, Jan 11, 2013 at 7:38 PM, Antti Palosaari <crope@iki.fi> wrote:
>>>>>
>>>>> 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
> 
> 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

> 
> 
>> 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
> Antti
> 

Re: af9035 test needed!

[pierigno]

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.



diff --git a/drivers/media/usb/dvb-usb-v2/af9035.c
b/drivers/media/usb/dvb-usb-v2/af9035.c
index a1e953a..c051083 100644
--- a/drivers/media/usb/dvb-usb-v2/af9035.c
+++ b/drivers/media/usb/dvb-usb-v2/af9035.c
@@ -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,