On Thu, 20 Jun 2019 08:17:33 +0200 Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com wrote:
Could you please give a bit more context on what error you see when this happens?
Hi,
I get Oops. This is what happens with all other patches in this series and only this one reverted:
root@APL:~# rmmod snd_soc_sst_bxt_rt298 root@APL:~# rmmod snd_soc_hdac_hdmi root@APL:~# rmmod snd_soc_skl
Thanks, Amadeusz. I think the order in which the drivers are removed is what's causing the oops in your case. With SOF, the order we remove is
- rmmod sof_pci_dev
- rmmod snd_soc_sst_bxt_rt298
- rmmod snd_soc_hdac_hdmi
Well, there is nothing enforcing the order in which modules can be unloaded (and I see no reason to force it), as you can see from following excerpt, you can either start unloading from snd_soc_sst_bxt_rt298 or snd_soc_skl, and yes if you start from snd_soc_skl, there is no problem.
there is a fundamental dependency that you are ignoring: the module snd_soc_sst_bxt_rt298 is a machine driver which will be probed when snd_soc_skl creates a platform_device. Sure you can remove modules in a different order, but that's a bit of an artificial/academic exercise isn't it?
I am good with this patch. I just wanted to understand why we werent seeing this error with SOF. Sure, there's nothing enforcing the order in which modules are unloaded but there must be a logical order for testing purposes.
Pierre, can you please comment on it. I vaguely remember discussing this with you last year.
Our tests remove the modules by taking care of dependencies and it's already unveiled dozens of issues. We could add a sequence similar to Amadeusz and unbind the modules which are loaded with the creation of a platform_device (machine driver, dmic), I am just not sure how of useful this would be.
You work under the assumption that users will remove modules in "correct" order. Because it is not enforced by modules dependencies you can expect users to do everything possible at some point in time. In this case unloading modules in not expected order will lead to kernel Oops, which is not what should happen.