[alsa-devel] [PATCH] Add M2Tech hiFace USB-SPDIF driver

[Takashi Iwai]

At Tue, 12 Feb 2013 15:29:19 +0100,
Clemens Ladisch wrote:
> 
> >>> +			snd_pcm_stop(rt->playback.instance,
> >>> +					SNDRV_PCM_STATE_XRUN);
> >>
> >> This requres locking the stream with something like
> >> snd_pcm_stream_lock_irq().
> >
> > OK, but would you mind elaborating a little more why this is needed?
> >
> > I do not see other drivers doing that,
> 
> sound/firewire/{amdtp,isight}.c do.  I've never said that all those
> other drivers would be correct.  (And you have to be careful to avoid
> deadlocks.)
> 
> > and BTW I see also that some other drivers not calling snd_pcm_stop()
> > at all, e.g. it is commented in sound/usb/endpoint.c::snd_complete_urb().
> 
> It's commented because it would crash.  That driver isn't in a very
> good state regarding state changes.

The problem is that you can't kill the urb itself gracefully while in
the complete callback.  But a patch like below could work a little bit
better, although it doesn't notify the XRUN at that moment.


Takashi

---
diff --git a/sound/usb/card.h b/sound/usb/card.h
index 8a751b4..d98e7bc 100644
--- a/sound/usb/card.h
+++ b/sound/usb/card.h
@@ -118,6 +118,7 @@ struct snd_usb_substream {
 	unsigned int fmt_type;		/* USB audio format type (1-3) */
 
 	unsigned int running: 1;	/* running status */
+	unsigned int xrun:1;
 
 	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/endpoint.c b/sound/usb/endpoint.c
index 21049b8..e52aee9 100644
--- a/sound/usb/endpoint.c
+++ b/sound/usb/endpoint.c
@@ -357,6 +357,9 @@ static void snd_complete_urb(struct urb *urb)
 		     ep->chip->shutdown))		/* device disconnected */
 		goto exit_clear;
 
+	if (ep->data_subs && ep->data_subs->xrun)
+		goto exit_clear;
+
 	if (usb_pipeout(ep->pipe)) {
 		retire_outbound_urb(ep, ctx);
 		/* can be stopped during retire callback */
@@ -389,7 +392,8 @@ static void snd_complete_urb(struct urb *urb)
 		return;
 
 	snd_printk(KERN_ERR "cannot submit urb (err = %d)\n", err);
-	//snd_pcm_stop(substream, SNDRV_PCM_STATE_XRUN);
+	if (ep->data_subs)
+		ep->data_subs->xrun = 1;
 
 exit_clear:
 	clear_bit(ctx->index, &ep->active_mask);
diff --git a/sound/usb/pcm.c b/sound/usb/pcm.c
index f94397b..6be6662 100644
--- a/sound/usb/pcm.c
+++ b/sound/usb/pcm.c
@@ -79,7 +79,7 @@ static snd_pcm_uframes_t snd_usb_pcm_pointer(struct snd_pcm_substream *substream
 	unsigned int hwptr_done;
 
 	subs = (struct snd_usb_substream *)substream->runtime->private_data;
-	if (subs->stream->chip->shutdown)
+	if (subs->xrun || subs->stream->chip->shutdown)
 		return SNDRV_PCM_POS_XRUN;
 	spin_lock(&subs->lock);
 	hwptr_done = subs->hwptr_done;
@@ -726,6 +726,7 @@ static int snd_usb_pcm_prepare(struct snd_pcm_substream *substream)
 	subs->transfer_done = 0;
 	subs->last_delay = 0;
 	subs->last_frame_number = 0;
+	subs->xrun = 0;
 	runtime->delay = 0;
 
 	/* for playback, submit the URBs now; otherwise, the first hwptr_done