On Tue, 12 Jul 2022 12:58:09 +0200, Cezary Rojewski wrote:
On 2022-07-12 12:46 PM, Takashi Iwai wrote:
On Tue, 12 Jul 2022 11:42:56 +0200, Cezary Rojewski wrote:
...
snd_hda_codec_cleanup_for_unbind() is called in more places than just HDAudio codec driver's probe() and remove(). It's also called whenever HDAudio codec soc-component is being removed. Relevant part of the stack showing when does the cleanup function get called during rmmod:
[ 220.549349] snd_hda_codec_cleanup_for_unbind+0x25/0x451 [snd_hda_codec] [ 220.549536] ? dump_stack_lvl+0x45/0x49 [ 220.549568] hda_codec_remove.cold+0x14/0x138 [snd_soc_hda_codec] [ 220.549609] snd_soc_component_remove+0x34/0x40 [snd_soc_core] [ 220.549942] soc_remove_component+0x113/0x120 [snd_soc_core] [ 220.550249] soc_cleanup_card_resources+0x1a7/0x4a0 [snd_soc_core] [ 220.550561] snd_soc_unbind_card+0x9e/0x190 [snd_soc_core] [ 220.550885] snd_soc_unregister_card+0x28/0x80 [snd_soc_core] [ 220.551193] devm_card_release+0x1d/0x20 [snd_soc_core] [ 220.551527] release_nodes+0x73/0x170 [ 220.551549] ? preempt_count_sub+0x18/0xc0 [ 220.551576] devres_release_all+0x10a/0x150 [ 220.551600] ? devres_remove_group+0x260/0x260 [ 220.551630] device_unbind_cleanup+0x14/0xd0 [ 220.551656] device_release_driver_internal+0x146/0x1d0 [ 220.551688] driver_detach+0x81/0xf0 [ 220.551716] bus_remove_driver+0xae/0x170 [ 220.551743] driver_unregister+0x4d/0x70 [ 220.551770] platform_driver_unregister+0x12/0x20 [ 220.551799] avs_hdaudio_driver_exit+0x10/0x12 [snd_soc_avs_hdaudio]
So, IMO, you're scratching a wrong surface. The problem is rather that snd_hda_codec_cleanup_for_unbind() is called even if it's not for unbinding the codec.
This is how ASoC HDAudio codec component was behaving for years, see sound/soc/codecs/hdac_hda.c. If the intention is _not_ do call snd_hda_codec_cleanup_for_unbind() then the teardown procedure will probably need a little update.
Do we see a similar crash with the hdac-hda stuff, too?
And, after avs_hdaudio_driver_exit() is called, why the codec object still remains bound with the HD-audio (Realtek or whatever) codec driver?
Actually.. I'm afraid the init one would need an update to. Given how the implementation of HDAudio codec component's probe() and remove() looks like, there is no dropping the cleanup function without changing the upward path accordingly.
Well, to be honest the init/free procedures of HDAudio codec are a little hairy, perhaps it's time to address this.
Admittedly, the plumbing work for ASoC HD-audio was somewhat messy, and it's fine if we can clean things up.
snd_hda_codec_cleanup_for_unbind() is certainly written for unbinding the codec driver, and if a part of that function code is needed for different purposes, it should be factored out properly, at least.
thanks,
Takashi