[5/12] tm6000: update init table and sequence for tm6010

Message ID 1265410096-11788-5-git-send-email-stefan.ringel@arcor.de (mailing list archive)
State Superseded, archived
Headers

Commit Message

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

---
 drivers/staging/tm6000/tm6000-core.c |  179 ++++++++++++++++++++++++----------
 1 files changed, 128 insertions(+), 51 deletions(-)
  

Comments

Mauro Carvalho Chehab Feb. 8, 2010, 11:21 a.m. UTC | #1
Hi Stefan,

First, a few comments about your patch series:

I've committed almost of your patches, and added an extra patch to make the
driver to compile it with -git. There were other broken things when compiling
against -git.

Several of your patches are adding leading whitespaces. Please review them before
submitting. On -git, those whitespaces are shown with a red background color.

I've re-made most of the patch descriptions. Please take a look on them and try
to improve on a next time.

We've got 2 submission for each patches. I just discarded the older one.

I've removed the two BEHOLDER board descriptions from one of your patches. It is
not related to your board, but it is another compilation fix.

From your series, I didn't merge those 3 patches:

[5/12] tm6000: update init table and sequence for tm6010                http://patchwork.kernel.org/patch/77451
[6/12] tm6000: tuner reset timeing optimation                           http://patchwork.kernel.org/patch/77459
[11/12] tm6000: bugfix firmware xc3028L-v36.fw used with Zarlink and    http://patchwork.kernel.org/patch/77462

I'll send you separate comments why I didn't merge them, in reply to each email you've sent,
starting with this one (patch 5/12).


stefan.ringel@arcor.de wrote:
> From: Stefan Ringel <stefan.ringel@arcor.de>
> 
> ---
>  drivers/staging/tm6000/tm6000-core.c |  179 ++++++++++++++++++++++++----------
>  1 files changed, 128 insertions(+), 51 deletions(-)
> 
> diff --git a/drivers/staging/tm6000/tm6000-core.c b/drivers/staging/tm6000/tm6000-core.c
> index 7ec13d5..a2e2af5 100644
> --- a/drivers/staging/tm6000/tm6000-core.c
> +++ b/drivers/staging/tm6000/tm6000-core.c
> @@ -414,7 +414,15 @@ struct reg_init tm6010_init_tab[] = {
>  	{ REQ_07_SET_GET_AVREG, 0x3f, 0x00 },
>  
>  	{ REQ_05_SET_GET_USBREG, 0x18, 0x00 },
> -
> +	
> +	/* additional from Terratec Cinergy Hybrid XE */
> +	{ REQ_07_SET_GET_AVREG, 0xdc, 0xaa },
> +	{ REQ_07_SET_GET_AVREG, 0xdd, 0x30 },
> +	{ REQ_07_SET_GET_AVREG, 0xde, 0x20 },
> +	{ REQ_07_SET_GET_AVREG, 0xdf, 0xd0 },
> +	{ REQ_04_EN_DISABLE_MCU_INT, 0x02, 0x00 },
> +	{ REQ_07_SET_GET_AVREG, 0xd8, 0x2f },
> +	
>  	/* set remote wakeup key:any key wakeup */
>  	{ REQ_07_SET_GET_AVREG,  0xe5,  0xfe },
>  	{ REQ_07_SET_GET_AVREG,  0xda,  0xff },
> @@ -424,6 +432,7 @@ int tm6000_init (struct tm6000_core *dev)
>  {
>  	int board, rc=0, i, size;
>  	struct reg_init *tab;
> +	u8 buf[40];
>  
>  	if (dev->dev_type == TM6010) {
>  		tab = tm6010_init_tab;
> @@ -444,61 +453,129 @@ int tm6000_init (struct tm6000_core *dev)
>  		}
>  	}
>  
> -	msleep(5); /* Just to be conservative */
> -
> -	/* Check board version - maybe 10Moons specific */
> -	board=tm6000_get_reg16 (dev, 0x40, 0, 0);
> -	if (board >=0) {
> -		printk (KERN_INFO "Board version = 0x%04x\n",board);
> -	} else {
> -		printk (KERN_ERR "Error %i while retrieving board version\n",board);
> -	}
> -
> +	/* hack */
>  	if (dev->dev_type == TM6010) {
> -		/* Turn xceive 3028 on */
> -		tm6000_set_reg(dev, REQ_03_SET_GET_MCU_PIN, TM6010_GPIO_3, 0x01);
> -		msleep(11);
> -	}

The above is board-specific. It is needed for the tm6010 device I have here
(HVR900H), otherwise no xc3028 command will be handled.

The better here is to add a setup routine to tm6000-cards and move all 
those GPIO codes to it. Then, add there your board-specific setup.

I've added a patch that moves those GPIO board-specific setup to tm6000-cards:
tm6000_cards_setup(). Please move your board specific GPIO init to there.


> -
> -	/* Reset GPIO1 and GPIO4. */
> -	for (i=0; i< 2; i++) {
> -		rc = tm6000_set_reg(dev, REQ_03_SET_GET_MCU_PIN,
> -					dev->tuner_reset_gpio, 0x00);
> -		if (rc<0) {
> -			printk (KERN_ERR "Error %i doing GPIO1 reset\n",rc);
> -			return rc;
> -		}
> -
> -		msleep(10); /* Just to be conservative */
> -		rc = tm6000_set_reg(dev, REQ_03_SET_GET_MCU_PIN,
> -					dev->tuner_reset_gpio, 0x01);
> -		if (rc<0) {
> -			printk (KERN_ERR "Error %i doing GPIO1 reset\n",rc);
> -			return rc;
> -		}
> -
> -		msleep(10);
> -		rc=tm6000_set_reg (dev, REQ_03_SET_GET_MCU_PIN, TM6000_GPIO_4, 0);
> -		if (rc<0) {
> -			printk (KERN_ERR "Error %i doing GPIO4 reset\n",rc);
> -			return rc;
> -		}
> -
> -		msleep(10);
> -		rc=tm6000_set_reg (dev, REQ_03_SET_GET_MCU_PIN, TM6000_GPIO_4, 1);
> -		if (rc<0) {
> -			printk (KERN_ERR "Error %i doing GPIO4 reset\n",rc);
> -			return rc;
> -		}
> -
> -		if (!i) {
> -			rc=tm6000_get_reg16(dev, 0x40,0,0);
> -			if (rc>=0) {
> -				printk ("board=%d\n", rc);
> +		
> +		msleep(15);
> +		tm6000_set_reg(dev, REQ_03_SET_GET_MCU_PIN,
> +				TM6010_GPIO_4, 0);
> +		msleep(15);
> +				
> +		tm6000_set_reg(dev, REQ_03_SET_GET_MCU_PIN,
> +				TM6010_GPIO_1, 0);
> +	
> +		msleep(50);
> +		tm6000_set_reg(dev, REQ_03_SET_GET_MCU_PIN,
> +				TM6010_GPIO_1, 1);
> +		

The above reflects the timing needed by your device. Depending on the board,
the sleep time may eventually be different.

> +		msleep(15);
> +		tm6000_read_write_usb (dev, 0xc0, 0x0e, 0x0010, 0x4400, buf, 2);
> +		
> +		msleep(15);
> +		tm6000_read_write_usb (dev, 0xc0, 0x10, 0xf432, 0x0000, buf, 2);
> +	
> +		msleep(15);
> +		buf[0] = 0x12;
> +		buf[1] = 0x34;
> +		tm6000_read_write_usb (dev, 0x40, 0x10, 0xf432, 0x0000, buf, 2);
> +	
> +		msleep(15);
> +		tm6000_read_write_usb (dev, 0xc0, 0x10, 0xf432, 0x0000, buf, 2);
> +	
> +		msleep(15);
> +		tm6000_read_write_usb (dev, 0xc0, 0x10, 0x0032, 0x0000, buf, 2);
> +
> +		msleep(15);
> +		buf[0] = 0x00;
> +		buf[1] = 0x01;
> +		tm6000_read_write_usb (dev, 0x40, 0x10, 0xf332, 0x0000, buf, 2);
> +	
> +		msleep(15);
> +		tm6000_read_write_usb (dev, 0xc0, 0x10, 0x00c0, 0x0000, buf, 39);
> +	
> +		msleep(15);
> +		buf[0] = 0x00;
> +		buf[1] = 0x00;
> +		tm6000_read_write_usb (dev, 0x40, 0x10, 0xf332, 0x0000, buf, 2);
> +	
> +		msleep(15);
> +		tm6000_read_write_usb (dev, 0xc0, 0x10, 0x7f1f, 0x0000, buf, 2);
> +//		printk(KERN_INFO "buf %#x %#x \n", buf[0], buf [1]);
> +		msleep(15);


At the above, you're just trying to reproduce whatever the original driver does,
instead of relying on the i2c drivers.

At the Linux drivers, we don't just send random i2c sequences in the middle of
the setup. Instead, we let each i2c driver to do the initialization they need
to do. 

If you take a look on each call, for example:
		tm6000_read_write_usb (dev, 0x40, 0x10, 0xf332, 0x0000, buf, 2);

The first value determines the USB direction: 0x40 is write; 0xc0 is read;
The second value is the request. Both 0x0e (REQ_14) and 0x10 (REQ_16) are used for
i2c. From the past experiences, REQ_16 works better when the size is 1, where REQ_14
works better for bigger sizes.

The third value gives the first byte of a write message and the i2c address. The lower
8 bits is the i2c address. The above sequence is playing with several different 
i2c devices, at addresses 0x10, 0x32, 0xc0 and 0x1f.

Most of the calls there are read (0xc0). I don't know any device that requires
a read for it to work. I suspect that the above code is just probing to check
what i2c devices are found at the board. The writes are to a device at address
0x32 (in i2c 8 bit notation - or 0x19 at i2c 7bit notation).

I suspect that the probe sequence noticed something at the address 0x32 and is
sending some init sequence for it. As this is not the tuner nor the demod, you
don't need those setup for your device to work. Also, this address is not typical
for eeprom. Without taking a look at the hardware, we can only guess what's there.
My guess is that it is for some i2c-based remote controller chip. We don't need
this for now. After having the rest working, we may need to return on it when
patching ir-kbd.i2c.

> +		tm6000_set_reg(dev, REQ_03_SET_GET_MCU_PIN,
> +				TM6010_GPIO_4, 1);
> +		msleep(15);
> +		tm6000_set_reg(dev, REQ_03_SET_GET_MCU_PIN,
> +	    			TM6010_GPIO_0, 1);
> +		msleep(15);
> +		tm6000_set_reg(dev, REQ_03_SET_GET_MCU_PIN,
> +				TM6010_GPIO_7, 0);
> +		msleep(15);
> +		tm6000_set_reg(dev, REQ_03_SET_GET_MCU_PIN,
> +				TM6010_GPIO_5, 1);
> +	
> +		msleep(15);
> +	
> +		for (i=0; i< size; i++) {
> +			rc= tm6000_set_reg (dev, tab[i].req, tab[i].reg, tab[i].val);
> +			if (rc<0) {
> +				printk (KERN_ERR "Error %i while setting req %d, "
> +						 "reg %d to value %d\n", rc,
> +						 tab[i].req,tab[i].reg, tab[i].val);
> +				return rc;
>  			}
>  		}
> +			
> +		msleep(15);
> +	
> +		tm6000_set_reg(dev, REQ_03_SET_GET_MCU_PIN,
> +				TM6010_GPIO_4, 0);
> +		msleep(15);
> +
> +		tm6000_set_reg(dev, REQ_03_SET_GET_MCU_PIN,
> +				TM6010_GPIO_1, 0);
> +	
> +		msleep(50);
> +		tm6000_set_reg(dev, REQ_03_SET_GET_MCU_PIN,
> +				TM6010_GPIO_1, 1);
> +		
> +		msleep(15);
> +		tm6000_read_write_usb (dev, 0xc0, 0x0e, 0x00c2, 0x0008, buf, 2);
> +//		printk(KERN_INFO "buf %#x %#x \n", buf[0], buf[1]);
> +		msleep(15);
> +		tm6000_set_reg(dev, REQ_03_SET_GET_MCU_PIN,
> +				TM6010_GPIO_2, 1);
> +		msleep(15);
> +		tm6000_set_reg(dev, REQ_03_SET_GET_MCU_PIN,
> +				TM6010_GPIO_2, 0);
> +		msleep(15);
> +		tm6000_set_reg(dev, REQ_03_SET_GET_MCU_PIN,
> +				TM6010_GPIO_2, 1);
> +		msleep(15);
> +		tm6000_set_reg(dev, REQ_03_SET_GET_MCU_PIN,
> +				TM6010_GPIO_2, 1);
> +		msleep(15);
> +		tm6000_set_reg(dev, REQ_03_SET_GET_MCU_PIN,
> +				TM6010_GPIO_2, 0);
> +		msleep(15);
> +		tm6000_set_reg(dev, REQ_03_SET_GET_MCU_PIN,
> +				TM6010_GPIO_2, 1);
> +		msleep(15);
>  	}
> +	/* hack end 

The above sequence is device-specific. Please add your code after I patch
tm6000-cards.

*/
> +	
> +	msleep(5); /* Just to be conservative */
>  
> +	/* Check board version - maybe 10Moons specific */
> +	if (dev->dev_type == TM5600) {
> +		 board=tm6000_get_reg16 (dev, 0x40, 0, 0);
> +		if (board >=0) {
> +			printk (KERN_INFO "Board version = 0x%04x\n",board);
> +		} else {
> +			printk (KERN_ERR "Error %i while retrieving board version\n",board);
> +		}
> +	}
> +	
>  	msleep(50);
>  
>  	return 0;
  
Mauro Carvalho Chehab Feb. 8, 2010, 11:37 a.m. UTC | #2
Mauro Carvalho Chehab wrote:
>> +		tm6000_read_write_usb (dev, 0xc0, 0x10, 0x7f1f, 0x0000, buf, 2);

> Most of the calls there are read (0xc0). I don't know any device that requires
> a read for it to work. I suspect that the above code is just probing to check
> what i2c devices are found at the board.

Btw, by looking at drivers/media/dvb/frontends/zl10353_priv.h, we have an idea
on what the above does:

The register 0x7f is:

        CHIP_ID            = 0x7F,

So, basically, the above code is reading the ID of the chip, likely to be sure that it
is a Zarlink 10353.

Cheers,
Mauro
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
  
Stefan Ringel Feb. 8, 2010, 4:12 p.m. UTC | #3
Am 08.02.2010 12:37, schrieb Mauro Carvalho Chehab:
> Mauro Carvalho Chehab wrote:
>   
>>> +		tm6000_read_write_usb (dev, 0xc0, 0x10, 0x7f1f, 0x0000, buf, 2);
>>>       
>   
>> Most of the calls there are read (0xc0). I don't know any device that requires
>> a read for it to work. I suspect that the above code is just probing to check
>> what i2c devices are found at the board.
>>     
> Btw, by looking at drivers/media/dvb/frontends/zl10353_priv.h, we have an idea
> on what the above does:
>
> The register 0x7f is:
>
>         CHIP_ID            = 0x7F,
>
> So, basically, the above code is reading the ID of the chip, likely to be sure that it
> is a Zarlink 10353.
>
> Cheers,
> Mauro
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>   

yes, but that's for activating Zarlink zl10353 and checking it --> hello
Zarlink? If doesn't use that sequence, then cannot use Zarlink zl10353.
  
Mauro Carvalho Chehab Feb. 8, 2010, 5:29 p.m. UTC | #4
Stefan Ringel wrote:
> Am 08.02.2010 12:37, schrieb Mauro Carvalho Chehab:
>> Mauro Carvalho Chehab wrote:
>>   
>>>> +		tm6000_read_write_usb (dev, 0xc0, 0x10, 0x7f1f, 0x0000, buf, 2);
>>>>       
>>   
>>> Most of the calls there are read (0xc0). I don't know any device that requires
>>> a read for it to work. I suspect that the above code is just probing to check
>>> what i2c devices are found at the board.
>>>     
>> Btw, by looking at drivers/media/dvb/frontends/zl10353_priv.h, we have an idea
>> on what the above does:
>>
>> The register 0x7f is:
>>
>>         CHIP_ID            = 0x7F,
>>
>> So, basically, the above code is reading the ID of the chip, likely to be sure that it
>> is a Zarlink 10353.
>>
>> Cheers,
>> Mauro
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-media" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>   
> 
> yes, but that's for activating Zarlink zl10353 and checking it --> hello
> Zarlink? If doesn't use that sequence, then cannot use Zarlink zl10353.
> 
Are you sure about that? Is this a new bug on tm6000?

Anyway, the proper place for such code is inside zl10353 driver, not outside.
  
Stefan Ringel Feb. 8, 2010, 5:30 p.m. UTC | #5
Am 08.02.2010 12:21, schrieb Mauro Carvalho Chehab:
> Hi Stefan,
>
> First, a few comments about your patch series:
>
> I've committed almost of your patches, and added an extra patch to make the
> driver to compile it with -git. There were other broken things when compiling
> against -git.
>
> Several of your patches are adding leading whitespaces. Please review them before
> submitting. On -git, those whitespaces are shown with a red background color.
>
> I've re-made most of the patch descriptions. Please take a look on them and try
> to improve on a next time.
>
> We've got 2 submission for each patches. I just discarded the older one.
>
> I've removed the two BEHOLDER board descriptions from one of your patches. It is
> not related to your board, but it is another compilation fix.
>
> From your series, I didn't merge those 3 patches:
>
> [5/12] tm6000: update init table and sequence for tm6010                http://patchwork.kernel.org/patch/77451
> [6/12] tm6000: tuner reset timeing optimation                           http://patchwork.kernel.org/patch/77459
> [11/12] tm6000: bugfix firmware xc3028L-v36.fw used with Zarlink and    http://patchwork.kernel.org/patch/77462
>
> I'll send you separate comments why I didn't merge them, in reply to each email you've sent,
> starting with this one (patch 5/12).
>
>
> stefan.ringel@arcor.de wrote:
>   
>> From: Stefan Ringel <stefan.ringel@arcor.de>
>>
>> ---
>>  drivers/staging/tm6000/tm6000-core.c |  179 ++++++++++++++++++++++++----------
>>  1 files changed, 128 insertions(+), 51 deletions(-)
>>
>> diff --git a/drivers/staging/tm6000/tm6000-core.c b/drivers/staging/tm6000/tm6000-core.c
>> index 7ec13d5..a2e2af5 100644
>> --- a/drivers/staging/tm6000/tm6000-core.c
>> +++ b/drivers/staging/tm6000/tm6000-core.c
>> @@ -414,7 +414,15 @@ struct reg_init tm6010_init_tab[] = {
>>  	{ REQ_07_SET_GET_AVREG, 0x3f, 0x00 },
>>  
>>  	{ REQ_05_SET_GET_USBREG, 0x18, 0x00 },
>> -
>> +	
>> +	/* additional from Terratec Cinergy Hybrid XE */
>> +	{ REQ_07_SET_GET_AVREG, 0xdc, 0xaa },
>> +	{ REQ_07_SET_GET_AVREG, 0xdd, 0x30 },
>> +	{ REQ_07_SET_GET_AVREG, 0xde, 0x20 },
>> +	{ REQ_07_SET_GET_AVREG, 0xdf, 0xd0 },
>> +	{ REQ_04_EN_DISABLE_MCU_INT, 0x02, 0x00 },
>> +	{ REQ_07_SET_GET_AVREG, 0xd8, 0x2f },
>> +	
>>  	/* set remote wakeup key:any key wakeup */
>>  	{ REQ_07_SET_GET_AVREG,  0xe5,  0xfe },
>>  	{ REQ_07_SET_GET_AVREG,  0xda,  0xff },
>> @@ -424,6 +432,7 @@ int tm6000_init (struct tm6000_core *dev)
>>  {
>>  	int board, rc=0, i, size;
>>  	struct reg_init *tab;
>> +	u8 buf[40];
>>  
>>  	if (dev->dev_type == TM6010) {
>>  		tab = tm6010_init_tab;
>> @@ -444,61 +453,129 @@ int tm6000_init (struct tm6000_core *dev)
>>  		}
>>  	}
>>  
>> -	msleep(5); /* Just to be conservative */
>> -
>> -	/* Check board version - maybe 10Moons specific */
>> -	board=tm6000_get_reg16 (dev, 0x40, 0, 0);
>> -	if (board >=0) {
>> -		printk (KERN_INFO "Board version = 0x%04x\n",board);
>> -	} else {
>> -		printk (KERN_ERR "Error %i while retrieving board version\n",board);
>> -	}
>> -
>> +	/* hack */
>>  	if (dev->dev_type == TM6010) {
>> -		/* Turn xceive 3028 on */
>> -		tm6000_set_reg(dev, REQ_03_SET_GET_MCU_PIN, TM6010_GPIO_3, 0x01);
>> -		msleep(11);
>> -	}
>>     
> The above is board-specific. It is needed for the tm6010 device I have here
> (HVR900H), otherwise no xc3028 command will be handled.
>
> The better here is to add a setup routine to tm6000-cards and move all 
> those GPIO codes to it. Then, add there your board-specific setup.
>
> I've added a patch that moves those GPIO board-specific setup to tm6000-cards:
> tm6000_cards_setup(). Please move your board specific GPIO init to there.
>
>
>   
>> -
>> -	/* Reset GPIO1 and GPIO4. */
>> -	for (i=0; i< 2; i++) {
>> -		rc = tm6000_set_reg(dev, REQ_03_SET_GET_MCU_PIN,
>> -					dev->tuner_reset_gpio, 0x00);
>> -		if (rc<0) {
>> -			printk (KERN_ERR "Error %i doing GPIO1 reset\n",rc);
>> -			return rc;
>> -		}
>> -
>> -		msleep(10); /* Just to be conservative */
>> -		rc = tm6000_set_reg(dev, REQ_03_SET_GET_MCU_PIN,
>> -					dev->tuner_reset_gpio, 0x01);
>> -		if (rc<0) {
>> -			printk (KERN_ERR "Error %i doing GPIO1 reset\n",rc);
>> -			return rc;
>> -		}
>> -
>> -		msleep(10);
>> -		rc=tm6000_set_reg (dev, REQ_03_SET_GET_MCU_PIN, TM6000_GPIO_4, 0);
>> -		if (rc<0) {
>> -			printk (KERN_ERR "Error %i doing GPIO4 reset\n",rc);
>> -			return rc;
>> -		}
>> -
>> -		msleep(10);
>> -		rc=tm6000_set_reg (dev, REQ_03_SET_GET_MCU_PIN, TM6000_GPIO_4, 1);
>> -		if (rc<0) {
>> -			printk (KERN_ERR "Error %i doing GPIO4 reset\n",rc);
>> -			return rc;
>> -		}
>> -
>> -		if (!i) {
>> -			rc=tm6000_get_reg16(dev, 0x40,0,0);
>> -			if (rc>=0) {
>> -				printk ("board=%d\n", rc);
>> +		
>> +		msleep(15);
>> +		tm6000_set_reg(dev, REQ_03_SET_GET_MCU_PIN,
>> +				TM6010_GPIO_4, 0);
>> +		msleep(15);
>> +				
>> +		tm6000_set_reg(dev, REQ_03_SET_GET_MCU_PIN,
>> +				TM6010_GPIO_1, 0);
>> +	
>> +		msleep(50);
>> +		tm6000_set_reg(dev, REQ_03_SET_GET_MCU_PIN,
>> +				TM6010_GPIO_1, 1);
>> +		
>>     
> The above reflects the timing needed by your device. Depending on the board,
> the sleep time may eventually be different.
>
>   
>> +		msleep(15);
>> +		tm6000_read_write_usb (dev, 0xc0, 0x0e, 0x0010, 0x4400, buf, 2);
>> +		
>> +		msleep(15);
>> +		tm6000_read_write_usb (dev, 0xc0, 0x10, 0xf432, 0x0000, buf, 2);
>> +	
>> +		msleep(15);
>> +		buf[0] = 0x12;
>> +		buf[1] = 0x34;
>> +		tm6000_read_write_usb (dev, 0x40, 0x10, 0xf432, 0x0000, buf, 2);
>> +	
>> +		msleep(15);
>> +		tm6000_read_write_usb (dev, 0xc0, 0x10, 0xf432, 0x0000, buf, 2);
>> +	
>> +		msleep(15);
>> +		tm6000_read_write_usb (dev, 0xc0, 0x10, 0x0032, 0x0000, buf, 2);
>> +
>> +		msleep(15);
>> +		buf[0] = 0x00;
>> +		buf[1] = 0x01;
>> +		tm6000_read_write_usb (dev, 0x40, 0x10, 0xf332, 0x0000, buf, 2);
>> +	
>> +		msleep(15);
>> +		tm6000_read_write_usb (dev, 0xc0, 0x10, 0x00c0, 0x0000, buf, 39);
>> +	
>> +		msleep(15);
>> +		buf[0] = 0x00;
>> +		buf[1] = 0x00;
>> +		tm6000_read_write_usb (dev, 0x40, 0x10, 0xf332, 0x0000, buf, 2);
>> +	
>> +		msleep(15);
>> +		tm6000_read_write_usb (dev, 0xc0, 0x10, 0x7f1f, 0x0000, buf, 2);
>> +//		printk(KERN_INFO "buf %#x %#x \n", buf[0], buf [1]);
>> +		msleep(15);
>>     
>
> At the above, you're just trying to reproduce whatever the original driver does,
> instead of relying on the i2c drivers.
>
> At the Linux drivers, we don't just send random i2c sequences in the middle of
> the setup. Instead, we let each i2c driver to do the initialization they need
> to do. 
>
> If you take a look on each call, for example:
> 		tm6000_read_write_usb (dev, 0x40, 0x10, 0xf332, 0x0000, buf, 2);
>
> The first value determines the USB direction: 0x40 is write; 0xc0 is read;
> The second value is the request. Both 0x0e (REQ_14) and 0x10 (REQ_16) are used for
> i2c. From the past experiences, REQ_16 works better when the size is 1, where REQ_14
> works better for bigger sizes.
>
> The third value gives the first byte of a write message and the i2c address. The lower
> 8 bits is the i2c address. The above sequence is playing with several different 
> i2c devices, at addresses 0x10, 0x32, 0xc0 and 0x1f.
>
> Most of the calls there are read (0xc0). I don't know any device that requires
> a read for it to work. I suspect that the above code is just probing to check
> what i2c devices are found at the board. The writes are to a device at address
> 0x32 (in i2c 8 bit notation - or 0x19 at i2c 7bit notation).
>
> I suspect that the probe sequence noticed something at the address 0x32 and is
> sending some init sequence for it. As this is not the tuner nor the demod, you
> don't need those setup for your device to work. Also, this address is not typical
> for eeprom. Without taking a look at the hardware, we can only guess what's there.
> My guess is that it is for some i2c-based remote controller chip. We don't need
> this for now. After having the rest working, we may need to return on it when
> patching ir-kbd.i2c.
>
>   
>> +		tm6000_set_reg(dev, REQ_03_SET_GET_MCU_PIN,
>> +				TM6010_GPIO_4, 1);
>> +		msleep(15);
>> +		tm6000_set_reg(dev, REQ_03_SET_GET_MCU_PIN,
>> +	    			TM6010_GPIO_0, 1);
>> +		msleep(15);
>> +		tm6000_set_reg(dev, REQ_03_SET_GET_MCU_PIN,
>> +				TM6010_GPIO_7, 0);
>> +		msleep(15);
>> +		tm6000_set_reg(dev, REQ_03_SET_GET_MCU_PIN,
>> +				TM6010_GPIO_5, 1);
>> +	
>> +		msleep(15);
>> +	
>> +		for (i=0; i< size; i++) {
>> +			rc= tm6000_set_reg (dev, tab[i].req, tab[i].reg, tab[i].val);
>> +			if (rc<0) {
>> +				printk (KERN_ERR "Error %i while setting req %d, "
>> +						 "reg %d to value %d\n", rc,
>> +						 tab[i].req,tab[i].reg, tab[i].val);
>> +				return rc;
>>  			}
>>  		}
>> +			
>> +		msleep(15);
>> +	
>> +		tm6000_set_reg(dev, REQ_03_SET_GET_MCU_PIN,
>> +				TM6010_GPIO_4, 0);
>> +		msleep(15);
>> +
>> +		tm6000_set_reg(dev, REQ_03_SET_GET_MCU_PIN,
>> +				TM6010_GPIO_1, 0);
>> +	
>> +		msleep(50);
>> +		tm6000_set_reg(dev, REQ_03_SET_GET_MCU_PIN,
>> +				TM6010_GPIO_1, 1);
>> +		
>> +		msleep(15);
>> +		tm6000_read_write_usb (dev, 0xc0, 0x0e, 0x00c2, 0x0008, buf, 2);
>> +//		printk(KERN_INFO "buf %#x %#x \n", buf[0], buf[1]);
>> +		msleep(15);
>> +		tm6000_set_reg(dev, REQ_03_SET_GET_MCU_PIN,
>> +				TM6010_GPIO_2, 1);
>> +		msleep(15);
>> +		tm6000_set_reg(dev, REQ_03_SET_GET_MCU_PIN,
>> +				TM6010_GPIO_2, 0);
>> +		msleep(15);
>> +		tm6000_set_reg(dev, REQ_03_SET_GET_MCU_PIN,
>> +				TM6010_GPIO_2, 1);
>> +		msleep(15);
>> +		tm6000_set_reg(dev, REQ_03_SET_GET_MCU_PIN,
>> +				TM6010_GPIO_2, 1);
>> +		msleep(15);
>> +		tm6000_set_reg(dev, REQ_03_SET_GET_MCU_PIN,
>> +				TM6010_GPIO_2, 0);
>> +		msleep(15);
>> +		tm6000_set_reg(dev, REQ_03_SET_GET_MCU_PIN,
>> +				TM6010_GPIO_2, 1);
>> +		msleep(15);
>>  	}
>> +	/* hack end 
>>     
> The above sequence is device-specific. Please add your code after I patch
> tm6000-cards.
>
> */
>   
>> +	
>> +	msleep(5); /* Just to be conservative */
>>  
>> +	/* Check board version - maybe 10Moons specific */
>> +	if (dev->dev_type == TM5600) {
>> +		 board=tm6000_get_reg16 (dev, 0x40, 0, 0);
>> +		if (board >=0) {
>> +			printk (KERN_INFO "Board version = 0x%04x\n",board);
>> +		} else {
>> +			printk (KERN_ERR "Error %i while retrieving board version\n",board);
>> +		}
>> +	}
>> +	
>>  	msleep(50);
>>  
>>  	return 0;
>>     
>
>   

I have a question, how can I implemented the reinit after activating
demodulator when it use tm6000_cards_setup().
  
Stefan Ringel Feb. 8, 2010, 5:34 p.m. UTC | #6
Am 08.02.2010 18:29, schrieb Mauro Carvalho Chehab:
> Stefan Ringel wrote:
>   
>> Am 08.02.2010 12:37, schrieb Mauro Carvalho Chehab:
>>     
>>> Mauro Carvalho Chehab wrote:
>>>   
>>>       
>>>>> +		tm6000_read_write_usb (dev, 0xc0, 0x10, 0x7f1f, 0x0000, buf, 2);
>>>>>       
>>>>>           
>>>   
>>>       
>>>> Most of the calls there are read (0xc0). I don't know any device that requires
>>>> a read for it to work. I suspect that the above code is just probing to check
>>>> what i2c devices are found at the board.
>>>>     
>>>>         
>>> Btw, by looking at drivers/media/dvb/frontends/zl10353_priv.h, we have an idea
>>> on what the above does:
>>>
>>> The register 0x7f is:
>>>
>>>         CHIP_ID            = 0x7F,
>>>
>>> So, basically, the above code is reading the ID of the chip, likely to be sure that it
>>> is a Zarlink 10353.
>>>
>>> Cheers,
>>> Mauro
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-media" in
>>> the body of a message to majordomo@vger.kernel.org
>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>>   
>>>       
>> yes, but that's for activating Zarlink zl10353 and checking it --> hello
>> Zarlink? If doesn't use that sequence, then cannot use Zarlink zl10353.
>>
>>     
> Are you sure about that? Is this a new bug on tm6000?
>
> Anyway, the proper place for such code is inside zl10353 driver, not outside.
>
>   

It cannot activate after load xc3028 firmware.
  
Stefan Ringel Feb. 8, 2010, 5:37 p.m. UTC | #7
Am 08.02.2010 18:34, schrieb Stefan Ringel:
> Am 08.02.2010 18:29, schrieb Mauro Carvalho Chehab:
>   
>> Stefan Ringel wrote:
>>   
>>     
>>> Am 08.02.2010 12:37, schrieb Mauro Carvalho Chehab:
>>>     
>>>       
>>>> Mauro Carvalho Chehab wrote:
>>>>   
>>>>       
>>>>         
>>>>>> +		tm6000_read_write_usb (dev, 0xc0, 0x10, 0x7f1f, 0x0000, buf, 2);
>>>>>>       
>>>>>>           
>>>>>>             
>>>>   
>>>>       
>>>>         
>>>>> Most of the calls there are read (0xc0). I don't know any device that requires
>>>>> a read for it to work. I suspect that the above code is just probing to check
>>>>> what i2c devices are found at the board.
>>>>>     
>>>>>         
>>>>>           
>>>> Btw, by looking at drivers/media/dvb/frontends/zl10353_priv.h, we have an idea
>>>> on what the above does:
>>>>
>>>> The register 0x7f is:
>>>>
>>>>         CHIP_ID            = 0x7F,
>>>>
>>>> So, basically, the above code is reading the ID of the chip, likely to be sure that it
>>>> is a Zarlink 10353.
>>>>
>>>> Cheers,
>>>> Mauro
>>>> --
>>>> To unsubscribe from this list: send the line "unsubscribe linux-media" in
>>>> the body of a message to majordomo@vger.kernel.org
>>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>>>   
>>>>       
>>>>         
>>> yes, but that's for activating Zarlink zl10353 and checking it --> hello
>>> Zarlink? If doesn't use that sequence, then cannot use Zarlink zl10353.
>>>
>>>     
>>>       
>> Are you sure about that? Is this a new bug on tm6000?
>>
>> Anyway, the proper place for such code is inside zl10353 driver, not outside.
>>
>>   
>>     
> It cannot activate after load xc3028 firmware.
>
>   
That part is I think it's board specific or tm6010.
  
Mauro Carvalho Chehab Feb. 8, 2010, 5:51 p.m. UTC | #8
Stefan Ringel wrote:
> Am 08.02.2010 18:34, schrieb Stefan Ringel:
>> Am 08.02.2010 18:29, schrieb Mauro Carvalho Chehab:
>>   
>>> Stefan Ringel wrote:
>>>   
>>>     
>>>> Am 08.02.2010 12:37, schrieb Mauro Carvalho Chehab:
>>>>     
>>>>       
>>>>> Mauro Carvalho Chehab wrote:
>>>>>   
>>>>>       
>>>>>         
>>>>>>> +		tm6000_read_write_usb (dev, 0xc0, 0x10, 0x7f1f, 0x0000, buf, 2);
>>>>>>>       
>>>>>>>           
>>>>>>>             
>>>>>   
>>>>>       
>>>>>         
>>>>>> Most of the calls there are read (0xc0). I don't know any device that requires
>>>>>> a read for it to work. I suspect that the above code is just probing to check
>>>>>> what i2c devices are found at the board.
>>>>>>     
>>>>>>         
>>>>>>           
>>>>> Btw, by looking at drivers/media/dvb/frontends/zl10353_priv.h, we have an idea
>>>>> on what the above does:
>>>>>
>>>>> The register 0x7f is:
>>>>>
>>>>>         CHIP_ID            = 0x7F,
>>>>>
>>>>> So, basically, the above code is reading the ID of the chip, likely to be sure that it
>>>>> is a Zarlink 10353.
>>>>>
>>>>> Cheers,
>>>>> Mauro
>>>>> --
>>>>> To unsubscribe from this list: send the line "unsubscribe linux-media" in
>>>>> the body of a message to majordomo@vger.kernel.org
>>>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>>>>   
>>>>>       
>>>>>         
>>>> yes, but that's for activating Zarlink zl10353 and checking it --> hello
>>>> Zarlink? If doesn't use that sequence, then cannot use Zarlink zl10353.
>>>>
>>>>     
>>>>       
>>> Are you sure about that? Is this a new bug on tm6000?
>>>
>>> Anyway, the proper place for such code is inside zl10353 driver, not outside.
>>>
>>>   
>>>     
>> It cannot activate after load xc3028 firmware.
>>
>>   
> That part is I think it's board specific or tm6010.
> 
Probably yet-another-i2c-bug-on-tm6000... Ah, well...

then, convert this call into an i2c call. You may get one example of such in em28xx-cards.
In that specific case, em28xx-based webcams can be shipped with more than one different
sensor. So, the driver needs to read the sensor from I2C:

        rc = i2c_master_recv(&dev->i2c_client, (char *)&version_be, 2);
        if (rc != 2)
                return -EINVAL;

Of course, you need to be sure to register the i2c bus before calling i2c_master_recv.
  
Mauro Carvalho Chehab Feb. 8, 2010, 6:14 p.m. UTC | #9
Stefan Ringel wrote:
> Am 08.02.2010 12:21, schrieb Mauro Carvalho Chehab:
>> Hi Stefan,
>>
>> First, a few comments about your patch series:
>>
>> I've committed almost of your patches, and added an extra patch to make the
>> driver to compile it with -git. There were other broken things when compiling
>> against -git.
>>
>> Several of your patches are adding leading whitespaces. Please review them before
>> submitting. On -git, those whitespaces are shown with a red background color.
>>
>> I've re-made most of the patch descriptions. Please take a look on them and try
>> to improve on a next time.
>>
>> We've got 2 submission for each patches. I just discarded the older one.
>>
>> I've removed the two BEHOLDER board descriptions from one of your patches. It is
>> not related to your board, but it is another compilation fix.
>>
>> From your series, I didn't merge those 3 patches:
>>
>> [5/12] tm6000: update init table and sequence for tm6010                http://patchwork.kernel.org/patch/77451
>> [6/12] tm6000: tuner reset timeing optimation                           http://patchwork.kernel.org/patch/77459
>> [11/12] tm6000: bugfix firmware xc3028L-v36.fw used with Zarlink and    http://patchwork.kernel.org/patch/77462
>>
>> I'll send you separate comments why I didn't merge them, in reply to each email you've sent,
>> starting with this one (patch 5/12).
>>
>>
>> stefan.ringel@arcor.de wrote:
>>   
>>> From: Stefan Ringel <stefan.ringel@arcor.de>
>>>
>>> ---
>>>  drivers/staging/tm6000/tm6000-core.c |  179 ++++++++++++++++++++++++----------
>>>  1 files changed, 128 insertions(+), 51 deletions(-)
>>>
>>> diff --git a/drivers/staging/tm6000/tm6000-core.c b/drivers/staging/tm6000/tm6000-core.c
>>> index 7ec13d5..a2e2af5 100644
>>> --- a/drivers/staging/tm6000/tm6000-core.c
>>> +++ b/drivers/staging/tm6000/tm6000-core.c
>>> @@ -414,7 +414,15 @@ struct reg_init tm6010_init_tab[] = {
>>>  	{ REQ_07_SET_GET_AVREG, 0x3f, 0x00 },
>>>  
>>>  	{ REQ_05_SET_GET_USBREG, 0x18, 0x00 },
>>> -
>>> +	
>>> +	/* additional from Terratec Cinergy Hybrid XE */
>>> +	{ REQ_07_SET_GET_AVREG, 0xdc, 0xaa },
>>> +	{ REQ_07_SET_GET_AVREG, 0xdd, 0x30 },
>>> +	{ REQ_07_SET_GET_AVREG, 0xde, 0x20 },
>>> +	{ REQ_07_SET_GET_AVREG, 0xdf, 0xd0 },
>>> +	{ REQ_04_EN_DISABLE_MCU_INT, 0x02, 0x00 },
>>> +	{ REQ_07_SET_GET_AVREG, 0xd8, 0x2f },
>>> +	
>>>  	/* set remote wakeup key:any key wakeup */
>>>  	{ REQ_07_SET_GET_AVREG,  0xe5,  0xfe },
>>>  	{ REQ_07_SET_GET_AVREG,  0xda,  0xff },
>>> @@ -424,6 +432,7 @@ int tm6000_init (struct tm6000_core *dev)
>>>  {
>>>  	int board, rc=0, i, size;
>>>  	struct reg_init *tab;
>>> +	u8 buf[40];
>>>  
>>>  	if (dev->dev_type == TM6010) {
>>>  		tab = tm6010_init_tab;
>>> @@ -444,61 +453,129 @@ int tm6000_init (struct tm6000_core *dev)
>>>  		}
>>>  	}
>>>  
>>> -	msleep(5); /* Just to be conservative */
>>> -
>>> -	/* Check board version - maybe 10Moons specific */
>>> -	board=tm6000_get_reg16 (dev, 0x40, 0, 0);
>>> -	if (board >=0) {
>>> -		printk (KERN_INFO "Board version = 0x%04x\n",board);
>>> -	} else {
>>> -		printk (KERN_ERR "Error %i while retrieving board version\n",board);
>>> -	}
>>> -
>>> +	/* hack */
>>>  	if (dev->dev_type == TM6010) {
>>> -		/* Turn xceive 3028 on */
>>> -		tm6000_set_reg(dev, REQ_03_SET_GET_MCU_PIN, TM6010_GPIO_3, 0x01);
>>> -		msleep(11);
>>> -	}
>>>     
>> The above is board-specific. It is needed for the tm6010 device I have here
>> (HVR900H), otherwise no xc3028 command will be handled.
>>
>> The better here is to add a setup routine to tm6000-cards and move all 
>> those GPIO codes to it. Then, add there your board-specific setup.
>>
>> I've added a patch that moves those GPIO board-specific setup to tm6000-cards:
>> tm6000_cards_setup(). Please move your board specific GPIO init to there.
>>
>>
>>   
>>> -
>>> -	/* Reset GPIO1 and GPIO4. */
>>> -	for (i=0; i< 2; i++) {
>>> -		rc = tm6000_set_reg(dev, REQ_03_SET_GET_MCU_PIN,
>>> -					dev->tuner_reset_gpio, 0x00);
>>> -		if (rc<0) {
>>> -			printk (KERN_ERR "Error %i doing GPIO1 reset\n",rc);
>>> -			return rc;
>>> -		}
>>> -
>>> -		msleep(10); /* Just to be conservative */
>>> -		rc = tm6000_set_reg(dev, REQ_03_SET_GET_MCU_PIN,
>>> -					dev->tuner_reset_gpio, 0x01);
>>> -		if (rc<0) {
>>> -			printk (KERN_ERR "Error %i doing GPIO1 reset\n",rc);
>>> -			return rc;
>>> -		}
>>> -
>>> -		msleep(10);
>>> -		rc=tm6000_set_reg (dev, REQ_03_SET_GET_MCU_PIN, TM6000_GPIO_4, 0);
>>> -		if (rc<0) {
>>> -			printk (KERN_ERR "Error %i doing GPIO4 reset\n",rc);
>>> -			return rc;
>>> -		}
>>> -
>>> -		msleep(10);
>>> -		rc=tm6000_set_reg (dev, REQ_03_SET_GET_MCU_PIN, TM6000_GPIO_4, 1);
>>> -		if (rc<0) {
>>> -			printk (KERN_ERR "Error %i doing GPIO4 reset\n",rc);
>>> -			return rc;
>>> -		}
>>> -
>>> -		if (!i) {
>>> -			rc=tm6000_get_reg16(dev, 0x40,0,0);
>>> -			if (rc>=0) {
>>> -				printk ("board=%d\n", rc);
>>> +		
>>> +		msleep(15);
>>> +		tm6000_set_reg(dev, REQ_03_SET_GET_MCU_PIN,
>>> +				TM6010_GPIO_4, 0);
>>> +		msleep(15);
>>> +				
>>> +		tm6000_set_reg(dev, REQ_03_SET_GET_MCU_PIN,
>>> +				TM6010_GPIO_1, 0);
>>> +	
>>> +		msleep(50);
>>> +		tm6000_set_reg(dev, REQ_03_SET_GET_MCU_PIN,
>>> +				TM6010_GPIO_1, 1);
>>> +		
>>>     
>> The above reflects the timing needed by your device. Depending on the board,
>> the sleep time may eventually be different.
>>
>>   
>>> +		msleep(15);
>>> +		tm6000_read_write_usb (dev, 0xc0, 0x0e, 0x0010, 0x4400, buf, 2);
>>> +		
>>> +		msleep(15);
>>> +		tm6000_read_write_usb (dev, 0xc0, 0x10, 0xf432, 0x0000, buf, 2);
>>> +	
>>> +		msleep(15);
>>> +		buf[0] = 0x12;
>>> +		buf[1] = 0x34;
>>> +		tm6000_read_write_usb (dev, 0x40, 0x10, 0xf432, 0x0000, buf, 2);
>>> +	
>>> +		msleep(15);
>>> +		tm6000_read_write_usb (dev, 0xc0, 0x10, 0xf432, 0x0000, buf, 2);
>>> +	
>>> +		msleep(15);
>>> +		tm6000_read_write_usb (dev, 0xc0, 0x10, 0x0032, 0x0000, buf, 2);
>>> +
>>> +		msleep(15);
>>> +		buf[0] = 0x00;
>>> +		buf[1] = 0x01;
>>> +		tm6000_read_write_usb (dev, 0x40, 0x10, 0xf332, 0x0000, buf, 2);
>>> +	
>>> +		msleep(15);
>>> +		tm6000_read_write_usb (dev, 0xc0, 0x10, 0x00c0, 0x0000, buf, 39);
>>> +	
>>> +		msleep(15);
>>> +		buf[0] = 0x00;
>>> +		buf[1] = 0x00;
>>> +		tm6000_read_write_usb (dev, 0x40, 0x10, 0xf332, 0x0000, buf, 2);
>>> +	
>>> +		msleep(15);
>>> +		tm6000_read_write_usb (dev, 0xc0, 0x10, 0x7f1f, 0x0000, buf, 2);
>>> +//		printk(KERN_INFO "buf %#x %#x \n", buf[0], buf [1]);
>>> +		msleep(15);
>>>     
>> At the above, you're just trying to reproduce whatever the original driver does,
>> instead of relying on the i2c drivers.
>>
>> At the Linux drivers, we don't just send random i2c sequences in the middle of
>> the setup. Instead, we let each i2c driver to do the initialization they need
>> to do. 
>>
>> If you take a look on each call, for example:
>> 		tm6000_read_write_usb (dev, 0x40, 0x10, 0xf332, 0x0000, buf, 2);
>>
>> The first value determines the USB direction: 0x40 is write; 0xc0 is read;
>> The second value is the request. Both 0x0e (REQ_14) and 0x10 (REQ_16) are used for
>> i2c. From the past experiences, REQ_16 works better when the size is 1, where REQ_14
>> works better for bigger sizes.
>>
>> The third value gives the first byte of a write message and the i2c address. The lower
>> 8 bits is the i2c address. The above sequence is playing with several different 
>> i2c devices, at addresses 0x10, 0x32, 0xc0 and 0x1f.
>>
>> Most of the calls there are read (0xc0). I don't know any device that requires
>> a read for it to work. I suspect that the above code is just probing to check
>> what i2c devices are found at the board. The writes are to a device at address
>> 0x32 (in i2c 8 bit notation - or 0x19 at i2c 7bit notation).
>>
>> I suspect that the probe sequence noticed something at the address 0x32 and is
>> sending some init sequence for it. As this is not the tuner nor the demod, you
>> don't need those setup for your device to work. Also, this address is not typical
>> for eeprom. Without taking a look at the hardware, we can only guess what's there.
>> My guess is that it is for some i2c-based remote controller chip. We don't need
>> this for now. After having the rest working, we may need to return on it when
>> patching ir-kbd.i2c.
>>
>>   
>>> +		tm6000_set_reg(dev, REQ_03_SET_GET_MCU_PIN,
>>> +				TM6010_GPIO_4, 1);
>>> +		msleep(15);
>>> +		tm6000_set_reg(dev, REQ_03_SET_GET_MCU_PIN,
>>> +	    			TM6010_GPIO_0, 1);
>>> +		msleep(15);
>>> +		tm6000_set_reg(dev, REQ_03_SET_GET_MCU_PIN,
>>> +				TM6010_GPIO_7, 0);
>>> +		msleep(15);
>>> +		tm6000_set_reg(dev, REQ_03_SET_GET_MCU_PIN,
>>> +				TM6010_GPIO_5, 1);
>>> +	
>>> +		msleep(15);
>>> +	
>>> +		for (i=0; i< size; i++) {
>>> +			rc= tm6000_set_reg (dev, tab[i].req, tab[i].reg, tab[i].val);
>>> +			if (rc<0) {
>>> +				printk (KERN_ERR "Error %i while setting req %d, "
>>> +						 "reg %d to value %d\n", rc,
>>> +						 tab[i].req,tab[i].reg, tab[i].val);
>>> +				return rc;
>>>  			}
>>>  		}
>>> +			
>>> +		msleep(15);
>>> +	
>>> +		tm6000_set_reg(dev, REQ_03_SET_GET_MCU_PIN,
>>> +				TM6010_GPIO_4, 0);
>>> +		msleep(15);
>>> +
>>> +		tm6000_set_reg(dev, REQ_03_SET_GET_MCU_PIN,
>>> +				TM6010_GPIO_1, 0);
>>> +	
>>> +		msleep(50);
>>> +		tm6000_set_reg(dev, REQ_03_SET_GET_MCU_PIN,
>>> +				TM6010_GPIO_1, 1);
>>> +		
>>> +		msleep(15);
>>> +		tm6000_read_write_usb (dev, 0xc0, 0x0e, 0x00c2, 0x0008, buf, 2);
>>> +//		printk(KERN_INFO "buf %#x %#x \n", buf[0], buf[1]);
>>> +		msleep(15);
>>> +		tm6000_set_reg(dev, REQ_03_SET_GET_MCU_PIN,
>>> +				TM6010_GPIO_2, 1);
>>> +		msleep(15);
>>> +		tm6000_set_reg(dev, REQ_03_SET_GET_MCU_PIN,
>>> +				TM6010_GPIO_2, 0);
>>> +		msleep(15);
>>> +		tm6000_set_reg(dev, REQ_03_SET_GET_MCU_PIN,
>>> +				TM6010_GPIO_2, 1);
>>> +		msleep(15);
>>> +		tm6000_set_reg(dev, REQ_03_SET_GET_MCU_PIN,
>>> +				TM6010_GPIO_2, 1);
>>> +		msleep(15);
>>> +		tm6000_set_reg(dev, REQ_03_SET_GET_MCU_PIN,
>>> +				TM6010_GPIO_2, 0);
>>> +		msleep(15);
>>> +		tm6000_set_reg(dev, REQ_03_SET_GET_MCU_PIN,
>>> +				TM6010_GPIO_2, 1);
>>> +		msleep(15);
>>>  	}
>>> +	/* hack end 
>>>     
>> The above sequence is device-specific. Please add your code after I patch
>> tm6000-cards.
>>
>> */
>>   
>>> +	
>>> +	msleep(5); /* Just to be conservative */
>>>  
>>> +	/* Check board version - maybe 10Moons specific */
>>> +	if (dev->dev_type == TM5600) {
>>> +		 board=tm6000_get_reg16 (dev, 0x40, 0, 0);
>>> +		if (board >=0) {
>>> +			printk (KERN_INFO "Board version = 0x%04x\n",board);
>>> +		} else {
>>> +			printk (KERN_ERR "Error %i while retrieving board version\n",board);
>>> +		}
>>> +	}
>>> +	
>>>  	msleep(50);
>>>  
>>>  	return 0;
>>>     
>>   
> 
> I have a question, how can I implemented the reinit after activating
> demodulator when it use tm6000_cards_setup().

We'll need some function to change between analog and digital modes, doing the right
GPIO changes. See em28xx_set_mode() for a way of implementing it.
  
Stefan Ringel Feb. 8, 2010, 7:07 p.m. UTC | #10
Am 08.02.2010 19:14, schrieb Mauro Carvalho Chehab:
> Stefan Ringel wrote:
>   
>> Am 08.02.2010 12:21, schrieb Mauro Carvalho Chehab:
>>     
>>> Hi Stefan,
>>>
>>> First, a few comments about your patch series:
>>>
>>> I've committed almost of your patches, and added an extra patch to make the
>>> driver to compile it with -git. There were other broken things when compiling
>>> against -git.
>>>
>>> Several of your patches are adding leading whitespaces. Please review them before
>>> submitting. On -git, those whitespaces are shown with a red background color.
>>>
>>> I've re-made most of the patch descriptions. Please take a look on them and try
>>> to improve on a next time.
>>>
>>> We've got 2 submission for each patches. I just discarded the older one.
>>>
>>> I've removed the two BEHOLDER board descriptions from one of your patches. It is
>>> not related to your board, but it is another compilation fix.
>>>
>>> From your series, I didn't merge those 3 patches:
>>>
>>> [5/12] tm6000: update init table and sequence for tm6010                http://patchwork.kernel.org/patch/77451
>>> [6/12] tm6000: tuner reset timeing optimation                           http://patchwork.kernel.org/patch/77459
>>> [11/12] tm6000: bugfix firmware xc3028L-v36.fw used with Zarlink and    http://patchwork.kernel.org/patch/77462
>>>
>>> I'll send you separate comments why I didn't merge them, in reply to each email you've sent,
>>> starting with this one (patch 5/12).
>>>
>>>
>>> stefan.ringel@arcor.de wrote:
>>>   
>>>       
>>>> From: Stefan Ringel <stefan.ringel@arcor.de>
>>>>
>>>> ---
>>>>  drivers/staging/tm6000/tm6000-core.c |  179 ++++++++++++++++++++++++----------
>>>>  1 files changed, 128 insertions(+), 51 deletions(-)
>>>>
>>>> diff --git a/drivers/staging/tm6000/tm6000-core.c b/drivers/staging/tm6000/tm6000-core.c
>>>> index 7ec13d5..a2e2af5 100644
>>>> --- a/drivers/staging/tm6000/tm6000-core.c
>>>> +++ b/drivers/staging/tm6000/tm6000-core.c
>>>> @@ -414,7 +414,15 @@ struct reg_init tm6010_init_tab[] = {
>>>>  	{ REQ_07_SET_GET_AVREG, 0x3f, 0x00 },
>>>>  
>>>>  	{ REQ_05_SET_GET_USBREG, 0x18, 0x00 },
>>>> -
>>>> +	
>>>> +	/* additional from Terratec Cinergy Hybrid XE */
>>>> +	{ REQ_07_SET_GET_AVREG, 0xdc, 0xaa },
>>>> +	{ REQ_07_SET_GET_AVREG, 0xdd, 0x30 },
>>>> +	{ REQ_07_SET_GET_AVREG, 0xde, 0x20 },
>>>> +	{ REQ_07_SET_GET_AVREG, 0xdf, 0xd0 },
>>>> +	{ REQ_04_EN_DISABLE_MCU_INT, 0x02, 0x00 },
>>>> +	{ REQ_07_SET_GET_AVREG, 0xd8, 0x2f },
>>>> +	
>>>>  	/* set remote wakeup key:any key wakeup */
>>>>  	{ REQ_07_SET_GET_AVREG,  0xe5,  0xfe },
>>>>  	{ REQ_07_SET_GET_AVREG,  0xda,  0xff },
>>>> @@ -424,6 +432,7 @@ int tm6000_init (struct tm6000_core *dev)
>>>>  {
>>>>  	int board, rc=0, i, size;
>>>>  	struct reg_init *tab;
>>>> +	u8 buf[40];
>>>>  
>>>>  	if (dev->dev_type == TM6010) {
>>>>  		tab = tm6010_init_tab;
>>>> @@ -444,61 +453,129 @@ int tm6000_init (struct tm6000_core *dev)
>>>>  		}
>>>>  	}
>>>>  
>>>> -	msleep(5); /* Just to be conservative */
>>>> -
>>>> -	/* Check board version - maybe 10Moons specific */
>>>> -	board=tm6000_get_reg16 (dev, 0x40, 0, 0);
>>>> -	if (board >=0) {
>>>> -		printk (KERN_INFO "Board version = 0x%04x\n",board);
>>>> -	} else {
>>>> -		printk (KERN_ERR "Error %i while retrieving board version\n",board);
>>>> -	}
>>>> -
>>>> +	/* hack */
>>>>  	if (dev->dev_type == TM6010) {
>>>> -		/* Turn xceive 3028 on */
>>>> -		tm6000_set_reg(dev, REQ_03_SET_GET_MCU_PIN, TM6010_GPIO_3, 0x01);
>>>> -		msleep(11);
>>>> -	}
>>>>     
>>>>         
>>> The above is board-specific. It is needed for the tm6010 device I have here
>>> (HVR900H), otherwise no xc3028 command will be handled.
>>>
>>> The better here is to add a setup routine to tm6000-cards and move all 
>>> those GPIO codes to it. Then, add there your board-specific setup.
>>>
>>> I've added a patch that moves those GPIO board-specific setup to tm6000-cards:
>>> tm6000_cards_setup(). Please move your board specific GPIO init to there.
>>>
>>>
>>>   
>>>       
>>>> -
>>>> -	/* Reset GPIO1 and GPIO4. */
>>>> -	for (i=0; i< 2; i++) {
>>>> -		rc = tm6000_set_reg(dev, REQ_03_SET_GET_MCU_PIN,
>>>> -					dev->tuner_reset_gpio, 0x00);
>>>> -		if (rc<0) {
>>>> -			printk (KERN_ERR "Error %i doing GPIO1 reset\n",rc);
>>>> -			return rc;
>>>> -		}
>>>> -
>>>> -		msleep(10); /* Just to be conservative */
>>>> -		rc = tm6000_set_reg(dev, REQ_03_SET_GET_MCU_PIN,
>>>> -					dev->tuner_reset_gpio, 0x01);
>>>> -		if (rc<0) {
>>>> -			printk (KERN_ERR "Error %i doing GPIO1 reset\n",rc);
>>>> -			return rc;
>>>> -		}
>>>> -
>>>> -		msleep(10);
>>>> -		rc=tm6000_set_reg (dev, REQ_03_SET_GET_MCU_PIN, TM6000_GPIO_4, 0);
>>>> -		if (rc<0) {
>>>> -			printk (KERN_ERR "Error %i doing GPIO4 reset\n",rc);
>>>> -			return rc;
>>>> -		}
>>>> -
>>>> -		msleep(10);
>>>> -		rc=tm6000_set_reg (dev, REQ_03_SET_GET_MCU_PIN, TM6000_GPIO_4, 1);
>>>> -		if (rc<0) {
>>>> -			printk (KERN_ERR "Error %i doing GPIO4 reset\n",rc);
>>>> -			return rc;
>>>> -		}
>>>> -
>>>> -		if (!i) {
>>>> -			rc=tm6000_get_reg16(dev, 0x40,0,0);
>>>> -			if (rc>=0) {
>>>> -				printk ("board=%d\n", rc);
>>>> +		
>>>> +		msleep(15);
>>>> +		tm6000_set_reg(dev, REQ_03_SET_GET_MCU_PIN,
>>>> +				TM6010_GPIO_4, 0);
>>>> +		msleep(15);
>>>> +				
>>>> +		tm6000_set_reg(dev, REQ_03_SET_GET_MCU_PIN,
>>>> +				TM6010_GPIO_1, 0);
>>>> +	
>>>> +		msleep(50);
>>>> +		tm6000_set_reg(dev, REQ_03_SET_GET_MCU_PIN,
>>>> +				TM6010_GPIO_1, 1);
>>>> +		
>>>>     
>>>>         
>>> The above reflects the timing needed by your device. Depending on the board,
>>> the sleep time may eventually be different.
>>>
>>>   
>>>       
>>>> +		msleep(15);
>>>> +		tm6000_read_write_usb (dev, 0xc0, 0x0e, 0x0010, 0x4400, buf, 2);
>>>> +		
>>>> +		msleep(15);
>>>> +		tm6000_read_write_usb (dev, 0xc0, 0x10, 0xf432, 0x0000, buf, 2);
>>>> +	
>>>> +		msleep(15);
>>>> +		buf[0] = 0x12;
>>>> +		buf[1] = 0x34;
>>>> +		tm6000_read_write_usb (dev, 0x40, 0x10, 0xf432, 0x0000, buf, 2);
>>>> +	
>>>> +		msleep(15);
>>>> +		tm6000_read_write_usb (dev, 0xc0, 0x10, 0xf432, 0x0000, buf, 2);
>>>> +	
>>>> +		msleep(15);
>>>> +		tm6000_read_write_usb (dev, 0xc0, 0x10, 0x0032, 0x0000, buf, 2);
>>>> +
>>>> +		msleep(15);
>>>> +		buf[0] = 0x00;
>>>> +		buf[1] = 0x01;
>>>> +		tm6000_read_write_usb (dev, 0x40, 0x10, 0xf332, 0x0000, buf, 2);
>>>> +	
>>>> +		msleep(15);
>>>> +		tm6000_read_write_usb (dev, 0xc0, 0x10, 0x00c0, 0x0000, buf, 39);
>>>> +	
>>>> +		msleep(15);
>>>> +		buf[0] = 0x00;
>>>> +		buf[1] = 0x00;
>>>> +		tm6000_read_write_usb (dev, 0x40, 0x10, 0xf332, 0x0000, buf, 2);
>>>> +	
>>>> +		msleep(15);
>>>> +		tm6000_read_write_usb (dev, 0xc0, 0x10, 0x7f1f, 0x0000, buf, 2);
>>>> +//		printk(KERN_INFO "buf %#x %#x \n", buf[0], buf [1]);
>>>> +		msleep(15);
>>>>     
>>>>         
>>> At the above, you're just trying to reproduce whatever the original driver does,
>>> instead of relying on the i2c drivers.
>>>
>>> At the Linux drivers, we don't just send random i2c sequences in the middle of
>>> the setup. Instead, we let each i2c driver to do the initialization they need
>>> to do. 
>>>
>>> If you take a look on each call, for example:
>>> 		tm6000_read_write_usb (dev, 0x40, 0x10, 0xf332, 0x0000, buf, 2);
>>>
>>> The first value determines the USB direction: 0x40 is write; 0xc0 is read;
>>> The second value is the request. Both 0x0e (REQ_14) and 0x10 (REQ_16) are used for
>>> i2c. From the past experiences, REQ_16 works better when the size is 1, where REQ_14
>>> works better for bigger sizes.
>>>
>>> The third value gives the first byte of a write message and the i2c address. The lower
>>> 8 bits is the i2c address. The above sequence is playing with several different 
>>> i2c devices, at addresses 0x10, 0x32, 0xc0 and 0x1f.
>>>
>>> Most of the calls there are read (0xc0). I don't know any device that requires
>>> a read for it to work. I suspect that the above code is just probing to check
>>> what i2c devices are found at the board. The writes are to a device at address
>>> 0x32 (in i2c 8 bit notation - or 0x19 at i2c 7bit notation).
>>>
>>> I suspect that the probe sequence noticed something at the address 0x32 and is
>>> sending some init sequence for it. As this is not the tuner nor the demod, you
>>> don't need those setup for your device to work. Also, this address is not typical
>>> for eeprom. Without taking a look at the hardware, we can only guess what's there.
>>> My guess is that it is for some i2c-based remote controller chip. We don't need
>>> this for now. After having the rest working, we may need to return on it when
>>> patching ir-kbd.i2c.
>>>
>>>   
>>>       
>>>> +		tm6000_set_reg(dev, REQ_03_SET_GET_MCU_PIN,
>>>> +				TM6010_GPIO_4, 1);
>>>> +		msleep(15);
>>>> +		tm6000_set_reg(dev, REQ_03_SET_GET_MCU_PIN,
>>>> +	    			TM6010_GPIO_0, 1);
>>>> +		msleep(15);
>>>> +		tm6000_set_reg(dev, REQ_03_SET_GET_MCU_PIN,
>>>> +				TM6010_GPIO_7, 0);
>>>> +		msleep(15);
>>>> +		tm6000_set_reg(dev, REQ_03_SET_GET_MCU_PIN,
>>>> +				TM6010_GPIO_5, 1);
>>>> +	
>>>> +		msleep(15);
>>>> +	
>>>> +		for (i=0; i< size; i++) {
>>>> +			rc= tm6000_set_reg (dev, tab[i].req, tab[i].reg, tab[i].val);
>>>> +			if (rc<0) {
>>>> +				printk (KERN_ERR "Error %i while setting req %d, "
>>>> +						 "reg %d to value %d\n", rc,
>>>> +						 tab[i].req,tab[i].reg, tab[i].val);
>>>> +				return rc;
>>>>  			}
>>>>  		}
>>>> +			
>>>> +		msleep(15);
>>>> +	
>>>> +		tm6000_set_reg(dev, REQ_03_SET_GET_MCU_PIN,
>>>> +				TM6010_GPIO_4, 0);
>>>> +		msleep(15);
>>>> +
>>>> +		tm6000_set_reg(dev, REQ_03_SET_GET_MCU_PIN,
>>>> +				TM6010_GPIO_1, 0);
>>>> +	
>>>> +		msleep(50);
>>>> +		tm6000_set_reg(dev, REQ_03_SET_GET_MCU_PIN,
>>>> +				TM6010_GPIO_1, 1);
>>>> +		
>>>> +		msleep(15);
>>>> +		tm6000_read_write_usb (dev, 0xc0, 0x0e, 0x00c2, 0x0008, buf, 2);
>>>> +//		printk(KERN_INFO "buf %#x %#x \n", buf[0], buf[1]);
>>>> +		msleep(15);
>>>> +		tm6000_set_reg(dev, REQ_03_SET_GET_MCU_PIN,
>>>> +				TM6010_GPIO_2, 1);
>>>> +		msleep(15);
>>>> +		tm6000_set_reg(dev, REQ_03_SET_GET_MCU_PIN,
>>>> +				TM6010_GPIO_2, 0);
>>>> +		msleep(15);
>>>> +		tm6000_set_reg(dev, REQ_03_SET_GET_MCU_PIN,
>>>> +				TM6010_GPIO_2, 1);
>>>> +		msleep(15);
>>>> +		tm6000_set_reg(dev, REQ_03_SET_GET_MCU_PIN,
>>>> +				TM6010_GPIO_2, 1);
>>>> +		msleep(15);
>>>> +		tm6000_set_reg(dev, REQ_03_SET_GET_MCU_PIN,
>>>> +				TM6010_GPIO_2, 0);
>>>> +		msleep(15);
>>>> +		tm6000_set_reg(dev, REQ_03_SET_GET_MCU_PIN,
>>>> +				TM6010_GPIO_2, 1);
>>>> +		msleep(15);
>>>>  	}
>>>> +	/* hack end 
>>>>     
>>>>         
>>> The above sequence is device-specific. Please add your code after I patch
>>> tm6000-cards.
>>>
>>> */
>>>   
>>>       
>>>> +	
>>>> +	msleep(5); /* Just to be conservative */
>>>>  
>>>> +	/* Check board version - maybe 10Moons specific */
>>>> +	if (dev->dev_type == TM5600) {
>>>> +		 board=tm6000_get_reg16 (dev, 0x40, 0, 0);
>>>> +		if (board >=0) {
>>>> +			printk (KERN_INFO "Board version = 0x%04x\n",board);
>>>> +		} else {
>>>> +			printk (KERN_ERR "Error %i while retrieving board version\n",board);
>>>> +		}
>>>> +	}
>>>> +	
>>>>  	msleep(50);
>>>>  
>>>>  	return 0;
>>>>     
>>>>         
>>>   
>>>       
>> I have a question, how can I implemented the reinit after activating
>> demodulator when it use tm6000_cards_setup().
>>     
> We'll need some function to change between analog and digital modes, doing the right
> GPIO changes. See em28xx_set_mode() for a way of implementing it.
>
>   
I don't mean that. I mean it loads the init table then goes to
tm600_cards_setup, then goes to return and loads the init table new and
then ... reset the demodulator or can it without the reset demodulator?
I can test it next weekend.

cheers

Stefan Ringel
  
Mauro Carvalho Chehab Feb. 8, 2010, 7:17 p.m. UTC | #11
Stefan Ringel wrote:
> Am 08.02.2010 19:14, schrieb Mauro Carvalho Chehab:
>> Stefan Ringel wrote:
>>   
    
>> We'll need some function to change between analog and digital modes, doing the right
>> GPIO changes. See em28xx_set_mode() for a way of implementing it.
>>
>>   
> I don't mean that. I mean it loads the init table then goes to
> tm600_cards_setup, then goes to return and loads the init table new and
> then ... reset the demodulator or can it without the reset demodulator?
> I can test it next weekend.

Tests are required. Maybe you'll need to call it again. The tm6000 chip has lot of
weird behaviours. In the case of xc3028 on analog, you need to re-load the firmware
every time the stream starts. Also, it seems that tm6000 has a timeout: if the image
is not ok for a few seconds, it cuts the tuner down. So, I ended to make it to re-load
part of the firmware (the smaller part of the firmware) every time the channel changes,
when I wrote the first version of the driver. I suspect that this behavior of tuner-xc2028
were removed on the last driver reviews, to speedup tuning with all other devices that
use those chips.
  
Stefan Ringel Feb. 8, 2010, 8:04 p.m. UTC | #12
I have switched from hack to zl10353 module, and  I have tested more
different setups. I have found what wrong is, in function
tl10353_read_status() and zl10353_read_snr(), not positive value.

zl10353_read_status()

reg                        has digital                            hasn't
digital

0x05                    0x40                                    0x00
0x06                    0x00                                    0x21
0x07                    0x33                                    0x03
0x08                    0x00                                    0x00
more than 0x00
0x09                    0x58                                    0x00

zl10353_read_snr()

reg                        has digital                            hasn't
digital

0x0f                     0x28   inv ( 0xd8) =^87%    0x2b  inv (0xd5) =^ 84%
0x10                    0x00                                    0x00


the function set_parameters is working. I have added (only for test) a
full HAS_FE_* status, so that I tested the set_parameter function.
  
Stefan Ringel Feb. 10, 2010, 5:09 p.m. UTC | #13
Am 08.02.2010 12:21, schrieb Mauro Carvalho Chehab:
>
> At the above, you're just trying to reproduce whatever the original driver does,
> instead of relying on the i2c drivers.
>
> At the Linux drivers, we don't just send random i2c sequences in the middle of
> the setup. Instead, we let each i2c driver to do the initialization they need
> to do. 
>
> If you take a look on each call, for example:
> 		tm6000_read_write_usb (dev, 0x40, 0x10, 0xf332, 0x0000, buf, 2);
>
> The first value determines the USB direction: 0x40 is write; 0xc0 is read;
> The second value is the request. Both 0x0e (REQ_14) and 0x10 (REQ_16) are used for
> i2c. From the past experiences, REQ_16 works better when the size is 1, where REQ_14
> works better for bigger sizes.
>
> The third value gives the first byte of a write message and the i2c address. The lower
> 8 bits is the i2c address. The above sequence is playing with several different 
> i2c devices, at addresses 0x10, 0x32, 0xc0 and 0x1f.
>
> Most of the calls there are read (0xc0). I don't know any device that requires
> a read for it to work. I suspect that the above code is just probing to check
> what i2c devices are found at the board. The writes are to a device at address
> 0x32 (in i2c 8 bit notation - or 0x19 at i2c 7bit notation).
>
> I suspect that the probe sequence noticed something at the address 0x32 and is
> sending some init sequence for it. As this is not the tuner nor the demod, you
> don't need those setup for your device to work. Also, this address is not typical
> for eeprom. Without taking a look at the hardware, we can only guess what's there.
> My guess is that it is for some i2c-based remote controller chip. We don't need
> this for now. After having the rest working, we may need to return on it when
> patching ir-kbd.i2c.
>   

The i2c address 0x32 isn't ir, but I think it sets the power or so. Ir
has vendor requests. see tm6000_regs.h . The i2c addresses 0x10 and 0xc0
cannot say what this is (check i2c address space or so). and the i2c
address 0x1f is the read address from demodulator.
  
Mauro Carvalho Chehab Feb. 10, 2010, 5:21 p.m. UTC | #14
Stefan Ringel wrote:
> Am 08.02.2010 12:21, schrieb Mauro Carvalho Chehab:
>> At the above, you're just trying to reproduce whatever the original driver does,
>> instead of relying on the i2c drivers.
>>
>> At the Linux drivers, we don't just send random i2c sequences in the middle of
>> the setup. Instead, we let each i2c driver to do the initialization they need
>> to do. 
>>
>> If you take a look on each call, for example:
>> 		tm6000_read_write_usb (dev, 0x40, 0x10, 0xf332, 0x0000, buf, 2);
>>
>> The first value determines the USB direction: 0x40 is write; 0xc0 is read;
>> The second value is the request. Both 0x0e (REQ_14) and 0x10 (REQ_16) are used for
>> i2c. From the past experiences, REQ_16 works better when the size is 1, where REQ_14
>> works better for bigger sizes.
>>
>> The third value gives the first byte of a write message and the i2c address. The lower
>> 8 bits is the i2c address. The above sequence is playing with several different 
>> i2c devices, at addresses 0x10, 0x32, 0xc0 and 0x1f.
>>
>> Most of the calls there are read (0xc0). I don't know any device that requires
>> a read for it to work. I suspect that the above code is just probing to check
>> what i2c devices are found at the board. The writes are to a device at address
>> 0x32 (in i2c 8 bit notation - or 0x19 at i2c 7bit notation).
>>
>> I suspect that the probe sequence noticed something at the address 0x32 and is
>> sending some init sequence for it. As this is not the tuner nor the demod, you
>> don't need those setup for your device to work. Also, this address is not typical
>> for eeprom. Without taking a look at the hardware, we can only guess what's there.
>> My guess is that it is for some i2c-based remote controller chip. We don't need
>> this for now. After having the rest working, we may need to return on it when
>> patching ir-kbd.i2c.
>>   
> 
> The i2c address 0x32 isn't ir, but I think it sets the power or so. Ir
> has vendor requests. see tm6000_regs.h .

The way IR works depend on vendor's decision. Even having some URB messages dedicated
to IR, someone may use instead an i2c chip.

If 0x32 is not IR, then maybe it might be an audio demod. The only way to know for sure
is to open a device that answers to this address and see what chips are inside.

 The i2c addresses 0x10 and 0xc0
> cannot say what this is (check i2c address space or so). and the i2c
> address 0x1f is the read address from demodulator.

0xc0 is for sure tuner. Allmost 100% of the tuners in the market (including xc3028) can
use 0xc0. All it is needed to change is to send 5V or GND to certain pins at the tuner,
chip in order to define what address will be used. Yet, all projects I've seen so far with
xc3028 use 0xc2.

0x10 can be used by some AM/FM radio chips with RDS.

Anyway, I'm almost sure that your board doesn't have any of those chips. If you're in doubt,
then you'll need to open your device and look what chips are inside, seeking for datasheets
or other info for the other i2c chips you might find.
  

Patch

diff --git a/drivers/staging/tm6000/tm6000-core.c b/drivers/staging/tm6000/tm6000-core.c
index 7ec13d5..a2e2af5 100644
--- a/drivers/staging/tm6000/tm6000-core.c
+++ b/drivers/staging/tm6000/tm6000-core.c
@@ -414,7 +414,15 @@  struct reg_init tm6010_init_tab[] = {
 	{ REQ_07_SET_GET_AVREG, 0x3f, 0x00 },
 
 	{ REQ_05_SET_GET_USBREG, 0x18, 0x00 },
-
+	
+	/* additional from Terratec Cinergy Hybrid XE */
+	{ REQ_07_SET_GET_AVREG, 0xdc, 0xaa },
+	{ REQ_07_SET_GET_AVREG, 0xdd, 0x30 },
+	{ REQ_07_SET_GET_AVREG, 0xde, 0x20 },
+	{ REQ_07_SET_GET_AVREG, 0xdf, 0xd0 },
+	{ REQ_04_EN_DISABLE_MCU_INT, 0x02, 0x00 },
+	{ REQ_07_SET_GET_AVREG, 0xd8, 0x2f },
+	
 	/* set remote wakeup key:any key wakeup */
 	{ REQ_07_SET_GET_AVREG,  0xe5,  0xfe },
 	{ REQ_07_SET_GET_AVREG,  0xda,  0xff },
@@ -424,6 +432,7 @@  int tm6000_init (struct tm6000_core *dev)
 {
 	int board, rc=0, i, size;
 	struct reg_init *tab;
+	u8 buf[40];
 
 	if (dev->dev_type == TM6010) {
 		tab = tm6010_init_tab;
@@ -444,61 +453,129 @@  int tm6000_init (struct tm6000_core *dev)
 		}
 	}
 
-	msleep(5); /* Just to be conservative */
-
-	/* Check board version - maybe 10Moons specific */
-	board=tm6000_get_reg16 (dev, 0x40, 0, 0);
-	if (board >=0) {
-		printk (KERN_INFO "Board version = 0x%04x\n",board);
-	} else {
-		printk (KERN_ERR "Error %i while retrieving board version\n",board);
-	}
-
+	/* hack */
 	if (dev->dev_type == TM6010) {
-		/* Turn xceive 3028 on */
-		tm6000_set_reg(dev, REQ_03_SET_GET_MCU_PIN, TM6010_GPIO_3, 0x01);
-		msleep(11);
-	}
-
-	/* Reset GPIO1 and GPIO4. */
-	for (i=0; i< 2; i++) {
-		rc = tm6000_set_reg(dev, REQ_03_SET_GET_MCU_PIN,
-					dev->tuner_reset_gpio, 0x00);
-		if (rc<0) {
-			printk (KERN_ERR "Error %i doing GPIO1 reset\n",rc);
-			return rc;
-		}
-
-		msleep(10); /* Just to be conservative */
-		rc = tm6000_set_reg(dev, REQ_03_SET_GET_MCU_PIN,
-					dev->tuner_reset_gpio, 0x01);
-		if (rc<0) {
-			printk (KERN_ERR "Error %i doing GPIO1 reset\n",rc);
-			return rc;
-		}
-
-		msleep(10);
-		rc=tm6000_set_reg (dev, REQ_03_SET_GET_MCU_PIN, TM6000_GPIO_4, 0);
-		if (rc<0) {
-			printk (KERN_ERR "Error %i doing GPIO4 reset\n",rc);
-			return rc;
-		}
-
-		msleep(10);
-		rc=tm6000_set_reg (dev, REQ_03_SET_GET_MCU_PIN, TM6000_GPIO_4, 1);
-		if (rc<0) {
-			printk (KERN_ERR "Error %i doing GPIO4 reset\n",rc);
-			return rc;
-		}
-
-		if (!i) {
-			rc=tm6000_get_reg16(dev, 0x40,0,0);
-			if (rc>=0) {
-				printk ("board=%d\n", rc);
+		
+		msleep(15);
+		tm6000_set_reg(dev, REQ_03_SET_GET_MCU_PIN,
+				TM6010_GPIO_4, 0);
+		msleep(15);
+				
+		tm6000_set_reg(dev, REQ_03_SET_GET_MCU_PIN,
+				TM6010_GPIO_1, 0);
+	
+		msleep(50);
+		tm6000_set_reg(dev, REQ_03_SET_GET_MCU_PIN,
+				TM6010_GPIO_1, 1);
+		
+		msleep(15);
+		tm6000_read_write_usb (dev, 0xc0, 0x0e, 0x0010, 0x4400, buf, 2);
+		
+		msleep(15);
+		tm6000_read_write_usb (dev, 0xc0, 0x10, 0xf432, 0x0000, buf, 2);
+	
+		msleep(15);
+		buf[0] = 0x12;
+		buf[1] = 0x34;
+		tm6000_read_write_usb (dev, 0x40, 0x10, 0xf432, 0x0000, buf, 2);
+	
+		msleep(15);
+		tm6000_read_write_usb (dev, 0xc0, 0x10, 0xf432, 0x0000, buf, 2);
+	
+		msleep(15);
+		tm6000_read_write_usb (dev, 0xc0, 0x10, 0x0032, 0x0000, buf, 2);
+
+		msleep(15);
+		buf[0] = 0x00;
+		buf[1] = 0x01;
+		tm6000_read_write_usb (dev, 0x40, 0x10, 0xf332, 0x0000, buf, 2);
+	
+		msleep(15);
+		tm6000_read_write_usb (dev, 0xc0, 0x10, 0x00c0, 0x0000, buf, 39);
+	
+		msleep(15);
+		buf[0] = 0x00;
+		buf[1] = 0x00;
+		tm6000_read_write_usb (dev, 0x40, 0x10, 0xf332, 0x0000, buf, 2);
+	
+		msleep(15);
+		tm6000_read_write_usb (dev, 0xc0, 0x10, 0x7f1f, 0x0000, buf, 2);
+//		printk(KERN_INFO "buf %#x %#x \n", buf[0], buf [1]);
+		msleep(15);
+		tm6000_set_reg(dev, REQ_03_SET_GET_MCU_PIN,
+				TM6010_GPIO_4, 1);
+		msleep(15);
+		tm6000_set_reg(dev, REQ_03_SET_GET_MCU_PIN,
+	    			TM6010_GPIO_0, 1);
+		msleep(15);
+		tm6000_set_reg(dev, REQ_03_SET_GET_MCU_PIN,
+				TM6010_GPIO_7, 0);
+		msleep(15);
+		tm6000_set_reg(dev, REQ_03_SET_GET_MCU_PIN,
+				TM6010_GPIO_5, 1);
+	
+		msleep(15);
+	
+		for (i=0; i< size; i++) {
+			rc= tm6000_set_reg (dev, tab[i].req, tab[i].reg, tab[i].val);
+			if (rc<0) {
+				printk (KERN_ERR "Error %i while setting req %d, "
+						 "reg %d to value %d\n", rc,
+						 tab[i].req,tab[i].reg, tab[i].val);
+				return rc;
 			}
 		}
+			
+		msleep(15);
+	
+		tm6000_set_reg(dev, REQ_03_SET_GET_MCU_PIN,
+				TM6010_GPIO_4, 0);
+		msleep(15);
+
+		tm6000_set_reg(dev, REQ_03_SET_GET_MCU_PIN,
+				TM6010_GPIO_1, 0);
+	
+		msleep(50);
+		tm6000_set_reg(dev, REQ_03_SET_GET_MCU_PIN,
+				TM6010_GPIO_1, 1);
+		
+		msleep(15);
+		tm6000_read_write_usb (dev, 0xc0, 0x0e, 0x00c2, 0x0008, buf, 2);
+//		printk(KERN_INFO "buf %#x %#x \n", buf[0], buf[1]);
+		msleep(15);
+		tm6000_set_reg(dev, REQ_03_SET_GET_MCU_PIN,
+				TM6010_GPIO_2, 1);
+		msleep(15);
+		tm6000_set_reg(dev, REQ_03_SET_GET_MCU_PIN,
+				TM6010_GPIO_2, 0);
+		msleep(15);
+		tm6000_set_reg(dev, REQ_03_SET_GET_MCU_PIN,
+				TM6010_GPIO_2, 1);
+		msleep(15);
+		tm6000_set_reg(dev, REQ_03_SET_GET_MCU_PIN,
+				TM6010_GPIO_2, 1);
+		msleep(15);
+		tm6000_set_reg(dev, REQ_03_SET_GET_MCU_PIN,
+				TM6010_GPIO_2, 0);
+		msleep(15);
+		tm6000_set_reg(dev, REQ_03_SET_GET_MCU_PIN,
+				TM6010_GPIO_2, 1);
+		msleep(15);
 	}
+	/* hack end */
+	
+	msleep(5); /* Just to be conservative */
 
+	/* Check board version - maybe 10Moons specific */
+	if (dev->dev_type == TM5600) {
+		 board=tm6000_get_reg16 (dev, 0x40, 0, 0);
+		if (board >=0) {
+			printk (KERN_INFO "Board version = 0x%04x\n",board);
+		} else {
+			printk (KERN_ERR "Error %i while retrieving board version\n",board);
+		}
+	}
+	
 	msleep(50);
 
 	return 0;