Hi Jean-Francois,
On Fri, 8 Jul 2005, Jean-Francois Thibert (SageTV) wrote:
> Hello Patrick,
>
> Here is the patch for the dvb-kernel cvs. This will create also the 2 new
> files, let me know if there is something else.
>
> Thanks
>
> Jean-Francois
I'll try to point out some things which IMHO are better suited to kernel
coding-style. Johannes and others, please correct me, if I'm wrong.
+static int i2c_writebytes (struct nxt2004_state* state, u8 reg, u8 *buf, u8 len)
+{
+ /* probbably a much better way or doing this */
+ u8 buf2 [256],x;
What about buf2[len+1]; ?
+ int err;
+ struct i2c_msg msg = { .addr = state->config->demod_address, .flags = 0, .buf = buf2, .len = len + 1 };
+
+ buf2[0] = reg;
+ for (x = 0 ; x < len ; x++)
+ buf2[x+1] = buf[x];
memcpy(&buf[1],buf); does the same and looks IMHO cleaner here.
+ if ((err = i2c_transfer (state->i2c, &msg, 1)) != 1) {
+ dprintk ("%s: i2c write error (addr %02x, err == %i)\n",
+ __FUNCTION__, state->config->demod_address, err);
+ return -EREMOTEIO;
+ }
+
+ return 0;
+ return 0;
One return is sufficient ;)
+static int nxt2004_writetuner (struct nxt2004_state* state, u8* data)
[..]
+ /* set tuner i2c address */
+ buf = 0xC2;
+ i2c_writebytes(state,0x35,&buf,1);
Here would be the appropriate place for calling a pll_set callback, when
pll-programming would have been moved to the device-driver.
+static int nxt2004_load_firmware (struct dvb_frontend* fe, const struct firmware *fw)
[..]
+ position = 0;
+
Here e.g. is an empty line filled with spaces. There are several places in
the file where you have trailing spaces, please remove. (when you're using
vim: let c_space_errors=1). It would also be good if all indentions are
made with tabs instead of spaces. I did not found any of these in your
files, though.
+/* Modified for our alps tuner... don't know what other tuner was */
+static int nxt2004_setup_frontend_parameters (struct dvb_frontend* fe,
+ struct dvb_frontend_parameters *p)
+{
+ struct nxt2004_state* state = fe->demodulator_priv;
+ u32 freq = 0;
+ u16 tunerfreq = 0;
+ u8 buf[4];
+
+ freq = 44000 + ( p->frequency / 1000 );
+
+ dprintk("freq = %d p->frequency = %d\n",freq,p->frequency);
+
+ tunerfreq = freq * 2/125;
+
+ buf[0] = (tunerfreq >> 8) & 0x7F;
+ buf[1] = (tunerfreq & 0xFF);
+
+ if (p->frequency <= 162000000) {
+ buf[2] = 0x85;
+ buf[3] = 0x01;
+ } else if (p->frequency <= 426000000) {
+ buf[2] = 0x85;
+ buf[3] = 0x02;
+ } else if (p->frequency <= 782000000) {
+ buf[2] = 0x85;
+ buf[3] = 0x08;
+ } else {
+ buf[2] = 0x85;
+ buf[3] = 0x88;
+ }
+
+ /* write frequency information */
+ nxt2004_writetuner(state,buf);
Please try to implement pll-programming like all the other
frontend-drivers are doing it (by adding and calling a pll_set and
pll_init-callback inthe nxt2004_config or by using struct dvb_pll_desc). I
know the nxt2002 doesn't implement a pll-programming-callback, but this is
extremely helpful, when the frontend appears on other boards with another
pll.
Please also add a "Signed-off-by: Name <Emailaddress>" - line below your
next Emails which include patches for (dvb-)kernel. See <file:dvb-kernel/SubmittingPatches>
Please resend your patches with the modifications. Thanks for your
efforts.
Feel free to ask question or point out corrections if I got something
wrong.
best regards,
Patrick.
