| Message ID | 20200305130616.28658-2-pierre-louis.bossart@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ASoC: add dailink .exit() callback | expand |
On Thu, Mar 05, 2020 at 07:06:14AM -0600, Pierre-Louis Bossart wrote: > The exit() is invoked in soc_free_pcm_runtime(), which is not > completely symmetric with the init() invoked in soc_init_pcm_runtime() > - not soc_add_pcm_runtime(), but that's the best solution so far. We *could* look at moving the init back. In any case this seems reasonable by itself (I'm less convinced by the users). However... > @@ -1069,7 +1075,7 @@ static int soc_init_pcm_runtime(struct snd_soc_card *card, > /* set default power off timeout */ > rtd->pmdown_time = pmdown_time; > > - /* do machine specific initialization */ > + /* do machine specific allocations and initialization */ > if (dai_link->init) { > ret = dai_link->init(rtd); > if (ret < 0) { ...I'm not sure why we're saying to do allocations here? That really, really shouldn't be a normal thing - allocations should generally be done at the device model probe.
diff --git a/include/sound/soc.h b/include/sound/soc.h index 81e5d17be935..2beebe89ebbc 100644 --- a/include/sound/soc.h +++ b/include/sound/soc.h @@ -794,6 +794,9 @@ struct snd_soc_dai_link { /* codec/machine specific init - e.g. add machine controls */ int (*init)(struct snd_soc_pcm_runtime *rtd); + /* codec/machine specific exit - dual of init() */ + void (*exit)(struct snd_soc_pcm_runtime *rtd); + /* optional hw_params re-writing for BE and FE sync */ int (*be_hw_params_fixup)(struct snd_soc_pcm_runtime *rtd, struct snd_pcm_hw_params *params); diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c index f2cfbf182f49..09a0976d6a62 100644 --- a/sound/soc/soc-core.c +++ b/sound/soc/soc-core.c @@ -937,8 +937,14 @@ static int soc_dai_link_sanity_check(struct snd_soc_card *card, void snd_soc_remove_pcm_runtime(struct snd_soc_card *card, struct snd_soc_pcm_runtime *rtd) { + struct snd_soc_dai_link *dai_link = rtd->dai_link; + lockdep_assert_held(&client_mutex); + /* release machine specific resources */ + if (dai_link->exit) + dai_link->exit(rtd); + /* * Notify the machine driver for extra destruction */ @@ -1069,7 +1075,7 @@ static int soc_init_pcm_runtime(struct snd_soc_card *card, /* set default power off timeout */ rtd->pmdown_time = pmdown_time; - /* do machine specific initialization */ + /* do machine specific allocations and initialization */ if (dai_link->init) { ret = dai_link->init(rtd); if (ret < 0) {
Some machine drivers allocate or request resources during the init() phase, which need to be released at some point, e.g. when rebooting or unloading modules. In an initial pass, we added a .remove() callback for the platform driver, but that's not symmetrical at all and would be difficult to handle if there are more than one dailink implementing an .init(). We looked also into using .remove_dai_link() callback, but that would also be imlanced. The suggested solution is to use a dual exit() phase for dailinks to release all resources. The exit() is invoked in soc_free_pcm_runtime(), which is not completely symmetric with the init() invoked in soc_init_pcm_runtime() - not soc_add_pcm_runtime(), but that's the best solution so far. Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com> --- include/sound/soc.h | 3 +++ sound/soc/soc-core.c | 8 +++++++- 2 files changed, 10 insertions(+), 1 deletion(-)