Re: [RFC/PATCH v3 01/10] media: Media device node support

2010-08-02 Thread Laurent Pinchart
Hi Hans,

On Sunday 01 August 2010 13:14:18 Hans Verkuil wrote:
 On Thursday 29 July 2010 18:06:34 Laurent Pinchart wrote:
  The media_devnode structure provides support for registering and
  unregistering character devices using a dynamic major number. Reference
  counting is handled internally, making device drivers easier to write
  without having to solve the open/disconnect race condition issue over
  and over again.
  
  The code is based on video/v4l2-dev.c.
  
  Signed-off-by: Laurent Pinchart laurent.pinch...@ideasonboard.com
  ---
  
   drivers/media/Makefile|8 +-
   drivers/media/media-devnode.c |  326
   + include/media/media-devnode.h
   |   84 +++
   3 files changed, 416 insertions(+), 2 deletions(-)
   create mode 100644 drivers/media/media-devnode.c
   create mode 100644 include/media/media-devnode.h
 
 snip
 
  diff --git a/drivers/media/media-devnode.c
  b/drivers/media/media-devnode.c new file mode 100644
  index 000..6f5558c
  --- /dev/null
  +++ b/drivers/media/media-devnode.c
 
 snip
 
  +/* Override for the open function */
  +static int media_open(struct inode *inode, struct file *filp)
  +{
  +   struct media_devnode *mdev;
  +   int ret;
  +
  +   /* Check if the media device is available. This needs to be done with
  +* the media_devnode_lock held to prevent an open/unregister race:
  +* without the lock, the device could be unregistered and freed between
  +* the media_devnode_is_registered() and get_device() calls, leading to
  +* a crash.
  +*/
  +   mutex_lock(media_devnode_lock);
  +   mdev = container_of(inode-i_cdev, struct media_devnode, cdev);
  +   /* return ENODEV if the media device has been removed
  +  already or if it is not registered anymore. */
  +   if (!media_devnode_is_registered(mdev)) {
  +   mutex_unlock(media_devnode_lock);
  +   return -ENODEV;
  +   }
  +   /* and increase the device refcount */
  +   get_device(mdev-dev);
  +   mutex_unlock(media_devnode_lock);
  +   if (mdev-fops-open) {
  +   ret = mdev-fops-open(filp);
  +   if (ret) {
  +   put_device(mdev-dev);
  +   return ret;
  +   }
  +   }
  +
  +   filp-private_data = mdev;
 
 This line should be moved up to before the if: that way the open op can
 rely on private_data being setup correctly.

Agreed, I'll fix that.

  +   return 0;
  +}

-- 
Regards,

Laurent Pinchart
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[RFC/PATCH v3 01/10] media: Media device node support

2010-07-29 Thread Laurent Pinchart
The media_devnode structure provides support for registering and
unregistering character devices using a dynamic major number. Reference
counting is handled internally, making device drivers easier to write
without having to solve the open/disconnect race condition issue over
and over again.

The code is based on video/v4l2-dev.c.

Signed-off-by: Laurent Pinchart laurent.pinch...@ideasonboard.com
---
 drivers/media/Makefile|8 +-
 drivers/media/media-devnode.c |  326 +
 include/media/media-devnode.h |   84 +++
 3 files changed, 416 insertions(+), 2 deletions(-)
 create mode 100644 drivers/media/media-devnode.c
 create mode 100644 include/media/media-devnode.h

diff --git a/drivers/media/Makefile b/drivers/media/Makefile
index 499b081..c1b5938 100644
--- a/drivers/media/Makefile
+++ b/drivers/media/Makefile
@@ -2,7 +2,11 @@
 # Makefile for the kernel multimedia device drivers.
 #
 
+media-objs := media-devnode.o
+
+obj-$(CONFIG_MEDIA_SUPPORT)+= media.o
+
 obj-y += common/ IR/ video/
 
-obj-$(CONFIG_VIDEO_DEV) += radio/
-obj-$(CONFIG_DVB_CORE)  += dvb/
+obj-$(CONFIG_VIDEO_DEV)+= radio/
+obj-$(CONFIG_DVB_CORE) += dvb/
diff --git a/drivers/media/media-devnode.c b/drivers/media/media-devnode.c
new file mode 100644
index 000..6f5558c
--- /dev/null
+++ b/drivers/media/media-devnode.c
@@ -0,0 +1,326 @@
+/*
+ * Media device node
+ *
+ * Generic media device node infrastructure to register and unregister
+ * character devices using a dynamic major number and proper reference
+ * counting.
+ *
+ * Copyright 2010 Laurent Pinchart laurent.pinch...@ideasonboard.com
+ *
+ * Based on drivers/media/video/v4l2_dev.c code authored by
+ *
+ * Mauro Carvalho Chehab mche...@infradead.org (version 2)
+ * Alan Cox, a...@lxorguk.ukuu.org.uk (version 1)
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version
+ * 2 of the License, or (at your option) any later version.
+ *
+ */
+
+#include linux/errno.h
+#include linux/init.h
+#include linux/module.h
+#include linux/kernel.h
+#include linux/kmod.h
+#include linux/slab.h
+#include linux/mm.h
+#include linux/smp_lock.h
+#include linux/string.h
+#include linux/types.h
+#include linux/uaccess.h
+#include asm/system.h
+
+#include media/media-devnode.h
+
+#define MEDIA_NUM_DEVICES  256
+#define MEDIA_NAME media
+
+static dev_t media_dev_t;
+
+/*
+ * sysfs stuff
+ */
+
+static ssize_t show_name(struct device *cd,
+struct device_attribute *attr, char *buf)
+{
+   struct media_devnode *mdev = to_media_devnode(cd);
+
+   return sprintf(buf, %.*s\n, (int)sizeof(mdev-name), mdev-name);
+}
+
+static struct device_attribute media_devnode_attrs[] = {
+   __ATTR(name, S_IRUGO, show_name, NULL),
+   __ATTR_NULL
+};
+
+/*
+ * Active devices
+ */
+static DEFINE_MUTEX(media_devnode_lock);
+static DECLARE_BITMAP(media_devnode_nums, MEDIA_NUM_DEVICES);
+
+/* Called when the last user of the media device exits. */
+static void media_devnode_release(struct device *cd)
+{
+   struct media_devnode *mdev = to_media_devnode(cd);
+
+   mutex_lock(media_devnode_lock);
+
+   /* Delete the cdev on this minor as well */
+   cdev_del(mdev-cdev);
+
+   /* Mark device node number as free */
+   clear_bit(mdev-minor, media_devnode_nums);
+
+   mutex_unlock(media_devnode_lock);
+
+   /* Release media_devnode and perform other cleanups as needed. */
+   if (mdev-release)
+   mdev-release(mdev);
+}
+
+static struct class media_class = {
+   .name = MEDIA_NAME,
+   .dev_attrs = media_devnode_attrs,
+};
+
+static ssize_t media_read(struct file *filp, char __user *buf,
+   size_t sz, loff_t *off)
+{
+   struct media_devnode *mdev = media_devnode_data(filp);
+
+   if (!mdev-fops-read)
+   return -EINVAL;
+   if (!media_devnode_is_registered(mdev))
+   return -EIO;
+   return mdev-fops-read(filp, buf, sz, off);
+}
+
+static ssize_t media_write(struct file *filp, const char __user *buf,
+   size_t sz, loff_t *off)
+{
+   struct media_devnode *mdev = media_devnode_data(filp);
+
+   if (!mdev-fops-write)
+   return -EINVAL;
+   if (!media_devnode_is_registered(mdev))
+   return -EIO;
+   return mdev-fops-write(filp, buf, sz, off);
+}
+
+static unsigned int media_poll(struct file *filp,
+  struct poll_table_struct *poll)
+{
+   struct media_devnode *mdev = media_devnode_data(filp);
+
+   if (!mdev-fops-poll || !media_devnode_is_registered(mdev))
+   return DEFAULT_POLLMASK;
+   return mdev-fops-poll(filp, poll);
+}
+
+static long media_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
+{
+   struct