[21/41] ALSA: usb-audio: Stop both endpoints properly at error

Commit Message

Takashi Iwai Nov. 23, 2020, 8:53 a.m. UTC

start_endpoints() may leave the data endpoint running if an error
happens at starting the sync endpoint.  We should stop both streams
properly, instead.

While we're at it, move the debug prints into the endpoint.c that is a
more suitable place.

Tested-by: Keith Milner <[e-mail]>
Tested-by: Dylan Robinson <[e-mail]>
Signed-off-by: Takashi Iwai <[e-mail]>
---
 sound/usb/endpoint.c |  6 ++++++
 sound/usb/pcm.c      | 39 +++++++++++++++++----------------------
 2 files changed, 23 insertions(+), 22 deletions(-)

Patch

diff --git a/sound/usb/endpoint.c b/sound/usb/endpoint.c
index eb459db511f8..0cc7e9c01263 100644
--- a/sound/usb/endpoint.c
+++ b/sound/usb/endpoint.c
@@ -1172,6 +1172,9 @@  int snd_usb_endpoint_start(struct snd_usb_endpoint *ep)
 	if (atomic_read(&ep->chip->shutdown))
 		return -EBADFD;
 
+	usb_audio_dbg(ep->chip, "Starting %s EP 0x%x (count %d)\n",
+		      ep_type_name(ep->type), ep->ep_num, ep->use_count);
+
 	/* already running? */
 	if (++ep->use_count != 1)
 		return 0;
@@ -1254,6 +1257,9 @@  void snd_usb_endpoint_stop(struct snd_usb_endpoint *ep)
 	if (!ep)
 		return;
 
+	usb_audio_dbg(ep->chip, "Stopping %s EP 0x%x (count %d)\n",
+		      ep_type_name(ep->type), ep->ep_num, ep->use_count);
+
 	if (snd_BUG_ON(ep->use_count == 0))
 		return;
 
diff --git a/sound/usb/pcm.c b/sound/usb/pcm.c
index 0998be109af3..c4e39aa92a84 100644
--- a/sound/usb/pcm.c
+++ b/sound/usb/pcm.c
@@ -211,6 +211,17 @@  int snd_usb_init_pitch(struct snd_usb_audio *chip,
 	}
 }
 
+static void stop_endpoints(struct snd_usb_substream *subs)
+{
+	if (test_and_clear_bit(SUBSTREAM_FLAG_SYNC_EP_STARTED, &subs->flags)) {
+		snd_usb_endpoint_stop(subs->sync_endpoint);
+		subs->sync_endpoint->sync_slave = NULL;
+	}
+
+	if (test_and_clear_bit(SUBSTREAM_FLAG_DATA_EP_STARTED, &subs->flags))
+		snd_usb_endpoint_stop(subs->data_endpoint);
+}
+
 static int start_endpoints(struct snd_usb_substream *subs)
 {
 	int err;
@@ -221,13 +232,11 @@  static int start_endpoints(struct snd_usb_substream *subs)
 	if (!test_and_set_bit(SUBSTREAM_FLAG_DATA_EP_STARTED, &subs->flags)) {
 		struct snd_usb_endpoint *ep = subs->data_endpoint;
 
-		dev_dbg(&subs->dev->dev, "Starting data EP 0x%x\n", ep->ep_num);
-
 		ep->data_subs = subs;
 		err = snd_usb_endpoint_start(ep);
 		if (err < 0) {
 			clear_bit(SUBSTREAM_FLAG_DATA_EP_STARTED, &subs->flags);
-			return err;
+			goto error;
 		}
 	}
 
@@ -235,18 +244,20 @@  static int start_endpoints(struct snd_usb_substream *subs)
 	    !test_and_set_bit(SUBSTREAM_FLAG_SYNC_EP_STARTED, &subs->flags)) {
 		struct snd_usb_endpoint *ep = subs->sync_endpoint;
 
-		dev_dbg(&subs->dev->dev, "Starting sync EP 0x%x\n", ep->ep_num);
-
 		ep->sync_slave = subs->data_endpoint;
 		err = snd_usb_endpoint_start(ep);
 		if (err < 0) {
 			clear_bit(SUBSTREAM_FLAG_SYNC_EP_STARTED, &subs->flags);
 			ep->sync_slave = NULL;
-			return err;
+			goto error;
 		}
 	}
 
 	return 0;
+
+ error:
+	stop_endpoints(subs);
+	return err;
 }
 
 static void sync_pending_stops(struct snd_usb_substream *subs)
@@ -255,22 +266,6 @@  static void sync_pending_stops(struct snd_usb_substream *subs)
 	snd_usb_endpoint_sync_pending_stop(subs->data_endpoint);
 }
 
-static void stop_endpoints(struct snd_usb_substream *subs)
-{
-	if (test_and_clear_bit(SUBSTREAM_FLAG_SYNC_EP_STARTED, &subs->flags)) {
-		dev_dbg(&subs->dev->dev, "Stopping sync EP 0x%x\n",
-			subs->sync_endpoint->ep_num);
-		snd_usb_endpoint_stop(subs->sync_endpoint);
-		subs->sync_endpoint->sync_slave = NULL;
-	}
-
-	if (test_and_clear_bit(SUBSTREAM_FLAG_DATA_EP_STARTED, &subs->flags)) {
-		dev_dbg(&subs->dev->dev, "Stopping data EP 0x%x\n",
-			subs->data_endpoint->ep_num);
-		snd_usb_endpoint_stop(subs->data_endpoint);
-	}
-}
-
 /* PCM sync_stop callback */
 static int snd_usb_pcm_sync_stop(struct snd_pcm_substream *substream)
 {