| Message ID | 20200219074950.23344-1-masahiroy@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | kconfig: make 'imply' obey the direct dependency | expand |
Hi Yamada-san, On Wed, Feb 19, 2020 at 8:51 AM Masahiro Yamada <masahiroy@kernel.org> wrote: > The 'imply' statement may create unmet direct dependency when the > implied symbol depends on m. > > [Test Code] > > config FOO > tristate "foo" > imply BAZ > > config BAZ > tristate "baz" > depends on BAR > > config BAR > def_tristate m > > config MODULES > def_bool y > option modules > > If you set FOO=y, BAZ is also promoted to y, which results in the > following .config file: > > CONFIG_FOO=y > CONFIG_BAZ=y > CONFIG_BAR=m > CONFIG_MODULES=y > > This ignores the dependency "BAZ depends on BAR". > > Unlike 'select', what is worse, Kconfig never shows the > "WARNING: unmet direct dependencies detected for ..." for this case. > > Because 'imply' should be weaker than 'depends on', Kconfig should > take the direct dependency into account. > > Describe this case in Documentation/kbuild/kconfig-language.rst for > clarification. > > Commit 237e3ad0f195 ("Kconfig: Introduce the "imply" keyword") says that > a symbol implied by y is restricted to y or n, excluding m. > > As for the combination of FOO=y and BAR=m, the case of BAZ=m is excluded > by the 'imply', and BAZ=y is also excluded by 'depends on'. So, only the > possible value is BAZ=n. > > Having said that, this case was probably "We don't care" at that time > because Kconfig did not handle 'depends on m' correctly until > commit f622f8279581 ("kconfig: warn unmet direct dependency of tristate > symbols selected by y") fixed it. > > Backporting this to 4.19+ will probably be fine. If you care this > problem on 4.14.x, you need to backport f622f8279581 as well. > > Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> Thanks a lot! This fixes the build issues in https://lore.kernel.org/alsa-devel/CAMuHMdW8SvDgQJyenTtEm4Xn2Ma6PK9pfwKR2_gn60t2AqNWXg@mail.gmail.com/ Tested-by: Geert Uytterhoeven <geert@linux-m68k.org> Gr{oetje,eeting}s, Geert
On Wed, Feb 19, 2020 at 5:42 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote: > > Hi Yamada-san, > > On Wed, Feb 19, 2020 at 8:51 AM Masahiro Yamada <masahiroy@kernel.org> wrote: > > The 'imply' statement may create unmet direct dependency when the > > implied symbol depends on m. > > > > [Test Code] > > > > config FOO > > tristate "foo" > > imply BAZ > > > > config BAZ > > tristate "baz" > > depends on BAR > > > > config BAR > > def_tristate m > > > > config MODULES > > def_bool y > > option modules > > > > If you set FOO=y, BAZ is also promoted to y, which results in the > > following .config file: > > > > CONFIG_FOO=y > > CONFIG_BAZ=y > > CONFIG_BAR=m > > CONFIG_MODULES=y > > > > This ignores the dependency "BAZ depends on BAR". > > > > Unlike 'select', what is worse, Kconfig never shows the > > "WARNING: unmet direct dependencies detected for ..." for this case. > > > > Because 'imply' should be weaker than 'depends on', Kconfig should > > take the direct dependency into account. > > > > Describe this case in Documentation/kbuild/kconfig-language.rst for > > clarification. > > > > Commit 237e3ad0f195 ("Kconfig: Introduce the "imply" keyword") says that > > a symbol implied by y is restricted to y or n, excluding m. > > > > As for the combination of FOO=y and BAR=m, the case of BAZ=m is excluded > > by the 'imply', and BAZ=y is also excluded by 'depends on'. So, only the > > possible value is BAZ=n. > > > > Having said that, this case was probably "We don't care" at that time > > because Kconfig did not handle 'depends on m' correctly until > > commit f622f8279581 ("kconfig: warn unmet direct dependency of tristate > > symbols selected by y") fixed it. > > > > Backporting this to 4.19+ will probably be fine. If you care this > > problem on 4.14.x, you need to backport f622f8279581 as well. > > > > Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> > > Thanks a lot! This fixes the build issues in > https://lore.kernel.org/alsa-devel/CAMuHMdW8SvDgQJyenTtEm4Xn2Ma6PK9pfwKR2_gn60t2AqNWXg@mail.gmail.com/ > > Tested-by: Geert Uytterhoeven <geert@linux-m68k.org> > Even if this patch fixes the build issues, the result may not be what you expect. The very subtle part of 'imply' is that symbols implied by 'y' cannot be 'm'. When CONFIG_SND_SOC_ALL_CODECS=y, the select'ed drivers were previously able to be 'm'. After ea00d95200d02, drivers that were previously 'm' are now 'n'. Probably, it shrunk the test-coverage. I'd recommend to compare the .config file.
Hi Yamada-san, On Wed, Feb 19, 2020 at 10:24 AM Masahiro Yamada <masahiroy@kernel.org> wrote: > On Wed, Feb 19, 2020 at 5:42 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote: > > On Wed, Feb 19, 2020 at 8:51 AM Masahiro Yamada <masahiroy@kernel.org> wrote: > > > The 'imply' statement may create unmet direct dependency when the > > > implied symbol depends on m. > > > > > > [Test Code] > > > > > > config FOO > > > tristate "foo" > > > imply BAZ > > > > > > config BAZ > > > tristate "baz" > > > depends on BAR > > > > > > config BAR > > > def_tristate m > > > > > > config MODULES > > > def_bool y > > > option modules > > > > > > If you set FOO=y, BAZ is also promoted to y, which results in the > > > following .config file: > > > > > > CONFIG_FOO=y > > > CONFIG_BAZ=y > > > CONFIG_BAR=m > > > CONFIG_MODULES=y > > > > > > This ignores the dependency "BAZ depends on BAR". > > > > > > Unlike 'select', what is worse, Kconfig never shows the > > > "WARNING: unmet direct dependencies detected for ..." for this case. > > > > > > Because 'imply' should be weaker than 'depends on', Kconfig should > > > take the direct dependency into account. > > > > > > Describe this case in Documentation/kbuild/kconfig-language.rst for > > > clarification. > > > > > > Commit 237e3ad0f195 ("Kconfig: Introduce the "imply" keyword") says that > > > a symbol implied by y is restricted to y or n, excluding m. > > > > > > As for the combination of FOO=y and BAR=m, the case of BAZ=m is excluded > > > by the 'imply', and BAZ=y is also excluded by 'depends on'. So, only the > > > possible value is BAZ=n. > > > > > > Having said that, this case was probably "We don't care" at that time > > > because Kconfig did not handle 'depends on m' correctly until > > > commit f622f8279581 ("kconfig: warn unmet direct dependency of tristate > > > symbols selected by y") fixed it. > > > > > > Backporting this to 4.19+ will probably be fine. If you care this > > > problem on 4.14.x, you need to backport f622f8279581 as well. > > > > > > Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> > > > > Thanks a lot! This fixes the build issues in > > https://lore.kernel.org/alsa-devel/CAMuHMdW8SvDgQJyenTtEm4Xn2Ma6PK9pfwKR2_gn60t2AqNWXg@mail.gmail.com/ > > > > Tested-by: Geert Uytterhoeven <geert@linux-m68k.org> > > Even if this patch fixes the build issues, > the result may not be what you expect. > > The very subtle part of 'imply' is that > symbols implied by 'y' cannot be 'm'. > > When CONFIG_SND_SOC_ALL_CODECS=y, > the select'ed drivers were previously able to be 'm'. > > After ea00d95200d02, drivers that were previously > 'm' are now 'n'. > > Probably, it shrunk the test-coverage. > > I'd recommend to compare the .config file. I always do, when running make oldconfig ;-) I only see expected changes from y to m. I don't see any unexpected changes from m to n. I do see a few unexpected "CONFIG_<foo> is not set" that are no longer suppressed. --- .config.orig 2020-02-19 09:26:41.411812042 +0100 +++ .config 2020-02-19 09:28:05.143822097 +0100 @@ -1193,7 +1193,7 @@ CONFIG_REGMAP=y CONFIG_REGMAP_AC97=y CONFIG_REGMAP_I2C=y -CONFIG_REGMAP_SLIMBUS=y +CONFIG_REGMAP_SLIMBUS=m CONFIG_REGMAP_SPI=y CONFIG_REGMAP_MMIO=y CONFIG_REGMAP_IRQ=y @@ -5235,7 +5235,7 @@ CONFIG_SND_SOC_UDA1334=y CONFIG_SND_SOC_UDA134X=y CONFIG_SND_SOC_UDA1380=y CONFIG_SND_SOC_WCD934X=m CONFIG_SND_SOC_WL1273=y CONFIG_SND_SOC_WM0010=y @@ -5017,6 +5017,7 @@ CONFIG_SND_SOC_ARIZONA=y CONFIG_SND_SOC_WM_HUBS=y CONFIG_SND_SOC_WM_ADSP=y +# CONFIG_SND_SOC_AB8500_CODEC is not set CONFIG_SND_SOC_AC97_CODEC=y CONFIG_SND_SOC_AD1836=y CONFIG_SND_SOC_AD193X=y @@ -5085,6 +5086,7 @@ CONFIG_SND_SOC_CS4341=y CONFIG_SND_SOC_CS4349=y CONFIG_SND_SOC_CS47L15=y +# CONFIG_SND_SOC_CS47L24 is not set CONFIG_SND_SOC_CS47L35=y CONFIG_SND_SOC_CS47L85=y CONFIG_SND_SOC_CS47L90=y @@ -5117,6 +5119,7 @@ CONFIG_SND_SOC_INNO_RK3036=y CONFIG_SND_SOC_ISABELLE=y CONFIG_SND_SOC_LM49453=y +# CONFIG_SND_SOC_LOCHNAGAR_SC is not set CONFIG_SND_SOC_MADERA=y CONFIG_SND_SOC_MAX98088=y CONFIG_SND_SOC_MAX98090=y @@ -5228,11 +5231,12 @@ CONFIG_SND_SOC_TSCS42XX=y CONFIG_SND_SOC_TSCS454=y CONFIG_SND_SOC_TWL4030=y +# CONFIG_SND_SOC_TWL6040 is not set CONFIG_SND_SOC_UDA1334=y CONFIG_SND_SOC_UDA134X=y CONFIG_SND_SOC_UDA1380=y -CONFIG_SND_SOC_WCD9335=y -CONFIG_SND_SOC_WCD934X=y +CONFIG_SND_SOC_WCD9335=m +CONFIG_SND_SOC_WCD934X=m CONFIG_SND_SOC_WL1273=y CONFIG_SND_SOC_WM0010=y CONFIG_SND_SOC_WM1250_EV1=y @@ -5241,6 +5245,8 @@ CONFIG_SND_SOC_WM5100=y CONFIG_SND_SOC_WM5102=m CONFIG_SND_SOC_WM5110=y +# CONFIG_SND_SOC_WM8350 is not set +# CONFIG_SND_SOC_WM8400 is not set CONFIG_SND_SOC_WM8510=y CONFIG_SND_SOC_WM8523=y CONFIG_SND_SOC_WM8524=y @@ -5293,7 +5299,7 @@ CONFIG_SND_SOC_MAX9768=y CONFIG_SND_SOC_MAX9877=y # CONFIG_SND_SOC_DUMMY is not set -CONFIG_SND_SOC_MC13783=y +CONFIG_SND_SOC_MC13783=m CONFIG_SND_SOC_ML26124=y CONFIG_SND_SOC_MT6351=y CONFIG_SND_SOC_MT6358=y @@ -5899,6 +5905,7 @@ # CONFIG_COMMON_CLK_MT8516 is not set # end of Clock driver for MediaTek SoC +# CONFIG_COMMON_CLK_AXG_AUDIO is not set CONFIG_QCOM_GDSC=y CONFIG_COMMON_CLK_QCOM=m CONFIG_QCOM_A53PLL=m Gr{oetje,eeting}s, Geert
On Wed, 19 Feb 2020, Masahiro Yamada wrote: > The 'imply' statement may create unmet direct dependency when the > implied symbol depends on m. > > [Test Code] > > config FOO > tristate "foo" > imply BAZ > > config BAZ > tristate "baz" > depends on BAR > > config BAR > def_tristate m > > config MODULES > def_bool y > option modules > > If you set FOO=y, BAZ is also promoted to y, which results in the > following .config file: > > CONFIG_FOO=y > CONFIG_BAZ=y > CONFIG_BAR=m > CONFIG_MODULES=y > > This ignores the dependency "BAZ depends on BAR". > > Unlike 'select', what is worse, Kconfig never shows the > "WARNING: unmet direct dependencies detected for ..." for this case. > > Because 'imply' should be weaker than 'depends on', Kconfig should > take the direct dependency into account. > > Describe this case in Documentation/kbuild/kconfig-language.rst for > clarification. > > Commit 237e3ad0f195 ("Kconfig: Introduce the "imply" keyword") says that > a symbol implied by y is restricted to y or n, excluding m. > > As for the combination of FOO=y and BAR=m, the case of BAZ=m is excluded > by the 'imply', and BAZ=y is also excluded by 'depends on'. So, only the > possible value is BAZ=n. I don't think this is right. The imply keyword provide influence over another symbol but it should not impose any restrictions. If BAR=m then BAZ should still be allowed to be m or n. > @@ -174,6 +174,9 @@ applicable everywhere (see syntax). > n y n N/m/y > m y m M/y/n > y y y Y/n > + n m n N/m > + m m m M/n > + y m n N Here the last line shoule be y m n N/m. Generally speaking, the code enabled by FOO may rely on functionalities provided by BAZ only when BAZ >= FOO. This is accomplished with IS_REACHABLE(): foo_init() { if (IS_REACHABLE(CONFIG_BAZ)) baz_register(&foo); ... } So if FOO=y and BAZ=m then IS_REACHABLE(CONFIG_BAZ) will be false. Maybe adding a note to that effect linked to the "y m n N/m" line in the table would be a good idea. Nicolas
Hi Nicolas, On Thu, Feb 20, 2020 at 1:16 AM Nicolas Pitre <nico@fluxnic.net> wrote: > > On Wed, 19 Feb 2020, Masahiro Yamada wrote: > > > The 'imply' statement may create unmet direct dependency when the > > implied symbol depends on m. > > > > [Test Code] > > > > config FOO > > tristate "foo" > > imply BAZ > > > > config BAZ > > tristate "baz" > > depends on BAR > > > > config BAR > > def_tristate m > > > > config MODULES > > def_bool y > > option modules > > > > If you set FOO=y, BAZ is also promoted to y, which results in the > > following .config file: > > > > CONFIG_FOO=y > > CONFIG_BAZ=y > > CONFIG_BAR=m > > CONFIG_MODULES=y > > > > This ignores the dependency "BAZ depends on BAR". > > > > Unlike 'select', what is worse, Kconfig never shows the > > "WARNING: unmet direct dependencies detected for ..." for this case. > > > > Because 'imply' should be weaker than 'depends on', Kconfig should > > take the direct dependency into account. > > > > Describe this case in Documentation/kbuild/kconfig-language.rst for > > clarification. > > > > Commit 237e3ad0f195 ("Kconfig: Introduce the "imply" keyword") says that > > a symbol implied by y is restricted to y or n, excluding m. > > > > As for the combination of FOO=y and BAR=m, the case of BAZ=m is excluded > > by the 'imply', and BAZ=y is also excluded by 'depends on'. So, only the > > possible value is BAZ=n. > > I don't think this is right. The imply keyword provide influence over > another symbol but it should not impose any restrictions. If BAR=m then > BAZ should still be allowed to be m or n. > > > @@ -174,6 +174,9 @@ applicable everywhere (see syntax). > > n y n N/m/y > > m y m M/y/n > > y y y Y/n > > + n m n N/m > > + m m m M/n > > + y m n N > > Here the last line shoule be y m n N/m. > > Generally speaking, the code enabled by FOO may rely on functionalities > provided by BAZ only when BAZ >= FOO. This is accomplished with > IS_REACHABLE(): > > foo_init() > { > if (IS_REACHABLE(CONFIG_BAZ)) > baz_register(&foo); > ... > } > > So if FOO=y and BAZ=m then IS_REACHABLE(CONFIG_BAZ) will be false. Maybe > adding a note to that effect linked to the "y m n N/m" line in the table > would be a good idea. > I also thought so. I agree IS_REACHABLE() is much saner approach. So, do you agree to change the current behavior as follows? index d0111dd26410..47dbfd1ee003 100644 --- a/Documentation/kbuild/kconfig-language.rst +++ b/Documentation/kbuild/kconfig-language.rst @@ -173,7 +173,7 @@ applicable everywhere (see syntax). === === ============= ============== n y n N/m/y m y m M/y/n - y y y Y/n + y y y Y/m/n y n * N === === ============= ==============
On Thu, 20 Feb 2020, Masahiro Yamada wrote: > Hi Nicolas, > > On Thu, Feb 20, 2020 at 1:16 AM Nicolas Pitre <nico@fluxnic.net> wrote: > > > > On Wed, 19 Feb 2020, Masahiro Yamada wrote: > > > > > As for the combination of FOO=y and BAR=m, the case of BAZ=m is excluded > > > by the 'imply', and BAZ=y is also excluded by 'depends on'. So, only the > > > possible value is BAZ=n. > > > > I don't think this is right. The imply keyword provide influence over > > another symbol but it should not impose any restrictions. If BAR=m then > > BAZ should still be allowed to be m or n. > > > > > @@ -174,6 +174,9 @@ applicable everywhere (see syntax). > > > n y n N/m/y > > > m y m M/y/n > > > y y y Y/n > > > + n m n N/m > > > + m m m M/n > > > + y m n N > > > > Here the last line shoule be y m n N/m. > > > > Generally speaking, the code enabled by FOO may rely on functionalities > > provided by BAZ only when BAZ >= FOO. This is accomplished with > > IS_REACHABLE(): > > > > foo_init() > > { > > if (IS_REACHABLE(CONFIG_BAZ)) > > baz_register(&foo); > > ... > > } > > > > So if FOO=y and BAZ=m then IS_REACHABLE(CONFIG_BAZ) will be false. Maybe > > adding a note to that effect linked to the "y m n N/m" line in the table > > would be a good idea. > > > > I also thought so. > > I agree IS_REACHABLE() is much saner approach. > > So, do you agree to change the current behavior > as follows? > > > index d0111dd26410..47dbfd1ee003 100644 > --- a/Documentation/kbuild/kconfig-language.rst > +++ b/Documentation/kbuild/kconfig-language.rst > @@ -173,7 +173,7 @@ applicable everywhere (see syntax). > === === ============= ============== > n y n N/m/y > m y m M/y/n > - y y y Y/n > + y y y Y/m/n > y n * N > === === ============= ============== > Yes. That should have been the case all along. Nicolas
Hi Geert, On Wed, Feb 19, 2020 at 6:54 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote: > > Hi Yamada-san, > > On Wed, Feb 19, 2020 at 10:24 AM Masahiro Yamada <masahiroy@kernel.org> wrote: > > On Wed, Feb 19, 2020 at 5:42 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote: > > > On Wed, Feb 19, 2020 at 8:51 AM Masahiro Yamada <masahiroy@kernel.org> wrote: > > > > The 'imply' statement may create unmet direct dependency when the > > > > implied symbol depends on m. > > > > > > > > [Test Code] > > > > > > > > config FOO > > > > tristate "foo" > > > > imply BAZ > > > > > > > > config BAZ > > > > tristate "baz" > > > > depends on BAR > > > > > > > > config BAR > > > > def_tristate m > > > > > > > > config MODULES > > > > def_bool y > > > > option modules > > > > > > > > If you set FOO=y, BAZ is also promoted to y, which results in the > > > > following .config file: > > > > > > > > CONFIG_FOO=y > > > > CONFIG_BAZ=y > > > > CONFIG_BAR=m > > > > CONFIG_MODULES=y > > > > > > > > This ignores the dependency "BAZ depends on BAR". > > > > > > > > Unlike 'select', what is worse, Kconfig never shows the > > > > "WARNING: unmet direct dependencies detected for ..." for this case. > > > > > > > > Because 'imply' should be weaker than 'depends on', Kconfig should > > > > take the direct dependency into account. > > > > > > > > Describe this case in Documentation/kbuild/kconfig-language.rst for > > > > clarification. > > > > > > > > Commit 237e3ad0f195 ("Kconfig: Introduce the "imply" keyword") says that > > > > a symbol implied by y is restricted to y or n, excluding m. > > > > > > > > As for the combination of FOO=y and BAR=m, the case of BAZ=m is excluded > > > > by the 'imply', and BAZ=y is also excluded by 'depends on'. So, only the > > > > possible value is BAZ=n. > > > > > > > > Having said that, this case was probably "We don't care" at that time > > > > because Kconfig did not handle 'depends on m' correctly until > > > > commit f622f8279581 ("kconfig: warn unmet direct dependency of tristate > > > > symbols selected by y") fixed it. > > > > > > > > Backporting this to 4.19+ will probably be fine. If you care this > > > > problem on 4.14.x, you need to backport f622f8279581 as well. > > > > > > > > Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> > > > > > > Thanks a lot! This fixes the build issues in > > > https://lore.kernel.org/alsa-devel/CAMuHMdW8SvDgQJyenTtEm4Xn2Ma6PK9pfwKR2_gn60t2AqNWXg@mail.gmail.com/ > > > > > > Tested-by: Geert Uytterhoeven <geert@linux-m68k.org> > > > > Even if this patch fixes the build issues, > > the result may not be what you expect. > > > > The very subtle part of 'imply' is that > > symbols implied by 'y' cannot be 'm'. > > > > When CONFIG_SND_SOC_ALL_CODECS=y, > > the select'ed drivers were previously able to be 'm'. > > > > After ea00d95200d02, drivers that were previously > > 'm' are now 'n'. > > > > Probably, it shrunk the test-coverage. > > > > I'd recommend to compare the .config file. > > I always do, when running make oldconfig ;-) > > I only see expected changes from y to m. > I don't see any unexpected changes from m to n. This is because you used oldconfig. The 'imply' keyword defines the default of symbols _without_ user-defined values. oldconfig loads the .config file, which contains the user-defined values. When you test the 'imply' keyword, you must start Kconfig from a pristine state (e.g. 'make defconfig') Compare the difference between v5.6-rc4 and v5.6-rc4 + the following two patches. https://lore.kernel.org/patchwork/patch/1190445/ https://lore.kernel.org/patchwork/patch/1196514/ [How to reproduce] Tweak x86_64_defconfig as follows, then compare the result of 'make defconfig'. masahiro@pug:~/ref/linux$ git diff diff --git a/arch/x86/configs/x86_64_defconfig b/arch/x86/configs/x86_64_defconfig index 0b9654c7a05c..e102d75d3707 100644 --- a/arch/x86/configs/x86_64_defconfig +++ b/arch/x86/configs/x86_64_defconfig @@ -194,7 +194,7 @@ CONFIG_WATCHDOG=y CONFIG_AGP=y CONFIG_AGP_AMD64=y CONFIG_AGP_INTEL=y -CONFIG_DRM=y +# CONFIG_DRM is not set CONFIG_DRM_I915=y CONFIG_FB_MODE_HELPERS=y CONFIG_FB_TILEBLITTING=y @@ -296,3 +296,11 @@ CONFIG_SECURITY_SELINUX_DISABLE=y CONFIG_EFI_STUB=y CONFIG_EFI_MIXED=y CONFIG_ACPI_BGRT=y +CONFIG_EXPERT=y +# CONFIG_TTY is not set +CONFIG_INPUT=m +CONFIG_I2C=y +CONFIG_SPI=y +CONFIG_COMPILE_TEST=y +CONFIG_SND_SOC=y +CONFIG_SND_SOC_ALL_CODECS=y masahiro@pug:~/ref/linux$ git checkout v5.6-rc4 M arch/x86/configs/x86_64_defconfig Note: checking out 'v5.6-rc4'. You are in 'detached HEAD' state. You can look around, make experimental changes and commit them, and you can discard any commits you make in this state without impacting any branches by performing another checkout. If you want to create a new branch to retain commits you create, you may do so (now or later) by using -b with the checkout command again. Example: git checkout -b <new-branch-name> HEAD is now at 98d54f81e36b Linux 5.6-rc4 masahiro@pug:~/ref/linux$ git log --oneline -1 98d54f81e36b (HEAD, tag: v5.6-rc4, origin/master, origin/HEAD, master) Linux 5.6-rc4 masahiro@pug:~/ref/linux$ make -s O=before defconfig M arch/x86/configs/x86_64_defconfig Previous HEAD position was 98d54f81e36b Linux 5.6-rc4 Switched to branch 'geert' masahiro@pug:~/ref/linux$ git log --oneline -3 231be9523aee (HEAD -> geert) kconfig: make 'imply' obey the direct dependency 4132db11c62e ASoC: Use imply for SND_SOC_ALL_CODECS 98d54f81e36b (tag: v5.6-rc4, origin/master, origin/HEAD, master) Linux 5.6-rc4 masahiro@pug:~/ref/linux$ make -s O=after defconfig masahiro@pug:~/ref/linux$ ./scripts/diffconfig before/.config after/.config SND_SOC_CS42L52 m -> n SND_SOC_CS42L56 m -> n SND_SOC_MT6351 n -> y SND_SOC_MT6358 n -> y SND_SOC_WM8962 m -> n +SND_SOC_88PM860X n +SND_SOC_ARIZONA y +SND_SOC_CROS_EC_CODEC n +SND_SOC_CS47L15 y +SND_SOC_CS47L24 y +SND_SOC_CS47L35 y +SND_SOC_CS47L85 y +SND_SOC_CS47L90 y +SND_SOC_CS47L92 y +SND_SOC_CX20442 n +SND_SOC_LOCHNAGAR_SC n +SND_SOC_MADERA y +SND_SOC_MC13783 y +SND_SOC_RT1308_SDW n +SND_SOC_RT700_SDW n +SND_SOC_RT711_SDW n +SND_SOC_RT715_SDW n +SND_SOC_SI476X y +SND_SOC_TWL4030 n +SND_SOC_TWL6040 n +SND_SOC_WCD9335 n +SND_SOC_WCD934X n +SND_SOC_WL1273 y +SND_SOC_WM5102 y +SND_SOC_WM5110 y +SND_SOC_WM8350 n +SND_SOC_WM8400 n +SND_SOC_WM8994 y +SND_SOC_WM8997 y +SND_SOC_WM8998 y +SND_SOC_WSA881X n SND_SOC_CS42L52, SND_SOC_CS42L56 and SND_SOC_WM8962 were previously m, but now disabled. -- Best Regards Masahiro Yamada
On Mon, Mar 02, 2020 at 05:21:51PM +0900, Masahiro Yamada wrote: > On Wed, Feb 19, 2020 at 6:54 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote: > > I always do, when running make oldconfig ;-) > > I only see expected changes from y to m. > > I don't see any unexpected changes from m to n. > This is because you used oldconfig. > The 'imply' keyword defines the default > of symbols _without_ user-defined values. This is going to make the behaviour of imply statements a bit inconsistent which doesn't seem ideal though it's *probably* fine for the SND_SOC_ALL_CODECS case.
diff --git a/Documentation/kbuild/kconfig-language.rst b/Documentation/kbuild/kconfig-language.rst index d0111dd26410..9141f03ff744 100644 --- a/Documentation/kbuild/kconfig-language.rst +++ b/Documentation/kbuild/kconfig-language.rst @@ -159,11 +159,11 @@ applicable everywhere (see syntax). Given the following example:: config FOO - tristate + tristate "foo" imply BAZ config BAZ - tristate + tristate "baz" depends on BAR The following values are possible: @@ -174,6 +174,9 @@ applicable everywhere (see syntax). n y n N/m/y m y m M/y/n y y y Y/n + n m n N/m + m m m M/n + y m n N y n * N === === ============= ============== diff --git a/scripts/kconfig/symbol.c b/scripts/kconfig/symbol.c index 8d38b700b314..6100e6ead8ad 100644 --- a/scripts/kconfig/symbol.c +++ b/scripts/kconfig/symbol.c @@ -221,7 +221,7 @@ static void sym_calc_visibility(struct symbol *sym) sym_set_changed(sym); } tri = no; - if (sym->implied.expr && sym->dir_dep.tri != no) + if (sym->implied.expr) tri = expr_calc_value(sym->implied.expr); if (tri == mod && sym_get_type(sym) == S_BOOLEAN) tri = yes; @@ -393,7 +393,17 @@ void sym_calc_value(struct symbol *sym) } if (sym->implied.tri != no) { sym->flags |= SYMBOL_WRITE; - newval.tri = EXPR_OR(newval.tri, sym->implied.tri); + newval.tri = EXPR_OR(newval.tri, + sym->implied.tri); + newval.tri = EXPR_AND(newval.tri, + sym->dir_dep.tri); + + if (newval.tri == mod && sym->implied.tri == yes) { + if (sym->dir_dep.tri == yes) + newval.tri = yes; + else + newval.tri = no; + } } } calc_newval: @@ -401,8 +411,7 @@ void sym_calc_value(struct symbol *sym) sym_warn_unmet_dep(sym); newval.tri = EXPR_OR(newval.tri, sym->rev_dep.tri); } - if (newval.tri == mod && - (sym_get_type(sym) == S_BOOLEAN || sym->implied.tri == yes)) + if (newval.tri == mod && sym_get_type(sym) == S_BOOLEAN) newval.tri = yes; break; case S_STRING:
The 'imply' statement may create unmet direct dependency when the implied symbol depends on m. [Test Code] config FOO tristate "foo" imply BAZ config BAZ tristate "baz" depends on BAR config BAR def_tristate m config MODULES def_bool y option modules If you set FOO=y, BAZ is also promoted to y, which results in the following .config file: CONFIG_FOO=y CONFIG_BAZ=y CONFIG_BAR=m CONFIG_MODULES=y This ignores the dependency "BAZ depends on BAR". Unlike 'select', what is worse, Kconfig never shows the "WARNING: unmet direct dependencies detected for ..." for this case. Because 'imply' should be weaker than 'depends on', Kconfig should take the direct dependency into account. Describe this case in Documentation/kbuild/kconfig-language.rst for clarification. Commit 237e3ad0f195 ("Kconfig: Introduce the "imply" keyword") says that a symbol implied by y is restricted to y or n, excluding m. As for the combination of FOO=y and BAR=m, the case of BAZ=m is excluded by the 'imply', and BAZ=y is also excluded by 'depends on'. So, only the possible value is BAZ=n. Having said that, this case was probably "We don't care" at that time because Kconfig did not handle 'depends on m' correctly until commit f622f8279581 ("kconfig: warn unmet direct dependency of tristate symbols selected by y") fixed it. Backporting this to 4.19+ will probably be fine. If you care this problem on 4.14.x, you need to backport f622f8279581 as well. Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> --- Documentation/kbuild/kconfig-language.rst | 7 +++++-- scripts/kconfig/symbol.c | 17 +++++++++++++---- 2 files changed, 18 insertions(+), 6 deletions(-)