Support faulty USB IDs on DIBUSB_MC

Hi,

I got a LITE-ON USB2.0 DVB-T Tuner that loose it's cold state vid/pid and got FX2 dev kit one (0x04b4, 0x8613).

This patch introduce an option similar to the DVB_USB_DIBUSB_MB_FAULTY :
it add the FX2 dev kit ids to the DIBUSB_MC driver if DVB_USB_DIBUSB_MC_FAULTY is selected.

Signed-off-by Matthieu CASTET <[email protected]>

Quoted 11 lines · matthieu castet

Hi,

I got a LITE-ON USB2.0 DVB-T Tuner that loose it's cold state vid/pid and got FX2 dev kit one (0x04b4, 0x8613).

This patch introduce an option similar to the DVB_USB_DIBUSB_MB_FAULTY :
it add the FX2 dev kit ids to the DIBUSB_MC driver if DVB_USB_DIBUSB_MC_FAULTY is selected.

Signed-off-by Matthieu CASTET <[email protected]>

drivers/media/dvb/dvb-usb/Kconfig +6 -0

index f00a0eb..a656b9b 100644
@@ -68,6 +68,12 @@ config DVB_USB_DIBUSB_MC
 	  Say Y if you own such a device and want to use it. You should build it as
 	  a module.
 
+config DVB_USB_DIBUSB_MC_FAULTY
+	bool "Support faulty USB IDs"
+	depends on DVB_USB_DIBUSB_MC
+	help
+	  Support for faulty USB IDs due to an invalid EEPROM on some LITE-ON devices.
+
 config DVB_USB_DIB0700
 	tristate "DiBcom DiB0700 USB DVB devices (see help for supported devices)"
 	depends on DVB_USB

drivers/media/dvb/dvb-usb/dibusb-mc.c +1 -0

index 059cec9..ab5766a 100644
@@ -42,6 +42,17 @@ static struct usb_device_id dibusb_dib3000mc_table [] = {
 /* 11 */	{ USB_DEVICE(USB_VID_ULTIMA_ELECTRONIC,	USB_PID_ARTEC_T14_WARM) },
 /* 12 */	{ USB_DEVICE(USB_VID_LEADTEK,		USB_PID_WINFAST_DTV_DONGLE_COLD) },
 /* 13 */	{ USB_DEVICE(USB_VID_LEADTEK,		USB_PID_WINFAST_DTV_DONGLE_WARM) },
+/*

drivers/media/dvb/dvb-usb/dibusb-mc.c +0 -0

 	.devices = {
 		{   "DiBcom USB2.0 DVB-T reference design (MOD3000P)",
 			{ &dibusb_dib3000mc_table[0], NULL },

@@ -119,6 +134,13 @@ static struct dvb_usb_device_properties dibusb_mc_properties = {
 			{ &dibusb_dib3000mc_table[12], NULL },
 			{ &dibusb_dib3000mc_table[13], NULL },
 		},

drivers/media/dvb/dvb-usb/dibusb-mc.c static struct usb_device_id dibusb_dib3000mc_table [] = { · line 55

Quoted 2 lines

drivers/media/dvb/dvb-usb/dibusb-mc.c +2 -0

+/* 14 */	{ USB_DEVICE(USB_VID_CYPRESS,		USB_PID_ULTIMA_TVBOX_USB2_FX_COLD) },
+#endif

It doesn't sound a very good approach the need of recompiling the driver to allow it to work with a broken card. The better would be to have some modprobe option to force it to accept a certain USB ID as a valid ID for the card.

The above is really ugly. IMO, we should replace this by ARRAY_SIZE(dibusb_mc_properties.devices). Of course, for this to work, num_device_descs should be bellow devices.

Quoted 3 lines

It doesn't sound a very good approach the need of recompiling the driver to allow it to work with a broken card. The better would be to have some modprobe option to force it to accept a certain USB ID as a valid ID for the card.

The most correct way would be to reprogram the eeprom, by simply writing to 0xa0 (0x50 << 1) I2C address... There was a thread on the linux-dvb some time ago.

Quoted 3 lines

The above is really ugly. IMO, we should replace this by ARRAY_SIZE(dibusb_mc_properties.devices). Of course, for this to work, num_device_descs should be bellow devices.

We could do that, still I'm not sure if ARRAY_SIZE will work in that situation?! Are you
sure, Mauro?

Quoted 3 lines

We could do that, still I'm not sure if ARRAY_SIZE will work in that situation?! Are you
sure, Mauro?

Well, at least here, it is compiling fine. I can't really test it, since I don't have any dib0700 devices here.

Quoted 6 lines · Mauro Carvalho Chehab

Quoted 3 lines

We could do that, still I'm not sure if ARRAY_SIZE will work in that situation?! Are you
sure, Mauro?

Well, at least here, it is compiling fine. I can't really test it, since I don't have any dib0700 devices here.

Hmm, your patch is shifting the counting problem to another place. Instead of counting manually the devices-array-elements, one now needs to count the number of device_properties ;) .

With such a patch we would risk to break some device support and as I never saw a patch which broke the current num_device_descs-manual-count I don't see the need to change.

--
Mail: [email protected]
WWW: http://www.wi-bw.tfh-wildau.de/~pboettch/

Hi,

sorry for not answering ealier, recently I became the master of postponing things. :(

Quoted 7 lines · Mauro Carvalho Chehab

Quoted 3 lines

+/* 14 */ { USB_DEVICE(USB_VID_CYPRESS,
USB_PID_ULTIMA_TVBOX_USB2_FX_COLD) },
+#endif

It doesn't sound a very good approach the need of recompiling the driver to allow it to work with a broken card. The better would be to have some modprobe option to force it to accept a certain USB ID as a valid ID for the card.

The most correct way would be to reprogram the eeprom, by simply writing to 0xa0 (0x50 << 1) I2C address... There was a thread on the linux-dvb some time ago.

Hi Patrick,

Quoted 16 lines · Patrick Boettcher

Hi,

sorry for not answering ealier, recently I became the master of postponing things. :(

Quoted 7 lines · Mauro Carvalho Chehab

Quoted 3 lines

+/* 14 */ { USB_DEVICE(USB_VID_CYPRESS,
USB_PID_ULTIMA_TVBOX_USB2_FX_COLD) },
+#endif

It doesn't sound a very good approach the need of recompiling the driver to allow it to work with a broken card. The better would be to have some modprobe option to force it to accept a certain USB ID as a valid ID for the card.

The most correct way would be to reprogram the eeprom, by simply writing to 0xa0 (0x50 << 1) I2C address... There was a thread on the linux-dvb some time ago.

Quoted 19 lines · matthieu castet

Hi Patrick,

Quoted 16 lines · Patrick Boettcher

Hi,

sorry for not answering ealier, recently I became the master of postponing things. :(

Quoted 7 lines · Mauro Carvalho Chehab

Quoted 3 lines

+/* 14 */ { USB_DEVICE(USB_VID_CYPRESS,
USB_PID_ULTIMA_TVBOX_USB2_FX_COLD) },
+#endif

It doesn't sound a very good approach the need of recompiling the driver to allow it to work with a broken card. The better would be to have some modprobe option to force it to accept a certain USB ID as a valid ID for the card.

The most correct way would be to reprogram the eeprom, by simply writing to 0xa0 (0x50 << 1) I2C address... There was a thread on the linux-dvb some time ago.

BTW dibusb_i2c_xfer seems to do things very dangerous :
it assumes that it get only write/read request or write request.

That means that read can be understood as write. For example a program doing file = open("/dev/i2c-x", O_RDWR);
ioctl(file, I2C_SLAVE, 0x50)
read(file, data, 10)
will corrupt the eeprom as it will be understood as a write.

Hi,

Quoted 32 lines · matthieu castet

Quoted 27 lines · matthieu castet

Hi Patrick,

Quoted 23 lines · Patrick Boettcher

Hi,

sorry for not answering ealier, recently I became the master of postponing things. :(

Quoted 10 lines · Mauro Carvalho Chehab

Review

Quoted 3 lines

(quoted diff) +2 -0

+/* 14 */    { USB_DEVICE(USB_VID_CYPRESS,
 USB_PID_ULTIMA_TVBOX_USB2_FX_COLD) },
+#endif

It doesn't sound a very good approach the need of recompiling the driver to
allow it to work with a broken card. The better would be to have some modprobe
option to force it to accept a certain USB ID as a valid ID for the card.

The most correct way would be to reprogram the eeprom, by simply writing to 0xa0 (0x50 << 1) I2C address... There was a thread on the linux-dvb some time ago.

BTW dibusb_i2c_xfer seems to do things very dangerous :
it assumes that it get only write/read request or write request.

Quoted 7 lines

That means that read can be understood as write. For example a program doing
file = open("/dev/i2c-x", O_RDWR);
ioctl(file, I2C_SLAVE, 0x50)
read(file, data, 10)
will corrupt the eeprom as it will be understood as a write.

Patrick, any info about that.

I attach a possible (untested) patch.

Matthieu

Signed-off-by Matthieu CASTET <[email protected]>

Index: linux-2.6/drivers/media/dvb/dvb-usb/dibusb-common.c
===================================================================
--- linux-2.6.orig/drivers/media/dvb/dvb-usb/dibusb-common.c 2009-02-09
20:36:03.000000000 +0100
+++ linux-2.6/drivers/media/dvb/dvb-usb/dibusb-common.c 2009-02-09 20:38:21.000000000 +0100

drivers/media/dvb/dvb-usb/dibusb-common.c +6 -2

@@ -133,14 +133,18 @@
 
       for (i = 0; i < num; i++) {
               /* write/read request */
-               if (i+1 < num && (msg[i+1].flags & I2C_M_RD)) {
+               if (i+1 < num && (msg[i].flags & I2C_M_RD) == 0
+                                         && (msg[i+1].flags & I2C_M_RD)) {
                       if (dibusb_i2c_msg(d, msg[i].addr,
 msg[i].buf,msg[i].len,
                                               msg[i+1].buf,msg[i+1].len) <
 0)
                               break;
                       i++;
-               } else
+               } else if ((msg[i].flags & I2C_M_RD) == 0) {
                       if (dibusb_i2c_msg(d, msg[i].addr,
 msg[i].buf,msg[i].len,NULL,0) < 0)
                               break;
+               }
+               else
+                       break;

       }

       mutex_unlock(&d->i2c_mutex);

Quoted 6 lines · matthieu castet

Hi,

Quoted 3 lines · matthieu castet

Quoted 2 lines · matthieu castet

Hi Patrick,

Quoted 3 lines

Quoted 3 lines

Quoted 3 lines

Quoted 2 lines · Patrick Boettcher

Hi,

The assumption that you can only have write/read or write requests is one of the big things fixed in the 1.20 firmware. If you are running the 1.20 firmware, you just need to add the option to use the new i2c function instead of the legacy i2c transfer function.

Quoted 11 lines · Devin Heitmueller

Quoted 9 lines · matthieu castet

Hi,

Quoted 6 lines · matthieu castet

Quoted 5 lines · matthieu castet

Hi Patrick,

Quoted 2 lines · Patrick Boettcher

Hi,

Quoted 4 lines

The assumption that you can only have write/read or write requests is one of the big things fixed in the 1.20 firmware. If you are running the 1.20 firmware, you just need to add the option to use the new i2c function instead of the legacy i2c transfer function.

Unfortunamty this is for dib0700 and not dib3000.
dib3000 doesn't got new firmware/i2c API.

But the same patch should be done for dib0700_i2c_xfer_legacy...

Quoted 10 lines

BTW dibusb_i2c_xfer seems to do things very dangerous :
it assumes that it get only write/read request or write request.

That means that read can be understood as write. For example a program doing
file = open("/dev/i2c-x", O_RDWR);
ioctl(file, I2C_SLAVE, 0x50)
read(file, data, 10)
will corrupt the eeprom as it will be understood as a write.

Patrick, any info about that.

I attach a possible (untested) patch.

Matthieu

Signed-off-by Matthieu CASTET <[email protected]>

Index: linux-2.6/drivers/media/dvb/dvb-usb/dibusb-common.c
===================================================================
--- linux-2.6.orig/drivers/media/dvb/dvb-usb/dibusb-common.c 2009-02-09
20:36:03.000000000 +0100
+++ linux-2.6/drivers/media/dvb/dvb-usb/dibusb-common.c 2009-02-09 20:38:21.000000000 +0100

drivers/media/dvb/dvb-usb/dibusb-common.c +6 -2

@@ -133,14 +133,18 @@
 
       for (i = 0; i < num; i++) {
               /* write/read request */
-               if (i+1 < num && (msg[i+1].flags & I2C_M_RD)) {
+               if (i+1 < num && (msg[i].flags & I2C_M_RD) == 0
+                                         && (msg[i+1].flags & I2C_M_RD)) {
                       if (dibusb_i2c_msg(d, msg[i].addr,
 msg[i].buf,msg[i].len,
                                               msg[i+1].buf,msg[i+1].len) <
 0)
                               break;
                       i++;
-               } else
+               } else if ((msg[i].flags & I2C_M_RD) == 0) {
                       if (dibusb_i2c_msg(d, msg[i].addr,
 msg[i].buf,msg[i].len,NULL,0) < 0)
                               break;
+               }
+               else
+                       break;

       }

       mutex_unlock(&d->i2c_mutex);

Hi,

dibusb_i2c_xfer seems to do things very dangerous :
it assumes that it get only write/read request or write request.

That means that read can be understood as write. For example a program doing
file = open("/dev/i2c-x", O_RDWR);
ioctl(file, I2C_SLAVE, 0x50)
read(file, data, 10)
will corrupt the eeprom as it will be understood as a write.

I attach a possible (untested) patch.

Matthieu

Signed-off-by Matthieu CASTET <[email protected]>