[alsa-devel] [PATCH] ASoC: Intel: Add defaults for SND_SST_ options and machine drivers
Add 'default m' when sensible, move common dependencies in if/endif block and clarify which options default to n - distro configurations should not enable legacy Baytrail stuff and NOCODEC (test only)
Basic tests run with defconfig, oldconfig, olddefconfig, there should be no functionality change.
Fixes: f6a118a800e3 ("ASoC: Intel: clarify Kconfig dependencies") Reported-by: Linus Torvalds torvalds@linux-foundation.org Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com --- sound/soc/intel/Kconfig | 5 ++++ sound/soc/intel/boards/Kconfig | 60 ++++++++++++++++++++++++++---------------- 2 files changed, 43 insertions(+), 22 deletions(-)
diff --git a/sound/soc/intel/Kconfig b/sound/soc/intel/Kconfig index 7b49d04e3c60..b40719396e9d 100644 --- a/sound/soc/intel/Kconfig +++ b/sound/soc/intel/Kconfig @@ -31,6 +31,7 @@ config SND_SOC_ACPI_INTEL_MATCH
config SND_SOC_INTEL_SST_TOPLEVEL tristate "Intel ASoC SST drivers" + default m depends on X86 || COMPILE_TEST select SND_SOC_INTEL_MACH select SND_SOC_INTEL_COMMON @@ -42,6 +43,7 @@ config SND_SOC_INTEL_SST_TOPLEVEL
config SND_SOC_INTEL_HASWELL tristate "Intel ASoC SST driver for Haswell/Broadwell" + default m depends on SND_SOC_INTEL_SST_TOPLEVEL && SND_DMA_SGBUF depends on DMADEVICES select SND_SOC_INTEL_SST @@ -49,6 +51,7 @@ config SND_SOC_INTEL_HASWELL
config SND_SOC_INTEL_BAYTRAIL tristate "Intel ASoC SST driver for Baytrail (legacy)" + default n depends on SND_SOC_INTEL_SST_TOPLEVEL depends on DMADEVICES select SND_SOC_INTEL_SST @@ -56,11 +59,13 @@ config SND_SOC_INTEL_BAYTRAIL
config SND_SST_ATOM_HIFI2_PLATFORM tristate "Intel ASoC SST driver for HiFi2 platforms (*field, *trail)" + default m depends on SND_SOC_INTEL_SST_TOPLEVEL && X86 select SND_SOC_COMPRESS
config SND_SOC_INTEL_SKYLAKE tristate "Intel ASoC SST driver for SKL/BXT/KBL/GLK/CNL" + default m depends on SND_SOC_INTEL_SST_TOPLEVEL && PCI && ACPI select SND_HDA_EXT_CORE select SND_HDA_DSP_LOADER diff --git a/sound/soc/intel/boards/Kconfig b/sound/soc/intel/boards/Kconfig index 6f754708a48c..6069328c5c28 100644 --- a/sound/soc/intel/boards/Kconfig +++ b/sound/soc/intel/boards/Kconfig @@ -17,10 +17,12 @@ config SND_MFLD_MACHINE Say Y if you have such a device. If unsure select "N".
+if SND_SOC_INTEL_HASWELL + config SND_SOC_INTEL_HASWELL_MACH tristate "ASoC Audio DSP support for Intel Haswell Lynxpoint" + default m depends on X86_INTEL_LPSS && I2C && I2C_DESIGNWARE_PLATFORM - depends on SND_SOC_INTEL_HASWELL select SND_SOC_RT5640 help This adds support for the Lynxpoint Audio DSP on Intel(R) Haswell @@ -30,8 +32,8 @@ config SND_SOC_INTEL_HASWELL_MACH
config SND_SOC_INTEL_BDW_RT5677_MACH tristate "ASoC Audio driver for Intel Broadwell with RT5677 codec" + default m depends on X86_INTEL_LPSS && GPIOLIB && I2C - depends on SND_SOC_INTEL_HASWELL select SND_SOC_RT5677 help This adds support for Intel Broadwell platform based boards with @@ -39,41 +41,50 @@ config SND_SOC_INTEL_BDW_RT5677_MACH
config SND_SOC_INTEL_BROADWELL_MACH tristate "ASoC Audio DSP support for Intel Broadwell Wildcatpoint" + default m depends on X86_INTEL_LPSS && I2C && I2C_DESIGNWARE_PLATFORM - depends on SND_SOC_INTEL_HASWELL select SND_SOC_RT286 help This adds support for the Wilcatpoint Audio DSP on Intel(R) Broadwell Ultrabook platforms. Say Y if you have such a device. If unsure select "N". +endif + +if SND_SOC_INTEL_BAYTRAIL
config SND_SOC_INTEL_BYT_MAX98090_MACH tristate "ASoC Audio driver for Intel Baytrail with MAX98090 codec" + default n depends on X86_INTEL_LPSS && I2C depends on SND_SST_IPC_ACPI = n - depends on SND_SOC_INTEL_BAYTRAIL select SND_SOC_MAX98090 help This adds audio driver for Intel Baytrail platform based boards - with the MAX98090 audio codec. + with the MAX98090 audio codec. This driver is deprecated, use + SND_SOC_INTEL_CHT_BSW_MAX98090_TI_MACH instead for better + functionality.
config SND_SOC_INTEL_BYT_RT5640_MACH tristate "ASoC Audio driver for Intel Baytrail with RT5640 codec" + default n depends on X86_INTEL_LPSS && I2C depends on SND_SST_IPC_ACPI = n - depends on SND_SOC_INTEL_BAYTRAIL select SND_SOC_RT5640 help This adds audio driver for Intel Baytrail platform based boards with the RT5640 audio codec. This driver is deprecated, use SND_SOC_INTEL_BYTCR_RT5640_MACH instead for better functionality.
+endif + +if SND_SST_ATOM_HIFI2_PLATFORM + config SND_SOC_INTEL_BYTCR_RT5640_MACH tristate "ASoC Audio driver for Intel Baytrail and Baytrail-CR with RT5640 codec" + default m depends on X86 && I2C && ACPI select SND_SOC_RT5640 - depends on SND_SST_ATOM_HIFI2_PLATFORM select SND_SST_IPC_ACPI help This adds support for ASoC machine driver for Intel(R) Baytrail and Baytrail-CR @@ -83,9 +94,9 @@ config SND_SOC_INTEL_BYTCR_RT5640_MACH
config SND_SOC_INTEL_BYTCR_RT5651_MACH tristate "ASoC Audio driver for Intel Baytrail and Baytrail-CR with RT5651 codec" + default m depends on X86 && I2C && ACPI select SND_SOC_RT5651 - depends on SND_SST_ATOM_HIFI2_PLATFORM select SND_SST_IPC_ACPI help This adds support for ASoC machine driver for Intel(R) Baytrail and Baytrail-CR @@ -95,9 +106,9 @@ config SND_SOC_INTEL_BYTCR_RT5651_MACH
config SND_SOC_INTEL_CHT_BSW_RT5672_MACH tristate "ASoC Audio driver for Intel Cherrytrail & Braswell with RT5672 codec" + default m depends on X86_INTEL_LPSS && I2C && ACPI select SND_SOC_RT5670 - depends on SND_SST_ATOM_HIFI2_PLATFORM select SND_SST_IPC_ACPI help This adds support for ASoC machine driver for Intel(R) Cherrytrail & Braswell @@ -107,9 +118,9 @@ config SND_SOC_INTEL_CHT_BSW_RT5672_MACH
config SND_SOC_INTEL_CHT_BSW_RT5645_MACH tristate "ASoC Audio driver for Intel Cherrytrail & Braswell with RT5645/5650 codec" + default m depends on X86_INTEL_LPSS && I2C && ACPI select SND_SOC_RT5645 - depends on SND_SST_ATOM_HIFI2_PLATFORM select SND_SST_IPC_ACPI help This adds support for ASoC machine driver for Intel(R) Cherrytrail & Braswell @@ -118,10 +129,10 @@ config SND_SOC_INTEL_CHT_BSW_RT5645_MACH
config SND_SOC_INTEL_CHT_BSW_MAX98090_TI_MACH tristate "ASoC Audio driver for Intel Cherrytrail & Braswell with MAX98090 & TI codec" + default m depends on X86_INTEL_LPSS && I2C && ACPI select SND_SOC_MAX98090 select SND_SOC_TS3A227E - depends on SND_SST_ATOM_HIFI2_PLATFORM select SND_SST_IPC_ACPI help This adds support for ASoC machine driver for Intel(R) Cherrytrail & Braswell @@ -130,9 +141,9 @@ config SND_SOC_INTEL_CHT_BSW_MAX98090_TI_MACH
config SND_SOC_INTEL_BYT_CHT_DA7213_MACH tristate "ASoC Audio driver for Intel Baytrail & Cherrytrail with DA7212/7213 codec" + default m depends on X86_INTEL_LPSS && I2C && ACPI select SND_SOC_DA7213 - depends on SND_SST_ATOM_HIFI2_PLATFORM select SND_SST_IPC_ACPI help This adds support for ASoC machine driver for Intel(R) Baytrail & CherryTrail @@ -141,9 +152,9 @@ config SND_SOC_INTEL_BYT_CHT_DA7213_MACH
config SND_SOC_INTEL_BYT_CHT_ES8316_MACH tristate "ASoC Audio driver for Intel Baytrail & Cherrytrail with ES8316 codec" + default m depends on X86_INTEL_LPSS && I2C && ACPI select SND_SOC_ES8316 - depends on SND_SST_ATOM_HIFI2_PLATFORM select SND_SST_IPC_ACPI help This adds support for ASoC machine driver for Intel(R) Baytrail & @@ -152,19 +163,23 @@ config SND_SOC_INTEL_BYT_CHT_ES8316_MACH
config SND_SOC_INTEL_BYT_CHT_NOCODEC_MACH tristate "ASoC Audio driver for Intel Baytrail & Cherrytrail platform with no codec (MinnowBoard MAX, Up)" + default n depends on X86_INTEL_LPSS && I2C && ACPI - depends on SND_SST_ATOM_HIFI2_PLATFORM select SND_SST_IPC_ACPI help This adds support for ASoC machine driver for the MinnowBoard Max or Up boards and provides access to I2S signals on the Low-Speed - connector + connector. It is not intended to be enabled by distros by default. If unsure select "N".
+endif + +if SND_SOC_INTEL_SKYLAKE + config SND_SOC_INTEL_SKL_RT286_MACH tristate "ASoC Audio driver for SKL with RT286 I2S mode" + default m depends on X86 && ACPI && I2C - depends on SND_SOC_INTEL_SKYLAKE select SND_SOC_RT286 select SND_SOC_DMIC select SND_SOC_HDAC_HDMI @@ -176,8 +191,8 @@ config SND_SOC_INTEL_SKL_RT286_MACH
config SND_SOC_INTEL_SKL_NAU88L25_SSM4567_MACH tristate "ASoC Audio driver for SKL with NAU88L25 and SSM4567 in I2S Mode" + default m depends on X86_INTEL_LPSS && I2C - depends on SND_SOC_INTEL_SKYLAKE select SND_SOC_NAU8825 select SND_SOC_SSM4567 select SND_SOC_DMIC @@ -190,8 +205,8 @@ config SND_SOC_INTEL_SKL_NAU88L25_SSM4567_MACH
config SND_SOC_INTEL_SKL_NAU88L25_MAX98357A_MACH tristate "ASoC Audio driver for SKL with NAU88L25 and MAX98357A in I2S Mode" + default m depends on X86_INTEL_LPSS && I2C - depends on SND_SOC_INTEL_SKYLAKE select SND_SOC_NAU8825 select SND_SOC_MAX98357A select SND_SOC_DMIC @@ -204,8 +219,8 @@ config SND_SOC_INTEL_SKL_NAU88L25_MAX98357A_MACH
config SND_SOC_INTEL_BXT_DA7219_MAX98357A_MACH tristate "ASoC Audio driver for Broxton with DA7219 and MAX98357A in I2S Mode" + default m depends on X86 && ACPI && I2C - depends on SND_SOC_INTEL_SKYLAKE select SND_SOC_DA7219 select SND_SOC_MAX98357A select SND_SOC_DMIC @@ -219,8 +234,8 @@ config SND_SOC_INTEL_BXT_DA7219_MAX98357A_MACH
config SND_SOC_INTEL_BXT_RT298_MACH tristate "ASoC Audio driver for Broxton with RT298 I2S mode" + default m depends on X86 && ACPI && I2C - depends on SND_SOC_INTEL_SKYLAKE select SND_SOC_RT298 select SND_SOC_DMIC select SND_SOC_HDAC_HDMI @@ -233,9 +248,9 @@ config SND_SOC_INTEL_BXT_RT298_MACH
config SND_SOC_INTEL_KBL_RT5663_MAX98927_MACH tristate "ASoC Audio driver for KBL with RT5663 and MAX98927 in I2S Mode" + default m depends on X86_INTEL_LPSS && I2C select SND_SOC_INTEL_SST - depends on SND_SOC_INTEL_SKYLAKE select SND_SOC_RT5663 select SND_SOC_MAX98927 select SND_SOC_DMIC @@ -248,9 +263,9 @@ config SND_SOC_INTEL_KBL_RT5663_MAX98927_MACH
config SND_SOC_INTEL_KBL_RT5663_RT5514_MAX98927_MACH tristate "ASoC Audio driver for KBL with RT5663, RT5514 and MAX98927 in I2S Mode" + default m depends on X86_INTEL_LPSS && I2C && SPI select SND_SOC_INTEL_SST - depends on SND_SOC_INTEL_SKYLAKE select SND_SOC_RT5663 select SND_SOC_RT5514 select SND_SOC_RT5514_SPI @@ -261,5 +276,6 @@ config SND_SOC_INTEL_KBL_RT5663_RT5514_MAX98927_MACH create an alsa sound card for RT5663 + RT5514 + MAX98927. Say Y if you have such a device. If unsure select "N". +endif
endif
On Thu, Nov 16, 2017 at 2:16 PM, Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com wrote:
Add 'default m' when sensible
No.
This is not sensible, and is not at all what I suggested.
Actual new code should *never* be default 'm' or 'y'.
You may think it's supremely important because you maintain it, but so does EVERY other developer for their code, and no, we don'[t want to just default everything on.
So the only thing that should be on by default is stuff that either
(a) is a new option for something that used to not have an option at all and was always enabled
IOW, this is a "it used to always be there, we default it on now that we've made it optional".
(b) stuff that doesn't actually generate any code, but is there as a choice.
So this is stuff like "show me the config options for vendor XYZ".
(c) stuff that really is so common that it is the majority of users.
This is stuff like USB, or block devices, or "enable networking".
But some individual driver for some hardware that nobody has? No.
So things like this is pure garbage:
config SND_SST_ATOM_HIFI2_PLATFORM tristate "Intel ASoC SST driver for HiFi2 platforms (*field, *trail)" default m
because clearly "Intel ASoC SST driver for HiFi2 platforms" should not at all default to being on for everybody.
So NAK NAK NAK on stuff like this.
Linus
On 11/16/2017 04:24 PM, Linus Torvalds wrote:
On Thu, Nov 16, 2017 at 2:16 PM, Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com wrote:
Add 'default m' when sensible
No.
This is not sensible, and is not at all what I suggested.
Actual new code should *never* be default 'm' or 'y'.
You may think it's supremely important because you maintain it, but so does EVERY other developer for their code, and no, we don'[t want to just default everything on.
So the only thing that should be on by default is stuff that either
(a) is a new option for something that used to not have an option at all and was always enabled
IOW, this is a "it used to always be there, we default it on nowthat we've made it optional".
(b) stuff that doesn't actually generate any code, but is there as a choice.
So this is stuff like "show me the config options for vendor XYZ".(c) stuff that really is so common that it is the majority of users.
This is stuff like USB, or block devices, or "enable networking".But some individual driver for some hardware that nobody has? No.
So things like this is pure garbage:
config SND_SST_ATOM_HIFI2_PLATFORM tristate "Intel ASoC SST driver for HiFi2 platforms(*field, *trail)" default m
because clearly "Intel ASoC SST driver for HiFi2 platforms" should not at all default to being on for everybody.
It's only on for people who select X86 and ASoC (which isn't on by default). With make defconfig, none of this is selected and I *thought* making those suboptions default to m would help distros who don't necessarily know the status of each driver, plus what was selected is known to be conflict-free. I guess I will follow the network example and state 'this is recommended'.
This is also not *new* code, what I tried here is to reverse the selection since the existing Kconfigs selected a target and inferred what SOC options are needed, and it needs to be the other way around to share machine drivers between SST and SOF drivers. Not to mention that there is no reason for machine driver configurations to be dependent on SOC-level configs like LPSS, this should be handled elsewhere.
I will move the TOPLEVEL config to follow what you suggested with the network example, this config does not have any impact on the code. will send a v2 tomorrow. If you want to send me your config i'll be happy to check the changes are ok. Thanks.
So NAK NAK NAK on stuff like this.
Linus
Alsa-devel mailing list Alsa-devel@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
On Fri, 17 Nov 2017 00:01:06 +0100, Pierre-Louis Bossart wrote:
On 11/16/2017 04:24 PM, Linus Torvalds wrote:
On Thu, Nov 16, 2017 at 2:16 PM, Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com wrote:
Add 'default m' when sensible
No.
This is not sensible, and is not at all what I suggested.
Actual new code should *never* be default 'm' or 'y'.
You may think it's supremely important because you maintain it, but so does EVERY other developer for their code, and no, we don'[t want to just default everything on.
So the only thing that should be on by default is stuff that either
(a) is a new option for something that used to not have an option at all and was always enabled
IOW, this is a "it used to always be there, we default it on nowthat we've made it optional".
(b) stuff that doesn't actually generate any code, but is there as a choice.
So this is stuff like "show me the config options for vendor XYZ".(c) stuff that really is so common that it is the majority of users.
This is stuff like USB, or block devices, or "enable networking".But some individual driver for some hardware that nobody has? No.
So things like this is pure garbage:
config SND_SST_ATOM_HIFI2_PLATFORM tristate "Intel ASoC SST driver for HiFi2 platforms(*field, *trail)" default m
because clearly "Intel ASoC SST driver for HiFi2 platforms" should not at all default to being on for everybody.
It's only on for people who select X86 and ASoC (which isn't on by default). With make defconfig, none of this is selected and I *thought* making those suboptions default to m would help distros who don't necessarily know the status of each driver, plus what was selected is known to be conflict-free. I guess I will follow the network example and state 'this is recommended'.
This is also not *new* code, what I tried here is to reverse the selection since the existing Kconfigs selected a target and inferred what SOC options are needed, and it needs to be the other way around to share machine drivers between SST and SOF drivers. Not to mention that there is no reason for machine driver configurations to be dependent on SOC-level configs like LPSS, this should be handled elsewhere.
I will move the TOPLEVEL config to follow what you suggested with the network example, this config does not have any impact on the code. will send a v2 tomorrow. If you want to send me your config i'll be happy to check the changes are ok. Thanks.
Well, think of creating a config from scratch on the tree with your patch and user just selects the default choice. What will happen? Then ASoC Intel drivers appear there. That should be avoided.
Also another problem is that the helper modules are selected by CONFIG_SND_SOC_INTEL_TOPLEVEL. With default on, these modules are also enabled unconditionally. That is, even if the board driver's default is off, you'll still get these modules with the default choice.
Lasts but not least, default=m looks definitely strange. Distros set CONFIG_SND=m, so it'll be module in anyway. That is, you don't have to care about modularity in that level at all.
A usual pattern is something like:
config SOME_TOPLEVEL bool "some toplevel gateway config" default y help ....
config BACKEND_A tristate select MORE_STUFF
config HELPER_DRIVERS tristate select ANOTHER_STUFF ....
if SOME_TOPLEVEL
config DRIVER_A tristate "driver a" depends on PLATFORM_A select BACKEND_A select HELPER_DRIVERS help ... config DRIVER_B tristate "driver b" depends on SOMETHING_ELSE select BACKEND_B select HELPER_DRIVERS help ...
....
endif
Here we set the top-level config default=y, so that you'll go into the board driver selection as default. If you know that you don't need it and are annoyed to deselect all, you can filter out at this point. That's the *only* meaning of the toplevel config: just filtering. (In the above it's bool, but it can be a tristate, too; but the purpose is still only for filtering.)
The rest drivers have no default, thus turned off. So the default are without any drivers. And note that CONFIG_SOME_TOPLEVEL doesn't select any others. Thus it works nothing but a gatekeeper.
Takashi
participants (3)
-
Linus Torvalds -
Pierre-Louis Bossart -
Takashi Iwai