V4L/DVB (5767): ZC0301 driver updates

- Make the driver depend on V4L2 only (KConfig)
- Better and safe locking mechanism of the device structure on open(), close()
  and disconnect()
- Use kref for handling device deallocation
- Generic cleanups

Signed-off-by: Luca Risolia <luca.risolia@studio.unibo.it>
Signed-off-by: Mauro Carvalho Chehab <mchehab@infradead.org>

drivers/media/video/zc0301/Kconfig

@@ -1,6 +1,6 @@
 config USB_ZC0301
 	tristate "USB ZC0301[P] Image Processor and Control Chip support"
-	depends on VIDEO_V4L1
+	depends on VIDEO_V4L2
 	---help---
 	  Say Y here if you want support for cameras based on the ZC0301 or
 	  ZC0301P Image Processors and Control Chips.

drivers/media/video/zc0301/zc0301.h

@@ -36,6 +36,7 @@
 #include <linux/rwsem.h>
 #include <linux/stddef.h>
 #include <linux/string.h>
+#include <linux/kref.h>
 
 #include "zc0301_sensor.h"
 
@@ -98,7 +99,7 @@ struct zc0301_module_param {
 	u16 frame_timeout;
 };
 
-static DECLARE_RWSEM(zc0301_disconnect);
+static DECLARE_RWSEM(zc0301_dev_lock);
 
 struct zc0301_device {
 	struct video_device* v4ldev;
@@ -121,12 +122,14 @@ struct zc0301_device {
 
 	struct zc0301_module_param module_param;
 
+	struct kref kref;
 	enum zc0301_dev_state state;
 	u8 users;
 
-	struct mutex dev_mutex, fileop_mutex;
+	struct completion probe;
+	struct mutex open_mutex, fileop_mutex;
 	spinlock_t queue_lock;
-	wait_queue_head_t open, wait_frame, wait_stream;
+	wait_queue_head_t wait_open, wait_frame, wait_stream;
 };
 
 /*****************************************************************************/
@@ -156,8 +159,8 @@ do {                                                                          \
 		else if ((level) == 2)                                        \
 			dev_info(&cam->usbdev->dev, fmt "\n", ## args);       \
 		else if ((level) >= 3)                                        \
-			dev_info(&cam->usbdev->dev, "[%s:%d] " fmt "\n",      \
+			dev_info(&cam->usbdev->dev, "[%s:%s:%d] " fmt "\n",   \
-				 __FUNCTION__, __LINE__ , ## args);           \
+				 __FILE__, __FUNCTION__, __LINE__ , ## args); \
 	}                                                                     \
 } while (0)
 #	define KDBG(level, fmt, args...)                                      \
@@ -166,8 +169,8 @@ do {                                                                          \
 		if ((level) == 1 || (level) == 2)                             \
 			pr_info("zc0301: " fmt "\n", ## args);                \
 		else if ((level) == 3)                                        \
-			pr_debug("zc0301: [%s:%d] " fmt "\n", __FUNCTION__,   \
+			pr_debug("sn9c102: [%s:%s:%d] " fmt "\n", __FILE__,   \
-				 __LINE__ , ## args);                         \
+				 __FUNCTION__, __LINE__ , ## args);           \
 	}                                                                     \
 } while (0)
 #	define V4LDBG(level, name, cmd)                                       \
@@ -183,8 +186,8 @@ do {                                                                          \
 
 #undef PDBG
 #define PDBG(fmt, args...)                                                    \
-dev_info(&cam->usbdev->dev, "[%s:%d] " fmt "\n",                              \
+dev_info(&cam->usbdev->dev, "[%s:%s:%d] " fmt "\n", __FILE__, __FUNCTION__,   \
-	 __FUNCTION__, __LINE__ , ## args)
+	 __LINE__ , ## args)
 
 #undef PDBGG
 #define PDBGG(fmt, args...) do {;} while(0) /* placeholder */

drivers/media/video/zc0301/zc0301_core.c

@@ -49,11 +49,11 @@
 
 #define ZC0301_MODULE_NAME    "V4L2 driver for ZC0301[P] "                    \
 			      "Image Processor and Control Chip"
-#define ZC0301_MODULE_AUTHOR  "(C) 2006 Luca Risolia"
+#define ZC0301_MODULE_AUTHOR  "(C) 2006-2007 Luca Risolia"
 #define ZC0301_AUTHOR_EMAIL   "<luca.risolia@studio.unibo.it>"
 #define ZC0301_MODULE_LICENSE "GPL"
-#define ZC0301_MODULE_VERSION "1:1.07"
+#define ZC0301_MODULE_VERSION "1:1.10"
-#define ZC0301_MODULE_VERSION_CODE  KERNEL_VERSION(1, 1, 7)
+#define ZC0301_MODULE_VERSION_CODE  KERNEL_VERSION(1, 1, 10)
 
 /*****************************************************************************/
 
@@ -573,7 +573,8 @@ static int zc0301_init(struct zc0301_device* cam)
 	int err = 0;
 
 	if (!(cam->state & DEV_INITIALIZED)) {
-		init_waitqueue_head(&cam->open);
+		mutex_init(&cam->open_mutex);
+		init_waitqueue_head(&cam->wait_open);
 		qctrl = s->qctrl;
 		rect = &(s->cropcap.defrect);
 		cam->compression.quality = ZC0301_COMPRESSION_QUALITY;
@@ -634,59 +635,73 @@ static int zc0301_init(struct zc0301_device* cam)
 	return 0;
 }
 
+/*****************************************************************************/
 
-static void zc0301_release_resources(struct zc0301_device* cam)
+static void zc0301_release_resources(struct kref *kref)
 {
+	struct zc0301_device *cam = container_of(kref, struct zc0301_device,
+						 kref);
 	DBG(2, "V4L2 device /dev/video%d deregistered", cam->v4ldev->minor);
 	video_set_drvdata(cam->v4ldev, NULL);
 	video_unregister_device(cam->v4ldev);
+	usb_put_dev(cam->usbdev);
 	kfree(cam->control_buffer);
+	kfree(cam);
 }
 
-/*****************************************************************************/
 
 static int zc0301_open(struct inode* inode, struct file* filp)
 {
 	struct zc0301_device* cam;
 	int err = 0;
 
-	/*
+	if (!down_read_trylock(&zc0301_dev_lock))
-	   This is the only safe way to prevent race conditions with
-	   disconnect
-	*/
-	if (!down_read_trylock(&zc0301_disconnect))
 		return -ERESTARTSYS;
 
 	cam = video_get_drvdata(video_devdata(filp));
 
-	if (mutex_lock_interruptible(&cam->dev_mutex)) {
+	if (wait_for_completion_interruptible(&cam->probe)) {
-		up_read(&zc0301_disconnect);
+		up_read(&zc0301_dev_lock);
 		return -ERESTARTSYS;
 	}
 
+	kref_get(&cam->kref);
+
+	if (mutex_lock_interruptible(&cam->open_mutex)) {
+		kref_put(&cam->kref, zc0301_release_resources);
+		up_read(&zc0301_dev_lock);
+		return -ERESTARTSYS;
+	}
+
+	if (cam->state & DEV_DISCONNECTED) {
+		DBG(1, "Device not present");
+		err = -ENODEV;
+		goto out;
+	}
+
 	if (cam->users) {
 		DBG(2, "Device /dev/video%d is busy...", cam->v4ldev->minor);
+		DBG(3, "Simultaneous opens are not supported");
 		if ((filp->f_flags & O_NONBLOCK) ||
 		    (filp->f_flags & O_NDELAY)) {
 			err = -EWOULDBLOCK;
 			goto out;
 		}
-		mutex_unlock(&cam->dev_mutex);
+		DBG(2, "A blocking open() has been requested. Wait for the "
-		err = wait_event_interruptible_exclusive(cam->open,
+		       "device to be released...");
-						  cam->state & DEV_DISCONNECTED
+		up_read(&zc0301_dev_lock);
+		err = wait_event_interruptible_exclusive(cam->wait_open,
+						(cam->state & DEV_DISCONNECTED)
 							 || !cam->users);
-		if (err) {
+		down_read(&zc0301_dev_lock);
-			up_read(&zc0301_disconnect);
+		if (err)
-			return err;
+			goto out;
-		}
 		if (cam->state & DEV_DISCONNECTED) {
-			up_read(&zc0301_disconnect);
+			err = -ENODEV;
-			return -ENODEV;
+			goto out;
 		}
-		mutex_lock(&cam->dev_mutex);
 	}
 
-
 	if (cam->state & DEV_MISCONFIGURED) {
 		err = zc0301_init(cam);
 		if (err) {
@@ -711,36 +726,32 @@ static int zc0301_open(struct inode* inode, struct file* filp)
 	DBG(3, "Video device /dev/video%d is open", cam->v4ldev->minor);
 
 out:
-	mutex_unlock(&cam->dev_mutex);
+	mutex_unlock(&cam->open_mutex);
-	up_read(&zc0301_disconnect);
+	if (err)
+		kref_put(&cam->kref, zc0301_release_resources);
+	up_read(&zc0301_dev_lock);
 	return err;
 }
 
 
 static int zc0301_release(struct inode* inode, struct file* filp)
 {
-	struct zc0301_device* cam = video_get_drvdata(video_devdata(filp));
+	struct zc0301_device* cam;
 
-	mutex_lock(&cam->dev_mutex); /* prevent disconnect() to be called */
+	down_write(&zc0301_dev_lock);
+
+	cam = video_get_drvdata(video_devdata(filp));
 
 	zc0301_stop_transfer(cam);
-
 	zc0301_release_buffers(cam);
-
-	if (cam->state & DEV_DISCONNECTED) {
-		zc0301_release_resources(cam);
-		usb_put_dev(cam->usbdev);
-		mutex_unlock(&cam->dev_mutex);
-		kfree(cam);
-		return 0;
-	}
-
 	cam->users--;
-	wake_up_interruptible_nr(&cam->open, 1);
+	wake_up_interruptible_nr(&cam->wait_open, 1);
 
 	DBG(3, "Video device /dev/video%d closed", cam->v4ldev->minor);
 
-	mutex_unlock(&cam->dev_mutex);
+	kref_put(&cam->kref, zc0301_release_resources);
+
+	up_write(&zc0301_dev_lock);
 
 	return 0;
 }
@@ -775,7 +786,7 @@ zc0301_read(struct file* filp, char __user * buf, size_t count, loff_t* f_pos)
 		DBG(3, "Close and open the device again to choose the read "
 		       "method");
 		mutex_unlock(&cam->fileop_mutex);
-		return -EINVAL;
+		return -EBUSY;
 	}
 
 	if (cam->io == IO_NONE) {
@@ -953,7 +964,12 @@ static int zc0301_mmap(struct file* filp, struct vm_area_struct *vma)
 		return -EIO;
 	}
 
-	if (cam->io != IO_MMAP || !(vma->vm_flags & VM_WRITE) ||
+	if (!(vma->vm_flags & (VM_WRITE | VM_READ))) {
+		mutex_unlock(&cam->fileop_mutex);
+		return -EACCES;
+	}
+
+	if (cam->io != IO_MMAP ||
 	    size != PAGE_ALIGN(cam->frame[0].buf.length)) {
 		mutex_unlock(&cam->fileop_mutex);
 		return -EINVAL;
@@ -984,7 +1000,6 @@ static int zc0301_mmap(struct file* filp, struct vm_area_struct *vma)
 
 	vma->vm_ops = &zc0301_vm_ops;
 	vma->vm_private_data = &cam->frame[i];
-
 	zc0301_vm_open(vma);
 
 	mutex_unlock(&cam->fileop_mutex);
@@ -1211,7 +1226,7 @@ zc0301_vidioc_s_crop(struct zc0301_device* cam, void __user * arg)
 			if (cam->frame[i].vma_use_count) {
 				DBG(3, "VIDIOC_S_CROP failed. "
 				       "Unmap the buffers first.");
-				return -EINVAL;
+				return -EBUSY;
 			}
 
 	if (!s->set_crop) {
@@ -1434,7 +1449,7 @@ zc0301_vidioc_try_s_fmt(struct zc0301_device* cam, unsigned int cmd,
 			if (cam->frame[i].vma_use_count) {
 				DBG(3, "VIDIOC_S_FMT failed. "
 				       "Unmap the buffers first.");
-				return -EINVAL;
+				return -EBUSY;
 			}
 
 	if (cam->stream == STREAM_ON)
@@ -1544,14 +1559,14 @@ zc0301_vidioc_reqbufs(struct zc0301_device* cam, void __user * arg)
 	if (cam->io == IO_READ) {
 		DBG(3, "Close and open the device again to choose the mmap "
 		       "I/O method");
-		return -EINVAL;
+		return -EBUSY;
 	}
 
 	for (i = 0; i < cam->nbuffers; i++)
 		if (cam->frame[i].vma_use_count) {
 			DBG(3, "VIDIOC_REQBUFS failed. "
 			       "Previous buffers are still mapped.");
-			return -EINVAL;
+			return -EBUSY;
 		}
 
 	if (cam->stream == STREAM_ON)
@@ -1699,9 +1714,6 @@ zc0301_vidioc_streamon(struct zc0301_device* cam, void __user * arg)
 	if (type != V4L2_BUF_TYPE_VIDEO_CAPTURE || cam->io != IO_MMAP)
 		return -EINVAL;
 
-	if (list_empty(&cam->inqueue))
-		return -EINVAL;
-
 	cam->stream = STREAM_ON;
 
 	DBG(3, "Stream on");
@@ -1949,8 +1961,6 @@ zc0301_usb_probe(struct usb_interface* intf, const struct usb_device_id* id)
 		goto fail;
 	}
 
-	mutex_init(&cam->dev_mutex);
-
 	DBG(2, "ZC0301[P] Image Processor and Control Chip detected "
 	       "(vid/pid 0x%04X:0x%04X)",id->idVendor, id->idProduct);
 
@@ -1982,7 +1992,7 @@ zc0301_usb_probe(struct usb_interface* intf, const struct usb_device_id* id)
 	cam->v4ldev->release = video_device_release;
 	video_set_drvdata(cam->v4ldev, cam);
 
-	mutex_lock(&cam->dev_mutex);
+	init_completion(&cam->probe);
 
 	err = video_register_device(cam->v4ldev, VFL_TYPE_GRABBER,
 				    video_nr[dev_nr]);
@@ -1992,7 +2002,7 @@ zc0301_usb_probe(struct usb_interface* intf, const struct usb_device_id* id)
 			DBG(1, "Free /dev/videoX node not found");
 		video_nr[dev_nr] = -1;
 		dev_nr = (dev_nr < ZC0301_MAX_DEVICES-1) ? dev_nr+1 : 0;
-		mutex_unlock(&cam->dev_mutex);
+		complete_all(&cam->probe);
 		goto fail;
 	}
 
@@ -2004,8 +2014,10 @@ zc0301_usb_probe(struct usb_interface* intf, const struct usb_device_id* id)
 	dev_nr = (dev_nr < ZC0301_MAX_DEVICES-1) ? dev_nr+1 : 0;
 
 	usb_set_intfdata(intf, cam);
+	kref_init(&cam->kref);
+	usb_get_dev(cam->usbdev);
 
-	mutex_unlock(&cam->dev_mutex);
+	complete_all(&cam->probe);
 
 	return 0;
 
@@ -2022,40 +2034,31 @@ fail:
 
 static void zc0301_usb_disconnect(struct usb_interface* intf)
 {
-	struct zc0301_device* cam = usb_get_intfdata(intf);
+	struct zc0301_device* cam;
 
-	if (!cam)
+	down_write(&zc0301_dev_lock);
-		return;
 
-	down_write(&zc0301_disconnect);
+	cam = usb_get_intfdata(intf);
-
-	mutex_lock(&cam->dev_mutex);
 
 	DBG(2, "Disconnecting %s...", cam->v4ldev->name);
 
-	wake_up_interruptible_all(&cam->open);
-
 	if (cam->users) {
 		DBG(2, "Device /dev/video%d is open! Deregistration and "
-		       "memory deallocation are deferred on close.",
+		       "memory deallocation are deferred.",
 		    cam->v4ldev->minor);
 		cam->state |= DEV_MISCONFIGURED;
 		zc0301_stop_transfer(cam);
 		cam->state |= DEV_DISCONNECTED;
 		wake_up_interruptible(&cam->wait_frame);
 		wake_up(&cam->wait_stream);
-		usb_get_dev(cam->usbdev);
+	} else
-	} else {
 		cam->state |= DEV_DISCONNECTED;
-		zc0301_release_resources(cam);
-	}
 
-	mutex_unlock(&cam->dev_mutex);
+	wake_up_interruptible_all(&cam->wait_open);
 
-	if (!cam->users)
+	kref_put(&cam->kref, zc0301_release_resources);
-		kfree(cam);
 
-	up_write(&zc0301_disconnect);
+	up_write(&zc0301_dev_lock);
 }
 
 

drivers/media/video/zc0301/zc0301_pas202bcb.c

@@ -327,6 +327,7 @@ static struct zc0301_sensor pas202bcb = {
 		.height = 480,
 		.pixelformat = V4L2_PIX_FMT_JPEG,
 		.priv = 8,
+		.colorspace = V4L2_COLORSPACE_JPEG,
 	},
 };
 

drivers/media/video/zc0301/zc0301_pb0330.c

@@ -157,6 +157,7 @@ static struct zc0301_sensor pb0330 = {
 		.height = 480,
 		.pixelformat = V4L2_PIX_FMT_JPEG,
 		.priv = 8,
+		.colorspace = V4L2_COLORSPACE_JPEG,
 	},
 };
 

drivers/media/video/zc0301/zc0301_sensor.h

@@ -23,7 +23,7 @@
 #define _ZC0301_SENSOR_H_
 
 #include <linux/usb.h>
-#include <linux/videodev.h>
+#include <linux/videodev2.h>
 #include <linux/device.h>
 #include <linux/stddef.h>
 #include <linux/errno.h>