v4l: sysfs'ify videodev
PATCH 1/1 v4l: sysfs'ify videodev
Hi,
This patch drops procfs code from videodev.c and adds sysfs support instead. It adds a new class where all v4l devices are listed. It also provides some new helper functions for v4l drivers.
Gerd
==============================[ cut here ]==============================
drivers/media/video/videodev.c +78 -191
@@ -15,7 +15,6 @@* - Added procfs support */ -#include <linux/config.h> #include <linux/version.h> #include <linux/module.h> #include <linux/types.h>
@@ -28,6 +27,7 @@#include <linux/init.h> #include <linux/kmod.h> #include <linux/slab.h> +#include <linux/types.h> #include <linux/devfs_fs_kernel.h> #include <asm/uaccess.h> #include <asm/system.h>
@@ -35,33 +35,67 @@#include <linux/videodev.h> -#define VIDEO_NUM_DEVICES 256 +#define VIDEO_NUM_DEVICES 256 +#define VIDEO_NAME "video4linux"/* -* Active devices +* sysfs stuff */ - -static struct video_device *video_device[VIDEO_NUM_DEVICES ]; -static DECLARE_MUTEX (videodev_lock); +static ssize_t show_name (struct class_device *cd,char *buf) +{ + struct video_device *vfd = container_of(cd, struct video_device, class_dev); +return sprintf (buf,"%.*s\n" ,(int )sizeof (vfd->name),vfd->name); +} + +static ssize_t show_dev (struct class_device *cd,char *buf) +{ + struct video_device *vfd = container_of(cd, struct video_device, class_dev); +dev_t dev =MKDEV (VIDEO_MAJOR , vfd->minor); +return sprintf (buf,"%04x\n" ,(int )dev); +} -#ifdef CONFIG_VIDEO_PROC_FS +static CLASS_DEVICE_ATTR(name, S_IRUGO, show_name, NULL); +static CLASS_DEVICE_ATTR(dev, S_IRUGO, show_dev, NULL); -#include <linux/proc_fs.h> +struct video_device *video_device_alloc (void ) +{ +struct video_device *vfd; -struct videodev_proc_data { -struct list_head proc_list; -char name[16 ]; -struct video_device *vdev; -struct proc_dir_entry *proc_entry; -}; + vfd =kmalloc (sizeof (*vfd),GFP_KERNEL ); +if (NULL == vfd) +return NULL ; +memset (vfd,0 ,sizeof (*vfd)); +return vfd; +} + +void video_device_release (struct video_device *vfd) +{ +kfree (vfd); +} + +static void video_release (struct class_device *cd) +{ + struct video_device *vfd = container_of(cd, struct video_device, class_dev); -static struct proc_dir_entry *video_dev_proc_entry =NULL ; -struct proc_dir_entry *video_proc_entry =NULL ; -EXPORT_SYMBOL (video_proc_entry); -LIST_HEAD (videodev_proc_list); +#if 1 /* needed until all drivers are fixed */ +if (!vfd->release) +return ; +#endif + vfd->release (vfd); +} -#endif /* CONFIG_VIDEO_PROC_FS */ +static struct class video_class = { + .name =VIDEO_NAME , + .release = video_release, +}; + +/* +* Active devices +*/ + +static struct video_device *video_device[VIDEO_NUM_DEVICES ]; +static DECLARE_MUTEX (videodev_lock);struct video_device *video_devdata (struct file *file) {
@@ -192,156 +226,6 @@return 0 ; } -/* -* /proc support -*/ - -#ifdef CONFIG_VIDEO_PROC_FS - -/* Hmm... i'd like to see video_capability information here, but -* how can I access it (without changing the other drivers? -claudio -*/ -static int videodev_proc_read (char *page,char **start,off_t off, -int count,int *eof,void *data) -{ -char *out = page; -struct video_device *vfd = data; -struct videodev_proc_data *d; -struct list_head *tmp; -int len; -char c =' ' ; - - list_for_each (tmp, &videodev_proc_list) { - d = list_entry(tmp, struct videodev_proc_data, proc_list); -if (vfd == d->vdev) -break ; - } - -/* Sanity check */ -if (tmp == &videodev_proc_list) - goto skip; - -#define PRINT_VID_TYPE (x) do { if (vfd->type & x) \ - out += sprintf (out, "%c%s", c, #x); c='|';} while (0) - - out +=sprintf (out,"name : %s\n" , vfd->name); - out +=sprintf (out,"type :" ); -PRINT_VID_TYPE (VID_TYPE_CAPTURE ); -PRINT_VID_TYPE (VID_TYPE_TUNER ); -PRINT_VID_TYPE (VID_TYPE_TELETEXT ); -PRINT_VID_TYPE (VID_TYPE_OVERLAY ); -PRINT_VID_TYPE (VID_TYPE_CHROMAKEY ); -PRINT_VID_TYPE (VID_TYPE_CLIPPING ); -PRINT_VID_TYPE (VID_TYPE_FRAMERAM ); -PRINT_VID_TYPE (VID_TYPE_SCALES ); -PRINT_VID_TYPE (VID_TYPE_MONOCHROME ); -PRINT_VID_TYPE (VID_TYPE_SUBCAPTURE ); -PRINT_VID_TYPE (VID_TYPE_MPEG_DECODER ); -PRINT_VID_TYPE (VID_TYPE_MPEG_ENCODER ); -PRINT_VID_TYPE (VID_TYPE_MJPEG_DECODER ); -PRINT_VID_TYPE (VID_TYPE_MJPEG_ENCODER ); - out +=sprintf (out,"\n" ); - out +=sprintf (out,"hardware : 0x%x\n" , vfd->hardware); -#if 0 - out +=sprintf (out,"channels : %d\n" , d->vcap.channels); - out +=sprintf (out,"audios : %d\n" , d->vcap.audios); - out +=sprintf (out,"maxwidth : %d\n" , d->vcap.maxwidth); - out +=sprintf (out,"maxheight : %d\n" , d->vcap.maxheight); - out +=sprintf (out,"minwidth : %d\n" , d->vcap.minwidth); - out +=sprintf (out,"minheight : %d\n" , d->vcap.minheight); -#endif - -skip: - len = out - page; - len -= off; -if (len < count) { - *eof =1 ; -if (len <=0 ) -return 0 ; - }else - len = count; - - *start = page + off; - -return len; -} - -static void videodev_proc_create (void ) -{ - video_proc_entry =create_proc_entry ("video" ,S_IFDIR , &proc_root); - -if (video_proc_entry ==NULL ) { -printk ("video_dev: unable to initialise /proc/video\n" ); -return ; - } - - video_proc_entry->owner =THIS_MODULE ; - video_dev_proc_entry =create_proc_entry ("dev" ,S_IFDIR , video_proc_entry); - -if (video_dev_proc_entry ==NULL ) { -printk ("video_dev: unable to initialise /proc/video/dev\n" ); -return ; - } - - video_dev_proc_entry->owner =THIS_MODULE ; -} - -static void __exit videodev_proc_destroy(void) -{ -if (video_dev_proc_entry !=NULL ) -remove_proc_entry ("dev" , video_proc_entry); - -if (video_proc_entry !=NULL ) -remove_proc_entry ("video" , &proc_root); -} - -static void videodev_proc_create_dev (struct video_device *vfd,char *name) -{ -struct videodev_proc_data *d; -struct proc_dir_entry *p; - -if (video_dev_proc_entry ==NULL ) -return ; - - d =kmalloc (sizeof (struct videodev_proc_data ),GFP_KERNEL ); -if (!d) -return ; - - p =create_proc_entry (name,S_IFREG |S_IRUGO |S_IWUSR , video_dev_proc_entry); -if (!p) { -kfree (d); -return ; - } - p->data = vfd; - p->read_proc = videodev_proc_read; - - d->proc_entry = p; - d->vdev = vfd; -strcpy (d->name, name); - -/* How can I get capability information ? */ - -list_add (&d->proc_list, &videodev_proc_list); -} - -static void videodev_proc_destroy_dev (struct video_device *vfd) -{ -struct list_head *tmp; -struct videodev_proc_data *d; - - list_for_each (tmp, &videodev_proc_list) { - d = list_entry(tmp, struct videodev_proc_data, proc_list); -if (vfd == d->vdev) { -remove_proc_entry (d->name, video_dev_proc_entry); -list_del (&d->proc_list); -kfree (d); -break ; - } - } -} - -#endif /* CONFIG_VIDEO_PROC_FS */ -extern struct file_operations video_fops;/**
@@ -429,15 +313,25 @@devfs_mk_cdev (MKDEV (VIDEO_MAJOR , vfd->minor),S_IFCHR |S_IRUSR |S_IWUSR , vfd->devfs_name);init_MUTEX (&vfd->lock); - -#ifdef CONFIG_VIDEO_PROC_FS -{ -char name[16 ]; -sprintf (name,"%s%d" , name_base, i - base); -videodev_proc_create_dev (vfd, name); -} -#endif +/* sysfs class */ +memset (&vfd->class_dev,0x00 ,sizeof (vfd->class_dev)); +if (vfd->dev) + vfd->class_dev.dev = vfd->dev; + vfd->class_dev.class = &video_class; +strlcpy (vfd->class_dev.class_id, vfd->devfs_name +4 ,BUS_ID_SIZE ); +class_device_register (&vfd->class_dev); +class_device_create_file (&vfd->class_dev, + &class_device_attr_name); +class_device_create_file (&vfd->class_dev, + &class_device_attr_dev); + +#if 1 /* needed until all drivers are fixed */ +if (!vfd->release) +printk (KERN_WARNING "videodev: \"%s\" has no release callback. " +"Please fix your driver for proper sysfs support, see " +"http://lwn.net/Articles/36850/\n" , vfd->name); +#endif return 0 ; }
@@ -455,10 +349,7 @@if (video_device[vfd->minor]!=vfd)panic ("videodev: bad unregister" ); -#ifdef CONFIG_VIDEO_PROC_FS -videodev_proc_destroy_dev (vfd); -#endif - +class_device_unregister (&vfd->class_dev);devfs_remove (vfd->devfs_name); video_device[vfd->minor]=NULL ;up (&videodev_lock);
@@ -479,24 +370,18 @@static int __init videodev_init (void ) {printk (KERN_INFO "Linux video capture interface: v1.00\n" ); -if (register_chrdev (VIDEO_MAJOR ,"video_capture" , &video_fops)) { +if (register_chrdev (VIDEO_MAJOR ,VIDEO_NAME , &video_fops)) {printk ("video_dev: unable to get major %d\n" ,VIDEO_MAJOR );return -EIO ; } - -#ifdef CONFIG_VIDEO_PROC_FS -videodev_proc_create (); -#endif - +class_register (&video_class);return 0 ; } static void __exit videodev_exit(void) { -#ifdef CONFIG_VIDEO_PROC_FS -videodev_proc_destroy (); -#endif -unregister_chrdev (VIDEO_MAJOR ,"video_capture" ); +class_unregister (&video_class); +unregister_chrdev (VIDEO_MAJOR ,VIDEO_NAME ); }module_init (videodev_init)
@@ -508,6 +393,8 @@EXPORT_SYMBOL (video_usercopy);EXPORT_SYMBOL (video_exclusive_open);EXPORT_SYMBOL (video_exclusive_release); +EXPORT_SYMBOL (video_device_alloc); +EXPORT_SYMBOL (video_device_release);MODULE_AUTHOR ("Alan Cox" );MODULE_DESCRIPTION ("Device registrar for Video4Linux drivers" );
include/linux/videodev.h +53 -25
106 lines not on this page · raw message
Signature · 1 line
sigfault
Quoted 4 lines · Gerd Knorr
drivers/media/video/videodev.c +3
-0+static void video_release (struct class_device *cd) +{ + struct video_device *vfd = container_of(cd, struct video_device, class_dev);
drivers/media/video/videodev.c line 86
Quoted 10 lines
drivers/media/video/videodev.c +6
-4-static struct proc_dir_entry *video_dev_proc_entry =NULL ; -struct proc_dir_entry *video_proc_entry =NULL ; -EXPORT_SYMBOL (video_proc_entry); -LIST_HEAD (videodev_proc_list); +#if 1 /* needed until all drivers are fixed */ +if (!vfd->release) +return ; +#endif + vfd->release (vfd); +}
Ok, so you're allowing the release to happen elsewhere. How are you ensuring that the code which vfd->release points to hasn't been unloaded before the video device has been released, or, even, how are you preventing the module containing the above code being removed before all video devices have been released?
Signature · 2 lines
Russell King ([email protected]) The developer of ARM Linux http://www.arm.linux.org.uk/personal/aboutme.html
Quoted 5 lines · Russell King
Quoted 4 lines · Gerd Knorr
drivers/media/video/videodev.c +3
-0+static void video_release (struct class_device *cd) +{ + struct video_device *vfd = container_of(cd, struct video_device, class_dev);
Quoted 14 lines
drivers/media/video/videodev.c line 86
Quoted 10 lines
drivers/media/video/videodev.c +6
-4-static struct proc_dir_entry *video_dev_proc_entry =NULL ; -struct proc_dir_entry *video_proc_entry =NULL ; -EXPORT_SYMBOL (video_proc_entry); -LIST_HEAD (videodev_proc_list); +#if 1 /* needed until all drivers are fixed */ +if (!vfd->release) +return ; +#endif + vfd->release (vfd); +}Ok, so you're allowing the release to happen elsewhere. How are you ensuring that the code which
vfd->releasepoints to hasn't been unloaded before the video device has been released,
Which exact corner case you are refering to? video_release() being called _after_ video_unregister_device() returns due to a sysfs file being busy? Doesn't sysfs refcounting take care of that? If not module_{get|put}(vfd->fops->owner) at sensible places should fix that. get in video_unregister_device(), put in video_release()?
Quoted 3 lines
or, even, how
are you preventing the module containing the above code being removed before all video devices have been released?
videodev.ko simply can't be unloaded while v4l drivers are loaded due to symbol references.
Gerd
Signature · 1 line
sigfault