diff mbox

[11/12] tm6000: bugfix firmware xc3028L-v36.fw used with Zarlink and DTV78 or DTV8 no shift

Message ID 1265411214-12231-11-git-send-email-stefan.ringel@arcor.de (mailing list archive)
State Superseded, archived
Headers

Commit Message

Stefan Ringel Feb. 5, 2010, 11:06 p.m. UTC 
  From: Stefan Ringel <stefan.ringel@arcor.de>

Signed-off-by: Stefan Ringel <stefan.ringel@arcor.de>
---
 drivers/media/common/tuners/tuner-xc2028.c |    7 ++++++-
 1 files changed, 6 insertions(+), 1 deletions(-)

Comments

Mauro Carvalho Chehab Feb. 8, 2010, 11:27 a.m. UTC | #1 
stefan.ringel@arcor.de wrote:
> From: Stefan Ringel <stefan.ringel@arcor.de>
> 
> Signed-off-by: Stefan Ringel <stefan.ringel@arcor.de>
> ---
>  drivers/media/common/tuners/tuner-xc2028.c |    7 ++++++-
>  1 files changed, 6 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/media/common/tuners/tuner-xc2028.c b/drivers/media/common/tuners/tuner-xc2028.c
> index ed50168..fcf19cc 100644
> --- a/drivers/media/common/tuners/tuner-xc2028.c
> +++ b/drivers/media/common/tuners/tuner-xc2028.c
> @@ -1114,7 +1114,12 @@ static int xc2028_set_params(struct dvb_frontend *fe,
>  
>  	/* All S-code tables need a 200kHz shift */
>  	if (priv->ctrl.demod) {
> -		demod = priv->ctrl.demod + 200;
> +		if ((strcmp (priv->ctrl.fname, "xc3028L-v36.fw") == 0) && 
> +			(priv->ctrl.demod == XC3028_FE_ZARLINK456) &&
> +				((type & DTV78) || (type & DTV8)))
> +			demod = priv->ctrl.demod;
> +		else
> +			demod = priv->ctrl.demod + 200;
>  		/*
>  		 * The DTV7 S-code table needs a 700 kHz shift.
>  		 * Thanks to Terry Wu <terrywu2009@gmail.com> for reporting this

The idea behind this patch is right, but you should be testing it against
priv->firm_version, instead comparing with a file name.

Also, this will likely cause regressions on other drivers, since the offsets for
v3.6 firmwares were handled on a different way on other drivers. I prefer to postpone
this patch and the discussion behind it after having tm6000 driver ready, since
it makes no sense to cause regressions or request changes on existing drivers due
to a driver that is not ready yet.

So, please hold your patch on your queue for now.

My suggestion is that you should use git and have this patch on a separate branch where you
do your tests, having a branch without this patch for upstream submission.
Stefan Ringel Feb. 8, 2010, 5:10 p.m. UTC | #2 
Am 08.02.2010 12:27, schrieb Mauro Carvalho Chehab:
> stefan.ringel@arcor.de wrote:
>   
>> From: Stefan Ringel <stefan.ringel@arcor.de>
>>
>> Signed-off-by: Stefan Ringel <stefan.ringel@arcor.de>
>> ---
>>  drivers/media/common/tuners/tuner-xc2028.c |    7 ++++++-
>>  1 files changed, 6 insertions(+), 1 deletions(-)
>>
>> diff --git a/drivers/media/common/tuners/tuner-xc2028.c b/drivers/media/common/tuners/tuner-xc2028.c
>> index ed50168..fcf19cc 100644
>> --- a/drivers/media/common/tuners/tuner-xc2028.c
>> +++ b/drivers/media/common/tuners/tuner-xc2028.c
>> @@ -1114,7 +1114,12 @@ static int xc2028_set_params(struct dvb_frontend *fe,
>>  
>>  	/* All S-code tables need a 200kHz shift */
>>  	if (priv->ctrl.demod) {
>> -		demod = priv->ctrl.demod + 200;
>> +		if ((strcmp (priv->ctrl.fname, "xc3028L-v36.fw") == 0) && 
>> +			(priv->ctrl.demod == XC3028_FE_ZARLINK456) &&
>> +				((type & DTV78) || (type & DTV8)))
>> +			demod = priv->ctrl.demod;
>> +		else
>> +			demod = priv->ctrl.demod + 200;
>>  		/*
>>  		 * The DTV7 S-code table needs a 700 kHz shift.
>>  		 * Thanks to Terry Wu <terrywu2009@gmail.com> for reporting this
>>     
> The idea behind this patch is right, but you should be testing it against
> priv->firm_version, instead comparing with a file name.
>
> Also, this will likely cause regressions on other drivers, since the offsets for
> v3.6 firmwares were handled on a different way on other drivers. I prefer to postpone
> this patch and the discussion behind it after having tm6000 driver ready, since
> it makes no sense to cause regressions or request changes on existing drivers due
> to a driver that is not ready yet.
>
> So, please hold your patch on your queue for now.
>
> My suggestion is that you should use git and have this patch on a separate branch where you
> do your tests, having a branch without this patch for upstream submission.
>
>   
In this firmware is for ZARLINK two parts, first for QAM, DTV6 and DTV7
with shift 200 kHz, and second for DTV78 and DTV8. I check the firmware
2.7 this use for ZARLINK for all this mode a 200 kHz shift. For the next
source part it says that DTV7 have 700 kHz shift.
That not for all firmware correct.
Mauro Carvalho Chehab Feb. 8, 2010, 5:56 p.m. UTC | #3 
Stefan Ringel wrote:
> Am 08.02.2010 12:27, schrieb Mauro Carvalho Chehab:
>> stefan.ringel@arcor.de wrote:
>>   
>>> From: Stefan Ringel <stefan.ringel@arcor.de>
>>>
>>> Signed-off-by: Stefan Ringel <stefan.ringel@arcor.de>
>>> ---
>>>  drivers/media/common/tuners/tuner-xc2028.c |    7 ++++++-
>>>  1 files changed, 6 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/drivers/media/common/tuners/tuner-xc2028.c b/drivers/media/common/tuners/tuner-xc2028.c
>>> index ed50168..fcf19cc 100644
>>> --- a/drivers/media/common/tuners/tuner-xc2028.c
>>> +++ b/drivers/media/common/tuners/tuner-xc2028.c
>>> @@ -1114,7 +1114,12 @@ static int xc2028_set_params(struct dvb_frontend *fe,
>>>  
>>>  	/* All S-code tables need a 200kHz shift */
>>>  	if (priv->ctrl.demod) {
>>> -		demod = priv->ctrl.demod + 200;
>>> +		if ((strcmp (priv->ctrl.fname, "xc3028L-v36.fw") == 0) && 
>>> +			(priv->ctrl.demod == XC3028_FE_ZARLINK456) &&
>>> +				((type & DTV78) || (type & DTV8)))
>>> +			demod = priv->ctrl.demod;
>>> +		else
>>> +			demod = priv->ctrl.demod + 200;
>>>  		/*
>>>  		 * The DTV7 S-code table needs a 700 kHz shift.
>>>  		 * Thanks to Terry Wu <terrywu2009@gmail.com> for reporting this
>>>     
>> The idea behind this patch is right, but you should be testing it against
>> priv->firm_version, instead comparing with a file name.
>>
>> Also, this will likely cause regressions on other drivers, since the offsets for
>> v3.6 firmwares were handled on a different way on other drivers. I prefer to postpone
>> this patch and the discussion behind it after having tm6000 driver ready, since
>> it makes no sense to cause regressions or request changes on existing drivers due
>> to a driver that is not ready yet.
>>
>> So, please hold your patch on your queue for now.
>>
>> My suggestion is that you should use git and have this patch on a separate branch where you
>> do your tests, having a branch without this patch for upstream submission.
>>
>>   
> In this firmware is for ZARLINK two parts, first for QAM, DTV6 and DTV7
> with shift 200 kHz, and second for DTV78 and DTV8. I check the firmware
> 2.7 this use for ZARLINK for all this mode a 200 kHz shift. For the next
> source part it says that DTV7 have 700 kHz shift.
> That not for all firmware correct.
> 
> 
From what we know, the name "zarlink" for the firmware is bogus: the firmware has nothing
special to work with zarlink, except for the IF offset. You may or select a firmware with
-200 KHz IF offset or to do the adjustment by adding 200 KHz for firmwares up to 2.7. 

The problem is that the driver that originally added the v3.6 implemented it on a different
place. So, we need to fix all the drivers at the patch that we're changing its behavior,
to avoid breakages.
diff mbox

Patch

diff --git a/drivers/media/common/tuners/tuner-xc2028.c b/drivers/media/common/tuners/tuner-xc2028.c
index ed50168..fcf19cc 100644
--- a/drivers/media/common/tuners/tuner-xc2028.c
+++ b/drivers/media/common/tuners/tuner-xc2028.c
@@ -1114,7 +1114,12 @@  static int xc2028_set_params(struct dvb_frontend *fe,
 
 	/* All S-code tables need a 200kHz shift */
 	if (priv->ctrl.demod) {
-		demod = priv->ctrl.demod + 200;
+		if ((strcmp (priv->ctrl.fname, "xc3028L-v36.fw") == 0) && 
+			(priv->ctrl.demod == XC3028_FE_ZARLINK456) &&
+				((type & DTV78) || (type & DTV8)))
+			demod = priv->ctrl.demod;
+		else
+			demod = priv->ctrl.demod + 200;
 		/*
 		 * The DTV7 S-code table needs a 700 kHz shift.
 		 * Thanks to Terry Wu <terrywu2009@gmail.com> for reporting this