[linux-dvb] [PATCH] Support faulty USB IDs on DIBUSB_MC

[linux-dvb] [PATCH] Support faulty USB IDs on DIBUSB_MC

[matthieu castet]

[-- Attachment #1: Type: text/plain, Size: 344 bytes --]

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 <castet.matthieu@free.fr>

[-- Attachment #2: dib3000c_faulty_id --]
[-- Type: text/plain, Size: 2565 bytes --]

diff --git a/drivers/media/dvb/dvb-usb/Kconfig b/drivers/media/dvb/dvb-usb/Kconfig
index f00a0eb..a656b9b 100644
--- a/drivers/media/dvb/dvb-usb/Kconfig
+++ b/drivers/media/dvb/dvb-usb/Kconfig
@@ -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
diff --git a/drivers/media/dvb/dvb-usb/dibusb-mc.c b/drivers/media/dvb/dvb-usb/dibusb-mc.c
index 059cec9..ab5766a 100644
--- a/drivers/media/dvb/dvb-usb/dibusb-mc.c
+++ b/drivers/media/dvb/dvb-usb/dibusb-mc.c
@@ -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) },
+/*
+ * XXX: Some LITE-ON devices seem to loose their id after some time. Bad EEPROM ???.
+ *      We don't catch these faulty IDs (namely 'Cypress FX2 USB controller') that
+ *      have been left on the device. If you don't have such a device but an LITE-ON
+ *      device that's supposed to work with this driver but is not detected by it,
+ *      free to enable CONFIG_DVB_USB_DIBUSB_MC_FAULTY via your kernel config.
+ */
+
+#ifdef CONFIG_DVB_USB_DIBUSB_MC_FAULTY
+/* 14 */	{ USB_DEVICE(USB_VID_CYPRESS,		USB_PID_ULTIMA_TVBOX_USB2_FX_COLD) },
+#endif
 			{ }		/* Terminating entry */
 };
 MODULE_DEVICE_TABLE (usb, dibusb_dib3000mc_table);
@@ -88,7 +99,11 @@ static struct dvb_usb_device_properties dibusb_mc_properties = {
 
 	.generic_bulk_ctrl_endpoint = 0x01,
 
+#ifdef CONFIG_DVB_USB_DIBUSB_MC_FAULTY
+	.num_device_descs = 8,
+#else
 	.num_device_descs = 7,
+#endif
 	.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 },
 		},
+#ifdef CONFIG_DVB_USB_DIBUSB_MC_FAULTY
+		{   "LITE-ON USB2.0 DVB-T Tuner (faulty USB IDs)",
+		    /* Also rebranded as Intuix S800, Toshiba */
+			{ &dibusb_dib3000mc_table[14], NULL },
+			{ NULL },
+		},
+#endif
 		{ NULL },
 	}
 };

[-- Attachment #3: Type: text/plain, Size: 150 bytes --]

_______________________________________________
linux-dvb mailing list
linux-dvb@linuxtv.org
http://www.linuxtv.org/cgi-bin/mailman/listinfo/linux-dvb

Re: [PATCH] Support faulty USB IDs on DIBUSB_MC

[matthieu castet]

[-- Attachment #1: Type: text/plain, Size: 396 bytes --]

matthieu castet wrote:
> 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 <castet.matthieu@free.fr>
> 

Ping

[-- Attachment #2: dib3000c_faulty_id --]
[-- Type: text/plain, Size: 2565 bytes --]

diff --git a/drivers/media/dvb/dvb-usb/Kconfig b/drivers/media/dvb/dvb-usb/Kconfig
index f00a0eb..a656b9b 100644
--- a/drivers/media/dvb/dvb-usb/Kconfig
+++ b/drivers/media/dvb/dvb-usb/Kconfig
@@ -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
diff --git a/drivers/media/dvb/dvb-usb/dibusb-mc.c b/drivers/media/dvb/dvb-usb/dibusb-mc.c
index 059cec9..ab5766a 100644
--- a/drivers/media/dvb/dvb-usb/dibusb-mc.c
+++ b/drivers/media/dvb/dvb-usb/dibusb-mc.c
@@ -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) },
+/*
+ * XXX: Some LITE-ON devices seem to loose their id after some time. Bad EEPROM ???.
+ *      We don't catch these faulty IDs (namely 'Cypress FX2 USB controller') that
+ *      have been left on the device. If you don't have such a device but an LITE-ON
+ *      device that's supposed to work with this driver but is not detected by it,
+ *      free to enable CONFIG_DVB_USB_DIBUSB_MC_FAULTY via your kernel config.
+ */
+
+#ifdef CONFIG_DVB_USB_DIBUSB_MC_FAULTY
+/* 14 */	{ USB_DEVICE(USB_VID_CYPRESS,		USB_PID_ULTIMA_TVBOX_USB2_FX_COLD) },
+#endif
 			{ }		/* Terminating entry */
 };
 MODULE_DEVICE_TABLE (usb, dibusb_dib3000mc_table);
@@ -88,7 +99,11 @@ static struct dvb_usb_device_properties dibusb_mc_properties = {
 
 	.generic_bulk_ctrl_endpoint = 0x01,
 
+#ifdef CONFIG_DVB_USB_DIBUSB_MC_FAULTY
+	.num_device_descs = 8,
+#else
 	.num_device_descs = 7,
+#endif
 	.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 },
 		},
+#ifdef CONFIG_DVB_USB_DIBUSB_MC_FAULTY
+		{   "LITE-ON USB2.0 DVB-T Tuner (faulty USB IDs)",
+		    /* Also rebranded as Intuix S800, Toshiba */
+			{ &dibusb_dib3000mc_table[14], NULL },
+			{ NULL },
+		},
+#endif
 		{ NULL },
 	}
 };

Re: [PATCH] Support faulty USB IDs on DIBUSB_MC

[Mauro Carvalho Chehab]

On Mon, 19 Jan 2009 21:38:22 +0100
matthieu castet <castet.matthieu@free.fr> wrote:

> matthieu castet wrote:
> > 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 <castet.matthieu@free.fr>
> > 
> 
> diff --git a/drivers/media/dvb/dvb-usb/Kconfig b/drivers/media/dvb/dvb-usb/Kconfig
> index f00a0eb..a656b9b 100644
> --- a/drivers/media/dvb/dvb-usb/Kconfig
> +++ b/drivers/media/dvb/dvb-usb/Kconfig
> @@ -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
> diff --git a/drivers/media/dvb/dvb-usb/dibusb-mc.c b/drivers/media/dvb/dvb-usb/dibusb-mc.c
> index 059cec9..ab5766a 100644
> --- a/drivers/media/dvb/dvb-usb/dibusb-mc.c
> +++ b/drivers/media/dvb/dvb-usb/dibusb-mc.c
> @@ -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) },
> +/*
> + * XXX: Some LITE-ON devices seem to loose their id after some time. Bad EEPROM ???.
> + *      We don't catch these faulty IDs (namely 'Cypress FX2 USB controller') that
> + *      have been left on the device. If you don't have such a device but an LITE-ON
> + *      device that's supposed to work with this driver but is not detected by it,
> + *      free to enable CONFIG_DVB_USB_DIBUSB_MC_FAULTY via your kernel config.
> + */
> +
> +#ifdef CONFIG_DVB_USB_DIBUSB_MC_FAULTY
> +/* 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.

>  			{ }		/* Terminating entry */
>  };
>  MODULE_DEVICE_TABLE (usb, dibusb_dib3000mc_table);
> @@ -88,7 +99,11 @@ static struct dvb_usb_device_properties dibusb_mc_properties = {
>  
>  	.generic_bulk_ctrl_endpoint = 0x01,
>  
> +#ifdef CONFIG_DVB_USB_DIBUSB_MC_FAULTY
> +	.num_device_descs = 8,
> +#else
>  	.num_device_descs = 7,
> +#endif

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.

>  	.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 },
>  		},
> +#ifdef CONFIG_DVB_USB_DIBUSB_MC_FAULTY
> +		{   "LITE-ON USB2.0 DVB-T Tuner (faulty USB IDs)",
> +		    /* Also rebranded as Intuix S800, Toshiba */
> +			{ &dibusb_dib3000mc_table[14], NULL },
> +			{ NULL },
> +		},
> +#endif
>  		{ NULL },
>  	}
>  };


Patrick,
Comments?


Cheers,
Mauro

Re: [PATCH] Support faulty USB IDs on DIBUSB_MC

[Patrick Boettcher]

Hi,

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

On Thu, 29 Jan 2009, Mauro Carvalho Chehab wrote:

>> +/* 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.

> 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?

Patrick.

--
   Mail: patrick.boettcher@desy.de
   WWW:  http://www.wi-bw.tfh-wildau.de/~pboettch/

Re: [PATCH] Support faulty USB IDs on DIBUSB_MC

[Mauro Carvalho Chehab]

On Thu, 29 Jan 2009 11:19:32 +0100 (CET)
Patrick Boettcher <patrick.boettcher@desy.de> wrote:

> > 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.

Yes. Yet, it might be interesting to have some option to allow forcing.

> > 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?

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

Signed-off-by: Mauro Carvalho Chehab <mchehab@redhat.com>

diff -r 39a646207d0c linux/drivers/media/dvb/dvb-usb/dib0700_devices.c
--- a/linux/drivers/media/dvb/dvb-usb/dib0700_devices.c	Thu Jan 29 09:11:45 2009 -0200
+++ b/linux/drivers/media/dvb/dvb-usb/dib0700_devices.c	Thu Jan 29 10:04:08 2009 -0200
@@ -1447,7 +1447,7 @@
 	}
 
 struct dvb_usb_device_properties dib0700_devices[] = {
-	{
+	[0] = {
 		DIB0700_DEFAULT_DEVICE_PROPERTIES,
 
 		.num_adapters = 1,
@@ -1460,7 +1460,6 @@
 			},
 		},
 
-		.num_device_descs = 8,
 		.devices = {
 			{   "DiBcom STK7700P reference design",
 				{ &dib0700_usb_id_table[0], &dib0700_usb_id_table[1] },
@@ -1496,11 +1495,13 @@
 			}
 		},
 
+		.num_device_descs = ARRAY_SIZE(dib0700_devices[0].devices),
 		.rc_interval      = DEFAULT_RC_INTERVAL,
 		.rc_key_map       = dib0700_rc_keys,
 		.rc_key_map_size  = ARRAY_SIZE(dib0700_rc_keys),
 		.rc_query         = dib0700_rc_query
-	}, { DIB0700_DEFAULT_DEVICE_PROPERTIES,
+	},
+	[1] = { DIB0700_DEFAULT_DEVICE_PROPERTIES,
 
 		.num_adapters = 2,
 		.adapter = {
@@ -1517,19 +1518,20 @@
 			}
 		},
 
-		.num_device_descs = 1,
 		.devices = {
 			{   "Hauppauge Nova-T 500 Dual DVB-T",
 				{ &dib0700_usb_id_table[2], &dib0700_usb_id_table[3], NULL },
 				{ NULL },
 			},
 		},
+		.num_device_descs = ARRAY_SIZE(dib0700_devices[1].devices),
 
 		.rc_interval      = DEFAULT_RC_INTERVAL,
 		.rc_key_map       = dib0700_rc_keys,
 		.rc_key_map_size  = ARRAY_SIZE(dib0700_rc_keys),
 		.rc_query         = dib0700_rc_query
-	}, { DIB0700_DEFAULT_DEVICE_PROPERTIES,
+	}, 
+	[2] = { DIB0700_DEFAULT_DEVICE_PROPERTIES,
 
 		.num_adapters = 2,
 		.adapter = {
@@ -1546,7 +1548,6 @@
 			}
 		},
 
-		.num_device_descs = 4,
 		.devices = {
 			{   "Pinnacle PCTV 2000e",
 				{ &dib0700_usb_id_table[11], NULL },
@@ -1566,13 +1567,15 @@
 			},
 
 		},
+		.num_device_descs = ARRAY_SIZE(dib0700_devices[2].devices),
 
 		.rc_interval      = DEFAULT_RC_INTERVAL,
 		.rc_key_map       = dib0700_rc_keys,
 		.rc_key_map_size  = ARRAY_SIZE(dib0700_rc_keys),
 		.rc_query         = dib0700_rc_query
 
-	}, { DIB0700_DEFAULT_DEVICE_PROPERTIES,
+	},
+	[3] = { DIB0700_DEFAULT_DEVICE_PROPERTIES,
 
 		.num_adapters = 1,
 		.adapter = {
@@ -1584,7 +1587,6 @@
 			},
 		},
 
-		.num_device_descs = 3,
 		.devices = {
 			{   "ASUS My Cinema U3000 Mini DVBT Tuner",
 				{ &dib0700_usb_id_table[23], NULL },
@@ -1599,12 +1601,14 @@
 				{ NULL },
 			}
 		},
+		.num_device_descs = ARRAY_SIZE(dib0700_devices[3].devices),
 
 		.rc_interval      = DEFAULT_RC_INTERVAL,
 		.rc_key_map       = dib0700_rc_keys,
 		.rc_key_map_size  = ARRAY_SIZE(dib0700_rc_keys),
 		.rc_query         = dib0700_rc_query
-	}, { DIB0700_DEFAULT_DEVICE_PROPERTIES,
+	},
+	[4] = { DIB0700_DEFAULT_DEVICE_PROPERTIES,
 
 		.num_adapters = 1,
 		.adapter = {
@@ -1618,7 +1622,6 @@
 			},
 		},
 
-		.num_device_descs = 9,
 		.devices = {
 			{   "DiBcom STK7070P reference design",
 				{ &dib0700_usb_id_table[15], NULL },
@@ -1657,13 +1660,15 @@
 				{ NULL },
 			},
 		},
+		.num_device_descs = ARRAY_SIZE(dib0700_devices[4].devices),
 
 		.rc_interval      = DEFAULT_RC_INTERVAL,
 		.rc_key_map       = dib0700_rc_keys,
 		.rc_key_map_size  = ARRAY_SIZE(dib0700_rc_keys),
 		.rc_query         = dib0700_rc_query
 
-	}, { DIB0700_DEFAULT_DEVICE_PROPERTIES,
+	},
+	[5] = { DIB0700_DEFAULT_DEVICE_PROPERTIES,
 
 		.num_adapters = 2,
 		.adapter = {
@@ -1684,7 +1689,6 @@
 			}
 		},
 
-		.num_device_descs = 5,
 		.devices = {
 			{   "DiBcom STK7070PD reference design",
 				{ &dib0700_usb_id_table[17], NULL },
@@ -1707,11 +1711,13 @@
 				{ NULL },
 			}
 		},
+		.num_device_descs = ARRAY_SIZE(dib0700_devices[5].devices),
 		.rc_interval      = DEFAULT_RC_INTERVAL,
 		.rc_key_map       = dib0700_rc_keys,
 		.rc_key_map_size  = ARRAY_SIZE(dib0700_rc_keys),
 		.rc_query         = dib0700_rc_query
-	}, { DIB0700_DEFAULT_DEVICE_PROPERTIES,
+	}, 
+	[6] = { DIB0700_DEFAULT_DEVICE_PROPERTIES,
 
 		.num_adapters = 1,
 		.adapter = {
@@ -1726,7 +1732,6 @@
 			},
 		},
 
-		.num_device_descs = 5,
 		.devices = {
 			{   "Terratec Cinergy HT USB XE",
 				{ &dib0700_usb_id_table[27], NULL },
@@ -1753,11 +1758,13 @@
 				{ NULL },
 			},
 		},
+		.num_device_descs = ARRAY_SIZE(dib0700_devices[6].devices),
 		.rc_interval      = DEFAULT_RC_INTERVAL,
 		.rc_key_map       = dib0700_rc_keys,
 		.rc_key_map_size  = ARRAY_SIZE(dib0700_rc_keys),
 		.rc_query         = dib0700_rc_query
-	}, { DIB0700_DEFAULT_DEVICE_PROPERTIES,
+	}, 
+	[7] = { DIB0700_DEFAULT_DEVICE_PROPERTIES,
 		.num_adapters = 1,
 		.adapter = {
 			{
@@ -1771,7 +1778,6 @@
 			},
 		},
 
-		.num_device_descs = 2,
 		.devices = {
 			{   "Pinnacle PCTV HD Pro USB Stick",
 				{ &dib0700_usb_id_table[40], NULL },
@@ -1782,6 +1788,7 @@
 				{ NULL },
 			},
 		},
+		.num_device_descs = ARRAY_SIZE(dib0700_devices[7].devices),
 		.rc_interval      = DEFAULT_RC_INTERVAL,
 		.rc_key_map       = dib0700_rc_keys,
 		.rc_key_map_size  = ARRAY_SIZE(dib0700_rc_keys),

Re: [PATCH] Support faulty USB IDs on DIBUSB_MC

[Patrick Boettcher]

On Thu, 29 Jan 2009, Mauro Carvalho Chehab wrote:
>> 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.

Patrick.

--
   Mail: patrick.boettcher@desy.de
   WWW:  http://www.wi-bw.tfh-wildau.de/~pboettch/

Re: [PATCH] Support faulty USB IDs on DIBUSB_MC

[Mauro Carvalho Chehab]

On Thu, 29 Jan 2009 14:08:01 +0100 (CET)
Patrick Boettcher <patrick.boettcher@desy.de> wrote:

> On Thu, 29 Jan 2009, Mauro Carvalho Chehab wrote:
> >> 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.

Nothing is perfect ;)

I suspect that you have more additions at the number of the devices than on the
number of device properties. So, the risk of doing bad things seems lower.
Also, a simple board addition won't need to touch at the number of devices.

IMO, it is really bad to have to explicitly say the number of devices at those
arrays. Maybe we may use some macro logic here to avoid such risks, or use a
NULL terminated list instead.

> 
> --
>    Mail: patrick.boettcher@desy.de
>    WWW:  http://www.wi-bw.tfh-wildau.de/~pboettch/




Cheers,
Mauro

Re: [PATCH] Support faulty USB IDs on DIBUSB_MC

[matthieu castet]

Hi Patrick,

Patrick Boettcher wrote:
> Hi,
> 
> sorry for not answering ealier, recently I became the master of 
> postponing things. :(
> 
> On Thu, 29 Jan 2009, Mauro Carvalho Chehab wrote:
> 
>>> +/* 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.
> 
Why not, I only don't want to maintain a patch for my device.

I wonder why didn't they use WP pin of the eeprom to avoid write.

Do you know what should be written.
After a quick search, I found [1]. Is that ok ?


Matthieu

[1]
EEPROM Address            Contents
       0        0xC0
       1        Vendor ID (VID) L
       2        Vendor ID (VID) H
       3        Product ID (PID) L
       4        Product ID (PID) H
       5        Device ID (DID) L
       6        Device ID (DID) H
       7        Configuration byte

Re: [PATCH] Support faulty USB IDs on DIBUSB_MC

[matthieu castet]

matthieu castet wrote:
> Hi Patrick,
> 
> Patrick Boettcher wrote:
>> Hi,
>>
>> sorry for not answering ealier, recently I became the master of 
>> postponing things. :(
>>
>> On Thu, 29 Jan 2009, Mauro Carvalho Chehab wrote:
>>
>>>> +/* 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.

Now that I think of that, I run sensors-detect on this machine, may be 
this is what trash the eeprom ?


Matthieu

Re: [PATCH] Support faulty USB IDs on DIBUSB_MC

[matthieu castet]

[-- Attachment #1: Type: text/plain, Size: 1279 bytes --]

Hi,

matthieu castet wrote:
> matthieu castet wrote:
>> Hi Patrick,
>>
>> Patrick Boettcher wrote:
>>> Hi,
>>>
>>> sorry for not answering ealier, recently I became the master of 
>>> postponing things. :(
>>>
>>> On Thu, 29 Jan 2009, Mauro Carvalho Chehab wrote:
>>>
>>>>> +/* 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.
> 
Patrick, any info about that.

I attach a possible (untested) patch.


Matthieu

[-- Attachment #2: dibusb_i2c --]
[-- Type: text/plain, Size: 926 bytes --]

Signed-off-by: Matthieu CASTET <castet.matthieu@free.fr>
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
@@ -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);

Re: [linux-dvb] [PATCH] Support faulty USB IDs on DIBUSB_MC

[Devin Heitmueller]

On Mon, Feb 9, 2009 at 2:40 PM, matthieu castet <castet.matthieu@free.fr> wrote:
> Hi,
>
> matthieu castet wrote:
>>
>> matthieu castet wrote:
>>>
>>> Hi Patrick,
>>>
>>> Patrick Boettcher wrote:
>>>>
>>>> Hi,
>>>>
>>>> sorry for not answering ealier, recently I became the master of
>>>> postponing things. :(
>>>>
>>>> On Thu, 29 Jan 2009, Mauro Carvalho Chehab wrote:
>>>>
>>>>>> +/* 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.
>>
> Patrick, any info about that.
>
> I attach a possible (untested) patch.
>
>
> Matthieu
>
> Signed-off-by: Matthieu CASTET <castet.matthieu@free.fr>
> 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
> @@ -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);

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.

Devin

-- 
Devin J. Heitmueller
http://www.devinheitmueller.com
AIM: devinheitmueller

Re: [linux-dvb] [PATCH] Support faulty USB IDs on DIBUSB_MC

[matthieu castet]

Devin Heitmueller wrote:
> On Mon, Feb 9, 2009 at 2:40 PM, matthieu castet <castet.matthieu@free.fr> wrote:
>> Hi,
>>
>> matthieu castet wrote:
>>> matthieu castet wrote:
>>>> Hi Patrick,
>>>>
>>>> Patrick Boettcher wrote:
>>>>> 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.
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...


Matthieu

Re: [linux-dvb] [PATCH] Support faulty USB IDs on DIBUSB_MC

[Devin Heitmueller]

On Mon, Feb 9, 2009 at 3:16 PM, matthieu castet <castet.matthieu@free.fr> wrote:
> Devin Heitmueller wrote:
>> On Mon, Feb 9, 2009 at 2:40 PM, matthieu castet <castet.matthieu@free.fr> wrote:
>>> Hi,
>>>
>>> matthieu castet wrote:
>>>> matthieu castet wrote:
>>>>> Hi Patrick,
>>>>>
>>>>> Patrick Boettcher wrote:
>>>>>> 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.
> 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...

Wow, I totally overlooked that your patch was against dibusb-common.c
instead of dib0700_core.c.  Yes, you are correct then.  However I
would still suggest sending an error to the console and returning
failure rather than just silently dropping the i2c read request.

Cheers,

Devin

-- 
Devin J. Heitmueller
http://www.devinheitmueller.com
AIM: devinheitmueller

Re: [linux-dvb] [PATCH] Support faulty USB IDs on DIBUSB_MC

[Devin Heitmueller]

On Mon, Feb 9, 2009 at 2:40 PM, matthieu castet
>> 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 <castet.matthieu@free.fr>
> 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
> @@ -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);
>

Regarding the patch itself, it looks ok at face value, but I would
rather see it scream loudly that you're trying to do this rather just
silently not performing the read.  This would help catch cases where
the calling driver is doing something the i2c implementation doesn't
support (so that the developer knows to either change his i2c code or
use the 1.20 version of the function).

I'm just speaking as a developer who got bit by this making the
dib0700 work with the xc5000, where I didn't realize the xc5000 driver
was doing something that the dib0700 i2c didn't support and it failed
silently.

Devin


-- 
Devin J. Heitmueller
http://www.devinheitmueller.com
AIM: devinheitmueller

[PATCH] DIBUSB_MC : fix i2c to not corrupt eeprom in case of strange read pattern

[matthieu castet]

[-- Attachment #1: Type: text/plain, Size: 458 bytes --]

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 <castet.matthieu@free.fr>




[-- Attachment #2: dibusb_i2c --]
[-- Type: text/plain, Size: 927 bytes --]

Signed-off-by: Matthieu CASTET <castet.matthieu@free.fr>
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
@@ -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);

Re: [PATCH] DIBUSB_MC : fix i2c to not corrupt eeprom in case of strange read pattern

[Patrick Boettcher]

Hi Matthieu,

On Sat, 16 May 2009, matthieu castet wrote:

> 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 <castet.matthieu@free.fr>

thanks a lot for your patch. I applied it, but could not test. But even 
it is breaks things, it's better to prevent those "false-reads" than not 
having this protection. Any breakage we will fix later.

Patrick.

--
   Mail: patrick.boettcher@desy.de
   WWW:  http://www.wi-bw.tfh-wildau.de/~pboettch/