| Message ID | 20220621115758.3154933-1-cezary.rojewski@intel.com |
|---|---|
| State | Accepted |
| Commit | c7eb967d70446971413061effca3226578cb4dab |
| Headers | show |
| Series | ASoC: core: Exit all links before removing their components | expand |
On 2022-06-21 4:53 PM, Cezary Rojewski wrote: > On 2022-06-21 3:31 PM, Pierre-Louis Bossart wrote: >> On 6/21/22 06:57, Cezary Rojewski wrote: > >>> diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c >>> index 4a3fca50a536..638e781df3b0 100644 >>> --- a/sound/soc/soc-core.c >>> +++ b/sound/soc/soc-core.c >>> @@ -935,9 +935,6 @@ void snd_soc_remove_pcm_runtime(struct >>> snd_soc_card *card, >>> { >>> lockdep_assert_held(&client_mutex); >>> - /* release machine specific resources */ >>> - snd_soc_link_exit(rtd); >>> - >>> /* >>> * Notify the machine driver for extra destruction >>> */ >> >> .... what is not shown here is the >> >> soc_free_pcm_runtime(rtd); >> >> which will call device_unregister(rtd->dev); >> >> .... >> >>> @@ -1888,6 +1885,9 @@ static void soc_cleanup_card_resources(struct >>> snd_soc_card *card) >>> snd_soc_dapm_shutdown(card); >>> + /* release machine specific resources */ >>> + for_each_card_rtds(card, rtd) >>> + snd_soc_link_exit(rtd); >> >> ... so there's still a risk that the link exit refers to memory that's >> been released already. >> >> We would need to make sure rtd->dev is not used in any of the existing >> callbacks - or other functions such as e.g. snd_soc_close_delayed_work() >> which makes use of rtd->dev >> > > The lack of soc_free_pcm_runtime() included here is done on purpose. > After revisiting probe and remove flows it seems that there are more > de-synchronization than just link->init() and link->exit(). However, I > believe providing incremental changes is better than providing one > single big patch which has much larger impact of the framework. Moving > just the snd_soc_link_exit() has very limited impact. > > In regard to the flow, we have to remember that 'someone' has to be the > first one, 'someone' has to be the last one too. If the probe flow ends > with link->init() (for the selected range of functions), then it feels > natural for remove flow to begin with link->exit(). > > I've scanned the repo for usages of link->exit() and it seems only Intel > platforms do so. Given their (functions) current content, status quo is > achieved. AVS and CATPT drivers reload-tests show no regression too. The > DAPM-pin warning I'd mentioned earlier in the bdw_rt286 discussion is > also gone. Hello, Are there any questions left unanswered here? This patch is a dependency for jack handling changes for avs/catpt machine boards. Regards, Czarek
diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c index 4a3fca50a536..638e781df3b0 100644 --- a/sound/soc/soc-core.c +++ b/sound/soc/soc-core.c @@ -935,9 +935,6 @@ void snd_soc_remove_pcm_runtime(struct snd_soc_card *card, { lockdep_assert_held(&client_mutex); - /* release machine specific resources */ - snd_soc_link_exit(rtd); - /* * Notify the machine driver for extra destruction */ @@ -1888,6 +1885,9 @@ static void soc_cleanup_card_resources(struct snd_soc_card *card) snd_soc_dapm_shutdown(card); + /* release machine specific resources */ + for_each_card_rtds(card, rtd) + snd_soc_link_exit(rtd); /* remove and free each DAI */ soc_remove_link_dais(card); soc_remove_link_components(card);