ALSA: usx2y: Don't peep the card internal object

Avoid traversing the device object list of the card instance just for
checking the PCM streams.  The driver's private object already
contains the array of substream pointers, so it can be simply looked
through.  The card internal may be restructured in future, thus better
not to rely on it.

Also, this fixes the possible deadlocks in PCM mutex.  Instead of
taking multiple PCM mutexes, just take the common mutex in all
places.  Along with it, rename prepare_mutex as pcm_mutex.

Signed-off-by: Takashi Iwai <tiwai@suse.de>

diff --git a/sound/usb/usx2y/usbusx2y.c b/sound/usb/usx2y/usbusx2y.c
index e38c87302b15b017f2b8f24f6d2e9b2e931881ad..91e0e2a4808c99f8d9193342908343875f745fdd 100644 (file)
--- a/sound/usb/usx2y/usbusx2y.c
+++ b/sound/usb/usx2y/usbusx2y.c
@@ -353,7 +353,7 @@ static int usX2Y_create_card(struct usb_device *device,
        card->private_free = snd_usX2Y_card_private_free;
        usX2Y(card)->dev = device;
        init_waitqueue_head(&usX2Y(card)->prepare_wait_queue);
-       mutex_init(&usX2Y(card)->prepare_mutex);
+       mutex_init(&usX2Y(card)->pcm_mutex);
        INIT_LIST_HEAD(&usX2Y(card)->midi_list);
        strcpy(card->driver, "USB "NAME_ALLCAPS"");
        sprintf(card->shortname, "TASCAM "NAME_ALLCAPS"");

diff --git a/sound/usb/usx2y/usbusx2y.h b/sound/usb/usx2y/usbusx2y.h
index e43c0a86441ae600c2725356e1cbf3c27ecb81a3..6ae6b08069389a77b1a68e01b6d78a9b48998e75 100644 (file)
--- a/sound/usb/usx2y/usbusx2y.h
+++ b/sound/usb/usx2y/usbusx2y.h
@@ -36,7 +36,7 @@ struct usX2Ydev {
        unsigned int            rate,
                                format;
        int                     chip_status;
-       struct mutex            prepare_mutex;
+       struct mutex            pcm_mutex;
        struct us428ctls_sharedmem      *us428ctls_sharedmem;
        int                     wait_iso_frame;
        wait_queue_head_t       us428ctls_wait_queue_head;

diff --git a/sound/usb/usx2y/usbusx2yaudio.c b/sound/usb/usx2y/usbusx2yaudio.c
index 6234a51625b1b6a556f252c2fea3a150db1bdbc1..a63330dd1407e721285285a559c8b90adddd74c5 100644 (file)
--- a/sound/usb/usx2y/usbusx2yaudio.c
+++ b/sound/usb/usx2y/usbusx2yaudio.c
@@ -752,36 +752,44 @@ static int snd_usX2Y_pcm_hw_params(struct snd_pcm_substream *substream,
        unsigned int            rate = params_rate(hw_params);
        snd_pcm_format_t        format = params_format(hw_params);
        struct snd_card *card = substream->pstr->pcm->card;
-       struct list_head *list;
+       struct usX2Ydev *dev = usX2Y(card);
+       int i;
 
+       mutex_lock(&usX2Y(card)->pcm_mutex);
        snd_printdd("snd_usX2Y_hw_params(%p, %p)\n", substream, hw_params);
-       // all pcm substreams off one usX2Y have to operate at the same rate & format
-       list_for_each(list, &card->devices) {
-               struct snd_device *dev;
-               struct snd_pcm *pcm;
-               int s;
-               dev = snd_device(list);
-               if (dev->type != SNDRV_DEV_PCM)
+       /* all pcm substreams off one usX2Y have to operate at the same
+        * rate & format
+        */
+       for (i = 0; i < dev->pcm_devs * 2; i++) {
+               struct snd_usX2Y_substream *subs = dev->subs[i];
+               struct snd_pcm_substream *test_substream;
+
+               if (!subs)
+                       continue;
+               test_substream = subs->pcm_substream;
+               if (!test_substream || test_substream == substream ||
+                   !test_substream->runtime)
                        continue;
-               pcm = dev->device_data;
-               for (s = 0; s < 2; ++s) {
-                       struct snd_pcm_substream *test_substream;
-                       test_substream = pcm->streams[s].substream;
-                       if (test_substream && test_substream != substream  &&
-                           test_substream->runtime &&
-                           ((test_substream->runtime->format &&
-                             test_substream->runtime->format != format) ||
-                            (test_substream->runtime->rate &&
-                             test_substream->runtime->rate != rate)))
-                               return -EINVAL;
+               if ((test_substream->runtime->format &&
+                    test_substream->runtime->format != format) ||
+                   (test_substream->runtime->rate &&
+                    test_substream->runtime->rate != rate)) {
+                       err = -EINVAL;
+                       goto error;
                }
        }
-       if (0 > (err = snd_pcm_lib_malloc_pages(substream, params_buffer_bytes(hw_params)))) {
+
+       err = snd_pcm_lib_malloc_pages(substream,
+                                      params_buffer_bytes(hw_params));
+       if (err < 0) {
                snd_printk(KERN_ERR "snd_pcm_lib_malloc_pages(%p, %i) returned %i\n",
                           substream, params_buffer_bytes(hw_params), err);
-               return err;
+               goto error;
        }
-       return 0;
+
+ error:
+       mutex_unlock(&usX2Y(card)->pcm_mutex);
+       return err;
 }
 
 /*
@@ -791,7 +799,7 @@ static int snd_usX2Y_pcm_hw_free(struct snd_pcm_substream *substream)
 {
        struct snd_pcm_runtime *runtime = substream->runtime;
        struct snd_usX2Y_substream *subs = runtime->private_data;
-       mutex_lock(&subs->usX2Y->prepare_mutex);
+       mutex_lock(&subs->usX2Y->pcm_mutex);
        snd_printdd("snd_usX2Y_hw_free(%p)\n", substream);
 
        if (SNDRV_PCM_STREAM_PLAYBACK == substream->stream) {
@@ -812,7 +820,7 @@ static int snd_usX2Y_pcm_hw_free(struct snd_pcm_substream *substream)
                        usX2Y_urbs_release(subs);
                }
        }
-       mutex_unlock(&subs->usX2Y->prepare_mutex);
+       mutex_unlock(&subs->usX2Y->pcm_mutex);
        return snd_pcm_lib_free_pages(substream);
 }
 /*
@@ -829,7 +837,7 @@ static int snd_usX2Y_pcm_prepare(struct snd_pcm_substream *substream)
        int err = 0;
        snd_printdd("snd_usX2Y_pcm_prepare(%p)\n", substream);
 
-       mutex_lock(&usX2Y->prepare_mutex);
+       mutex_lock(&usX2Y->pcm_mutex);
        usX2Y_subs_prepare(subs);
 // Start hardware streams
 // SyncStream first....
@@ -849,7 +857,7 @@ static int snd_usX2Y_pcm_prepare(struct snd_pcm_substream *substream)
                err = usX2Y_urbs_start(subs);
 
  up_prepare_mutex:
-       mutex_unlock(&usX2Y->prepare_mutex);
+       mutex_unlock(&usX2Y->pcm_mutex);
        return err;
 }
 

diff --git a/sound/usb/usx2y/usx2yhwdeppcm.c b/sound/usb/usx2y/usx2yhwdeppcm.c
index 814d0e887c62e5c451c3ff7cc4b8f448c3a45007..90766a92e7fdf471e7f109def88bd21ebb50048f 100644 (file)
--- a/sound/usb/usx2y/usx2yhwdeppcm.c
+++ b/sound/usb/usx2y/usx2yhwdeppcm.c
@@ -358,7 +358,7 @@ static int snd_usX2Y_usbpcm_hw_free(struct snd_pcm_substream *substream)
        struct snd_pcm_runtime *runtime = substream->runtime;
        struct snd_usX2Y_substream *subs = runtime->private_data,
                *cap_subs2 = subs->usX2Y->subs[SNDRV_PCM_STREAM_CAPTURE + 2];
-       mutex_lock(&subs->usX2Y->prepare_mutex);
+       mutex_lock(&subs->usX2Y->pcm_mutex);
        snd_printdd("snd_usX2Y_usbpcm_hw_free(%p)\n", substream);
 
        if (SNDRV_PCM_STREAM_PLAYBACK == substream->stream) {
@@ -387,7 +387,7 @@ static int snd_usX2Y_usbpcm_hw_free(struct snd_pcm_substream *substream)
                                usX2Y_usbpcm_urbs_release(cap_subs2);
                }
        }
-       mutex_unlock(&subs->usX2Y->prepare_mutex);
+       mutex_unlock(&subs->usX2Y->pcm_mutex);
        return snd_pcm_lib_free_pages(substream);
 }
 
@@ -493,7 +493,7 @@ static int snd_usX2Y_usbpcm_prepare(struct snd_pcm_substream *substream)
                memset(usX2Y->hwdep_pcm_shm, 0, sizeof(struct snd_usX2Y_hwdep_pcm_shm));
        }
 
-       mutex_lock(&usX2Y->prepare_mutex);
+       mutex_lock(&usX2Y->pcm_mutex);
        usX2Y_subs_prepare(subs);
 // Start hardware streams
 // SyncStream first....
@@ -534,7 +534,7 @@ static int snd_usX2Y_usbpcm_prepare(struct snd_pcm_substream *substream)
                usX2Y->hwdep_pcm_shm->capture_iso_start = -1;
 
  up_prepare_mutex:
-       mutex_unlock(&usX2Y->prepare_mutex);
+       mutex_unlock(&usX2Y->pcm_mutex);
        return err;
 }
 
@@ -600,59 +600,30 @@ static struct snd_pcm_ops snd_usX2Y_usbpcm_ops =
 };
 
 
-static int usX2Y_pcms_lock_check(struct snd_card *card)
+static int usX2Y_pcms_busy_check(struct snd_card *card)
 {
-       struct list_head *list;
-       struct snd_device *dev;
-       struct snd_pcm *pcm;
-       int err = 0;
-       list_for_each(list, &card->devices) {
-               dev = snd_device(list);
-               if (dev->type != SNDRV_DEV_PCM)
-                       continue;
-               pcm = dev->device_data;
-               mutex_lock(&pcm->open_mutex);
-       }
-       list_for_each(list, &card->devices) {
-               int s;
-               dev = snd_device(list);
-               if (dev->type != SNDRV_DEV_PCM)
-                       continue;
-               pcm = dev->device_data;
-               for (s = 0; s < 2; ++s) {
-                       struct snd_pcm_substream *substream;
-                       substream = pcm->streams[s].substream;
-                       if (substream && SUBSTREAM_BUSY(substream))
-                               err = -EBUSY;
-               }
-       }
-       return err;
-}
-
+       struct usX2Ydev *dev = usX2Y(card);
+       int i;
 
-static void usX2Y_pcms_unlock(struct snd_card *card)
-{
-       struct list_head *list;
-       struct snd_device *dev;
-       struct snd_pcm *pcm;
-       list_for_each(list, &card->devices) {
-               dev = snd_device(list);
-               if (dev->type != SNDRV_DEV_PCM)
-                       continue;
-               pcm = dev->device_data;
-               mutex_unlock(&pcm->open_mutex);
+       for (i = 0; i < dev->pcm_devs * 2; i++) {
+               struct snd_usX2Y_substream *subs = dev->subs[i];
+               if (subs && subs->pcm_substream &&
+                   SUBSTREAM_BUSY(subs->pcm_substream))
+                       return -EBUSY;
        }
+       return 0;
 }
 
-
 static int snd_usX2Y_hwdep_pcm_open(struct snd_hwdep *hw, struct file *file)
 {
-       // we need to be the first 
        struct snd_card *card = hw->card;
-       int err = usX2Y_pcms_lock_check(card);
-       if (0 == err)
+       int err;
+
+       mutex_lock(&usX2Y(card)->pcm_mutex);
+       err = usX2Y_pcms_busy_check(card);
+       if (!err)
                usX2Y(card)->chip_status |= USX2Y_STAT_CHIP_MMAP_PCM_URBS;
-       usX2Y_pcms_unlock(card);
+       mutex_unlock(&usX2Y(card)->pcm_mutex);
        return err;
 }
 
@@ -660,10 +631,13 @@ static int snd_usX2Y_hwdep_pcm_open(struct snd_hwdep *hw, struct file *file)
 static int snd_usX2Y_hwdep_pcm_release(struct snd_hwdep *hw, struct file *file)
 {
        struct snd_card *card = hw->card;
-       int err = usX2Y_pcms_lock_check(card);
-       if (0 == err)
+       int err;
+
+       mutex_lock(&usX2Y(card)->pcm_mutex);
+       err = usX2Y_pcms_busy_check(card);
+       if (!err)
                usX2Y(hw->card)->chip_status &= ~USX2Y_STAT_CHIP_MMAP_PCM_URBS;
-       usX2Y_pcms_unlock(card);
+       mutex_unlock(&usX2Y(card)->pcm_mutex);
        return err;
 }