RE: [PATCH 00/14] ASoC: Intel/SOF: extend run-time driver selection to ACPI devices

From: "Rojewski, Cezary" <cezary.rojewski@intel.com>
To: Takashi Iwai <tiwai@suse.de>,
	Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Cc: Hans de Goede <hdegoede@redhat.com>,
	"alsa-devel@alsa-project.org" <alsa-devel@alsa-project.org>,
	Mark Brown <broonie@kernel.org>,
	"andriy.shevchenko@linux.intel.com"
	<andriy.shevchenko@linux.intel.com>
Subject: RE: [PATCH 00/14] ASoC: Intel/SOF: extend run-time driver selection to ACPI devices
Date: Tue, 17 Nov 2020 22:13:13 +0000	[thread overview]
Message-ID: <0286c6975f24432082f609d45adaa14c@intel.com> (raw)
In-Reply-To: <s5h1rgst6z4.wl-tiwai@suse.de>

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