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

[Kuninori Morimoto]

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
 - ASoC version of snd_card_disconnect_sync()
 - user driver (= rsnd) uses snd_soc_card_disconnect_sync()

---------------
diff --git a/include/sound/core.h b/include/sound/core.h
index 4104a9d..5f181b8 100644
--- a/include/sound/core.h
+++ b/include/sound/core.h
@@ -133,6 +133,7 @@ struct snd_card {
 	struct device card_dev;		/* cardX object for sysfs */
 	const struct attribute_group *dev_groups[4]; /* assigned sysfs attr */
 	bool registered;		/* card_dev is registered? */
+	wait_queue_head_t remove_sleep;
 
 #ifdef CONFIG_PM
 	unsigned int power_state;	/* power state */
@@ -240,6 +241,7 @@ int snd_card_new(struct device *parent, int idx, const char *xid,
 		 struct snd_card **card_ret);
 
 int snd_card_disconnect(struct snd_card *card);
+void snd_card_disconnect_sync(struct snd_card *card);
 int snd_card_free(struct snd_card *card);
 int snd_card_free_when_closed(struct snd_card *card);
 void snd_card_set_id(struct snd_card *card, const char *id);
diff --git a/include/sound/soc.h b/include/sound/soc.h
index 4f05b0e..56d02f0 100644
--- a/include/sound/soc.h
+++ b/include/sound/soc.h
@@ -464,6 +464,8 @@ struct snd_soc_component *snd_soc_lookup_component(struct device *dev,
 int snd_soc_new_compress(struct snd_soc_pcm_runtime *rtd, int num);
 #endif
 
+void snd_soc_card_disconnect_sync(struct device *dev);
+
 struct snd_pcm_substream *snd_soc_get_dai_substream(struct snd_soc_card *card,
 		const char *dai_link, int stream);
 struct snd_soc_pcm_runtime *snd_soc_get_pcm_runtime(struct snd_soc_card *card,
diff --git a/sound/core/init.c b/sound/core/init.c
index 32ebe2f..f7f7050 100644
--- a/sound/core/init.c
+++ b/sound/core/init.c
@@ -255,6 +255,7 @@ int snd_card_new(struct device *parent, int idx, const char *xid,
 #ifdef CONFIG_PM
 	init_waitqueue_head(&card->power_sleep);
 #endif
+	init_waitqueue_head(&card->remove_sleep);
 
 	device_initialize(&card->card_dev);
 	card->card_dev.parent = parent;
@@ -452,6 +453,21 @@ int snd_card_disconnect(struct snd_card *card)
 }
 EXPORT_SYMBOL(snd_card_disconnect);
 
+void snd_card_disconnect_sync(struct snd_card *card)
+{
+	DECLARE_COMPLETION(comp);
+
+	if (snd_card_disconnect(card) < 0)
+		return;
+
+	spin_lock_irq(&card->files_lock);
+	wait_event_lock_irq(card->remove_sleep,
+			    list_empty(&card->files_list),
+			    card->files_lock);
+	spin_unlock_irq(&card->files_lock);
+}
+EXPORT_SYMBOL_GPL(snd_card_disconnect_sync);
+
 static int snd_card_do_free(struct snd_card *card)
 {
 #if IS_ENABLED(CONFIG_SND_MIXER_OSS)
@@ -957,6 +973,8 @@ int snd_card_file_remove(struct snd_card *card, struct file *file)
 			break;
 		}
 	}
+	if (list_empty(&card->files_list))
+		wake_up_all(&card->remove_sleep);
 	spin_unlock(&card->files_lock);
 	if (!found) {
 		dev_err(card->dev, "card file remove problem (%p)\n", file);
diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c
index 2fec2fe..bc8124a 100644
--- a/sound/core/pcm_native.c
+++ b/sound/core/pcm_native.c
@@ -1239,9 +1239,11 @@ static int snd_pcm_pre_stop(struct snd_pcm_substream *substream, int state)
 
 static int snd_pcm_do_stop(struct snd_pcm_substream *substream, int state)
 {
-	if (substream->runtime->trigger_master == substream &&
-	    snd_pcm_running(substream))
+	if ((substream->runtime->trigger_master == substream) &&
+	    (snd_pcm_running(substream) ||
+	     substream->runtime->status->state == SNDRV_PCM_STATE_DISCONNECTED))
 		substream->ops->trigger(substream, SNDRV_PCM_TRIGGER_STOP);
+
 	return 0; /* unconditonally stop all substreams */
 }
 
@@ -1882,8 +1884,7 @@ static int snd_pcm_drop(struct snd_pcm_substream *substream)
 		return -ENXIO;
 	runtime = substream->runtime;
 
-	if (runtime->status->state == SNDRV_PCM_STATE_OPEN ||
-	    runtime->status->state == SNDRV_PCM_STATE_DISCONNECTED)
+	if (runtime->status->state == SNDRV_PCM_STATE_OPEN)
 		return -EBADFD;
 
 	snd_pcm_stream_lock_irq(substream);
diff --git a/sound/soc/sh/rcar/core.c b/sound/soc/sh/rcar/core.c
index 42366ce..31a6889 100644
--- a/sound/soc/sh/rcar/core.c
+++ b/sound/soc/sh/rcar/core.c
@@ -1473,6 +1473,8 @@ static int rsnd_remove(struct platform_device *pdev)
 		ret |= rsnd_dai_call(remove, &rdai->capture, priv);
 	}
 
+	snd_soc_card_disconnect_sync(&pdev->dev);
+
 	for (i = 0; i < ARRAY_SIZE(remove_func); i++)
 		remove_func[i](priv);
 
diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index 0e2c34e..bdb91aa 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -1136,6 +1136,16 @@ static int soc_init_dai_link(struct snd_soc_card *card,
 	return 0;
 }
 
+void snd_soc_card_disconnect_sync(struct device *dev)
+{
+	struct snd_soc_component *component = snd_soc_lookup_component(dev, NULL);
+
+	if (!component || !component->card)
+		return;
+
+	snd_card_disconnect_sync(component->card->snd_card);
+}
+
 /**
  * snd_soc_add_dai_link - Add a DAI link dynamically
  * @card: The ASoC card to which the DAI link is added
---------------

Best regards
---
Kuninori Morimoto