[linux-dvb] Re: [rfc/patch] pll handling and cx22702 update

[Andrew de Quincey <adq_dvb@lidskialf.net>]

On Sunday 19 Dec 2004 17:23, Johannes Stezenbach wrote:
> Andrew de Quincey wrote:
> > enum dvb_tuner_mode {
> >  DVB_PLL_SCAN,
> >  DVB_PLL_LOCK,
> > };
>
> Hm, do you care to explain what this means?

The TDM1316 datasheet has different recommendations for when you are scanning 
for channels than when you are locking onto a channel for receiving data for 
display. The scanning mode "enables a fast tuning speed". I'm not actually 
sure _why_ it is better to disable fast tuning speed when locking for 
receiving data for display - it just says that it is. The above was an 
attempt to expose this as a starting point for higher level support. I have 
seen this sort of thing in a couple of other datasheets.

> > Firstly, the tuner is more than just the PLL. Some of them have more than
> > one device which must be programmed... e.g. the TDM1316L has the
> > MC44bc374c chip which must be disabled for proper functionality. This is
> > a tuner-specific detail and not a card-specific one - it is inside the
> > tinbox.
>
> I think a lot of PLLs are of the simple variety, and we should leave
> the code in place that handles them. IMHO there is no need for dvb-pll to
> handle every exotic PLL, this can be left to specialized code.
> Of course if you find a way to generalize dvb-pll to work for
> more PLLs it is good, but we shouldn't sacrifice the code savings
> and the gain in clarity/readability Gerd's version of dvb-pll
> had by over-generalization.

If you look at the original implementation of the dtt759x from cx22702.c 
(pre-refactoring), it is actually more complex than the revised 
implementation that Gerd had in his example. It requires two writes to the 
PLL to tune it.. I'm not sure what difference (if any) removing one of the 
writes will make but that is my point - it is working right now - randomly 
removing pieces of the PLL code just because they don't fit into the model is 
not a good idea... no one has access to all cards in all situations, so there 
is no way to guarantee it won't break things for someone even though it 
appears to work fine for one person.

Several of the tuners use the PLL status registers to tell when it has locked 
instead of constant length msleeps(). This requires tuner-specific code for 
each.

Personally, I'd prefer to go for a standard flexible way to implement all 
tuners rather than an (admittedly much neater) way of implementing a subset 
of them. 

Hmm, what about combining the two then - use Gerd's idea for most of the 
contents of the data buffers - but also have a tuner specific function. There 
already is a tuner specific function in Gerd's example for setting the 
bandwidth stuff - we'd just expand that so it performed the tuner specific 
writing.

This way we get the added clarity of Gerd's suggestion, but also gain 
flexibility.

> As for the TDM1316L/MC44bc374c, IIRC Gerd's code just prepares the
> PLL messages but leaves it for card-specific code to send them.
> So there should be no problem.

This would mean every card using the TDM1316L will have to duplicate the tuner 
init code which disables the mc44bc374c - I'd prefer to avoid that if 
possible.