ALSA: usb-audio: remove is_playback from implicit feedback quirks

PATCH In review Eldad Zack 1 participant 4 messages Aug 3, 2013 Sound

Share & tools

Eldad Zack

author

2013-08-03 08:51 UTC #0 lore raw

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

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

Eldad Zack

author

2013-08-03 08:51 UTC #1 lore raw

PATCH v2 8/9 ALSA: usb-audio: remove implicit_fb from quirk

Since the quirks all apply to implicit feedback (the source endpoint is always a data endpoint), there's no need to set and check a flag for it.

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

---
 sound/usb/pcm.c | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

sound/usb/pcm.c +1 -7

index af30e08..b375d58 100644
@@ -334,7 +334,6 @@ static int set_sync_ep_implicit_fb_quirk(struct snd_usb_substream *subs,
 {
 	struct usb_host_interface *alts;
 	struct usb_interface *iface;
-	int implicit_fb = 0;
 	unsigned int ep;
 
 	/* Implicit feedback sync EPs consumers are always playback EPs */

@@ -344,7 +343,6 @@ static int set_sync_ep_implicit_fb_quirk(struct snd_usb_substream *subs,
 	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 */
-		implicit_fb = 1;
 		ep = 0x81;
 		iface = usb_ifnum_to_if(dev, 3);

@@ -356,7 +354,6 @@ static int set_sync_ep_implicit_fb_quirk(struct snd_usb_substream *subs,
 		break;
 	case USB_ID(0x0763, 0x2080): /* M-Audio FastTrack Ultra */
 	case USB_ID(0x0763, 0x2081):
-		implicit_fb = 1;
 		ep = 0x81;
 		iface = usb_ifnum_to_if(dev, 2);

@@ -374,7 +371,6 @@ static int set_sync_ep_implicit_fb_quirk(struct snd_usb_substream *subs,
 	    search_roland_implicit_fb(dev, altsd->bInterfaceNumber + 1,
 				      altsd->bAlternateSetting,
 				      &alts, &ep) >= 0) {
-		implicit_fb = 1;
 		goto add_sync_ep;
 	}

@@ -384,9 +380,7 @@ static int set_sync_ep_implicit_fb_quirk(struct snd_usb_substream *subs,
 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);
+						   SND_USB_ENDPOINT_TYPE_DATA);
 	if (!subs->sync_endpoint)
 		return -EINVAL;

Signature · 1 line

1.8.1.5

Eldad Zack

author

2013-08-03 08:51 UTC #2 lore raw

PATCH v2 9/9 ALSA: usb-audio: WARN_ON when alts is passed as NULL

In reply to Eldad Zack

Prevent NULL dereference in snd_usb_add_endpoints(), when alts is passed as NULL. In this case, WARN (since this is a non-fatal bug) and return NULL ep. Call sites treat a NULL return value as an error.

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

---
 sound/usb/endpoint.c | 3 +++
 1 file changed, 3 insertions(+)

sound/usb/endpoint.c +3 -0

index 7a444b5..92ea945 100644
@@ -418,6 +418,9 @@ struct snd_usb_endpoint *snd_usb_add_endpoint(struct snd_usb_audio *chip,
 	struct snd_usb_endpoint *ep;
 	int is_playback = direction == SNDRV_PCM_STREAM_PLAYBACK;
 
+	if (WARN_ON(!alts))
+		return NULL;
+
 	mutex_lock(&chip->mutex);
 
 	list_for_each_entry(ep, &chip->ep_list, list) {

Signature · 1 line

1.8.1.5

Eldad Zack

author

2013-08-03 09:05 UTC #3 lore raw

In reply to Eldad Zack

I had some error while sending, and this was sent twice.

Quoted 79 lines · Eldad Zack

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