public inbox for linux-media@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/4] tm6000: don't use BKL at the driver
       [not found] <cover.1286807828.git.mchehab@redhat.com>
@ 2010-10-11 15:37 ` Mauro Carvalho Chehab
  2010-10-11 15:37 ` [PATCH 2/4] V4L/DVB: tm6000-alsa: fix some locking issues Mauro Carvalho Chehab
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 4+ messages in thread
From: Mauro Carvalho Chehab @ 2010-10-11 15:37 UTC (permalink / raw)
  Cc: Linux Media Mailing List

Instead, use core lock handling.

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

diff --git a/drivers/staging/tm6000/tm6000-cards.c b/drivers/staging/tm6000/tm6000-cards.c
index 9d091c3..4106ae0 100644
--- a/drivers/staging/tm6000/tm6000-cards.c
+++ b/drivers/staging/tm6000/tm6000-cards.c
@@ -909,8 +909,6 @@ static void tm6000_usb_disconnect(struct usb_interface *interface)
 
 	printk(KERN_INFO "tm6000: disconnecting %s\n", dev->name);
 
-	mutex_lock(&dev->lock);
-
 	tm6000_ir_fini(dev);
 
 	if (dev->gpio.power_led) {
@@ -945,7 +943,6 @@ static void tm6000_usb_disconnect(struct usb_interface *interface)
 	tm6000_close_extension(dev);
 	tm6000_remove_from_devlist(dev);
 
-	mutex_unlock(&dev->lock);
 	kfree(dev);
 }
 
diff --git a/drivers/staging/tm6000/tm6000-core.c b/drivers/staging/tm6000/tm6000-core.c
index 57cb69e..f5f8632 100644
--- a/drivers/staging/tm6000/tm6000-core.c
+++ b/drivers/staging/tm6000/tm6000-core.c
@@ -326,10 +326,8 @@ int tm6000_init_analog_mode(struct tm6000_core *dev)
 
 	/*FIXME: Hack!!! */
 	struct v4l2_frequency f;
-	mutex_lock(&dev->lock);
 	f.frequency = dev->freq;
 	v4l2_device_call_all(&dev->v4l2_dev, 0, tuner, s_frequency, &f);
-	mutex_unlock(&dev->lock);
 
 	msleep(100);
 	tm6000_set_standard(dev, &dev->norm);
diff --git a/drivers/staging/tm6000/tm6000-video.c b/drivers/staging/tm6000/tm6000-video.c
index a45b012..23c85fd 100644
--- a/drivers/staging/tm6000/tm6000-video.c
+++ b/drivers/staging/tm6000/tm6000-video.c
@@ -791,16 +791,11 @@ static struct videobuf_queue_ops tm6000_video_qops = {
 static int res_get(struct tm6000_core *dev, struct tm6000_fh *fh)
 {
 	/* is it free? */
-	mutex_lock(&dev->lock);
-	if (dev->resources) {
-		/* no, someone else uses it */
-		mutex_unlock(&dev->lock);
+	if (dev->resources)
 		return 0;
-	}
 	/* it's free, grab it */
 	dev->resources =1;
 	dprintk(dev, V4L2_DEBUG_RES_LOCK, "res: get\n");
-	mutex_unlock(&dev->lock);
 	return 1;
 }
 
@@ -811,10 +806,8 @@ static int res_locked(struct tm6000_core *dev)
 
 static void res_free(struct tm6000_core *dev, struct tm6000_fh *fh)
 {
-	mutex_lock(&dev->lock);
 	dev->resources = 0;
 	dprintk(dev, V4L2_DEBUG_RES_LOCK, "res: put\n");
-	mutex_unlock(&dev->lock);
 }
 
 /* ------------------------------------------------------------------
@@ -1229,10 +1222,8 @@ static int vidioc_s_frequency (struct file *file, void *priv,
 	if (unlikely(f->tuner != 0))
 		return -EINVAL;
 
-//	mutex_lock(&dev->lock);
 	dev->freq = f->frequency;
 	v4l2_device_call_all(&dev->v4l2_dev, 0, tuner, s_frequency, f);
-//	mutex_unlock(&dev->lock);
 
 	return 0;
 }
@@ -1308,7 +1299,7 @@ static int tm6000_open(struct file *file)
 			NULL, &dev->slock,
 			fh->type,
 			V4L2_FIELD_INTERLACED,
-			sizeof(struct tm6000_buffer), fh, NULL);
+			sizeof(struct tm6000_buffer), fh, &dev->lock);
 
 	return 0;
 }
@@ -1389,7 +1380,7 @@ static struct v4l2_file_operations tm6000_fops = {
 	.owner		= THIS_MODULE,
 	.open           = tm6000_open,
 	.release        = tm6000_release,
-	.ioctl          = video_ioctl2, /* V4L2 ioctl handler */
+	.unlocked_ioctl	= video_ioctl2, /* V4L2 ioctl handler */
 	.read           = tm6000_read,
 	.poll		= tm6000_poll,
 	.mmap		= tm6000_mmap,
@@ -1451,8 +1442,10 @@ int tm6000_v4l2_register(struct tm6000_core *dev)
 	INIT_LIST_HEAD(&dev->vidq.active);
 	INIT_LIST_HEAD(&dev->vidq.queued);
 
-	memcpy (dev->vfd, &tm6000_template, sizeof(*(dev->vfd)));
-	dev->vfd->debug=tm6000_debug;
+	memcpy(dev->vfd, &tm6000_template, sizeof(*(dev->vfd)));
+	dev->vfd->debug = tm6000_debug;
+	dev->vfd->lock = &dev->lock;
+
 	vfd->v4l2_dev = &dev->v4l2_dev;
 	video_set_drvdata(vfd, dev);
 
-- 
1.7.1



^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH 2/4] V4L/DVB: tm6000-alsa: fix some locking issues
       [not found] <cover.1286807828.git.mchehab@redhat.com>
  2010-10-11 15:37 ` [PATCH 1/4] tm6000: don't use BKL at the driver Mauro Carvalho Chehab
@ 2010-10-11 15:37 ` Mauro Carvalho Chehab
  2010-10-11 15:37 ` [PATCH 3/4] V4L/DVB: Use just one lock for devlist Mauro Carvalho Chehab
  2010-10-11 15:37 ` [PATCH 4/4] V4L/DVB: tm6000: fix resource locking Mauro Carvalho Chehab
  3 siblings, 0 replies; 4+ messages in thread
From: Mauro Carvalho Chehab @ 2010-10-11 15:37 UTC (permalink / raw)
  Cc: Linux Media Mailing List

Those locking issues affect tvtime, causing a kernel oops/panic, due to
a race condition.

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

diff --git a/drivers/media/video/cx231xx/cx231xx-cards.c b/drivers/media/video/cx231xx/cx231xx-cards.c
index c4cfcab..9c3a926 100644
--- a/drivers/media/video/cx231xx/cx231xx-cards.c
+++ b/drivers/media/video/cx231xx/cx231xx-cards.c
@@ -462,6 +462,8 @@ struct usb_device_id cx231xx_id_table[] = {
 	 .driver_info = CX231XX_BOARD_HAUPPAUGE_USBLIVE2},
 	{USB_DEVICE_VER(USB_VID_PIXELVIEW, USB_PID_PIXELVIEW_SBTVD, 0x4000, 0x4001),
 	 .driver_info = CX231XX_BOARD_PV_PLAYTV_USB_HYBRID},
+	{USB_DEVICE(0x1b80, 0xe424),
+	 .driver_info = CX231XX_BOARD_PV_PLAYTV_USB_HYBRID},
 	{},
 };
 
diff --git a/drivers/staging/tm6000/tm6000-alsa.c b/drivers/staging/tm6000/tm6000-alsa.c
index a99101f..e379e3e 100644
--- a/drivers/staging/tm6000/tm6000-alsa.c
+++ b/drivers/staging/tm6000/tm6000-alsa.c
@@ -201,6 +201,14 @@ _error:
  */
 static int snd_tm6000_close(struct snd_pcm_substream *substream)
 {
+	struct snd_tm6000_card *chip = snd_pcm_substream_chip(substream);
+	struct tm6000_core *core = chip->core;
+
+	if (atomic_read(&core->stream_started) > 0) {
+		atomic_set(&core->stream_started, 0);
+		schedule_work(&core->wq_trigger);
+	}
+
 	return 0;
 }
 
@@ -213,6 +221,9 @@ static int tm6000_fillbuf(struct tm6000_core *core, char *buf, int size)
 	unsigned int stride, buf_pos;
 	int length;
 
+	if (atomic_read(&core->stream_started) == 0)
+		return 0;
+
 	if (!size || !substream) {
 		dprintk(1, "substream was NULL\n");
 		return -EINVAL;
@@ -298,8 +309,12 @@ static int snd_tm6000_hw_params(struct snd_pcm_substream *substream,
 static int snd_tm6000_hw_free(struct snd_pcm_substream *substream)
 {
 	struct snd_tm6000_card *chip = snd_pcm_substream_chip(substream);
+	struct tm6000_core *core = chip->core;
 
-	_tm6000_stop_audio_dma(chip);
+	if (atomic_read(&core->stream_started) > 0) {
+		atomic_set(&core->stream_started, 0);
+		schedule_work(&core->wq_trigger);
+	}
 
 	return 0;
 }
@@ -321,30 +336,42 @@ static int snd_tm6000_prepare(struct snd_pcm_substream *substream)
 /*
  * trigger callback
  */
+static void audio_trigger(struct work_struct *work)
+{
+	struct tm6000_core *core = container_of(work, struct tm6000_core,
+						wq_trigger);
+	struct snd_tm6000_card *chip = core->adev;
+
+	if (atomic_read(&core->stream_started)) {
+		dprintk(1, "starting capture");
+		_tm6000_start_audio_dma(chip);
+	} else {
+		dprintk(1, "stopping capture");
+		_tm6000_stop_audio_dma(chip);
+	}
+}
+
 static int snd_tm6000_card_trigger(struct snd_pcm_substream *substream, int cmd)
 {
 	struct snd_tm6000_card *chip = snd_pcm_substream_chip(substream);
-	int err;
-
-	spin_lock(&chip->reg_lock);
+	struct tm6000_core *core = chip->core;
+	int err = 0;
 
 	switch (cmd) {
 	case SNDRV_PCM_TRIGGER_START:
-		err = _tm6000_start_audio_dma(chip);
+		atomic_set(&core->stream_started, 1);
 		break;
 	case SNDRV_PCM_TRIGGER_STOP:
-		err = _tm6000_stop_audio_dma(chip);
+		atomic_set(&core->stream_started, 0);
 		break;
 	default:
 		err = -EINVAL;
 		break;
 	}
-
-	spin_unlock(&chip->reg_lock);
+	schedule_work(&core->wq_trigger);
 
 	return err;
 }
-
 /*
  * pointer callback
  */
@@ -437,6 +464,7 @@ int tm6000_audio_init(struct tm6000_core *dev)
 
 	snd_pcm_set_ops(pcm, SNDRV_PCM_STREAM_CAPTURE, &snd_tm6000_pcm_ops);
 
+	INIT_WORK(&dev->wq_trigger, audio_trigger);
 	rc = snd_card_register(card);
 	if (rc < 0)
 		goto error;
diff --git a/drivers/staging/tm6000/tm6000.h b/drivers/staging/tm6000/tm6000.h
index 1ec1bff..5d9dcab 100644
--- a/drivers/staging/tm6000/tm6000.h
+++ b/drivers/staging/tm6000/tm6000.h
@@ -205,6 +205,9 @@ struct tm6000_core {
 
 	/* audio support */
 	struct snd_tm6000_card		*adev;
+	struct work_struct		wq_trigger;   /* Trigger to start/stop audio for alsa module */
+	atomic_t			stream_started;  /* stream should be running if true */
+
 
 	struct tm6000_IR		*ir;
 
-- 
1.7.1



^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH 3/4] V4L/DVB: Use just one lock for devlist
       [not found] <cover.1286807828.git.mchehab@redhat.com>
  2010-10-11 15:37 ` [PATCH 1/4] tm6000: don't use BKL at the driver Mauro Carvalho Chehab
  2010-10-11 15:37 ` [PATCH 2/4] V4L/DVB: tm6000-alsa: fix some locking issues Mauro Carvalho Chehab
@ 2010-10-11 15:37 ` Mauro Carvalho Chehab
  2010-10-11 15:37 ` [PATCH 4/4] V4L/DVB: tm6000: fix resource locking Mauro Carvalho Chehab
  3 siblings, 0 replies; 4+ messages in thread
From: Mauro Carvalho Chehab @ 2010-10-11 15:37 UTC (permalink / raw)
  Cc: Linux Media Mailing List

This avoids a race condition

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

diff --git a/drivers/staging/tm6000/tm6000-core.c b/drivers/staging/tm6000/tm6000-core.c
index f5f8632..02c9c7c 100644
--- a/drivers/staging/tm6000/tm6000-core.c
+++ b/drivers/staging/tm6000/tm6000-core.c
@@ -657,7 +657,6 @@ void tm6000_add_into_devlist(struct tm6000_core *dev)
  */
 
 static LIST_HEAD(tm6000_extension_devlist);
-static DEFINE_MUTEX(tm6000_extension_devlist_lock);
 
 int tm6000_call_fillbuf(struct tm6000_core *dev, enum tm6000_ops_type type,
 			char *buf, int size)
@@ -681,14 +680,12 @@ int tm6000_register_extension(struct tm6000_ops *ops)
 	struct tm6000_core *dev = NULL;
 
 	mutex_lock(&tm6000_devlist_mutex);
-	mutex_lock(&tm6000_extension_devlist_lock);
 	list_add_tail(&ops->next, &tm6000_extension_devlist);
 	list_for_each_entry(dev, &tm6000_devlist, devlist) {
 		ops->init(dev);
 		printk(KERN_INFO "%s: Initialized (%s) extension\n",
 		       dev->name, ops->name);
 	}
-	mutex_unlock(&tm6000_extension_devlist_lock);
 	mutex_unlock(&tm6000_devlist_mutex);
 	return 0;
 }
@@ -704,10 +701,8 @@ void tm6000_unregister_extension(struct tm6000_ops *ops)
 			ops->fini(dev);
 	}
 
-	mutex_lock(&tm6000_extension_devlist_lock);
 	printk(KERN_INFO "tm6000: Remove (%s) extension\n", ops->name);
 	list_del(&ops->next);
-	mutex_unlock(&tm6000_extension_devlist_lock);
 	mutex_unlock(&tm6000_devlist_mutex);
 }
 EXPORT_SYMBOL(tm6000_unregister_extension);
@@ -716,26 +711,26 @@ void tm6000_init_extension(struct tm6000_core *dev)
 {
 	struct tm6000_ops *ops = NULL;
 
-	mutex_lock(&tm6000_extension_devlist_lock);
+	mutex_lock(&tm6000_devlist_mutex);
 	if (!list_empty(&tm6000_extension_devlist)) {
 		list_for_each_entry(ops, &tm6000_extension_devlist, next) {
 			if (ops->init)
 				ops->init(dev);
 		}
 	}
-	mutex_unlock(&tm6000_extension_devlist_lock);
+	mutex_unlock(&tm6000_devlist_mutex);
 }
 
 void tm6000_close_extension(struct tm6000_core *dev)
 {
 	struct tm6000_ops *ops = NULL;
 
-	mutex_lock(&tm6000_extension_devlist_lock);
+	mutex_lock(&tm6000_devlist_mutex);
 	if (!list_empty(&tm6000_extension_devlist)) {
 		list_for_each_entry(ops, &tm6000_extension_devlist, next) {
 			if (ops->fini)
 				ops->fini(dev);
 		}
 	}
-	mutex_unlock(&tm6000_extension_devlist_lock);
+	mutex_lock(&tm6000_devlist_mutex);
 }
-- 
1.7.1



^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH 4/4] V4L/DVB: tm6000: fix resource locking
       [not found] <cover.1286807828.git.mchehab@redhat.com>
                   ` (2 preceding siblings ...)
  2010-10-11 15:37 ` [PATCH 3/4] V4L/DVB: Use just one lock for devlist Mauro Carvalho Chehab
@ 2010-10-11 15:37 ` Mauro Carvalho Chehab
  3 siblings, 0 replies; 4+ messages in thread
From: Mauro Carvalho Chehab @ 2010-10-11 15:37 UTC (permalink / raw)
  Cc: Linux Media Mailing List

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

diff --git a/drivers/staging/tm6000/tm6000-video.c b/drivers/staging/tm6000/tm6000-video.c
index 23c85fd..f184585 100644
--- a/drivers/staging/tm6000/tm6000-video.c
+++ b/drivers/staging/tm6000/tm6000-video.c
@@ -788,25 +788,49 @@ static struct videobuf_queue_ops tm6000_video_qops = {
 	IOCTL handling
    ------------------------------------------------------------------*/
 
-static int res_get(struct tm6000_core *dev, struct tm6000_fh *fh)
+static bool is_res_read(struct tm6000_core *dev, struct tm6000_fh *fh)
 {
+	/* Is the current fh handling it? if so, that's OK */
+	if (dev->resources == fh && dev->is_res_read)
+		return true;
+
+	return false;
+}
+
+static bool is_res_streaming(struct tm6000_core *dev, struct tm6000_fh *fh)
+{
+	/* Is the current fh handling it? if so, that's OK */
+	if (dev->resources == fh)
+		return true;
+
+	return false;
+}
+
+static bool res_get(struct tm6000_core *dev, struct tm6000_fh *fh,
+		   bool is_res_read)
+{
+	/* Is the current fh handling it? if so, that's OK */
+	if (dev->resources == fh && dev->is_res_read == is_res_read)
+		return true;
+
 	/* is it free? */
 	if (dev->resources)
-		return 0;
-	/* it's free, grab it */
-	dev->resources =1;
+		return false;
+
+	/* grab it */
+	dev->resources = fh;
+	dev->is_res_read = is_res_read;
 	dprintk(dev, V4L2_DEBUG_RES_LOCK, "res: get\n");
-	return 1;
-}
-
-static int res_locked(struct tm6000_core *dev)
-{
-	return (dev->resources);
+	return true;
 }
 
 static void res_free(struct tm6000_core *dev, struct tm6000_fh *fh)
 {
-	dev->resources = 0;
+	/* Is the current fh handling it? if so, that's OK */
+	if (dev->resources != fh)
+		return;
+
+	dev->resources = NULL;
 	dprintk(dev, V4L2_DEBUG_RES_LOCK, "res: put\n");
 }
 
@@ -981,7 +1005,7 @@ static int vidioc_streamon(struct file *file, void *priv, enum v4l2_buf_type i)
 	if (i != fh->type)
 		return -EINVAL;
 
-	if (!res_get(dev,fh))
+	if (!res_get(dev, fh, false))
 		return -EBUSY;
 	return (videobuf_streamon(&fh->vb_vidq));
 }
@@ -1310,7 +1334,7 @@ tm6000_read(struct file *file, char __user *data, size_t count, loff_t *pos)
 	struct tm6000_fh        *fh = file->private_data;
 
 	if (fh->type==V4L2_BUF_TYPE_VIDEO_CAPTURE) {
-		if (res_locked(fh->dev))
+		if (!res_get(fh->dev, fh, true))
 			return -EBUSY;
 
 		return videobuf_read_stream(&fh->vb_vidq, data, count, pos, 0,
@@ -1328,7 +1352,10 @@ tm6000_poll(struct file *file, struct poll_table_struct *wait)
 	if (V4L2_BUF_TYPE_VIDEO_CAPTURE != fh->type)
 		return POLLERR;
 
-	if (res_get(fh->dev,fh)) {
+	if (!!is_res_streaming(fh->dev, fh))
+		return POLLERR;
+
+	if (!is_res_read(fh->dev, fh)) {
 		/* streaming capture */
 		if (list_empty(&fh->vb_vidq.stream))
 			return POLLERR;
@@ -1356,6 +1383,7 @@ static int tm6000_release(struct file *file)
 
 	dev->users--;
 
+	res_free(dev, fh);
 	if (!dev->users) {
 		tm6000_uninit_isoc(dev);
 		videobuf_mmap_free(&fh->vb_vidq);
diff --git a/drivers/staging/tm6000/tm6000.h b/drivers/staging/tm6000/tm6000.h
index 5d9dcab..a1c6ca2 100644
--- a/drivers/staging/tm6000/tm6000.h
+++ b/drivers/staging/tm6000/tm6000.h
@@ -189,7 +189,9 @@ struct tm6000_core {
 	int				users;
 
 	/* various device info */
-	unsigned int			resources;
+	struct tm6000_fh		*resources;	/* Points to fh that is streaming */
+	bool				is_res_read;
+
 	struct video_device		*vfd;
 	struct tm6000_dmaqueue		vidq;
 	struct v4l2_device		v4l2_dev;
-- 
1.7.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2010-10-11 15:43 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <cover.1286807828.git.mchehab@redhat.com>
2010-10-11 15:37 ` [PATCH 1/4] tm6000: don't use BKL at the driver Mauro Carvalho Chehab
2010-10-11 15:37 ` [PATCH 2/4] V4L/DVB: tm6000-alsa: fix some locking issues Mauro Carvalho Chehab
2010-10-11 15:37 ` [PATCH 3/4] V4L/DVB: Use just one lock for devlist Mauro Carvalho Chehab
2010-10-11 15:37 ` [PATCH 4/4] V4L/DVB: tm6000: fix resource locking Mauro Carvalho Chehab

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox