Re: [PATCH] Leadtek WinFast DTV-1800H support

["Miroslav Šustek" <sustmidown@centrum.cz>]

[-- Attachment #1: Type: text/plain, Size: 4186 bytes --]

Trent Piepho <xyzzy <at> speakeasy.org> writes:

> Instead of raising the reset line here, why not change the gpio settings in
> the card definition to have it high? Change gpio1 for television to 0x7050
> and radio to 0x7010.
Personally, I don't know when these .gpioX members are used (before
firmware loads or after...).
But I assume that adding the high on reset pin shouldn't break anything,
so we can do this.

And shouldn't we put tuner reset pin to 0 when in composite and s-video mode?
These inputs don't use tuner or do they?
If I look in dmesg I can see that firmware is loaded into tuner even
when these modes are used (I'm using MPlayer to select the input).
And due to callbacks issued duting firmware loading, tuner is turned on
(reset pin = 1) no matter if it was turned off by .gpioX setting.

And shouldn't we use the mask bits [24:16] of MO_GPX_IO
in .gpioX members too? I know only few GPIO pins and the other I don't
know either what direction they should be. That means GPIO pins which
I don't know are set as Hi-Z = inputs... Now, when I think of that,
if it works it's safer when the other pins are in Hi-Z mode. Never mind.

>
> Then the reset can be done with:
>
> case XC2028_TUNER_RESET:
> /* GPIO 12 (xc3028 tuner reset) */
> cx_write(MO_GP1_IO, 0x101000);
> mdelay(50);
> cx_write(MO_GP1_IO, 0x101010);
> mdelay(50);
> return 0;
>
Earlier I was told to use 'cx_set' and 'cx_clear' because using 'cx_write'
is risky.
see here: http://www.spinics.net/lists/linux-dvb/msg29777.html
And when you are using 'cx_set' and 'cx_clear' you need 3 calls.
The first to set the direction bit, the second to set 0 on reset pin
and the third to set 1 on reset pin.
If you ask me which I think is nicer I'll tell you: that one with 'cx_write'.
If you ask me which one I want to use, I'll tell: I don't care. :)

> Though I have to wonder why each card needs its own xc2028 reset function.
> Shouldn't they all be the same other than what gpio they change?
My English goes lame here. Do you mean that reset function shouldn't use
GPIO at all?

>
> @@ -2882,6 +2946,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;
> }
> }
>
> Couldn't you replace this with:
>
> case CX88_BOARD_WINFAST_DTV1800H:
> cx88_xc3028_winfast1800h_callback(code, XC2028_TUNER_RESET, 0);
> break;
Yes, this will do the same job.
I think that 'cx88_card_setup_pre_i2c' is to be called before any I2C
communication. The 'cx88_xc3028_winfast1800h_callback' (cx88_tuner_callback)
is meant to be used when tuner code (during firmware loading) needs it.
This is probably why others did it this way (these are separated issues
even if they do the same thing) and I only obey existing form.

I only want to finally add the support for this card.
You know many people (not developers) don't care "if this function is used
or that function is used twice, instead". They just want to install they distro
and watch the tv.
I classify myself as an programmer rather than ordinary user, so I care how
the code looks like. I'm open to the discussion about these things, but
this can take long time and I just want the card to be supported asap.
There are more cards which has code like this so if linuxtv developers realize
eg. to not use callbacks or use only cx_set and cx_clear (instead of cx_write)
they'll do it all at once (not every card separately).

I attached modified patch:
- .gpioX members of inputs which use tuner have reset pin 1 (tuner enabled)
- .gpioX members of inputs which don't use tuner have reset pin 0 (tuner disabled)
- resets (in callback and the one in pre_i2c) use only two 'cx_write' calls

I'm keeping the "tested-by" lines even if this modified version of patch wasn't
tested by those people (the previous version was). I trust this changes can't
break the functionality.
If you think it's too audacious then drop them.

It's on linuxtv developers which of these two patches will be chosen.

- Miroslav Šustek


[-- Attachment #2: leadtek_winfast_dtv1800h_v2.patch --]
[-- Type: application/octet-stream, Size: 5789 bytes --]

Adds support for Leadtek WinFast DTV-1800H

From: Miroslav Sustek <sustmidown@centrum.cz>

Enables analog/digital tv, radio and remote control (gpio).

Signed-off-by: Miroslav Sustek <sustmidown@centrum.cz>
Tested-by: Marcin Wojcikowski <emtees.mts@gmail.com>
Tested-by: Karel Juhanak <karel.juhanak@warnet.cz>
Tested-by: Andrew Goff <goffa72@gmail.com>
Tested-by: Jan Novak <novak-j@seznam.cz>

diff -r 25bc0580359a linux/Documentation/video4linux/CARDLIST.cx88
--- a/linux/Documentation/video4linux/CARDLIST.cx88	Fri May 29 17:03:31 2009 -0300
+++ b/linux/Documentation/video4linux/CARDLIST.cx88	Sun May 31 18:44:05 2009 +0200
@@ -79,3 +79,4 @@
  78 -> Prof 6200 DVB-S                                     [b022:3022]
  79 -> Terratec Cinergy HT PCI MKII                        [153b:1177]
  80 -> Hauppauge WinTV-IR Only                             [0070:9290]
+ 81 -> Leadtek WinFast DTV1800 Hybrid                      [107d:6654]
diff -r 25bc0580359a linux/drivers/media/video/cx88/cx88-cards.c
--- a/linux/drivers/media/video/cx88/cx88-cards.c	Fri May 29 17:03:31 2009 -0300
+++ b/linux/drivers/media/video/cx88/cx88-cards.c	Sun May 31 18:44:05 2009 +0200
@@ -2009,6 +2009,47 @@
 		.tuner_addr	= ADDR_UNSET,
 		.radio_addr	= ADDR_UNSET,
 	},
+	[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  = 0x7050,       /* p12 = 1, p13 = 0, p14 = 1 */
+			.gpio2  = 0x0000,
+		}, {
+			.type   = CX88_VMUX_COMPOSITE1,
+			.vmux   = 1,
+			.gpio0  = 0x0400,       /* pin 2 = 0 */
+			.gpio1  = 0x7060,       /* p12 = 0, p13 = 1, p14 = 1 */
+			.gpio2  = 0x0000,
+		}, {
+			.type   = CX88_VMUX_SVIDEO,
+			.vmux   = 2,
+			.gpio0  = 0x0400,       /* pin 2 = 0 */
+			.gpio1  = 0x7060,       /* p12 = 0, p13 = 1, p14 = 1 */
+			.gpio2  = 0x0000,
+		} },
+		.radio = {
+			.type   = CX88_RADIO,
+			.gpio0  = 0x0400,       /* pin 2 = 0 */
+			.gpio1  = 0x7010,       /* p12 = 1, p13 = 0, p14 = 0 */
+			.gpio2  = 0x0000,
+		},
+		.mpeg           = CX88_MPEG_DVB,
+	},
 };
 
 /* ------------------------------------------------------------------ */
@@ -2426,6 +2467,10 @@
 		.subvendor = 0x0070,
 		.subdevice = 0x9290,
 		.card      = CX88_BOARD_HAUPPAUGE_IRONLY,
+	}, {
+		.subvendor = 0x107d,
+		.subdevice = 0x6654,
+		.card      = CX88_BOARD_WINFAST_DTV1800H,
 	},
 };
 
@@ -2624,6 +2669,21 @@
 	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_write(MO_GP1_IO, 0x101000);
+		mdelay(50);
+		cx_write(MO_GP1_IO, 0x101010);
+		mdelay(50);
+		return 0;
+	}
+	return -EINVAL;
+}
+
 /* ------------------------------------------------------------------- */
 /* some Divco specific stuff                                           */
 static int cx88_pv_8000gt_callback(struct cx88_core *core,
@@ -2696,6 +2756,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) {
@@ -2882,6 +2944,14 @@
 		cx_set(MO_GP0_IO, 0x00000080); /* 702 out of reset */
 		udelay(1000);
 		break;
+
+	case CX88_BOARD_WINFAST_DTV1800H:
+		/* GPIO 12 (xc3028 tuner reset) */
+		cx_write(MO_GP1_IO, 0x101000);
+		mdelay(50);
+		cx_write(MO_GP1_IO, 0x101010);
+		mdelay(50);
+		break;
 	}
 }
 
@@ -2902,6 +2972,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 25bc0580359a linux/drivers/media/video/cx88/cx88-dvb.c
--- a/linux/drivers/media/video/cx88/cx88-dvb.c	Fri May 29 17:03:31 2009 -0300
+++ b/linux/drivers/media/video/cx88/cx88-dvb.c	Sun May 31 18:44:05 2009 +0200
@@ -1021,6 +1021,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 25bc0580359a linux/drivers/media/video/cx88/cx88-input.c
--- a/linux/drivers/media/video/cx88/cx88-input.c	Fri May 29 17:03:31 2009 -0300
+++ b/linux/drivers/media/video/cx88/cx88-input.c	Sun May 31 18:44:05 2009 +0200
@@ -92,6 +92,7 @@
 		gpio=(gpio & 0x7fd) + (auxgpio & 0xef);
 		break;
 	case CX88_BOARD_WINFAST_DTV1000:
+	case CX88_BOARD_WINFAST_DTV1800H:
 	case CX88_BOARD_WINFAST_TV2000_XP_GLOBAL:
 		gpio = (gpio & 0x6ff) | ((cx_read(MO_GP1_IO) << 8) & 0x900);
 		auxgpio = gpio;
@@ -237,6 +238,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 25bc0580359a linux/drivers/media/video/cx88/cx88.h
--- a/linux/drivers/media/video/cx88/cx88.h	Fri May 29 17:03:31 2009 -0300
+++ b/linux/drivers/media/video/cx88/cx88.h	Sun May 31 18:44:05 2009 +0200
@@ -237,6 +237,7 @@
 #define CX88_BOARD_PROF_6200               78
 #define CX88_BOARD_TERRATEC_CINERGY_HT_PCI_MKII 79
 #define CX88_BOARD_HAUPPAUGE_IRONLY        80
+#define CX88_BOARD_WINFAST_DTV1800H        81
 
 enum cx88_itype {
 	CX88_VMUX_COMPOSITE1 = 1,