ChangeSet 1.872.3.1, 2002/11/18 16:54:24-08:00, joe@wavicle.org [PATCH] vicam.c Included in this patch: - (From John Tyner) Move allocation of memory out of send_control_msg. With the allocation moved to open, control messages are less expensive since they don't allocate and free memory every time. - (From John Tyner) Change the behaviour of send_control_msg to return 0 on success instead of the number of bytes transferred. - Clean up of a couple down_interruptible() calls that weren't checking for failure - Rewrite of proc fs entries to use one file per value instead of parsing in the kernel diff -Nru a/drivers/usb/media/vicam.c b/drivers/usb/media/vicam.c --- a/drivers/usb/media/vicam.c Wed Nov 20 01:01:26 2002 +++ b/drivers/usb/media/vicam.c Wed Nov 20 01:01:26 2002 @@ -1,6 +1,7 @@ /* * USB ViCam WebCam driver - * Copyright (c) 2002 Joe Burks (jburks@wavicle.org) + * Copyright (c) 2002 Joe Burks (jburks@wavicle.org), + * John Tyner (fill in email address) * * Supports 3COM HomeConnect PC Digital WebCam * @@ -403,13 +404,14 @@ } vfree(mem); } - + struct vicam_camera { u16 shutter_speed; // capture shutter speed u16 gain; // capture gain u8 *raw_image; // raw data captured from the camera u8 *framebuf; // processed data in RGB24 format + u8 *cntrlbuf; // area used to send control msgs struct video_device vdev; // v4l video device struct usb_device *udev; // usb device @@ -421,8 +423,8 @@ u8 bulkEndpoint; bool needsDummyRead; -#ifdef CONFIG_PROC_FS - struct proc_dir_entry *proc_entry; +#if defined(CONFIG_VIDEO_PROC_FS) + struct proc_dir_entry *proc_dir; #endif }; @@ -437,22 +439,16 @@ { int status; - // for reasons not yet known to me, you can't send USB control messages - // with data in the module (if you are compiled as a module). Whatever - // the reason, copying it to memory allocated as kernel memory then - // doing the usb control message fixes the problem. - - unsigned char *transfer_buffer = kmalloc(size, GFP_KERNEL); - memcpy(transfer_buffer, cp, size); + /* cp must be memory that has been allocated by kmalloc */ status = usb_control_msg(udev, usb_sndctrlpipe(udev, 0), request, USB_DIR_OUT | USB_TYPE_VENDOR | USB_RECIP_DEVICE, value, index, - transfer_buffer, size, HZ); + cp, size, HZ); - kfree(transfer_buffer); + status = min(status, 0); if (status < 0) { printk(KERN_INFO "Failed sending control message, error %d.\n", @@ -465,29 +461,30 @@ static int initialize_camera(struct vicam_camera *cam) { + const struct { + u8 *data; + u32 size; + } firmware[] = { + { .data = setup1, .size = sizeof(setup1) }, + { .data = setup2, .size = sizeof(setup2) }, + { .data = setup3, .size = sizeof(setup3) }, + { .data = setup4, .size = sizeof(setup4) }, + { .data = setup5, .size = sizeof(setup5) }, + { .data = setup3, .size = sizeof(setup3) }, + { .data = NULL, .size = 0 } + }; + struct usb_device *udev = cam->udev; - int status; + int err, i; - if ((status = - send_control_msg(udev, 0xff, 0, 0, setup1, sizeof (setup1))) < 0) - return status; - if ((status = - send_control_msg(udev, 0xff, 0, 0, setup2, sizeof (setup2))) < 0) - return status; - if ((status = - send_control_msg(udev, 0xff, 0, 0, setup3, sizeof (setup3))) < 0) - return status; - if ((status = - send_control_msg(udev, 0xff, 0, 0, setup4, sizeof (setup4))) < 0) - return status; - if ((status = - send_control_msg(udev, 0xff, 0, 0, setup5, sizeof (setup5))) < 0) - return status; - if ((status = - send_control_msg(udev, 0xff, 0, 0, setup3, sizeof (setup3))) < 0) - return status; + for (i = 0, err = 0; firmware[i].data && !err; i++) { + memcpy(cam->cntrlbuf, firmware[i].data, firmware[i].size); - return 0; + err = send_control_msg(udev, 0xff, 0, 0, + cam->cntrlbuf, firmware[i].size); + } + + return err; } static int @@ -752,7 +749,8 @@ "vicam video_device improperly initialized"); } - down_interruptible(&cam->busy_lock); + if ( down_interruptible(&cam->busy_lock) ) + return -EINTR; if (cam->open_count > 0) { printk(KERN_INFO @@ -774,6 +772,14 @@ return -ENOMEM; } + cam->cntrlbuf = kmalloc(PAGE_SIZE, GFP_KERNEL); + if (!cam->cntrlbuf) { + kfree(cam->raw_image); + rvfree(cam->framebuf, VICAM_MAX_FRAME_SIZE * VICAM_FRAMES); + up(&cam->busy_lock); + return -ENOMEM; + } + // First upload firmware, then turn the camera on if (!cam->is_initialized) { @@ -803,6 +809,7 @@ kfree(cam->raw_image); rvfree(cam->framebuf, VICAM_MAX_FRAME_SIZE * VICAM_FRAMES); + kfree(cam->cntrlbuf); cam->open_count--; @@ -915,7 +922,7 @@ static void read_frame(struct vicam_camera *cam, int framenum) { - unsigned char request[16]; + unsigned char *request = cam->cntrlbuf; int realShutter; int n; int actual_length; @@ -984,7 +991,8 @@ DBG("read %d bytes.\n", (int) count); - down_interruptible(&cam->busy_lock); + if ( down_interruptible(&cam->busy_lock) ) + return -EINTR; if (*ppos >= VICAM_MAX_FRAME_SIZE) { *ppos = 0; @@ -1057,24 +1065,17 @@ return 0; } -#ifdef CONFIG_PROC_FS +#if defined(CONFIG_VIDEO_PROC_FS) static struct proc_dir_entry *vicam_proc_root = NULL; -static int -vicam_read_proc(char *page, char **start, off_t off, - int count, int *eof, void *data) +int vicam_read_helper(char *page, char **start, off_t off, + int count, int *eof, int value) { char *out = page; int len; - struct vicam_camera *cam = (struct vicam_camera *) data; - out += - sprintf(out, "Vicam-based WebCam Linux Driver.\n"); - out += sprintf(out, "(c) 2002 Joe Burks (jburks@wavicle.org)\n"); - out += sprintf(out, "vicam stats:\n"); - out += sprintf(out, " Shutter Speed: 1/%d\n", cam->shutter_speed); - out += sprintf(out, " Gain: %d\n", cam->gain); + out += sprintf(out, "%d",value); len = out - page; len -= off; @@ -1089,37 +1090,42 @@ return len; } -static int -vicam_write_proc(struct file *file, const char *buffer, - unsigned long count, void *data) +int vicam_read_proc_shutter(char *page, char **start, off_t off, + int count, int *eof, void *data) { - char *in; - char *start; - struct vicam_camera *cam = (struct vicam_camera *) data; + return vicam_read_helper(page,start,off,count,eof, + ((struct vicam_camera *)data)->shutter_speed); +} - in = kmalloc(count + 1, GFP_KERNEL); - if (!in) - return -ENOMEM; +int vicam_read_proc_gain(char *page, char **start, off_t off, + int count, int *eof, void *data) +{ + return vicam_read_helper(page,start,off,count,eof, + ((struct vicam_camera *)data)->gain); +} - in[count] = 0; // I'm not sure buffer is gauranteed to be null terminated - // so I do this to make sure I have a null in there. +int vicam_write_proc_shutter(struct file *file, const char *buffer, + unsigned long count, void *data) +{ + struct vicam_camera *cam = (struct vicam_camera *)data; + + cam->shutter_speed = simple_strtoul(buffer, NULL, 10); - strncpy(in, buffer, count); + return count; +} - start = strstr(in, "gain="); - if (start - && (start == in || *(start - 1) == ' ' || *(start - 1) == ',')) - cam->gain = simple_strtoul(start + 5, NULL, 10); - - start = strstr(in, "shutter="); - if (start - && (start == in || *(start - 1) == ' ' || *(start - 1) == ',')) - cam->shutter_speed = simple_strtoul(start + 8, NULL, 10); +int vicam_write_proc_gain(struct file *file, const char *buffer, + unsigned long count, void *data) +{ + struct vicam_camera *cam = (struct vicam_camera *)data; + + cam->gain = simple_strtoul(buffer, NULL, 10); - kfree(in); return count; } + + void vicam_create_proc_root(void) { @@ -1140,11 +1146,9 @@ } void -vicam_create_proc_entry(void *ptr) +vicam_create_proc_entry(struct vicam_camera *cam) { - struct vicam_camera *cam = (struct vicam_camera *) ptr; - - char name[7]; + char name[64]; struct proc_dir_entry *ent; DBG(KERN_INFO "vicam: creating proc entry\n"); @@ -1158,46 +1162,49 @@ sprintf(name, "video%d", cam->vdev.minor); - ent = - create_proc_entry(name, S_IFREG | S_IRUGO | S_IWUSR, - vicam_proc_root); - if (!ent) - return; + cam->proc_dir = create_proc_entry(name, S_IFDIR, vicam_proc_root); - ent->data = cam; - ent->read_proc = vicam_read_proc; - ent->write_proc = vicam_write_proc; - ent->size = 512; - cam->proc_entry = ent; + if ( !cam->proc_dir ) return; // We should probably return an error here + + ent = + create_proc_entry("shutter", S_IFREG | S_IRUGO | S_IWUSR, + cam->proc_dir); + if (ent) { + ent->data = cam; + ent->read_proc = vicam_read_proc_shutter; + ent->write_proc = vicam_write_proc_shutter; + ent->size = 64; + } + + ent = create_proc_entry("gain", S_IFREG | S_IRUGO | S_IWUSR, + cam->proc_dir); + if ( ent ) { + ent->data = cam; + ent->read_proc = vicam_read_proc_gain; + ent->write_proc = vicam_write_proc_gain; + ent->size = 64; + } } void vicam_destroy_proc_entry(void *ptr) { struct vicam_camera *cam = (struct vicam_camera *) ptr; - char name[7]; + char name[16]; - if (!cam || !cam->proc_entry) + if ( !cam->proc_dir ) return; sprintf(name, "video%d", cam->vdev.minor); - remove_proc_entry(name, vicam_proc_root); - cam->proc_entry = NULL; + remove_proc_entry("shutter", cam->proc_dir); + remove_proc_entry("gain", cam->proc_dir); + remove_proc_entry(name,vicam_proc_root); + cam->proc_dir = NULL; } #endif -int -vicam_video_init(struct video_device *vdev) -{ - // This would normally create the proc entry for this camera -#ifdef CONFIG_PROC_FS - vicam_create_proc_entry(vdev->priv); -#endif - return 0; -} - static struct file_operations vicam_fops = { .owner = THIS_MODULE, .open = vicam_open, @@ -1296,6 +1303,8 @@ return -EIO; } + vicam_create_proc_entry(cam); + printk(KERN_INFO "ViCam webcam driver now controlling video device %d\n",cam->vdev.minor); dev_set_drvdata(&intf->dev, cam); @@ -1314,7 +1323,7 @@ video_unregister_device(&cam->vdev); -#ifdef CONFIG_PROC_FS +#if defined(CONFIG_VIDEO_PROC_FS) vicam_destroy_proc_entry(cam); #endif @@ -1329,7 +1338,7 @@ usb_vicam_init(void) { DBG(KERN_INFO "ViCam-based WebCam driver startup\n"); -#ifdef CONFIG_PROC_FS +#if defined(CONFIG_VIDEO_PROC_FS) vicam_create_proc_root(); #endif if (usb_register(&vicam_driver) != 0) @@ -1344,7 +1353,7 @@ "ViCam-based WebCam driver shutdown\n"); usb_deregister(&vicam_driver); -#ifdef CONFIG_PROC_FS +#if defined(CONFIG_VIDEO_PROC_FS) vicam_destroy_proc_root(); #endif }