em28xx: fix/improve em28xx locking schema

From: Mauro Carvalho Chehab <mchehab@redhat.com>

Changes/fixes on em28xx dev->lock:

- em28xx_init_dev() were unlocking without a previous lock;

- some read ioctls need to lock after the removal of KBL, since a write
  may be happening at the same time an ioctl is reading;

- keep the device locked during all device initialization;

- lock/unlock while reading/writing registers.

Priority: normal

Signed-off-by: Mauro Carvalho Chehab <mchehab@redhat.com>

1 files changed, 43 insertions, 12 deletions

diff --git a/linux/drivers/media/video/em28xx/em28xx-video.c b/linux/drivers/media/video/em28xx/em28xx-video.c
index 415ac9861..acf6141aa 100644
--- a/linux/drivers/media/video/em28xx/em28xx-video.c
+++ b/linux/drivers/media/video/em28xx/em28xx-video.c
@@ -789,10 +789,10 @@ static int vidioc_s_fmt_vid_cap(struct file *file, void *priv,
 	if (rc < 0)
 		return rc;
 
-	vidioc_try_fmt_vid_cap(file, priv, f);
-
 	mutex_lock(&dev->lock);
 
+	vidioc_try_fmt_vid_cap(file, priv, f);
+
 	if (videobuf_queue_is_busy(&fh->vb_vidq)) {
 		em28xx_errdev("%s queue busy\n", __func__);
 		rc = -EBUSY;
@@ -833,15 +833,12 @@ static int vidioc_s_std(struct file *file, void *priv, v4l2_std_id * norm)
 
 	mutex_lock(&dev->lock);
 	dev->norm = *norm;
-	mutex_unlock(&dev->lock);
 
 	/* Adjusts width/height, if needed */
 	f.fmt.pix.width = dev->width;
 	f.fmt.pix.height = dev->height;
 	vidioc_try_fmt_vid_cap(file, priv, &f);
 
-	mutex_lock(&dev->lock);
-
 	/* set new image size */
 	dev->width = f.fmt.pix.width;
 	dev->height = f.fmt.pix.height;
@@ -976,11 +973,15 @@ static int vidioc_s_audio(struct file *file, void *priv, struct v4l2_audio *a)
 	if (a->index != dev->ctl_ainput)
 		return -EINVAL;
 #else
+	mutex_lock(&dev->lock);
+
 	dev->ctl_ainput = INPUT(a->index)->amux;
 	dev->ctl_aoutput = INPUT(a->index)->aout;
 
 	if (!dev->ctl_aoutput)
 		dev->ctl_aoutput = EM28XX_AOUT_MASTER;
+
+	mutex_unlock(&dev->lock);
 #endif
 	return 0;
 }
@@ -1030,6 +1031,7 @@ static int vidioc_g_ctrl(struct file *file, void *priv,
 	rc = check_dev(dev);
 	if (rc < 0)
 		return rc;
+
 	mutex_lock(&dev->lock);
 
 	if (!dev->board.has_msp34xx)
@@ -1140,8 +1142,10 @@ static int vidioc_g_frequency(struct file *file, void *priv,
 	struct em28xx_fh      *fh  = priv;
 	struct em28xx         *dev = fh->dev;
 
+	mutex_lock(&dev->lock);
 	f->type = fh->radio ? V4L2_TUNER_RADIO : V4L2_TUNER_ANALOG_TV;
 	f->frequency = dev->ctl_freq;
+	mutex_unlock(&dev->lock);
 
 	return 0;
 }
@@ -1171,6 +1175,7 @@ static int vidioc_s_frequency(struct file *file, void *priv,
 	em28xx_i2c_call_clients(dev, VIDIOC_S_FREQUENCY, f);
 
 	mutex_unlock(&dev->lock);
+
 	return 0;
 }
 
@@ -1198,15 +1203,20 @@ static int vidioc_g_register(struct file *file, void *priv,
 		return -EINVAL;
 
 	if (em28xx_reg_len(reg->reg) == 1) {
+		mutex_lock(&dev->lock);
 		ret = em28xx_read_reg(dev, reg->reg);
+		mutex_unlock(&dev->lock);
+
 		if (ret < 0)
 			return ret;
 
 		reg->val = ret;
 	} else {
 		__le64 val = 0;
+		mutex_lock(&dev->lock);
 		ret = em28xx_read_reg_req_len(dev, USB_REQ_GET_STATUS,
 						   reg->reg, (char *)&val, 2);
+		mutex_unlock(&dev->lock);
 		if (ret < 0)
 			return ret;
 
@@ -1222,11 +1232,16 @@ static int vidioc_s_register(struct file *file, void *priv,
 	struct em28xx_fh      *fh  = priv;
 	struct em28xx         *dev = fh->dev;
 	__le64 buf;
+	int    rc;
 
 	buf = cpu_to_le64(reg->val);
 
-	return em28xx_write_regs(dev, reg->reg, (char *)&buf,
-				 em28xx_reg_len(reg->reg));
+	mutex_lock(&dev->lock);
+	rc = em28xx_write_regs(dev, reg->reg, (char *)&buf,
+			       em28xx_reg_len(reg->reg));
+	mutex_unlock(&dev->lock);
+
+	return rc;
 }
 #endif
 
@@ -1265,12 +1280,15 @@ static int vidioc_streamon(struct file *file, void *priv,
 
 	mutex_lock(&dev->lock);
 	rc = res_get(fh);
-	mutex_unlock(&dev->lock);
 
 	if (unlikely(rc < 0))
 		return rc;
 
-	return (videobuf_streamon(&fh->vb_vidq));
+	rc = videobuf_streamon(&fh->vb_vidq);
+
+	mutex_unlock(&dev->lock);
+
+	return rc;
 }
 
 static int vidioc_streamoff(struct file *file, void *priv,
@@ -1289,9 +1307,11 @@ static int vidioc_streamoff(struct file *file, void *priv,
 	if (type != fh->type)
 		return -EINVAL;
 
-	videobuf_streamoff(&fh->vb_vidq);
 	mutex_lock(&dev->lock);
+
+	videobuf_streamoff(&fh->vb_vidq);
 	res_free(fh);
+
 	mutex_unlock(&dev->lock);
 
 	return 0;
@@ -1513,7 +1533,10 @@ static int radio_g_tuner(struct file *file, void *priv,
 	strcpy(t->name, "Radio");
 	t->type = V4L2_TUNER_RADIO;
 
+	mutex_lock(&dev->lock);
 	em28xx_i2c_call_clients(dev, VIDIOC_G_TUNER, t);
+	mutex_unlock(&dev->lock);
+
 	return 0;
 }
 
@@ -1545,7 +1568,9 @@ static int radio_s_tuner(struct file *file, void *priv,
 	if (0 != t->index)
 		return -EINVAL;
 
+	mutex_lock(&dev->lock);
 	em28xx_i2c_call_clients(dev, VIDIOC_S_TUNER, t);
+	mutex_unlock(&dev->lock);
 
 	return 0;
 }
@@ -1609,6 +1634,7 @@ static int em28xx_v4l2_open(struct inode *inode, struct file *filp)
 		}
 	}
 	mutex_unlock(&em28xx_devlist_mutex);
+
 	if (NULL == dev)
 		return -ENODEV;
 
@@ -2104,7 +2130,6 @@ static int em28xx_init_dev(struct em28xx **devhandle, struct usb_device *udev,
 	unsigned int maxh, maxw;
 
 	dev->udev = udev;
-	mutex_init(&dev->lock);
 	mutex_init(&dev->ctrl_urb_lock);
 	spin_lock_init(&dev->slock);
 	init_waitqueue_head(&dev->open);
@@ -2222,7 +2247,6 @@ static int em28xx_init_dev(struct em28xx **devhandle, struct usb_device *udev,
 	return 0;
 
 fail_reg_devices:
-	mutex_unlock(&dev->lock);
 	return retval;
 }
 
@@ -2425,6 +2449,8 @@ static int em28xx_usb_probe(struct usb_interface *interface,
 		dev->model = card[nr];
 
 	/* allocate device struct */
+	mutex_init(&dev->lock);
+	mutex_lock(&dev->lock);
 	retval = em28xx_init_dev(&dev, udev, nr);
 	if (retval) {
 		em28xx_devused &= ~(1<<dev->devno);
@@ -2438,6 +2464,11 @@ static int em28xx_usb_probe(struct usb_interface *interface,
 
 	request_modules(dev);
 
+	/* Should be the last thing to do, to avoid newer udev's to
+	   open the device before fully initializing it
+	 */
+	mutex_unlock(&dev->lock);
+
 	return 0;
 }