Stefan Ringel wrote: > Am 04.02.2010 03:43, schrieb Mauro Carvalho Chehab: >> Stefan Ringel wrote: >> >>> Am 03.02.2010 21:49, schrieb Devin Heitmueller: >>> >>>> On Wed, Feb 3, 2010 at 3:38 PM, Stefan Ringel <stefan.ringel@xxxxxxxx> wrote: >>>> >>>> >>>>> signed-off-by: Stefan Ringel <stefan.ringel@xxxxxxxx> >>>>> >>>>> --- a/drivers/media/dvb/frontends/zl10353.h >>>>> +++ b/drivers/media/dvb/frontends/zl10353.h >>>>> @@ -45,6 +45,8 @@ struct zl10353_config >>>>> /* clock control registers (0x51-0x54) */ >>>>> u8 clock_ctl_1; /* default: 0x46 */ >>>>> u8 pll_0; /* default: 0x15 */ >>>>> + >>>>> + int tm6000:1; >>>>> }; >>>>> >>>>> >>>> Why is this being submitted as its own patch? It is code that is not >>>> used by *anything*. If you really did require a new field in the >>>> zl10353 config, that field should be added in the same patch as >>>> whatever requires it. >>>> >>>> Devin >>>> >>>> >>>> >> Hi Stefan, >> >> Due to the problem with your emailer, most of the patches failed: >> >> 2/15 3/15 4/15 5/15 6/15 7/15 2/15 (the second one 8th in the sequence) >> 9/15 11/15 12/15 13/15 and 15/15. >> >> In brief, only two patches applied: the 1/15 and a patch also called 2/15 >> (I suspect it is the 10th patch). >> >> So, after reading our comments, reading README.patches and applying the >> Asaulted-patches extension to your thunderbird, please resubmit the patches. >> >> >>> Actually doesn't work zl10353 with tm6010, it have a little different >>> between a few registers, so I think that I use it. >>> >>> for example: >>> zl10353 use 0x64 , but not mine (0x63) >>> register 0x5f is 0x17 not 0x13 >>> register 0x5e is 0x40 not 0x00 for auto >>> and tuner go is 0x70 not 0x71 >>> >>> the other register are ok. I have no idea how I can set it. >>> >> I agree with Devin that the patch 14/15 shouldn't be applied as-is. >> >> I have no idea why zl10353 needs a different setup, but you don't need >> to add an extra parameter to identify the bridge driver. There's a field >> at struct i2c_client that can be used: i2c_adapter->id. >> >> This is already initialized with I2C_HW_B_TM6000 (although it currently >> uses a fake value). >> >> It is still ugly to do some specific initialization like this, but, when >> we have no other glue, due to the lack of datasheets, this is a better >> alternative than adding an extra fake parameter at the config struct. >> >> All you need is some code like: >> >> >> diff --git a/linux/drivers/media/dvb/frontends/zl10353.c b/linux/drivers/media/dvb/frontends/zl10353.c >> --- a/linux/drivers/media/dvb/frontends/zl10353.c >> +++ b/linux/drivers/media/dvb/frontends/zl10353.c >> @@ -597,6 +597,10 @@ static int zl10353_init(struct dvb_front >> zl10353_read_register(state, 0x51) != zl10353_reset_attach[2]) { >> rc = zl10353_write(fe, zl10353_reset_attach, >> sizeof(zl10353_reset_attach)); >> + >> + if (state->i2c->id == I2C_HW_B_TM6000) { >> + /* Do special init needed by tm6000 driver */ >> + } >> #if 1 >> if (debug_regs) >> zl10353_dump_regs(fe); >> diff --git a/linux/drivers/staging/tm6000/tm6000-i2c.c b/linux/drivers/staging/tm6000/tm6000-i2c.c >> --- a/linux/drivers/staging/tm6000/tm6000-i2c.c >> +++ b/linux/drivers/staging/tm6000/tm6000-i2c.c >> @@ -33,8 +33,6 @@ >> #include "tuner-xc2028.h" >> >> >> -/*FIXME: Hack to avoid needing to patch i2c-id.h */ >> -#define I2C_HW_B_TM6000 I2C_HW_B_EM28XX >> /* ----------------------------------------------------------- */ >> >> static unsigned int i2c_debug = 0; >> diff --git a/linux/include/linux/i2c-id.h b/linux/include/linux/i2c-id.h >> --- a/linux/include/linux/i2c-id.h >> +++ b/linux/include/linux/i2c-id.h >> @@ -42,6 +42,7 @@ >> #define I2C_HW_B_AU0828 0x010023 /* auvitek au0828 usb bridge */ >> #define I2C_HW_B_CX231XX 0x010024 /* Conexant CX231XX USB based cards */ >> #define I2C_HW_B_HDPVR 0x010025 /* Hauppauge HD PVR */ >> +#define I2C_HW_B_TM6000 0x010026 /* TM5600/6000/6010 video bridge */ >> >> /* --- SGI adapters */ >> #define I2C_HW_SGI_VINO 0x160000 >> > init is ok, but it has only one additional register 0x55 (value 0x40) > reset after second. I think it can work without this additional once. > The different is in function "set_parameters". Ok. Well, just add the logic to check for the bridge at the place it is needed, and only if it won't work otherwise. I would love to have the datasheet to check for the values, but unfortunately I don't have it. Anyway, after having it working, someone may help to figure out why the changes are needed and provide a better approach. - A side note: due to the backport code that you've added on your patches, I suspect that you just copied the files from the -hg to -git. You shouldn't do it, since we remove all backport stuff at -git, by running the gentree.pl script, found at v4l/scripts/gentree.pl. -- Cheers, Mauro -- 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