Skip to contentSkip to conversation

ALSA: usb-audio: Refactor set_format()

PATCH In review Eldad Zack 2 participants 9 messages Aug 3, 2013 – Aug 6, 2013 Sound

lore
Share & tools
lore

Eldad Zack

author
#0 lore raw

v2 cover ALSA: usb-audio: Refactor set_format()

Hi Takashi,

This is a low-priority series (applies against current for-next). This series refactors set_format() and moves the sync endpoint initilization code into separate function. The quirks for implicit feedback are moved to a separate function as well.

The intended result is code that is easier to read without function changes.

Changes from v2:
* Cleaned up the patchset, indentations and whitespace fixes.
* Patch #2: removed early returns when subs->sync_endpoint is already set. This change now follows original semantics.
* Patch #9: I first corrected it thanks to Clemens Ladisch's feedback, but then I wasn't sure if it improves anything, so I dropped it entirely.
* Dropped patch #10. Instead, added patch (now #9) to WARN_ON and return NULL in snd_usb_add_endpoint. Otherwise, the check should be done at each call site, and it "shouldn't happen" anyway.

Cheers,
Eldad

v1:
http://mailman.alsa-project.org/pipermail/alsa-devel/2013-July/064268.html

Eldad Zack (9):
ALSA: usb-audio: remove disabled debug code in set_format
ALSA: usb-audio: remove assignment from if condition
ALSA: usb-audio: separate sync endpoint setting from set_format
ALSA: usb-audio: move implicit fb quirks to separate function
ALSA: usb-audio: reverse condition logic in set_sync_endpoint
ALSA: usb-audio: do not initialize and check implicit_fb
ALSA: usb-audio: remove is_playback from implicit feedback quirks
ALSA: usb-audio: remove implicit_fb from quirk
ALSA: usb-audio: WARN_ON when alts is passed as NULL

 sound/usb/endpoint.c |   3 +
 sound/usb/pcm.c      | 243 +++++++++++++++++++++++++++++----------------------
 2 files changed, 141 insertions(+), 105 deletions(-)
Signature · 1 line
1.8.1.5
author
#1 lore raw

PATCH v2 1/9 ALSA: usb-audio: remove disabled debug code in set_format

Code block does not compile when enabled.

Signed-off-by Eldad Zack <[email protected]>
---
 sound/usb/pcm.c | 9 ---------
 1 file changed, 9 deletions(-)
sound/usb/pcm.c +0 -9
index 15b151e..3d3e8d1 100644
@@ -486,15 +486,6 @@ add_sync_ep:
 
 	snd_usb_set_format_quirk(subs, fmt);
 
-#if 0
-	printk(KERN_DEBUG
-	       "setting done: format = %d, rate = %d..%d, channels = %d\n",
-	       fmt->format, fmt->rate_min, fmt->rate_max, fmt->channels);
-	printk(KERN_DEBUG
-	       "  datapipe = 0x%0x, syncpipe = 0x%0x\n",
-	       subs->datapipe, subs->syncpipe);
-#endif
-
 	return 0;
 }
Signature · 1 line
1.8.1.5
author
#2 lore raw

PATCH v2 2/9 ALSA: usb-audio: remove assignment from if condition

In reply to Eldad Zack

Following general kernel style.

Signed-off-by Eldad Zack <[email protected]>
---
 sound/usb/pcm.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
sound/usb/pcm.c +2 -1
index 3d3e8d1..be5c7c2 100644
@@ -479,7 +479,8 @@ add_sync_ep:
 		subs->data_endpoint->sync_master = subs->sync_endpoint;
 	}
 
-	if ((err = snd_usb_init_pitch(subs->stream->chip, fmt->iface, alts, fmt)) < 0)
+	err = snd_usb_init_pitch(subs->stream->chip, fmt->iface, alts, fmt);
+	if (err < 0)
 		return err;
 
 	subs->cur_audiofmt = fmt;
Signature · 1 line
1.8.1.5
author
#3 lore raw

PATCH v2 3/9 ALSA: usb-audio: separate sync endpoint setting from set_format

In reply to Eldad Zack

Setting the sync endpoint currently takes up about half of set_format(). Move it to a dedicated function.
No functional change.

Signed-off-by Eldad Zack <[email protected]>
---
 sound/usb/pcm.c | 128 ++++++++++++++++++++++++++++++++------------------------
 1 file changed, 73 insertions(+), 55 deletions(-)
sound/usb/pcm.c +73 -55
index be5c7c2..e24ce7d 100644
@@ -327,64 +327,17 @@ static int search_roland_implicit_fb(struct usb_device *dev, int ifnum,
 	return 0;
 }
 
-/*
- * find a matching format and set up the interface
- */
-static int set_format(struct snd_usb_substream *subs, struct audioformat *fmt)
+
+static int set_sync_endpoint(struct snd_usb_substream *subs,
+			     struct audioformat *fmt,
+			     struct usb_device *dev,
+			     struct usb_host_interface *alts,
+			     struct usb_interface_descriptor *altsd)
 {
-	struct usb_device *dev = subs->dev;
-	struct usb_host_interface *alts;
-	struct usb_interface_descriptor *altsd;
 	struct usb_interface *iface;
-	unsigned int ep, attr;
 	int is_playback = subs->direction == SNDRV_PCM_STREAM_PLAYBACK;
-	int err, implicit_fb = 0;
-
-	iface = usb_ifnum_to_if(dev, fmt->iface);
-	if (WARN_ON(!iface))
-		return -EINVAL;
-	alts = &iface->altsetting[fmt->altset_idx];
-	altsd = get_iface_desc(alts);
-	if (WARN_ON(altsd->bAlternateSetting != fmt->altsetting))
-		return -EINVAL;
-
-	if (fmt == subs->cur_audiofmt)
-		return 0;
-
-	/* close the old interface */
-	if (subs->interface >= 0 && subs->interface != fmt->iface) {
-		err = usb_set_interface(subs->dev, subs->interface, 0);
-		if (err < 0) {
-			snd_printk(KERN_ERR "%d:%d:%d: return to setting 0 failed (%d)\n",
-				dev->devnum, fmt->iface, fmt->altsetting, err);
-			return -EIO;
-		}
-		subs->interface = -1;
-		subs->altset_idx = 0;
-	}
-
-	/* set interface */
-	if (subs->interface != fmt->iface ||
-	    subs->altset_idx != fmt->altset_idx) {
-		err = usb_set_interface(dev, fmt->iface, fmt->altsetting);
-		if (err < 0) {
-			snd_printk(KERN_ERR "%d:%d:%d: usb_set_interface failed (%d)\n",
-				   dev->devnum, fmt->iface, fmt->altsetting, err);
-			return -EIO;
-		}
-		snd_printdd(KERN_INFO "setting usb interface %d:%d\n",
-				fmt->iface, fmt->altsetting);
-		subs->interface = fmt->iface;
-		subs->altset_idx = fmt->altset_idx;
-
-		snd_usb_set_interface_quirk(dev);
-	}
-
-	subs->data_endpoint = snd_usb_add_endpoint(subs->stream->chip,
-						   alts, fmt->endpoint, subs->direction,
-						   SND_USB_ENDPOINT_TYPE_DATA);
-	if (!subs->data_endpoint)
-		return -EINVAL;
+	unsigned int ep, attr;
+	int implicit_fb = 0;
 
 	/* we need a sync pipe in async OUT or adaptive IN mode */
 	/* check the number of EP, since some devices have broken

@@ -479,6 +432,71 @@ add_sync_ep:
 		subs->data_endpoint->sync_master = subs->sync_endpoint;
 	}
 
+	return 0;
+}
+
+/*
+ * find a matching format and set up the interface
+ */
+static int set_format(struct snd_usb_substream *subs, struct audioformat *fmt)
+{
+	struct usb_device *dev = subs->dev;
+	struct usb_host_interface *alts;
+	struct usb_interface_descriptor *altsd;
+	struct usb_interface *iface;
+	int err;
+
+	iface = usb_ifnum_to_if(dev, fmt->iface);
+	if (WARN_ON(!iface))
+		return -EINVAL;
+	alts = &iface->altsetting[fmt->altset_idx];
+	altsd = get_iface_desc(alts);
+	if (WARN_ON(altsd->bAlternateSetting != fmt->altsetting))
+		return -EINVAL;
+
+	if (fmt == subs->cur_audiofmt)
+		return 0;
+
+	/* close the old interface */
+	if (subs->interface >= 0 && subs->interface != fmt->iface) {
+		err = usb_set_interface(subs->dev, subs->interface, 0);
+		if (err < 0) {
+			snd_printk(KERN_ERR "%d:%d:%d: return to setting 0 failed (%d)\n",
+				dev->devnum, fmt->iface, fmt->altsetting, err);
+			return -EIO;
+		}
+		subs->interface = -1;
+		subs->altset_idx = 0;
+	}
+
+	/* set interface */
+	if (subs->interface != fmt->iface ||
+	    subs->altset_idx != fmt->altset_idx) {
+		err = usb_set_interface(dev, fmt->iface, fmt->altsetting);
+		if (err < 0) {
+			snd_printk(KERN_ERR "%d:%d:%d: usb_set_interface failed (%d)\n",
+				   dev->devnum, fmt->iface, fmt->altsetting, err);
+			return -EIO;
+		}
+		snd_printdd(KERN_INFO "setting usb interface %d:%d\n",
+				fmt->iface, fmt->altsetting);
+		subs->interface = fmt->iface;
+		subs->altset_idx = fmt->altset_idx;
+
+		snd_usb_set_interface_quirk(dev);
+	}
+
+	subs->data_endpoint = snd_usb_add_endpoint(subs->stream->chip,
+						   alts, fmt->endpoint, subs->direction,
+						   SND_USB_ENDPOINT_TYPE_DATA);
+
+	if (!subs->data_endpoint)
+		return -EINVAL;
+
+	err = set_sync_endpoint(subs, fmt, dev, alts, altsd);
+	if (err < 0)
+		return err;
+
 	err = snd_usb_init_pitch(subs->stream->chip, fmt->iface, alts, fmt);
 	if (err < 0)
 		return err;
Signature · 1 line
1.8.1.5
author
#4 lore raw

PATCH v2 4/9 ALSA: usb-audio: move implicit fb quirks to separate function

In reply to Eldad Zack

Separate setting implicit feedback quirks from setting a sync endpoint (which may also be explicit feedback or async).

Signed-off-by Eldad Zack <[email protected]>

---
v2: removed check to return early if subs->sync_endpoint is already set, as this is a functional change (although it is initialized to NULL and set to NULL on disconnect).

---
 sound/usb/pcm.c | 60 ++++++++++++++++++++++++++++++++++++++++++---------------
 1 file changed, 45 insertions(+), 15 deletions(-)
sound/usb/pcm.c +45 -15
index e24ce7d..0016f28 100644
@@ -327,24 +327,16 @@ static int search_roland_implicit_fb(struct usb_device *dev, int ifnum,
 	return 0;
 }
 
-
-static int set_sync_endpoint(struct snd_usb_substream *subs,
-			     struct audioformat *fmt,
-			     struct usb_device *dev,
-			     struct usb_host_interface *alts,
-			     struct usb_interface_descriptor *altsd)
+static int set_sync_ep_implicit_fb_quirk(struct snd_usb_substream *subs,
+					 struct usb_device *dev,
+					 struct usb_interface_descriptor *altsd,
+					 unsigned int attr)
 {
+	struct usb_host_interface *alts;
 	struct usb_interface *iface;
 	int is_playback = subs->direction == SNDRV_PCM_STREAM_PLAYBACK;
-	unsigned int ep, attr;
 	int implicit_fb = 0;
-
-	/* we need a sync pipe in async OUT or adaptive IN mode */
-	/* check the number of EP, since some devices have broken
-	 * descriptors which fool us.  if it has only one EP,
-	 * assume it as adaptive-out or sync-in.
-	 */
-	attr = fmt->ep_attr & USB_ENDPOINT_SYNCTYPE;
+	unsigned int ep;
 
 	switch (subs->stream->chip->usb_id) {
 	case USB_ID(0x0763, 0x2030): /* M-Audio Fast Track C400 */

@@ -388,6 +380,45 @@ static int set_sync_endpoint(struct snd_usb_substream *subs,
 		goto add_sync_ep;
 	}
 
+	/* No quirk */
+	return 0;
+
+add_sync_ep:
+	subs->sync_endpoint = snd_usb_add_endpoint(subs->stream->chip,
+						   alts, ep, !subs->direction,
+						   implicit_fb ?
+							SND_USB_ENDPOINT_TYPE_DATA :
+							SND_USB_ENDPOINT_TYPE_SYNC);
+	if (!subs->sync_endpoint)
+		return -EINVAL;
+
+	subs->data_endpoint->sync_master = subs->sync_endpoint;
+
+	return 0;
+}
+
+static int set_sync_endpoint(struct snd_usb_substream *subs,
+			     struct audioformat *fmt,
+			     struct usb_device *dev,
+			     struct usb_host_interface *alts,
+			     struct usb_interface_descriptor *altsd)
+{
+	int is_playback = subs->direction == SNDRV_PCM_STREAM_PLAYBACK;
+	unsigned int ep, attr;
+	int implicit_fb = 0;
+	int err;
+
+	/* we need a sync pipe in async OUT or adaptive IN mode */
+	/* check the number of EP, since some devices have broken
+	 * descriptors which fool us.  if it has only one EP,
+	 * assume it as adaptive-out or sync-in.
+	 */
+	attr = fmt->ep_attr & USB_ENDPOINT_SYNCTYPE;
+
+	err = set_sync_ep_implicit_fb_quirk(subs, dev, altsd, attr);
+	if (err < 0)
+		return err;
+
 	if (((is_playback && attr == USB_ENDPOINT_SYNC_ASYNC) ||
 	     (!is_playback && attr == USB_ENDPOINT_SYNC_ADAPTIVE)) &&
 	    altsd->bNumEndpoints >= 2) {

@@ -420,7 +451,6 @@ static int set_sync_endpoint(struct snd_usb_substream *subs,
 		implicit_fb = (get_endpoint(alts, 1)->bmAttributes & USB_ENDPOINT_USAGE_MASK)
 				== USB_ENDPOINT_USAGE_IMPLICIT_FB;
 
-add_sync_ep:
 		subs->sync_endpoint = snd_usb_add_endpoint(subs->stream->chip,
 							   alts, ep, !subs->direction,
 							   implicit_fb ?
Signature · 1 line
1.8.1.5
author
#5 lore raw

PATCH v2 5/9 ALSA: usb-audio: reverse condition logic in set_sync_endpoint

In reply to Eldad Zack

Reverse logic on the conditions required to qualify for a sync endpoint and remove one level of indendation.

Signed-off-by Eldad Zack <[email protected]>

---
v2: identation and whitespace corrections

---
 sound/usb/pcm.c | 81 ++++++++++++++++++++++++++++++---------------------------
 1 file changed, 42 insertions(+), 39 deletions(-)
sound/usb/pcm.c +42 -39
index 0016f28..c31dbdc 100644
@@ -419,49 +419,52 @@ static int set_sync_endpoint(struct snd_usb_substream *subs,
 	if (err < 0)
 		return err;
 
-	if (((is_playback && attr == USB_ENDPOINT_SYNC_ASYNC) ||
-	     (!is_playback && attr == USB_ENDPOINT_SYNC_ADAPTIVE)) &&
-	    altsd->bNumEndpoints >= 2) {
-		/* check sync-pipe endpoint */
-		/* ... and check descriptor size before accessing bSynchAddress
-		   because there is a version of the SB Audigy 2 NX firmware lacking
-		   the audio fields in the endpoint descriptors */
-		if ((get_endpoint(alts, 1)->bmAttributes & USB_ENDPOINT_XFERTYPE_MASK) != USB_ENDPOINT_XFER_ISOC ||
-		    (get_endpoint(alts, 1)->bLength >= USB_DT_ENDPOINT_AUDIO_SIZE &&
-		     get_endpoint(alts, 1)->bSynchAddress != 0 &&
-		     !implicit_fb)) {
-			snd_printk(KERN_ERR "%d:%d:%d : invalid sync pipe. bmAttributes %02x, bLength %d, bSynchAddress %02x\n",
-				   dev->devnum, fmt->iface, fmt->altsetting,
-				   get_endpoint(alts, 1)->bmAttributes,
-				   get_endpoint(alts, 1)->bLength,
-				   get_endpoint(alts, 1)->bSynchAddress);
-			return -EINVAL;
-		}
-		ep = get_endpoint(alts, 1)->bEndpointAddress;
-		if (!implicit_fb &&
-		    get_endpoint(alts, 0)->bLength >= USB_DT_ENDPOINT_AUDIO_SIZE &&
-		    (( is_playback && ep != (unsigned int)(get_endpoint(alts, 0)->bSynchAddress | USB_DIR_IN)) ||
-		     (!is_playback && ep != (unsigned int)(get_endpoint(alts, 0)->bSynchAddress & ~USB_DIR_IN)))) {
-			snd_printk(KERN_ERR "%d:%d:%d : invalid sync pipe. is_playback %d, ep %02x, bSynchAddress %02x\n",
-				   dev->devnum, fmt->iface, fmt->altsetting,
-				   is_playback, ep, get_endpoint(alts, 0)->bSynchAddress);
-			return -EINVAL;
-		}
-
-		implicit_fb = (get_endpoint(alts, 1)->bmAttributes & USB_ENDPOINT_USAGE_MASK)
-				== USB_ENDPOINT_USAGE_IMPLICIT_FB;
+	if (altsd->bNumEndpoints < 2)
+		return 0;
 
-		subs->sync_endpoint = snd_usb_add_endpoint(subs->stream->chip,
-							   alts, ep, !subs->direction,
-							   implicit_fb ?
-								SND_USB_ENDPOINT_TYPE_DATA :
-								SND_USB_ENDPOINT_TYPE_SYNC);
-		if (!subs->sync_endpoint)
-			return -EINVAL;
+	if ((is_playback && attr != USB_ENDPOINT_SYNC_ASYNC) ||
+	    (!is_playback && attr != USB_ENDPOINT_SYNC_ADAPTIVE))
+		return 0;
 
-		subs->data_endpoint->sync_master = subs->sync_endpoint;
+	/* check sync-pipe endpoint */
+	/* ... and check descriptor size before accessing bSynchAddress
+	   because there is a version of the SB Audigy 2 NX firmware lacking
+	   the audio fields in the endpoint descriptors */
+	if ((get_endpoint(alts, 1)->bmAttributes & USB_ENDPOINT_XFERTYPE_MASK) != USB_ENDPOINT_XFER_ISOC ||
+	    (get_endpoint(alts, 1)->bLength >= USB_DT_ENDPOINT_AUDIO_SIZE &&
+	     get_endpoint(alts, 1)->bSynchAddress != 0 &&
+	     !implicit_fb)) {
+		snd_printk(KERN_ERR "%d:%d:%d : invalid sync pipe. bmAttributes %02x, bLength %d, bSynchAddress %02x\n",
+			   dev->devnum, fmt->iface, fmt->altsetting,
+			   get_endpoint(alts, 1)->bmAttributes,
+			   get_endpoint(alts, 1)->bLength,
+			   get_endpoint(alts, 1)->bSynchAddress);
+		return -EINVAL;
+	}
+	ep = get_endpoint(alts, 1)->bEndpointAddress;
+	if (!implicit_fb &&
+	    get_endpoint(alts, 0)->bLength >= USB_DT_ENDPOINT_AUDIO_SIZE &&
+	    ((is_playback && ep != (unsigned int)(get_endpoint(alts, 0)->bSynchAddress | USB_DIR_IN)) ||
+	     (!is_playback && ep != (unsigned int)(get_endpoint(alts, 0)->bSynchAddress & ~USB_DIR_IN)))) {
+		snd_printk(KERN_ERR "%d:%d:%d : invalid sync pipe. is_playback %d, ep %02x, bSynchAddress %02x\n",
+			   dev->devnum, fmt->iface, fmt->altsetting,
+			   is_playback, ep, get_endpoint(alts, 0)->bSynchAddress);
+		return -EINVAL;
 	}
 
+	implicit_fb = (get_endpoint(alts, 1)->bmAttributes & USB_ENDPOINT_USAGE_MASK)
+			== USB_ENDPOINT_USAGE_IMPLICIT_FB;
+
+	subs->sync_endpoint = snd_usb_add_endpoint(subs->stream->chip,
+						   alts, ep, !subs->direction,
+						   implicit_fb ?
+							SND_USB_ENDPOINT_TYPE_DATA :
+							SND_USB_ENDPOINT_TYPE_SYNC);
+	if (!subs->sync_endpoint)
+		return -EINVAL;
+
+	subs->data_endpoint->sync_master = subs->sync_endpoint;
+
 	return 0;
 }
Signature · 1 line
1.8.1.5
author
#6 lore raw

PATCH v2 6/9 ALSA: usb-audio: do not initialize and check implicit_fb

In reply to Eldad Zack

Since implicit_fb is not changed, !implicit_fb will always be true - it is set only after these checks.
Similarly, there's also no need to set it at the top of the function.

Change the type of implicit_fb to bool (more appropriate).

Signed-off-by Eldad Zack <[email protected]>

---
v2: implicit_fb type set to bool

---
 sound/usb/pcm.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)
sound/usb/pcm.c +3 -5
index c31dbdc..bb2e0f5 100644
@@ -405,7 +405,7 @@ static int set_sync_endpoint(struct snd_usb_substream *subs,
 {
 	int is_playback = subs->direction == SNDRV_PCM_STREAM_PLAYBACK;
 	unsigned int ep, attr;
-	int implicit_fb = 0;
+	bool implicit_fb;
 	int err;
 
 	/* we need a sync pipe in async OUT or adaptive IN mode */

@@ -432,8 +432,7 @@ static int set_sync_endpoint(struct snd_usb_substream *subs,
 	   the audio fields in the endpoint descriptors */
 	if ((get_endpoint(alts, 1)->bmAttributes & USB_ENDPOINT_XFERTYPE_MASK) != USB_ENDPOINT_XFER_ISOC ||
 	    (get_endpoint(alts, 1)->bLength >= USB_DT_ENDPOINT_AUDIO_SIZE &&
-	     get_endpoint(alts, 1)->bSynchAddress != 0 &&
-	     !implicit_fb)) {
+	     get_endpoint(alts, 1)->bSynchAddress != 0)) {
 		snd_printk(KERN_ERR "%d:%d:%d : invalid sync pipe. bmAttributes %02x, bLength %d, bSynchAddress %02x\n",
 			   dev->devnum, fmt->iface, fmt->altsetting,
 			   get_endpoint(alts, 1)->bmAttributes,

@@ -442,8 +441,7 @@ static int set_sync_endpoint(struct snd_usb_substream *subs,
 		return -EINVAL;
 	}
 	ep = get_endpoint(alts, 1)->bEndpointAddress;
-	if (!implicit_fb &&
-	    get_endpoint(alts, 0)->bLength >= USB_DT_ENDPOINT_AUDIO_SIZE &&
+	if (get_endpoint(alts, 0)->bLength >= USB_DT_ENDPOINT_AUDIO_SIZE &&
 	    ((is_playback && ep != (unsigned int)(get_endpoint(alts, 0)->bSynchAddress | USB_DIR_IN)) ||
 	     (!is_playback && ep != (unsigned int)(get_endpoint(alts, 0)->bSynchAddress & ~USB_DIR_IN)))) {
 		snd_printk(KERN_ERR "%d:%d:%d : invalid sync pipe. is_playback %d, ep %02x, bSynchAddress %02x\n",
Signature · 1 line
1.8.1.5
author
#7 lore raw

PATCH v2 7/9 ALSA: usb-audio: remove is_playback from implicit feedback quirks

In reply to Eldad Zack

An implicit feedback endpoint can only be a capture source. The consumer (sink) of the implicit feedback endpoint is therefore limited to playback EPs.
Check if the target endpoint is a playback first and remove redundant checks.

Signed-off-by Eldad Zack <[email protected]>
---
 sound/usb/pcm.c | 40 +++++++++++++++++++---------------------
 1 file changed, 19 insertions(+), 21 deletions(-)
sound/usb/pcm.c +19 -21
index bb2e0f5..af30e08 100644
@@ -334,41 +334,39 @@ static int set_sync_ep_implicit_fb_quirk(struct snd_usb_substream *subs,
 {
 	struct usb_host_interface *alts;
 	struct usb_interface *iface;
-	int is_playback = subs->direction == SNDRV_PCM_STREAM_PLAYBACK;
 	int implicit_fb = 0;
 	unsigned int ep;
 
+	/* Implicit feedback sync EPs consumers are always playback EPs */
+	if (subs->direction != SNDRV_PCM_STREAM_PLAYBACK)
+		return 0;
+
 	switch (subs->stream->chip->usb_id) {
 	case USB_ID(0x0763, 0x2030): /* M-Audio Fast Track C400 */
 	case USB_ID(0x0763, 0x2031): /* M-Audio Fast Track C600 */
-		if (is_playback) {
-			implicit_fb = 1;
-			ep = 0x81;
-			iface = usb_ifnum_to_if(dev, 3);
+		implicit_fb = 1;
+		ep = 0x81;
+		iface = usb_ifnum_to_if(dev, 3);
 
-			if (!iface || iface->num_altsetting == 0)
-				return -EINVAL;
+		if (!iface || iface->num_altsetting == 0)
+			return -EINVAL;
 
-			alts = &iface->altsetting[1];
-			goto add_sync_ep;
-		}
+		alts = &iface->altsetting[1];
+		goto add_sync_ep;
 		break;
 	case USB_ID(0x0763, 0x2080): /* M-Audio FastTrack Ultra */
 	case USB_ID(0x0763, 0x2081):
-		if (is_playback) {
-			implicit_fb = 1;
-			ep = 0x81;
-			iface = usb_ifnum_to_if(dev, 2);
+		implicit_fb = 1;
+		ep = 0x81;
+		iface = usb_ifnum_to_if(dev, 2);
 
-			if (!iface || iface->num_altsetting == 0)
-				return -EINVAL;
+		if (!iface || iface->num_altsetting == 0)
+			return -EINVAL;
 
-			alts = &iface->altsetting[1];
-			goto add_sync_ep;
-		}
+		alts = &iface->altsetting[1];
+		goto add_sync_ep;
 	}
-	if (is_playback &&
-	    attr == USB_ENDPOINT_SYNC_ASYNC &&
+	if (attr == USB_ENDPOINT_SYNC_ASYNC &&
 	    altsd->bInterfaceClass == USB_CLASS_VENDOR_SPEC &&
 	    altsd->bInterfaceProtocol == 2 &&
 	    altsd->bNumEndpoints == 1 &&
Signature · 1 line
1.8.1.5
#8 lore raw
In reply to Eldad Zack

At Sat, 3 Aug 2013 10:50:14 +0200,

Quoted 11 lines · Eldad Zack

Hi Takashi,

This is a low-priority series (applies against current for-next). This series refactors set_format() and moves the sync endpoint initilization code into separate function. The quirks for implicit feedback are moved to a separate function as well.

The intended result is code that is easier to read without function changes.

Quoted 9 lines

Changes from v2:
* Cleaned up the patchset, indentations and whitespace fixes.
* Patch #2: removed early returns when subs->sync_endpoint is already set. This change now follows original semantics.
* Patch #9: I first corrected it thanks to Clemens Ladisch's feedback, but then I wasn't sure if it improves anything, so I dropped it entirely.
* Dropped patch #10. Instead, added patch (now #9) to WARN_ON and return NULL in snd_usb_add_endpoint. Otherwise, the check should be done at each call site, and it "shouldn't happen" anyway.

Thanks, now I applied all patches to for-next branch.

Takashi

Quoted 25 lines

Cheers,
Eldad

v1:
http://mailman.alsa-project.org/pipermail/alsa-devel/2013-July/064268.html

Eldad Zack (9):
ALSA: usb-audio: remove disabled debug code in set_format
ALSA: usb-audio: remove assignment from if condition
ALSA: usb-audio: separate sync endpoint setting from set_format
ALSA: usb-audio: move implicit fb quirks to separate function
ALSA: usb-audio: reverse condition logic in set_sync_endpoint
ALSA: usb-audio: do not initialize and check implicit_fb
ALSA: usb-audio: remove is_playback from implicit feedback quirks
ALSA: usb-audio: remove implicit_fb from quirk
ALSA: usb-audio: WARN_ON when alts is passed as NULL

 sound/usb/endpoint.c |   3 +
 sound/usb/pcm.c      | 243 +++++++++++++++++++++++++++++----------------------
 2 files changed, 141 insertions(+), 105 deletions(-)
Signature · 1 line
1.8.1.5