[linux-dvb] Re: [Patch] Adding support for the Hauppage HVR1100

Johannes Stezenbach js at linuxtv.org
Thu Dec 1 16:18:01 CET 2005


On Thu, Dec 01, 2005, Steve Toth wrote:
> Mauro Carvalho Chehab wrote:
> >Em Qui, 2005-12-01 às 01:54 -0500, Steve Toth escreveu:
> >>--- 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.

Hm, it is called enable_plli2c(). Can it be added in a place where
we know that the following i2c transaction talks to the PLL?
Or do you want to rename it?

> 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 :)

No. My only concern is that we don't want to add quick hacks to
support one type of card which mgith create maintenance problems for other
cards later.
BTW, it isn't my code you're patching, in fact I do very
little development myself. I wish more of the developers on linux-dvb
would take the time to review patches. And I also wish the developers
with CVS accounts would stil post their patches here for review
instead of just whacking their cruft into CVS.

The current lack of peer review for DVB stuff is bad.

Johannes



More information about the linux-dvb mailing list