On 11/5/19 12:46 AM, Kuninori Morimoto wrote:
From: Kuninori Morimoto <kuninori.morimoto.gx@xxxxxxxxxxx>
It is easy to read code if it is cleanly using paired function/naming,
like start <-> stop, register <-> unregister, etc, etc.
But, current ALSA SoC code is very random, unbalance, not paired, etc.
It is easy to create bug at the such code, and it will be difficult to
debug.
ALSA SoC has soc_bind_dai_link(), but its paired soc_unbind_dai_link()
is not implemented.
More confusable is that soc_remove_pcm_runtimes() which should be
soc_unbind_dai_link() is implemented without synchronised
to soc_bind_dai_link().
This patch cleanup this unbalance.
Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@xxxxxxxxxxx>
Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@xxxxxxxxxxxxxxx>
Morimoto-san, this patch seems to introduce a regression in our SOF
module removal tests. Without a couple of load/unload modules cycles, we
hit a kernel oops while freeing the card.
see https://github.com/thesofproject/linux/issues/1466 for some logs.
This issue did not happen with our November 6 rebase on Mark's tree, and
showed up today. I couldn't really bisect the whole tree due to other
issues, so manually applied your patches on top of this 11/06 tree and
bisected from there.
I will need to confirm this finding (it's quite late for me) but looking
at the code I wonder if the move of pcm_runtime deletion is correct?
---
v2 -> v3
- no change
- add Reviewed-by
sound/soc/soc-core.c | 22 ++++++++++++----------
1 file changed, 12 insertions(+), 10 deletions(-)
diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index e8ff6f2..1e8dd19 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -470,14 +470,6 @@ static struct snd_soc_pcm_runtime *soc_new_pcm_runtime(
return NULL;
}
-static void soc_remove_pcm_runtimes(struct snd_soc_card *card)
-{
- struct snd_soc_pcm_runtime *rtd, *_rtd;
-
- for_each_card_rtds_safe(card, rtd, _rtd)
- soc_free_pcm_runtime(rtd);
-}
-
struct snd_soc_pcm_runtime *snd_soc_get_pcm_runtime(struct snd_soc_card *card,
const char *dai_link)
{
@@ -1037,6 +1029,16 @@ static int soc_dai_link_sanity_check(struct snd_soc_card *card,
return 0;
}
+static void soc_unbind_dai_link(struct snd_soc_card *card,
+ struct snd_soc_dai_link *dai_link)
+{
+ struct snd_soc_pcm_runtime *rtd;
+
+ rtd = snd_soc_get_pcm_runtime(card, dai_link->name);
+ if (rtd)
+ soc_free_pcm_runtime(rtd);
+}
+
static int soc_bind_dai_link(struct snd_soc_card *card,
struct snd_soc_dai_link *dai_link)
{
@@ -1466,6 +1468,8 @@ void snd_soc_remove_dai_link(struct snd_soc_card *card,
card->remove_dai_link(card, dai_link);
list_del(&dai_link->list);
+
+ soc_unbind_dai_link(card, dai_link);
}
EXPORT_SYMBOL_GPL(snd_soc_remove_dai_link);
@@ -1974,8 +1978,6 @@ static void soc_cleanup_card_resources(struct snd_soc_card *card)
for_each_card_links_safe(card, link, _link)
snd_soc_remove_dai_link(card, link);
- soc_remove_pcm_runtimes(card);
-
/* remove auxiliary devices */
soc_remove_aux_devices(card);
soc_unbind_aux_dev(card);
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
