On Wed, Apr 29, 2020 at 1:00 AM Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com wrote:
Thanks Arnd, do you mind sharing your config?
will give it a try, thanks!
We noticed last week that there's a depend/select confusion might be simpler to fix, see https://github.com/thesofproject/linux/pull/2047/commits
If I look at the first line I see a IMX_DSP=n which looks exactly like what we wanted to fix.
Yes, I think that fix addresses the build warning as well, but looking more closely I don't think it's what you want: If you do this on a config that has the IMX_DSP disabled, it would appear to the user that you have enabled the drivers, but the actual code is still disabled.
Are you sure? we added a select IMX_DSP, so not sure how it can be disabled?
I just tested Arnd's config with the patch we came up with for SOF (attached) and it makes the unmet dependency go away and builds fine. the problem is really using select IMX_DSP if it can be disabled by something else. My proposal looks simpler but I will agree it's not necessarily super elegant to move the dependency on IMX_BOX into SOF, so no sustained objection from me on Arnd's proposal.
Ok, thanks for testing!
I looked at the bigger picture again and found that the more fundamental problem is the dependency reversal in sound/soc/sof/sof-of-dev.c, where you have common code that knows about and links against a hardware specific driver. This is something we try hard do avoid in general in the kernel, as it causes all kinds of problems:
- Expressing the Kconfig dependencies is rather unnatural and error-prone, as you found
- Adding multiple new drivers at the same time leads to merge conflicts
- A kernel that supports multiple SoC families, like all general-purpose distros do, and Android is going to do in the future means that you have to load every hardware specific module in order to just use one of them.
- In Android's case, it also breaks the model of having one vendor provide support for a new SoC by enabling additional modules they ship in their vendor specific partition
I think this is all solved by moving the "module_platform_driver()" and of_device_id array for each driver into the module that defines the corresponding sof_dev_desc structure, and have those drivers call the exported sof_of_probe() and sof_of_remove() functions.
Arnd