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