On 2020-11-17 3:04 PM, Takashi Iwai wrote:
On Mon, 16 Nov 2020 18:47:22 +0100, Pierre-Louis Bossart wrote:
Explicit 'ifs' asking whether we're dealing with SOF or other solution is at best a code smell. I believe this is unnecessary complexity added to the code especially once you realize user needs to play with module parameters to switch between solutions. If we assume user is able to play with module parameters then why not simply make use of blacklist mechanism?
Been there, done that. We don't want to use either denylist of kernel parameters.
denylists create confusion for users, it's an endless stream of false errors and time lost in bug reports.
The use of the kernel parameter is ONLY for expert users who want to tinker with the system or debug issues, the average user should not have to play with either denylists or kernel parameters.
I guess Cezary mean the modprobe blacklist? This was used in the early stage of ASoC Skylake driver development, but in the end, it's more cumbersome because user needs to change multiple places. The single module parameter was easier to handle.
Thanks for joining the discussion, Takashi.
If the switch of solution for atom-based products is imminent, why add code which becomes redundant soon after?
Yes, indeed I meant the modprobe blacklisting as it solves the problem without addition of any code. Doubt alsa-driver entries are scattered in /etc/modprobe.d/ so switching between one solution to another via blacklist becomes as easy as changing 'options intel-dsp-config <param>==<value>' entry.
In regard to catpt, solution is even simpler: just remove sound/soc/sof/intel/bdw.c as that code is not valid & recommended anyway and linux kernel is not place for such. There shouldn't be really any options for not recommended stuff. Leave the selection explicit.
And last but not least: intel-dsp-config is (as already stated) a mean for solving the HDA-runtime-driver-selection problem. Mixing it with ACPI devices is another layer of confusion.
Why? Who said it was PCI only? We already take care of DMIC, SoundWire, Google Chromebooks, open platforms, why not ACPI? It's just one API to be used when more than one driver can register to the same device.
Well, currently intel-dsp-config sits in sound/hda, which isn't really intuitive. Though, Intel driver file paths are already fairly scattered, so it doesn't matter too much :)
I don't mind to move it to another directory, but which one...? x86 might match, but shuffling the place won't help for maintenance.
I personally find this move good, at least it makes things easier for distros. There are small details like the above, but technically seen, I see no big problem.
Well, if non-Intel guys see the localization of code counter-intuitive then how about those who play with it daily.. The new "sof-parent" checks won't make maintaining any easier and I believe there are easier solutions as written above.
Czarek