* [PATCH v2] Support conditional deps using "depends on X if Y" @ 2025-11-18 18:46 Graham Roff 2025-12-05 1:53 ` Nathan Chancellor 2025-12-05 9:06 ` Jani Nikula 0 siblings, 2 replies; 9+ messages in thread From: Graham Roff @ 2025-11-18 18:46 UTC (permalink / raw) To: Nathan Chancellor, Nicolas Schier, Jonathan Corbet Cc: linux-kbuild, linux-doc, linux-kernel, Nicolas Pitre, Graham Roff From: Nicolas Pitre <nico@fluxnic.net> Extend the "depends on" syntax to support conditional dependencies using "depends on X if Y". While functionally equivalent to "depends on X || (Y == n)", "depends on X if Y" is much more readable and makes the kconfig language uniform in supporting the "if <expr>" suffix. This also improves readability for "optional" dependencies, which are the subset of conditional dependencies where X is Y. Previously such optional dependencies had to be expressed as the counterintuitive "depends on X || !X", now this can be represented as "depends on X if X". The change is implemented by converting the "X if Y" syntax into the "X || (Y == n)" syntax during "depends on" token processing. Signed-off-by: Nicolas Pitre <nico@fluxnic.net> [Graham Roff: Rewrote commit message and redid patch for latest kernel] Signed-off-by: Graham Roff <grahamr@qti.qualcomm.com> --- This patch updates an earlier one that was not merged to work on the latest kernel release. Link: https://lwn.net/ml/linux-kernel/nycvar.YSQ.7.76.2004231102480.2671@knanqh.ubzr/#t Support for this change has been expressed by a number of developers since the original patch was proposed back in 2020, and has recently also been raised as a patch to the Zephyr kconfig system. One specific use is when mapping the Bluetooth specification to Kconfig, as it explicitly provides dependencies between features as conditional on other features. Many other cases exist where the "slightly counterintuitive" (quoted from the Kconfig specification) expression "depends on BAR || !BAR" has been used when a proper "if" condition would be more readable. Some examples: arch/arm64/Kconfig: depends on ARM64_64K_PAGES || !ARM64_VA_BITS_52 --> depends on ARM64_64K_PAGES if ARM64_VA_BITS_52 arch/mips/Kconfig: depends on SYS_SUPPORTS_HOTPLUG_CPU || !SMP --> depends on SYS_SUPPORTS_HOTPLUG_CPU if SMP arch/riscv/Kconfig: depends on CC_HAS_MIN_FUNCTION_ALIGNMENT || !RISCV_ISA_C --> depends on CC_HAS_MIN_FUNCTION_ALIGNMENT if RISCV_ISA_C arch/x86/Kconfig: depends on X86_64 || !SPARSEMEM --> depends on X86_64 if SPARSEMEM drivers/acpi/Kconfig: depends on ACPI_WMI || !X86 --> depends on ACPI_WMI if X86 drivers/bluetooth/Kconfig: depends on USB || !BT_HCIBTUSB_MTK depends on USB if BT_HCIBTUSB_MTK mm/Kconfig: depends on !ARM || CPU_CACHE_VIPT --> depends on CPU_CACHE_VIPT if ARM kernel/Kconfig.locks: depends on !PREEMPTION || ARCH_INLINE_READ_UNLOCK --> depends on ARCH_INLINE_READ_UNLOCK if PREEMPTION The earlier patch discussion ended without a real conclusion and should be revisited now. --- Changes in v2: - EDITME: describe what is new in this series revision. - EDITME: use bulletpoints and terse descriptions. - Link to v1: https://lore.kernel.org/r/20251107-kconfig_conditional_deps-v1-1-aff22199ec0b@qti.qualcomm.com --- Documentation/kbuild/kconfig-language.rst | 22 +++++++++++++++--- scripts/kconfig/lkc.h | 2 +- scripts/kconfig/menu.c | 12 +++++++++- scripts/kconfig/parser.y | 6 ++--- scripts/kconfig/tests/conditional_dep/Kconfig | 27 ++++++++++++++++++++++ scripts/kconfig/tests/conditional_dep/__init__.py | 14 +++++++++++ .../kconfig/tests/conditional_dep/expected_config1 | 10 ++++++++ .../kconfig/tests/conditional_dep/expected_config2 | 8 +++++++ .../kconfig/tests/conditional_dep/expected_config3 | 10 ++++++++ scripts/kconfig/tests/conditional_dep/test_config1 | 4 ++++ scripts/kconfig/tests/conditional_dep/test_config2 | 7 ++++++ scripts/kconfig/tests/conditional_dep/test_config3 | 6 +++++ 12 files changed, 120 insertions(+), 8 deletions(-) diff --git a/Documentation/kbuild/kconfig-language.rst b/Documentation/kbuild/kconfig-language.rst index abce88f15d7c..9ff3e530b2b4 100644 --- a/Documentation/kbuild/kconfig-language.rst +++ b/Documentation/kbuild/kconfig-language.rst @@ -118,7 +118,7 @@ applicable everywhere (see syntax). This is a shorthand notation for a type definition plus a value. Optionally dependencies for this default value can be added with "if". -- dependencies: "depends on" <expr> +- dependencies: "depends on" <expr> ["if" <expr>] This defines a dependency for this menu entry. If multiple dependencies are defined, they are connected with '&&'. Dependencies @@ -134,6 +134,16 @@ applicable everywhere (see syntax). bool "foo" default y + The dependency definition itself may be conditional by appending "if" + followed by an expression. For example:: + + config FOO + tristate + depends on BAR if BAZ + + meaning that FOO is constrained by the value of BAR only if BAZ is + also set. + - reverse dependencies: "select" <symbol> ["if" <expr>] While normal dependencies reduce the upper limit of a symbol (see @@ -602,8 +612,14 @@ Some drivers are able to optionally use a feature from another module or build cleanly with that module disabled, but cause a link failure when trying to use that loadable module from a built-in driver. -The most common way to express this optional dependency in Kconfig logic -uses the slightly counterintuitive:: +The recommended way to express this optional dependency in Kconfig logic +uses the conditional form:: + + config FOO + tristate "Support for foo hardware" + depends on BAR if BAR + +This slightly counterintuitive style is also widely used:: config FOO tristate "Support for foo hardware" diff --git a/scripts/kconfig/lkc.h b/scripts/kconfig/lkc.h index 56548efc14d7..798985961215 100644 --- a/scripts/kconfig/lkc.h +++ b/scripts/kconfig/lkc.h @@ -82,7 +82,7 @@ void menu_warn(const struct menu *menu, const char *fmt, ...); struct menu *menu_add_menu(void); void menu_end_menu(void); void menu_add_entry(struct symbol *sym, enum menu_type type); -void menu_add_dep(struct expr *dep); +void menu_add_dep(struct expr *dep, struct expr *cond); void menu_add_visibility(struct expr *dep); struct property *menu_add_prompt(enum prop_type type, const char *prompt, struct expr *dep); diff --git a/scripts/kconfig/menu.c b/scripts/kconfig/menu.c index 0f1a6513987c..b2d8d4e11e07 100644 --- a/scripts/kconfig/menu.c +++ b/scripts/kconfig/menu.c @@ -127,8 +127,18 @@ static struct expr *rewrite_m(struct expr *e) return e; } -void menu_add_dep(struct expr *dep) +void menu_add_dep(struct expr *dep, struct expr *cond) { + if (cond) { + /* + * We have "depends on X if Y" and we want: + * Y != n --> X + * Y == n --> y + * That simplifies to: (X || (Y == n)) + */ + dep = expr_alloc_or(dep, + expr_trans_compare(cond, E_EQUAL, &symbol_no)); + } current_entry->dep = expr_alloc_and(current_entry->dep, dep); } diff --git a/scripts/kconfig/parser.y b/scripts/kconfig/parser.y index 49b79dde1725..6d1bbee38f5d 100644 --- a/scripts/kconfig/parser.y +++ b/scripts/kconfig/parser.y @@ -323,7 +323,7 @@ if_entry: T_IF expr T_EOL { printd(DEBUG_PARSE, "%s:%d:if\n", cur_filename, cur_lineno); menu_add_entry(NULL, M_IF); - menu_add_dep($2); + menu_add_dep($2, NULL); $$ = menu_add_menu(); }; @@ -422,9 +422,9 @@ help: help_start T_HELPTEXT /* depends option */ -depends: T_DEPENDS T_ON expr T_EOL +depends: T_DEPENDS T_ON expr if_expr T_EOL { - menu_add_dep($3); + menu_add_dep($3, $4); printd(DEBUG_PARSE, "%s:%d:depends on\n", cur_filename, cur_lineno); }; diff --git a/scripts/kconfig/tests/conditional_dep/Kconfig b/scripts/kconfig/tests/conditional_dep/Kconfig new file mode 100644 index 000000000000..ea2bdef9016c --- /dev/null +++ b/scripts/kconfig/tests/conditional_dep/Kconfig @@ -0,0 +1,27 @@ +# SPDX-License-Identifier: GPL-2.0 +# Test Kconfig file for conditional dependencies. + +config FOO + bool "FOO symbol" + +config BAR + bool "BAR symbol" + +config TEST_BASIC + bool "Test basic conditional dependency" + depends on FOO if BAR + default y + +config TEST_COMPLEX + bool "Test complex conditional dependency" + depends on (FOO && BAR) if (FOO || BAR) + default y + +config BAZ + tristate "BAZ symbol" + +config TEST_OPTIONAL + tristate "Test simple optional dependency" + depends on BAZ if BAZ + default y + diff --git a/scripts/kconfig/tests/conditional_dep/__init__.py b/scripts/kconfig/tests/conditional_dep/__init__.py new file mode 100644 index 000000000000..ab16df6487ec --- /dev/null +++ b/scripts/kconfig/tests/conditional_dep/__init__.py @@ -0,0 +1,14 @@ +# SPDX-License-Identifier: GPL-2.0 +""" +Correctly handle conditional dependencies. +""" + +def test(conf): + assert conf.oldconfig('test_config1') == 0 + assert conf.config_matches('expected_config1') + + assert conf.oldconfig('test_config2') == 0 + assert conf.config_matches('expected_config2') + + assert conf.oldconfig('test_config3') == 0 + assert conf.config_matches('expected_config3') diff --git a/scripts/kconfig/tests/conditional_dep/expected_config1 b/scripts/kconfig/tests/conditional_dep/expected_config1 new file mode 100644 index 000000000000..2ad02aa66b06 --- /dev/null +++ b/scripts/kconfig/tests/conditional_dep/expected_config1 @@ -0,0 +1,10 @@ +# +# Automatically generated file; DO NOT EDIT. +# Main menu +# +CONFIG_FOO=y +CONFIG_BAR=y +CONFIG_TEST_BASIC=y +CONFIG_TEST_COMPLEX=y +CONFIG_BAZ=y +CONFIG_TEST_OPTIONAL=y diff --git a/scripts/kconfig/tests/conditional_dep/expected_config2 b/scripts/kconfig/tests/conditional_dep/expected_config2 new file mode 100644 index 000000000000..b4b19cf50730 --- /dev/null +++ b/scripts/kconfig/tests/conditional_dep/expected_config2 @@ -0,0 +1,8 @@ +# +# Automatically generated file; DO NOT EDIT. +# Main menu +# +# CONFIG_FOO is not set +CONFIG_BAR=y +CONFIG_BAZ=y +CONFIG_TEST_OPTIONAL=y diff --git a/scripts/kconfig/tests/conditional_dep/expected_config3 b/scripts/kconfig/tests/conditional_dep/expected_config3 new file mode 100644 index 000000000000..c788f6c710e1 --- /dev/null +++ b/scripts/kconfig/tests/conditional_dep/expected_config3 @@ -0,0 +1,10 @@ +# +# Automatically generated file; DO NOT EDIT. +# Main menu +# +# CONFIG_FOO is not set +# CONFIG_BAR is not set +CONFIG_TEST_BASIC=y +CONFIG_TEST_COMPLEX=y +# CONFIG_BAZ is not set +CONFIG_TEST_OPTIONAL=y diff --git a/scripts/kconfig/tests/conditional_dep/test_config1 b/scripts/kconfig/tests/conditional_dep/test_config1 new file mode 100644 index 000000000000..5cc1ecedcba3 --- /dev/null +++ b/scripts/kconfig/tests/conditional_dep/test_config1 @@ -0,0 +1,4 @@ +# Basic check that everything can be configured if selected. +CONFIG_FOO=y +CONFIG_BAR=y +CONFIG_BAZ=m diff --git a/scripts/kconfig/tests/conditional_dep/test_config2 b/scripts/kconfig/tests/conditional_dep/test_config2 new file mode 100644 index 000000000000..1175c5307308 --- /dev/null +++ b/scripts/kconfig/tests/conditional_dep/test_config2 @@ -0,0 +1,7 @@ +# If FOO is not selected, then TEST_BASIC should fail the conditional +# dependency since BAR is set. +# TEST_COMPLEX will fail dependency as it depends on both FOO and BAR +# if either of those is selected. +CONFIG_FOO=n +CONFIG_BAR=y +CONFIG_BAZ=y diff --git a/scripts/kconfig/tests/conditional_dep/test_config3 b/scripts/kconfig/tests/conditional_dep/test_config3 new file mode 100644 index 000000000000..3815ad744e89 --- /dev/null +++ b/scripts/kconfig/tests/conditional_dep/test_config3 @@ -0,0 +1,6 @@ +# If FOO is not selected, but BAR is also not selected, then TEST_BASIC +# should pass since the dependency on FOO is conditional on BAR. +# TEST_COMPLEX should be also set since neither FOO nor BAR are selected +# so it has no dependencies. +CONFIG_FOO=n +CONFIG_BAR=n --- base-commit: a1388fcb52fcad3e0b06e2cdd0ed757a82a5be30 change-id: 20251106-kconfig_conditional_deps-51f1c903f863 Best regards, -- Graham Roff <grahamr@qti.qualcomm.com> ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v2] Support conditional deps using "depends on X if Y" 2025-11-18 18:46 [PATCH v2] Support conditional deps using "depends on X if Y" Graham Roff @ 2025-12-05 1:53 ` Nathan Chancellor 2025-12-05 8:01 ` Arnd Bergmann 2025-12-05 9:06 ` Jani Nikula 1 sibling, 1 reply; 9+ messages in thread From: Nathan Chancellor @ 2025-12-05 1:53 UTC (permalink / raw) To: Graham Roff Cc: Nicolas Schier, Jonathan Corbet, linux-kbuild, linux-doc, linux-kernel, Nicolas Pitre, Arnd Bergmann Hi Graham, + Arnd, who often fixes Kconfig issues and might have some thoughts around this. I also forgot to mention this on v1 but please add the "kconfig:" subject prefix to the commit message to clearly indicate what area this is touching. On Tue, Nov 18, 2025 at 10:46:51AM -0800, Graham Roff wrote: > From: Nicolas Pitre <nico@fluxnic.net> > > Extend the "depends on" syntax to support conditional dependencies > using "depends on X if Y". While functionally equivalent to "depends > on X || (Y == n)", "depends on X if Y" is much more readable and > makes the kconfig language uniform in supporting the "if <expr>" > suffix. > This also improves readability for "optional" dependencies, which > are the subset of conditional dependencies where X is Y. > Previously such optional dependencies had to be expressed as > the counterintuitive "depends on X || !X", now this can be > represented as "depends on X if X". > > The change is implemented by converting the "X if Y" syntax into the > "X || (Y == n)" syntax during "depends on" token processing. > > Signed-off-by: Nicolas Pitre <nico@fluxnic.net> > > [Graham Roff: Rewrote commit message and redid patch for latest kernel] > > Signed-off-by: Graham Roff <grahamr@qti.qualcomm.com> > --- > This patch updates an earlier one that was not merged to work on > the latest kernel release. > > Link: https://lwn.net/ml/linux-kernel/nycvar.YSQ.7.76.2004231102480.2671@knanqh.ubzr/#t > > Support for this change has been expressed by a number of developers > since the original patch was proposed back in 2020, and has recently > also been raised as a patch to the Zephyr kconfig system. > One specific use is when mapping the Bluetooth specification to Kconfig, > as it explicitly provides dependencies between features as conditional > on other features. Many other cases exist where the "slightly > counterintuitive" (quoted from the Kconfig specification) expression > "depends on BAR || !BAR" has been used when a proper "if" condition > would be more readable. Some examples: > > arch/arm64/Kconfig: > depends on ARM64_64K_PAGES || !ARM64_VA_BITS_52 --> > depends on ARM64_64K_PAGES if ARM64_VA_BITS_52 > arch/mips/Kconfig: > depends on SYS_SUPPORTS_HOTPLUG_CPU || !SMP --> > depends on SYS_SUPPORTS_HOTPLUG_CPU if SMP > arch/riscv/Kconfig: > depends on CC_HAS_MIN_FUNCTION_ALIGNMENT || !RISCV_ISA_C --> > depends on CC_HAS_MIN_FUNCTION_ALIGNMENT if RISCV_ISA_C > arch/x86/Kconfig: > depends on X86_64 || !SPARSEMEM --> > depends on X86_64 if SPARSEMEM > drivers/acpi/Kconfig: > depends on ACPI_WMI || !X86 --> > depends on ACPI_WMI if X86 > drivers/bluetooth/Kconfig: > depends on USB || !BT_HCIBTUSB_MTK > depends on USB if BT_HCIBTUSB_MTK > mm/Kconfig: > depends on !ARM || CPU_CACHE_VIPT --> > depends on CPU_CACHE_VIPT if ARM > kernel/Kconfig.locks: > depends on !PREEMPTION || ARCH_INLINE_READ_UNLOCK --> > depends on ARCH_INLINE_READ_UNLOCK if PREEMPTION On the surface, the vast majority these become more readable using the 'if' syntax. > The earlier patch discussion ended without a real conclusion and should > be revisited now. > --- > Changes in v2: > - EDITME: describe what is new in this series revision. > - EDITME: use bulletpoints and terse descriptions. > - Link to v1: https://lore.kernel.org/r/20251107-kconfig_conditional_deps-v1-1-aff22199ec0b@qti.qualcomm.com > --- > Documentation/kbuild/kconfig-language.rst | 22 +++++++++++++++--- > scripts/kconfig/lkc.h | 2 +- > scripts/kconfig/menu.c | 12 +++++++++- > scripts/kconfig/parser.y | 6 ++--- > scripts/kconfig/tests/conditional_dep/Kconfig | 27 ++++++++++++++++++++++ > scripts/kconfig/tests/conditional_dep/__init__.py | 14 +++++++++++ > .../kconfig/tests/conditional_dep/expected_config1 | 10 ++++++++ > .../kconfig/tests/conditional_dep/expected_config2 | 8 +++++++ > .../kconfig/tests/conditional_dep/expected_config3 | 10 ++++++++ > scripts/kconfig/tests/conditional_dep/test_config1 | 4 ++++ > scripts/kconfig/tests/conditional_dep/test_config2 | 7 ++++++ > scripts/kconfig/tests/conditional_dep/test_config3 | 6 +++++ > 12 files changed, 120 insertions(+), 8 deletions(-) > > diff --git a/Documentation/kbuild/kconfig-language.rst b/Documentation/kbuild/kconfig-language.rst > index abce88f15d7c..9ff3e530b2b4 100644 > --- a/Documentation/kbuild/kconfig-language.rst > +++ b/Documentation/kbuild/kconfig-language.rst > @@ -118,7 +118,7 @@ applicable everywhere (see syntax). > This is a shorthand notation for a type definition plus a value. > Optionally dependencies for this default value can be added with "if". > > -- dependencies: "depends on" <expr> > +- dependencies: "depends on" <expr> ["if" <expr>] > > This defines a dependency for this menu entry. If multiple > dependencies are defined, they are connected with '&&'. Dependencies > @@ -134,6 +134,16 @@ applicable everywhere (see syntax). > bool "foo" > default y > > + The dependency definition itself may be conditional by appending "if" > + followed by an expression. For example:: > + > + config FOO > + tristate > + depends on BAR if BAZ > + > + meaning that FOO is constrained by the value of BAR only if BAZ is > + also set. > + > - reverse dependencies: "select" <symbol> ["if" <expr>] > > While normal dependencies reduce the upper limit of a symbol (see > @@ -602,8 +612,14 @@ Some drivers are able to optionally use a feature from another module > or build cleanly with that module disabled, but cause a link failure > when trying to use that loadable module from a built-in driver. > > -The most common way to express this optional dependency in Kconfig logic > -uses the slightly counterintuitive:: > +The recommended way to express this optional dependency in Kconfig logic > +uses the conditional form:: > + > + config FOO > + tristate "Support for foo hardware" > + depends on BAR if BAR > + > +This slightly counterintuitive style is also widely used:: > > config FOO > tristate "Support for foo hardware" > diff --git a/scripts/kconfig/lkc.h b/scripts/kconfig/lkc.h > index 56548efc14d7..798985961215 100644 > --- a/scripts/kconfig/lkc.h > +++ b/scripts/kconfig/lkc.h > @@ -82,7 +82,7 @@ void menu_warn(const struct menu *menu, const char *fmt, ...); > struct menu *menu_add_menu(void); > void menu_end_menu(void); > void menu_add_entry(struct symbol *sym, enum menu_type type); > -void menu_add_dep(struct expr *dep); > +void menu_add_dep(struct expr *dep, struct expr *cond); > void menu_add_visibility(struct expr *dep); > struct property *menu_add_prompt(enum prop_type type, const char *prompt, > struct expr *dep); > diff --git a/scripts/kconfig/menu.c b/scripts/kconfig/menu.c > index 0f1a6513987c..b2d8d4e11e07 100644 > --- a/scripts/kconfig/menu.c > +++ b/scripts/kconfig/menu.c > @@ -127,8 +127,18 @@ static struct expr *rewrite_m(struct expr *e) > return e; > } > > -void menu_add_dep(struct expr *dep) > +void menu_add_dep(struct expr *dep, struct expr *cond) > { > + if (cond) { > + /* > + * We have "depends on X if Y" and we want: > + * Y != n --> X > + * Y == n --> y > + * That simplifies to: (X || (Y == n)) > + */ > + dep = expr_alloc_or(dep, > + expr_trans_compare(cond, E_EQUAL, &symbol_no)); > + } > current_entry->dep = expr_alloc_and(current_entry->dep, dep); > } > > diff --git a/scripts/kconfig/parser.y b/scripts/kconfig/parser.y > index 49b79dde1725..6d1bbee38f5d 100644 > --- a/scripts/kconfig/parser.y > +++ b/scripts/kconfig/parser.y > @@ -323,7 +323,7 @@ if_entry: T_IF expr T_EOL > { > printd(DEBUG_PARSE, "%s:%d:if\n", cur_filename, cur_lineno); > menu_add_entry(NULL, M_IF); > - menu_add_dep($2); > + menu_add_dep($2, NULL); > $$ = menu_add_menu(); > }; > > @@ -422,9 +422,9 @@ help: help_start T_HELPTEXT > > /* depends option */ > > -depends: T_DEPENDS T_ON expr T_EOL > +depends: T_DEPENDS T_ON expr if_expr T_EOL > { > - menu_add_dep($3); > + menu_add_dep($3, $4); > printd(DEBUG_PARSE, "%s:%d:depends on\n", cur_filename, cur_lineno); > }; > > diff --git a/scripts/kconfig/tests/conditional_dep/Kconfig b/scripts/kconfig/tests/conditional_dep/Kconfig > new file mode 100644 > index 000000000000..ea2bdef9016c > --- /dev/null > +++ b/scripts/kconfig/tests/conditional_dep/Kconfig > @@ -0,0 +1,27 @@ > +# SPDX-License-Identifier: GPL-2.0 > +# Test Kconfig file for conditional dependencies. > + > +config FOO > + bool "FOO symbol" > + > +config BAR > + bool "BAR symbol" > + > +config TEST_BASIC > + bool "Test basic conditional dependency" > + depends on FOO if BAR > + default y > + > +config TEST_COMPLEX > + bool "Test complex conditional dependency" > + depends on (FOO && BAR) if (FOO || BAR) > + default y > + > +config BAZ > + tristate "BAZ symbol" > + > +config TEST_OPTIONAL > + tristate "Test simple optional dependency" > + depends on BAZ if BAZ > + default y These tristates do not work properly because the "modules" keyword is not present. Copying the MODULES config from the transitional test fixes this. diff --git a/scripts/kconfig/tests/conditional_dep/Kconfig b/scripts/kconfig/tests/conditional_dep/Kconfig index ab5e3cf7824f..8f762e9c4d38 100644 --- a/scripts/kconfig/tests/conditional_dep/Kconfig +++ b/scripts/kconfig/tests/conditional_dep/Kconfig @@ -1,6 +1,12 @@ # SPDX-License-Identifier: GPL-2.0 # Test Kconfig file for conditional dependencies. +# Enable module support for tristate testing +config MODULES + bool "Enable loadable module support" + modules + default y + config FOO bool "FOO symbol" > diff --git a/scripts/kconfig/tests/conditional_dep/__init__.py b/scripts/kconfig/tests/conditional_dep/__init__.py > new file mode 100644 > index 000000000000..ab16df6487ec > --- /dev/null > +++ b/scripts/kconfig/tests/conditional_dep/__init__.py > @@ -0,0 +1,14 @@ > +# SPDX-License-Identifier: GPL-2.0 > +""" > +Correctly handle conditional dependencies. > +""" > + > +def test(conf): > + assert conf.oldconfig('test_config1') == 0 > + assert conf.config_matches('expected_config1') > + > + assert conf.oldconfig('test_config2') == 0 > + assert conf.config_matches('expected_config2') > + > + assert conf.oldconfig('test_config3') == 0 > + assert conf.config_matches('expected_config3') > diff --git a/scripts/kconfig/tests/conditional_dep/expected_config1 b/scripts/kconfig/tests/conditional_dep/expected_config1 > new file mode 100644 > index 000000000000..2ad02aa66b06 > --- /dev/null > +++ b/scripts/kconfig/tests/conditional_dep/expected_config1 > @@ -0,0 +1,10 @@ > +# > +# Automatically generated file; DO NOT EDIT. > +# Main menu > +# > +CONFIG_FOO=y > +CONFIG_BAR=y > +CONFIG_TEST_BASIC=y > +CONFIG_TEST_COMPLEX=y > +CONFIG_BAZ=y > +CONFIG_TEST_OPTIONAL=y Which will result in the following change here, which looks correct to me. diff --git a/scripts/kconfig/tests/conditional_dep/expected_config1 b/scripts/kconfig/tests/conditional_dep/expected_config1 index 3198a6d09d9d..826ed7f541b8 100644 --- a/scripts/kconfig/tests/conditional_dep/expected_config1 +++ b/scripts/kconfig/tests/conditional_dep/expected_config1 @@ -2,9 +2,10 @@ # Automatically generated file; DO NOT EDIT. # Main menu # +CONFIG_MODULES=y CONFIG_FOO=y CONFIG_BAR=y CONFIG_TEST_BASIC=y CONFIG_TEST_COMPLEX=y -CONFIG_BAZ=y -CONFIG_TEST_OPTIONAL=y +CONFIG_BAZ=m +CONFIG_TEST_OPTIONAL=m > diff --git a/scripts/kconfig/tests/conditional_dep/expected_config2 b/scripts/kconfig/tests/conditional_dep/expected_config2 > new file mode 100644 > index 000000000000..b4b19cf50730 > --- /dev/null > +++ b/scripts/kconfig/tests/conditional_dep/expected_config2 > @@ -0,0 +1,8 @@ > +# > +# Automatically generated file; DO NOT EDIT. > +# Main menu > +# > +# CONFIG_FOO is not set > +CONFIG_BAR=y > +CONFIG_BAZ=y > +CONFIG_TEST_OPTIONAL=y > diff --git a/scripts/kconfig/tests/conditional_dep/expected_config3 b/scripts/kconfig/tests/conditional_dep/expected_config3 > new file mode 100644 > index 000000000000..c788f6c710e1 > --- /dev/null > +++ b/scripts/kconfig/tests/conditional_dep/expected_config3 > @@ -0,0 +1,10 @@ > +# > +# Automatically generated file; DO NOT EDIT. > +# Main menu > +# > +# CONFIG_FOO is not set > +# CONFIG_BAR is not set > +CONFIG_TEST_BASIC=y > +CONFIG_TEST_COMPLEX=y > +# CONFIG_BAZ is not set > +CONFIG_TEST_OPTIONAL=y > diff --git a/scripts/kconfig/tests/conditional_dep/test_config1 b/scripts/kconfig/tests/conditional_dep/test_config1 > new file mode 100644 > index 000000000000..5cc1ecedcba3 > --- /dev/null > +++ b/scripts/kconfig/tests/conditional_dep/test_config1 > @@ -0,0 +1,4 @@ > +# Basic check that everything can be configured if selected. > +CONFIG_FOO=y > +CONFIG_BAR=y > +CONFIG_BAZ=m It might be worth adding something like # Ensure that TEST_OPTIONAL=y with BAZ=m is converted to TEST_OPTIONAL=m CONFIG_TEST_OPTIONAL=y to the end here to ensure the new logic works the same as the old logic. It might be worth checking that TEST_OPTIONAL=m works with BAZ=n but not sure. > diff --git a/scripts/kconfig/tests/conditional_dep/test_config2 b/scripts/kconfig/tests/conditional_dep/test_config2 > new file mode 100644 > index 000000000000..1175c5307308 > --- /dev/null > +++ b/scripts/kconfig/tests/conditional_dep/test_config2 > @@ -0,0 +1,7 @@ > +# If FOO is not selected, then TEST_BASIC should fail the conditional > +# dependency since BAR is set. > +# TEST_COMPLEX will fail dependency as it depends on both FOO and BAR > +# if either of those is selected. > +CONFIG_FOO=n > +CONFIG_BAR=y > +CONFIG_BAZ=y > diff --git a/scripts/kconfig/tests/conditional_dep/test_config3 b/scripts/kconfig/tests/conditional_dep/test_config3 > new file mode 100644 > index 000000000000..3815ad744e89 > --- /dev/null > +++ b/scripts/kconfig/tests/conditional_dep/test_config3 > @@ -0,0 +1,6 @@ > +# If FOO is not selected, but BAR is also not selected, then TEST_BASIC > +# should pass since the dependency on FOO is conditional on BAR. > +# TEST_COMPLEX should be also set since neither FOO nor BAR are selected > +# so it has no dependencies. > +CONFIG_FOO=n > +CONFIG_BAR=n Other than that, this seems reasonable to me. The actual code changes are small and the tests prove this works properly. I won't pick up v3 until after 6.19-rc1 is out at the least. Cheers, Nathan ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v2] Support conditional deps using "depends on X if Y" 2025-12-05 1:53 ` Nathan Chancellor @ 2025-12-05 8:01 ` Arnd Bergmann 2025-12-05 16:14 ` Nicolas Pitre 2025-12-05 18:23 ` Nathan Chancellor 0 siblings, 2 replies; 9+ messages in thread From: Arnd Bergmann @ 2025-12-05 8:01 UTC (permalink / raw) To: Nathan Chancellor, Graham Roff Cc: Nicolas Schier, Jonathan Corbet, linux-kbuild, linux-doc, linux-kernel, Nicolas Pitre On Fri, Dec 5, 2025, at 02:53, Nathan Chancellor wrote: > On Tue, Nov 18, 2025 at 10:46:51AM -0800, Graham Roff wrote: >> >> arch/arm64/Kconfig: >> depends on ARM64_64K_PAGES || !ARM64_VA_BITS_52 --> >> depends on ARM64_64K_PAGES if ARM64_VA_BITS_52 >> arch/mips/Kconfig: >> depends on SYS_SUPPORTS_HOTPLUG_CPU || !SMP --> >> depends on SYS_SUPPORTS_HOTPLUG_CPU if SMP >> arch/riscv/Kconfig: >> depends on CC_HAS_MIN_FUNCTION_ALIGNMENT || !RISCV_ISA_C --> >> depends on CC_HAS_MIN_FUNCTION_ALIGNMENT if RISCV_ISA_C >> arch/x86/Kconfig: >> depends on X86_64 || !SPARSEMEM --> >> depends on X86_64 if SPARSEMEM >> drivers/acpi/Kconfig: >> depends on ACPI_WMI || !X86 --> >> depends on ACPI_WMI if X86 >> drivers/bluetooth/Kconfig: >> depends on USB || !BT_HCIBTUSB_MTK >> depends on USB if BT_HCIBTUSB_MTK >> mm/Kconfig: >> depends on !ARM || CPU_CACHE_VIPT --> >> depends on CPU_CACHE_VIPT if ARM >> kernel/Kconfig.locks: >> depends on !PREEMPTION || ARCH_INLINE_READ_UNLOCK --> >> depends on ARCH_INLINE_READ_UNLOCK if PREEMPTION > > On the surface, the vast majority these become more readable using the > 'if' syntax. Agreed, the question is whether a small improvement in readability is worth the complexity of having multiple ways of expressing the same thing. I don't see anything that the new syntax would allow that we were currently missing. >> @@ -602,8 +612,14 @@ Some drivers are able to optionally use a feature from another module >> or build cleanly with that module disabled, but cause a link failure >> when trying to use that loadable module from a built-in driver. >> >> -The most common way to express this optional dependency in Kconfig logic >> -uses the slightly counterintuitive:: >> +The recommended way to express this optional dependency in Kconfig logic >> +uses the conditional form:: >> + >> + config FOO >> + tristate "Support for foo hardware" >> + depends on BAR if BAR >> + >> +This slightly counterintuitive style is also widely used:: >> This is the bit that frequently confuses developers with the current syntax, and I agree it would be nice to have a better way, but I'm not sure the proposal actually helps enough to warrant a mass-conversion of existing Kconfig files. >> +config TEST_COMPLEX >> + bool "Test complex conditional dependency" >> + depends on (FOO && BAR) if (FOO || BAR) >> + default y With the existing syntax, this could be expressed as depends on FOO = BAR or depends on (FOO && BAR) || (!FOO && !BAR) and I don't see how the new syntax is an improvement over these. Overall, I'm not convinced by this patch. I have no strong objection to anything in here, but I'm worried that extending the syntax adds more problems than this one solves. Arnd ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2] Support conditional deps using "depends on X if Y" 2025-12-05 8:01 ` Arnd Bergmann @ 2025-12-05 16:14 ` Nicolas Pitre 2025-12-05 18:23 ` Nathan Chancellor 1 sibling, 0 replies; 9+ messages in thread From: Nicolas Pitre @ 2025-12-05 16:14 UTC (permalink / raw) To: Arnd Bergmann Cc: Nathan Chancellor, Graham Roff, Nicolas Schier, Jonathan Corbet, linux-kbuild, linux-doc, linux-kernel On Fri, 5 Dec 2025, Arnd Bergmann wrote: > On Fri, Dec 5, 2025, at 02:53, Nathan Chancellor wrote: > > On Tue, Nov 18, 2025 at 10:46:51AM -0800, Graham Roff wrote: > >> > >> arch/arm64/Kconfig: > >> depends on ARM64_64K_PAGES || !ARM64_VA_BITS_52 --> > >> depends on ARM64_64K_PAGES if ARM64_VA_BITS_52 > >> arch/mips/Kconfig: > >> depends on SYS_SUPPORTS_HOTPLUG_CPU || !SMP --> > >> depends on SYS_SUPPORTS_HOTPLUG_CPU if SMP > >> arch/riscv/Kconfig: > >> depends on CC_HAS_MIN_FUNCTION_ALIGNMENT || !RISCV_ISA_C --> > >> depends on CC_HAS_MIN_FUNCTION_ALIGNMENT if RISCV_ISA_C > >> arch/x86/Kconfig: > >> depends on X86_64 || !SPARSEMEM --> > >> depends on X86_64 if SPARSEMEM > >> drivers/acpi/Kconfig: > >> depends on ACPI_WMI || !X86 --> > >> depends on ACPI_WMI if X86 > >> drivers/bluetooth/Kconfig: > >> depends on USB || !BT_HCIBTUSB_MTK > >> depends on USB if BT_HCIBTUSB_MTK > >> mm/Kconfig: > >> depends on !ARM || CPU_CACHE_VIPT --> > >> depends on CPU_CACHE_VIPT if ARM > >> kernel/Kconfig.locks: > >> depends on !PREEMPTION || ARCH_INLINE_READ_UNLOCK --> > >> depends on ARCH_INLINE_READ_UNLOCK if PREEMPTION > > > > On the surface, the vast majority these become more readable using the > > 'if' syntax. > > Agreed, the question is whether a small improvement in > readability is worth the complexity of having multiple > ways of expressing the same thing. It is a tradeoff. Sometimes it is advantageous to increase the complexity in one place so other areas with more exposure to more people are simplified. Nicolas ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2] Support conditional deps using "depends on X if Y" 2025-12-05 8:01 ` Arnd Bergmann 2025-12-05 16:14 ` Nicolas Pitre @ 2025-12-05 18:23 ` Nathan Chancellor 2025-12-09 22:45 ` Graham Roff 2025-12-10 15:07 ` Jani Nikula 1 sibling, 2 replies; 9+ messages in thread From: Nathan Chancellor @ 2025-12-05 18:23 UTC (permalink / raw) To: Arnd Bergmann Cc: Graham Roff, Nicolas Schier, Jonathan Corbet, linux-kbuild, linux-doc, linux-kernel, Nicolas Pitre On Fri, Dec 05, 2025 at 09:01:51AM +0100, Arnd Bergmann wrote: > Agreed, the question is whether a small improvement in > readability is worth the complexity of having multiple > ways of expressing the same thing. I think the biggest thing that this patch has going for this is that there is minimal additional complexity within scripts/kconfig and that it basically internally converts the 'depends on ... if ...' into the simple 'depends on' so there is no behavioral difference. The diff stat of the core of the change speaks to that I think. scripts/kconfig/lkc.h | 2 +- scripts/kconfig/menu.c | 12 +++++++++++- scripts/kconfig/parser.y | 6 +++--- 3 files changed, 15 insertions(+), 5 deletions(-) > I don't see anything that the new syntax would allow > that we were currently missing. I see this as syntactic sugar. It is just giving users a different (and possibly more intuitive) way of expressing the same thing but I understand being concerned about people misusing it (even though I think it is already hard enough to get dependencies right sometimes). > This is the bit that frequently confuses developers with the > current syntax, and I agree it would be nice to have a better > way, but I'm not sure the proposal actually helps enough to > warrant a mass-conversion of existing Kconfig files. I do agree that the 'depends on A || !A' syntax is confusing and that this does not really address that but I think that is besides the point here. I also agree that it is probably not worth converting existing users to this syntax (unless there is solid reasoning), I would not want to see cleanup patches of that nature, just use in new code. > With the existing syntax, this could be expressed as > > depends on FOO = BAR > > or > > depends on (FOO && BAR) || (!FOO && !BAR) > > and I don't see how the new syntax is an improvement > over these. Maybe the "if" syntax could be easier to understand with actual real world values? I cannot think of anything off the top of my head but real world dependencies might read a bit more naturally with this syntax. > Overall, I'm not convinced by this patch. I have no strong > objection to anything in here, but I'm worried that extending > the syntax adds more problems than this one solves. Thanks a lot for the input! Cheers, Nathan ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2] Support conditional deps using "depends on X if Y" 2025-12-05 18:23 ` Nathan Chancellor @ 2025-12-09 22:45 ` Graham Roff 2025-12-10 15:07 ` Jani Nikula 1 sibling, 0 replies; 9+ messages in thread From: Graham Roff @ 2025-12-09 22:45 UTC (permalink / raw) To: nathan Cc: arnd, corbet, grahamr, linux-doc, linux-kbuild, linux-kernel, nico, nsc > Other than that, this seems reasonable to me. The actual code changes > are small and the tests prove this works properly. I won't pick up v3 > until after 6.19-rc1 is out at the least. Thanks Nathan! I will send out a v3 with the commit text updated, and the test code changes you suggested. > > > On the surface, the vast majority these become more readable using the > > > 'if' syntax. > > > > Agreed, the question is whether a small improvement in > > readability is worth the complexity of having multiple > > ways of expressing the same thing. > > It is a tradeoff. Sometimes it is advantageous to increase the > complexity in one place so other areas with more exposure to more people > are simplified. Exactly, a small code update here provides a much simpler syntax for expressing a fairly common thing (conditional or optional dependencies). It also makes the language more consistent since most other kconfig commands accept a trailing "if <expr>" - I just assumed that "depends on" did as well when first writing Kconfig files! > >> +config TEST_COMPLEX > >> + bool "Test complex conditional dependency" > >> + depends on (FOO && BAR) if (FOO || BAR) > >> + default y > > With the existing syntax, this could be expressed as > > depends on FOO = BAR > > or > > depends on (FOO && BAR) || (!FOO && !BAR) > > and I don't see how the new syntax is an improvement > over these. In this case I agree, the test was chosen to validate a complicated conditional, not to show a particularly useful real-world example. I added much better examples earlier of where the "if" style provides improved readability. > Overall, I'm not convinced by this patch. I have no strong > objection to anything in here, but I'm worried that extending > the syntax adds more problems than this one solves. Both syntaxes have their places, it comes down to which one is most understandable to the reader. In a lot of cases it is definitely easier to understand the intent of the expression using "if" rather than ors and nots. Combine that with the simplicity of the change and any problems this might add (not sure what those are though) would seem on balance to be worth it. Graham ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2] Support conditional deps using "depends on X if Y" 2025-12-05 18:23 ` Nathan Chancellor 2025-12-09 22:45 ` Graham Roff @ 2025-12-10 15:07 ` Jani Nikula 2025-12-13 1:40 ` Nathan Chancellor 1 sibling, 1 reply; 9+ messages in thread From: Jani Nikula @ 2025-12-10 15:07 UTC (permalink / raw) To: Nathan Chancellor, Arnd Bergmann Cc: Graham Roff, Nicolas Schier, Jonathan Corbet, linux-kbuild, linux-doc, linux-kernel, Nicolas Pitre On Fri, 05 Dec 2025, Nathan Chancellor <nathan@kernel.org> wrote: > On Fri, Dec 05, 2025 at 09:01:51AM +0100, Arnd Bergmann wrote: >> This is the bit that frequently confuses developers with the >> current syntax, and I agree it would be nice to have a better >> way, but I'm not sure the proposal actually helps enough to >> warrant a mass-conversion of existing Kconfig files. > > I do agree that the 'depends on A || !A' syntax is confusing and that > this does not really address that but I think that is besides the point > here. I also agree that it is probably not worth converting existing > users to this syntax (unless there is solid reasoning), I would not want > to see cleanup patches of that nature, just use in new code. I think "depends on A if A" is an improvement over "A || !A". But not a drastic improvement. I think the question is, can we figure out an even better syntax for that use case? Something that conveys the "optionally depends on A" meaning? Is there something so good that it would warrant cleanup conversions just for the improved clarity? If we can't come up with anything, let's just roll with what we have here? BR, Jani. -- Jani Nikula, Intel ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2] Support conditional deps using "depends on X if Y" 2025-12-10 15:07 ` Jani Nikula @ 2025-12-13 1:40 ` Nathan Chancellor 0 siblings, 0 replies; 9+ messages in thread From: Nathan Chancellor @ 2025-12-13 1:40 UTC (permalink / raw) To: Jani Nikula Cc: Arnd Bergmann, Graham Roff, Nicolas Schier, Jonathan Corbet, linux-kbuild, linux-doc, linux-kernel, Nicolas Pitre On Wed, Dec 10, 2025 at 05:07:39PM +0200, Jani Nikula wrote: > On Fri, 05 Dec 2025, Nathan Chancellor <nathan@kernel.org> wrote: > > On Fri, Dec 05, 2025 at 09:01:51AM +0100, Arnd Bergmann wrote: > >> This is the bit that frequently confuses developers with the > >> current syntax, and I agree it would be nice to have a better > >> way, but I'm not sure the proposal actually helps enough to > >> warrant a mass-conversion of existing Kconfig files. > > > > I do agree that the 'depends on A || !A' syntax is confusing and that > > this does not really address that but I think that is besides the point > > here. I also agree that it is probably not worth converting existing > > users to this syntax (unless there is solid reasoning), I would not want > > to see cleanup patches of that nature, just use in new code. > > I think "depends on A if A" is an improvement over "A || !A". But not a > drastic improvement. Agreed. > I think the question is, can we figure out an even better syntax for > that use case? Something that conveys the "optionally depends on A" > meaning? Is there something so good that it would warrant cleanup > conversions just for the improved clarity? I cannot think of anything off the top of my head but given how new I am to actually maintaining Kconfig, maybe something else will come up over time (or maybe Nicolas has some thoughts). > If we can't come up with anything, let's just roll with what we have > here? This is my plan personally, as I feel like this (or the future v3) is good enough (and brings consistency to "depends" with regards to supporting "if" like "prompt" and "select" do). I am a firm believer in "don't let perfect be the enemy of good". Cheers, Nathan ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2] Support conditional deps using "depends on X if Y" 2025-11-18 18:46 [PATCH v2] Support conditional deps using "depends on X if Y" Graham Roff 2025-12-05 1:53 ` Nathan Chancellor @ 2025-12-05 9:06 ` Jani Nikula 1 sibling, 0 replies; 9+ messages in thread From: Jani Nikula @ 2025-12-05 9:06 UTC (permalink / raw) To: Graham Roff, Nathan Chancellor, Nicolas Schier, Jonathan Corbet Cc: linux-kbuild, linux-doc, linux-kernel, Nicolas Pitre, Graham Roff On Tue, 18 Nov 2025, Graham Roff <grahamr@qti.qualcomm.com> wrote: > @@ -602,8 +612,14 @@ Some drivers are able to optionally use a feature from another module > or build cleanly with that module disabled, but cause a link failure > when trying to use that loadable module from a built-in driver. > > -The most common way to express this optional dependency in Kconfig logic > -uses the slightly counterintuitive:: > +The recommended way to express this optional dependency in Kconfig logic > +uses the conditional form:: > + > + config FOO > + tristate "Support for foo hardware" > + depends on BAR if BAR > + > +This slightly counterintuitive style is also widely used:: > > config FOO > tristate "Support for foo hardware" Thanks for adding this documentation hunk. BR, Jani. -- Jani Nikula, Intel ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2025-12-13 1:40 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-11-18 18:46 [PATCH v2] Support conditional deps using "depends on X if Y" Graham Roff 2025-12-05 1:53 ` Nathan Chancellor 2025-12-05 8:01 ` Arnd Bergmann 2025-12-05 16:14 ` Nicolas Pitre 2025-12-05 18:23 ` Nathan Chancellor 2025-12-09 22:45 ` Graham Roff 2025-12-10 15:07 ` Jani Nikula 2025-12-13 1:40 ` Nathan Chancellor 2025-12-05 9:06 ` Jani Nikula
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).