Mauro Carvalho Chehab wrote: > Em Qui, 2005-12-01 ?s 01:54 -0500, Steve Toth escreveu: > >>>> No, we shouldn't add cx88 specific stuff to tuner.ko, but in >>>> cx88 where the VIDIOC_S_FREQUENCY ioctl is processed you should be able >>>> to open the i2c gate before cx88_call_i2c_clients(), no? >>>> >>>> >>>> >>> I was trying to avoid any changes outside of cx88_dvb.ko -- It was >>> probably unavoidable. >>> >>> A patch is attached for review. >>> >>> >> I cleaned up this patch again this evening. See Attached. >> >> > > >> --- linux/Documentation/video4linux/CARDLIST.cx88 22 Nov 2005 19:32:26 -0000 1.16 >> +++ linux/Documentation/video4linux/CARDLIST.cx88 1 Dec 2005 06:39:31 -0000 >> @@ -38,3 +38,5 @@ >> 37 -> Hauppauge Nova-S-Plus DVB-S [0070:9201,0070:9202] >> 38 -> Hauppauge Nova-SE2 DVB-S [0070:9200] >> 39 -> KWorld DVB-S 100 [17de:08b2] >> + 40 -> Hauppauge WinTV-HVR1100 DVB-T/Hybrid [0070:9400,0070:9402] >> + 41 -> Hauppauge WinTV-HVR1100 DVB-T/Hybrid (Low Profile) [0070:9800,0070:9802] >> > > You don't need to edit this by hand. Some scripts do regenerate > cardlists at commit. > > Ok, I ran make cardlist (?) for this, my mistake. >> --- linux/drivers/media/video/cx88/cx88-i2c.c 16 Oct 2005 12:13:58 -0000 1.33 >> +++ linux/drivers/media/video/cx88/cx88-i2c.c 1 Dec 2005 06:39:32 -0000 >> @@ -140,7 +140,20 @@ void cx88_call_i2c_clients(struct cx88_c >> { >> if (0 != core->i2c_rc) >> return; >> - i2c_clients_command(&core->i2c_adap, cmd, arg); >> + >> + if (core->dvbdev == NULL) { >> + i2c_clients_command(&core->i2c_adap, cmd, arg); >> + } else { >> + >> + if (core->dvbdev->dvb.frontend->ops->enable_plli2c) >> + core->dvbdev->dvb.frontend->ops->enable_plli2c(core->dvbdev->dvb.frontend); >> + >> + i2c_clients_command(&core->i2c_adap, cmd, arg); >> + >> + if (core->dvbdev->dvb.frontend->ops->disable_plli2c) >> + core->dvbdev->dvb.frontend->ops->disable_plli2c(core->dvbdev->dvb.frontend); >> + } >> + >> } >> > Hmmm... IMHO, cx88-i2c is not the best place for it. It seems to be > better somewere at cx88-dvb or at a merger tuner-simple/dvb-pll code. > >> >> Thanks. In a previous patch I tried the cx88-dvb method. It made cx88 look very ugly. See the previous patch on the list, and all of my comments/findings. The fact is: enable/disable has to be execute before and after every possible i2c transaction in cx88. It's basically a patch designed to solve an I2C related gate problem with any dvb frontend (not specifically the cx22702). In the original patch, (posted a few days ago) I had many external symbols defined in cx22702, cx88-dvb and had patches to cx88-dvb, cx88-video. It was pretty ugly and created a lot of circular depmod references. This is a small patch, with a cleaner and more generic approach to solve the problem. One of the benefits of this approach is that all frontends, with the correct definitions during registration, will automatically be handled properly by cx88 without any further involvement. One other consideration I tried was putting the enable/disable functions in the card setup structure (cx88-cards) but that meant exposing more external symbols (register read/write) from the demod drivers, and felt more like a baord specific hack - which would have to be done for every Hybrid board we work on.... This is a 'single patch fits all' approach. I would agree with you however (at your posting yesterday), that moving all of the tuner/i2c handing out of the demod frontend drivers and into a single place would be a good thing. One place where tuning occurs. However, I do not believe that will happen quickly. (Or, more accurately - I do not believe that my influence could make that happen quickly). This is a patch which means we can get the HVR1100/HVR110LP/HVR1300 and another prototype reference designs supported by the kernel quickly, in a clean implementation. Johannes might want to shoot me for patching dvb_frontend.h though :)