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
On Saturday 09 February 2002 19:46, Gerd Knorr wrote:
> Hi,
>
> 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.
That is a large improvement.
But you don't include a lock against reentry, which is bad.
> Comments?
Could you make a helper for open like for ioctl ?
And please don't use a pointer to the device descriptor
in the file structure. It makes live for USB devices much harder.
Regards
Oliver
On Saturday 09 February 2002 19:46, Gerd Knorr wrote:
> Hi,
>
> 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.
That is a large improvement.
But you don't include a lock against reentry, which is bad.
> Comments?
Could you make a helper for open like for ioctl ?
And please don't use a pointer to the device descriptor
in the file structure. It makes live for USB devices much harder.
Regards
Oliver
> > 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.
>
> That is a large improvement.
> But you don't include a lock against reentry, which is bad.
I don't want to handle the wrapper function too much. IMHO it is the
job of the driver to do locking if needed. For some read-only ioctls
like VIDIOCGCAP you don't need locking at all.
> > Comments?
>
> Could you make a helper for open like for ioctl ?
video_open does call video_device[minor]->fops->open(), isn't that
enought?
> And please don't use a pointer to the device descriptor
> in the file structure. It makes live for USB devices much harder.
Sorry, I don't understand. What exactly do you mean?
file->private_data? videodev.c doesn't touch it ...
Gerd
--
#define ENOCLUE 125 /* userland programmer induced race condition */
On Saturday 09 February 2002 21:44, Gerd Knorr wrote:
> > > 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.
> >
> > That is a large improvement.
> > But you don't include a lock against reentry, which is bad.
>
> I don't want to handle the wrapper function too much. IMHO it is the
> job of the driver to do locking if needed. For some read-only ioctls
> like VIDIOCGCAP you don't need locking at all.
OK.
> > > Comments?
> >
> > Could you make a helper for open like for ioctl ?
>
> video_open does call video_device[minor]->fops->open(), isn't that
> enought?
I'd prefer seeing exclusive opening handeled in the helper initially.
> > And please don't use a pointer to the device descriptor
> > in the file structure. It makes live for USB devices much harder.
>
> Sorry, I don't understand. What exactly do you mean?
> file->private_data? videodev.c doesn't touch it ...
But the skeleton driver you provide does so.
Regards
Oliver
> 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?
I'm not sure 2.4 should change but for 2.5 this is absolutely bang on
perfect
Gerd Knorr wrote:
>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.
>
Excellent work. I have no complaints, just a few questions:
1. Would it be better to memset the temp buffer in video_generic_ioctl()
rather than in the driver? I've seen so many drivers forget to do this,
and it's a potential (albeit very small) security hole.
2. In skeleton_open(), couldn't the device_data lookup code be replaced
with:
struct video_device *vdev = video_devdata(file);
struct device_data *dev = vdev->priv;
3. In skeleton_initdev(), shouldn't...
dev->vdev = skeleton_template;
...be...
memcpy(&dev->vdev, &skeleton_template, sizeof(skeleton_template);
4. Is it safe to keep even 128 bytes on the stack in
video_generic_ioctl()? Consider that devices might spend a relatively
long time blocking on VIDIOCSYNC. With 32 devices in use at once, you'd
be coming dangerously close to a stack overflow. IMHO it would be better
to only allocate as much as MCAPTURE and SYNC need, and fall back on
kmalloc for the less time-critical ones (if necessary).
Other than that, I extremely happy with what you've done!
--
Mark McClelland
[email protected]
> > > Could you make a helper for open like for ioctl ?
> >
> > video_open does call video_device[minor]->fops->open(), isn't that
> > enought?
>
> I'd prefer seeing exclusive opening handeled in the helper initially.
Ok, I see what you mean. That was another issue I was thinking about.
Current videodev.c allows one open at a time only, the new code removes
that restriction (intentionally).
Should videodev.c provide a way to ask for exclusive opens to maintain
backward compatibility, using some flag in struct video_device maybe?
Is there some sane way to do this without having to hook into
fops->release? Checking inode->i_count maybe?
> > Sorry, I don't understand. What exactly do you mean?
> > file->private_data? videodev.c doesn't touch it ...
>
> But the skeleton driver you provide does so.
Thats just some sample code, a dummy driver which does nothing but print
some messages to the log. If you want allow multiple applications
access the driver at the same you need some way to disturgish the file
handles, and skeleton.c demonstrates how this can be done using
file->private_data.
usb drivers are free to do with file->private_data whatever they want
(like skeleton.c does), videodev.c doesn't use it.
Gerd
--
#define ENOCLUE 125 /* userland programmer induced race condition */
> Excellent work. I have no complaints, just a few questions:
>
> 1. Would it be better to memset the temp buffer in video_generic_ioctl()
> rather than in the driver? I've seen so many drivers forget to do this,
> and it's a potential (albeit very small) security hole.
The wrapper fills the buffer using copy_from_user() -- even for _IOR
ioctls because some are labeled wrong -- and the driver needs that data.
I can't zero the buffer ...
> 2. In skeleton_open(), couldn't the device_data lookup code be replaced
> with:
>
> struct video_device *vdev = video_devdata(file);
> struct device_data *dev = vdev->priv;
Good point. Yes, that should work.
> 3. In skeleton_initdev(), shouldn't...
>
> dev->vdev = skeleton_template;
>
> ...be...
>
> memcpy(&dev->vdev, &skeleton_template, sizeof(skeleton_template);
No. It does the same.
> 4. Is it safe to keep even 128 bytes on the stack in
> video_generic_ioctl()? Consider that devices might spend a relatively
> long time blocking on VIDIOCSYNC. With 32 devices in use at once, you'd
> be coming dangerously close to a stack overflow.
I don't see a overflow can easily happen here. There is one kernel
strack _per process_. Calling schedule() will also switch the stack.
> IMHO it would be better
> to only allocate as much as MCAPTURE and SYNC need, and fall back on
> kmalloc for the less time-critical ones (if necessary).
struct v4l2_buffer should fit onto the stack too.
Gerd
--
#define ENOCLUE 125 /* userland programmer induced race condition */
Alan Cox wrote:
> > 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?
>
> I'm not sure 2.4 should change but for 2.5 this is absolutely bang on
> perfect
For 2.5 I want switch over to using file_operations completely and drop
the old stuff. Thus 2.4 should provide both old+new way to handle
stuff: The old one for backward compatibility and the new one to allow
backporting 2.5.x drivers to 2.4 ...
Yesterday I've booted a 2.5.x kernel the first time and managed to make
bttv work there. 2.5.x version of the will follow ...
Gerd
--
#define ENOCLUE 125 /* userland programmer induced race condition */
OK, agreed on all points. Thanks for the clarification.
BTW, is there any chance for vmalloc() and pals to be moved to
videodev.c, or something higher-up? I realize that vmalloc() can be used
in instead in most cases, but at the expense of a more complex and
potentially slower mmap().
--
Mark McClelland
[email protected]
On Sun, Feb 10, 2002 at 04:54:48AM -0800, Mark McClelland wrote:
> OK, agreed on all points. Thanks for the clarification.
>
> BTW, is there any chance for vmalloc() and pals to be moved to
> videodev.c, or something higher-up?
What do you mean exactly? bttv's memory management code, which has
been copied to various places, and which is now broken in 2.5.x due
to virt_to_bus() being gone finally?
Some of this is work-in-progress. I'm talking to Dave to put some
helper functions to handle DMA to vmalloced memory blocks to some
sensible place within the kernel. If someone wants to have a look
(not final yet): http://bytesex.org/patches/15_pci-2.4.18-pre8.diff
Gerd
--
#define ENOCLUE 125 /* userland programmer induced race condition */
Gerd Knorr wrote:
>On Sun, Feb 10, 2002 at 04:54:48AM -0800, Mark McClelland wrote:
>
>>BTW, is there any chance for vmalloc() and pals to be moved to
>>videodev.c, or something higher-up?
>>
>
>What do you mean exactly? bttv's memory management code, which has
>been copied to various places, and which is now broken in 2.5.x due
>to virt_to_bus() being gone finally?
>
Sorry, I meant to type rvmalloc().
Many drivers (eg. USB webcam drivers), don't need virt_to_bus(). They
only need a way to allocate reserved pages that they can safely do
remap_page_range() on, for mmap().
>Some of this is work-in-progress. I'm talking to Dave to put some
>helper functions to handle DMA to vmalloced memory blocks to some
>sensible place within the kernel. If someone wants to have a look
>(not final yet): http://bytesex.org/patches/15_pci-2.4.18-pre8.diff
>
Thanks, that's exactly what I was looking for. pci_vmalloc_to_page()
should satisfy all of the USB drivers, if they override
vma->vm_ops->nopage().
--
Mark McClelland
[email protected]
Hi,
Ok, next batch of patches. Below are the videodev.c patches for both
2.5.x and 2.4.x. The 2.5.x patches drop the old function pointers from
struct video_device and switch over to the new, struct file_operations
based setup completely (which breaks almost all existing v4l drivers).
The 2.4.x version of the patch supports both old (for backward
compatibility) and new (for 2.5.x compatibility) way to register v4l
drivers.
Related stuff:
http://bytesex.org/patches/11_v4l_skel-2.4.18-pre9.diff
The skeleton driver, if you want some sample code how to use
the new-style v4l registration. Last patch version had this
included, I've separated it now.
http://bytesex.org/bttv/
bttv 0.8.x uses the new-style v4l registration too.
http://bytesex.org/patches/2.5/
Patches for 2.5.x, fix drivers to use the new-style v4l
registration. Right now there are a few patches for some of the
radio drivers, more patches will follow next days. Testers
are very welcome ...
Gerd
========== [ 2.5.x patch ] ==================================
--- linux-2.5.4/include/linux/videodev.h Mon Feb 11 12:49:31 2002
+++ linux/include/linux/videodev.h Mon Feb 11 19:29:43 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>
@@ -13,21 +25,20 @@
struct video_device
{
struct module *owner;
- char name[32];
- int type;
+ char name[32];
+ int type; /* v4l1 */
+ int type2; /* v4l2 */
int hardware;
- 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 vm_area_struct *vma, struct video_device *, const char *, unsigned long);
- 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;
@@ -43,8 +54,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 */
@@ -150,6 +164,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
@@ -379,4 +394,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.5.4/drivers/media/video/videodev.c Mon Feb 11 11:57:11 2002
+++ linux/drivers/media/video/videodev.c Mon Feb 11 15:09:50 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
@@ -62,61 +60,20 @@
#endif /* CONFIG_PROC_FS && CONFIG_VIDEO_PROC_FS */
-
-/*
- * Read will do some smarts later on. Buffer pin etc.
- */
-
-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)];
- if(vfl->read)
- return vfl->read(vfl, buf, count, file->f_flags&O_NONBLOCK);
- else
- return -EINVAL;
-}
-
-
-/*
- * Write for now does nothing. No reason it shouldnt do overlay setting
- * for some boards I guess..
- */
-
-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)];
- if(vfl->write)
- return vfl->write(vfl, buf, count, file->f_flags&O_NONBLOCK);
- else
- return 0;
-}
-
-/*
- * Poll to see if we're readable, can probably be used for timing on incoming
- * frames, etc..
- */
-
-static unsigned int video_poll(struct file *file, poll_table * wait)
+struct video_device* video_devdata(struct file *file)
{
- struct video_device *vfl=video_device[minor(file->f_dentry->d_inode->i_rdev)];
- if(vfl->poll)
- return vfl->poll(vfl, file, wait);
- else
- return 0;
+ return video_device[minor(file->f_dentry->d_inode->i_rdev)];
}
-
/*
* Open a video device.
*/
-
static int video_open(struct inode *inode, struct file *file)
{
unsigned int minor = minor(inode->i_rdev);
- int err, retval = 0;
+ int err = 0;
struct video_device *vfl;
+ struct file_operations *old_fops;
if(minor>=VIDEO_NUM_DEVICES)
return -ENODEV;
@@ -129,88 +86,89 @@
request_module(modname);
vfl=video_device[minor];
if (vfl==NULL) {
- retval = -ENODEV;
+ err = -ENODEV;
goto error_out;
}
}
- if(vfl->busy) {
- retval = -EBUSY;
- goto error_out;
+ 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);
}
- vfl->busy=1; /* In case vfl->open sleeps */
+ fops_put(old_fops);
+ return err;
- if(vfl->owner)
- __MOD_INC_USE_COUNT(vfl->owner);
-
- if(vfl->open)
- {
- err=vfl->open(vfl,0); /* Tell the device it is open */
- if(err)
- {
- vfl->busy=0;
- if(vfl->owner)
- __MOD_DEC_USE_COUNT(vfl->owner);
-
- unlock_kernel();
- return err;
- }
- }
- unlock_kernel();
- return 0;
error_out:
unlock_kernel();
- return retval;
+ return err;
}
/*
- * Last close of a video for Linux device
+ * 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;
-static int video_release(struct inode *inode, struct file *file)
-{
- struct video_device *vfl;
- lock_kernel();
- vfl=video_device[minor(inode->i_rdev)];
- if(vfl->close)
- vfl->close(vfl);
- vfl->busy=0;
- if(vfl->owner)
- __MOD_DEC_USE_COUNT(vfl->owner);
- unlock_kernel();
- return 0;
-}
+ if (vfl->kernel_ioctl == NULL)
+ return -EINVAL;
-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)];
- int err=vfl->ioctl(vfl, cmd, (void *)arg);
+ /* 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;
+ }
- if(err!=-ENOIOCTLCMD)
- return err;
-
- switch(cmd)
+ /* 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))
{
- default:
- return -EINVAL;
+ case _IOC_READ:
+ case (_IOC_WRITE | _IOC_READ):
+ if (copy_to_user((void *)arg, parg, _IOC_SIZE(cmd)))
+ err = -EFAULT;
+ break;
}
-}
-/*
- * We need to do MMAP support
- */
-
-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)];
- if(vfl->mmap) {
- lock_kernel();
- ret = vfl->mmap(vma, vfl, (char *)vma->vm_start,
- (unsigned long)(vma->vm_end-vma->vm_start));
- unlock_kernel();
- }
- return ret;
+out:
+ if (mbuf)
+ kfree(mbuf);
+ return err;
}
/*
@@ -398,7 +356,6 @@
{
int i=0;
int base;
- int err;
int end;
char *name_base;
char name[16];
@@ -452,17 +409,7 @@
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
@@ -509,13 +456,7 @@
{
owner: THIS_MODULE,
llseek: no_llseek,
- read: video_read,
- write: video_write,
- ioctl: video_ioctl,
- mmap: video_mmap,
open: video_open,
- release: video_release,
- poll: video_poll,
};
/*
@@ -540,12 +481,9 @@
static void __exit videodev_exit(void)
{
-#ifdef MODULE
#if defined(CONFIG_PROC_FS) && defined(CONFIG_VIDEO_PROC_FS)
videodev_proc_destroy ();
#endif
-#endif
-
devfs_unregister_chrdev(VIDEO_MAJOR, "video_capture");
}
@@ -554,6 +492,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");
========== [ 2.4.x patch ] ==================================
--- linux-2.4.18-pre9/include/linux/videodev.h Mon Feb 11 13:20:44 2002
+++ linux/include/linux/videodev.h Mon Feb 11 15:11:05 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 -- dropped in 2.5.x, don't use it */
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-pre9/drivers/media/video/videodev.c Mon Feb 11 13:20:08 2002
+++ linux/drivers/media/video/videodev.c Mon Feb 11 15:06:35 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
*/
@@ -540,12 +622,9 @@
static void __exit videodev_exit(void)
{
-#ifdef MODULE
#if defined(CONFIG_PROC_FS) && defined(CONFIG_VIDEO_PROC_FS)
videodev_proc_destroy ();
#endif
-#endif
-
devfs_unregister_chrdev(VIDEO_MAJOR, "video_capture");
}
@@ -554,6 +633,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");