[alsa-devel] [PATCH v2 2/5] ALSA: hda: move parts of NHLT code to new module

[Pierre-Louis Bossart]

On 7/22/19 7:26 AM, Takashi Iwai wrote:
> On Mon, 22 Jul 2019 14:14:28 +0200,
> Pierre-Louis Bossart wrote:
>>
>>
>>
>> On 7/22/19 3:54 AM, Takashi Iwai wrote:
>>> On Sat, 20 Jul 2019 23:06:46 +0200,
>>> Cezary Rojewski wrote:
>>>>
>>>>> --- a/sound/hda/Kconfig
>>>>> +++ b/sound/hda/Kconfig
>>>>> @@ -29,3 +29,6 @@ config SND_HDA_PREALLOC_SIZE
>>>>>       	  Note that the pre-allocation size can be changed dynamically
>>>>>     	  via a proc file (/proc/asound/card*/pcm*/sub*/prealloc), too.
>>>>> +
>>>>> +config SND_INTEL_NHLT
>>>>> +	tristate
>>>>
>>>> If above is true, "depends on ACPI" would be expected.
>>>
>>> This won't fix things in practice as the Kconfig reverse selection
>>> ignores the dependencies of the selected item.  It'd be as a help for
>>> readers, though.
>>
>> There is a fallback if ACPI is not defined, so the code would always
>> compile. Configurations which select SND_INTEL_NHLT already depend on
>> ACPI.
> 
> IIUC, the question above came from the point:
> 
>   #if IS_ENABLED(CONFIG_ACPI) && IS_ENABLED(CONFIG_SND_INTEL_NHLT)
>   ....
>   #else
>   ....
>   #endif
> 
> and here Cezary suggested to drop IS_ENABLED(CONFIG_ACPI) *iff* the
> dependency can be assured in Kconfig side.  But for that assurance,
> putting "depends on ACPI" in config SND_INTEL_NHLT block won't
> suffice; that was my followup.
> 
> So, as of the current code, we can drop IS_ENABLED(CONFIG_ACPI) from
> the ifdef above, yes.  But the dependency is no rock solid at this
> point, so either some comments or keeping the extra ifdef like the
> above would be needed, IMO.

this extra ifdef is a bit overkill, I added it to make sure that the 
fallbacks are used in nonsensical configurations w/ randconfig. In 
practice, all Intel drivers already depend on ACPI and for the legacy we 
already have:

select SND_INTEL_NHLT if ACPI

Not sure if we need to do anything more.

> 
> 
> thanks,
> 
> Takashi
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel at alsa-project.org
> https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
>