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@xxxxxxxx wrote: >>> >>> >>>> From: Stefan Ringel <stefan.ringel@xxxxxxxx> >>>> >>>> --- >>>> 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 -- Stefan Ringel <stefan.ringel@xxxxxxxx> -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
