[linux-dvb] [v4l-dvb-maintainer] [saa7134] Fwd: [PATCH] Spezial Lifeview DVB-S Card without eeprom

Mauro Carvalho Chehab mchehab at infradead.org
Wed Oct 17 16:06:06 CEST 2007


Hi Oliver and Martin,

> since there was no reply on the DVB ML:
> Is anyone maintaining the DVB part of the saa7134 driver?
> Can this patch be accepted?

Hartmut is the guy who did more work at saa7134 dvb. 

Let me contribute with my review.

The patch looks sane for me, except for a few improvements that should
be done.

> ------------------------------------------------------------------ */
> +
> +static int philips_su1278_ty_ci_tuner_set_params(struct dvb_frontend
> *fe,
> +                                                struct
> dvb_frontend_parameters *params)
> +{
> +       u32 div;
> +       u8 buf[4];
> +       struct saa7134_dev *dev = fe->dvb->priv;
> +       struct i2c_msg msg = {.addr = 0x61,.flags = 0,.buf = buf,.len
> = sizeof(buf) };
> +
> +       if ((params->frequency < 950000) || (params->frequency >
> 2150000))
> +               return -EINVAL;
> +
> +       div = (params->frequency + (125 - 1)) / 125;    // round
> correctly
> +       buf[0] = (div >> 8) & 0x7f;
> +       buf[1] = div & 0xff;
> +       buf[2] = 0x80 | ((div & 0x18000) >> 10) | 4;
> +       buf[3] = 0x20;

The Philips su1278 tuner support should be mapped, instead, at
dvb-pll.c. It makes no sense to have tuner-specific stuff inside saa7134
dvb glue. This also means easier usage on other board types that has the
same tuner.

An example of a better implementation is the way saa7134-dvb does for
Philips td1316.

> +       0x10, 0x3f,             // AGC2  0x3d

The source code is violating CodingStyle. It should be reviewed using
Kernel scripts/checkpatch.pl. Running "make commit" will automatically
run this script, and adding warnings (as comments) at the commit
message.

In this particular case, C99 comments are forbidden inside kernel.
 
Cheers,
Mauro




More information about the linux-dvb mailing list