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.
Regards, Czarek