Hi, I'm just browsing the v4l and v4l2 code. I'd like to see v4l2 early in 2.5.x in the standard kernel and looking for a clever way to get this done (and fix some videodev problems). I've seen my videodev patch from bttv 0.8.x made it into the videodevX code. Now we have *three* methods to register a driver: (1) The very old one (video_device + function pointers) (2) The bttv 0.8.x one (video_device + fops) (3) The v4l2 one (v4l2_device). This is one to much. We have to keep (1) for backward compatibility with the existing drivers in the kernel. I'd like to drop (3) and keep (2) for the following reasons: * I think it's clever to simply use a struct file_operations directly. Most of the fops functions in videodevX.c simply pass down the call to the driver (the only exception is v4l2_video_ioctl), so why not call the driver directly in the first place? Also we don't have to fiddle with videodev if struct file_operations changes again for some reason. * I'd like to have videodev.c just register drivers and hand out the minor numbers and do _nothing_ else, i.e. do what soundcore does for the sound drivers. videodev.c shouldn't care which API a driver uses. All the v4l1 compatibility helper code which is in videodevX right now should go to some other module (v4l1-compat.c or something like that). Plan as follows: * fix videodev.c. A patch is below. It does add register method (2) to the kernel's videodev.c. It adds a lock to the register function to fix a race. It allows drivers to ask for a specific device number. This becomes important these days as people start using different devices (bttv+usbcam) at the same time. Fixed module load order being the only way to get a fixed special file -> device mapping is not very nice [longer version of the patch which fixes all kernel drivers is available too]. IMHO this should go into 2.4.x. * Fixes/cleanups in the v4l2 capture API. Major item on the TODO list is the dma-to-userspace issue discussed here some time ago. Also some minor byte order problems (15/16 bpp rgb is available in little endian only, ...). Put the v4l2 capture API largely as-is into the 2.5.x kernel then. The vbi stuff can be dropped from v4l2 I think. The current vbi API in 2.4.x is cloned from v4l2, and we certainly don't need exactly the same API twice in the kernel. Comments? What is the status of the other v4l2 API's? codec? effects? Any existing drivers which use these? Gerd ---------------------------- cut here --------------------------- diff -ur vanilla-2.4.3/drivers/media/video/videodev.c 2.4.3/drivers/media/video/videodev.c --- vanilla-2.4.3/drivers/media/video/videodev.c Fri Mar 2 20:12:10 2001 +++ 2.4.3/drivers/media/video/videodev.c Tue Apr 10 15:51:54 2001 @@ -30,6 +30,7 @@ #include <asm/uaccess.h> #include <asm/system.h> +#include <asm/semaphore.h> #include <linux/kmod.h> @@ -161,6 +162,22 @@ goto error_out; } } + if (vfl->fops) { + int err = 0; + struct file_operations *old_fops; + + unlock_kernel(); + old_fops = file->f_op; + file->f_op = fops_get(vfl->fops); + if(file->f_op->open) + err = file->f_op->open(inode,file); + if (err) { + fops_put(file->f_op); + file->f_op = fops_get(old_fops); + } + fops_put(old_fops); + return err; + } if(vfl->busy) { retval = -EBUSY; goto error_out; @@ -394,7 +411,7 @@ if (vfd == d->vdev) { remove_proc_entry(d->name, video_dev_proc_entry); list_del (&d->proc_list); - kfree (d); + kfree(d); break; } } @@ -406,9 +423,10 @@ /** * video_register_device - register video4linux devices - * @vfd: video device structure we want to register + * @vfd: video device structure we want to register * @type: type of device to register - * FIXME: needs a semaphore on 2.3.x + * @nr: which device number (0 == /dev/video0, 1 == /dev/video1, ... + * -1 == first free) * * The registration code assigns minor numbers based on the type * requested. -ENFILE is returned in all the device slots for this @@ -427,14 +445,17 @@ * * %VFL_TYPE_RADIO - A radio card */ - -int video_register_device(struct video_device *vfd, int type) + +static DECLARE_MUTEX(videodev_register_lock); + +int video_register_device(struct video_device *vfd, int type, int nr) { int i=0; int base; int err; int end; char *name_base; + char name[16]; switch(type) { @@ -461,49 +482,58 @@ default: return -1; } - - for(i=base;i<end;i++) - { - if(video_device[i]==NULL) - { - char name[16]; - - video_device[i]=vfd; - vfd->minor=i; - /* The init call may sleep so we book the slot out - then call */ - MOD_INC_USE_COUNT; - if(vfd->initialize) - { - err=vfd->initialize(vfd); - if(err<0) - { - video_device[i]=NULL; - MOD_DEC_USE_COUNT; - return err; - } - } - sprintf (name, "v4l/%s%d", name_base, i - base); - /* - * Start the device root only. Anything else - * has serious privacy issues. - */ - vfd->devfs_handle = - devfs_register (NULL, name, DEVFS_FL_DEFAULT, - VIDEO_MAJOR, vfd->minor, - S_IFCHR | S_IRUSR | S_IWUSR, - &video_fops, NULL); - -#if defined(CONFIG_PROC_FS) && defined(CONFIG_VIDEO_PROC_FS) - sprintf (name, "%s%d", name_base, i - base); - videodev_proc_create_dev (vfd, name); -#endif - - return 0; + /* pick a minor number */ + down(&videodev_register_lock); + if (-1 == nr) { + /* use first free */ + for(i=base;i<end;i++) + if (NULL == video_device[i]) + break; + if (i == end) { + up(&videodev_register_lock); + return -ENFILE; + } + } else { + /* use the one the driver asked for */ + i = base+nr; + if (NULL != video_device[i]) { + up(&videodev_register_lock); + return -ENFILE; } } - return -ENFILE; + video_device[i]=vfd; + vfd->minor=i; + up(&videodev_register_lock); + + /* The init call may sleep so we book the slot out + then call */ + MOD_INC_USE_COUNT; + if(vfd->initialize) { + err=vfd->initialize(vfd); + if(err<0) { + video_device[i]=NULL; + MOD_DEC_USE_COUNT; + return err; + } + } + sprintf (name, "v4l/%s%d", name_base, i - base); + /* + * Start the device root only. Anything else + * has serious privacy issues. + */ + vfd->devfs_handle = + devfs_register (NULL, name, DEVFS_FL_DEFAULT, + VIDEO_MAJOR, vfd->minor, + S_IFCHR | S_IRUSR | S_IWUSR, + &video_fops, + NULL); + +#if defined(CONFIG_PROC_FS) && defined(CONFIG_VIDEO_PROC_FS) + sprintf (name, "%s%d", name_base, i - base); + videodev_proc_create_dev (vfd, name); +#endif + return 0; } /** @@ -595,3 +625,9 @@ MODULE_AUTHOR("Alan Cox"); MODULE_DESCRIPTION("Device registrar for Video4Linux drivers"); + +/* + * Local variables: + * c-basic-offset: 8 + * End: + */ diff -ur vanilla-2.4.3/include/linux/videodev.h 2.4.3/include/linux/videodev.h --- vanilla-2.4.3/include/linux/videodev.h Wed Apr 4 16:11:12 2001 +++ 2.4.3/include/linux/videodev.h Tue Apr 10 16:09:53 2001 @@ -31,11 +31,12 @@ int busy; int minor; devfs_handle_t devfs_handle; + struct file_operations *fops; }; extern int videodev_init(void); #define VIDEO_MAJOR 81 -extern int video_register_device(struct video_device *, int type); +extern int video_register_device(struct video_device *, int type, int nr); #define VFL_TYPE_GRABBER 0 #define VFL_TYPE_VBI 1