On 7/19/19 1:55 PM, Takashi Iwai wrote:
On Fri, 19 Jul 2019 20:29:10 +0200, Pierre-Louis Bossart wrote:
@@ -2055,6 +2078,17 @@ static int azx_probe(struct pci_dev *pci, card->private_data = chip; hda = container_of(chip, struct hda_intel, chip);
- /*
* stop probe if digital microphones detected on Skylake+ platform* with the DSP enabled. This is an opt-in behavior defined at build* time or at run-time with a module parameter*/- if (IS_ENABLED(CONFIG_SND_HDA_INTEL_DETECT_DMIC) || dmic_detect >= 0) {
Isn't it "dmic_detect != 0" ? Otherwise passing dmic_detect=0 would be treated as positive here.
Ah, good catch. I literally copied the enable_msi example here, which relies on >= 0.
if (enable_msi >= 0) { chip->msi = !!enable_msi; return; }
Not sure what the intention was here.
The MSI-enablement depends on the platform. enable_msi >=0 means that user passed module option explicitly so we follow that. Otherwise, let the system chooses whether to enable MSI or not.
Using dmic_detect != 0 wouldn't work for the default -1 value, maybe dmic_detect > 0 is probably a better solution?
In your case, the logic doesn't look like the dynamically platform dependent, so it should be simpler as below:
static bool dmic_detect = IS_ENABLED(CONFIG_SND_HDA_INTEL_DETECT_DMIC); module_param(dmic_detect, bool, 0444);
and evaluate it
if (dmic_detect) { ret = azx_check_dmic(); .... That is, if Kconfig is set, it's per default enabled, but user can still turn it off via option. Otherwise user needs to enable it via option.
Makes sense, thanks for the feedback. Will send a v2 shortly. Have a nice week-end.