[linux-dvb] Separating / conditionalizing pll code from within nxt200x module

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Kirk Lapray wrote:

> On 10/26/05, *Michael Krufky* <mkrufky@xxxxxxx 
> <mailto:mkrufky@xxxxxxx>> wrote:
>
>     Kirk Lapray wrote:
>
>     > As for these additional devices showing up, did you load the nxt200x
>     > before these started showing up?  All the changes I made in
>     > tuner-simple should only apply to the HDTV Wonder, but some of the
>     > same init steps are also done in the nxt200x driver at the end of
>     > nxt2004_init function under "initialize tuner."  You can see these
>     > same commands in cx88-cards for the HDTV Wonder.  These commands
>     > enable the tuner so it shows up on the I2C bus on the HDTV
>     Wonder, and
>     > I wonder if it is also doing this on the A180.  If this is the case
>     > try using the tuning code in the nxt200x driver that writes
>     directly
>     > to the tuner instead of the code that goes through the NXT chip.
>
>     This is exactly what I did to make it work.
>
>     I dont like the way that you hardcoded nxt200x to give special
>     treatment
>     to Philips TUV1236D.... I am going to commit the module to cvs exactly
>     as you provided it, but I plan to make some changes.
>
>     Instead of conditionally writing directly to Philips TUV1236D, I think
>     that it should handle all pll's in this manner by default.  We
>     already
>     know this works for Philips TUV1236D and Alps TDHU2.  We should
>     instead
>     have a parameter that we pass through the config struct that will
>     instruct the module to go through the NXT chip when communicating with
>     the pll.....  Any ideas for what I should name this parameter?
>
>
> The reason it was hard coded only for this tuner is because I thought 
> this was the only tuner that worked this way.  Now that we know the 
> A180 is also this way, and that these are the only NXT2004 based cards 
> that I am aware of, it would probably be the easiest to just write 
> directly to the tuner on all NXT2004 based card.  Just put a switch 
> statement to write to the tuner on NXT2004 based cards and to the NXT 
> chip on NXT2002 based cards.  This way we would not have to have to 
> have to add another parameter.

The easiest way is not always the best way, although maybe this solution 
will suffice in the case of nxt200x.

IMHO, we have three options here:

1) If it is nxt2004, then use "direct pll write" method, if nxt2002, use 
"nxt pll write" method.

Sounds like this is the method preferred by Kirk, and the easiest one to 
implement.  This is probably the choice that makes the most sense.

---

2) Pass a boolean (int) parameter through the config struct that will 
determine which method to use.

I realize now that this is NOT the best method to choose.... This method 
is a compromise between method #1 and method #3, both of which seem to 
be better.
In fact, I completely retract this suggestion of method #2. ... I'll 
leave it here in the email for argument's sake.

---

3) Implement a pll_set callback.

This is done by many of the other frontend modules.  This method is the 
most portable solution, although it increases the size of the kernel, 
due to the fact that some code will be duplicated (cx88-dvb and 
saa7134-dvb will contain the same nxt200x_pll_set function). This is a 
trade-off that doesnt really matter at all, considering the benefits of 
more standardized / portable code... Especially considering the fact 
that is seems to be the common, accepted method for this situation.  
Using this method, each driver that depends on nxt200x module will 
contain a pll_set function that is appropriate for the tuner being used.


*I* prefer method #3.....  Although I think I will implement method #1 
when I get home tonight, as it is better than what's in there now, and 
requires a 2-3 line patch.  Hopefully this email will spark some 
discussion and we can collectively make the best decision.

-- 
Michael Krufky




[Index of Archives]     [Linux Media]     [Video 4 Linux]     [Asterisk]     [Samba]     [Xorg]     [Xfree86]     [Linux USB]

  Powered by Linux