Hi,

In 2.5.x I want to drop the open/read/write/poll/mmap/... function
pointers from struct video_device and all the useless
video_read/write/poll/mmap/...  wrapper functions from videodev.c.

Instead, I'd like to see the video4linux drivers use struct
file_operations directly and handle the device registration like
soundcore does it for the sound drivers: simply swap file->f_op in
video_open().

The patch below does part one of the plan -- for 2.4.x kernels.  It adds
the fops pointer to struct video_device and makes video_open use it if
available, so both old + new style drivers will work.

It also provides a ioctl wrapper function which handles copying the
ioctl args from/to userspace, so we have this at one place can drop all
the copy_from/to_user calls within the v4l device driver ioctl handlers.

Comments?

  Gerd

[mailed to both lkml + video4linux list]

-------------------------- cut here -------------------------
--- linux-2.4.18-pre8/include/linux/videodev.h  Tue Feb  5 11:21:37 2002
+++ linux/include/linux/videodev.h      Tue Feb  5 11:33:04 2002
@@ -4,6 +4,18 @@
 #include <linux/types.h>
 #include <linux/version.h>
 
+#if 0
+/*
+ * v4l2 is still work-in-progress, integration planed for 2.5.x
+ *   v4l2 project homepage:   http://www.thedirks.org/v4l2/
+ *   patches available from:  http://bytesex.org/patches/
+ */ 
+# define HAVE_V4L2 1
+# include <linux/videodev2.h>
+#else
+# undef HAVE_V4L2
+#endif
+
 #ifdef __KERNEL__
 
 #include <linux/poll.h>
@@ -12,22 +24,31 @@
 struct video_device
 {
        struct module *owner;
-       char name[32];
-       int type;
+       char name[32];
+       int type;       /* v4l1 */
+       int type2;      /* v4l2 */
        int hardware;
 
+       /* old, obsolete interface -- to be removed some day (2.5.x ?) */
        int (*open)(struct video_device *, int mode);
        void (*close)(struct video_device *);
        long (*read)(struct video_device *, char *, unsigned long, int noblock);
-       /* Do we need a write method ? */
        long (*write)(struct video_device *, const char *, unsigned long, int noblock);
-#if LINUX_VERSION_CODE >= 0x020100
        unsigned int (*poll)(struct video_device *, struct file *, poll_table *);
-#endif
        int (*ioctl)(struct video_device *, unsigned int , void *);
        int (*mmap)(struct video_device *, const char *, unsigned long);
-       int (*initialize)(struct video_device *);       
-       void *priv;             /* Used to be 'private' but that upsets C++ */
+       int (*initialize)(struct video_device *);       
+
+       /* new interface -- we will use file_operations directly
+        * like soundcore does.
+        * kernel_ioctl() will be called by video_generic_ioctl.
+        * video_generic_ioctl() does the userspace copying of the
+        * ioctl arguments */
+       struct file_operations *fops;
+       int (*kernel_ioctl)(struct inode *inode, struct file *file,
+                           unsigned int cmd, void *arg);
+
+       void *priv; /* Used to be 'private' but that upsets C++ */
        int busy;
        int minor;
        devfs_handle_t devfs_handle;
@@ -42,8 +63,11 @@
 #define VFL_TYPE_VTX           3
 
 extern void video_unregister_device(struct video_device *);
-#endif
+extern struct video_device* video_devdata(struct file*);
 
+extern int video_generic_ioctl(struct inode *inode, struct file *file,
+                              unsigned int cmd, unsigned long arg);
+#endif /* __KERNEL__ */
 
 #define VID_TYPE_CAPTURE       1       /* Can capture */
 #define VID_TYPE_TUNER         2       /* Can tune */
@@ -149,6 +173,7 @@
 #define VIDEO_AUDIO_VOLUME     4
 #define VIDEO_AUDIO_BASS       8
 #define VIDEO_AUDIO_TREBLE     16      
+#define VIDEO_AUDIO_BALANCE    32
        char    name[16];
 #define VIDEO_SOUND_MONO       1
 #define VIDEO_SOUND_STEREO     2
@@ -378,4 +403,10 @@
 #define VID_HARDWARE_MEYE      32      /* Sony Vaio MotionEye cameras */
 #define VID_HARDWARE_CPIA2     33
 
-#endif
+#endif /* __LINUX_VIDEODEV_H */
+
+/*
+ * Local variables:
+ * c-basic-offset: 8
+ * End:
+ */
--- linux-2.4.18-pre8/drivers/media/video/Config.in     Tue Feb  5 11:21:40 2002
+++ linux/drivers/media/video/Config.in Tue Feb  5 11:33:04 2002
@@ -8,6 +8,7 @@
 dep_tristate '  I2C on parallel port' CONFIG_I2C_PARPORT $CONFIG_PARPORT $CONFIG_I2C
 
 comment 'Video Adapters'
+dep_tristate '  Skeleton Driver Video For Linux' CONFIG_VIDEO_SKELETON 
+$CONFIG_VIDEO_DEV
 if [ "$CONFIG_I2C_ALGOBIT" = "y" -o "$CONFIG_I2C_ALGOBIT" = "m" ]; then
    dep_tristate '  BT848 Video For Linux' CONFIG_VIDEO_BT848 $CONFIG_VIDEO_DEV 
$CONFIG_PCI $CONFIG_I2C_ALGOBIT
 fi
--- linux-2.4.18-pre8/drivers/media/video/Makefile      Tue Feb  5 11:21:32 2002
+++ linux/drivers/media/video/Makefile  Tue Feb  5 11:33:04 2002
@@ -33,6 +33,7 @@
 
 obj-$(CONFIG_VIDEO_DEV) += videodev.o
 
+obj-$(CONFIG_VIDEO_SKELETON) += skeleton.o
 obj-$(CONFIG_BUS_I2C) += i2c-old.o
 obj-$(CONFIG_VIDEO_BT848) += bttv.o msp3400.o tvaudio.o \
        tda7432.o tda9875.o tuner.o
--- linux-2.4.18-pre8/drivers/media/video/skeleton.c    Tue Feb  5 11:33:04 2002
+++ linux/drivers/media/video/skeleton.c        Tue Feb  5 12:27:18 2002
@@ -0,0 +1,249 @@
+/*
+ * video4linux driver skeleton
+ *
+ *   You can build and insmod it.  It doesn't do anything fancy, just
+ *   prints some messages when the file_operations functions are called.
+ *
+ *   written by Gerd Knorr <[EMAIL PROTECTED]>
+ *   this source file is public domain, you can do with it
+ *   whatever you like ...
+ *
+ */
+
+#include <linux/init.h>
+#include <linux/list.h>
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/slab.h>
+#include <linux/videodev.h>
+
+#define DEVNAME "skeleton"
+
+/* ------------------------------------------------------------------ */
+
+struct device_data {
+       struct list_head     devlist;
+       struct video_device  vdev;
+       struct semaphore     lock;
+       int                  users;
+       /* more device specific stuff */
+};
+
+struct file_data {
+       struct device_data   *dev;
+
+       /* just some random value to show how to keep multiple file
+        * handles apart and decorate the debug printk's a bit */
+       int                  cookie; 
+
+       /* more filehandle specific data */
+};
+
+/* ------------------------------------------------------------------ */
+
+static unsigned int exclusive = 0;
+static unsigned int debug = 1;
+static unsigned int video_nr = -1;
+MODULE_PARM(debug,"i");
+MODULE_PARM_DESC(debug,"enable debug messages (default: on)");
+MODULE_PARM(exclusive,"i");
+MODULE_PARM_DESC(exclusive,"only one process can open the device (default: off)");
+MODULE_PARM(video_nr,"i");
+MODULE_PARM_DESC(video_nr,"device number");
+
+MODULE_AUTHOR("Gerd Knorr");
+MODULE_DESCRIPTION("video4linux driver skeleton");
+MODULE_LICENSE("GPL and additional rights");
+
+#define dprintk(fmt, arg...)   if (debug) \
+       printk(KERN_DEBUG DEVNAME ": " fmt, ## arg)
+
+static struct list_head skeldev;
+
+/* ------------------------------------------------------------------ */
+
+static int skeleton_open(struct inode *inode, struct file *file)
+{
+       static int cookie = 42;
+       unsigned int minor = MINOR(inode->i_rdev);
+       struct device_data *h,*dev = NULL;
+       struct file_data *fh;
+       struct list_head *list;
+       
+       dprintk("open: looking for my device [minor=%d]\n",minor);
+
+       list_for_each(list,&skeldev) {
+               h = list_entry(list, struct device_data, devlist);
+               if (h->vdev.minor == minor)
+                       dev = h;
+       }
+       if (NULL == dev)
+               return -ENODEV;
+       
+       down(&dev->lock);
+       dprintk("open: device found (%d users)\n",dev->users);
+       if (exclusive && dev->users > 0) {
+               dprintk("open: device busy\n");
+               up(&dev->lock);
+               return -EBUSY;
+       }
+       
+       /* allocate + initialize per filehandle data */
+       fh = kmalloc(sizeof(*fh),GFP_KERNEL);
+       if (NULL == fh)
+               return -ENOMEM;
+       file->private_data = fh;
+       memset(fh,0,sizeof(*fh));
+       fh->dev    = dev;
+       fh->cookie = cookie++;
+
+       dprintk("open successful [minor=%d,cookie=%d]\n",
+               dev->vdev.minor,fh->cookie);
+       dev->users++;
+       up(&dev->lock);
+        return 0;
+}
+
+static int skeleton_release(struct inode *inode, struct file *file)
+{
+       struct file_data *fh    = file->private_data;
+       struct device_data *dev = fh->dev;
+
+       dprintk("release called [minor=%d,cookie=%d]\n",
+               dev->vdev.minor,fh->cookie);
+       file->private_data = NULL;
+       kfree(fh);
+       dev->users--;
+       return 0;
+}
+
+static ssize_t
+skeleton_read(struct file *file, char *data, size_t count, loff_t *ppos)
+{
+       struct file_data *fh    = file->private_data;
+       struct device_data *dev = fh->dev;
+
+       dprintk("read called [minor=%d,cookie=%d]\n",
+               dev->vdev.minor,fh->cookie);
+       return 0; /* EOF */
+}
+
+static int
+skeleton_mmap(struct file *file, struct vm_area_struct * vma)
+{
+       struct file_data *fh    = file->private_data;
+       struct device_data *dev = fh->dev;
+
+       dprintk("mmap called [minor=%d,cookie=%d]\n",
+               dev->vdev.minor,fh->cookie);
+       return -EINVAL;
+}
+
+/*
+ * This function is _not_ called directly, but from
+ * video_generic_ioctl (and maybe others).  userspace
+ * copying is done already, arg is a kernel pointer.
+ */
+static int skeleton_ioctl(struct inode *inode, struct file *file,
+                         unsigned int cmd, void *arg)
+{
+       struct file_data *fh    = file->private_data;
+       struct device_data *dev = fh->dev;
+
+       dprintk("ioctl called [minor=%d,cookie=%d,cmd=0x%x,arg=%p]\n",
+               dev->vdev.minor,fh->cookie,cmd,arg);
+       switch (cmd) {
+       case VIDIOCGCAP:
+       {
+                struct video_capability *cap = arg;
+               memset(cap,0,sizeof(*cap));
+               strcpy(cap->name,dev->vdev.name);
+                return 0;
+       }
+       /* all the ioctls go here */
+
+       default:
+               return -ENOIOCTLCMD;
+       }
+       return 0;
+}
+
+static struct file_operations skeleton_fops =
+{
+       owner:    THIS_MODULE,
+       open:     skeleton_open,
+       release:  skeleton_release,
+       read:     skeleton_read,
+       mmap:     skeleton_mmap,
+       ioctl:    video_generic_ioctl,
+       llseek:   no_llseek,
+};
+
+static struct video_device skeleton_template =
+{
+       name:          "undef",
+       type:          0,
+       hardware:      0,
+       fops:          &skeleton_fops,
+       kernel_ioctl:  skeleton_ioctl,
+       minor:         -1,
+};
+
+/* ------------------------------------------------------------------ */
+
+int skeleton_initdev(char *name)
+{
+       struct device_data *dev;
+       int err;
+
+       dev = kmalloc(sizeof(*dev),GFP_KERNEL);
+       if (NULL == dev)
+               return -ENOMEM;
+       memset(dev,0,sizeof(*dev));
+        init_MUTEX(&dev->lock);
+
+       /* initialize device here */
+
+       dev->vdev = skeleton_template;
+       strcpy(dev->vdev.name, name);
+       dev->vdev.priv = dev;
+       err = video_register_device(&dev->vdev,VFL_TYPE_GRABBER,video_nr);
+       if (err < 0) {
+               kfree(dev);
+               return err;
+       }
+       list_add_tail(&dev->devlist,&skeldev);
+       dprintk("registered device %s [minor=%d]\n",name,dev->vdev.minor);
+       return 0;
+}
+
+int skeleton_init(void)
+{
+       dprintk("video4linux skeleton driver loaded\n");
+       INIT_LIST_HEAD(&skeldev);
+       skeleton_initdev("skeleton-1");
+       skeleton_initdev("skeleton-2");
+       return 0;
+}
+
+void skeleton_fini(void)
+{
+       struct device_data *dev;
+
+       while (!list_empty(&skeldev)) {
+               dev = list_entry(skeldev.next, struct device_data, devlist);
+               video_unregister_device(&dev->vdev);
+               kfree(dev);
+               list_del(&dev->devlist);
+       }
+       dprintk("unloaded\n");
+}
+
+module_init(skeleton_init);
+module_exit(skeleton_fini);
+
+/*
+ * Local variables:
+ * c-basic-offset: 8
+ * End:
+ */
--- linux-2.4.18-pre8/drivers/media/video/videodev.c    Tue Feb  5 11:21:16 2002
+++ linux/drivers/media/video/videodev.c        Tue Feb  5 11:33:04 2002
@@ -25,15 +25,13 @@
 #include <linux/mm.h>
 #include <linux/string.h>
 #include <linux/errno.h>
-#include <linux/videodev.h>
 #include <linux/init.h>
-
+#include <linux/kmod.h>
 #include <asm/uaccess.h>
 #include <asm/system.h>
 #include <asm/semaphore.h>
 
-#include <linux/kmod.h>
-
+#include <linux/videodev.h>
 
 #define VIDEO_NUM_DEVICES      256 
 
@@ -70,7 +68,7 @@
 static ssize_t video_read(struct file *file,
        char *buf, size_t count, loff_t *ppos)
 {
-       struct video_device *vfl=video_device[MINOR(file->f_dentry->d_inode->i_rdev)];
+       struct video_device *vfl = video_devdata(file);
        if(vfl->read)
                return vfl->read(vfl, buf, count, file->f_flags&O_NONBLOCK);
        else
@@ -86,13 +84,18 @@
 static ssize_t video_write(struct file *file, const char *buf, 
        size_t count, loff_t *ppos)
 {
-       struct video_device *vfl=video_device[MINOR(file->f_dentry->d_inode->i_rdev)];
+       struct video_device *vfl = video_devdata(file);
        if(vfl->write)
                return vfl->write(vfl, buf, count, file->f_flags&O_NONBLOCK);
        else
                return 0;
 }
 
+struct video_device* video_devdata(struct file *file)
+{
+       return video_device[MINOR(file->f_dentry->d_inode->i_rdev)];
+}
+
 /*
  *     Poll to see if we're readable, can probably be used for timing on incoming
  *     frames, etc..
@@ -100,7 +103,7 @@
 
 static unsigned int video_poll(struct file *file, poll_table * wait)
 {
-       struct video_device *vfl=video_device[MINOR(file->f_dentry->d_inode->i_rdev)];
+       struct video_device *vfl = video_devdata(file);
        if(vfl->poll)
                return vfl->poll(vfl, file, wait);
        else
@@ -133,6 +136,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;
@@ -170,7 +189,7 @@
 {
        struct video_device *vfl;
        lock_kernel();
-       vfl=video_device[MINOR(inode->i_rdev)];
+       vfl = video_devdata(file);
        if(vfl->close)
                vfl->close(vfl);
        vfl->busy=0;
@@ -183,7 +202,7 @@
 static int video_ioctl(struct inode *inode, struct file *file,
        unsigned int cmd, unsigned long arg)
 {
-       struct video_device *vfl=video_device[MINOR(inode->i_rdev)];
+       struct video_device *vfl = video_devdata(file);
        int err=vfl->ioctl(vfl, cmd, (void *)arg);
 
        if(err!=-ENOIOCTLCMD)
@@ -203,7 +222,7 @@
 int video_mmap(struct file *file, struct vm_area_struct *vma)
 {
        int ret = -EINVAL;
-       struct video_device *vfl=video_device[MINOR(file->f_dentry->d_inode->i_rdev)];
+       struct video_device *vfl = video_devdata(file);
        if(vfl->mmap) {
                lock_kernel();
                ret = vfl->mmap(vfl, (char *)vma->vm_start, 
@@ -214,6 +233,69 @@
 }
 
 /*
+ * ioctl helper function -- handles userspace copying
+ */
+int
+video_generic_ioctl(struct inode *inode, struct file *file,
+                   unsigned int cmd, unsigned long arg)
+{
+       struct  video_device *vfl = video_devdata(file);
+       char    sbuf[128];
+       void    *mbuf = NULL;
+       void    *parg = NULL;
+       int     err  = -EINVAL;
+       
+       if (vfl->kernel_ioctl == NULL)
+               return -EINVAL;
+
+       /*  Copy arguments into temp kernel buffer  */
+       switch (_IOC_DIR(cmd)) {
+       case _IOC_NONE:
+               parg = (void *)arg;
+               break;
+       case _IOC_READ: /* some v4l ioctls are marked wrong ... */
+       case _IOC_WRITE:
+       case (_IOC_WRITE | _IOC_READ):
+               if (_IOC_SIZE(cmd) <= sizeof(sbuf)) {
+                       parg = sbuf;
+               } else {
+                       /* too big to allocate from stack */
+                       mbuf = kmalloc(_IOC_SIZE(cmd),GFP_KERNEL);
+                       if (NULL == mbuf)
+                               return -ENOMEM;
+                       parg = mbuf;
+               }
+               
+               err = -EFAULT;
+               if (copy_from_user(parg, (void *)arg, _IOC_SIZE(cmd)))
+                       goto out;
+               break;
+       }
+
+       /* call driver */
+       err = vfl->kernel_ioctl(inode, file, cmd, parg);
+       if (err == -ENOIOCTLCMD)
+               err = -EINVAL;
+       if (err < 0)
+               goto out;
+
+       /*  Copy results into user buffer  */
+       switch (_IOC_DIR(cmd))
+       {
+       case _IOC_READ:
+       case (_IOC_WRITE | _IOC_READ):
+               if (copy_to_user((void *)arg, parg, _IOC_SIZE(cmd)))
+                       err = -EFAULT;
+               break;
+       }
+
+out:
+       if (mbuf)
+               kfree(mbuf);
+       return err;
+}
+
+/*
  *     /proc support
  */
 
@@ -554,6 +636,8 @@
 
 EXPORT_SYMBOL(video_register_device);
 EXPORT_SYMBOL(video_unregister_device);
+EXPORT_SYMBOL(video_devdata);
+EXPORT_SYMBOL(video_generic_ioctl);
 
 MODULE_AUTHOR("Alan Cox");
 MODULE_DESCRIPTION("Device registrar for Video4Linux drivers");
--- linux-2.4.18-pre8/Documentation/Configure.help      Tue Feb  5 11:21:49 2002
+++ linux/Documentation/Configure.help  Tue Feb  5 11:33:04 2002
@@ -21269,6 +21269,19 @@
   To use this option, you have to check, that the "/proc file system
   support" (CONFIG_PROC_FS) is enabled too.
 
+Skeleton Video Capture Driver
+CONFIG_VIDEO_SKELETON
+  This code is a skeleton driver that only illustrates how V4L
+  drivers register and communicate with the Video for Linux subsystem.
+  It does nothing.
+
+  This driver is also available as a module called skeleton.o ( = code
+  which can be inserted in and removed from the running kernel 
+  whenever you want). If you want to compile it as a module, say M 
+  here and read Documentation/modules.txt.
+
+  If unsure, say N, otherwise say N anyway.
+
 AIMSlab RadioTrack (aka RadioReveal) support
 CONFIG_RADIO_RTRACK
   Choose Y here if you have one of these FM radio cards, and then fill



_______________________________________________
Video4linux-list mailing list
[EMAIL PROTECTED]
https://listman.redhat.com/mailman/listinfo/video4linux-list

Reply via email to