[PATCH v4 00/15] ALSA: usb-audio: fix playback/capture concurrent usage

[PATCH v4 00/15] ALSA: usb-audio: fix playback/capture concurrent usage

[Eldad Zack]

Hi,

This patch series attempts to fix a long standing problem with the concurrent
usage of playback and capture on implicit feedback devices.

Now the following work properly on my system:
* Full-duplex with jack :)

* All tests I ran with my tool. Will post it in case anyone want to try it,
  it takes about 15 minutes to run all the tests.

* Starting a capture-only jackd instance, e.g.:
    jackd -n 1 -d alsa -d hw:2,0 -C
  and then playback-only:
    jackd -n 2 -d alsa -d hw:2,0 -P
  and then stop and start the playback.
  The same with playback running and restarting capture.

* Trying to start a second substream with parameter mismatch (rate or
  period bytes is all I can test with my device) will fail,
  without ill effects.

With this series applied there are now 3 usage tracking variables:
 - substream usage flag
 - endpoint param_set (to protect the parameters until the endpoint is started)
 - the existing endpoint use_count

Changes from v3:
* Added substream "used" flag, to prevent failure in some combinations of ops. (#13)
* Moved start_endpoints for capture to prepare (#14)
* Added more constraints to the endpoint_may_set_params function (#10)
* Cleanups (#12, #15)

v3:
http://mailman.alsa-project.org/pipermail/alsa-devel/2013-August/065744.html

Applies against for-next, tested on upstream 3.12-rc3 (needs Alan Stern's recent
patch "improve buffer size computations for USB PCM audio" to apply cleanly
on current mainline).

Daniel and Nikolay: if you can test again with this series I'd appreciate it.

Cheers,
Eldad

Eldad Zack (15):
  ALSA: usb-audio: remove unused parameter from sync_ep_set_params
  ALSA: usb-audio: remove deactivate_endpoints()
  ALSA: usb-audio: prevent NULL dereference on stop trigger
  ALSA: usb-audio: don't deactivate URBs on in-use EP
  ALSA: usb-audio: void return type of snd_usb_endpoint_deactivate()
  ALSA: usb-audio: clear SUBSTREAM_FLAG_SYNC_EP_STARTED on error
  ALSA: usb-audio: correct ep use_count semantics (add set_param flag)
  ALSA: usb-audio: rename alt_idx to altsetting
  ALSA: usb-audio: conditional interface altsetting
  ALSA: usb-audio: conditional concurrent usage of endpoint
  ALSA: usb-audio: remove altset_idx from snd_usb_substream
  ALSA: usb-audio: remove unused endpoint flag EP_FLAG_ACTIVATED
  ALSA: usb-audio: clear sync subs hw_params
  ALSA: usb-audio: always wait in start_endpoints
  ALSA: usb-audio: improve logging messages

 sound/usb/card.h     |   9 ++-
 sound/usb/endpoint.c | 122 ++++++++++++++++++++---------
 sound/usb/endpoint.h |  13 +++-
 sound/usb/pcm.c      | 214 ++++++++++++++++++++++++++++++++++++---------------
 sound/usb/pcm.h      |   2 +
 sound/usb/proc.c     |   4 +-
 6 files changed, 258 insertions(+), 106 deletions(-)

-- 
1.8.1.5

[PATCH v4 01/15] ALSA: usb-audio: remove unused parameter from sync_ep_set_params

[Eldad Zack]

Since the format is not actually used in sync_ep_set_params(),
there is no need to pass it down.

Signed-off-by: Eldad Zack <eldad@fogrefinery.com>
---
 sound/usb/endpoint.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/sound/usb/endpoint.c b/sound/usb/endpoint.c
index 21dc642..5dd51af 100644
--- a/sound/usb/endpoint.c
+++ b/sound/usb/endpoint.c
@@ -714,8 +714,7 @@ out_of_memory:
 /*
  * configure a sync endpoint
  */
-static int sync_ep_set_params(struct snd_usb_endpoint *ep,
-			      struct audioformat *fmt)
+static int sync_ep_set_params(struct snd_usb_endpoint *ep)
 {
 	int i;
 
@@ -812,7 +811,7 @@ int snd_usb_endpoint_set_params(struct snd_usb_endpoint *ep,
 					 buffer_periods, fmt, sync_ep);
 		break;
 	case  SND_USB_ENDPOINT_TYPE_SYNC:
-		err = sync_ep_set_params(ep, fmt);
+		err = sync_ep_set_params(ep);
 		break;
 	default:
 		err = -EINVAL;
-- 
1.8.1.5

[PATCH v4 02/15] ALSA: usb-audio: remove deactivate_endpoints()

[Eldad Zack]

The only call site for deactivate_endpoints() at snd_usb_hw_free().
The return value is not checked there, as it is irrelevant if it
fails on hw_free.
This patch moves the deactivation of the endpoints directly into
snd_usb_hw_free().

Signed-off-by: Eldad Zack <eldad@fogrefinery.com>
---
 sound/usb/pcm.c | 19 ++-----------------
 1 file changed, 2 insertions(+), 17 deletions(-)

diff --git a/sound/usb/pcm.c b/sound/usb/pcm.c
index 19e7995..1a9a018 100644
--- a/sound/usb/pcm.c
+++ b/sound/usb/pcm.c
@@ -282,22 +282,6 @@ static void stop_endpoints(struct snd_usb_substream *subs, bool wait)
 	}
 }
 
-static int deactivate_endpoints(struct snd_usb_substream *subs)
-{
-	int reta, retb;
-
-	reta = snd_usb_endpoint_deactivate(subs->sync_endpoint);
-	retb = snd_usb_endpoint_deactivate(subs->data_endpoint);
-
-	if (reta < 0)
-		return reta;
-
-	if (retb < 0)
-		return retb;
-
-	return 0;
-}
-
 static int search_roland_implicit_fb(struct usb_device *dev, int ifnum,
 				     unsigned int altsetting,
 				     struct usb_host_interface **alts,
@@ -736,7 +720,8 @@ static int snd_usb_hw_free(struct snd_pcm_substream *substream)
 	down_read(&subs->stream->chip->shutdown_rwsem);
 	if (!subs->stream->chip->shutdown) {
 		stop_endpoints(subs, true);
-		deactivate_endpoints(subs);
+		snd_usb_endpoint_deactivate(subs->sync_endpoint);
+		snd_usb_endpoint_deactivate(subs->data_endpoint);
 	}
 	up_read(&subs->stream->chip->shutdown_rwsem);
 	return snd_pcm_lib_free_vmalloc_buffer(substream);
-- 
1.8.1.5

[PATCH v4 03/15] ALSA: usb-audio: prevent NULL dereference on stop trigger

[Eldad Zack]

If an endpoint uses another endpoint for synchronization, and the
other endpoint is stopped, an oops will occur on NULL dereference.
Clearing the prepare/retire callbacks solves this issue.

v2: Thanks to Daniel Mack, fixed (an ironic) NULL dereference when
the pcm substream is opened and closed immediately.

Signed-off-by: Eldad Zack <eldad@fogrefinery.com>
---
 sound/usb/pcm.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/sound/usb/pcm.c b/sound/usb/pcm.c
index 1a9a018..525bc8c 100644
--- a/sound/usb/pcm.c
+++ b/sound/usb/pcm.c
@@ -1205,6 +1205,11 @@ static int snd_usb_pcm_close(struct snd_pcm_substream *substream, int direction)
 		subs->interface = -1;
 	}
 
+	if (subs->data_endpoint) {
+		subs->data_endpoint->prepare_data_urb = NULL;
+		subs->data_endpoint->retire_data_urb = NULL;
+	}
+
 	subs->pcm_substream = NULL;
 	snd_usb_autosuspend(subs->stream->chip);
 
@@ -1535,6 +1540,8 @@ static int snd_usb_substream_playback_trigger(struct snd_pcm_substream *substrea
 		subs->running = 1;
 		return 0;
 	case SNDRV_PCM_TRIGGER_STOP:
+		subs->data_endpoint->prepare_data_urb = NULL;
+		subs->data_endpoint->retire_data_urb = NULL;
 		stop_endpoints(subs, false);
 		subs->running = 0;
 		return 0;
@@ -1565,6 +1572,7 @@ static int snd_usb_substream_capture_trigger(struct snd_pcm_substream *substream
 		subs->running = 1;
 		return 0;
 	case SNDRV_PCM_TRIGGER_STOP:
+		subs->data_endpoint->retire_data_urb = NULL;
 		stop_endpoints(subs, false);
 		subs->running = 0;
 		return 0;
-- 
1.8.1.5

Re: [PATCH v4 03/15] ALSA: usb-audio: prevent NULL dereference on stop trigger

[Takashi Iwai]

At Sun,  6 Oct 2013 22:31:08 +0200,
Eldad Zack wrote:
> 
> If an endpoint uses another endpoint for synchronization, and the
> other endpoint is stopped, an oops will occur on NULL dereference.
> Clearing the prepare/retire callbacks solves this issue.
> 
> v2: Thanks to Daniel Mack, fixed (an ironic) NULL dereference when
> the pcm substream is opened and closed immediately.
> 
> Signed-off-by: Eldad Zack <eldad@fogrefinery.com>
> ---
>  sound/usb/pcm.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/sound/usb/pcm.c b/sound/usb/pcm.c
> index 1a9a018..525bc8c 100644
> --- a/sound/usb/pcm.c
> +++ b/sound/usb/pcm.c
> @@ -1205,6 +1205,11 @@ static int snd_usb_pcm_close(struct snd_pcm_substream *substream, int direction)
>  		subs->interface = -1;
>  	}
>  
> +	if (subs->data_endpoint) {
> +		subs->data_endpoint->prepare_data_urb = NULL;
> +		subs->data_endpoint->retire_data_urb = NULL;
> +	}
> +

This is superfluous as the close callback is always called after
tigger stop and hw_free.


Takashi

>  	subs->pcm_substream = NULL;
>  	snd_usb_autosuspend(subs->stream->chip);
>  
> @@ -1535,6 +1540,8 @@ static int snd_usb_substream_playback_trigger(struct snd_pcm_substream *substrea
>  		subs->running = 1;
>  		return 0;
>  	case SNDRV_PCM_TRIGGER_STOP:
> +		subs->data_endpoint->prepare_data_urb = NULL;
> +		subs->data_endpoint->retire_data_urb = NULL;
>  		stop_endpoints(subs, false);
>  		subs->running = 0;
>  		return 0;
> @@ -1565,6 +1572,7 @@ static int snd_usb_substream_capture_trigger(struct snd_pcm_substream *substream
>  		subs->running = 1;
>  		return 0;
>  	case SNDRV_PCM_TRIGGER_STOP:
> +		subs->data_endpoint->retire_data_urb = NULL;
>  		stop_endpoints(subs, false);
>  		subs->running = 0;
>  		return 0;
> -- 
> 1.8.1.5
> 

[PATCH v4 04/15] ALSA: usb-audio: don't deactivate URBs on in-use EP

[Eldad Zack]

If an endpoint in use, its associated URBs should not be
deactivated.

Signed-off-by: Eldad Zack <eldad@fogrefinery.com>
---
 sound/usb/endpoint.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/sound/usb/endpoint.c b/sound/usb/endpoint.c
index 5dd51af..e84732c 100644
--- a/sound/usb/endpoint.c
+++ b/sound/usb/endpoint.c
@@ -959,12 +959,12 @@ int snd_usb_endpoint_deactivate(struct snd_usb_endpoint *ep)
 	if (!ep)
 		return -EINVAL;
 
-	deactivate_urbs(ep, true);
-	wait_clear_urbs(ep);
-
 	if (ep->use_count != 0)
 		return 0;
 
+	deactivate_urbs(ep, true);
+	wait_clear_urbs(ep);
+
 	clear_bit(EP_FLAG_ACTIVATED, &ep->flags);
 
 	return 0;
-- 
1.8.1.5

[PATCH v4 05/15] ALSA: usb-audio: void return type of snd_usb_endpoint_deactivate()

[Eldad Zack]

The return value of snd_usb_endpoint_deactivate() is not used,
make the function have no return value.
Update the documentation to reflect what the function is actually
doing.

Signed-off-by: Eldad Zack <eldad@fogrefinery.com>
---
 sound/usb/endpoint.c | 15 +++++----------
 sound/usb/endpoint.h |  2 +-
 2 files changed, 6 insertions(+), 11 deletions(-)

diff --git a/sound/usb/endpoint.c b/sound/usb/endpoint.c
index e84732c..2685660 100644
--- a/sound/usb/endpoint.c
+++ b/sound/usb/endpoint.c
@@ -946,28 +946,23 @@ void snd_usb_endpoint_stop(struct snd_usb_endpoint *ep)
  *
  * @ep: the endpoint to deactivate
  *
- * If the endpoint is not currently in use, this functions will select the
- * alternate interface setting 0 for the interface of this endpoint.
+ * If the endpoint is not currently in use, this functions will
+ * deactivate its associated URBs.
  *
  * 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_deactivate(struct snd_usb_endpoint *ep)
+void snd_usb_endpoint_deactivate(struct snd_usb_endpoint *ep)
 {
 	if (!ep)
-		return -EINVAL;
+		return;
 
 	if (ep->use_count != 0)
-		return 0;
+		return;
 
 	deactivate_urbs(ep, true);
 	wait_clear_urbs(ep);
 
 	clear_bit(EP_FLAG_ACTIVATED, &ep->flags);
-
-	return 0;
 }
 
 /**
diff --git a/sound/usb/endpoint.h b/sound/usb/endpoint.h
index 3bd02f0..1c7e8ee 100644
--- a/sound/usb/endpoint.h
+++ b/sound/usb/endpoint.h
@@ -22,7 +22,7 @@ int  snd_usb_endpoint_start(struct snd_usb_endpoint *ep, bool can_sleep);
 void snd_usb_endpoint_stop(struct snd_usb_endpoint *ep);
 void snd_usb_endpoint_sync_pending_stop(struct snd_usb_endpoint *ep);
 int  snd_usb_endpoint_activate(struct snd_usb_endpoint *ep);
-int  snd_usb_endpoint_deactivate(struct snd_usb_endpoint *ep);
+void snd_usb_endpoint_deactivate(struct snd_usb_endpoint *ep);
 void snd_usb_endpoint_free(struct list_head *head);
 
 int snd_usb_endpoint_implicit_feedback_sink(struct snd_usb_endpoint *ep);
-- 
1.8.1.5

[PATCH v4 06/15] ALSA: usb-audio: clear SUBSTREAM_FLAG_SYNC_EP_STARTED on error

[Eldad Zack]

If setting the interface fails, the SUBSTREAM_FLAG_SYNC_EP_STARTED
should be cleared.

Signed-off-by: Eldad Zack <eldad@fogrefinery.com>
---
 sound/usb/pcm.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/sound/usb/pcm.c b/sound/usb/pcm.c
index 525bc8c..e38961e 100644
--- a/sound/usb/pcm.c
+++ b/sound/usb/pcm.c
@@ -246,6 +246,7 @@ static int start_endpoints(struct snd_usb_substream *subs, bool can_sleep)
 						subs->sync_endpoint->iface,
 						subs->sync_endpoint->alt_idx);
 			if (err < 0) {
+				clear_bit(SUBSTREAM_FLAG_SYNC_EP_STARTED, &subs->flags);
 				snd_printk(KERN_ERR
 					   "%d:%d:%d: cannot set interface (%d)\n",
 					   subs->dev->devnum,
-- 
1.8.1.5

[PATCH v4 07/15] ALSA: usb-audio: correct ep use_count semantics (add set_param flag)

[Eldad Zack]

Currently, use_count is used in snd_usb_endpoint_set_params to
ensure the parameters don't get changed for an in-use endpoint.

However, there is a subtle condition where this check fails -
if hw_params is called on both substreams before calling prepare (for
playback) or start trigger (for capture): the endpoint is not yet
started, i.e., snd_usb_endpoint_start() does not yet increment use_count.

This adds a flag to check if the parameters are set, but does not
omit checking the use_count.

Signed-off-by: Eldad Zack <eldad@fogrefinery.com>

---
v2: Cleaned up the patch, now it does only one thing (add the flag
and the check).
---
 sound/usb/card.h     | 1 +
 sound/usb/endpoint.c | 6 +++++-
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/sound/usb/card.h b/sound/usb/card.h
index ca98a9b..1014136 100644
--- a/sound/usb/card.h
+++ b/sound/usb/card.h
@@ -51,6 +51,7 @@ struct snd_usb_endpoint {
 	struct snd_usb_audio *chip;
 
 	int use_count;
+	bool param_set;
 	int ep_num;		/* the referenced endpoint number */
 	int type;		/* SND_USB_ENDPOINT_TYPE_* */
 	unsigned long flags;
diff --git a/sound/usb/endpoint.c b/sound/usb/endpoint.c
index 2685660..9acc864 100644
--- a/sound/usb/endpoint.c
+++ b/sound/usb/endpoint.c
@@ -472,6 +472,8 @@ struct snd_usb_endpoint *snd_usb_add_endpoint(struct snd_usb_audio *chip,
 		ep->syncmaxsize = le16_to_cpu(get_endpoint(alts, 1)->wMaxPacketSize);
 	}
 
+	ep->param_set = false;
+
 	list_add_tail(&ep->list, &chip->ep_list);
 
 __exit_unlock:
@@ -780,11 +782,12 @@ int snd_usb_endpoint_set_params(struct snd_usb_endpoint *ep,
 {
 	int err;
 
-	if (ep->use_count != 0) {
+	if (ep->param_set || ep->use_count != 0) {
 		snd_printk(KERN_WARNING "Unable to change format on ep #%x: already in use\n",
 			   ep->ep_num);
 		return -EBUSY;
 	}
+	ep->param_set = true;
 
 	/* release old buffers, if any */
 	release_urbs(ep, 0);
@@ -937,6 +940,7 @@ void snd_usb_endpoint_stop(struct snd_usb_endpoint *ep)
 		ep->sync_slave = NULL;
 		ep->retire_data_urb = NULL;
 		ep->prepare_data_urb = NULL;
+		ep->param_set = false;
 		set_bit(EP_FLAG_STOPPING, &ep->flags);
 	}
 }
-- 
1.8.1.5

Re: [PATCH v4 07/15] ALSA: usb-audio: correct ep use_count semantics (add set_param flag)

[Takashi Iwai]

At Sun,  6 Oct 2013 22:31:12 +0200,
Eldad Zack wrote:
> 
> Currently, use_count is used in snd_usb_endpoint_set_params to
> ensure the parameters don't get changed for an in-use endpoint.
> 
> However, there is a subtle condition where this check fails -
> if hw_params is called on both substreams before calling prepare (for
> playback) or start trigger (for capture): the endpoint is not yet
> started, i.e., snd_usb_endpoint_start() does not yet increment use_count.
> 
> This adds a flag to check if the parameters are set, but does not
> omit checking the use_count.
> 
> Signed-off-by: Eldad Zack <eldad@fogrefinery.com>
> 
> ---
> v2: Cleaned up the patch, now it does only one thing (add the flag
> and the check).

I guess this logic doesn't work if a capture stream is re-prepared
before started.  I know that you'll change the capture stream starting
in the later patch, but then this patch must be applied after that,
because it implicitly assumes the scheme.


Takashi

> ---
>  sound/usb/card.h     | 1 +
>  sound/usb/endpoint.c | 6 +++++-
>  2 files changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/sound/usb/card.h b/sound/usb/card.h
> index ca98a9b..1014136 100644
> --- a/sound/usb/card.h
> +++ b/sound/usb/card.h
> @@ -51,6 +51,7 @@ struct snd_usb_endpoint {
>  	struct snd_usb_audio *chip;
>  
>  	int use_count;
> +	bool param_set;
>  	int ep_num;		/* the referenced endpoint number */
>  	int type;		/* SND_USB_ENDPOINT_TYPE_* */
>  	unsigned long flags;
> diff --git a/sound/usb/endpoint.c b/sound/usb/endpoint.c
> index 2685660..9acc864 100644
> --- a/sound/usb/endpoint.c
> +++ b/sound/usb/endpoint.c
> @@ -472,6 +472,8 @@ struct snd_usb_endpoint *snd_usb_add_endpoint(struct snd_usb_audio *chip,
>  		ep->syncmaxsize = le16_to_cpu(get_endpoint(alts, 1)->wMaxPacketSize);
>  	}
>  
> +	ep->param_set = false;
> +
>  	list_add_tail(&ep->list, &chip->ep_list);
>  
>  __exit_unlock:
> @@ -780,11 +782,12 @@ int snd_usb_endpoint_set_params(struct snd_usb_endpoint *ep,
>  {
>  	int err;
>  
> -	if (ep->use_count != 0) {
> +	if (ep->param_set || ep->use_count != 0) {
>  		snd_printk(KERN_WARNING "Unable to change format on ep #%x: already in use\n",
>  			   ep->ep_num);
>  		return -EBUSY;
>  	}
> +	ep->param_set = true;
>  
>  	/* release old buffers, if any */
>  	release_urbs(ep, 0);
> @@ -937,6 +940,7 @@ void snd_usb_endpoint_stop(struct snd_usb_endpoint *ep)
>  		ep->sync_slave = NULL;
>  		ep->retire_data_urb = NULL;
>  		ep->prepare_data_urb = NULL;
> +		ep->param_set = false;
>  		set_bit(EP_FLAG_STOPPING, &ep->flags);
>  	}
>  }
> -- 
> 1.8.1.5
> 

Re: [PATCH v4 07/15] ALSA: usb-audio: correct ep use_count semantics (add set_param flag)

[Eldad Zack]

On Mon, 7 Oct 2013, Takashi Iwai wrote:

> At Sun,  6 Oct 2013 22:31:12 +0200,
> Eldad Zack wrote:
> > 
> > Currently, use_count is used in snd_usb_endpoint_set_params to
> > ensure the parameters don't get changed for an in-use endpoint.
> > 
> > However, there is a subtle condition where this check fails -
> > if hw_params is called on both substreams before calling prepare (for
> > playback) or start trigger (for capture): the endpoint is not yet
> > started, i.e., snd_usb_endpoint_start() does not yet increment use_count.
> > 
> > This adds a flag to check if the parameters are set, but does not
> > omit checking the use_count.
> > 
> > Signed-off-by: Eldad Zack <eldad@fogrefinery.com>
> > 
> > ---
> > v2: Cleaned up the patch, now it does only one thing (add the flag
> > and the check).
> 
> I guess this logic doesn't work if a capture stream is re-prepared
> before started.  I know that you'll change the capture stream starting
> in the later patch, but then this patch must be applied after that,
> because it implicitly assumes the scheme.

Yes and it causes a regression too, it prevents full-duplex usage
(with jack or client that use the same sequence). Sorry, I forgot to add 
that to the commit message.

I left this so the change is clear, I figured it helps when reading the 
history.
Should I combine this with the later patch? Or should I put a notice in
this commit message that "this patch breaks... will be fixed later 
by..."?

BTW - sorry if that's a trivial question - is it possible for ops to 
race, or do the ops get serialized?
If it is possible, I believe there's a few places that need locking.

Cheers,
Eldad

Re: [PATCH v4 07/15] ALSA: usb-audio: correct ep use_count semantics (add set_param flag)

[Takashi Iwai]

At Mon, 7 Oct 2013 21:31:35 +0200 (CEST),
Eldad Zack wrote:
> 
> 
> 
> On Mon, 7 Oct 2013, Takashi Iwai wrote:
> 
> > At Sun,  6 Oct 2013 22:31:12 +0200,
> > Eldad Zack wrote:
> > > 
> > > Currently, use_count is used in snd_usb_endpoint_set_params to
> > > ensure the parameters don't get changed for an in-use endpoint.
> > > 
> > > However, there is a subtle condition where this check fails -
> > > if hw_params is called on both substreams before calling prepare (for
> > > playback) or start trigger (for capture): the endpoint is not yet
> > > started, i.e., snd_usb_endpoint_start() does not yet increment use_count.
> > > 
> > > This adds a flag to check if the parameters are set, but does not
> > > omit checking the use_count.
> > > 
> > > Signed-off-by: Eldad Zack <eldad@fogrefinery.com>
> > > 
> > > ---
> > > v2: Cleaned up the patch, now it does only one thing (add the flag
> > > and the check).
> > 
> > I guess this logic doesn't work if a capture stream is re-prepared
> > before started.  I know that you'll change the capture stream starting
> > in the later patch, but then this patch must be applied after that,
> > because it implicitly assumes the scheme.
> 
> Yes and it causes a regression too, it prevents full-duplex usage
> (with jack or client that use the same sequence). Sorry, I forgot to add 
> that to the commit message.
> 
> I left this so the change is clear, I figured it helps when reading the 
> history.
> Should I combine this with the later patch? Or should I put a notice in
> this commit message that "this patch breaks... will be fixed later 
> by..."?

At best shuffle the order of patches so that this one is applied after
the required patches.

> BTW - sorry if that's a trivial question - is it possible for ops to 
> race, or do the ops get serialized?
> If it is possible, I believe there's a few places that need locking.

Which ops specifically?  For a PCM substream, it's serialized.  For
duplex substreams, both can be called at the same time, thus you'd
need to protect concurrent accesses, if any.


Takashi

[PATCH v4 08/15] ALSA: usb-audio: rename alt_idx to altsetting

[Eldad Zack]

As Clemens Ladisch kindly explained:
 "Please note that there are two methods to identify alternate settings:
  the number, which is the value in bAlternateSetting, and the index,
  which is the index in the descriptor array.  There might be some wording
  in the USB spec that these two values must be the same, but in reality,
  [insert standard rant about firmware writers], bAlternateSetting
  must be treated as a random ID value."

This patch changes the name to express the correct usage semantics.
No functional change.

Signed-off-by: Eldad Zack <eldad@fogrefinery.com>
---
 sound/usb/card.h     | 2 +-
 sound/usb/endpoint.c | 6 +++---
 sound/usb/pcm.c      | 6 +++---
 3 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/sound/usb/card.h b/sound/usb/card.h
index 1014136..7ccacb4 100644
--- a/sound/usb/card.h
+++ b/sound/usb/card.h
@@ -97,7 +97,7 @@ struct snd_usb_endpoint {
 	unsigned int syncinterval;	/* P for adaptive mode, 0 otherwise */
 	unsigned char silence_value;
 	unsigned int stride;
-	int iface, alt_idx;
+	int iface, altsetting;
 	int skip_packets;		/* quirks for devices to ignore the first n packets
 					   in a stream */
 
diff --git a/sound/usb/endpoint.c b/sound/usb/endpoint.c
index 9acc864..c90d065 100644
--- a/sound/usb/endpoint.c
+++ b/sound/usb/endpoint.c
@@ -426,9 +426,9 @@ struct snd_usb_endpoint *snd_usb_add_endpoint(struct snd_usb_audio *chip,
 	list_for_each_entry(ep, &chip->ep_list, list) {
 		if (ep->ep_num == ep_num &&
 		    ep->iface == alts->desc.bInterfaceNumber &&
-		    ep->alt_idx == alts->desc.bAlternateSetting) {
+		    ep->altsetting == alts->desc.bAlternateSetting) {
 			snd_printdd(KERN_DEBUG "Re-using EP %x in iface %d,%d @%p\n",
-					ep_num, ep->iface, ep->alt_idx, ep);
+					ep_num, ep->iface, ep->altsetting, ep);
 			goto __exit_unlock;
 		}
 	}
@@ -447,7 +447,7 @@ struct snd_usb_endpoint *snd_usb_add_endpoint(struct snd_usb_audio *chip,
 	ep->type = type;
 	ep->ep_num = ep_num;
 	ep->iface = alts->desc.bInterfaceNumber;
-	ep->alt_idx = alts->desc.bAlternateSetting;
+	ep->altsetting = alts->desc.bAlternateSetting;
 	INIT_LIST_HEAD(&ep->ready_playback_urbs);
 	ep_num &= USB_ENDPOINT_NUMBER_MASK;
 
diff --git a/sound/usb/pcm.c b/sound/usb/pcm.c
index e38961e..89381ad 100644
--- a/sound/usb/pcm.c
+++ b/sound/usb/pcm.c
@@ -241,17 +241,17 @@ static int start_endpoints(struct snd_usb_substream *subs, bool can_sleep)
 		struct snd_usb_endpoint *ep = subs->sync_endpoint;
 
 		if (subs->data_endpoint->iface != subs->sync_endpoint->iface ||
-		    subs->data_endpoint->alt_idx != subs->sync_endpoint->alt_idx) {
+		    subs->data_endpoint->altsetting != subs->sync_endpoint->altsetting) {
 			err = usb_set_interface(subs->dev,
 						subs->sync_endpoint->iface,
-						subs->sync_endpoint->alt_idx);
+						subs->sync_endpoint->altsetting);
 			if (err < 0) {
 				clear_bit(SUBSTREAM_FLAG_SYNC_EP_STARTED, &subs->flags);
 				snd_printk(KERN_ERR
 					   "%d:%d:%d: cannot set interface (%d)\n",
 					   subs->dev->devnum,
 					   subs->sync_endpoint->iface,
-					   subs->sync_endpoint->alt_idx, err);
+					   subs->sync_endpoint->altsetting, err);
 				return -EIO;
 			}
 		}
-- 
1.8.1.5

[PATCH v4 09/15] ALSA: usb-audio: conditional interface altsetting

[Eldad Zack]

On some devices, if the endpoint for the other direction is in use,
setting one interface will shutdown the other (in use) endpoint.
This patch moves all of the alternate setting operations for pcm
ops to one function which checks if we can do so.

If current alternate is 0, it is safe to set.

Thanks to Clemens Ladisch for pointing out that cur_altsetting
in usb_interface can be used to determine the current altsetting
index as well as the correct naming convention for the altsetting
number.

Signed-off-by: Eldad Zack <eldad@fogrefinery.com>

---
v2:
* Removed get_interface usage.
* Fixed NULL dereference for explicit feedback - now other_subs will
  be set only after testing that subs is not NULL.
v3:
* Changed "altset_idx" to "altsetting".
v4:
* Removed helper functions
---
 sound/usb/pcm.c | 68 ++++++++++++++++++++++++++++++++++++++++++++++++++-------
 1 file changed, 60 insertions(+), 8 deletions(-)

diff --git a/sound/usb/pcm.c b/sound/usb/pcm.c
index 89381ad..c165ccf 100644
--- a/sound/usb/pcm.c
+++ b/sound/usb/pcm.c
@@ -216,6 +216,60 @@ int snd_usb_init_pitch(struct snd_usb_audio *chip, int iface,
 	}
 }
 
+static int get_cur_altsetting(struct usb_device *dev, int ifnum)
+{
+	struct usb_interface *iface;
+	struct usb_interface_descriptor *altsd;
+
+	if (ifnum == -1)
+		return 0;
+
+	iface = usb_ifnum_to_if(dev, ifnum);
+	if (!iface)
+		return -EINVAL;
+
+	if (!iface->cur_altsetting)
+		return -EINVAL;
+	altsd = &(iface->cur_altsetting->desc);
+
+	return altsd->bAlternateSetting;
+}
+
+static int subs_set_interface(struct snd_usb_substream *subs, int ifnum,
+			      int altsetting)
+{
+	struct snd_usb_substream *other_subs;
+	int err;
+
+	snd_printdd(KERN_DEBUG "%s: Requested set interface %d alts %d\n",
+		    __func__, ifnum, altsetting);
+
+	if (subs == NULL)
+		return usb_set_interface(subs->dev, ifnum, altsetting);
+	other_subs = &(subs->stream->substream[subs->direction ^ 1]);
+
+	if (other_subs->data_endpoint &&
+	    other_subs->data_endpoint->use_count != 0) {
+		int cur_altsetting = get_cur_altsetting(subs->dev, ifnum);
+
+		if (cur_altsetting == altsetting)
+			return 0;
+
+		if (cur_altsetting > 0 && altsetting == 0)
+			return 0;
+	}
+
+	err = usb_set_interface(subs->dev, ifnum, altsetting);
+	snd_printdd(KERN_DEBUG "%s: Interface %d alts set to %d (err %d)\n",
+		    __func__, ifnum, altsetting, err);
+	if (err < 0)
+		return err;
+
+	subs->interface = altsetting == 0 ? -1 : ifnum;
+
+	return 0;
+}
+
 static int start_endpoints(struct snd_usb_substream *subs, bool can_sleep)
 {
 	int err;
@@ -242,9 +296,9 @@ static int start_endpoints(struct snd_usb_substream *subs, bool can_sleep)
 
 		if (subs->data_endpoint->iface != subs->sync_endpoint->iface ||
 		    subs->data_endpoint->altsetting != subs->sync_endpoint->altsetting) {
-			err = usb_set_interface(subs->dev,
-						subs->sync_endpoint->iface,
-						subs->sync_endpoint->altsetting);
+			err = subs_set_interface(subs,
+						 subs->sync_endpoint->iface,
+						 subs->sync_endpoint->altsetting);
 			if (err < 0) {
 				clear_bit(SUBSTREAM_FLAG_SYNC_EP_STARTED, &subs->flags);
 				snd_printk(KERN_ERR
@@ -467,20 +521,19 @@ static int set_format(struct snd_usb_substream *subs, struct audioformat *fmt)
 
 	/* close the old interface */
 	if (subs->interface >= 0 && subs->interface != fmt->iface) {
-		err = usb_set_interface(subs->dev, subs->interface, 0);
+		err = subs_set_interface(subs, 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);
+		err = subs_set_interface(subs, 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);
@@ -488,7 +541,6 @@ static int set_format(struct snd_usb_substream *subs, struct audioformat *fmt)
 		}
 		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);
@@ -1202,7 +1254,7 @@ static int snd_usb_pcm_close(struct snd_pcm_substream *substream, int direction)
 	stop_endpoints(subs, true);
 
 	if (!as->chip->shutdown && subs->interface >= 0) {
-		usb_set_interface(subs->dev, subs->interface, 0);
+		subs_set_interface(subs, subs->interface, 0);
 		subs->interface = -1;
 	}
 
-- 
1.8.1.5

Re: [PATCH v4 09/15] ALSA: usb-audio: conditional interface altsetting

[Takashi Iwai]

At Sun,  6 Oct 2013 22:31:14 +0200,
Eldad Zack wrote:
> 
> On some devices, if the endpoint for the other direction is in use,
> setting one interface will shutdown the other (in use) endpoint.
> This patch moves all of the alternate setting operations for pcm
> ops to one function which checks if we can do so.
> 
> If current alternate is 0, it is safe to set.

Is this check needed for all devices?

The combination of playback and capture streams in snd_usb_substream
is casual, just depending on the enumeration.  It doesn't always mean
that these are coupled.  For non-implicit-fb devices, won't this
result in a false positive?

Also...

> +static int subs_set_interface(struct snd_usb_substream *subs, int ifnum,
> +			      int altsetting)
> +{
> +	struct snd_usb_substream *other_subs;
> +	int err;
> +
> +	snd_printdd(KERN_DEBUG "%s: Requested set interface %d alts %d\n",
> +		    __func__, ifnum, altsetting);
> +
> +	if (subs == NULL)
> +		return usb_set_interface(subs->dev, ifnum, altsetting);

It must lead to Oops.


thanks,

Takashi

Re: [PATCH v4 09/15] ALSA: usb-audio: conditional interface altsetting

[Eldad Zack]

On Mon, 7 Oct 2013, Takashi Iwai wrote:

> At Sun,  6 Oct 2013 22:31:14 +0200,
> Eldad Zack wrote:
> > 
> > On some devices, if the endpoint for the other direction is in use,
> > setting one interface will shutdown the other (in use) endpoint.
> > This patch moves all of the alternate setting operations for pcm
> > ops to one function which checks if we can do so.
> > 
> > If current alternate is 0, it is safe to set.
> 
> Is this check needed for all devices?

I'm not sure, I only have this one device to test with.
I asked once on the list if this behavior (when the alts is set to 0,
the endpoint stops if it is running) or does it only happen on implicit
feedback, but got no response.

> The combination of playback and capture streams in snd_usb_substream
> is casual, just depending on the enumeration.  It doesn't always mean
> that these are coupled.  For non-implicit-fb devices, won't this
> result in a false positive?

The worst that can happen is that the altsetting will not go back to 0, 
if the other substream is in use. Is that an issue?

> 
> Also...
> 
> > +static int subs_set_interface(struct snd_usb_substream *subs, int ifnum,
> > +			      int altsetting)
> > +{
> > +	struct snd_usb_substream *other_subs;
> > +	int err;
> > +
> > +	snd_printdd(KERN_DEBUG "%s: Requested set interface %d alts %d\n",
> > +		    __func__, ifnum, altsetting);
> > +
> > +	if (subs == NULL)
> > +		return usb_set_interface(subs->dev, ifnum, altsetting);
> 
> It must lead to Oops.

Yes, that's bad code. :/
Thank you for pointing it out!

Although it won't get triggered, since it won't get any NULL subs from 
any callsite as far as I see. I'll remove this and add WARN_ON(!subs).

Cheers,
Eldad

Re: [PATCH v4 09/15] ALSA: usb-audio: conditional interface altsetting

[Clemens Ladisch]

Eldad Zack wrote:
> On Mon, 7 Oct 2013, Takashi Iwai wrote:
>> Eldad Zack wrote:
>>> On some devices, if the endpoint for the other direction is in use,
>>> setting one interface will shutdown the other (in use) endpoint.
>>> This patch moves all of the alternate setting operations for pcm
>>> ops to one function which checks if we can do so.
>>>
>>> If current alternate is 0, it is safe to set.
>>
>> Is this check needed for all devices?
>
> I'm not sure, I only have this one device to test with.

In theory, interfaces are independent, but in practice, the firmware
does whatever it does.  For implicit feedback devices, it's likely that
they were expected to be used in full duplex mode only.


Regards,
Clemens

Re: [PATCH v4 09/15] ALSA: usb-audio: conditional interface altsetting

[Takashi Iwai]

At Mon, 07 Oct 2013 20:23:38 +0200,
Clemens Ladisch wrote:
> 
> Eldad Zack wrote:
> > On Mon, 7 Oct 2013, Takashi Iwai wrote:
> >> Eldad Zack wrote:
> >>> On some devices, if the endpoint for the other direction is in use,
> >>> setting one interface will shutdown the other (in use) endpoint.
> >>> This patch moves all of the alternate setting operations for pcm
> >>> ops to one function which checks if we can do so.
> >>>
> >>> If current alternate is 0, it is safe to set.
> >>
> >> Is this check needed for all devices?
> >
> > I'm not sure, I only have this one device to test with.
> 
> In theory, interfaces are independent, but in practice, the firmware
> does whatever it does.  For implicit feedback devices, it's likely that
> they were expected to be used in full duplex mode only.

For the implicit feedback devices, I find the change being good, too.
But my concern is rather that this filtering is applied to all cases,
no matter whether it's implicit fb or not.  In such cases, the
assignment of full duplex streams is just casual, and they are likely
individual.


thanks,

Takashi

[PATCH v4 10/15] ALSA: usb-audio: conditional concurrent usage of endpoint

[Eldad Zack]

If the current endpoint is in use, check if the hardware parameters
match. If they do, allow the usage.
This also requires setting the parameters in case an data endpoint
is used as a synchornization source, and it is first set by its
sink endpoint.

Also check the data endpoint on the substream in the other direction.
If it is in use, the rate and format must match.
The channel count may differ - but the period bytes must match.
If the channel count is different, adjust the expected period bytes
by the ratio of the channel count.

Add the same check to hw_params, to prevent overwriting the
current params.
Clear the hardware parameters on hw_free only if the other endpoint
is not in use.

No change for sync endpoints (explicit feedback).

Signed-off-by: Eldad Zack <eldad@fogrefinery.com>

---
v2: added checking the data endpoint on the substream in the other
    direction.
---
 sound/usb/endpoint.c | 63 ++++++++++++++++++++++++++++++++++++++++++++++++----
 sound/usb/endpoint.h |  9 +++++++-
 sound/usb/pcm.c      | 59 ++++++++++++++++++++++++++++++++++++++----------
 3 files changed, 114 insertions(+), 17 deletions(-)

diff --git a/sound/usb/endpoint.c b/sound/usb/endpoint.c
index c90d065..6cb3137 100644
--- a/sound/usb/endpoint.c
+++ b/sound/usb/endpoint.c
@@ -753,6 +753,51 @@ out_of_memory:
 	return -ENOMEM;
 }
 
+bool snd_usb_endpoint_may_set_params(struct snd_usb_substream *subs,
+				     snd_pcm_format_t pcm_format,
+				     unsigned int channels,
+				     unsigned int period_bytes,
+				     unsigned int rate)
+{
+	struct snd_usb_substream *other_subs =
+		&subs->stream->substream[subs->direction ^ 1];
+	struct snd_usb_endpoint *other_ep = other_subs->data_endpoint;
+
+	/* no associated substream */
+	if (!subs)
+		return true;
+
+	if (other_ep && other_ep->use_count != 0) {
+		int other_pb;
+		if (other_subs->pcm_format != pcm_format ||
+		    other_subs->cur_rate != rate)
+			return false;
+
+		other_pb = (other_subs->channels == channels ?
+			    other_subs->period_bytes :
+			    other_subs->period_bytes * channels / other_subs->channels);
+		if (other_pb != period_bytes)
+			return false;
+	}
+
+	/* data_endpoint not yet initialized */
+	if (!subs->data_endpoint)
+		return true;
+
+	if (subs->data_endpoint->use_count == 0)
+		return true;
+
+	if (subs->pcm_format != pcm_format ||
+	    subs->channels != channels ||
+	    subs->period_bytes != period_bytes ||
+	    subs->cur_rate != rate)
+		return false;
+
+	snd_printdd(KERN_INFO "ep #%x in use, parameters match\n",
+		    subs->data_endpoint->ep_num);
+	return true;
+}
+
 /**
  * snd_usb_endpoint_set_params: configure an snd_usb_endpoint
  *
@@ -765,6 +810,7 @@ out_of_memory:
  * @rate: the frame rate.
  * @fmt: the USB audio format information
  * @sync_ep: the sync endpoint to use, if any
+ * @subs: the substream that uses this endpoint as data endpoint
  *
  * Determine the number of URBs to be used on this endpoint.
  * An endpoint must be configured before it can be started.
@@ -778,14 +824,23 @@ int snd_usb_endpoint_set_params(struct snd_usb_endpoint *ep,
 				unsigned int buffer_periods,
 				unsigned int rate,
 				struct audioformat *fmt,
-				struct snd_usb_endpoint *sync_ep)
+				struct snd_usb_endpoint *sync_ep,
+				struct snd_usb_substream *subs)
 {
 	int err;
 
 	if (ep->param_set || ep->use_count != 0) {
-		snd_printk(KERN_WARNING "Unable to change format on ep #%x: already in use\n",
-			   ep->ep_num);
-		return -EBUSY;
+		if (!subs ||
+		    !snd_usb_endpoint_may_set_params(subs, pcm_format,
+						     channels, period_bytes,
+						     rate)) {
+			snd_printk(KERN_WARNING
+				   "Unable to change format on ep #%x: already in use\n",
+				   ep->ep_num);
+			return -EBUSY;
+		}
+		/* Endpoint already set with the same parameters */
+		return 0;
 	}
 	ep->param_set = true;
 
diff --git a/sound/usb/endpoint.h b/sound/usb/endpoint.h
index 1c7e8ee..188675d 100644
--- a/sound/usb/endpoint.h
+++ b/sound/usb/endpoint.h
@@ -16,7 +16,8 @@ int snd_usb_endpoint_set_params(struct snd_usb_endpoint *ep,
 				unsigned int buffer_periods,
 				unsigned int rate,
 				struct audioformat *fmt,
-				struct snd_usb_endpoint *sync_ep);
+				struct snd_usb_endpoint *sync_ep,
+				struct snd_usb_substream *subs);
 
 int  snd_usb_endpoint_start(struct snd_usb_endpoint *ep, bool can_sleep);
 void snd_usb_endpoint_stop(struct snd_usb_endpoint *ep);
@@ -32,4 +33,10 @@ void snd_usb_handle_sync_urb(struct snd_usb_endpoint *ep,
 			     struct snd_usb_endpoint *sender,
 			     const struct urb *urb);
 
+bool snd_usb_endpoint_may_set_params(struct snd_usb_substream *subs,
+				     snd_pcm_format_t pcm_format,
+				     unsigned int channels,
+				     unsigned int period_bytes,
+				     unsigned int rate);
+
 #endif /* __USBAUDIO_ENDPOINT_H */
diff --git a/sound/usb/pcm.c b/sound/usb/pcm.c
index c165ccf..7895c81 100644
--- a/sound/usb/pcm.c
+++ b/sound/usb/pcm.c
@@ -635,6 +635,7 @@ static int configure_sync_endpoint(struct snd_usb_substream *subs)
 						   0, 0,
 						   subs->cur_rate,
 						   subs->cur_audiofmt,
+						   NULL,
 						   NULL);
 
 	/* Try to find the best matching audioformat. */
@@ -672,9 +673,18 @@ static int configure_sync_endpoint(struct snd_usb_substream *subs)
 					  0, 0,
 					  subs->cur_rate,
 					  sync_fp,
-					  NULL);
+					  NULL,
+					  sync_subs);
+	if (ret < 0)
+		return ret;
 
-	return ret;
+	sync_subs->pcm_format = subs->pcm_format;
+	sync_subs->period_bytes = sync_period_bytes;
+	sync_subs->channels = sync_fp->channels;
+	sync_subs->cur_rate = subs->cur_rate;
+	sync_subs->data_endpoint = subs->sync_endpoint;
+
+	return 0;
 }
 
 /*
@@ -696,7 +706,8 @@ static int configure_endpoint(struct snd_usb_substream *subs)
 					  subs->buffer_periods,
 					  subs->cur_rate,
 					  subs->cur_audiofmt,
-					  subs->sync_endpoint);
+					  subs->sync_endpoint,
+					  subs);
 	if (ret < 0)
 		return ret;
 
@@ -722,18 +733,38 @@ static int snd_usb_hw_params(struct snd_pcm_substream *substream,
 	struct snd_usb_substream *subs = substream->runtime->private_data;
 	struct audioformat *fmt;
 	int ret;
+	snd_pcm_format_t pcm_format;
+	unsigned int period_bytes;
+	unsigned int period_frames;
+	unsigned int buffer_periods;
+	unsigned int channels;
+	unsigned int rate;
 
 	ret = snd_pcm_lib_alloc_vmalloc_buffer(substream,
 					       params_buffer_bytes(hw_params));
 	if (ret < 0)
 		return ret;
 
-	subs->pcm_format = params_format(hw_params);
-	subs->period_bytes = params_period_bytes(hw_params);
-	subs->period_frames = params_period_size(hw_params);
-	subs->buffer_periods = params_periods(hw_params);
-	subs->channels = params_channels(hw_params);
-	subs->cur_rate = params_rate(hw_params);
+	pcm_format = params_format(hw_params);
+	period_bytes = params_period_bytes(hw_params);
+	period_frames = params_period_size(hw_params);
+	buffer_periods = params_periods(hw_params);
+	channels = params_channels(hw_params);
+	rate = params_rate(hw_params);
+
+	if (!snd_usb_endpoint_may_set_params(subs, pcm_format, channels,
+					     period_bytes, rate)) {
+		snd_printk(KERN_WARNING "Unable to set hw_params on substream (dir: %d), data EP in use\n",
+			   subs->direction);
+		return -EBUSY;
+	}
+
+	subs->pcm_format = pcm_format;
+	subs->period_bytes = period_bytes;
+	subs->period_frames = period_frames;
+	subs->buffer_periods = buffer_periods;
+	subs->channels = channels;
+	subs->cur_rate = rate;
 
 	fmt = find_format(subs);
 	if (!fmt) {
@@ -767,9 +798,13 @@ static int snd_usb_hw_free(struct snd_pcm_substream *substream)
 {
 	struct snd_usb_substream *subs = substream->runtime->private_data;
 
-	subs->cur_audiofmt = NULL;
-	subs->cur_rate = 0;
-	subs->period_bytes = 0;
+	if (!subs->data_endpoint || subs->data_endpoint->use_count == 0) {
+		subs->cur_audiofmt = NULL;
+		subs->cur_rate = 0;
+		subs->period_bytes = 0;
+		subs->channels = 0;
+	}
+
 	down_read(&subs->stream->chip->shutdown_rwsem);
 	if (!subs->stream->chip->shutdown) {
 		stop_endpoints(subs, true);
-- 
1.8.1.5

[PATCH v4 11/15] ALSA: usb-audio: remove altset_idx from snd_usb_substream

[Eldad Zack]

We only need to check the current altsetting in one place, so there's no
benefit from tracking it.
This patch replaces that one check with get_cur_altset_idx() added in
a previous patch and in the proc substream dump routine.

Thanks to Clemens Ladisch for pointing out the difference between
altset_idx and altsetting - the check is changed accordingly to match
against fmt->altsetting.

Note that the proc dump now returns the altsetting number and not the
array index.

This allows removing altset_idx from snd_usb_substream.

Signed-off-by: Eldad Zack <eldad@fogrefinery.com>

---
v2: corrected check.
---
 sound/usb/card.h | 1 -
 sound/usb/pcm.c  | 8 ++------
 sound/usb/pcm.h  | 2 ++
 sound/usb/proc.c | 4 +++-
 4 files changed, 7 insertions(+), 8 deletions(-)

diff --git a/sound/usb/card.h b/sound/usb/card.h
index 7ccacb4..eb38b0a 100644
--- a/sound/usb/card.h
+++ b/sound/usb/card.h
@@ -120,7 +120,6 @@ struct snd_usb_substream {
 	unsigned int period_bytes;	/* current period bytes (for hw_params callback) */
 	unsigned int period_frames;	/* current frames per period */
 	unsigned int buffer_periods;	/* current periods per buffer */
-	unsigned int altset_idx;     /* USB data format: index of alternate setting */
 	unsigned int txfr_quirk:1;	/* allow sub-frame alignment */
 	unsigned int fmt_type;		/* USB audio format type (1-3) */
 	unsigned int pkt_offset_adj;	/* Bytes to drop from beginning of packets (for non-compliant devices) */
diff --git a/sound/usb/pcm.c b/sound/usb/pcm.c
index 7895c81..8854ee5 100644
--- a/sound/usb/pcm.c
+++ b/sound/usb/pcm.c
@@ -216,7 +216,7 @@ int snd_usb_init_pitch(struct snd_usb_audio *chip, int iface,
 	}
 }
 
-static int get_cur_altsetting(struct usb_device *dev, int ifnum)
+int get_cur_altsetting(struct usb_device *dev, int ifnum)
 {
 	struct usb_interface *iface;
 	struct usb_interface_descriptor *altsd;
@@ -527,12 +527,11 @@ static int set_format(struct snd_usb_substream *subs, struct audioformat *fmt)
 				dev->devnum, fmt->iface, fmt->altsetting, err);
 			return -EIO;
 		}
-		subs->altset_idx = 0;
 	}
 
 	/* set interface */
 	if (subs->interface != fmt->iface ||
-	    subs->altset_idx != fmt->altset_idx) {
+	    get_cur_altsetting(subs->dev, subs->interface) != fmt->altsetting) {
 		err = subs_set_interface(subs, fmt->iface, fmt->altsetting);
 		if (err < 0) {
 			snd_printk(KERN_ERR "%d:%d:%d: usb_set_interface failed (%d)\n",
@@ -541,7 +540,6 @@ static int set_format(struct snd_usb_substream *subs, struct audioformat *fmt)
 		}
 		snd_printdd(KERN_INFO "setting usb interface %d:%d\n",
 				fmt->iface, fmt->altsetting);
-		subs->altset_idx = fmt->altset_idx;
 
 		snd_usb_set_interface_quirk(dev);
 	}
@@ -783,7 +781,6 @@ static int snd_usb_hw_params(struct snd_pcm_substream *substream,
 		return ret;
 
 	subs->interface = fmt->iface;
-	subs->altset_idx = fmt->altset_idx;
 	subs->need_setup_ep = true;
 
 	return 0;
@@ -1267,7 +1264,6 @@ static int snd_usb_pcm_open(struct snd_pcm_substream *substream, int direction)
 	struct snd_usb_substream *subs = &as->substream[direction];
 
 	subs->interface = -1;
-	subs->altset_idx = 0;
 	runtime->hw = snd_usb_hardware;
 	runtime->private_data = subs;
 	subs->pcm_substream = substream;
diff --git a/sound/usb/pcm.h b/sound/usb/pcm.h
index df7a003..0c9cb8c 100644
--- a/sound/usb/pcm.h
+++ b/sound/usb/pcm.h
@@ -10,5 +10,7 @@ int snd_usb_init_pitch(struct snd_usb_audio *chip, int iface,
 		       struct usb_host_interface *alts,
 		       struct audioformat *fmt);
 
+int get_cur_altsetting(struct usb_device *dev, int ifnum);
+
 
 #endif /* __USBAUDIO_PCM_H */
diff --git a/sound/usb/proc.c b/sound/usb/proc.c
index 5f761ab..ab6aafe 100644
--- a/sound/usb/proc.c
+++ b/sound/usb/proc.c
@@ -27,6 +27,7 @@
 #include "card.h"
 #include "endpoint.h"
 #include "proc.h"
+#include "pcm.h"
 
 /* convert our full speed USB rate into sampling rate in Hz */
 static inline unsigned get_full_speed_hz(unsigned int usb_rate)
@@ -140,7 +141,8 @@ static void proc_dump_substream_status(struct snd_usb_substream *subs, struct sn
 	if (subs->running) {
 		snd_iprintf(buffer, "  Status: Running\n");
 		snd_iprintf(buffer, "    Interface = %d\n", subs->interface);
-		snd_iprintf(buffer, "    Altset = %d\n", subs->altset_idx);
+		snd_iprintf(buffer, "    Altset = %d\n",
+			    get_cur_altsetting(subs->dev, subs->interface));
 		proc_dump_ep_status(subs, subs->data_endpoint, subs->sync_endpoint, buffer);
 	} else {
 		snd_iprintf(buffer, "  Status: Stop\n");
-- 
1.8.1.5

[PATCH v4 12/15] ALSA: usb-audio: remove unused endpoint flag EP_FLAG_ACTIVATED

[Eldad Zack]

EP_FLAG_ACTIVATED is never tested for, remove it.

Signed-off-by: Eldad Zack <eldad@fogrefinery.com>
---
 sound/usb/endpoint.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/sound/usb/endpoint.c b/sound/usb/endpoint.c
index 6cb3137..8379989 100644
--- a/sound/usb/endpoint.c
+++ b/sound/usb/endpoint.c
@@ -33,7 +33,6 @@
 #include "pcm.h"
 #include "quirks.h"
 
-#define EP_FLAG_ACTIVATED	0
 #define EP_FLAG_RUNNING		1
 #define EP_FLAG_STOPPING	2
 
@@ -1020,8 +1019,6 @@ void snd_usb_endpoint_deactivate(struct snd_usb_endpoint *ep)
 
 	deactivate_urbs(ep, true);
 	wait_clear_urbs(ep);
-
-	clear_bit(EP_FLAG_ACTIVATED, &ep->flags);
 }
 
 /**
-- 
1.8.1.5

[PATCH v4 13/15] ALSA: usb-audio: clear sync subs hw_params

[Eldad Zack]

If the snd_usb_substream associated with the sync endpoint is
not in use as a data endpoint, clear its hw_params.

For implicit feedback, this prevents a failure condition when the
capture substream is closed while the playback substream is open
(here the capture substream does not clear its parameters, since
use_count is 1), the playback is then closed and capture is opened
again. The code will then assume that the endpoint is already
running and will fail later.

Instead of just using the use_count of the endpoint, add a usage bit
to the struct to track if the substream is opened, to allow clearing
the parameters of the other substream if and only if it is not opened.

This flag must be set as soon as the substream is opened, since
the endpoint is started only later (at prepare).

Also group together the bit flags.

Signed-off-by: Eldad Zack <eldad@fogrefinery.com>
---
 sound/usb/card.h |  5 +++--
 sound/usb/pcm.c  | 27 ++++++++++++++++++++++-----
 2 files changed, 25 insertions(+), 7 deletions(-)

diff --git a/sound/usb/card.h b/sound/usb/card.h
index eb38b0a..d9cb674 100644
--- a/sound/usb/card.h
+++ b/sound/usb/card.h
@@ -120,11 +120,12 @@ struct snd_usb_substream {
 	unsigned int period_bytes;	/* current period bytes (for hw_params callback) */
 	unsigned int period_frames;	/* current frames per period */
 	unsigned int buffer_periods;	/* current periods per buffer */
-	unsigned int txfr_quirk:1;	/* allow sub-frame alignment */
 	unsigned int fmt_type;		/* USB audio format type (1-3) */
 	unsigned int pkt_offset_adj;	/* Bytes to drop from beginning of packets (for non-compliant devices) */
 
-	unsigned int running: 1;	/* running status */
+	unsigned int txfr_quirk:1;	/* allow sub-frame alignment */
+	unsigned int running:1;		/* running status */
+	unsigned int used:1;		/* substream used */
 
 	unsigned int hwptr_done;	/* processed byte position in the buffer */
 	unsigned int transfer_done;		/* processed frames since last period update */
diff --git a/sound/usb/pcm.c b/sound/usb/pcm.c
index 8854ee5..7ca9b47 100644
--- a/sound/usb/pcm.c
+++ b/sound/usb/pcm.c
@@ -786,6 +786,20 @@ static int snd_usb_hw_params(struct snd_pcm_substream *substream,
 	return 0;
 }
 
+static void clear_substream(struct snd_usb_substream *subs)
+{
+	if (subs->used != 0)
+		return;
+
+	if (subs->data_endpoint && subs->data_endpoint->use_count != 0)
+		return;
+
+	subs->cur_audiofmt = NULL;
+	subs->cur_rate = 0;
+	subs->period_bytes = 0;
+	subs->channels = 0;
+}
+
 /*
  * hw_free callback
  *
@@ -795,11 +809,13 @@ static int snd_usb_hw_free(struct snd_pcm_substream *substream)
 {
 	struct snd_usb_substream *subs = substream->runtime->private_data;
 
-	if (!subs->data_endpoint || subs->data_endpoint->use_count == 0) {
-		subs->cur_audiofmt = NULL;
-		subs->cur_rate = 0;
-		subs->period_bytes = 0;
-		subs->channels = 0;
+	subs->used = 0;
+	clear_substream(subs);
+
+	if (subs->sync_endpoint) {
+		struct snd_usb_substream *sync_subs =
+			&subs->stream->substream[subs->direction ^ 1];
+		clear_substream(sync_subs);
 	}
 
 	down_read(&subs->stream->chip->shutdown_rwsem);
@@ -1264,6 +1280,7 @@ static int snd_usb_pcm_open(struct snd_pcm_substream *substream, int direction)
 	struct snd_usb_substream *subs = &as->substream[direction];
 
 	subs->interface = -1;
+	subs->used = 1;
 	runtime->hw = snd_usb_hardware;
 	runtime->private_data = subs;
 	subs->pcm_substream = substream;
-- 
1.8.1.5

[PATCH v4 14/15] ALSA: usb-audio: always wait in start_endpoints

[Eldad Zack]

Start the endpoints at prepare also for capture endpoints,
since it might be needed to wait for the URBs to be unlinked.

If an implicit feedback source endpoint stops being used by its
sink endpoint, but immediately used as a data endpoint, usb_submit_urb
will return -EBUSY.

Merge two trigger cases since they are now the same.

Signed-off-by: Eldad Zack <eldad@fogrefinery.com>
---
 sound/usb/endpoint.c |  8 +++-----
 sound/usb/endpoint.h |  2 +-
 sound/usb/pcm.c      | 28 ++++++++++++----------------
 3 files changed, 16 insertions(+), 22 deletions(-)

diff --git a/sound/usb/endpoint.c b/sound/usb/endpoint.c
index 8379989..81056b6 100644
--- a/sound/usb/endpoint.c
+++ b/sound/usb/endpoint.c
@@ -884,18 +884,17 @@ int snd_usb_endpoint_set_params(struct snd_usb_endpoint *ep,
  * snd_usb_endpoint_start: start an snd_usb_endpoint
  *
  * @ep:		the endpoint to start
- * @can_sleep:	flag indicating whether the operation is executed in
- * 		non-atomic context
  *
  * A call to this function will increment the use count of the endpoint.
  * In case it is not already running, the URBs for this endpoint will be
  * submitted. Otherwise, this function does nothing.
  *
  * Must be balanced to calls of snd_usb_endpoint_stop().
+ * This operation can sleep and must not be executed in atomic context.
  *
  * Returns an error if the URB submission failed, 0 in all other cases.
  */
-int snd_usb_endpoint_start(struct snd_usb_endpoint *ep, bool can_sleep)
+int snd_usb_endpoint_start(struct snd_usb_endpoint *ep)
 {
 	int err;
 	unsigned int i;
@@ -909,8 +908,7 @@ int snd_usb_endpoint_start(struct snd_usb_endpoint *ep, bool can_sleep)
 
 	/* just to be sure */
 	deactivate_urbs(ep, false);
-	if (can_sleep)
-		wait_clear_urbs(ep);
+	wait_clear_urbs(ep);
 
 	ep->active_mask = 0;
 	ep->unlink_mask = 0;
diff --git a/sound/usb/endpoint.h b/sound/usb/endpoint.h
index 188675d..22baec8 100644
--- a/sound/usb/endpoint.h
+++ b/sound/usb/endpoint.h
@@ -19,7 +19,7 @@ int snd_usb_endpoint_set_params(struct snd_usb_endpoint *ep,
 				struct snd_usb_endpoint *sync_ep,
 				struct snd_usb_substream *subs);
 
-int  snd_usb_endpoint_start(struct snd_usb_endpoint *ep, bool can_sleep);
+int  snd_usb_endpoint_start(struct snd_usb_endpoint *ep);
 void snd_usb_endpoint_stop(struct snd_usb_endpoint *ep);
 void snd_usb_endpoint_sync_pending_stop(struct snd_usb_endpoint *ep);
 int  snd_usb_endpoint_activate(struct snd_usb_endpoint *ep);
diff --git a/sound/usb/pcm.c b/sound/usb/pcm.c
index 7ca9b47..f3d48d4 100644
--- a/sound/usb/pcm.c
+++ b/sound/usb/pcm.c
@@ -270,7 +270,7 @@ static int subs_set_interface(struct snd_usb_substream *subs, int ifnum,
 	return 0;
 }
 
-static int start_endpoints(struct snd_usb_substream *subs, bool can_sleep)
+static int start_endpoints(struct snd_usb_substream *subs)
 {
 	int err;
 
@@ -283,7 +283,7 @@ static int start_endpoints(struct snd_usb_substream *subs, bool can_sleep)
 		snd_printdd(KERN_DEBUG "Starting data EP @%p\n", ep);
 
 		ep->data_subs = subs;
-		err = snd_usb_endpoint_start(ep, can_sleep);
+		err = snd_usb_endpoint_start(ep);
 		if (err < 0) {
 			clear_bit(SUBSTREAM_FLAG_DATA_EP_STARTED, &subs->flags);
 			return err;
@@ -313,7 +313,7 @@ static int start_endpoints(struct snd_usb_substream *subs, bool can_sleep)
 		snd_printdd(KERN_DEBUG "Starting sync EP @%p\n", ep);
 
 		ep->sync_slave = subs->data_endpoint;
-		err = snd_usb_endpoint_start(ep, can_sleep);
+		err = snd_usb_endpoint_start(ep);
 		if (err < 0) {
 			clear_bit(SUBSTREAM_FLAG_SYNC_EP_STARTED, &subs->flags);
 			return err;
@@ -893,10 +893,14 @@ static int snd_usb_pcm_prepare(struct snd_pcm_substream *substream)
 	subs->last_frame_number = 0;
 	runtime->delay = 0;
 
-	/* for playback, submit the URBs now; otherwise, the first hwptr_done
-	 * updates for all URBs would happen at the same time when starting */
-	if (subs->direction == SNDRV_PCM_STREAM_PLAYBACK)
-		ret = start_endpoints(subs, true);
+	/*
+	 * for playback, submit the URBs now; otherwise, the first hwptr_done
+	 * updates for all URBs would happen at the same time when starting.
+	 * for capture, waiting is required in case the endpoint is an implicit
+	 * feedback source and it is has just been stopped (by the playback
+	 * substream).
+	 */
+	ret = start_endpoints(subs);
 
  unlock:
 	up_read(&subs->stream->chip->shutdown_rwsem);
@@ -1660,15 +1664,11 @@ static int snd_usb_substream_playback_trigger(struct snd_pcm_substream *substrea
 static int snd_usb_substream_capture_trigger(struct snd_pcm_substream *substream,
 					     int cmd)
 {
-	int err;
 	struct snd_usb_substream *subs = substream->runtime->private_data;
 
 	switch (cmd) {
 	case SNDRV_PCM_TRIGGER_START:
-		err = start_endpoints(subs, false);
-		if (err < 0)
-			return err;
-
+	case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
 		subs->data_endpoint->retire_data_urb = retire_capture_urb;
 		subs->running = 1;
 		return 0;
@@ -1681,10 +1681,6 @@ static int snd_usb_substream_capture_trigger(struct snd_pcm_substream *substream
 		subs->data_endpoint->retire_data_urb = NULL;
 		subs->running = 0;
 		return 0;
-	case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
-		subs->data_endpoint->retire_data_urb = retire_capture_urb;
-		subs->running = 1;
-		return 0;
 	}
 
 	return -EINVAL;
-- 
1.8.1.5

Re: [PATCH v4 14/15] ALSA: usb-audio: always wait in start_endpoints

[Takashi Iwai]

At Sun,  6 Oct 2013 22:31:19 +0200,
Eldad Zack wrote:
> 
> Start the endpoints at prepare also for capture endpoints,
> since it might be needed to wait for the URBs to be unlinked.
> 
> If an implicit feedback source endpoint stops being used by its
> sink endpoint, but immediately used as a data endpoint, usb_submit_urb
> will return -EBUSY.
> 
> Merge two trigger cases since they are now the same.

This change worries me about the timing.  This change means that the
capture stream isn't started at the moment the trigger callback is
called but at the next urb handling.  It means a possible regression
in the case of realtime usage.

Is there any reason to do this except for clean up?  IOW, does this
fix any problem by itself?


Takashi

> 
> Signed-off-by: Eldad Zack <eldad@fogrefinery.com>
> ---
>  sound/usb/endpoint.c |  8 +++-----
>  sound/usb/endpoint.h |  2 +-
>  sound/usb/pcm.c      | 28 ++++++++++++----------------
>  3 files changed, 16 insertions(+), 22 deletions(-)
> 
> diff --git a/sound/usb/endpoint.c b/sound/usb/endpoint.c
> index 8379989..81056b6 100644
> --- a/sound/usb/endpoint.c
> +++ b/sound/usb/endpoint.c
> @@ -884,18 +884,17 @@ int snd_usb_endpoint_set_params(struct snd_usb_endpoint *ep,
>   * snd_usb_endpoint_start: start an snd_usb_endpoint
>   *
>   * @ep:		the endpoint to start
> - * @can_sleep:	flag indicating whether the operation is executed in
> - * 		non-atomic context
>   *
>   * A call to this function will increment the use count of the endpoint.
>   * In case it is not already running, the URBs for this endpoint will be
>   * submitted. Otherwise, this function does nothing.
>   *
>   * Must be balanced to calls of snd_usb_endpoint_stop().
> + * This operation can sleep and must not be executed in atomic context.
>   *
>   * Returns an error if the URB submission failed, 0 in all other cases.
>   */
> -int snd_usb_endpoint_start(struct snd_usb_endpoint *ep, bool can_sleep)
> +int snd_usb_endpoint_start(struct snd_usb_endpoint *ep)
>  {
>  	int err;
>  	unsigned int i;
> @@ -909,8 +908,7 @@ int snd_usb_endpoint_start(struct snd_usb_endpoint *ep, bool can_sleep)
>  
>  	/* just to be sure */
>  	deactivate_urbs(ep, false);
> -	if (can_sleep)
> -		wait_clear_urbs(ep);
> +	wait_clear_urbs(ep);
>  
>  	ep->active_mask = 0;
>  	ep->unlink_mask = 0;
> diff --git a/sound/usb/endpoint.h b/sound/usb/endpoint.h
> index 188675d..22baec8 100644
> --- a/sound/usb/endpoint.h
> +++ b/sound/usb/endpoint.h
> @@ -19,7 +19,7 @@ int snd_usb_endpoint_set_params(struct snd_usb_endpoint *ep,
>  				struct snd_usb_endpoint *sync_ep,
>  				struct snd_usb_substream *subs);
>  
> -int  snd_usb_endpoint_start(struct snd_usb_endpoint *ep, bool can_sleep);
> +int  snd_usb_endpoint_start(struct snd_usb_endpoint *ep);
>  void snd_usb_endpoint_stop(struct snd_usb_endpoint *ep);
>  void snd_usb_endpoint_sync_pending_stop(struct snd_usb_endpoint *ep);
>  int  snd_usb_endpoint_activate(struct snd_usb_endpoint *ep);
> diff --git a/sound/usb/pcm.c b/sound/usb/pcm.c
> index 7ca9b47..f3d48d4 100644
> --- a/sound/usb/pcm.c
> +++ b/sound/usb/pcm.c
> @@ -270,7 +270,7 @@ static int subs_set_interface(struct snd_usb_substream *subs, int ifnum,
>  	return 0;
>  }
>  
> -static int start_endpoints(struct snd_usb_substream *subs, bool can_sleep)
> +static int start_endpoints(struct snd_usb_substream *subs)
>  {
>  	int err;
>  
> @@ -283,7 +283,7 @@ static int start_endpoints(struct snd_usb_substream *subs, bool can_sleep)
>  		snd_printdd(KERN_DEBUG "Starting data EP @%p\n", ep);
>  
>  		ep->data_subs = subs;
> -		err = snd_usb_endpoint_start(ep, can_sleep);
> +		err = snd_usb_endpoint_start(ep);
>  		if (err < 0) {
>  			clear_bit(SUBSTREAM_FLAG_DATA_EP_STARTED, &subs->flags);
>  			return err;
> @@ -313,7 +313,7 @@ static int start_endpoints(struct snd_usb_substream *subs, bool can_sleep)
>  		snd_printdd(KERN_DEBUG "Starting sync EP @%p\n", ep);
>  
>  		ep->sync_slave = subs->data_endpoint;
> -		err = snd_usb_endpoint_start(ep, can_sleep);
> +		err = snd_usb_endpoint_start(ep);
>  		if (err < 0) {
>  			clear_bit(SUBSTREAM_FLAG_SYNC_EP_STARTED, &subs->flags);
>  			return err;
> @@ -893,10 +893,14 @@ static int snd_usb_pcm_prepare(struct snd_pcm_substream *substream)
>  	subs->last_frame_number = 0;
>  	runtime->delay = 0;
>  
> -	/* for playback, submit the URBs now; otherwise, the first hwptr_done
> -	 * updates for all URBs would happen at the same time when starting */
> -	if (subs->direction == SNDRV_PCM_STREAM_PLAYBACK)
> -		ret = start_endpoints(subs, true);
> +	/*
> +	 * for playback, submit the URBs now; otherwise, the first hwptr_done
> +	 * updates for all URBs would happen at the same time when starting.
> +	 * for capture, waiting is required in case the endpoint is an implicit
> +	 * feedback source and it is has just been stopped (by the playback
> +	 * substream).
> +	 */
> +	ret = start_endpoints(subs);
>  
>   unlock:
>  	up_read(&subs->stream->chip->shutdown_rwsem);
> @@ -1660,15 +1664,11 @@ static int snd_usb_substream_playback_trigger(struct snd_pcm_substream *substrea
>  static int snd_usb_substream_capture_trigger(struct snd_pcm_substream *substream,
>  					     int cmd)
>  {
> -	int err;
>  	struct snd_usb_substream *subs = substream->runtime->private_data;
>  
>  	switch (cmd) {
>  	case SNDRV_PCM_TRIGGER_START:
> -		err = start_endpoints(subs, false);
> -		if (err < 0)
> -			return err;
> -
> +	case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
>  		subs->data_endpoint->retire_data_urb = retire_capture_urb;
>  		subs->running = 1;
>  		return 0;
> @@ -1681,10 +1681,6 @@ static int snd_usb_substream_capture_trigger(struct snd_pcm_substream *substream
>  		subs->data_endpoint->retire_data_urb = NULL;
>  		subs->running = 0;
>  		return 0;
> -	case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
> -		subs->data_endpoint->retire_data_urb = retire_capture_urb;
> -		subs->running = 1;
> -		return 0;
>  	}
>  
>  	return -EINVAL;
> -- 
> 1.8.1.5
> 

Re: [PATCH v4 14/15] ALSA: usb-audio: always wait in start_endpoints

[Eldad Zack]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 2734 bytes --]



On Mon, 7 Oct 2013, Takashi Iwai wrote:

> At Sun,  6 Oct 2013 22:31:19 +0200,
> Eldad Zack wrote:
> > 
> > Start the endpoints at prepare also for capture endpoints,
> > since it might be needed to wait for the URBs to be unlinked.
> > 
> > If an implicit feedback source endpoint stops being used by its
> > sink endpoint, but immediately used as a data endpoint, usb_submit_urb
> > will return -EBUSY.
> > 
> > Merge two trigger cases since they are now the same.
> This change worries me about the timing.  This change means that the
> capture stream isn't started at the moment the trigger callback is
> called but at the next urb handling.  It means a possible regression
> in the case of realtime usage.

I'm not sure I understand. Do you mean it might cause the delay between 
capture and playback to vary at each startup?

> Is there any reason to do this except for clean up?  IOW, does this
> fix any problem by itself?

Yes, I only became aware of it since I bumped into it with my test tool.
Attached here - I hope the mailing list accepts attachments.

I reverted the patch right now (mainline rc4 + this series) and this is 
the exact sequence:
$ ./atest --device hw:2 stream --first 0x0038f0
atest v1
Testing device hw:2, rate 96000, range 38f0:0
 * Test stream, Steps:  [pcm_open] [pcm_hw_params] [pcm_prepare] [pcm_start] [waitsleep] [pcm_stop] [pcm_close]
++ Test: stream, permutation: 0x0038f0
  * [1/14] playback => pcm_open
  * [2/14] playback => pcm_hw_params
  * [3/14] playback => pcm_prepare
  * [4/14] playback => pcm_start
  * [5/14] capture => pcm_open
  * [6/14] capture => pcm_hw_params
  * [7/14] capture => pcm_prepare
  * [8/14] capture => pcm_start
  * [9/14] playback => waitsleep
  * [10/14] playback => pcm_stop
  ++ Frames [playback]: 58560
  * [11/14] playback => pcm_close
  * [12/14] capture => waitsleep
  * [13/14] capture => pcm_stop
  ++ Frames [capture]: 5760
  * [14/14] capture => pcm_close
++ permutation: 0x0038f0, runtime: 0.228940 sec
++ Test: stream, permutation: 0x003907
  * [1/14] capture => pcm_open
  * [2/14] capture => pcm_hw_params
  * [3/14] capture => pcm_prepare
  * [4/14] playback => pcm_open
  * [5/14] playback => pcm_hw_params
  * [6/14] playback => pcm_prepare
  * [7/14] playback => pcm_start
  * [8/14] playback => waitsleep
  * [9/14] capture => pcm_start
  * [10/14] playback => pcm_stop
  ++ Frames [playback]: 58560
  * [11/14] playback => pcm_close
capture_thread:209: error -32 on readi 0
 ** IO Error (capture)
++ permutation: 0x003907, runtime: 0.142909 sec
!! Test stream permutation 0x003907 failed !!

...and dmesg shows:

snd_usb_endpoint_start: cannot submit urb 0, error -16: device busy

After I removed the revert, no test fail.

Cheers,
Eldad

[-- Attachment #2: atest-1.tar.bz2 --]
[-- Type: APPLICATION/x-bzip2, Size: 10559 bytes --]

[-- Attachment #3: Type: text/plain, Size: 0 bytes --]

Re: [PATCH v4 14/15] ALSA: usb-audio: always wait in start_endpoints

[Takashi Iwai]

At Mon, 7 Oct 2013 21:26:57 +0200 (CEST),
Eldad Zack wrote:
> 
> 
> 
> On Mon, 7 Oct 2013, Takashi Iwai wrote:
> 
> > At Sun,  6 Oct 2013 22:31:19 +0200,
> > Eldad Zack wrote:
> > > 
> > > Start the endpoints at prepare also for capture endpoints,
> > > since it might be needed to wait for the URBs to be unlinked.
> > > 
> > > If an implicit feedback source endpoint stops being used by its
> > > sink endpoint, but immediately used as a data endpoint, usb_submit_urb
> > > will return -EBUSY.
> > > 
> > > Merge two trigger cases since they are now the same.
> > This change worries me about the timing.  This change means that the
> > capture stream isn't started at the moment the trigger callback is
> > called but at the next urb handling.  It means a possible regression
> > in the case of realtime usage.
> 
> I'm not sure I understand. Do you mean it might cause the delay between 
> capture and playback to vary at each startup?

No, I mean that the actual begin of the recording will be delayed in
comparison with the driver without your patch.  The delay can't be
avoided in this style.

> > Is there any reason to do this except for clean up?  IOW, does this
> > fix any problem by itself?
> 
> Yes, I only became aware of it since I bumped into it with my test tool.
> Attached here - I hope the mailing list accepts attachments.

I understand that this will be required for the implicit feedback
case.  But why applying this to *all* other cases, too, although we
know that this may regress slightly with respect to the latency?
This is my biggest concern.


thanks,

Takashi

> I reverted the patch right now (mainline rc4 + this series) and this is 
> the exact sequence:
> $ ./atest --device hw:2 stream --first 0x0038f0
> atest v1
> Testing device hw:2, rate 96000, range 38f0:0
>  * Test stream, Steps:  [pcm_open] [pcm_hw_params] [pcm_prepare] [pcm_start] [waitsleep] [pcm_stop] [pcm_close]
> ++ Test: stream, permutation: 0x0038f0
>   * [1/14] playback => pcm_open
>   * [2/14] playback => pcm_hw_params
>   * [3/14] playback => pcm_prepare
>   * [4/14] playback => pcm_start
>   * [5/14] capture => pcm_open
>   * [6/14] capture => pcm_hw_params
>   * [7/14] capture => pcm_prepare
>   * [8/14] capture => pcm_start
>   * [9/14] playback => waitsleep
>   * [10/14] playback => pcm_stop
>   ++ Frames [playback]: 58560
>   * [11/14] playback => pcm_close
>   * [12/14] capture => waitsleep
>   * [13/14] capture => pcm_stop
>   ++ Frames [capture]: 5760
>   * [14/14] capture => pcm_close
> ++ permutation: 0x0038f0, runtime: 0.228940 sec
> ++ Test: stream, permutation: 0x003907
>   * [1/14] capture => pcm_open
>   * [2/14] capture => pcm_hw_params
>   * [3/14] capture => pcm_prepare
>   * [4/14] playback => pcm_open
>   * [5/14] playback => pcm_hw_params
>   * [6/14] playback => pcm_prepare
>   * [7/14] playback => pcm_start
>   * [8/14] playback => waitsleep
>   * [9/14] capture => pcm_start
>   * [10/14] playback => pcm_stop
>   ++ Frames [playback]: 58560
>   * [11/14] playback => pcm_close
> capture_thread:209: error -32 on readi 0
>  ** IO Error (capture)
> ++ permutation: 0x003907, runtime: 0.142909 sec
> !! Test stream permutation 0x003907 failed !!
> 
> ...and dmesg shows:
> 
> snd_usb_endpoint_start: cannot submit urb 0, error -16: device busy
> 
> After I removed the revert, no test fail.
> 
> Cheers,
> Eldad

Re: [PATCH v4 14/15] ALSA: usb-audio: always wait in start_endpoints

[Eldad Zack]

On Tue, 8 Oct 2013, Takashi Iwai wrote:

> At Mon, 7 Oct 2013 21:26:57 +0200 (CEST),
> Eldad Zack wrote:
> > 
> > 
> > 
> > On Mon, 7 Oct 2013, Takashi Iwai wrote:
> > 
> > > At Sun,  6 Oct 2013 22:31:19 +0200,
> > > Eldad Zack wrote:
> > > > 
> > > > Start the endpoints at prepare also for capture endpoints,
> > > > since it might be needed to wait for the URBs to be unlinked.
> > > > 
> > > > If an implicit feedback source endpoint stops being used by its
> > > > sink endpoint, but immediately used as a data endpoint, usb_submit_urb
> > > > will return -EBUSY.
> > > > 
> > > > Merge two trigger cases since they are now the same.
> > > This change worries me about the timing.  This change means that the
> > > capture stream isn't started at the moment the trigger callback is
> > > called but at the next urb handling.  It means a possible regression
> > > in the case of realtime usage.
> > 
> > I'm not sure I understand. Do you mean it might cause the delay between 
> > capture and playback to vary at each startup?
> 
> No, I mean that the actual begin of the recording will be delayed in
> comparison with the driver without your patch.  The delay can't be
> avoided in this style.

Ah. I see what you mean.

> > > Is there any reason to do this except for clean up?  IOW, does this
> > > fix any problem by itself?
> > 
> > Yes, I only became aware of it since I bumped into it with my test tool.
> > Attached here - I hope the mailing list accepts attachments.
> 
> I understand that this will be required for the implicit feedback
> case.  But why applying this to *all* other cases, too, although we
> know that this may regress slightly with respect to the latency?
> This is my biggest concern.

Yes, I understand now. I'll of course change that along with all the 
rest and post a new set.
Thanks!

Cheers,
Eldad

[PATCH v4 15/15] ALSA: usb-audio: improve logging messages

[Eldad Zack]

Add -EBUSY to the list of returned USB error strings.
Change some debugging messages to KERN_DEBUG.
Add the calling function for usb_submit_urb related errors.

Signed-off-by: Eldad Zack <eldad@fogrefinery.com>
---
 sound/usb/endpoint.c | 12 +++++++-----
 sound/usb/pcm.c      |  4 ++--
 2 files changed, 9 insertions(+), 7 deletions(-)

diff --git a/sound/usb/endpoint.c b/sound/usb/endpoint.c
index 81056b6..ac4dedf 100644
--- a/sound/usb/endpoint.c
+++ b/sound/usb/endpoint.c
@@ -114,6 +114,8 @@ static const char *usb_error_string(int err)
 	case -EFBIG:
 	case -EMSGSIZE:
 		return "internal error";
+	case -EBUSY:
+		return "device busy";
 	default:
 		return "unknown error";
 	}
@@ -333,8 +335,8 @@ static void queue_pending_output_urbs(struct snd_usb_endpoint *ep)
 
 		err = usb_submit_urb(ctx->urb, GFP_ATOMIC);
 		if (err < 0)
-			snd_printk(KERN_ERR "Unable to submit urb #%d: %d (urb %p)\n",
-				   ctx->index, err, ctx->urb);
+			snd_printk(KERN_ERR "%s: cannot submit urb #%d: %d (urb %p)\n",
+				   __func__, ctx->index, err, ctx->urb);
 		else
 			set_bit(ctx->index, &ep->active_mask);
 	}
@@ -387,7 +389,7 @@ static void snd_complete_urb(struct urb *urb)
 	if (err == 0)
 		return;
 
-	snd_printk(KERN_ERR "cannot submit urb (err = %d)\n", err);
+	snd_printk(KERN_ERR "%s: cannot submit urb (err = %d)\n", __func__, err);
 	//snd_pcm_stop(substream, SNDRV_PCM_STATE_XRUN);
 
 exit_clear:
@@ -948,8 +950,8 @@ int snd_usb_endpoint_start(struct snd_usb_endpoint *ep)
 
 		err = usb_submit_urb(urb, GFP_ATOMIC);
 		if (err < 0) {
-			snd_printk(KERN_ERR "cannot submit urb %d, error %d: %s\n",
-				   i, err, usb_error_string(err));
+			snd_printk(KERN_ERR "%s: cannot submit urb %d, error %d: %s\n",
+				   __func__, i, err, usb_error_string(err));
 			goto __error;
 		}
 		set_bit(i, &ep->active_mask);
diff --git a/sound/usb/pcm.c b/sound/usb/pcm.c
index f3d48d4..8897da7 100644
--- a/sound/usb/pcm.c
+++ b/sound/usb/pcm.c
@@ -606,7 +606,7 @@ static int match_endpoint_audioformats(struct audioformat *fp,
 	if (fp->channels == match->channels)
 		score++;
 
-	snd_printdd("%s: (fmt @%p) score %d\n", __func__, fp, score);
+	snd_printdd(KERN_DEBUG "%s: (fmt @%p) score %d\n", __func__, fp, score);
 
 	return score;
 }
@@ -660,7 +660,7 @@ static int configure_sync_endpoint(struct snd_usb_substream *subs)
 	if (sync_fp->channels != subs->channels) {
 		sync_period_bytes = (subs->period_bytes / subs->channels) *
 			sync_fp->channels;
-		snd_printdd("%s: adjusted sync ep period bytes (%d -> %d)\n",
+		snd_printdd(KERN_DEBUG "%s: adjusted sync ep period bytes (%d -> %d)\n",
 			__func__, subs->period_bytes, sync_period_bytes);
 	}
 
-- 
1.8.1.5

Re: [PATCH v4 15/15] ALSA: usb-audio: improve logging messages

[Takashi Iwai]

At Sun,  6 Oct 2013 22:31:20 +0200,
Eldad Zack wrote:
> 
> Add -EBUSY to the list of returned USB error strings.
> Change some debugging messages to KERN_DEBUG.

snd_printdd() implies KERN_DEBUG as default, so it's superfluous.


Takashi

> Add the calling function for usb_submit_urb related errors.
> 
> Signed-off-by: Eldad Zack <eldad@fogrefinery.com>
> ---
>  sound/usb/endpoint.c | 12 +++++++-----
>  sound/usb/pcm.c      |  4 ++--
>  2 files changed, 9 insertions(+), 7 deletions(-)
> 
> diff --git a/sound/usb/endpoint.c b/sound/usb/endpoint.c
> index 81056b6..ac4dedf 100644
> --- a/sound/usb/endpoint.c
> +++ b/sound/usb/endpoint.c
> @@ -114,6 +114,8 @@ static const char *usb_error_string(int err)
>  	case -EFBIG:
>  	case -EMSGSIZE:
>  		return "internal error";
> +	case -EBUSY:
> +		return "device busy";
>  	default:
>  		return "unknown error";
>  	}
> @@ -333,8 +335,8 @@ static void queue_pending_output_urbs(struct snd_usb_endpoint *ep)
>  
>  		err = usb_submit_urb(ctx->urb, GFP_ATOMIC);
>  		if (err < 0)
> -			snd_printk(KERN_ERR "Unable to submit urb #%d: %d (urb %p)\n",
> -				   ctx->index, err, ctx->urb);
> +			snd_printk(KERN_ERR "%s: cannot submit urb #%d: %d (urb %p)\n",
> +				   __func__, ctx->index, err, ctx->urb);
>  		else
>  			set_bit(ctx->index, &ep->active_mask);
>  	}
> @@ -387,7 +389,7 @@ static void snd_complete_urb(struct urb *urb)
>  	if (err == 0)
>  		return;
>  
> -	snd_printk(KERN_ERR "cannot submit urb (err = %d)\n", err);
> +	snd_printk(KERN_ERR "%s: cannot submit urb (err = %d)\n", __func__, err);
>  	//snd_pcm_stop(substream, SNDRV_PCM_STATE_XRUN);
>  
>  exit_clear:
> @@ -948,8 +950,8 @@ int snd_usb_endpoint_start(struct snd_usb_endpoint *ep)
>  
>  		err = usb_submit_urb(urb, GFP_ATOMIC);
>  		if (err < 0) {
> -			snd_printk(KERN_ERR "cannot submit urb %d, error %d: %s\n",
> -				   i, err, usb_error_string(err));
> +			snd_printk(KERN_ERR "%s: cannot submit urb %d, error %d: %s\n",
> +				   __func__, i, err, usb_error_string(err));
>  			goto __error;
>  		}
>  		set_bit(i, &ep->active_mask);
> diff --git a/sound/usb/pcm.c b/sound/usb/pcm.c
> index f3d48d4..8897da7 100644
> --- a/sound/usb/pcm.c
> +++ b/sound/usb/pcm.c
> @@ -606,7 +606,7 @@ static int match_endpoint_audioformats(struct audioformat *fp,
>  	if (fp->channels == match->channels)
>  		score++;
>  
> -	snd_printdd("%s: (fmt @%p) score %d\n", __func__, fp, score);
> +	snd_printdd(KERN_DEBUG "%s: (fmt @%p) score %d\n", __func__, fp, score);
>  
>  	return score;
>  }
> @@ -660,7 +660,7 @@ static int configure_sync_endpoint(struct snd_usb_substream *subs)
>  	if (sync_fp->channels != subs->channels) {
>  		sync_period_bytes = (subs->period_bytes / subs->channels) *
>  			sync_fp->channels;
> -		snd_printdd("%s: adjusted sync ep period bytes (%d -> %d)\n",
> +		snd_printdd(KERN_DEBUG "%s: adjusted sync ep period bytes (%d -> %d)\n",
>  			__func__, subs->period_bytes, sync_period_bytes);
>  	}
>  
> -- 
> 1.8.1.5
> 

Re: [PATCH v4 00/15] ALSA: usb-audio: fix playback/capture concurrent usage

[Takashi Iwai]

At Sun,  6 Oct 2013 22:31:05 +0200,
Eldad Zack wrote:
> 
> Hi,
> 
> This patch series attempts to fix a long standing problem with the concurrent
> usage of playback and capture on implicit feedback devices.
> 
> Now the following work properly on my system:
> * Full-duplex with jack :)
> 
> * All tests I ran with my tool. Will post it in case anyone want to try it,
>   it takes about 15 minutes to run all the tests.
> 
> * Starting a capture-only jackd instance, e.g.:
>     jackd -n 1 -d alsa -d hw:2,0 -C
>   and then playback-only:
>     jackd -n 2 -d alsa -d hw:2,0 -P
>   and then stop and start the playback.
>   The same with playback running and restarting capture.
> 
> * Trying to start a second substream with parameter mismatch (rate or
>   period bytes is all I can test with my device) will fail,
>   without ill effects.
> 
> With this series applied there are now 3 usage tracking variables:
>  - substream usage flag
>  - endpoint param_set (to protect the parameters until the endpoint is started)
>  - the existing endpoint use_count
> 
> Changes from v3:
> * Added substream "used" flag, to prevent failure in some combinations of ops. (#13)
> * Moved start_endpoints for capture to prepare (#14)
> * Added more constraints to the endpoint_may_set_params function (#10)
> * Cleanups (#12, #15)
> 
> v3:
> http://mailman.alsa-project.org/pipermail/alsa-devel/2013-August/065744.html
> 
> Applies against for-next, tested on upstream 3.12-rc3 (needs Alan Stern's recent
> patch "improve buffer size computations for USB PCM audio" to apply cleanly
> on current mainline).
> 
> Daniel and Nikolay: if you can test again with this series I'd appreciate it.
> 
> Cheers,
> Eldad
> 
> Eldad Zack (15):
>   ALSA: usb-audio: remove unused parameter from sync_ep_set_params
>   ALSA: usb-audio: remove deactivate_endpoints()
>   ALSA: usb-audio: prevent NULL dereference on stop trigger
>   ALSA: usb-audio: don't deactivate URBs on in-use EP
>   ALSA: usb-audio: void return type of snd_usb_endpoint_deactivate()
>   ALSA: usb-audio: clear SUBSTREAM_FLAG_SYNC_EP_STARTED on error
>   ALSA: usb-audio: correct ep use_count semantics (add set_param flag)
>   ALSA: usb-audio: rename alt_idx to altsetting
>   ALSA: usb-audio: conditional interface altsetting
>   ALSA: usb-audio: conditional concurrent usage of endpoint
>   ALSA: usb-audio: remove altset_idx from snd_usb_substream
>   ALSA: usb-audio: remove unused endpoint flag EP_FLAG_ACTIVATED
>   ALSA: usb-audio: clear sync subs hw_params
>   ALSA: usb-audio: always wait in start_endpoints
>   ALSA: usb-audio: improve logging messages

For reducing the further development, I applied trivial cleanup / fix
patches now, namely, below have been merged now:
  ALSA: usb-audio: remove unused parameter from sync_ep_set_params
  ALSA: usb-audio: remove deactivate_endpoints()
  ALSA: usb-audio: don't deactivate URBs on in-use EP
  ALSA: usb-audio: void return type of snd_usb_endpoint_deactivate()
  ALSA: usb-audio: clear SUBSTREAM_FLAG_SYNC_EP_STARTED on error
  ALSA: usb-audio: rename alt_idx to altsetting
  ALSA: usb-audio: remove unused endpoint flag EP_FLAG_ACTIVATED

The patches below need trivial fixes:
  ALSA: usb-audio: prevent NULL dereference on stop trigger
  ALSA: usb-audio: improve logging messages

And these need reconsideration:
  ALSA: usb-audio: always wait in start_endpoints
  ALSA: usb-audio: correct ep use_count semantics (add set_param flag)

I'll check the rest later.


thanks,

Takashi

> 
>  sound/usb/card.h     |   9 ++-
>  sound/usb/endpoint.c | 122 ++++++++++++++++++++---------
>  sound/usb/endpoint.h |  13 +++-
>  sound/usb/pcm.c      | 214 ++++++++++++++++++++++++++++++++++++---------------
>  sound/usb/pcm.h      |   2 +
>  sound/usb/proc.c     |   4 +-
>  6 files changed, 258 insertions(+), 106 deletions(-)
> 
> -- 
> 1.8.1.5
> 

Re: [PATCH v4 00/15] ALSA: usb-audio: fix playback/capture concurrent usage

[Eldad Zack]

On Mon, 7 Oct 2013, Takashi Iwai wrote:

> At Sun,  6 Oct 2013 22:31:05 +0200,
> Eldad Zack wrote:
> > 
> > Hi,
> > 
> > This patch series attempts to fix a long standing problem with the concurrent
> > usage of playback and capture on implicit feedback devices.
[snip]
> 
> For reducing the further development, I applied trivial cleanup / fix
> patches now, namely, below have been merged now:

Thanks, Takashi!

>   ALSA: usb-audio: remove unused parameter from sync_ep_set_params
>   ALSA: usb-audio: remove deactivate_endpoints()
>   ALSA: usb-audio: don't deactivate URBs on in-use EP
>   ALSA: usb-audio: void return type of snd_usb_endpoint_deactivate()
>   ALSA: usb-audio: clear SUBSTREAM_FLAG_SYNC_EP_STARTED on error
>   ALSA: usb-audio: rename alt_idx to altsetting
>   ALSA: usb-audio: remove unused endpoint flag EP_FLAG_ACTIVATED
> 
> The patches below need trivial fixes:
>   ALSA: usb-audio: prevent NULL dereference on stop trigger
>   ALSA: usb-audio: improve logging messages
> 
> And these need reconsideration:
>   ALSA: usb-audio: always wait in start_endpoints
>   ALSA: usb-audio: correct ep use_count semantics (add set_param flag)
> 
> I'll check the rest later.

Thanks for the very fast response and the review! I'll try to fix all 
the issues for the rest and repost.

Cheers,
Eldad

Re: [PATCH v4 00/15] ALSA: usb-audio: fix playback/capture concurrent usage

[Nikolay Martynov]

Hi.

I've tried this series applied against 3.11.6 in ubuntu with MS headset -
seems to be working fine with Skype.

Thanks!


2013/10/6 Eldad Zack <eldad@fogrefinery.com>

> Hi,
>
> This patch series attempts to fix a long standing problem with the
> concurrent
> usage of playback and capture on implicit feedback devices.
>
> Now the following work properly on my system:
> * Full-duplex with jack :)
>
> * All tests I ran with my tool. Will post it in case anyone want to try it,
>   it takes about 15 minutes to run all the tests.
>
> * Starting a capture-only jackd instance, e.g.:
>     jackd -n 1 -d alsa -d hw:2,0 -C
>   and then playback-only:
>     jackd -n 2 -d alsa -d hw:2,0 -P
>   and then stop and start the playback.
>   The same with playback running and restarting capture.
>
> * Trying to start a second substream with parameter mismatch (rate or
>   period bytes is all I can test with my device) will fail,
>   without ill effects.
>
> With this series applied there are now 3 usage tracking variables:
>  - substream usage flag
>  - endpoint param_set (to protect the parameters until the endpoint is
> started)
>  - the existing endpoint use_count
>
> Changes from v3:
> * Added substream "used" flag, to prevent failure in some combinations of
> ops. (#13)
> * Moved start_endpoints for capture to prepare (#14)
> * Added more constraints to the endpoint_may_set_params function (#10)
> * Cleanups (#12, #15)
>
> v3:
>
> http://mailman.alsa-project.org/pipermail/alsa-devel/2013-August/065744.html
>
> Applies against for-next, tested on upstream 3.12-rc3 (needs Alan Stern's
> recent
> patch "improve buffer size computations for USB PCM audio" to apply cleanly
> on current mainline).
>
> Daniel and Nikolay: if you can test again with this series I'd appreciate
> it.
>
> Cheers,
> Eldad
>
> Eldad Zack (15):
>   ALSA: usb-audio: remove unused parameter from sync_ep_set_params
>   ALSA: usb-audio: remove deactivate_endpoints()
>   ALSA: usb-audio: prevent NULL dereference on stop trigger
>   ALSA: usb-audio: don't deactivate URBs on in-use EP
>   ALSA: usb-audio: void return type of snd_usb_endpoint_deactivate()
>   ALSA: usb-audio: clear SUBSTREAM_FLAG_SYNC_EP_STARTED on error
>   ALSA: usb-audio: correct ep use_count semantics (add set_param flag)
>   ALSA: usb-audio: rename alt_idx to altsetting
>   ALSA: usb-audio: conditional interface altsetting
>   ALSA: usb-audio: conditional concurrent usage of endpoint
>   ALSA: usb-audio: remove altset_idx from snd_usb_substream
>   ALSA: usb-audio: remove unused endpoint flag EP_FLAG_ACTIVATED
>   ALSA: usb-audio: clear sync subs hw_params
>   ALSA: usb-audio: always wait in start_endpoints
>   ALSA: usb-audio: improve logging messages
>
>  sound/usb/card.h     |   9 ++-
>  sound/usb/endpoint.c | 122 ++++++++++++++++++++---------
>  sound/usb/endpoint.h |  13 +++-
>  sound/usb/pcm.c      | 214
> ++++++++++++++++++++++++++++++++++++---------------
>  sound/usb/pcm.h      |   2 +
>  sound/usb/proc.c     |   4 +-
>  6 files changed, 258 insertions(+), 106 deletions(-)
>
> --
> 1.8.1.5
>
>


-- 
Martynov Nikolay.
Email: mar.kolya@gmail.com