On Mon, 16 Apr 2007, Mauro Carvalho Chehab wrote:
> On Mon, 16 Apr 2007, Marco Gittler wrote:
> > so i this first enough to put it in and have some more testern for this
> > device ?
> For me, it seems ok now. I'll apply it later at master tree, to allow more
> people to test it. This will bring us about 3 weeks for testing and decide
> if this is ready for 2.6.22 or not.
>
> This will also give opportunity to other guys to send their comments,
> patches and sugestions to improve your driver.
diff -r b5be3479f070 linux/drivers/media/dvb/dvb-usb/Kconfig
--- a/linux/drivers/media/dvb/dvb-usb/Kconfig Sat Apr 14 16:19:13 2007 -0300
+++ b/linux/drivers/media/dvb/dvb-usb/Kconfig Mon Apr 16 12:06:05 2007 +0200
@@ -110,6 +110,7 @@ config DVB_USB_CXUSB
Medion MD95700 hybrid USB2.0 device.
DViCO FusionHDTV (Bluebird) USB2.0 devices
+
Random blank line inserted.
diff -r b5be3479f070 linux/drivers/media/dvb/dvb-usb/opera1.c
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/linux/drivers/media/dvb/dvb-usb/opera1.c Mon Apr 16 13:16:17 2007 +0200
[...]
+#include "opera1.h"
+#include "linux/module.h"
+#include "linux/init.h"
+#include "linux/firmware.h"
+#include "dvb-usb.h"
+#include "dvb-pll.h"
+#include "stv0299.h"
Includes should be first <linux/*.h> includes (with <> not ""), then
include "compat.h", then sub-system includes (e.g., "dvb-usb.h"), then your
includes. The <linux/*> then compat.h order is important, the rest not as
much.
+ int ret = 0, t = 0;
+ u8 r[10];
+ u8 u8buf[len];
[...]
+ memset(r, 0, sizeof(*r));
You don't need to initialize ret or t. It would easier to initialize r (I
don't think you need to anyway, but if you do) with "u8 r[10] = {0};" Instead
of adding an extra call to memset(). Also the memset is wrong, it should
be "memset(r, 0, sizeof(r));" as sizeof(*r) is 1, not 10.
Linux coding style is for one blank line after variables are declared and
before statements start.
The variable sized u8buf[len] array is a C99 feature. Are we allowed to
use this in the kernel? I'm not sure if this is allowed or not.
+ t = usb_control_msg(dev, usb_rcvctrlpipe(dev, 0),
+ OPERA_TUNER_REQ, USB_DIR_IN | USB_TYPE_VENDOR,
+ 0x01, 0x0, r, 10, 2000);
You never use t anywhere. You should probably check if usb_con...() failed
and return an error or print something.
static int opera1_usb_i2c_msgxfer(struct dvb_usb_device *dev, u16 addr,
[...]
+ request = (addr & 0xff00) >> 8;
+ if (!request)
+ request = 0xb1;
+ value = (addr & 0xff);
+ if (flag & OPERA_READ_MSG) {
+ value |= 0x01;
+static int opera1_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg msg[],
[...]
+ if ((tmp = opera1_usb_i2c_msgxfer(d,
+ msg[i].addr,
The Linux I2C subsystem uses 7-bit addresses, while your code is using
8-bit addresses. You should either change opera1_usb_i2c_msgxfer() so it
uses 7-bit addreses, or change opera1_i2c_xfer() so it passes
"msg[i].addr<<1" to opera1_usb_i2c_msgxfer(). This problem keeps comming
up again and again, you can check the list archives to find more
explainations.
opera1_i2c_xfer() would be better if you had this at the first line of
code:
if (!d)
return -ENODEV;
And the got rid of the extra if(d){ ... } part. Note that if d is NULL,
you will crash on the line that does "mutex_lock_interruptible(&d->i2c_mutex)",
which is run _before_ you check if d is null. So the current if(d){ } is
pointless, you will crash before you get there.
+static struct stv0299_config opera1_stv0299_config = {
+ .demod_address = 0xd0,
[...]
+static int opera1_tuner_attach(struct dvb_usb_adapter *adap)
+{
+ adap->pll_addr = 0xc0;
The addresses are wrong, it should be 0x68 and 0x60. It works because your
i2c code is also wrong, and the mistakes cancel each other out.
+static int opera1_streaming_ctrl(struct dvb_usb_adapter *adap, int onoff)
+{
+ u8 buf_start[2] = { 0xff, 0x03 };
+ u8 buf_stop[2] = { 0xff, 0x00 };
Declare constant arrays like this as static const.
+static int opera1_rc_query(struct dvb_usb_device *dev, u32 * event, int *state)
[...]
+ memset(rcbuffer, 0, 32);
This memset isn't necessary, you're just going to over-write it with the i2c
command.
_______________________________________________
linux-dvb mailing list
[email protected]
http://www.linuxtv.org/cgi-bin/mailman/listinfo/linux-dvb
