[alsa-devel] [PATCH] ASoC: rsnd: stop all working stream when .remove

[Takashi Iwai]

Morimoto-san,

sorry for the late reply.  It took time until I digest all pending
stuff after vacation.

On Wed, 27 Sep 2017 07:14:21 +0200,
Kuninori Morimoto wrote:
> 
> 
> Hi Takashi
> 
> > snd_pcm_drop() has this check
> > 
> > 	if (runtime->status->state == SNDRV_PCM_STATE_OPEN ||
> > 	    runtime->status->state == SNDRV_PCM_STATE_DISCONNECTED)
> > 		return -EBADFD;
> (snip)
> > Then, 2nd issue happen on snd_pcm_stop(). it will call snd_pcm_do_stop(),
> > and it has
> > 
> > 	if (substream->runtime->trigger_master == substream &&
> > 	    snd_pcm_running(substream))
> > 		substream->ops->trigger(substream, SNDRV_PCM_TRIGGER_STOP);
> 
> I used your patch, and modified above, then, hot-unplug
> during playback stops correctly without kernel panic.
> snd_pcm_drop() and snd_pcm_do_stop() care about
> SNDRV_PCM_STATE_DISCONNECTED on this patch.
> I think it means, "it should be stopped immediately
> if it was disconnected".
> But, I don't know this is OK or Not.
> 
> I added my local patch on this mail.
> Maybe we want to separate this patch into few small patches.
> but can you review this ?
> It is including
>  - your patch
>  - snd_pcm_stop() snd_pcm_do_stop() care DISCONNECTED

That needs a bit more investigation.
When the device is disconnected, not all drivers expect that further
PCM operations are done for non-existing devices.  We might need
either some flag to allow/prefer the stop-after-disconnection, or
rethink whether we should actually stop at snd_pcm_dev_disconnect()
like below.

>  - ASoC version of snd_card_disconnect_sync()
>  - user driver (= rsnd) uses snd_soc_card_disconnect_sync()

Yes, these should be split.


thanks,

Takashi

---
diff --git a/sound/core/pcm.c b/sound/core/pcm.c
index 7eadb7fd8074..054e47ad23ed 100644
--- a/sound/core/pcm.c
+++ b/sound/core/pcm.c
@@ -1152,6 +1152,10 @@ static int snd_pcm_dev_disconnect(struct snd_device *device)
 		for (substream = pcm->streams[cidx].substream; substream; substream = substream->next) {
 			snd_pcm_stream_lock_irq(substream);
 			if (substream->runtime) {
+				if (snd_pcm_running(substream))
+					snd_pcm_stop(substream,
+						     SNDRV_PCM_STATE_DISCONNECTED);
+				/* to be sure, set the state unconditionally */
 				substream->runtime->status->state = SNDRV_PCM_STATE_DISCONNECTED;
 				wake_up(&substream->runtime->sleep);
 				wake_up(&substream->runtime->tsleep);