* [PATCH 0/2] fix tunning for r820t tunner
@ 2013-09-29 10:45 Jiří Pinkava
2013-09-29 10:45 ` [PATCH 1/2] [media] r820t: fix nint range check Jiří Pinkava
0 siblings, 1 reply; 8+ messages in thread
From: Jiří Pinkava @ 2013-09-29 10:45 UTC (permalink / raw
To: gennarone, m.chehab; +Cc: mkrufky, linux-media
Fixes range check for VCO parameters, simplifies calculation of divisor.
drivers/media/tuners/r820t.c | 8 ++------
1 file changed, 2 insertions(+), 6 deletions(-)
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 1/2] [media] r820t: fix nint range check
2013-09-29 10:45 [PATCH 0/2] fix tunning for r820t tunner Jiří Pinkava
@ 2013-09-29 10:45 ` Jiří Pinkava
2013-09-29 10:46 ` Subject: [PATCH 2/2] [media] r820t: simplify divisor calculation Jiří Pinkava
2013-09-30 15:07 ` [PATCH 1/2] [media] r820t: fix nint range check Michael Krufky
0 siblings, 2 replies; 8+ messages in thread
From: Jiří Pinkava @ 2013-09-29 10:45 UTC (permalink / raw
To: gennarone, m.chehab; +Cc: mkrufky, linux-media
Use full range of VCO parameters, fixes tunning for some frequencies.
---
drivers/media/tuners/r820t.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/media/tuners/r820t.c b/drivers/media/tuners/r820t.c
index 1c23666..e25c720 100644
--- a/drivers/media/tuners/r820t.c
+++ b/drivers/media/tuners/r820t.c
@@ -637,7 +637,7 @@ static int r820t_set_pll(struct r820t_priv *priv,
enum v4l2_tuner_type type,
vco_fra = pll_ref * 129 / 128;
}
- if (nint > 63) {
+ if (nint > 76) {
tuner_info("No valid PLL values for %u kHz!\n", freq);
return -EINVAL;
}
--
1.8.3.2
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Subject: [PATCH 2/2] [media] r820t: simplify divisor calculation
2013-09-29 10:45 ` [PATCH 1/2] [media] r820t: fix nint range check Jiří Pinkava
@ 2013-09-29 10:46 ` Jiří Pinkava
2013-09-30 15:13 ` Michael Krufky
2013-09-30 15:07 ` [PATCH 1/2] [media] r820t: fix nint range check Michael Krufky
1 sibling, 1 reply; 8+ messages in thread
From: Jiří Pinkava @ 2013-09-29 10:46 UTC (permalink / raw
To: gennarone, m.chehab; +Cc: mkrufky, linux-media
---
drivers/media/tuners/r820t.c | 6 +-----
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/drivers/media/tuners/r820t.c b/drivers/media/tuners/r820t.c
index e25c720..36dc63e 100644
--- a/drivers/media/tuners/r820t.c
+++ b/drivers/media/tuners/r820t.c
@@ -596,13 +596,9 @@ static int r820t_set_pll(struct r820t_priv *priv,
enum v4l2_tuner_type type,
while (mix_div <= 64) {
if (((freq * mix_div) >= vco_min) &&
((freq * mix_div) < vco_max)) {
- div_buf = mix_div;
- while (div_buf > 2) {
- div_buf = div_buf >> 1;
- div_num++;
- }
break;
}
+ ++div_num;
mix_div = mix_div << 1;
}
--
1.8.3.2
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] [media] r820t: fix nint range check
2013-09-29 10:45 ` [PATCH 1/2] [media] r820t: fix nint range check Jiří Pinkava
2013-09-29 10:46 ` Subject: [PATCH 2/2] [media] r820t: simplify divisor calculation Jiří Pinkava
@ 2013-09-30 15:07 ` Michael Krufky
2013-09-30 16:42 ` Jiří Pinkava
1 sibling, 1 reply; 8+ messages in thread
From: Michael Krufky @ 2013-09-30 15:07 UTC (permalink / raw
To: Jiří Pinkava
Cc: Gianluca Gennari, Mauro Carvalho Chehab, linux-media
Jiří,
Do you have any documentation that supports this value change?
Changing this value affects the algorithm, and we'd be happier making
this change if the patch included some better description and perhaps
a reference explaining why the new value is correct.
Regards,
Mike Krufky
On Sun, Sep 29, 2013 at 6:45 AM, Jiří Pinkava <j-pi@seznam.cz> wrote:
>
>
> Use full range of VCO parameters, fixes tunning for some frequencies.
> ---
> drivers/media/tuners/r820t.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/media/tuners/r820t.c b/drivers/media/tuners/r820t.c
> index 1c23666..e25c720 100644
> --- a/drivers/media/tuners/r820t.c
> +++ b/drivers/media/tuners/r820t.c
> @@ -637,7 +637,7 @@ static int r820t_set_pll(struct r820t_priv *priv,
> enum v4l2_tuner_type type,
> vco_fra = pll_ref * 129 / 128;
> }
>
> - if (nint > 63) {
> + if (nint > 76) {
> tuner_info("No valid PLL values for %u kHz!\n", freq);
> return -EINVAL;
> }
> --
> 1.8.3.2
>
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: Subject: [PATCH 2/2] [media] r820t: simplify divisor calculation
2013-09-29 10:46 ` Subject: [PATCH 2/2] [media] r820t: simplify divisor calculation Jiří Pinkava
@ 2013-09-30 15:13 ` Michael Krufky
0 siblings, 0 replies; 8+ messages in thread
From: Michael Krufky @ 2013-09-30 15:13 UTC (permalink / raw
To: Jiří Pinkava
Cc: Gianluca Gennari, Mauro Carvalho Chehab, linux-media
Jiří,
It's difficult to understand a patch like this, let alone merging it,
without any kind of explanation. At a quick glance, the patch looks
wrong. If you believe the patch is correct, then please resubmit with
some sort of description and explanation for the change.
Best regards,
Mike Krufky
On Sun, Sep 29, 2013 at 6:46 AM, Jiří Pinkava <j-pi@seznam.cz> wrote:
>
> ---
> drivers/media/tuners/r820t.c | 6 +-----
> 1 file changed, 1 insertion(+), 5 deletions(-)
>
> diff --git a/drivers/media/tuners/r820t.c b/drivers/media/tuners/r820t.c
> index e25c720..36dc63e 100644
> --- a/drivers/media/tuners/r820t.c
> +++ b/drivers/media/tuners/r820t.c
> @@ -596,13 +596,9 @@ static int r820t_set_pll(struct r820t_priv *priv,
> enum v4l2_tuner_type type,
> while (mix_div <= 64) {
> if (((freq * mix_div) >= vco_min) &&
> ((freq * mix_div) < vco_max)) {
> - div_buf = mix_div;
> - while (div_buf > 2) {
> - div_buf = div_buf >> 1;
> - div_num++;
> - }
> break;
> }
> + ++div_num;
> mix_div = mix_div << 1;
> }
>
> --
> 1.8.3.2
>
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] [media] r820t: fix nint range check
2013-09-30 15:07 ` [PATCH 1/2] [media] r820t: fix nint range check Michael Krufky
@ 2013-09-30 16:42 ` Jiří Pinkava
2013-09-30 16:46 ` Michael Krufky
0 siblings, 1 reply; 8+ messages in thread
From: Jiří Pinkava @ 2013-09-30 16:42 UTC (permalink / raw
To: Michael Krufky; +Cc: Gianluca Gennari, Mauro Carvalho Chehab, linux-media
Mike,
unfortunately no documentation can be referenced except preliminary
version of
datasheet (1).This change is based on lucky guess and supported by lot of
testing on real hardware.
This change add support for devices with Xtal frequency bellow 28.8MHz.
>From Nint are computed values of Ni and Si. For 28.8MHz crystal can
reach up to 12 / 3 (Ni / Si). Tuner supports crystals with frequencies
(1) 12, 16, 20, 20.48, 24, 27, 28.8, 32 MHz, but this kind of device is
rare to found.
Allowing Ni to go up to 15 instead of only 12 should be safe and for 15
/ 3 (Ni / Si)
we can compute limit for Nint = max(Ni) * 4 + max(Si) + 13 = 76.
If This is sufficient and acceptable explanation I can add some sort of
documentation into patch and resend it (both patches, I can prove I'm
right :)
(1)
http://rtl-sdr.com/wp-content/uploads/2013/04/R820T_datasheet-Non_R-20111130_unlocked.pdf
> Jiří,
>
> Do you have any documentation that supports this value change?
> Changing this value affects the algorithm, and we'd be happier making
> this change if the patch included some better description and perhaps
> a reference explaining why the new value is correct.
>
> Regards,
>
> Mike Krufky
>
> On Sun, Sep 29, 2013 at 6:45 AM, Jiří Pinkava <j-pi@seznam.cz> wrote:
>>
>> Use full range of VCO parameters, fixes tunning for some frequencies.
>> ---
>> drivers/media/tuners/r820t.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/media/tuners/r820t.c b/drivers/media/tuners/r820t.c
>> index 1c23666..e25c720 100644
>> --- a/drivers/media/tuners/r820t.c
>> +++ b/drivers/media/tuners/r820t.c
>> @@ -637,7 +637,7 @@ static int r820t_set_pll(struct r820t_priv *priv,
>> enum v4l2_tuner_type type,
>> vco_fra = pll_ref * 129 / 128;
>> }
>>
>> - if (nint > 63) {
>> + if (nint > 76) {
>> tuner_info("No valid PLL values for %u kHz!\n", freq);
>> return -EINVAL;
>> }
>> --
>> 1.8.3.2
>>
>>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] [media] r820t: fix nint range check
2013-09-30 16:42 ` Jiří Pinkava
@ 2013-09-30 16:46 ` Michael Krufky
2013-09-30 17:34 ` Mauro Carvalho Chehab
0 siblings, 1 reply; 8+ messages in thread
From: Michael Krufky @ 2013-09-30 16:46 UTC (permalink / raw
To: Jiří Pinkava
Cc: Gianluca Gennari, Mauro Carvalho Chehab, linux-media
Do you have any comments on this, Mauro?
Assuming that Mauro is OK with this change, (since he is the author of
this driver) then yes - please resubmit the patch with some
explanation within comments inline or within the commit message.
Best regards,
Mike Krufky
On Mon, Sep 30, 2013 at 12:42 PM, Jiří Pinkava <j-pi@seznam.cz> wrote:
> Mike,
>
> unfortunately no documentation can be referenced except preliminary
> version of
> datasheet (1).This change is based on lucky guess and supported by lot of
> testing on real hardware.
>
> This change add support for devices with Xtal frequency bellow 28.8MHz.
>
> From Nint are computed values of Ni and Si. For 28.8MHz crystal can
> reach up to 12 / 3 (Ni / Si). Tuner supports crystals with frequencies
> (1) 12, 16, 20, 20.48, 24, 27, 28.8, 32 MHz, but this kind of device is
> rare to found.
> Allowing Ni to go up to 15 instead of only 12 should be safe and for 15
> / 3 (Ni / Si)
> we can compute limit for Nint = max(Ni) * 4 + max(Si) + 13 = 76.
>
> If This is sufficient and acceptable explanation I can add some sort of
> documentation into patch and resend it (both patches, I can prove I'm
> right :)
>
> (1)
> http://rtl-sdr.com/wp-content/uploads/2013/04/R820T_datasheet-Non_R-20111130_unlocked.pdf
>
>> Jiří,
>>
>> Do you have any documentation that supports this value change?
>> Changing this value affects the algorithm, and we'd be happier making
>> this change if the patch included some better description and perhaps
>> a reference explaining why the new value is correct.
>>
>> Regards,
>>
>> Mike Krufky
>>
>> On Sun, Sep 29, 2013 at 6:45 AM, Jiří Pinkava <j-pi@seznam.cz> wrote:
>>>
>>> Use full range of VCO parameters, fixes tunning for some frequencies.
>>> ---
>>> drivers/media/tuners/r820t.c | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/media/tuners/r820t.c b/drivers/media/tuners/r820t.c
>>> index 1c23666..e25c720 100644
>>> --- a/drivers/media/tuners/r820t.c
>>> +++ b/drivers/media/tuners/r820t.c
>>> @@ -637,7 +637,7 @@ static int r820t_set_pll(struct r820t_priv *priv,
>>> enum v4l2_tuner_type type,
>>> vco_fra = pll_ref * 129 / 128;
>>> }
>>>
>>> - if (nint > 63) {
>>> + if (nint > 76) {
>>> tuner_info("No valid PLL values for %u kHz!\n", freq);
>>> return -EINVAL;
>>> }
>>> --
>>> 1.8.3.2
>>>
>>>
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] [media] r820t: fix nint range check
2013-09-30 16:46 ` Michael Krufky
@ 2013-09-30 17:34 ` Mauro Carvalho Chehab
0 siblings, 0 replies; 8+ messages in thread
From: Mauro Carvalho Chehab @ 2013-09-30 17:34 UTC (permalink / raw
To: Michael Krufky, Jiří Pinkava; +Cc: Gianluca Gennari, linux-media
Em Mon, 30 Sep 2013 12:46:52 -0400
Michael Krufky <mkrufky@linuxtv.org> escreveu:
> Do you have any comments on this, Mauro?
>
> Assuming that Mauro is OK with this change, (since he is the author of
> this driver) then yes - please resubmit the patch with some
> explanation within comments inline or within the commit message.
Michael,
Please don't top post. My comments are below.
>
> Best regards,
>
> Mike Krufky
>
> On Mon, Sep 30, 2013 at 12:42 PM, Jiří Pinkava <j-pi@seznam.cz> wrote:
> > Mike,
> >
> > unfortunately no documentation can be referenced except preliminary
> > version of
> > datasheet (1).This change is based on lucky guess and supported by lot of
> > testing on real hardware.
> >
> > This change add support for devices with Xtal frequency bellow 28.8MHz.
Hmm... I'm not seeing any test there checking if the frequency of the xtal
is below to 28.8 MHz. So, if this logic apples only in this case, the
patch needs to add a test for the xtal frequency.
Also, as the driver does some tests for other Raphael tuners, if this change
is known to work fine only on rt820t, please add a test there for the tuner
model too.
Except for that, I'm ok with the changes, provided that you add the
explanations about it.
> >
> > From Nint are computed values of Ni and Si. For 28.8MHz crystal can
> > reach up to 12 / 3 (Ni / Si). Tuner supports crystals with frequencies
> > (1) 12, 16, 20, 20.48, 24, 27, 28.8, 32 MHz, but this kind of device is
> > rare to found.
> > Allowing Ni to go up to 15 instead of only 12 should be safe and for 15
> > / 3 (Ni / Si)
> > we can compute limit for Nint = max(Ni) * 4 + max(Si) + 13 = 76.
> >
> > If This is sufficient and acceptable explanation I can add some sort of
> > documentation into patch and resend it (both patches, I can prove I'm
> > right :)
> >
> > (1)
> > http://rtl-sdr.com/wp-content/uploads/2013/04/R820T_datasheet-Non_R-20111130_unlocked.pdf
> >
> >> Jiří,
> >>
> >> Do you have any documentation that supports this value change?
> >> Changing this value affects the algorithm, and we'd be happier making
> >> this change if the patch included some better description and perhaps
> >> a reference explaining why the new value is correct.
> >>
> >> Regards,
> >>
> >> Mike Krufky
> >>
> >> On Sun, Sep 29, 2013 at 6:45 AM, Jiří Pinkava <j-pi@seznam.cz> wrote:
> >>>
> >>> Use full range of VCO parameters, fixes tunning for some frequencies.
> >>> ---
> >>> drivers/media/tuners/r820t.c | 2 +-
> >>> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>>
> >>> diff --git a/drivers/media/tuners/r820t.c b/drivers/media/tuners/r820t.c
> >>> index 1c23666..e25c720 100644
> >>> --- a/drivers/media/tuners/r820t.c
> >>> +++ b/drivers/media/tuners/r820t.c
> >>> @@ -637,7 +637,7 @@ static int r820t_set_pll(struct r820t_priv *priv,
> >>> enum v4l2_tuner_type type,
> >>> vco_fra = pll_ref * 129 / 128;
> >>> }
> >>>
> >>> - if (nint > 63) {
> >>> + if (nint > 76) {
> >>> tuner_info("No valid PLL values for %u kHz!\n", freq);
> >>> return -EINVAL;
> >>> }
> >>> --
> >>> 1.8.3.2
> >>>
> >>>
> >
--
Cheers,
Mauro
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2013-09-30 17:34 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-09-29 10:45 [PATCH 0/2] fix tunning for r820t tunner Jiří Pinkava
2013-09-29 10:45 ` [PATCH 1/2] [media] r820t: fix nint range check Jiří Pinkava
2013-09-29 10:46 ` Subject: [PATCH 2/2] [media] r820t: simplify divisor calculation Jiří Pinkava
2013-09-30 15:13 ` Michael Krufky
2013-09-30 15:07 ` [PATCH 1/2] [media] r820t: fix nint range check Michael Krufky
2013-09-30 16:42 ` Jiří Pinkava
2013-09-30 16:46 ` Michael Krufky
2013-09-30 17:34 ` Mauro Carvalho Chehab
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.