Em Mon, 1 Apr 2013 19:12:24 -0300 Mauro Carvalho Chehab <mchehab@xxxxxxxxxx> escreveu: > Em Mon, 01 Apr 2013 22:39:28 +0200 > Frank Schäfer <fschaefer.oss@xxxxxxxxxxxxxx> escreveu: > > > Am 01.04.2013 21:22, schrieb Mauro Carvalho Chehab: > > > Em Mon, 01 Apr 2013 19:14:03 +0200 > > > Frank Schäfer <fschaefer.oss@xxxxxxxxxxxxxx> escreveu: > > > > > >> Am 18.03.2013 22:22, schrieb Mauro Carvalho Chehab: > > >>> Em Wed, 06 Mar 2013 18:44:07 +0100 > > >>> Frank Schäfer <fschaefer.oss@xxxxxxxxxxxxxx> escreveu: > > >>> > > >>>> Am 05.03.2013 16:43, schrieb Devin Heitmueller: > > >>>>> 2013/3/5 Mauro Carvalho Chehab <mchehab@xxxxxxxxxx>: > > >>>>>> The em2874 chips and upper have 2 buses. On all known devices, bus 0 is > > >>>>>> currently used only by eeprom, and bus 1 for the rest. Add support to > > >>>>>> register both buses. > > >>>>> Did you add a mutex to ensure that both buses cannot be used at the > > >>>>> same time? Because using the bus requires you to toggle a register > > >>>>> (thus you cannot be using both busses at the same time), you cannot > > >>>>> rely on the existing i2c adapter lock anymore. > > >>>>> > > >>>>> You don't want a situation where something is actively talking on bus > > >>>>> 0, and then something else tries to talk on bus 1, flips the register > > >>>>> bit and then the thread talking on bus 0 starts failing. > > >>>>> > > >>>>> Devin > > >>>> Hmm... there are several writes to EM28XX_R06_I2C_CLK in em28xx-dvb... > > >>>> See hauppauge_hvr930c_init(), terratec_h5_init() and > > >>>> terratec_htc_stick_init(). > > >>>> These functions are called from em28xx_dvb_init() at module init. > > >>>> Module init is async, so yes, this is (or could at least become) a > > >>>> problem... > > >>>> > > >>>> I wonder if we can't simply remove all those writes to > > >>>> EM28XX_R06_I2C_CLK from em28xx-dvb. > > >>>> This is what the functions are doing: > > >>>> > > >>>> hauppauge_hvr930c_init() > > >>>> ... > > >>>> em28xx_write_reg(dev, EM28XX_R06_I2C_CLK, 0x40); > > >>>> msleep(10); > > >>>> em28xx_write_reg(dev, EM28XX_R06_I2C_CLK, 0x44); > > >>>> msleep(10); > > >>>> ... [init sequence for slave at address 0x82] > > >>>> em28xx_write_reg(dev, EM28XX_R06_I2C_CLK, 0x44); > > >>>> msleep(30); > > >>>> em28xx_write_reg(dev, EM28XX_R06_I2C_CLK, 0x45); > > >>>> msleep(10); > > >>>> > > >>>> terratec_h5_init(): > > >>>> ... > > >>>> em28xx_write_reg(dev, EM28XX_R06_I2C_CLK, 0x40); > > >>>> msleep(10); > > >>>> em28xx_write_reg(dev, EM28XX_R06_I2C_CLK, 0x45); > > >>>> msleep(10); > > >>>> ... > > >>>> > > >>>> terratec_htc_stick_init() > > >>>> ... > > >>>> em28xx_write_reg(dev, EM28XX_R06_I2C_CLK, 0x40); > > >>>> msleep(10); > > >>>> em28xx_write_reg(dev, EM28XX_R06_I2C_CLK, 0x44); > > >>>> msleep(10); > > >>>> ... > > >>>> > > >>>> All three boards are using the following settings: > > >>>> .i2c_speed = EM2874_I2C_SECONDARY_BUS_SELECT | > > >>>> EM28XX_I2C_CLK_WAIT_ENABLE | EM28XX_I2C_FREQ_400_KHZ = 0x45 > > >>>> > > >>>> So what these functions are doing is > > >>>> - switch to bus A and do nothing fo 10ms > > >>>> - overwrite board settings for reg 0x06 with a local value (clears > > >>>> EM28XX_I2C_FREQ_400_KHZ permanently for the HTC-Stick !). > > >>>> > > >>>> I can test the HVR-930C next week. > > >> Ok, I finally had the chance to test with a HVR-930C, but it seems my > > >> device () is different. > > > > I forgot to insert the device/model of the device I tested: it's 16009, B1F0 > > It has the same model as the one I have here. > > > > > >> It has no slave device at i2c address 0x82, so I can't test this part. > > >> Btw, which slave device is this ? > > AFAIKT, at address 0x41, there is the analog demod (avf4910b). It is not > used by Digital TV (although I'm not sure if, for this device, it needs > to be initialized or not). > > We don't have enough documentation to write a driver for avf4910b. Some > developers at the ML are trying to implement support for it for HVR-930C: > > http://www.mail-archive.com/linux-media@xxxxxxxxxxxxxxx/msg59296.html > > There is a code pointed there for avf4910a: > https://github.com/wurststulle/ngene_2400i/blob/2377b1fd99d91ff355a5e46881ef27ccc87cb376/avf4910a.c > > Also, maybe to access the avf4910b some different GPIO setting may be needed, > as it might be powered off by the GPIO settings initialized at device init. > > > >> > > >> Removing the temporary switches to bus A makes no difference (as expected). > > >> > > >>> There are some things there on the init sequence that can be > > >>> cleaned/removed. Those sequences generally comes from observing > > >>> what the original driver does. While it produces a working driver, > > >>> in general, it is not optimized and part of the init sequence can > > >>> be removed. > > >> Do you want me to send patches to remove these writes ? > > >> Which i2c speed settings do you suggest for the HVR-930C and the > > >> HTC-Stick (board settings: > > >> EM28XX_I2C_FREQ_400_KHZ, code overwrites it with EM28XX_I2C_FREQ_100_KHZ) ? > > > Sure. The better would be to even remove the hauppauge_hvr930c_init() > > > function, as this is just a hack, and use the setup via the em28xx-cards > > > commented entries: > > > > > > .tuner_type = TUNER_XC5000, > > > .tuner_addr = 0x41, > > > .dvb_gpio = hauppauge_930c_digital, > > > .tuner_gpio = hauppauge_930c_gpio, > > > > Hmm... tuner address is 0x61 for the device I tested ! > > The register sequences in em28xx-cards.c also seem to be different to > > the ones used in hauppauge_hvr930c_init() in em28xx-dvb.c... > > I'm not telling that the entries there are right. They aren't. If they > where working, that data there weren't commented. This device entry > started with a clone from Terratec H5, which was the first em28xx > device with DRX-K. > > On Terratec H5, the tuner is different (based on tda8290/tda8275). > The current device initialization started as a clone of the code > under terratec_h5_init(). > > As it worked like that, the patch author that added support for HVR-930 > likely didn't touch on it. > > The tuner for HVR930C is clearly at 0x61 address, as it can be seen at > em28xx-dvb: > > /* Attach xc5000 */ > memset(&cfg, 0, sizeof(cfg)); > cfg.i2c_address = 0x61; > cfg.if_khz = 4000; > > if (dvb->fe[0]->ops.i2c_gate_ctrl) > dvb->fe[0]->ops.i2c_gate_ctrl(dvb->fe[0], 1); > if (!dvb_attach(xc5000_attach, dvb->fe[0], &dev->i2c_adap[dev->def_i2c_bus], > &cfg)) { > result = -EINVAL; > goto out_free; > } > > > Are you sure this will work for _all_ variants of the HVR-930C ? > > Well, the current code will only work with a HVR-930C with a xc5000 tuner, > a drx-k demod and an em28xx (and an avf4910b analog TV demod). > > Any other model with a different layout, if are there any, won't work > anyway. > > While we can't discard that a different model might have a different GPIO > setting, Hauppauge tends to keep the GPIO settings equal for the same > device brand name. > > So, it seems very unlikely that any change here will keep it working for > model 16009 while breaking it for other devices. In time, I meant to say: "So, it seems very unlikely that any change here will keep it working for model 16009 while breaking it for other HVR-930 devices." > > > I think it would be better if you would create those patches. > > I really don't like writing patches without completely understanding the > > code, not beeing able to test them and commit messages saying "Mauro > > told me to do this"... ;) > > > You also didn't answer my question concerning the i2c speed settings. ;) > > What question? > > Each bus may have a different max I2C speed, but the speed should not > change on the same I2C bus over the time. If the driver is doing that, > this is a bug that needs to be fixed. > > Regards, > Mauro -- 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
