* Re: [linux-dvb] Leadtek WinFast DTV-1800H support
2008-10-20 18:45 ` Steven Toth
@ 2008-10-22 19:29 ` Miroslav Šustek
2008-10-22 19:50 ` Miroslav Šustek
1 sibling, 0 replies; 4+ messages in thread
From: Miroslav Šustek @ 2008-10-22 19:29 UTC (permalink / raw)
To: linux-dvb
[-- Attachment #1: Type: text/plain, Size: 1662 bytes --]
>sustmi wrote:
>> Hi,
...
>> http://www.linuxtv.org/pipermail/linux-dvb/2008-August/028117.html
>> Why is using of cx_write() risky?
>
> From my orig email: "Don't call cx_write() inside the gpio card setup, you're potentially destroying the other bits, it's risky."
>
>cx_write destroys the content of the GPIO direction-enablement and values bits. That's a bad thing, and can lead to unexpected behaviors if used generally in drivers.
>
>It's better to have a driver read the previous register value, and/or in the appropriate bit and write the value back to the gpio registers. This is what cx_set/clear do. That way parts of the driver can toggle GPIO's for important pieces, without having to understand GPIO usage in other disconnected/unrelated parts of the driver.
>
>Your patch uses set/clear, which is good.
>
>From datasheet (www.domenech.org/datasheets/cx23880-cx23881-cx23882-cx23883-datasheet-08-2002.pdf),
Chapter 2.3.3:
GP{x}_IO register:
bits [23:16] = GP{x}_BWE
"If this field is equal to 8’h00, then the whole GPIO byte
register operates in normal RW mode. If any bit is set, then
the corresponding GP_OE and GP_IO bit locations are
enabled for writing. If the bit write enable is not set, the
corresponding GPIO bits will be unaffected."
So I think this is just like (this is) mask. I don't see any possibility of unexpected behavior.
You only have to know which bits you want change (=how to set the mask)...
Ok, persuaded me, using cx_set and cx_clear is prettier :) .
>Incidently, you patch cannot be merged as-is, it has C99 style comments '//' are not allowed, change to /* */ etc.
>
>- Steve
>
Of course, new patch is attached.
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: leadtek_winfast_dtv1800h_2.patch --]
[-- Type: text/x-patch; name="leadtek_winfast_dtv1800h_2.patch", Size: 4848 bytes --]
diff -r e2a8b9b9c294 linux/drivers/media/video/cx88/cx88-cards.c
--- a/linux/drivers/media/video/cx88/cx88-cards.c Fri Oct 17 19:45:55 2008 +0300
+++ b/linux/drivers/media/video/cx88/cx88-cards.c Wed Oct 22 20:22:24 2008 +0200
@@ -1904,6 +1904,47 @@
} },
.mpeg = CX88_MPEG_DVB,
},
+ [CX88_BOARD_WINFAST_DTV1800H] = {
+ .name = "Leadtek WinFast DTV1800 Hybrid",
+ .tuner_type = TUNER_XC2028,
+ .radio_type = TUNER_XC2028,
+ .tuner_addr = 0x61,
+ .radio_addr = 0x61,
+ /*
+ * GPIO setting
+ *
+ * 2: mute (0=off,1=on)
+ * 12: tuner reset pin
+ * 13: audio source (0=tuner audio,1=line in)
+ * 14: FM (0=on,1=off ???)
+ */
+ .input = {{
+ .type = CX88_VMUX_TELEVISION,
+ .vmux = 0,
+ .gpio0 = 0x0400, /* pin 2 = 0 */
+ .gpio1 = 0x6040, /* pin 13 = 0, pin 14 = 1 */
+ .gpio2 = 0x0000,
+ },{
+ .type = CX88_VMUX_COMPOSITE1,
+ .vmux = 1,
+ .gpio0 = 0x0400, /* pin 2 = 0 */
+ .gpio1 = 0x6060, /* pin 13 = 1, pin 14 = 1 */
+ .gpio2 = 0x0000,
+ },{
+ .type = CX88_VMUX_SVIDEO,
+ .vmux = 2,
+ .gpio0 = 0x0400, /* pin 2 = 0 */
+ .gpio1 = 0x6060, /* pin 13 = 1, pin 14 = 1 */
+ .gpio2 = 0x0000,
+ }},
+ .radio = {
+ .type = CX88_RADIO,
+ .gpio0 = 0x0400, /* pin 2 = 0 */
+ .gpio1 = 0x6000, /* pin 13 = 0, pin 14 = 0 */
+ .gpio2 = 0x0000,
+ },
+ .mpeg = CX88_MPEG_DVB,
+ },
};
/* ------------------------------------------------------------------ */
@@ -2297,6 +2338,10 @@
.subvendor = 0xB033,
.subdevice = 0x3033,
.card = CX88_BOARD_PROF_7300,
+ }, {
+ .subvendor = 0x107d,
+ .subdevice = 0x6654,
+ .card = CX88_BOARD_WINFAST_DTV1800H,
},
};
@@ -2494,6 +2539,23 @@
return -EINVAL;
}
+static int cx88_xc3028_winfast1800h_callback(struct cx88_core *core,
+ int command, int arg)
+{
+ switch (command) {
+ case XC2028_TUNER_RESET:
+ /* GPIO 12 (xc3028 tuner reset) */
+ cx_set(MO_GP1_IO, 0x1010);
+ mdelay(50);
+ cx_clear(MO_GP1_IO, 0x10);
+ mdelay(50);
+ cx_set(MO_GP1_IO, 0x10);
+ mdelay(50);
+ return 0;
+ }
+ return -EINVAL;
+}
+
/* ------------------------------------------------------------------- */
/* some Divco specific stuff */
static int cx88_pv_8000gt_callback(struct cx88_core *core,
@@ -2566,6 +2628,8 @@
case CX88_BOARD_DVICO_FUSIONHDTV_DVB_T_PRO:
case CX88_BOARD_DVICO_FUSIONHDTV_5_PCI_NANO:
return cx88_dvico_xc2028_callback(core, command, arg);
+ case CX88_BOARD_WINFAST_DTV1800H:
+ return cx88_xc3028_winfast1800h_callback(core, command, arg);
}
switch (command) {
@@ -2740,6 +2804,16 @@
cx_set(MO_GP0_IO, 0x00000080); /* 702 out of reset */
udelay(1000);
break;
+
+ case CX88_BOARD_WINFAST_DTV1800H:
+ /* GPIO 12 (xc3028 tuner reset) */
+ cx_set(MO_GP1_IO, 0x1010);
+ mdelay(50);
+ cx_clear(MO_GP1_IO, 0x10);
+ mdelay(50);
+ cx_set(MO_GP1_IO, 0x10);
+ mdelay(50);
+ break;
}
}
@@ -2760,6 +2834,7 @@
core->i2c_algo.udelay = 16;
break;
case CX88_BOARD_DVICO_FUSIONHDTV_DVB_T_PRO:
+ case CX88_BOARD_WINFAST_DTV1800H:
ctl->demod = XC3028_FE_ZARLINK456;
break;
case CX88_BOARD_KWORLD_ATSC_120:
diff -r e2a8b9b9c294 linux/drivers/media/video/cx88/cx88-dvb.c
--- a/linux/drivers/media/video/cx88/cx88-dvb.c Fri Oct 17 19:45:55 2008 +0300
+++ b/linux/drivers/media/video/cx88/cx88-dvb.c Wed Oct 22 20:22:24 2008 +0200
@@ -963,6 +963,7 @@
}
break;
case CX88_BOARD_PINNACLE_HYBRID_PCTV:
+ case CX88_BOARD_WINFAST_DTV1800H:
fe0->dvb.frontend = dvb_attach(zl10353_attach,
&cx88_pinnacle_hybrid_pctv,
&core->i2c_adap);
diff -r e2a8b9b9c294 linux/drivers/media/video/cx88/cx88-input.c
--- a/linux/drivers/media/video/cx88/cx88-input.c Fri Oct 17 19:45:55 2008 +0300
+++ b/linux/drivers/media/video/cx88/cx88-input.c Wed Oct 22 20:22:24 2008 +0200
@@ -93,6 +93,7 @@
gpio=(gpio & 0x7fd) + (auxgpio & 0xef);
break;
case CX88_BOARD_WINFAST_DTV1000:
+ case CX88_BOARD_WINFAST_DTV1800H:
gpio = (gpio & 0x6ff) | ((cx_read(MO_GP1_IO) << 8) & 0x900);
auxgpio = gpio;
break;
@@ -244,6 +245,7 @@
ir->sampling = 1;
break;
case CX88_BOARD_WINFAST_DTV2000H:
+ case CX88_BOARD_WINFAST_DTV1800H:
ir_codes = ir_codes_winfast;
ir->gpio_addr = MO_GP0_IO;
ir->mask_keycode = 0x8f8;
diff -r e2a8b9b9c294 linux/drivers/media/video/cx88/cx88.h
--- a/linux/drivers/media/video/cx88/cx88.h Fri Oct 17 19:45:55 2008 +0300
+++ b/linux/drivers/media/video/cx88/cx88.h Wed Oct 22 20:22:24 2008 +0200
@@ -230,6 +230,7 @@
#define CX88_BOARD_TEVII_S420 73
#define CX88_BOARD_PROLINK_PV_GLOBAL_XTREME 74
#define CX88_BOARD_PROF_7300 75
+#define CX88_BOARD_WINFAST_DTV1800H 76
enum cx88_itype {
CX88_VMUX_COMPOSITE1 = 1,
[-- Attachment #3: Type: text/plain, Size: 150 bytes --]
_______________________________________________
linux-dvb mailing list
linux-dvb@linuxtv.org
http://www.linuxtv.org/cgi-bin/mailman/listinfo/linux-dvb
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [linux-dvb] Leadtek WinFast DTV-1800H support
2008-10-20 18:45 ` Steven Toth
2008-10-22 19:29 ` Miroslav Šustek
@ 2008-10-22 19:50 ` Miroslav Šustek
1 sibling, 0 replies; 4+ messages in thread
From: Miroslav Šustek @ 2008-10-22 19:50 UTC (permalink / raw)
To: linux-dvb
[-- Attachment #1: Type: text/plain, Size: 1593 bytes --]
*Sorry for double-post, but first one was cut off.*
Steven Toth wrote:
>
> From my orig email: "Don't call cx_write() inside the gpio card setup, you're potentially destroying the other bits, it's risky."
>
>cx_write destroys the content of the GPIO direction-enablement and values bits. That's a bad thing, and can lead to unexpected behaviors if used generally in drivers.
>
>It's better to have a driver read the previous register value, and/or in the appropriate bit and write the value back to the gpio registers. This is what cx_set/clear do. That way parts of the driver can toggle GPIO's for important pieces, without having to understand GPIO usage in other disconnected/unrelated parts of the driver.
>
>Your patch uses set/clear, which is good.
>From datasheet (http://www.domenech.org/datasheets/cx23880-cx23881-cx23882-cx23883-datasheet-08-2002.pdf),
Chapter 2.3.3:
GP{x}_IO register:
bits [23:16] = GP{x}_BWE
"If this field is equal to 8’h00, then the whole GPIO byte
register operates in normal RW mode. If any bit is set, then
the corresponding GP_OE and GP_IO bit locations are
enabled for writing. If the bit write enable is not set, the
corresponding GPIO bits will be unaffected."
So I think this is just like (this is) mask. I don't see any possibility of unexpected behavior.
You only have to know which bits you want change (=how to set the mask)...
Ok, persuaded me, using cx_set and cx_clear is prettier :) .
>Incidently, you patch cannot be merged as-is, it has C99 style comments '//' are not allowed, change to /* */ etc.
Of course, new patch is attached.
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: leadtek_winfast_dtv1800h_2.patch --]
[-- Type: text/x-patch; name="leadtek_winfast_dtv1800h_2.patch", Size: 4848 bytes --]
diff -r e2a8b9b9c294 linux/drivers/media/video/cx88/cx88-cards.c
--- a/linux/drivers/media/video/cx88/cx88-cards.c Fri Oct 17 19:45:55 2008 +0300
+++ b/linux/drivers/media/video/cx88/cx88-cards.c Wed Oct 22 20:22:24 2008 +0200
@@ -1904,6 +1904,47 @@
} },
.mpeg = CX88_MPEG_DVB,
},
+ [CX88_BOARD_WINFAST_DTV1800H] = {
+ .name = "Leadtek WinFast DTV1800 Hybrid",
+ .tuner_type = TUNER_XC2028,
+ .radio_type = TUNER_XC2028,
+ .tuner_addr = 0x61,
+ .radio_addr = 0x61,
+ /*
+ * GPIO setting
+ *
+ * 2: mute (0=off,1=on)
+ * 12: tuner reset pin
+ * 13: audio source (0=tuner audio,1=line in)
+ * 14: FM (0=on,1=off ???)
+ */
+ .input = {{
+ .type = CX88_VMUX_TELEVISION,
+ .vmux = 0,
+ .gpio0 = 0x0400, /* pin 2 = 0 */
+ .gpio1 = 0x6040, /* pin 13 = 0, pin 14 = 1 */
+ .gpio2 = 0x0000,
+ },{
+ .type = CX88_VMUX_COMPOSITE1,
+ .vmux = 1,
+ .gpio0 = 0x0400, /* pin 2 = 0 */
+ .gpio1 = 0x6060, /* pin 13 = 1, pin 14 = 1 */
+ .gpio2 = 0x0000,
+ },{
+ .type = CX88_VMUX_SVIDEO,
+ .vmux = 2,
+ .gpio0 = 0x0400, /* pin 2 = 0 */
+ .gpio1 = 0x6060, /* pin 13 = 1, pin 14 = 1 */
+ .gpio2 = 0x0000,
+ }},
+ .radio = {
+ .type = CX88_RADIO,
+ .gpio0 = 0x0400, /* pin 2 = 0 */
+ .gpio1 = 0x6000, /* pin 13 = 0, pin 14 = 0 */
+ .gpio2 = 0x0000,
+ },
+ .mpeg = CX88_MPEG_DVB,
+ },
};
/* ------------------------------------------------------------------ */
@@ -2297,6 +2338,10 @@
.subvendor = 0xB033,
.subdevice = 0x3033,
.card = CX88_BOARD_PROF_7300,
+ }, {
+ .subvendor = 0x107d,
+ .subdevice = 0x6654,
+ .card = CX88_BOARD_WINFAST_DTV1800H,
},
};
@@ -2494,6 +2539,23 @@
return -EINVAL;
}
+static int cx88_xc3028_winfast1800h_callback(struct cx88_core *core,
+ int command, int arg)
+{
+ switch (command) {
+ case XC2028_TUNER_RESET:
+ /* GPIO 12 (xc3028 tuner reset) */
+ cx_set(MO_GP1_IO, 0x1010);
+ mdelay(50);
+ cx_clear(MO_GP1_IO, 0x10);
+ mdelay(50);
+ cx_set(MO_GP1_IO, 0x10);
+ mdelay(50);
+ return 0;
+ }
+ return -EINVAL;
+}
+
/* ------------------------------------------------------------------- */
/* some Divco specific stuff */
static int cx88_pv_8000gt_callback(struct cx88_core *core,
@@ -2566,6 +2628,8 @@
case CX88_BOARD_DVICO_FUSIONHDTV_DVB_T_PRO:
case CX88_BOARD_DVICO_FUSIONHDTV_5_PCI_NANO:
return cx88_dvico_xc2028_callback(core, command, arg);
+ case CX88_BOARD_WINFAST_DTV1800H:
+ return cx88_xc3028_winfast1800h_callback(core, command, arg);
}
switch (command) {
@@ -2740,6 +2804,16 @@
cx_set(MO_GP0_IO, 0x00000080); /* 702 out of reset */
udelay(1000);
break;
+
+ case CX88_BOARD_WINFAST_DTV1800H:
+ /* GPIO 12 (xc3028 tuner reset) */
+ cx_set(MO_GP1_IO, 0x1010);
+ mdelay(50);
+ cx_clear(MO_GP1_IO, 0x10);
+ mdelay(50);
+ cx_set(MO_GP1_IO, 0x10);
+ mdelay(50);
+ break;
}
}
@@ -2760,6 +2834,7 @@
core->i2c_algo.udelay = 16;
break;
case CX88_BOARD_DVICO_FUSIONHDTV_DVB_T_PRO:
+ case CX88_BOARD_WINFAST_DTV1800H:
ctl->demod = XC3028_FE_ZARLINK456;
break;
case CX88_BOARD_KWORLD_ATSC_120:
diff -r e2a8b9b9c294 linux/drivers/media/video/cx88/cx88-dvb.c
--- a/linux/drivers/media/video/cx88/cx88-dvb.c Fri Oct 17 19:45:55 2008 +0300
+++ b/linux/drivers/media/video/cx88/cx88-dvb.c Wed Oct 22 20:22:24 2008 +0200
@@ -963,6 +963,7 @@
}
break;
case CX88_BOARD_PINNACLE_HYBRID_PCTV:
+ case CX88_BOARD_WINFAST_DTV1800H:
fe0->dvb.frontend = dvb_attach(zl10353_attach,
&cx88_pinnacle_hybrid_pctv,
&core->i2c_adap);
diff -r e2a8b9b9c294 linux/drivers/media/video/cx88/cx88-input.c
--- a/linux/drivers/media/video/cx88/cx88-input.c Fri Oct 17 19:45:55 2008 +0300
+++ b/linux/drivers/media/video/cx88/cx88-input.c Wed Oct 22 20:22:24 2008 +0200
@@ -93,6 +93,7 @@
gpio=(gpio & 0x7fd) + (auxgpio & 0xef);
break;
case CX88_BOARD_WINFAST_DTV1000:
+ case CX88_BOARD_WINFAST_DTV1800H:
gpio = (gpio & 0x6ff) | ((cx_read(MO_GP1_IO) << 8) & 0x900);
auxgpio = gpio;
break;
@@ -244,6 +245,7 @@
ir->sampling = 1;
break;
case CX88_BOARD_WINFAST_DTV2000H:
+ case CX88_BOARD_WINFAST_DTV1800H:
ir_codes = ir_codes_winfast;
ir->gpio_addr = MO_GP0_IO;
ir->mask_keycode = 0x8f8;
diff -r e2a8b9b9c294 linux/drivers/media/video/cx88/cx88.h
--- a/linux/drivers/media/video/cx88/cx88.h Fri Oct 17 19:45:55 2008 +0300
+++ b/linux/drivers/media/video/cx88/cx88.h Wed Oct 22 20:22:24 2008 +0200
@@ -230,6 +230,7 @@
#define CX88_BOARD_TEVII_S420 73
#define CX88_BOARD_PROLINK_PV_GLOBAL_XTREME 74
#define CX88_BOARD_PROF_7300 75
+#define CX88_BOARD_WINFAST_DTV1800H 76
enum cx88_itype {
CX88_VMUX_COMPOSITE1 = 1,
[-- Attachment #3: Type: text/plain, Size: 150 bytes --]
_______________________________________________
linux-dvb mailing list
linux-dvb@linuxtv.org
http://www.linuxtv.org/cgi-bin/mailman/listinfo/linux-dvb
^ permalink raw reply [flat|nested] 4+ messages in thread