I think usbvision_probe is called because of the dummy device code. We should probably rethink the dummy device.
----- Original Message ----
From: Thierry MERLE <thierry.merle@xxxxxxx>
To: Mauro Carvalho Chehab <mchehab@xxxxxxxxxxxxx>
Cc: Dwaine Garden <dwainegarden@xxxxxxxxxx>; Linux and Kernel Video <video4linux-list@xxxxxxxxxx>; Linux DVB <linux-dvb@xxxxxxxxxxx>
Sent: Saturday, April 14, 2007 3:08:01 PM
Subject: Re: [Patch] USBVision - Fix NT1005 Bridge detection
Hi Mauro,
Mauro Carvalho Chehab a écrit :
> Hi Thierry,
>
>
>> I am currently debugging the Trent's patch that makes a kernel oops and
>> I don't know why.
>> The usbvision_probe is called event if the device is not plugged, and
>> the kernel oops occurs on this piece of code:
>> model = devid->driver_info;
>> printk(KERN_INFO "%s: %s found\n", __FUNCTION__,
>> usbvision_device_data[model].ModelString);
>>
>> Memory alignment problem ? Why this probe is called even if there is no
>> device plugged-in ?
>>
> Very weird. It shouldn't be called when no device is plugged...
>
>
>> By putting something like:
>> model = devid->driver_info;
>> if(model<0 || model >HPG_WINTV) {
>> printk(KERN_INFO "model out of bounds %d\n",model);
>> return -ENODEV;
>> } else printk(KERN_INFO "model is %d\n",model);
>>
>> The problem disappeared...
>>
>
> A similar code is also present on em28xx. Anyway, it is better to have
> this kind of protection. I've added a patch, based on yours, fixing the
> bug. As you'd agreed, I've pushed the patches to master tree.
>
> It is better, however, to check why it is calling the code, without a
> valid USB ID, since it may mean a future trouble...
>
> Btw, I think we can also apply this one also:
>
> diff -r 6b6805e3c76d linux/drivers/media/video/usbvision/usbvision-video.c
> --- a/linux/drivers/media/video/usbvision/usbvision-video.c Sat Apr 14 15:17:08 2007 -0300
> +++ b/linux/drivers/media/video/usbvision/usbvision-video.c Sat Apr 14 15:19:06 2007 -0300
> @@ -1985,8 +1985,6 @@ static int __devinit usbvision_probe(str
> return -ENODEV;
> }
>
> - usb_get_dev(dev);
> -
> if ((usbvision = usbvision_alloc(dev)) == NULL) {
> err("%s: couldn't allocate USBVision struct", __FUNCTION__);
> return -ENOMEM;
>
>
> usb_get_dev() is already called previously on usbvision_probe.
>
>
Latest v4l-dvb tree tested OK.
You are right for usb_get_dev, only one call is necessary to increment
the reference count.
Thierry
From: Thierry MERLE <thierry.merle@xxxxxxx>
To: Mauro Carvalho Chehab <mchehab@xxxxxxxxxxxxx>
Cc: Dwaine Garden <dwainegarden@xxxxxxxxxx>; Linux and Kernel Video <video4linux-list@xxxxxxxxxx>; Linux DVB <linux-dvb@xxxxxxxxxxx>
Sent: Saturday, April 14, 2007 3:08:01 PM
Subject: Re: [Patch] USBVision - Fix NT1005 Bridge detection
Hi Mauro,
Mauro Carvalho Chehab a écrit :
> Hi Thierry,
>
>
>> I am currently debugging the Trent's patch that makes a kernel oops and
>> I don't know why.
>> The usbvision_probe is called event if the device is not plugged, and
>> the kernel oops occurs on this piece of code:
>> model = devid->driver_info;
>> printk(KERN_INFO "%s: %s found\n", __FUNCTION__,
>> usbvision_device_data[model].ModelString);
>>
>> Memory alignment problem ? Why this probe is called even if there is no
>> device plugged-in ?
>>
> Very weird. It shouldn't be called when no device is plugged...
>
>
>> By putting something like:
>> model = devid->driver_info;
>> if(model<0 || model >HPG_WINTV) {
>> printk(KERN_INFO "model out of bounds %d\n",model);
>> return -ENODEV;
>> } else printk(KERN_INFO "model is %d\n",model);
>>
>> The problem disappeared...
>>
>
> A similar code is also present on em28xx. Anyway, it is better to have
> this kind of protection. I've added a patch, based on yours, fixing the
> bug. As you'd agreed, I've pushed the patches to master tree.
>
> It is better, however, to check why it is calling the code, without a
> valid USB ID, since it may mean a future trouble...
>
> Btw, I think we can also apply this one also:
>
> diff -r 6b6805e3c76d linux/drivers/media/video/usbvision/usbvision-video.c
> --- a/linux/drivers/media/video/usbvision/usbvision-video.c Sat Apr 14 15:17:08 2007 -0300
> +++ b/linux/drivers/media/video/usbvision/usbvision-video.c Sat Apr 14 15:19:06 2007 -0300
> @@ -1985,8 +1985,6 @@ static int __devinit usbvision_probe(str
> return -ENODEV;
> }
>
> - usb_get_dev(dev);
> -
> if ((usbvision = usbvision_alloc(dev)) == NULL) {
> err("%s: couldn't allocate USBVision struct", __FUNCTION__);
> return -ENOMEM;
>
>
> usb_get_dev() is already called previously on usbvision_probe.
>
>
Latest v4l-dvb tree tested OK.
You are right for usb_get_dev, only one call is necessary to increment
the reference count.
Thierry
_______________________________________________ linux-dvb mailing list linux-dvb@xxxxxxxxxxx http://www.linuxtv.org/cgi-bin/mailman/listinfo/linux-dvb