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
