Skip to content

Commit

Permalink
ALSA: snd-usb: move calls to usb_set_interface
Browse files Browse the repository at this point in the history 
The rework of the snd-usb endpoint logic moved the calls to
snd_usb_set_interface() into the snd_usb_endpoint implemenation. This
changed the order in which these calls are issued to the device, and
thereby caused regressions for some webcams.

Fix this by moving the calls back to pcm.c for now to make it work again
and use snd_usb_endpoint_activate() to really tear down all remaining
URBs in the flight, consequently fixing another regression caused by USB
packets on the wire after altsetting 0 has been selected.

Signed-off-by: Daniel Mack <zonque@gmail.com>
Reported-and-tested-by: Philipp Dreimann <philipp@dreimann.net>
Reported-by: Joseph Salisbury <joseph.salisbury@canonical.com>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
  • Loading branch information
Daniel Mack authored and Takashi Iwai committed Jul 13, 2012
1 parent 9e9b594 commit 68e67f4
Show file tree
Hide file tree
Showing 2 changed files with 41 additions and 89 deletions.
73 changes: 6 additions & 67 deletions sound/usb/endpoint.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -414,7 +414,7 @@ struct snd_usb_endpoint *snd_usb_add_endpoint(struct snd_usb_audio *chip,
{
struct list_head *p;
struct snd_usb_endpoint *ep;
int ret, is_playback = direction == SNDRV_PCM_STREAM_PLAYBACK;
int is_playback = direction == SNDRV_PCM_STREAM_PLAYBACK;

mutex_lock(&chip->mutex);

Expand All @@ -434,16 +434,6 @@ struct snd_usb_endpoint *snd_usb_add_endpoint(struct snd_usb_audio *chip,
type == SND_USB_ENDPOINT_TYPE_DATA ? "data" : "sync",
ep_num);

/* select the alt setting once so the endpoints become valid */
ret = usb_set_interface(chip->dev, alts->desc.bInterfaceNumber,
alts->desc.bAlternateSetting);
if (ret < 0) {
snd_printk(KERN_ERR "%s(): usb_set_interface() failed, ret = %d\n",
__func__, ret);
ep = NULL;
goto __exit_unlock;
}

ep = kzalloc(sizeof(*ep), GFP_KERNEL);
if (!ep)
goto __exit_unlock;
Expand Down Expand Up @@ -831,9 +821,6 @@ int snd_usb_endpoint_start(struct snd_usb_endpoint *ep)
if (++ep->use_count != 1)
return 0;

if (snd_BUG_ON(!test_bit(EP_FLAG_ACTIVATED, &ep->flags)))
return -EINVAL;

/* just to be sure */
deactivate_urbs(ep, 0, 1);
wait_clear_urbs(ep);
Expand Down Expand Up @@ -911,9 +898,6 @@ void snd_usb_endpoint_stop(struct snd_usb_endpoint *ep,
if (snd_BUG_ON(ep->use_count == 0))
return;

if (snd_BUG_ON(!test_bit(EP_FLAG_ACTIVATED, &ep->flags)))
return;

if (--ep->use_count == 0) {
deactivate_urbs(ep, force, can_sleep);
ep->data_subs = NULL;
Expand All @@ -926,42 +910,6 @@ void snd_usb_endpoint_stop(struct snd_usb_endpoint *ep,
}
}

/**
* snd_usb_endpoint_activate: activate an snd_usb_endpoint
*
* @ep: the endpoint to activate
*
* If the endpoint is not currently in use, this functions will select the
* correct alternate interface setting for the interface of this endpoint.
*
* In case of any active users, this functions does nothing.
*
* Returns an error if usb_set_interface() failed, 0 in all other
* cases.
*/
int snd_usb_endpoint_activate(struct snd_usb_endpoint *ep)
{
if (ep->use_count != 0)
return 0;

if (!ep->chip->shutdown &&
!test_and_set_bit(EP_FLAG_ACTIVATED, &ep->flags)) {
int ret;

ret = usb_set_interface(ep->chip->dev, ep->iface, ep->alt_idx);
if (ret < 0) {
snd_printk(KERN_ERR "%s() usb_set_interface() failed, ret = %d\n",
__func__, ret);
clear_bit(EP_FLAG_ACTIVATED, &ep->flags);
return ret;
}

return 0;
}

return -EBUSY;
}

/**
* snd_usb_endpoint_deactivate: deactivate an snd_usb_endpoint
*
Expand All @@ -980,24 +928,15 @@ int snd_usb_endpoint_deactivate(struct snd_usb_endpoint *ep)
if (!ep)
return -EINVAL;

deactivate_urbs(ep, 1, 1);
wait_clear_urbs(ep);

if (ep->use_count != 0)
return 0;

if (!ep->chip->shutdown &&
test_and_clear_bit(EP_FLAG_ACTIVATED, &ep->flags)) {
int ret;

ret = usb_set_interface(ep->chip->dev, ep->iface, 0);
if (ret < 0) {
snd_printk(KERN_ERR "%s(): usb_set_interface() failed, ret = %d\n",
__func__, ret);
return ret;
}
clear_bit(EP_FLAG_ACTIVATED, &ep->flags);

return 0;
}

return -EBUSY;
return 0;
}

/**
Expand Down
57 changes: 35 additions & 22 deletions sound/usb/pcm.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -261,19 +261,6 @@ static void stop_endpoints(struct snd_usb_substream *subs,
force, can_sleep, wait);
}

static int activate_endpoints(struct snd_usb_substream *subs)
{
if (subs->sync_endpoint) {
int ret;

ret = snd_usb_endpoint_activate(subs->sync_endpoint);
if (ret < 0)
return ret;
}

return snd_usb_endpoint_activate(subs->data_endpoint);
}

static int deactivate_endpoints(struct snd_usb_substream *subs)
{
int reta, retb;
Expand Down Expand Up @@ -314,6 +301,33 @@ static int set_format(struct snd_usb_substream *subs, struct audioformat *fmt)
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;
}

subs->data_endpoint = snd_usb_add_endpoint(subs->stream->chip,
alts, fmt->endpoint, subs->direction,
SND_USB_ENDPOINT_TYPE_DATA);
Expand Down Expand Up @@ -460,12 +474,6 @@ static int snd_usb_hw_params(struct snd_pcm_substream *substream,
mutex_lock(&subs->stream->chip->shutdown_mutex);
/* format changed */
stop_endpoints(subs, 0, 0, 0);
deactivate_endpoints(subs);

ret = activate_endpoints(subs);
if (ret < 0)
goto unlock;

ret = snd_usb_endpoint_set_params(subs->data_endpoint, hw_params, fmt,
subs->sync_endpoint);
if (ret < 0)
Expand Down Expand Up @@ -500,6 +508,7 @@ static int snd_usb_hw_free(struct snd_pcm_substream *substream)
subs->period_bytes = 0;
mutex_lock(&subs->stream->chip->shutdown_mutex);
stop_endpoints(subs, 0, 1, 1);
deactivate_endpoints(subs);
mutex_unlock(&subs->stream->chip->shutdown_mutex);
return snd_pcm_lib_free_vmalloc_buffer(substream);
}
Expand Down Expand Up @@ -938,16 +947,20 @@ static int snd_usb_pcm_open(struct snd_pcm_substream *substream, int direction)

static int snd_usb_pcm_close(struct snd_pcm_substream *substream, int direction)
{
int ret;
struct snd_usb_stream *as = snd_pcm_substream_chip(substream);
struct snd_usb_substream *subs = &as->substream[direction];

stop_endpoints(subs, 0, 0, 0);
ret = deactivate_endpoints(subs);

if (!as->chip->shutdown && subs->interface >= 0) {
usb_set_interface(subs->dev, subs->interface, 0);
subs->interface = -1;
}

subs->pcm_substream = NULL;
snd_usb_autosuspend(subs->stream->chip);

return ret;
return 0;
}

/* Since a URB can handle only a single linear buffer, we must use double
Expand Down

0 comments on commit 68e67f4

Please sign in to comment.