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@xxxxxxxxx > <mailto:kirk.lapray@xxxxxxxxx>> 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
