[linux-dvb] [PATCH] Combined support for NXT2002 and NXT2004 based cards

[Michael Krufky]

Kirk Lapray wrote:

> This patch replaces the nxt2002 driver with a nxt200x driver that 
> supports the NXT2002 and adds support for the NXT2004. I don't have a 
> NXT2002 based card so this new driver will have to be tested to make 
> sure it works.  This driver will also need to be tested on a AverTVHD 
> MCE A180, another NXT2004 based card.
>
> * build-2.6/Makefile:
> - Switched from nxt2002 to nxt200x
>
> * get_dvb_firmware:
> - Added support for the NXT2004 firmware. This was originally written 
> by Jean-Francois Thibert
>   This firmware also works with the ATI HDTV Wonder. I know where to 
> get the ATI firmware,
>   but the files are not able to be extracted in Linux (NSIS 
> self-extracting file) and I don't know the legality of
>   including the firmware in CVS.
>
> * flexcop-fe-tuner.c:
> - Switched from using the nxt2002 driver to the nxt200x driver
>
> * Kconfig:
> - Switched from nxt2002 to nxt200x
>
> * frontends/Makefile
> - Switched from nxt2002 to nxt200x
>
> * dvb-pll.c
> - Fixed minimum frequency for tuv1236d. It seems that the data sheets 
> are wrong.
>
> Signed-off-by: Kirk Lapray <kirk.lapray at gmail.com 
> <mailto:kirk.lapray at gmail.com>>

Kirk-

Thank you for the patches.  I think we should handle this in 2 steps:

1) Add nxt200x module (and apply the v4l-kernel patch).  (somewhat, but 
not totally trivial... we'll talk about it on the v4l list)

2) I don't think that we should deprecate the nxt2002 module until after 
testing the module with the flexcop driver (for compatability).

The module looks good to me, overall, but I have some questions:

diff -u -r1.12 flexcop-fe-tuner.c
--- linux/drivers/media/dvb/b2c2/flexcop-fe-tuner.c    15 Oct 2005 
17:48:57 -0000    1.12
+++ linux/drivers/media/dvb/b2c2/flexcop-fe-tuner.c    23 Oct 2005 
21:18:51 -0000
@@ -343,9 +343,10 @@
     .clock_polarity_flip = 1,
 };
 
-static struct nxt2002_config samsung_tbmv_config = {
+static struct nxt200x_config samsung_tbmv_config = {
     .demod_address    = 0x0a,
-    .request_firmware = flexcop_fe_request_firmware,
+    .pll_address      = 0xc2,
+    .pll_desc        = &dvb_pll_tbmv30111in,
 };
 

This doesn't look right to me... Shouldn't it be:
+    .pll_address      = 0x61,

Regardless, we'll need this module tested in the flexcop driver before 
we can make this switch.

More importantly, in nxt200x.c :

    /* if pll is a Philips TUV1236D then write directly to tuner */
    if (strcmp(state->config->pll_desc->name, "Philips TUV1236D") == 0) {
            if (i2c_writebytes(state, state->config->pll_address, data, 4))
                    printk(KERN_WARNING "nxt200x: error writing to 
tuner\n");
        /* wait until we have a lock */
        while (count < 20) {
            i2c_readbytes(state, state->config->pll_address, &buf, 1);
            if (buf & 0x40)
                return 0;
            msleep(100);
            count++;
        }
        printk("nxt200x: timeout waiting for tuner lock\n");
        return 0;
    } else {


WHY???  Is special handling of this pll really necessary?


    /* set additional params */
    switch (p->u.vsb.modulation) {
        case QAM_64:
        case QAM_256:
            /* Set punctured clock for QAM */
            /* This is just a guess since I am unable to test it */
            state->config->set_ts_params(fe, 1);

            /* set to use cable input */
            buf[3] |= 0x08;
            break;
        case VSB_8:
            /* Set non-punctured clock for VSB */
            state->config->set_ts_params(fe, 0);
            break;
        default:
            return -EINVAL;
            break;
    }

:-/  QAM256 is what I was using to test on the AVerTVHD 180 ... Now I 
see that it might not be perfect yet... Once I figure out what's going 
on in the AVerTVHD card, maybe I can look into this further...  
Meanwhile, I think we should do a check to be sure this callback is 
defined before we call it.  I'm not sure that this is going to work 
correctly on the saa7135-based AVerTVHD card.

So, these concerns that I have above are NOT considered showstoppers in 
my mind.  IMHO, I think it is safe to add this to cvs, but to keep 
nxt2002 around until we can test this more.  That's all I have time for 
right now... Maybe we'll hear some comments from some other people.

Cheers,

Michael Krufky