Thats Oliver. I will make the changes to the driver for the 2.4 kernels.
Since this is what most people are using at the moment. I want to get
opinions on the code. Get a close to finish version, then work on the
2.5 version.
Is the switch going to happen on V4L2 in 2.5? I hear tons of rumours....
Other rumours say 2.6.
I notice your interest in pointing out faults in some of the drivers. I
like it and thought to ask for your opinion.
Thanks for taking the time Oliver.
Oliver Neukum
<oliver@neukum.
name> To
[EMAIL PROTECTED]
12/19/2002 cc
04:58 PM [EMAIL PROTECTED]
bcc
Subject
Re: USBVision NT1003 and 4 Driver
Review
Am Donnerstag, 19. Dezember 2002 22:04 schrieb [EMAIL PROTECTED]:
> Oliver, could you take a couple of minutes and review the usbvision
NT1003
> code. I would love to have a list
> of bad code. I'm not the most experienced person with the Linux USB
API.
>
> heheh.... You look like the perfect person to point out some faults in
the
> code.
Well, if you ask for it.
static void *usbvideo_rvmalloc(unsigned long size)
- use the usbvideo library code
if (waitqueue_active(&frame->wq)) {
wake_up_interruptible(&frame->wq);
- always do wake_up unconditionally
static int usbvision_read_reg(struct usb_usbvision *usbvision, unsigned
char reg)
static int usbvision_write_reg(struct usb_usbvision *usbvision, unsigned
char reg
- they race with disconnect. use a semaphore. and check for disconnection
under it
for (retries = 5;;) {
rc = usb_control_msg(usbvision->dev,
usb_sndctrlpipe(usbvision->dev, 1),
USBVISION_OP_CODE,
- races with disconnect. see above
static int usbvision_set_compress_params(struct usb_usbvision *usbvision)
- same race
urb = usb_alloc_urb(FRAMES_PER_DESC);
if (urb == NULL) {
printk(KERN_ERR
"usbvision_init_isoc: usb_init_isoc() failed.\n");
return -ENOMEM;
}
- memory leak in error case. you have to free the already allocated urbs
/* Submit all URBs */
for (i = 0; i < USBVISION_NUMSBUF; i++) {
err = usb_submit_urb(usbvision->sbuf[i].urb);
if (err)
printk(KERN_ERR
"usbvision_init_isoc: usb_run_isoc(%d) ret %d\n",
i, err);
}
- won't do. if one fails, you have to unlink the others and return an error
/* Set packet size to 0 */
j = usb_set_interface(usbvision->dev, usbvision->iface,
usbvision->ifaceAltInactive);
- races with disconnect. see above.
if ((usbvision->fbuf == NULL) || (usbvision->scratch == NULL))
{
printk(KERN_ERR "usbvision_open: unable to allocate
memory for frame buffer and scratch\n");
err = -ENOMEM;
- memory leak in error case
if (usbvision->sbuf[i].data == NULL) {
err = -ENOMEM;
break;
}
- memory leak in error case
case VIDIOCGCHAN:
{
- not threadsafe. it needs a semaphore
case VIDIOCSPICT:
case VIDIOCMCAPTURE:
- see above
current->state = TASK_INTERRUPTIBLE;
- use set_current_state(). Note: if you have pending signals, your state
already is TASK_RUNNING
interruptible_sleep_on((void *) &frame->wq);
- never use this. always schedule
/* Code below may sleep, need to lock module while we are here */
MOD_INC_USE_COUNT;
- No longer true in 2.5
> I would like to fix the code that's broken or outdated. I would post to
> the dev list, but I would be killed before Christmas comes.
>
> Yes, I remember the last web cam driver discussion. The poor guy was
> pounded dead...
Your driver is not so bad. It's impossible to understand it fully in so
little
time, but it has no larger problems.
You should think about converting it to V4L2 and you need to change
to the new urb completion signature.
Then you should submit it for inclusion into the kernel.
Regards
Oliver
-------------------------------------------------------------------------------
The information in this e-mail is intended solely for the addressee(s)
named, and is confidential. Any other distribution, disclosure or copying
is strictly prohibited. If you have received this communication in error,
please reply by e-mail to the sender and delete or destroy all copies of
this message.
Les renseignements contenus dans le pr�sent message �lectronique sont
confidentiels et concernent exclusivement le(s) destinataire(s) d�sign�(s).
Il est strictement interdit de distribuer ou de copier ce message. Si vous
avez re�u ce message par erreur, veuillez r�pondre par courriel �
l'exp�diteur et effacer ou d�truire toutes les copies du pr�sent message.
-------------------------------------------------------
This SF.NET email is sponsored by: The Best Geek Holiday Gifts!
Time is running out! Thinkgeek.com has the coolest gifts for
your favorite geek. Let your fingers do the typing. Visit Now.
T H I N K G E E K . C O M http://www.thinkgeek.com/sf/
_______________________________________________
[EMAIL PROTECTED]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
