From ee92cf91b24226a1e00917edd8c3418f8ef67aa1 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Thu, 8 Nov 2018 17:10:32 -0600 Subject: [PATCH 1/2] ASoC: SOF: pcm: topology_free should not be done on pcm_free pcm_free is called for each BE, which results in the topology being freed multiple times. The topology_free should only be done on pcm_remove to mirror the topology_load done on pcm_probe. Signed-off-by: Pierre-Louis Bossart --- sound/soc/sof/pcm.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/sound/soc/sof/pcm.c b/sound/soc/sof/pcm.c index c3c388cdc42f11..3383a14fd6e3a4 100644 --- a/sound/soc/sof/pcm.c +++ b/sound/soc/sof/pcm.c @@ -588,7 +588,6 @@ static void sof_pcm_free(struct snd_pcm *pcm) if (spcm->pcm.capture) snd_dma_free_pages(&spcm->stream[SNDRV_PCM_STREAM_CAPTURE].page_table); - snd_sof_free_topology(sdev); } /* fixup the BE DAI link to match any values from topology */ @@ -716,7 +715,12 @@ static int sof_pcm_probe(struct snd_soc_component *component) static void sof_pcm_remove(struct snd_soc_component *component) { + struct snd_sof_dev *sdev = + snd_soc_component_get_drvdata(component); + pm_runtime_disable(component->dev); + snd_sof_free_topology(sdev); + } void snd_sof_new_platform_drv(struct snd_sof_dev *sdev) From 9233144b92f4f067b0ec393d3f721cfb1a412038 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Thu, 8 Nov 2018 17:13:44 -0600 Subject: [PATCH 2/2] ASoC: SOF: topology: add state variable to check topology load state With the complicated error flow, we don't have a record of whether the topology was loaded or not. Having a dmesg trace would help indicate an unintended path in error handling. Add a boolean to only load the topology if it's not already loaded, and free the topology if it's loaded, and log errors as needed. Note that the topology_free should itself never result in any crashes. If it does it's a separate issue from identifying bad error handling. Signed-off-by: Pierre-Louis Bossart --- sound/soc/sof/sof-priv.h | 1 + sound/soc/sof/topology.c | 13 +++++++++++++ 2 files changed, 14 insertions(+) diff --git a/sound/soc/sof/sof-priv.h b/sound/soc/sof/sof-priv.h index 71cfe8723397dd..de9fb2bdb5b941 100644 --- a/sound/soc/sof/sof-priv.h +++ b/sound/soc/sof/sof-priv.h @@ -357,6 +357,7 @@ struct snd_sof_dev { struct list_head dai_list; struct list_head route_list; struct snd_soc_component *component; + int tplg_loaded; /* keep track of topology load success */ /* FW configuration */ struct sof_ipc_dma_buffer_data *info_buffer; diff --git a/sound/soc/sof/topology.c b/sound/soc/sof/topology.c index bdda1f4c567622..cbedd176080e7f 100644 --- a/sound/soc/sof/topology.c +++ b/sound/soc/sof/topology.c @@ -2603,6 +2603,11 @@ int snd_sof_load_topology(struct snd_sof_dev *sdev, const char *file) struct snd_soc_tplg_hdr *hdr; int ret; + if (sdev->tplg_loaded) { + dev_err(sdev->dev, "topology already loaded ?\n"); + return -EINVAL; + } + dev_dbg(sdev->dev, "loading topology:%s\n", file); ret = request_firmware(&fw, file, sdev->dev); @@ -2622,6 +2627,8 @@ int snd_sof_load_topology(struct snd_sof_dev *sdev, const char *file) ret = -EINVAL; } + sdev->tplg_loaded = true; + release_firmware(fw); return ret; } @@ -2635,6 +2642,10 @@ void snd_sof_free_topology(struct snd_sof_dev *sdev) int ret; dev_dbg(sdev->dev, "free topology...\n"); + if (!sdev->tplg_loaded) { + dev_dbg(sdev->dev, "No topology loaded, nothing to free ...\n"); + return; + } /* remove routes */ list_for_each_entry_safe(sroute, temp, &sdev->route_list, list) { @@ -2654,5 +2665,7 @@ void snd_sof_free_topology(struct snd_sof_dev *sdev) if (ret < 0) dev_err(sdev->dev, "error: tplg component free failed %d\n", ret); + + sdev->tplg_loaded = false; } EXPORT_SYMBOL(snd_sof_free_topology);