From 39a96b4cf592e79aefd1b4f2b136c20ec7bf10a7 Mon Sep 17 00:00:00 2001 From: Mauro Carvalho Chehab Date: Sat, 9 Oct 2010 15:53:58 -0300 Subject: [PATCH] [media] em28xx-audio: fix some locking issues Those locking issues affect tvtime, causing a kernel oops/panic, due to a race condition. Signed-off-by: Mauro Carvalho Chehab --- drivers/media/video/em28xx/em28xx-audio.c | 75 ++++++++++++++----------------- drivers/media/video/em28xx/em28xx.h | 17 ++++--- 2 files changed, 42 insertions(+), 50 deletions(-) diff --git a/drivers/media/video/em28xx/em28xx-audio.c b/drivers/media/video/em28xx/em28xx-audio.c index e182abf476c..3c48a72eb7d 100644 --- a/drivers/media/video/em28xx/em28xx-audio.c +++ b/drivers/media/video/em28xx/em28xx-audio.c @@ -102,6 +102,9 @@ static void em28xx_audio_isocirq(struct urb *urb) break; } + if (atomic_read(&dev->stream_started) == 0) + return; + if (dev->adev.capture_pcm_substream) { substream = dev->adev.capture_pcm_substream; runtime = substream->runtime; @@ -217,31 +220,6 @@ static int em28xx_init_audio_isoc(struct em28xx *dev) return 0; } -static int em28xx_cmd(struct em28xx *dev, int cmd, int arg) -{ - dprintk("%s transfer\n", (dev->adev.capture_stream == STREAM_ON) ? - "stop" : "start"); - - switch (cmd) { - case EM28XX_CAPTURE_STREAM_EN: - if (dev->adev.capture_stream == STREAM_OFF && - arg == EM28XX_START_AUDIO) { - dev->adev.capture_stream = STREAM_ON; - em28xx_init_audio_isoc(dev); - } else if (dev->adev.capture_stream == STREAM_ON && - arg == EM28XX_STOP_AUDIO) { - dev->adev.capture_stream = STREAM_OFF; - em28xx_deinit_isoc_audio(dev); - } else { - em28xx_errdev("An underrun very likely occurred. " - "Ignoring it.\n"); - } - return 0; - default: - return -EINVAL; - } -} - static int snd_pcm_alloc_vmalloc_buffer(struct snd_pcm_substream *subs, size_t size) { @@ -303,7 +281,6 @@ static int snd_em28xx_capture_open(struct snd_pcm_substream *substream) dev->mute = 0; mutex_lock(&dev->lock); ret = em28xx_audio_analog_set(dev); - mutex_unlock(&dev->lock); if (ret < 0) goto err; @@ -311,11 +288,10 @@ static int snd_em28xx_capture_open(struct snd_pcm_substream *substream) if (dev->alt == 0 && dev->adev.users == 0) { int errCode; dev->alt = 7; - errCode = usb_set_interface(dev->udev, 0, 7); dprintk("changing alternate number to 7\n"); + errCode = usb_set_interface(dev->udev, 0, 7); } - mutex_lock(&dev->lock); dev->adev.users++; mutex_unlock(&dev->lock); @@ -325,6 +301,8 @@ static int snd_em28xx_capture_open(struct snd_pcm_substream *substream) return 0; err: + mutex_unlock(&dev->lock); + em28xx_err("Error while configuring em28xx mixer\n"); return ret; } @@ -338,6 +316,11 @@ static int snd_em28xx_pcm_close(struct snd_pcm_substream *substream) dev->mute = 1; mutex_lock(&dev->lock); dev->adev.users--; + if (atomic_read(&dev->stream_started) > 0) { + atomic_set(&dev->stream_started, 0); + schedule_work(&dev->wq_trigger); + } + em28xx_audio_analog_set(dev); if (substream->runtime->dma_area) { dprintk("freeing\n"); @@ -375,8 +358,10 @@ static int snd_em28xx_hw_capture_free(struct snd_pcm_substream *substream) dprintk("Stop capture, if needed\n"); - if (dev->adev.capture_stream == STREAM_ON) - em28xx_cmd(dev, EM28XX_CAPTURE_STREAM_EN, EM28XX_STOP_AUDIO); + if (atomic_read(&dev->stream_started) > 0) { + atomic_set(&dev->stream_started, 0); + schedule_work(&dev->wq_trigger); + } return 0; } @@ -391,31 +376,37 @@ static int snd_em28xx_prepare(struct snd_pcm_substream *substream) return 0; } +static void audio_trigger(struct work_struct *work) +{ + struct em28xx *dev = container_of(work, struct em28xx, wq_trigger); + + if (atomic_read(&dev->stream_started)) { + dprintk("starting capture"); + em28xx_init_audio_isoc(dev); + } else { + dprintk("stopping capture"); + em28xx_deinit_isoc_audio(dev); + } +} + static int snd_em28xx_capture_trigger(struct snd_pcm_substream *substream, int cmd) { struct em28xx *dev = snd_pcm_substream_chip(substream); int retval; - dprintk("Should %s capture\n", (cmd == SNDRV_PCM_TRIGGER_START) ? - "start" : "stop"); - - spin_lock(&dev->adev.slock); switch (cmd) { case SNDRV_PCM_TRIGGER_START: - em28xx_cmd(dev, EM28XX_CAPTURE_STREAM_EN, EM28XX_START_AUDIO); - retval = 0; + atomic_set(&dev->stream_started, 1); break; case SNDRV_PCM_TRIGGER_STOP: - em28xx_cmd(dev, EM28XX_CAPTURE_STREAM_EN, EM28XX_STOP_AUDIO); - retval = 0; + atomic_set(&dev->stream_started, 1); break; default: retval = -EINVAL; } - - spin_unlock(&dev->adev.slock); - return retval; + schedule_work(&dev->wq_trigger); + return 0; } static snd_pcm_uframes_t snd_em28xx_capture_pointer(struct snd_pcm_substream @@ -495,6 +486,8 @@ static int em28xx_audio_init(struct em28xx *dev) strcpy(card->shortname, "Em28xx Audio"); strcpy(card->longname, "Empia Em28xx Audio"); + INIT_WORK(&dev->wq_trigger, audio_trigger); + err = snd_card_register(card); if (err < 0) { snd_card_free(card); diff --git a/drivers/media/video/em28xx/em28xx.h b/drivers/media/video/em28xx/em28xx.h index 1c61a6b65d2..adb20eb6d90 100644 --- a/drivers/media/video/em28xx/em28xx.h +++ b/drivers/media/video/em28xx/em28xx.h @@ -25,12 +25,13 @@ #ifndef _EM28XX_H #define _EM28XX_H +#include +#include +#include #include + #include #include - -#include -#include #include #include #if defined(CONFIG_VIDEO_EM28XX_DVB) || defined(CONFIG_VIDEO_EM28XX_DVB_MODULE) @@ -184,11 +185,6 @@ enum em28xx_mode { EM28XX_DIGITAL_MODE, }; -enum em28xx_stream_state { - STREAM_OFF, - STREAM_INTERRUPT, - STREAM_ON, -}; struct em28xx; @@ -463,7 +459,6 @@ struct em28xx_audio { struct snd_card *sndcard; int users; - enum em28xx_stream_state capture_stream; spinlock_t slock; }; @@ -505,6 +500,10 @@ struct em28xx { unsigned int has_audio_class:1; unsigned int has_alsa_audio:1; + /* Controls audio streaming */ + struct work_struct wq_trigger; /* Trigger to start/stop audio for alsa module */ + atomic_t stream_started; /* stream should be running if true */ + struct em28xx_fmt *format; struct em28xx_IR *ir; -- 2.11.4.GIT