* [RFC][PATCH 0/2] localmodconfig / ALSA: hda - Have codecs be disabled by localmodconfig
@ 2013-12-18 17:35 Steven Rostedt
2013-12-18 17:35 ` [RFC][PATCH 1/2] localmodconfig: Add config depends by default settings Steven Rostedt
` (2 more replies)
0 siblings, 3 replies; 5+ messages in thread
From: Steven Rostedt @ 2013-12-18 17:35 UTC (permalink / raw)
To: linux-kernel; +Cc: Linus Torvalds, Takashi Iwai, Michal Marek, Andrew Morton
Linus Torvalds reported that 'make localmodconfig' would never work on
his ALSA codec modules. That is, although his box only had realtek and
hdmi codec modules, all of the codec modules would still be built after
running localmodconfig.
When he showed me his config file which had this:
CONFIG_SND_HDA_CODEC_REALTEK=y
CONFIG_SND_HDA_CODEC_ANALOG=y
CONFIG_SND_HDA_CODEC_SIGMATEL=y
CONFIG_SND_HDA_CODEC_VIA=y
CONFIG_SND_HDA_CODEC_HDMI=y
CONFIG_SND_HDA_CODEC_CIRRUS=y
CONFIG_SND_HDA_CODEC_CONEXANT=y
CONFIG_SND_HDA_CODEC_CA0110=y
CONFIG_SND_HDA_CODEC_CA0132=y
CONFIG_SND_HDA_CODEC_CA0132_DSP=y
CONFIG_SND_HDA_CODEC_CMEDIA=y
CONFIG_SND_HDA_CODEC_SI3054=y
..
I was confused to why they were '=y' if they were to build modules
and not '=m'. Localmodconfig will not disable anything with '=y' because
there's not enough information in lsmod to safely disable a built-in
config.
Investigating as to why these were '=y' I found this in the Makefile:
ifdef CONFIG_SND_HDA_CODEC_REALTEK
obj-$(CONFIG_SND_HDA_INTEL) += snd-hda-codec-realtek.o
endif
ifdef CONFIG_SND_HDA_CODEC_CMEDIA
obj-$(CONFIG_SND_HDA_INTEL) += snd-hda-codec-cmedia.o
endif
ifdef CONFIG_SND_HDA_CODEC_ANALOG
obj-$(CONFIG_SND_HDA_INTEL) += snd-hda-codec-analog.o
endif
ifdef CONFIG_SND_HDA_CODEC_SIGMATEL
obj-$(CONFIG_SND_HDA_INTEL) += snd-hda-codec-idt.o
endif
ifdef CONFIG_SND_HDA_CODEC_SI3054
obj-$(CONFIG_SND_HDA_INTEL) += snd-hda-codec-si3054.o
endif
[...]
And the reason for this is because if the code for SND_HDA_INTEL
is a built-in, then all the codecs that are configured must also be
built-in. If it is a module, then all the codecs must also be a module.
To put this another way; if SND_HDA_INTEL is 'y' then all the codecs
must either be 'y' or 'n'. If SND_HDA_INTEL is 'm', then all the
codecs must be 'm' or 'n'. The latter is already supported by the
kconfig code, but the former is not. That is, if a tristate config
is dependent on a config that is built-in 'y', then that config can
still become a module 'm'. This trick that is done above prevents that.
But unfortunately, it also confuses localmodconfig, and the user
does not get the correct result.
Ideally, having a "match_config" option in the kconfig subsystem would
be ideal. But anyone that ever took a look at the kconfig code knows that
is much more difficult to implement than one would expect, as that
code can cause severe eye strain.
But luckily for us, we can do another trick to keep the behavior needed
by the ALSA hda code, and still allow for localmodconfig to disable the
modules.
Now to fix this so that the codecs match the tristate of SND_HDA_INTEL
we can add a config that states that SND_HDA_INTEL is 'y', and then add
another config for each codec for it to build the module. We then switch
all the codecs to be tristates too.
The new config for HDA_INTEL set as 'y' is:
config SND_HDA_YES
default y if SND_HDA_INTEL=y
The new codec configs will have the following format:
config SND_HDA_CODEC_FOO_BUILD
def_tristate (SND_HDA_CODEC_FOO=m && SND_HDA_YES) || SND_HDA_CODEC_FOO
What the above does is to set SND_HDA_CODEC_FOO_BUILD to 'y' if
SND_HDA_CODEC_FOO is a module and SND_HDA_YES is set. This forces
the codec to be built-in. Otherwise, it just sets it to the state of
SND_HDA_CODEC_FOO.
This change lets localmodconfig turn off codecs that are not being used.
The first patch fixes a bug in localmodconfig that this ordeal uncovered,
and that was the fact that localmodconfig did not honor default dependencies
like "def_tristate (SND_HDA_CODEC_FOO=m && SND_HDA_YES) || SND_HDA_CODEC_FOO"
-- Steve
Steven Rostedt (Red Hat) (2):
localmodconfig: Add config depends by default settings
ALSA: hda - Change codec configs to be disabled by localmodconfig
----
scripts/kconfig/streamline_config.pl | 7 ++++
sound/pci/hda/Kconfig | 64 +++++++++++++++++++++++++++++-------
sound/pci/hda/Makefile | 44 +++++++------------------
3 files changed, 70 insertions(+), 45 deletions(-)
^ permalink raw reply [flat|nested] 5+ messages in thread
* [RFC][PATCH 1/2] localmodconfig: Add config depends by default settings
2013-12-18 17:35 [RFC][PATCH 0/2] localmodconfig / ALSA: hda - Have codecs be disabled by localmodconfig Steven Rostedt
@ 2013-12-18 17:35 ` Steven Rostedt
2013-12-18 17:35 ` [RFC][PATCH 2/2] ALSA: hda - Change codec configs to be disabled by localmodconfig Steven Rostedt
2013-12-18 19:05 ` [RFC][PATCH 0/2] localmodconfig / ALSA: hda - Have codecs " Takashi Iwai
2 siblings, 0 replies; 5+ messages in thread
From: Steven Rostedt @ 2013-12-18 17:35 UTC (permalink / raw)
To: linux-kernel; +Cc: Linus Torvalds, Takashi Iwai, Michal Marek, Andrew Morton
[-- Attachment #1: 0001-localmodconfig-Add-config-depends-by-default-setting.patch --]
[-- Type: text/plain, Size: 1254 bytes --]
From: "Steven Rostedt (Red Hat)" <rostedt@goodmis.org>
Currently localmodconfig will miss dependencies from the default option.
For example:
config FOO
default y if BAR || ZOO
If FOO is needed for a module and is set to '=m', and so are BAR or ZOO,
localmodconfig will not see that BOO or ZOO are also needed for the foo
module, and will incorrectly disable them.
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
scripts/kconfig/streamline_config.pl | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/scripts/kconfig/streamline_config.pl b/scripts/kconfig/streamline_config.pl
index 4606cdf..3133172 100644
--- a/scripts/kconfig/streamline_config.pl
+++ b/scripts/kconfig/streamline_config.pl
@@ -219,6 +219,13 @@ sub read_kconfig {
$depends{$config} = $1;
} elsif ($state eq "DEP" && /^\s*depends\s+on\s+(.*)$/) {
$depends{$config} .= " " . $1;
+ } elsif ($state eq "DEP" && /^\s*def(_(bool|tristate)|ault)\s+(\S.*)$/) {
+ my $dep = $3;
+ if ($dep !~ /^\s*(y|m|n)\s*$/) {
+ $dep =~ s/.*\sif\s+//;
+ $depends{$config} .= " " . $dep;
+ dprint "Added default depends $dep to $config\n";
+ }
# Get the configs that select this config
} elsif ($state ne "NONE" && /^\s*select\s+(\S+)/) {
--
1.8.4.3
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [RFC][PATCH 2/2] ALSA: hda - Change codec configs to be disabled by localmodconfig
2013-12-18 17:35 [RFC][PATCH 0/2] localmodconfig / ALSA: hda - Have codecs be disabled by localmodconfig Steven Rostedt
2013-12-18 17:35 ` [RFC][PATCH 1/2] localmodconfig: Add config depends by default settings Steven Rostedt
@ 2013-12-18 17:35 ` Steven Rostedt
2013-12-18 19:05 ` [RFC][PATCH 0/2] localmodconfig / ALSA: hda - Have codecs " Takashi Iwai
2 siblings, 0 replies; 5+ messages in thread
From: Steven Rostedt @ 2013-12-18 17:35 UTC (permalink / raw)
To: linux-kernel; +Cc: Linus Torvalds, Takashi Iwai, Michal Marek, Andrew Morton
[-- Attachment #1: 0002-ALSA-hda-Change-codec-configs-to-be-disabled-by-loca.patch --]
[-- Type: text/plain, Size: 11476 bytes --]
From: "Steven Rostedt (Red Hat)" <rostedt@goodmis.org>
If a user (or high level kernel developer) wants to use localmodconfig
to help them trim their config down to compile only the modules that
are needed by their computer (as reported by lsmod), they will be very
disappointed in the results of the ALSA hda codec modules.
The reason is that the way the kconfig is set up for codecs hides
the configs as being for modules by havinhg them be bools instead of
tristates. The Kconfig looks like this:
config SND_HDA_CODEC_REALTEK
bool "Build Realtek HD-audio codec support"
default y
[..]
config SND_HDA_CODEC_ANALOG
bool "Build Analog Device HD-audio codec support"
default y
[..]
config SND_HDA_CODEC_SIGMATEL
bool "Build IDT/Sigmatel HD-audio codec support"
[..]
And the Makefile looks like this:
# codec drivers (note: CONFIG_SND_HDA_CODEC_XXX are booleans)
ifdef CONFIG_SND_HDA_CODEC_REALTEK
obj-$(CONFIG_SND_HDA_INTEL) += snd-hda-codec-realtek.o
endif
ifdef CONFIG_SND_HDA_CODEC_CMEDIA
obj-$(CONFIG_SND_HDA_INTEL) += snd-hda-codec-cmedia.o
endif
ifdef CONFIG_SND_HDA_CODEC_ANALOG
obj-$(CONFIG_SND_HDA_INTEL) += snd-hda-codec-analog.o
endif
ifdef CONFIG_SND_HDA_CODEC_SIGMATEL
obj-$(CONFIG_SND_HDA_INTEL) += snd-hda-codec-idt.o
endif
ifdef CONFIG_SND_HDA_CODEC_SI3054
obj-$(CONFIG_SND_HDA_INTEL) += snd-hda-codec-si3054.o
endif
[...]
To enable snd-hda-codec-realtek.o as a module, you need to make sure
SND_HDA_INTEL is set to a module and then select SND_HDA_CODEC_REALTEK
as '=y'! That is not a module, but a boolean.
Localmodconfig will not turn off any '=y' because there's not enough
information in the lsmod output to know if it is safe to disable a '=y'
config or not. Localmodconfig can only safely disable '=m' options.
The reason the HDA codec code is like this is because all the codecs must
match the tristate SND_HDA_INTEL. That is, if SND_HDA_INTEL is a module
'=m', then all the codecs can only be modules or not defined '=m' or '=n'.
That case is very common, but HDA codecs can not be a module if SND_HDA_INTEL
is built-in '=y'. If SND_HDA_INTEL is '=y' then the codecs can only be
'=y' or '=n'. They can not be '=m', as the SND_HDA_INTEL code expects all
the codecs to be statically linked in at bootup when built-in.
Now to fix this so that the codecs match the tristate of SND_HDA_INTEL
we can add a config that states that SND_HDA_INTEL is 'y', and then add
another config for each codec for it to build the module. We then switch
all the codecs to be tristates too.
The new config for HDA_INTEL set as 'y' is:
config SND_HDA_YES
default y if SND_HDA_INTEL=y
The new codec configs will have the following format:
config SND_HDA_CODEC_FOO_BUILD
def_tristate (SND_HDA_CODEC_FOO=m && SND_HDA_YES) || SND_HDA_CODEC_FOO
What the above does is to set SND_HDA_CODEC_FOO_BUILD to 'y' if
SND_HDA_CODEC_FOO is a module and SND_HDA_YES is set. This forces
the codec to be built-in. Otherwise, it just sets it to the state of
SND_HDA_CODEC_FOO.
This change lets localmodconfig turn off codecs that are not being used.
Reported-by: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Takashi Iwai <tiwai@suse.de>
Cc: Michal Marek <mmarek@suse.cz>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
sound/pci/hda/Kconfig | 64 ++++++++++++++++++++++++++++++++++++++++----------
sound/pci/hda/Makefile | 44 +++++++++-------------------------
2 files changed, 63 insertions(+), 45 deletions(-)
diff --git a/sound/pci/hda/Kconfig b/sound/pci/hda/Kconfig
index 4cdd9de..97cd02b 100644
--- a/sound/pci/hda/Kconfig
+++ b/sound/pci/hda/Kconfig
@@ -1,3 +1,7 @@
+config SND_HDA_YES
+ bool
+ default y if SND_HDA_INTEL=y
+
menuconfig SND_HDA_INTEL
tristate "Intel HD Audio"
select SND_PCM
@@ -87,7 +91,7 @@ config SND_HDA_PATCH_LOADER
This option turns on hwdep and reconfig features automatically.
config SND_HDA_CODEC_REALTEK
- bool "Build Realtek HD-audio codec support"
+ tristate "Build Realtek HD-audio codec support"
default y
select SND_HDA_GENERIC
help
@@ -99,8 +103,11 @@ config SND_HDA_CODEC_REALTEK
snd-hda-codec-realtek.
This module is automatically loaded at probing.
+config SND_HDA_CODEC_REALTEK_BUILD
+ def_tristate (SND_HDA_CODEC_REALTEK=m && SND_HDA_YES) || SND_HDA_CODEC_REALTEK
+
config SND_HDA_CODEC_ANALOG
- bool "Build Analog Device HD-audio codec support"
+ tristate "Build Analog Device HD-audio codec support"
default y
select SND_HDA_GENERIC
help
@@ -112,8 +119,11 @@ config SND_HDA_CODEC_ANALOG
snd-hda-codec-analog.
This module is automatically loaded at probing.
+config SND_HDA_CODEC_ANALOG_BUILD
+ def_tristate (SND_HDA_CODEC_ANALOG=m && SND_HDA_YES) || SND_HDA_CODEC_ANALOG
+
config SND_HDA_CODEC_SIGMATEL
- bool "Build IDT/Sigmatel HD-audio codec support"
+ tristate "Build IDT/Sigmatel HD-audio codec support"
default y
select SND_HDA_GENERIC
help
@@ -125,8 +135,11 @@ config SND_HDA_CODEC_SIGMATEL
snd-hda-codec-idt.
This module is automatically loaded at probing.
+config SND_HDA_CODEC_SIGMATEL_BUILD
+ def_tristate (SND_HDA_CODEC_SIGMATEL=m && SND_HDA_YES) || SND_HDA_CODEC_SIGMATEL
+
config SND_HDA_CODEC_VIA
- bool "Build VIA HD-audio codec support"
+ tristate "Build VIA HD-audio codec support"
default y
select SND_HDA_GENERIC
help
@@ -138,8 +151,11 @@ config SND_HDA_CODEC_VIA
snd-hda-codec-via.
This module is automatically loaded at probing.
+config SND_HDA_CODEC_VIA_BUILD
+ def_tristate (SND_HDA_CODEC_VIA=m && SND_HDA_YES) || SND_HDA_CODEC_VIA
+
config SND_HDA_CODEC_HDMI
- bool "Build HDMI/DisplayPort HD-audio codec support"
+ tristate "Build HDMI/DisplayPort HD-audio codec support"
default y
help
Say Y here to include HDMI and DisplayPort HD-audio codec
@@ -151,13 +167,16 @@ config SND_HDA_CODEC_HDMI
snd-hda-codec-hdmi.
This module is automatically loaded at probing.
+config SND_HDA_CODEC_HDMI_BUILD
+ def_tristate (SND_HDA_CODEC_HDMI=m && SND_HDA_YES) || SND_HDA_CODEC_HDMI
+
config SND_HDA_I915
bool
default y
depends on DRM_I915
config SND_HDA_CODEC_CIRRUS
- bool "Build Cirrus Logic codec support"
+ tristate "Build Cirrus Logic codec support"
default y
select SND_HDA_GENERIC
help
@@ -169,8 +188,11 @@ config SND_HDA_CODEC_CIRRUS
snd-hda-codec-cirrus.
This module is automatically loaded at probing.
+config SND_HDA_CODEC_CIRRUS_BUILD
+ def_tristate (SND_HDA_CODEC_CIRRUS=m && SND_HDA_YES) || SND_HDA_CODEC_CIRRUS
+
config SND_HDA_CODEC_CONEXANT
- bool "Build Conexant HD-audio codec support"
+ tristate "Build Conexant HD-audio codec support"
default y
select SND_HDA_GENERIC
help
@@ -182,8 +204,11 @@ config SND_HDA_CODEC_CONEXANT
snd-hda-codec-conexant.
This module is automatically loaded at probing.
+config SND_HDA_CODEC_CONEXANT_BUILD
+ def_tristate (SND_HDA_CODEC_CONEXANT=m && SND_HDA_YES) || SND_HDA_CODEC_CONEXANT
+
config SND_HDA_CODEC_CA0110
- bool "Build Creative CA0110-IBG codec support"
+ tristate "Build Creative CA0110-IBG codec support"
default y
select SND_HDA_GENERIC
help
@@ -195,8 +220,11 @@ config SND_HDA_CODEC_CA0110
snd-hda-codec-ca0110.
This module is automatically loaded at probing.
+config SND_HDA_CODEC_CA0110_BUILD
+ def_tristate (SND_HDA_CODEC_CA0110=m && SND_HDA_YES) || SND_HDA_CODEC_CA0110
+
config SND_HDA_CODEC_CA0132
- bool "Build Creative CA0132 codec support"
+ tristate "Build Creative CA0132 codec support"
default y
help
Say Y here to include Creative CA0132 codec support in
@@ -207,8 +235,11 @@ config SND_HDA_CODEC_CA0132
snd-hda-codec-ca0132.
This module is automatically loaded at probing.
+config SND_HDA_CODEC_CA0132_BUILD
+ def_tristate (SND_HDA_CODEC_CA0132=m && SND_HDA_YES) || SND_HDA_CODEC_CA0132
+
config SND_HDA_CODEC_CA0132_DSP
- bool "Support new DSP code for CA0132 codec"
+ tristate "Support new DSP code for CA0132 codec"
depends on SND_HDA_CODEC_CA0132
select SND_HDA_DSP_LOADER
select FW_LOADER
@@ -219,8 +250,11 @@ config SND_HDA_CODEC_CA0132_DSP
Note that this option requires the external firmware file
(ctefx.bin).
+config SND_HDA_CODEC_CA0132_DSP_BUILD
+ def_tristate (SND_HDA_CODEC_CA0132_DSP=m && SND_HDA_YES) || SND_HDA_CODEC_CA0132_DSP
+
config SND_HDA_CODEC_CMEDIA
- bool "Build C-Media HD-audio codec support"
+ tristate "Build C-Media HD-audio codec support"
default y
select SND_HDA_GENERIC
help
@@ -232,8 +266,11 @@ config SND_HDA_CODEC_CMEDIA
snd-hda-codec-cmedia.
This module is automatically loaded at probing.
+config SND_HDA_CODEC_CMEDIA_BUILD
+ def_tristate (SND_HDA_CODEC_CMEDIA=m && SND_HDA_YES) || SND_HDA_CODEC_CMEDIA
+
config SND_HDA_CODEC_SI3054
- bool "Build Silicon Labs 3054 HD-modem codec support"
+ tristate "Build Silicon Labs 3054 HD-modem codec support"
default y
help
Say Y here to include Silicon Labs 3054 HD-modem codec
@@ -244,6 +281,9 @@ config SND_HDA_CODEC_SI3054
snd-hda-codec-si3054.
This module is automatically loaded at probing.
+config SND_HDA_CODEC_SI3054_BUILD
+ def_tristate (SND_HDA_CODEC_SI3054=m && SND_HDA_YES) || SND_HDA_CODEC_SI3054
+
config SND_HDA_GENERIC
bool "Enable generic HD-audio codec parser"
default y
diff --git a/sound/pci/hda/Makefile b/sound/pci/hda/Makefile
index c091438..41d00ac 100644
--- a/sound/pci/hda/Makefile
+++ b/sound/pci/hda/Makefile
@@ -28,39 +28,17 @@ snd-hda-codec-hdmi-objs := patch_hdmi.o hda_eld.o
obj-$(CONFIG_SND_HDA_INTEL) := snd-hda-codec.o
# codec drivers (note: CONFIG_SND_HDA_CODEC_XXX are booleans)
-ifdef CONFIG_SND_HDA_CODEC_REALTEK
-obj-$(CONFIG_SND_HDA_INTEL) += snd-hda-codec-realtek.o
-endif
-ifdef CONFIG_SND_HDA_CODEC_CMEDIA
-obj-$(CONFIG_SND_HDA_INTEL) += snd-hda-codec-cmedia.o
-endif
-ifdef CONFIG_SND_HDA_CODEC_ANALOG
-obj-$(CONFIG_SND_HDA_INTEL) += snd-hda-codec-analog.o
-endif
-ifdef CONFIG_SND_HDA_CODEC_SIGMATEL
-obj-$(CONFIG_SND_HDA_INTEL) += snd-hda-codec-idt.o
-endif
-ifdef CONFIG_SND_HDA_CODEC_SI3054
-obj-$(CONFIG_SND_HDA_INTEL) += snd-hda-codec-si3054.o
-endif
-ifdef CONFIG_SND_HDA_CODEC_CIRRUS
-obj-$(CONFIG_SND_HDA_INTEL) += snd-hda-codec-cirrus.o
-endif
-ifdef CONFIG_SND_HDA_CODEC_CA0110
-obj-$(CONFIG_SND_HDA_INTEL) += snd-hda-codec-ca0110.o
-endif
-ifdef CONFIG_SND_HDA_CODEC_CA0132
-obj-$(CONFIG_SND_HDA_INTEL) += snd-hda-codec-ca0132.o
-endif
-ifdef CONFIG_SND_HDA_CODEC_CONEXANT
-obj-$(CONFIG_SND_HDA_INTEL) += snd-hda-codec-conexant.o
-endif
-ifdef CONFIG_SND_HDA_CODEC_VIA
-obj-$(CONFIG_SND_HDA_INTEL) += snd-hda-codec-via.o
-endif
-ifdef CONFIG_SND_HDA_CODEC_HDMI
-obj-$(CONFIG_SND_HDA_INTEL) += snd-hda-codec-hdmi.o
-endif
+obj-$(CONFIG_SND_HDA_CODEC_REALTEK_BUILD) += snd-hda-codec-realtek.o
+obj-$(CONFIG_SND_HDA_CODEC_CMEDIA_BUILD) += snd-hda-codec-cmedia.o
+obj-$(CONFIG_SND_HDA_CODEC_ANALOG_BUILD) += snd-hda-codec-analog.o
+obj-$(CONFIG_SND_HDA_CODEC_SIGMATEL_BUILD) += snd-hda-codec-idt.o
+obj-$(CONFIG_SND_HDA_CODEC_SI3054_BUILD) += snd-hda-codec-si3054.o
+obj-$(CONFIG_SND_HDA_CODEC_CIRRUS_BUILD) += snd-hda-codec-cirrus.o
+obj-$(CONFIG_SND_HDA_CODEC_CA0110_BUILD) += snd-hda-codec-ca0110.o
+obj-$(CONFIG_SND_HDA_CODEC_CA0132_BUILD) += snd-hda-codec-ca0132.o
+obj-$(CONFIG_SND_HDA_CODEC_CONEXANT_BUILD) += snd-hda-codec-conexant.o
+obj-$(CONFIG_SND_HDA_CODEC_VIA_BUILD) += snd-hda-codec-via.o
+obj-$(CONFIG_SND_HDA_CODEC_HDMI_BUILD) += snd-hda-codec-hdmi.o
# this must be the last entry after codec drivers;
# otherwise the codec patches won't be hooked before the PCI probe
--
1.8.4.3
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [RFC][PATCH 0/2] localmodconfig / ALSA: hda - Have codecs be disabled by localmodconfig
2013-12-18 17:35 [RFC][PATCH 0/2] localmodconfig / ALSA: hda - Have codecs be disabled by localmodconfig Steven Rostedt
2013-12-18 17:35 ` [RFC][PATCH 1/2] localmodconfig: Add config depends by default settings Steven Rostedt
2013-12-18 17:35 ` [RFC][PATCH 2/2] ALSA: hda - Change codec configs to be disabled by localmodconfig Steven Rostedt
@ 2013-12-18 19:05 ` Takashi Iwai
2013-12-18 19:16 ` Steven Rostedt
2 siblings, 1 reply; 5+ messages in thread
From: Takashi Iwai @ 2013-12-18 19:05 UTC (permalink / raw)
To: Steven Rostedt; +Cc: linux-kernel, Linus Torvalds, Michal Marek, Andrew Morton
At Wed, 18 Dec 2013 12:35:19 -0500,
Steven Rostedt wrote:
>
> Linus Torvalds reported that 'make localmodconfig' would never work on
> his ALSA codec modules. That is, although his box only had realtek and
> hdmi codec modules, all of the codec modules would still be built after
> running localmodconfig.
>
> When he showed me his config file which had this:
>
> CONFIG_SND_HDA_CODEC_REALTEK=y
> CONFIG_SND_HDA_CODEC_ANALOG=y
> CONFIG_SND_HDA_CODEC_SIGMATEL=y
> CONFIG_SND_HDA_CODEC_VIA=y
> CONFIG_SND_HDA_CODEC_HDMI=y
> CONFIG_SND_HDA_CODEC_CIRRUS=y
> CONFIG_SND_HDA_CODEC_CONEXANT=y
> CONFIG_SND_HDA_CODEC_CA0110=y
> CONFIG_SND_HDA_CODEC_CA0132=y
> CONFIG_SND_HDA_CODEC_CA0132_DSP=y
> CONFIG_SND_HDA_CODEC_CMEDIA=y
> CONFIG_SND_HDA_CODEC_SI3054=y
> ..
>
> I was confused to why they were '=y' if they were to build modules
> and not '=m'. Localmodconfig will not disable anything with '=y' because
> there's not enough information in lsmod to safely disable a built-in
> config.
>
> Investigating as to why these were '=y' I found this in the Makefile:
>
> ifdef CONFIG_SND_HDA_CODEC_REALTEK
> obj-$(CONFIG_SND_HDA_INTEL) += snd-hda-codec-realtek.o
> endif
> ifdef CONFIG_SND_HDA_CODEC_CMEDIA
> obj-$(CONFIG_SND_HDA_INTEL) += snd-hda-codec-cmedia.o
> endif
> ifdef CONFIG_SND_HDA_CODEC_ANALOG
> obj-$(CONFIG_SND_HDA_INTEL) += snd-hda-codec-analog.o
> endif
> ifdef CONFIG_SND_HDA_CODEC_SIGMATEL
> obj-$(CONFIG_SND_HDA_INTEL) += snd-hda-codec-idt.o
> endif
> ifdef CONFIG_SND_HDA_CODEC_SI3054
> obj-$(CONFIG_SND_HDA_INTEL) += snd-hda-codec-si3054.o
> endif
> [...]
>
> And the reason for this is because if the code for SND_HDA_INTEL
> is a built-in, then all the codecs that are configured must also be
> built-in. If it is a module, then all the codecs must also be a module.
>
> To put this another way; if SND_HDA_INTEL is 'y' then all the codecs
> must either be 'y' or 'n'. If SND_HDA_INTEL is 'm', then all the
> codecs must be 'm' or 'n'. The latter is already supported by the
> kconfig code, but the former is not. That is, if a tristate config
> is dependent on a config that is built-in 'y', then that config can
> still become a module 'm'. This trick that is done above prevents that.
> But unfortunately, it also confuses localmodconfig, and the user
> does not get the correct result.
>
> Ideally, having a "match_config" option in the kconfig subsystem would
> be ideal. But anyone that ever took a look at the kconfig code knows that
> is much more difficult to implement than one would expect, as that
> code can cause severe eye strain.
>
> But luckily for us, we can do another trick to keep the behavior needed
> by the ALSA hda code, and still allow for localmodconfig to disable the
> modules.
>
> Now to fix this so that the codecs match the tristate of SND_HDA_INTEL
> we can add a config that states that SND_HDA_INTEL is 'y', and then add
> another config for each codec for it to build the module. We then switch
> all the codecs to be tristates too.
>
> The new config for HDA_INTEL set as 'y' is:
>
> config SND_HDA_YES
> default y if SND_HDA_INTEL=y
>
> The new codec configs will have the following format:
>
> config SND_HDA_CODEC_FOO_BUILD
> def_tristate (SND_HDA_CODEC_FOO=m && SND_HDA_YES) || SND_HDA_CODEC_FOO
>
> What the above does is to set SND_HDA_CODEC_FOO_BUILD to 'y' if
> SND_HDA_CODEC_FOO is a module and SND_HDA_YES is set. This forces
> the codec to be built-in. Otherwise, it just sets it to the state of
> SND_HDA_CODEC_FOO.
>
> This change lets localmodconfig turn off codecs that are not being used.
>
> The first patch fixes a bug in localmodconfig that this ordeal uncovered,
> and that was the fact that localmodconfig did not honor default dependencies
> like "def_tristate (SND_HDA_CODEC_FOO=m && SND_HDA_YES) || SND_HDA_CODEC_FOO"
Well, as mentioned in the mail thread, I'm afraid that people don't
like to see yet doubled kconfig items just for workarounds by this
approach. So it'd be likely better not to restrict but just let users
choose tristate. I already submitted the patches (the same ones as
attached before) to alsa-devel ML.
Another thing we may add is a warning like:
================================================================
config SND_HDA_CODEC_REALTEK
tristate "Build Realtek HD-audio codec support"
select SND_HDA_GENERIC
help
Say Y or M here to include Realtek HD-audio codec support in
snd-hda-intel driver, such as ALC880.
comment "Set to Y if you want auto-loading the codec driver"
depends on SND_HDA_CODEC_REALTEK && SND_HDA_INTEL != SND_HDA_CODEC_REALTEK
================================================================
The comment won't break make localmodconfig, right?
This should be intuitive enough for avoiding pitfalls, I hope.
thanks,
Takashi
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [RFC][PATCH 0/2] localmodconfig / ALSA: hda - Have codecs be disabled by localmodconfig
2013-12-18 19:05 ` [RFC][PATCH 0/2] localmodconfig / ALSA: hda - Have codecs " Takashi Iwai
@ 2013-12-18 19:16 ` Steven Rostedt
0 siblings, 0 replies; 5+ messages in thread
From: Steven Rostedt @ 2013-12-18 19:16 UTC (permalink / raw)
To: Takashi Iwai; +Cc: linux-kernel, Linus Torvalds, Michal Marek, Andrew Morton
On Wed, 18 Dec 2013 20:05:11 +0100
Takashi Iwai <tiwai@suse.de> wrote:
> Well, as mentioned in the mail thread, I'm afraid that people don't
> like to see yet doubled kconfig items just for workarounds by this
> approach. So it'd be likely better not to restrict but just let users
> choose tristate. I already submitted the patches (the same ones as
> attached before) to alsa-devel ML.
Sure, that's why I added the RFC.
>
> Another thing we may add is a warning like:
>
> ================================================================
> config SND_HDA_CODEC_REALTEK
> tristate "Build Realtek HD-audio codec support"
> select SND_HDA_GENERIC
> help
> Say Y or M here to include Realtek HD-audio codec support in
> snd-hda-intel driver, such as ALC880.
>
> comment "Set to Y if you want auto-loading the codec driver"
> depends on SND_HDA_CODEC_REALTEK && SND_HDA_INTEL != SND_HDA_CODEC_REALTEK
> ================================================================
>
> The comment won't break make localmodconfig, right?
Nope, it shouldn't affect localmodconfig at all.
> This should be intuitive enough for avoiding pitfalls, I hope.
But the first patch still needs to be added. I'll just send that out by
itself.
Thanks,
-- Steve
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2013-12-18 19:17 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-12-18 17:35 [RFC][PATCH 0/2] localmodconfig / ALSA: hda - Have codecs be disabled by localmodconfig Steven Rostedt
2013-12-18 17:35 ` [RFC][PATCH 1/2] localmodconfig: Add config depends by default settings Steven Rostedt
2013-12-18 17:35 ` [RFC][PATCH 2/2] ALSA: hda - Change codec configs to be disabled by localmodconfig Steven Rostedt
2013-12-18 19:05 ` [RFC][PATCH 0/2] localmodconfig / ALSA: hda - Have codecs " Takashi Iwai
2013-12-18 19:16 ` Steven Rostedt
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox