Re: [PATCH 1/9] ALSA: hda: Do not unset preset when cleaning up codec

[Takashi Iwai <tiwai@suse.de>]

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