Re: [PATCH 1/4] tda18271_set_analog_params major bugfix
On Thu, Sep 24, 2009 at 5:42 PM, s...@systol-ng.god.lan wrote:
On Thu, Sep 24, 2009 at 02:46:06PM -0400, Michael Krufky wrote:
On Tue, Sep 22, 2009 at 5:05 PM, s...@systol-ng.god.lan wrote:
Multiplication by 62500 causes an overflow in the 32 bits freq register
when
using radio. FM radio reception on a Zolid Hybrid PCI is now working. Other
tda18271 configurations may also benefit from this change ;)
Signed-off-by: henk.vergo...@gmail.com
diff -r 29e4ba1a09bc linux/drivers/media/common/tuners/tda18271-fe.c
...
- freq = freq / 1000;
+ freq = params-frequency * 625;
+ freq = freq / 10;
Hmm now that I review my own patch:
- freq = freq / 1000;
+ freq = params-frequency * 125;
+ freq = freq / 2;
might even be better...
Henk,
That certainly is better, but I am going to go with an even simpler
cleaner approach:
diff -r f52640ced9e8 linux/drivers/media/common/tuners/tda18271-fe.c
--- a/linux/drivers/media/common/tuners/tda18271-fe.c Tue Sep 15
01:25:35 2009 -0400
+++ b/linux/drivers/media/common/tuners/tda18271-fe.c Sun Sep 27
12:21:37 2009 -0400
@@ -1001,12 +1001,12 @@
struct tda18271_std_map_item *map;
char *mode;
int ret;
- u32 freq = params-frequency * 62500;
+ u32 freq = params-frequency *
+ ((params-mode == V4L2_TUNER_RADIO) ? 125 / 2 : 62500);
priv-mode = TDA18271_ANALOG;
if (params-mode == V4L2_TUNER_RADIO) {
- freq = freq / 1000;
map = std_map-fm_radio;
mode = fm;
} else if (params-std V4L2_STD_MN) {
You still get the credit for spotting the problem providing the
original fix -- thanks again! I'm going to push this along with the
others today.
Cheers,
Mike
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/4] tda18271_set_analog_params major bugfix
On Sun, Sep 27, 2009 at 12:35:00PM -0400, Michael Krufky wrote:
On Sun, Sep 27, 2009 at 12:25 PM, Michael Krufky mkru...@kernellabs.com
wrote:
On a second thought, I see that my above patch loses some precision
... this is even better:
diff -r f52640ced9e8 linux/drivers/media/common/tuners/tda18271-fe.c
--- a/linux/drivers/media/common/tuners/tda18271-fe.c Tue Sep 15
01:25:35 2009 -0400
+++ b/linux/drivers/media/common/tuners/tda18271-fe.c Sun Sep 27
12:33:20 2009 -0400
@@ -1001,12 +1001,12 @@
struct tda18271_std_map_item *map;
char *mode;
int ret;
- u32 freq = params-frequency * 62500;
+ u32 freq = params-frequency * 125 *
+ ((params-mode == V4L2_TUNER_RADIO) ? 1 : 1000) / 2;
priv-mode = TDA18271_ANALOG;
if (params-mode == V4L2_TUNER_RADIO) {
- freq = freq / 1000;
map = std_map-fm_radio;
mode = fm;
} else if (params-std V4L2_STD_MN) {
Cheers,
Mike
Much better!
Btw. It seems that the tuner is capable of tuning in 1000 Hz steps, is
there a reason why we are using 62500 Hz steps?
Regards,
Henk
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/4] tda18271_set_analog_params major bugfix
On Sun, Sep 27, 2009 at 4:24 PM, s...@systol-ng.god.lan wrote:
On Sun, Sep 27, 2009 at 12:35:00PM -0400, Michael Krufky wrote:
On Sun, Sep 27, 2009 at 12:25 PM, Michael Krufky mkru...@kernellabs.com
wrote:
On a second thought, I see that my above patch loses some precision
... this is even better:
diff -r f52640ced9e8 linux/drivers/media/common/tuners/tda18271-fe.c
--- a/linux/drivers/media/common/tuners/tda18271-fe.c Tue Sep 15
01:25:35 2009 -0400
+++ b/linux/drivers/media/common/tuners/tda18271-fe.c Sun Sep 27
12:33:20 2009 -0400
@@ -1001,12 +1001,12 @@
struct tda18271_std_map_item *map;
char *mode;
int ret;
- u32 freq = params-frequency * 62500;
+ u32 freq = params-frequency * 125 *
+ ((params-mode == V4L2_TUNER_RADIO) ? 1 : 1000) / 2;
priv-mode = TDA18271_ANALOG;
if (params-mode == V4L2_TUNER_RADIO) {
- freq = freq / 1000;
map = std_map-fm_radio;
mode = fm;
} else if (params-std V4L2_STD_MN) {
Cheers,
Mike
Much better!
Btw. It seems that the tuner is capable of tuning in 1000 Hz steps, is
there a reason why we are using 62500 Hz steps?
That's the v4l2 analog tuner API. It has nothing to do with the
tda18271 driver internals. If you look on the digital set_params
function, you'll see that this doesnt happen there.
-Mike
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/4] tda18271_set_analog_params major bugfix
On Tue, Sep 22, 2009 at 5:05 PM, s...@systol-ng.god.lan wrote:
Multiplication by 62500 causes an overflow in the 32 bits freq register when
using radio. FM radio reception on a Zolid Hybrid PCI is now working. Other
tda18271 configurations may also benefit from this change ;)
Signed-off-by: henk.vergo...@gmail.com
diff -r 29e4ba1a09bc linux/drivers/media/common/tuners/tda18271-fe.c
--- a/linux/drivers/media/common/tuners/tda18271-fe.c Sat Sep 19 09:45:22
2009 -0300
+++ b/linux/drivers/media/common/tuners/tda18271-fe.c Tue Sep 22 22:06:31
2009 +0200
@@ -1001,38 +1020,43 @@
struct tda18271_std_map_item *map;
char *mode;
int ret;
- u32 freq = params-frequency * 62500;
+ u32 freq;
priv-mode = TDA18271_ANALOG;
if (params-mode == V4L2_TUNER_RADIO) {
- freq = freq / 1000;
+ freq = params-frequency * 625;
+ freq = freq / 10;
map = std_map-fm_radio;
mode = fm;
- } else if (params-std V4L2_STD_MN) {
- map = std_map-atv_mn;
- mode = MN;
- } else if (params-std V4L2_STD_B) {
- map = std_map-atv_b;
- mode = B;
- } else if (params-std V4L2_STD_GH) {
- map = std_map-atv_gh;
- mode = GH;
- } else if (params-std V4L2_STD_PAL_I) {
- map = std_map-atv_i;
- mode = I;
- } else if (params-std V4L2_STD_DK) {
- map = std_map-atv_dk;
- mode = DK;
- } else if (params-std V4L2_STD_SECAM_L) {
- map = std_map-atv_l;
- mode = L;
- } else if (params-std V4L2_STD_SECAM_LC) {
- map = std_map-atv_lc;
- mode = L';
} else {
- map = std_map-atv_i;
- mode = xx;
+ freq = params-frequency * 62500;
+
+ if (params-std V4L2_STD_MN) {
+ map = std_map-atv_mn;
+ mode = MN;
+ } else if (params-std V4L2_STD_B) {
+ map = std_map-atv_b;
+ mode = B;
+ } else if (params-std V4L2_STD_GH) {
+ map = std_map-atv_gh;
+ mode = GH;
+ } else if (params-std V4L2_STD_PAL_I) {
+ map = std_map-atv_i;
+ mode = I;
+ } else if (params-std V4L2_STD_DK) {
+ map = std_map-atv_dk;
+ mode = DK;
+ } else if (params-std V4L2_STD_SECAM_L) {
+ map = std_map-atv_l;
+ mode = L;
+ } else if (params-std V4L2_STD_SECAM_LC) {
+ map = std_map-atv_lc;
+ mode = L';
+ } else {
+ map = std_map-atv_i;
+ mode = xx;
+ }
}
tda_dbg(setting tda18271 to system %s\n, mode);
Nice catch, Henk -- Thank you for this fix... I will have this one
merged as soon as I can.
Signed-off-by: Michael Krufky mkru...@kernellabs.com
Mauro, please do not merge the tda18271 / tda829x patches until I send
you a pull request -- there is a patch-order dependency with other
pending changes and I would prefer to send this to you in the proper
order.
Thanks,
Mike Krufky
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/4] tda18271_set_analog_params major bugfix
On Thu, Sep 24, 2009 at 02:46:06PM -0400, Michael Krufky wrote: On Tue, Sep 22, 2009 at 5:05 PM, s...@systol-ng.god.lan wrote: Multiplication by 62500 causes an overflow in the 32 bits freq register when using radio. FM radio reception on a Zolid Hybrid PCI is now working. Other tda18271 configurations may also benefit from this change ;) Signed-off-by: henk.vergo...@gmail.com diff -r 29e4ba1a09bc linux/drivers/media/common/tuners/tda18271-fe.c ... - freq = freq / 1000; + freq = params-frequency * 625; + freq = freq / 10; Hmm now that I review my own patch: - freq = freq / 1000; + freq = params-frequency * 125; + freq = freq / 2; might even be better... regards henk -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/4] tda18271_set_analog_params major bugfix
Multiplication by 62500 causes an overflow in the 32 bits freq register when
using radio. FM radio reception on a Zolid Hybrid PCI is now working. Other
tda18271 configurations may also benefit from this change ;)
Signed-off-by: henk.vergo...@gmail.com
diff -r 29e4ba1a09bc linux/drivers/media/common/tuners/tda18271-fe.c
--- a/linux/drivers/media/common/tuners/tda18271-fe.c Sat Sep 19 09:45:22
2009 -0300
+++ b/linux/drivers/media/common/tuners/tda18271-fe.c Tue Sep 22 22:06:31
2009 +0200
@@ -1001,38 +1020,43 @@
struct tda18271_std_map_item *map;
char *mode;
int ret;
- u32 freq = params-frequency * 62500;
+ u32 freq;
priv-mode = TDA18271_ANALOG;
if (params-mode == V4L2_TUNER_RADIO) {
- freq = freq / 1000;
+ freq = params-frequency * 625;
+ freq = freq / 10;
map = std_map-fm_radio;
mode = fm;
- } else if (params-std V4L2_STD_MN) {
- map = std_map-atv_mn;
- mode = MN;
- } else if (params-std V4L2_STD_B) {
- map = std_map-atv_b;
- mode = B;
- } else if (params-std V4L2_STD_GH) {
- map = std_map-atv_gh;
- mode = GH;
- } else if (params-std V4L2_STD_PAL_I) {
- map = std_map-atv_i;
- mode = I;
- } else if (params-std V4L2_STD_DK) {
- map = std_map-atv_dk;
- mode = DK;
- } else if (params-std V4L2_STD_SECAM_L) {
- map = std_map-atv_l;
- mode = L;
- } else if (params-std V4L2_STD_SECAM_LC) {
- map = std_map-atv_lc;
- mode = L';
} else {
- map = std_map-atv_i;
- mode = xx;
+ freq = params-frequency * 62500;
+
+ if (params-std V4L2_STD_MN) {
+ map = std_map-atv_mn;
+ mode = MN;
+ } else if (params-std V4L2_STD_B) {
+ map = std_map-atv_b;
+ mode = B;
+ } else if (params-std V4L2_STD_GH) {
+ map = std_map-atv_gh;
+ mode = GH;
+ } else if (params-std V4L2_STD_PAL_I) {
+ map = std_map-atv_i;
+ mode = I;
+ } else if (params-std V4L2_STD_DK) {
+ map = std_map-atv_dk;
+ mode = DK;
+ } else if (params-std V4L2_STD_SECAM_L) {
+ map = std_map-atv_l;
+ mode = L;
+ } else if (params-std V4L2_STD_SECAM_LC) {
+ map = std_map-atv_lc;
+ mode = L';
+ } else {
+ map = std_map-atv_i;
+ mode = xx;
+ }
}
tda_dbg(setting tda18271 to system %s\n, mode);
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
