Due to the changes to the semantics of imply keyword [1], which now
doesn't force any config options to the implied configs any more.
A module (FOO) that has a weak dependency on some other modules (BAR)
is now broken if it was using imply to force dependency restrictions.
e.g.: FOO needs BAR to be reachable, especially when FOO=y and BAR=m.
Which might now introduce build/link errors.
There are two options to solve this:
1. use IS_REACHABLE(BAR), everywhere BAR is referenced inside FOO.
2. in FOO's Kconfig add: depends on (BAR || !BAR)
The first option is not desirable, and will leave the user confused when
setting FOO=y and BAR=m, FOO will never reach BAR even though both are
compiled.
The 2nd one is the preferred approach, and will guarantee BAR is always
reachable by FOO if both are compiled. But, (BAR || !BAR) is really
confusing for those who don't really get how kconfig tristate arithmetics
work.
To solve this and hide this weird expression and to avoid repetition
across the tree, we introduce new keyword "uses" to the Kconfig options
family.
uses BAR:
Equivalent to: depends on symbol || !symbol
Semantically it means, if FOO is enabled (y/m) and has the option:
uses BAR, make sure it can reach/use BAR when possible.
For example: if FOO=y and BAR=m, FOO will be forced to m.
[1] https://lore.kernel.org/linux-doc/[email protected]/
Link: https://lkml.org/lkml/2020/4/8/839
Signed-off-by: Saeed Mahameed <[email protected]>
Cc: Masahiro Yamada <[email protected]>
Cc: [email protected]
Cc: [email protected]
---
Documentation/kbuild/kconfig-language.rst | 10 ++++++++++
scripts/kconfig/expr.h | 1 +
scripts/kconfig/lexer.l | 1 +
scripts/kconfig/menu.c | 4 +++-
scripts/kconfig/parser.y | 15 +++++++++++++++
scripts/kconfig/symbol.c | 2 ++
6 files changed, 32 insertions(+), 1 deletion(-)
diff --git a/Documentation/kbuild/kconfig-language.rst b/Documentation/kbuild/kconfig-language.rst
index a1601ec3317b..8db8c2d80794 100644
--- a/Documentation/kbuild/kconfig-language.rst
+++ b/Documentation/kbuild/kconfig-language.rst
@@ -130,6 +130,16 @@ applicable everywhere (see syntax).
bool "foo"
default y
+- uses dependencies: "uses" <symbol>
+
+ Equivalent to: depends on symbol || !symbol
+ Semantically it means, if FOO is enabled (y/m) and has the option:
+ uses BAR, make sure it can reach/use BAR when possible.
+ For example: if FOO=y and BAR=m, FOO will be forced to m.
+
+ Note:
+ To understand how (symbol || !symbol) is actually computed, please see `Menu dependencies`_
+
- reverse dependencies: "select" <symbol> ["if" <expr>]
While normal dependencies reduce the upper limit of a symbol (see
diff --git a/scripts/kconfig/expr.h b/scripts/kconfig/expr.h
index 5c3443692f34..face672fb4b4 100644
--- a/scripts/kconfig/expr.h
+++ b/scripts/kconfig/expr.h
@@ -185,6 +185,7 @@ enum prop_type {
P_CHOICE, /* choice value */
P_SELECT, /* select BAR */
P_IMPLY, /* imply BAR */
+ P_USES, /* uses BAR */
P_RANGE, /* range 7..100 (for a symbol) */
P_SYMBOL, /* where a symbol is defined */
};
diff --git a/scripts/kconfig/lexer.l b/scripts/kconfig/lexer.l
index 6354c905b006..c6a0017b10d4 100644
--- a/scripts/kconfig/lexer.l
+++ b/scripts/kconfig/lexer.l
@@ -102,6 +102,7 @@ n [A-Za-z0-9_-]
"default" return T_DEFAULT;
"defconfig_list" return T_DEFCONFIG_LIST;
"depends" return T_DEPENDS;
+"uses" return T_USES;
"endchoice" return T_ENDCHOICE;
"endif" return T_ENDIF;
"endmenu" return T_ENDMENU;
diff --git a/scripts/kconfig/menu.c b/scripts/kconfig/menu.c
index e436ba44c9c5..e26161b31a11 100644
--- a/scripts/kconfig/menu.c
+++ b/scripts/kconfig/menu.c
@@ -274,7 +274,9 @@ static void sym_check_prop(struct symbol *sym)
break;
case P_SELECT:
case P_IMPLY:
- use = prop->type == P_SELECT ? "select" : "imply";
+ case P_USES:
+ use = prop->type == P_SELECT ? "select" :
+ prop->type == P_IMPLY ? "imply" : "uses";
sym2 = prop_get_symbol(prop);
if (sym->type != S_BOOLEAN && sym->type != S_TRISTATE)
prop_warn(prop,
diff --git a/scripts/kconfig/parser.y b/scripts/kconfig/parser.y
index 708b6c4b13ca..c5e9abb49d29 100644
--- a/scripts/kconfig/parser.y
+++ b/scripts/kconfig/parser.y
@@ -57,6 +57,7 @@ static struct menu *current_menu, *current_entry;
%token T_DEF_BOOL
%token T_DEF_TRISTATE
%token T_DEPENDS
+%token T_USES
%token T_ENDCHOICE
%token T_ENDIF
%token T_ENDMENU
@@ -169,6 +170,7 @@ config_option_list:
/* empty */
| config_option_list config_option
| config_option_list depends
+ | config_option_list uses
| config_option_list help
;
@@ -261,6 +263,7 @@ choice_option_list:
/* empty */
| choice_option_list choice_option
| choice_option_list depends
+ | choice_option_list uses
| choice_option_list help
;
@@ -360,6 +363,7 @@ menu_option_list:
/* empty */
| menu_option_list visible
| menu_option_list depends
+ | menu_option_list uses
;
source_stmt: T_SOURCE T_WORD_QUOTE T_EOL
@@ -384,6 +388,7 @@ comment_stmt: comment comment_option_list
comment_option_list:
/* empty */
| comment_option_list depends
+ | comment_option_list uses
;
/* help option */
@@ -418,6 +423,16 @@ depends: T_DEPENDS T_ON expr T_EOL
printd(DEBUG_PARSE, "%s:%d:depends on\n", zconf_curname(), zconf_lineno());
};
+/* uses symbol: depends on symbol || !symbol */
+uses: T_USES symbol T_EOL
+{
+ struct expr *symexpr = expr_alloc_symbol($2);
+
+ menu_add_dep(expr_alloc_two(E_OR, symexpr, expr_alloc_one(E_NOT, symexpr)));
+ printd(DEBUG_PARSE, "%s:%d: uses: depends on %s || ! %s\n",
+ zconf_curname(), zconf_lineno(), $2->name, $2->name);
+};
+
/* visibility option */
visible: T_VISIBLE if_expr T_EOL
{
diff --git a/scripts/kconfig/symbol.c b/scripts/kconfig/symbol.c
index 3dc81397d003..422f7ea47722 100644
--- a/scripts/kconfig/symbol.c
+++ b/scripts/kconfig/symbol.c
@@ -1295,6 +1295,8 @@ const char *prop_get_type_name(enum prop_type type)
return "choice";
case P_SELECT:
return "select";
+ case P_USES:
+ return "uses";
case P_IMPLY:
return "imply";
case P_RANGE:
--
2.25.2
mlx5 uses the imply keyword to force weak dependencies on the implied
modules, to make sure they are always reachable by mlx5.
"imply" doesn't work this way any more, and it was wrong to use it in
first place.
The right solution is to use: "depends on symbol || !symbol", which is
exactly what the uses keyword is there for.
Use the new "uses" keyword to replace all mlx5 implied dependencies.
Signed-off-by: Saeed Mahameed <[email protected]>
Cc: Masahiro Yamada <[email protected]>
Cc: [email protected]
Cc: [email protected]
---
drivers/net/ethernet/mellanox/mlx5/core/Kconfig | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/Kconfig b/drivers/net/ethernet/mellanox/mlx5/core/Kconfig
index 312e0a1ad43d..4e07179217e8 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/Kconfig
+++ b/drivers/net/ethernet/mellanox/mlx5/core/Kconfig
@@ -7,10 +7,10 @@ config MLX5_CORE
tristate "Mellanox 5th generation network adapters (ConnectX series) core driver"
depends on PCI
select NET_DEVLINK
- imply PTP_1588_CLOCK
- imply VXLAN
- imply MLXFW
- imply PCI_HYPERV_INTERFACE
+ uses PTP_1588_CLOCK
+ uses VXLAN
+ uses MLXFW
+ uses PCI_HYPERV_INTERFACE
default n
---help---
Core driver for low level functionality of the ConnectX-4 and
--
2.25.2
On Thu, 16 Apr 2020, Saeed Mahameed <[email protected]> wrote:
> Due to the changes to the semantics of imply keyword [1], which now
> doesn't force any config options to the implied configs any more.
>
> A module (FOO) that has a weak dependency on some other modules (BAR)
> is now broken if it was using imply to force dependency restrictions.
> e.g.: FOO needs BAR to be reachable, especially when FOO=y and BAR=m.
> Which might now introduce build/link errors.
>
> There are two options to solve this:
> 1. use IS_REACHABLE(BAR), everywhere BAR is referenced inside FOO.
> 2. in FOO's Kconfig add: depends on (BAR || !BAR)
>
> The first option is not desirable, and will leave the user confused when
> setting FOO=y and BAR=m, FOO will never reach BAR even though both are
> compiled.
>
> The 2nd one is the preferred approach, and will guarantee BAR is always
> reachable by FOO if both are compiled. But, (BAR || !BAR) is really
> confusing for those who don't really get how kconfig tristate arithmetics
> work.
>
> To solve this and hide this weird expression and to avoid repetition
> across the tree, we introduce new keyword "uses" to the Kconfig options
> family.
>
> uses BAR:
> Equivalent to: depends on symbol || !symbol
> Semantically it means, if FOO is enabled (y/m) and has the option:
> uses BAR, make sure it can reach/use BAR when possible.
>
> For example: if FOO=y and BAR=m, FOO will be forced to m.
Thanks for doing this. I think *something* needs to be done to help
people grasp the "depends on FOO || FOO=n" construct; I've seen many
experienced stumble on this, it's not a rookie mistake.
I suggested "uses" as a keyword, but I'm not hung up on it.
Grepping some Kconfigs a problem I realized with *any* new keyword is
that (FOO || FOO=n) or (FOO || !FOO) is a construct that can be part of
a larger depends on.
For example,
drivers/net/ethernet/broadcom/Kconfig: depends on PCI && (IPV6 || IPV6=n)
Which means that would have to split up to two. Not ideal, but doable. I
did not find any (FOO || FOO=n) || BAR which would not work with a new
keyword.
An alternative approach that I thought of is adding a lower level
expression to tackle this? "FOO=optional" would expand to (FOO || FOO=n)
anywhere. I have no clue how hard this would be to implement.
For example:
depends on FOO=optional
=>
depends on (FOO || FOO=n)
and:
depends on FOO=optional || BAR
=>
depends on (FOO || FOO=n) || BAR
The "optional" keyword is of course open for bikeshedding, but the key
part here I think is that the "depends on" remains, and should be
obvious. And also the =optional ties better to the actual symbol being
depended on.
Thoughts?
BR,
Jani.
>
> [1] https://lore.kernel.org/linux-doc/[email protected]/
>
> Link: https://lkml.org/lkml/2020/4/8/839
> Signed-off-by: Saeed Mahameed <[email protected]>
> Cc: Masahiro Yamada <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> ---
> Documentation/kbuild/kconfig-language.rst | 10 ++++++++++
> scripts/kconfig/expr.h | 1 +
> scripts/kconfig/lexer.l | 1 +
> scripts/kconfig/menu.c | 4 +++-
> scripts/kconfig/parser.y | 15 +++++++++++++++
> scripts/kconfig/symbol.c | 2 ++
> 6 files changed, 32 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/kbuild/kconfig-language.rst b/Documentation/kbuild/kconfig-language.rst
> index a1601ec3317b..8db8c2d80794 100644
> --- a/Documentation/kbuild/kconfig-language.rst
> +++ b/Documentation/kbuild/kconfig-language.rst
> @@ -130,6 +130,16 @@ applicable everywhere (see syntax).
> bool "foo"
> default y
>
> +- uses dependencies: "uses" <symbol>
> +
> + Equivalent to: depends on symbol || !symbol
> + Semantically it means, if FOO is enabled (y/m) and has the option:
> + uses BAR, make sure it can reach/use BAR when possible.
> + For example: if FOO=y and BAR=m, FOO will be forced to m.
> +
> + Note:
> + To understand how (symbol || !symbol) is actually computed, please see `Menu dependencies`_
> +
> - reverse dependencies: "select" <symbol> ["if" <expr>]
>
> While normal dependencies reduce the upper limit of a symbol (see
> diff --git a/scripts/kconfig/expr.h b/scripts/kconfig/expr.h
> index 5c3443692f34..face672fb4b4 100644
> --- a/scripts/kconfig/expr.h
> +++ b/scripts/kconfig/expr.h
> @@ -185,6 +185,7 @@ enum prop_type {
> P_CHOICE, /* choice value */
> P_SELECT, /* select BAR */
> P_IMPLY, /* imply BAR */
> + P_USES, /* uses BAR */
> P_RANGE, /* range 7..100 (for a symbol) */
> P_SYMBOL, /* where a symbol is defined */
> };
> diff --git a/scripts/kconfig/lexer.l b/scripts/kconfig/lexer.l
> index 6354c905b006..c6a0017b10d4 100644
> --- a/scripts/kconfig/lexer.l
> +++ b/scripts/kconfig/lexer.l
> @@ -102,6 +102,7 @@ n [A-Za-z0-9_-]
> "default" return T_DEFAULT;
> "defconfig_list" return T_DEFCONFIG_LIST;
> "depends" return T_DEPENDS;
> +"uses" return T_USES;
> "endchoice" return T_ENDCHOICE;
> "endif" return T_ENDIF;
> "endmenu" return T_ENDMENU;
> diff --git a/scripts/kconfig/menu.c b/scripts/kconfig/menu.c
> index e436ba44c9c5..e26161b31a11 100644
> --- a/scripts/kconfig/menu.c
> +++ b/scripts/kconfig/menu.c
> @@ -274,7 +274,9 @@ static void sym_check_prop(struct symbol *sym)
> break;
> case P_SELECT:
> case P_IMPLY:
> - use = prop->type == P_SELECT ? "select" : "imply";
> + case P_USES:
> + use = prop->type == P_SELECT ? "select" :
> + prop->type == P_IMPLY ? "imply" : "uses";
> sym2 = prop_get_symbol(prop);
> if (sym->type != S_BOOLEAN && sym->type != S_TRISTATE)
> prop_warn(prop,
> diff --git a/scripts/kconfig/parser.y b/scripts/kconfig/parser.y
> index 708b6c4b13ca..c5e9abb49d29 100644
> --- a/scripts/kconfig/parser.y
> +++ b/scripts/kconfig/parser.y
> @@ -57,6 +57,7 @@ static struct menu *current_menu, *current_entry;
> %token T_DEF_BOOL
> %token T_DEF_TRISTATE
> %token T_DEPENDS
> +%token T_USES
> %token T_ENDCHOICE
> %token T_ENDIF
> %token T_ENDMENU
> @@ -169,6 +170,7 @@ config_option_list:
> /* empty */
> | config_option_list config_option
> | config_option_list depends
> + | config_option_list uses
> | config_option_list help
> ;
>
> @@ -261,6 +263,7 @@ choice_option_list:
> /* empty */
> | choice_option_list choice_option
> | choice_option_list depends
> + | choice_option_list uses
> | choice_option_list help
> ;
>
> @@ -360,6 +363,7 @@ menu_option_list:
> /* empty */
> | menu_option_list visible
> | menu_option_list depends
> + | menu_option_list uses
> ;
>
> source_stmt: T_SOURCE T_WORD_QUOTE T_EOL
> @@ -384,6 +388,7 @@ comment_stmt: comment comment_option_list
> comment_option_list:
> /* empty */
> | comment_option_list depends
> + | comment_option_list uses
> ;
>
> /* help option */
> @@ -418,6 +423,16 @@ depends: T_DEPENDS T_ON expr T_EOL
> printd(DEBUG_PARSE, "%s:%d:depends on\n", zconf_curname(), zconf_lineno());
> };
>
> +/* uses symbol: depends on symbol || !symbol */
> +uses: T_USES symbol T_EOL
> +{
> + struct expr *symexpr = expr_alloc_symbol($2);
> +
> + menu_add_dep(expr_alloc_two(E_OR, symexpr, expr_alloc_one(E_NOT, symexpr)));
> + printd(DEBUG_PARSE, "%s:%d: uses: depends on %s || ! %s\n",
> + zconf_curname(), zconf_lineno(), $2->name, $2->name);
> +};
> +
> /* visibility option */
> visible: T_VISIBLE if_expr T_EOL
> {
> diff --git a/scripts/kconfig/symbol.c b/scripts/kconfig/symbol.c
> index 3dc81397d003..422f7ea47722 100644
> --- a/scripts/kconfig/symbol.c
> +++ b/scripts/kconfig/symbol.c
> @@ -1295,6 +1295,8 @@ const char *prop_get_type_name(enum prop_type type)
> return "choice";
> case P_SELECT:
> return "select";
> + case P_USES:
> + return "uses";
> case P_IMPLY:
> return "imply";
> case P_RANGE:
--
Jani Nikula, Intel Open Source Graphics Center
On Fri, Apr 17, 2020 at 3:12 AM Saeed Mahameed <[email protected]> wrote:
> @@ -7,10 +7,10 @@ config MLX5_CORE
> tristate "Mellanox 5th generation network adapters (ConnectX series) core driver"
> depends on PCI
> select NET_DEVLINK
> - imply PTP_1588_CLOCK
> - imply VXLAN
> - imply MLXFW
> - imply PCI_HYPERV_INTERFACE
> + uses PTP_1588_CLOCK
> + uses VXLAN
> + uses MLXFW
> + uses PCI_HYPERV_INTERFACE
I have confirmed in the meantime that changing the PCI_HYPERV_INTERFACE
dependency is indeed required, in addition to PTP_1588_CLOCK and VXLAN
that the randconfig tests found earlier:
x86_64-linux-ld: drivers/net/ethernet/mellanox/mlx5/core/main.o: in
function `mlx5_unload':
main.c:(.text+0x325): undefined reference to `mlx5_hv_vhca_cleanup'
I also checked that there is no link failure with MLX5_CORE=y and MLXFW=m,
but only because of an IS_REACHABLE() check in mlxfw/mlxfw.h.
I suppose that should be changed back to IS_ENABLED() now along with
the Kconfig check.
Arnd
On Fri, Apr 17, 2020 at 3:12 AM Saeed Mahameed <[email protected]> wrote:
>
> Due to the changes to the semantics of imply keyword [1], which now
> doesn't force any config options to the implied configs any more.
>
> A module (FOO) that has a weak dependency on some other modules (BAR)
> is now broken if it was using imply to force dependency restrictions.
> e.g.: FOO needs BAR to be reachable, especially when FOO=y and BAR=m.
> Which might now introduce build/link errors.
>
> There are two options to solve this:
> 1. use IS_REACHABLE(BAR), everywhere BAR is referenced inside FOO.
> 2. in FOO's Kconfig add: depends on (BAR || !BAR)
>
> The first option is not desirable, and will leave the user confused when
> setting FOO=y and BAR=m, FOO will never reach BAR even though both are
> compiled.
>
> The 2nd one is the preferred approach, and will guarantee BAR is always
> reachable by FOO if both are compiled. But, (BAR || !BAR) is really
> confusing for those who don't really get how kconfig tristate arithmetics
> work.
>
> To solve this and hide this weird expression and to avoid repetition
> across the tree, we introduce new keyword "uses" to the Kconfig options
> family.
>
> uses BAR:
> Equivalent to: depends on symbol || !symbol
> Semantically it means, if FOO is enabled (y/m) and has the option:
> uses BAR, make sure it can reach/use BAR when possible.
>
> For example: if FOO=y and BAR=m, FOO will be forced to m.
>
> [1] https://lore.kernel.org/linux-doc/[email protected]/
Thanks a lot for getting this done. I've tried it out on my randconfig
build tree
and can confirm that this works together with your second patch to address the
specific MLX5 problem.
I also tried out replacing all other instances of 'depends on FOO ||
!FOO', using
this oneline script:
git ls-files | grep Kconfig | xargs sed -i
's:depends.on.\([A-Z0-9_a-z]\+\) || \(\1 \?= \?n\|!\1\):uses \1:'
Unfortunately, this immediately crashes with:
$ make -skj30
how to free type 0?
double free or corruption (fasttop)
make[6]: *** [/git/arm-soc/scripts/kconfig/Makefile:71: olddefconfig]
Aborted (core dumped)
make[5]: *** [/git/arm-soc/Makefile:587: olddefconfig] Error 2
make[4]: *** [/git/arm-soc/scripts/kconfig/Makefile:95:
allrandom.config] Error 2
make[3]: *** [/git/arm-soc/Makefile:587: allrandom.config] Error 2
make[2]: *** [Makefile:180: sub-make] Error 2
make[2]: Target 'allrandom.config' not remade because of errors.
make[1]: *** [makefile:127: allrandom.config] Error 2
It's probably easy to fix, but I did not look any deeper into the bug.
Arnd
On Fri, 2020-04-17 at 12:24 +0200, Arnd Bergmann wrote:
> On Fri, Apr 17, 2020 at 3:12 AM Saeed Mahameed <[email protected]>
> wrote:
> > Due to the changes to the semantics of imply keyword [1], which now
> > doesn't force any config options to the implied configs any more.
> >
> > A module (FOO) that has a weak dependency on some other modules
> > (BAR)
> > is now broken if it was using imply to force dependency
> > restrictions.
> > e.g.: FOO needs BAR to be reachable, especially when FOO=y and
> > BAR=m.
> > Which might now introduce build/link errors.
> >
> > There are two options to solve this:
> > 1. use IS_REACHABLE(BAR), everywhere BAR is referenced inside FOO.
> > 2. in FOO's Kconfig add: depends on (BAR || !BAR)
> >
> > The first option is not desirable, and will leave the user confused
> > when
> > setting FOO=y and BAR=m, FOO will never reach BAR even though both
> > are
> > compiled.
> >
> > The 2nd one is the preferred approach, and will guarantee BAR is
> > always
> > reachable by FOO if both are compiled. But, (BAR || !BAR) is really
> > confusing for those who don't really get how kconfig tristate
> > arithmetics
> > work.
> >
> > To solve this and hide this weird expression and to avoid
> > repetition
> > across the tree, we introduce new keyword "uses" to the Kconfig
> > options
> > family.
> >
> > uses BAR:
> > Equivalent to: depends on symbol || !symbol
> > Semantically it means, if FOO is enabled (y/m) and has the option:
> > uses BAR, make sure it can reach/use BAR when possible.
> >
> > For example: if FOO=y and BAR=m, FOO will be forced to m.
> >
> > [1]
> > https://lore.kernel.org/linux-doc/[email protected]/
>
> Thanks a lot for getting this done. I've tried it out on my
> randconfig
> build tree
> and can confirm that this works together with your second patch to
> address the
> specific MLX5 problem.
>
> I also tried out replacing all other instances of 'depends on FOO ||
> !FOO', using
> this oneline script:
>
> git ls-files | grep Kconfig | xargs sed -i
> 's:depends.on.\([A-Z0-9_a-z]\+\) || \(\1 \?= \?n\|!\1\):uses \1:'
>
> Unfortunately, this immediately crashes with:
>
> $ make -skj30
> how to free type 0?
> double free or corruption (fasttop)
> make[6]: *** [/git/arm-soc/scripts/kconfig/Makefile:71: olddefconfig]
> Aborted (core dumped)
> make[5]: *** [/git/arm-soc/Makefile:587: olddefconfig] Error 2
> make[4]: *** [/git/arm-soc/scripts/kconfig/Makefile:95:
> allrandom.config] Error 2
> make[3]: *** [/git/arm-soc/Makefile:587: allrandom.config] Error 2
> make[2]: *** [Makefile:180: sub-make] Error 2
> make[2]: Target 'allrandom.config' not remade because of errors.
> make[1]: *** [makefile:127: allrandom.config] Error 2
>
> It's probably easy to fix, but I did not look any deeper into the
> bug.
>
Ahh, I know what it is, i am allocating only one expression for the two
symbols (FOO || !FOO) .. in the rule action in parser.y, i must
allocate two individual instances per each of the FOO appearances ..
something like:
struct expr *symexpr1 = expr_alloc_symbol($2);
struct expr *symexpr2 = expr_alloc_symbol($2);
menu_add_dep(expr_alloc_two(E_OR, symexpr1, expr_alloc_one(E_NOT,
symexpr2)));
Thanks Arnd for testing this ! I will test this and send V2 later.
On Fri, Apr 17, 2020 at 09:23:59AM +0300, Jani Nikula wrote:
> Which means that would have to split up to two. Not ideal, but
> doable.
Why is this not ideal?
I think the one per line is easier to maintain (eg for merge
conflicts) and easier to read than a giant && expression.
I would not complicate things further by extending the boolean
language..
Jason
On Fri, 17 Apr 2020, Jason Gunthorpe <[email protected]> wrote:
> On Fri, Apr 17, 2020 at 09:23:59AM +0300, Jani Nikula wrote:
>
>> Which means that would have to split up to two. Not ideal, but
>> doable.
>
> Why is this not ideal?
>
> I think the one per line is easier to maintain (eg for merge
> conflicts) and easier to read than a giant && expression.
>
> I would not complicate things further by extending the boolean
> language..
Fair enough. I only found one instance where the patch at hand does not
cut it:
drivers/hwmon/Kconfig: depends on !OF || IIO=n || IIO
That can of course be left as it is.
As to the bikeshedding topic, I think I'm now leaning towards Andrzej's
suggestion:
optionally depends on FOO
in [1]. But I reserve my right to change my mind. ;)
BR,
Jani.
[1] http://lore.kernel.org/r/[email protected]
--
Jani Nikula, Intel Open Source Graphics Center
On Fri, Apr 17, 2020 at 05:01:18PM +0300, Jani Nikula wrote:
> On Fri, 17 Apr 2020, Jason Gunthorpe <[email protected]> wrote:
> > On Fri, Apr 17, 2020 at 09:23:59AM +0300, Jani Nikula wrote:
> >
> >> Which means that would have to split up to two. Not ideal, but
> >> doable.
> >
> > Why is this not ideal?
> >
> > I think the one per line is easier to maintain (eg for merge
> > conflicts) and easier to read than a giant && expression.
> >
> > I would not complicate things further by extending the boolean
> > language..
>
> Fair enough. I only found one instance where the patch at hand does not
> cut it:
>
> drivers/hwmon/Kconfig: depends on !OF || IIO=n || IIO
Ideally this constraint would be expressed as:
optionally depends on OF && IIO
And if the expression is n then IIO is not prevented from being y.
Ie the code is just doing:
#if defined(CONFIG_OF) && IS_ENABLED(CONFIG_IIO)
Jason
On Fri, Apr 17, 2020 at 10:12 AM Saeed Mahameed <[email protected]> wrote:
>
> Due to the changes to the semantics of imply keyword [1], which now
> doesn't force any config options to the implied configs any more.
>
> A module (FOO) that has a weak dependency on some other modules (BAR)
> is now broken if it was using imply to force dependency restrictions.
> e.g.: FOO needs BAR to be reachable, especially when FOO=y and BAR=m.
> Which might now introduce build/link errors.
>
> There are two options to solve this:
> 1. use IS_REACHABLE(BAR), everywhere BAR is referenced inside FOO.
> 2. in FOO's Kconfig add: depends on (BAR || !BAR)
>
> The first option is not desirable, and will leave the user confused when
> setting FOO=y and BAR=m, FOO will never reach BAR even though both are
> compiled.
>
> The 2nd one is the preferred approach, and will guarantee BAR is always
> reachable by FOO if both are compiled. But, (BAR || !BAR) is really
> confusing for those who don't really get how kconfig tristate arithmetics
> work.
>
> To solve this and hide this weird expression and to avoid repetition
> across the tree, we introduce new keyword "uses" to the Kconfig options
> family.
>
> uses BAR:
> Equivalent to: depends on symbol || !symbol
> Semantically it means, if FOO is enabled (y/m) and has the option:
> uses BAR, make sure it can reach/use BAR when possible.
>
> For example: if FOO=y and BAR=m, FOO will be forced to m.
>
> [1] https://lore.kernel.org/linux-doc/[email protected]/
>
> Link: https://lkml.org/lkml/2020/4/8/839
> Signed-off-by: Saeed Mahameed <[email protected]>
> Cc: Masahiro Yamada <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> ---
I am not convinced with this patch.
This patch adds another way to do the same thing.
It is true that it _hides_ the problems, and
makes the _surface_ cleaner at best,
but the internal will be more complicated.
(FOO || !FOO) is difficult to understand, but
the behavior of "uses FOO" is as difficult to grasp.
People would wonder, "what 'uses FOO' means?",
then they would find the explanation in kconfig-language.rst:
"Equivalent to: depends on symbol || !symbol
Semantically it means, if FOO is enabled (y/m) and has the option:
uses BAR, make sure it can reach/use BAR when possible."
To understand this correctly, people must study
the arithmetic of (symbol || !symbol) anyway.
I do not want to extend Kconfig for the iffy syntax sugar.
(symbol || !symbol) is horrible.
But, I am also scared to see people would think 'uses symbol'
is the right thing to do, and start using it liberally
all over the place.
--
Best Regards
Masahiro Yamada
On Sun, 19 Apr 2020, Masahiro Yamada wrote:
> (FOO || !FOO) is difficult to understand, but
> the behavior of "uses FOO" is as difficult to grasp.
Can't this be expressed as the following instead:
depends on FOO if FOO
That would be a little clearer.
Nicolas
On Sun, Apr 19, 2020 at 4:11 AM Nicolas Pitre <[email protected]> wrote:
>
> On Sun, 19 Apr 2020, Masahiro Yamada wrote:
>
> > (FOO || !FOO) is difficult to understand, but
> > the behavior of "uses FOO" is as difficult to grasp.
>
> Can't this be expressed as the following instead:
>
> depends on FOO if FOO
>
> That would be a little clearer.
>
>
> Nicolas
'depends on' does not take the 'if <expr>'
'depends on A if B' is the syntax sugar of
'depends on (A || !B), right ?
I do not know how clearer it would make things.
depends on (m || FOO != m)
is another equivalent, but we are always
talking about a matter of expression.
How important is it to stick to
depends on (FOO || !FOO)
or its equivalents?
If a driver wants to use the feature FOO
in most usecases, 'depends on FOO' is sensible.
If FOO is just optional, you can get rid of the dependency,
and IS_REACHABLE() will do logically correct things.
I do not think IS_REACHABLE() is too bad,
but if it is confusing, we can add one more
option to make it explicit.
config DRIVER_X
tristate "driver x"
config DRIVER_X_USES_FOO
bool "use FOO from driver X"
depends on DRIVER_X
depends on DRIVER_X <= FOO
help
DRIVER_X works without FOO, but
Using FOO will provide better usability.
Say Y if you want to make driver X use FOO.
Of course,
if (IS_ENABLED(CONFIG_DRIVER_X_USES_FOO))
foo_init();
works like
if (IS_REACHABLE(CONFIG_FOO))
foo_init();
At lease, it will eliminate a question like
"I loaded the module FOO, I swear.
But my built-in driver X still would not use FOO, why?"
--
Best Regards
Masahiro Yamada
On Sun, 19 Apr 2020, Masahiro Yamada <[email protected]> wrote:
> On Sun, Apr 19, 2020 at 4:11 AM Nicolas Pitre <[email protected]> wrote:
>>
>> On Sun, 19 Apr 2020, Masahiro Yamada wrote:
>>
>> > (FOO || !FOO) is difficult to understand, but
>> > the behavior of "uses FOO" is as difficult to grasp.
>>
>> Can't this be expressed as the following instead:
>>
>> depends on FOO if FOO
>>
>> That would be a little clearer.
>>
>>
>> Nicolas
>
>
>
> 'depends on' does not take the 'if <expr>'
>
> 'depends on A if B' is the syntax sugar of
> 'depends on (A || !B), right ?
>
> I do not know how clearer it would make things.
>
> depends on (m || FOO != m)
> is another equivalent, but we are always
> talking about a matter of expression.
>
>
> How important is it to stick to
> depends on (FOO || !FOO)
> or its equivalents?
>
>
> If a driver wants to use the feature FOO
> in most usecases, 'depends on FOO' is sensible.
>
> If FOO is just optional, you can get rid of the dependency,
> and IS_REACHABLE() will do logically correct things.
If by logically correct you mean the kernel builds, you're
right. However the proliferation of IS_REACHABLE() is making the kernel
config *harder* to understand. User enables FOO=m and expects BAR to use
it, however if BAR=y it silently gets ignored. I have and I will oppose
adding IS_REACHABLE() usage to i915 because it's just silently accepting
configurations that should be flagged and forbidden at kconfig stage.
> I do not think IS_REACHABLE() is too bad,
> but if it is confusing, we can add one more
> option to make it explicit.
>
>
>
> config DRIVER_X
> tristate "driver x"
>
> config DRIVER_X_USES_FOO
> bool "use FOO from driver X"
> depends on DRIVER_X
> depends on DRIVER_X <= FOO
> help
> DRIVER_X works without FOO, but
> Using FOO will provide better usability.
> Say Y if you want to make driver X use FOO.
>
>
>
> Of course,
>
> if (IS_ENABLED(CONFIG_DRIVER_X_USES_FOO))
> foo_init();
>
> works like
>
> if (IS_REACHABLE(CONFIG_FOO))
> foo_init();
>
>
> At lease, it will eliminate a question like
> "I loaded the module FOO, I swear.
> But my built-in driver X still would not use FOO, why?"
Please let's not make that a more widespread problem than it already
is. I have yet to hear *one* good rationale for allowing that in the
first place. And if that pops up, you can make it work by using
IS_REACHABLE() *without* the depends, simply by checking if the module
is there.
Most use cases increasingly solved by IS_REACHABLE() should use the
"depends on FOO || FOO=n" construct, but the problem is that's not
widely understood. I'd like to have another keyword for people to
copy-paste into their Kconfigs.
In another mail I suggested
optionally depends on FOO
might be a better alternative than "uses".
BR,
Jani.
--
Jani Nikula, Intel Open Source Graphics Center
On Sun, Apr 19, 2020 at 04:00:43AM +0900, Masahiro Yamada wrote:
> People would wonder, "what 'uses FOO' means?",
> then they would find the explanation in kconfig-language.rst:
>
> "Equivalent to: depends on symbol || !symbol
> Semantically it means, if FOO is enabled (y/m) and has the option:
> uses BAR, make sure it can reach/use BAR when possible."
>
> To understand this correctly, people must study
> the arithmetic of (symbol || !symbol) anyway.
I think people will just cargo-cult copy it and not think too hard
about how kconfig works.
The descriptions in kconfig-language.rst can be improved to better
guide C people using kconfig without entirely understanding
it. Something like:
BAR depends on FOO // BAR selects FOO: BAR requires functionality from
FOO
BAR uses FOO: BAR optionally consumes functionality from FOO using
IS_ENABLED
BAR implies FOO: BAR optionally consumes functionality from FOO using
IS_REACHABLE
Now someone adding IS_ENABLED or IS_REACHABLE checks to C code knows
exactly what to put in the kconfig.
Jason
On Mon, 20 Apr 2020 11:43:13 +0300 Jani Nikula wrote:
> On Sun, 19 Apr 2020, Masahiro Yamada <[email protected]> wrote:
> > On Sun, Apr 19, 2020 at 4:11 AM Nicolas Pitre <[email protected]> wrote:
> >>
> >> On Sun, 19 Apr 2020, Masahiro Yamada wrote:
> >>
> >> > (FOO || !FOO) is difficult to understand, but
> >> > the behavior of "uses FOO" is as difficult to grasp.
> >>
> >> Can't this be expressed as the following instead:
> >>
> >> depends on FOO if FOO
> >>
> >> That would be a little clearer.
> >>
> >>
> >> Nicolas
> >
> > 'depends on' does not take the 'if <expr>'
> >
> > 'depends on A if B' is the syntax sugar of
> > 'depends on (A || !B), right ?
> >
> > I do not know how clearer it would make things.
> >
> > depends on (m || FOO != m)
> > is another equivalent, but we are always
> > talking about a matter of expression.
> >
> >
> > How important is it to stick to
> > depends on (FOO || !FOO)
> > or its equivalents?
> >
> >
> > If a driver wants to use the feature FOO
> > in most usecases, 'depends on FOO' is sensible.
> >
> > If FOO is just optional, you can get rid of the dependency,
> > and IS_REACHABLE() will do logically correct things.
>
> If by logically correct you mean the kernel builds, you're
> right. However the proliferation of IS_REACHABLE() is making the kernel
> config *harder* to understand. User enables FOO=m and expects BAR to use
> it, however if BAR=y it silently gets ignored. I have and I will oppose
> adding IS_REACHABLE() usage to i915 because it's just silently accepting
> configurations that should be flagged and forbidden at kconfig stage.
+1
I wholeheartedly agree. In case of Ethernet drivers having higher
layers of the stack not able to communicate with drivers is just
broken IMHO.
On Mon, 2020-04-20 at 11:43 +0300, Jani Nikula wrote:
> On Sun, 19 Apr 2020, Masahiro Yamada <[email protected]> wrote:
> > On Sun, Apr 19, 2020 at 4:11 AM Nicolas Pitre <[email protected]>
> > wrote:
> > > On Sun, 19 Apr 2020, Masahiro Yamada wrote:
> > >
> > > > (FOO || !FOO) is difficult to understand, but
> > > > the behavior of "uses FOO" is as difficult to grasp.
> > >
> > > Can't this be expressed as the following instead:
> > >
> > > depends on FOO if FOO
> > >
> > > That would be a little clearer.
> > >
> > >
> > > Nicolas
> >
> >
> > 'depends on' does not take the 'if <expr>'
> >
> > 'depends on A if B' is the syntax sugar of
> > 'depends on (A || !B), right ?
> >
> > I do not know how clearer it would make things.
> >
> > depends on (m || FOO != m)
> > is another equivalent, but we are always
> > talking about a matter of expression.
> >
> >
> > How important is it to stick to
> > depends on (FOO || !FOO)
> > or its equivalents?
> >
> >
> > If a driver wants to use the feature FOO
> > in most usecases, 'depends on FOO' is sensible.
> >
> > If FOO is just optional, you can get rid of the dependency,
> > and IS_REACHABLE() will do logically correct things.
>
> If by logically correct you mean the kernel builds, you're
> right. However the proliferation of IS_REACHABLE() is making the
> kernel
> config *harder* to understand. User enables FOO=m and expects BAR to
> use
> it, however if BAR=y it silently gets ignored. I have and I will
> oppose
> adding IS_REACHABLE() usage to i915 because it's just silently
> accepting
> configurations that should be flagged and forbidden at kconfig stage.
>
> > I do not think IS_REACHABLE() is too bad,
> > but if it is confusing, we can add one more
> > option to make it explicit.
> >
> >
> >
> > config DRIVER_X
> > tristate "driver x"
> >
> > config DRIVER_X_USES_FOO
> > bool "use FOO from driver X"
> > depends on DRIVER_X
> > depends on DRIVER_X <= FOO
> > help
> > DRIVER_X works without FOO, but
> > Using FOO will provide better usability.
> > Say Y if you want to make driver X use FOO.
> >
> >
> >
> > Of course,
> >
> > if (IS_ENABLED(CONFIG_DRIVER_X_USES_FOO))
> > foo_init();
> >
> > works like
> >
> > if (IS_REACHABLE(CONFIG_FOO))
> > foo_init();
> >
> >
> > At lease, it will eliminate a question like
> > "I loaded the module FOO, I swear.
> > But my built-in driver X still would not use FOO, why?"
>
and duplicate this all over just to avoid new keyword.
> Please let's not make that a more widespread problem than it already
> is. I have yet to hear *one* good rationale for allowing that in the
> first place. And if that pops up, you can make it work by using
> IS_REACHABLE() *without* the depends, simply by checking if the
> module
> is there.
>
> Most use cases increasingly solved by IS_REACHABLE() should use the
> "depends on FOO || FOO=n" construct, but the problem is that's not
> widely understood. I'd like to have another keyword for people to
> copy-paste into their Kconfigs.
>
+1
do all C developers know how the C compiler works ? of course not !
Same goes here, there is a demand for a new keyword, so people will
avoid copy and pate and can use the kconfig language in a higher
simplified level.
I just did a quick grep to find out how really people use depend on:
# All usage of depends on
$ git ls-files | grep Kconfig | xargs grep -E "depends\s+on" | wc -l
15071
# simple single symbol expression usage
$ git ls-files | grep Kconfig | xargs grep -E "depends\s+on\s+[A-Za-z0-
9_]+\s*$" | wc -l
8889
almost 60%..
people really like simple things especially for the tools they are
using "like kconfig", no one really wants to understand how it really
work under the hood if it is a one time thing that you need to setup
for your kernel project, unless it is really necessary ..
I wonder how many of those 8889 cases wanted a weak dependency but
couldn't figure out how to do it ?
Users of depends on FOO || !FOO
$ git ls-files | grep Kconfig | xargs grep -E \
"depends\s+on\s+([A-Za-z0-9_]+)\s*\|\|\s*(\!\s*\1|\1\s*=\s*n)" \
| wc -l
156
a new keyword is required :) ..
> In another mail I suggested
>
> optionally depends on FOO
>
> might be a better alternative than "uses".
>
>
how about just:
optional FOO
It is clear and easy to document ..
> BR,
> Jani.
>
On Tue, 21 Apr 2020, Saeed Mahameed wrote:
> I wonder how many of those 8889 cases wanted a weak dependency but
> couldn't figure out how to do it ?
>
> Users of depends on FOO || !FOO
>
> $ git ls-files | grep Kconfig | xargs grep -E \
> "depends\s+on\s+([A-Za-z0-9_]+)\s*\|\|\s*(\!\s*\1|\1\s*=\s*n)" \
> | wc -l
>
> 156
>
> a new keyword is required :) ..
>
>
> > In another mail I suggested
> >
> > optionally depends on FOO
> >
> > might be a better alternative than "uses".
> >
> >
>
> how about just:
> optional FOO
>
> It is clear and easy to document ..
I don't dispute your argument for having a new keyword. But the most
difficult part as Arnd said is to find it. You cannot pretend that
"optional FOO" is clear when it actually imposes a restriction when
FOO=m. Try to justify to people why they cannot select y because of this
"optional" thing.
Nicolas
On Tue, 2020-04-21 at 09:58 -0400, Nicolas Pitre wrote:
> On Tue, 21 Apr 2020, Saeed Mahameed wrote:
>
> > I wonder how many of those 8889 cases wanted a weak dependency but
> > couldn't figure out how to do it ?
> >
> > Users of depends on FOO || !FOO
> >
> > $ git ls-files | grep Kconfig | xargs grep -E \
> > "depends\s+on\s+([A-Za-z0-9_]+)\s*\|\|\s*(\!\s*\1|\1\s*=\s*n)" \
> > | wc -l
> >
> > 156
> >
> > a new keyword is required :) ..
> >
> >
> > > In another mail I suggested
> > >
> > > optionally depends on FOO
> > >
> > > might be a better alternative than "uses".
> > >
> > >
> >
> > how about just:
> > optional FOO
> >
> > It is clear and easy to document ..
>
> I don't dispute your argument for having a new keyword. But the most
> difficult part as Arnd said is to find it. You cannot pretend that
kconfig-language.rst ?
> "optional FOO" is clear when it actually imposes a restriction when
> FOO=m. Try to justify to people why they cannot select y because of
> this
> "optional" thing.
>
Then let's use "uses" it is more assertive. Documentation will cover
any vague anything about it ..
>
> Nicolas
On Tue, 21 Apr 2020, Saeed Mahameed wrote:
> On Tue, 2020-04-21 at 09:58 -0400, Nicolas Pitre wrote:
> > On Tue, 21 Apr 2020, Saeed Mahameed wrote:
> >
> > > I wonder how many of those 8889 cases wanted a weak dependency but
> > > couldn't figure out how to do it ?
> > >
> > > Users of depends on FOO || !FOO
> > >
> > > $ git ls-files | grep Kconfig | xargs grep -E \
> > > "depends\s+on\s+([A-Za-z0-9_]+)\s*\|\|\s*(\!\s*\1|\1\s*=\s*n)" \
> > > | wc -l
> > >
> > > 156
> > >
> > > a new keyword is required :) ..
> > >
> > >
> > > > In another mail I suggested
> > > >
> > > > optionally depends on FOO
> > > >
> > > > might be a better alternative than "uses".
> > > >
> > > >
> > >
> > > how about just:
> > > optional FOO
> > >
> > > It is clear and easy to document ..
> >
> > I don't dispute your argument for having a new keyword. But the most
> > difficult part as Arnd said is to find it. You cannot pretend that
>
> kconfig-language.rst ?
>
> > "optional FOO" is clear when it actually imposes a restriction when
> > FOO=m. Try to justify to people why they cannot select y because of
> > this
> > "optional" thing.
> >
>
> Then let's use "uses" it is more assertive. Documentation will cover
> any vague anything about it ..
It uses what? And why can't I configure this with "uses FOO" when FOO=m?
That's not any clearer. And saying that "this is weird but it is
described in the documentation" is not good enough. We must make things
clear in the first place.
This is really a conditional dependency. That's all this is about.
So why not simply making it so rather than fooling ourselves? All that
is required is an extension that would allow:
depends on (expression) if (expression)
This construct should be obvious even without reading the doc, is
already used extensively for other things already, and is flexible
enough to cover all sort of cases in addition to this particular one.
Nicolas
On Tue, 21 Apr 2020, Nicolas Pitre <[email protected]> wrote:
> On Tue, 21 Apr 2020, Saeed Mahameed wrote:
>
>> On Tue, 2020-04-21 at 09:58 -0400, Nicolas Pitre wrote:
>> > On Tue, 21 Apr 2020, Saeed Mahameed wrote:
>> >
>> > > I wonder how many of those 8889 cases wanted a weak dependency but
>> > > couldn't figure out how to do it ?
>> > >
>> > > Users of depends on FOO || !FOO
>> > >
>> > > $ git ls-files | grep Kconfig | xargs grep -E \
>> > > "depends\s+on\s+([A-Za-z0-9_]+)\s*\|\|\s*(\!\s*\1|\1\s*=\s*n)" \
>> > > | wc -l
>> > >
>> > > 156
>> > >
>> > > a new keyword is required :) ..
>> > >
>> > >
>> > > > In another mail I suggested
>> > > >
>> > > > optionally depends on FOO
>> > > >
>> > > > might be a better alternative than "uses".
>> > > >
>> > > >
>> > >
>> > > how about just:
>> > > optional FOO
>> > >
>> > > It is clear and easy to document ..
>> >
>> > I don't dispute your argument for having a new keyword. But the most
>> > difficult part as Arnd said is to find it. You cannot pretend that
>>
>> kconfig-language.rst ?
>>
>> > "optional FOO" is clear when it actually imposes a restriction when
>> > FOO=m. Try to justify to people why they cannot select y because of
>> > this
>> > "optional" thing.
>> >
>>
>> Then let's use "uses" it is more assertive. Documentation will cover
>> any vague anything about it ..
>
> It uses what? And why can't I configure this with "uses FOO" when FOO=m?
> That's not any clearer. And saying that "this is weird but it is
> described in the documentation" is not good enough. We must make things
> clear in the first place.
>
> This is really a conditional dependency. That's all this is about.
> So why not simply making it so rather than fooling ourselves? All that
> is required is an extension that would allow:
>
> depends on (expression) if (expression)
>
> This construct should be obvious even without reading the doc, is
> already used extensively for other things already, and is flexible
> enough to cover all sort of cases in addition to this particular one.
Okay, you convinced me. Now you only need to convince whoever is doing
the actual work of implementing this stuff. ;)
BR,
Jani.
--
Jani Nikula, Intel Open Source Graphics Center
On Wed, 22 Apr 2020, Jani Nikula wrote:
> On Tue, 21 Apr 2020, Nicolas Pitre <[email protected]> wrote:
> > This is really a conditional dependency. That's all this is about.
> > So why not simply making it so rather than fooling ourselves? All that
> > is required is an extension that would allow:
> >
> > depends on (expression) if (expression)
> >
> > This construct should be obvious even without reading the doc, is
> > already used extensively for other things already, and is flexible
> > enough to cover all sort of cases in addition to this particular one.
>
> Okay, you convinced me. Now you only need to convince whoever is doing
> the actual work of implementing this stuff. ;)
What about this:
----- >8
Subject: [PATCH] kconfig: allow for conditional dependencies
This might appear to be a strange concept, but sometimes we want
a dependency to be conditionally applied. One such case is currently
expressed with:
depends on FOO || !FOO
This pattern is strange enough to give one's pause. Given that it is
also frequent, let's make the intent more obvious with some syntaxic
sugar by effectively making dependencies optionally conditional.
This also makes the kconfig language more uniform.
Signed-off-by: Nicolas Pitre <[email protected]>
diff --git a/Documentation/kbuild/kconfig-language.rst b/Documentation/kbuild/kconfig-language.rst
index d0111dd264..0f841e0037 100644
--- a/Documentation/kbuild/kconfig-language.rst
+++ b/Documentation/kbuild/kconfig-language.rst
@@ -114,7 +114,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
@@ -130,6 +130,16 @@ applicable everywhere (see syntax).
bool "foo"
default y
+ The dependency definition itself may be conditional by appending "if"
+ followed by an expression. If such expression is false (n) then this
+ dependency is ignored. One possible use case is:
+
+ config FOO
+ tristate
+ depends on BAZ if BAZ != n
+
+ meaning that FOO is constrained by the value of BAZ only when it is set.
+
- reverse dependencies: "select" <symbol> ["if" <expr>]
While normal dependencies reduce the upper limit of a symbol (see
diff --git a/scripts/kconfig/lkc.h b/scripts/kconfig/lkc.h
index d4ca829736..1a9337d1b9 100644
--- a/scripts/kconfig/lkc.h
+++ b/scripts/kconfig/lkc.h
@@ -72,7 +72,7 @@ void menu_warn(struct menu *menu, const char *fmt, ...);
struct menu *menu_add_menu(void);
void menu_end_menu(void);
void menu_add_entry(struct symbol *sym);
-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, char *prompt, struct expr *dep);
void menu_add_expr(enum prop_type type, struct expr *expr, struct expr *dep);
diff --git a/scripts/kconfig/menu.c b/scripts/kconfig/menu.c
index e436ba44c9..e6b204225e 100644
--- a/scripts/kconfig/menu.c
+++ b/scripts/kconfig/menu.c
@@ -103,8 +103,22 @@ 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) {
+ struct expr *cond2, *left, *right;
+ /*
+ * We have "depends on X if Y" and we want:
+ * Y != n --> X
+ * Y == n --> y
+ * Meaning: ((Y != n) && X) || (Y == n)
+ */
+ cond2 = expr_copy(cond);
+ left = expr_trans_compare(cond2, E_UNEQUAL, &symbol_no);
+ left = expr_alloc_and(dep, left);
+ right = expr_trans_compare(cond, E_EQUAL, &symbol_no);
+ dep = expr_alloc_or(left, right);
+ }
current_entry->dep = expr_alloc_and(current_entry->dep, dep);
}
diff --git a/scripts/kconfig/parser.y b/scripts/kconfig/parser.y
index 708b6c4b13..4161207da2 100644
--- a/scripts/kconfig/parser.y
+++ b/scripts/kconfig/parser.y
@@ -316,7 +316,7 @@ if_entry: T_IF expr T_EOL
{
printd(DEBUG_PARSE, "%s:%d:if\n", zconf_curname(), zconf_lineno());
menu_add_entry(NULL);
- menu_add_dep($2);
+ menu_add_dep($2, NULL);
$$ = menu_add_menu();
};
@@ -412,9 +412,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", zconf_curname(), zconf_lineno());
};
On 4/22/20 2:13 PM, Nicolas Pitre wrote:
> On Wed, 22 Apr 2020, Jani Nikula wrote:
>
>> On Tue, 21 Apr 2020, Nicolas Pitre <[email protected]> wrote:
>>> This is really a conditional dependency. That's all this is about.
>>> So why not simply making it so rather than fooling ourselves? All that
>>> is required is an extension that would allow:
>>>
>>> depends on (expression) if (expression)
>>>
>>> This construct should be obvious even without reading the doc, is
>>> already used extensively for other things already, and is flexible
>>> enough to cover all sort of cases in addition to this particular one.
>>
>> Okay, you convinced me. Now you only need to convince whoever is doing
>> the actual work of implementing this stuff. ;)
>
> What about this:
>
> ----- >8
> Subject: [PATCH] kconfig: allow for conditional dependencies
>
> This might appear to be a strange concept, but sometimes we want
> a dependency to be conditionally applied. One such case is currently
> expressed with:
>
> depends on FOO || !FOO
>
> This pattern is strange enough to give one's pause. Given that it is
> also frequent, let's make the intent more obvious with some syntaxic
> sugar by effectively making dependencies optionally conditional.
> This also makes the kconfig language more uniform.
>
> Signed-off-by: Nicolas Pitre <[email protected]>
Hi,
If we must do something here, I prefer this one.
Nicolas, would you do another example, specifically for
CRAMFS_MTD in fs/cramfs/Kconfig, please?
thanks.
--
~Randy
On Thu, Apr 23, 2020 at 11:01:40AM -0400, Nicolas Pitre wrote:
> On Wed, 22 Apr 2020, Randy Dunlap wrote:
>
> > On 4/22/20 2:13 PM, Nicolas Pitre wrote:
> > > On Wed, 22 Apr 2020, Jani Nikula wrote:
> > >
> > >> On Tue, 21 Apr 2020, Nicolas Pitre <[email protected]> wrote:
> > >>> This is really a conditional dependency. That's all this is about.
> > >>> So why not simply making it so rather than fooling ourselves? All that
> > >>> is required is an extension that would allow:
> > >>>
> > >>> depends on (expression) if (expression)
> > >>>
> > >>> This construct should be obvious even without reading the doc, is
> > >>> already used extensively for other things already, and is flexible
> > >>> enough to cover all sort of cases in addition to this particular one.
> > >>
> > >> Okay, you convinced me. Now you only need to convince whoever is doing
> > >> the actual work of implementing this stuff. ;)
> > >
> > > What about this:
> > >
> > > Subject: [PATCH] kconfig: allow for conditional dependencies
> > >
> > > This might appear to be a strange concept, but sometimes we want
> > > a dependency to be conditionally applied. One such case is currently
> > > expressed with:
> > >
> > > depends on FOO || !FOO
> > >
> > > This pattern is strange enough to give one's pause. Given that it is
> > > also frequent, let's make the intent more obvious with some syntaxic
> > > sugar by effectively making dependencies optionally conditional.
> > > This also makes the kconfig language more uniform.
> > >
> > > Signed-off-by: Nicolas Pitre <[email protected]>
> >
> > Hi,
> >
> > If we must do something here, I prefer this one.
> >
> > Nicolas, would you do another example, specifically for
> > CRAMFS_MTD in fs/cramfs/Kconfig, please?
>
> I don't see how that one can be helped. The MTD dependency is not
> optional.
Could it be done as
config MTD
depends on CRAMFS if CRAMFS_MTD
?
Jason
On Thu, 23 Apr 2020, Jason Gunthorpe wrote:
> On Thu, Apr 23, 2020 at 11:01:40AM -0400, Nicolas Pitre wrote:
> > On Wed, 22 Apr 2020, Randy Dunlap wrote:
> >
> > > On 4/22/20 2:13 PM, Nicolas Pitre wrote:
> > > > On Wed, 22 Apr 2020, Jani Nikula wrote:
> > > >
> > > >> On Tue, 21 Apr 2020, Nicolas Pitre <[email protected]> wrote:
> > > >>> This is really a conditional dependency. That's all this is about.
> > > >>> So why not simply making it so rather than fooling ourselves? All that
> > > >>> is required is an extension that would allow:
> > > >>>
> > > >>> depends on (expression) if (expression)
> > > >>>
> > > >>> This construct should be obvious even without reading the doc, is
> > > >>> already used extensively for other things already, and is flexible
> > > >>> enough to cover all sort of cases in addition to this particular one.
> > > >>
> > > >> Okay, you convinced me. Now you only need to convince whoever is doing
> > > >> the actual work of implementing this stuff. ;)
> > > >
> > > > What about this:
> > > >
> > > > Subject: [PATCH] kconfig: allow for conditional dependencies
> > > >
> > > > This might appear to be a strange concept, but sometimes we want
> > > > a dependency to be conditionally applied. One such case is currently
> > > > expressed with:
> > > >
> > > > depends on FOO || !FOO
> > > >
> > > > This pattern is strange enough to give one's pause. Given that it is
> > > > also frequent, let's make the intent more obvious with some syntaxic
> > > > sugar by effectively making dependencies optionally conditional.
> > > > This also makes the kconfig language more uniform.
> > > >
> > > > Signed-off-by: Nicolas Pitre <[email protected]>
> > >
> > > Hi,
> > >
> > > If we must do something here, I prefer this one.
> > >
> > > Nicolas, would you do another example, specifically for
> > > CRAMFS_MTD in fs/cramfs/Kconfig, please?
> >
> > I don't see how that one can be helped. The MTD dependency is not
> > optional.
>
> Could it be done as
>
> config MTD
> depends on CRAMFS if CRAMFS_MTD
>
> ?
No. There is no logic in restricting MTD usage based on CRAMFS or
CRAMFS_MTD.
Nicolas
On Thu, Apr 23, 2020 at 11:33:33AM -0400, Nicolas Pitre wrote:
> > > No. There is no logic in restricting MTD usage based on CRAMFS or
> > > CRAMFS_MTD.
> >
> > Ah, I got it backwards, maybe this:
> >
> > config CRAMFS
> > depends on MTD if CRAMFS_MTD
> >
> > ?
>
> Still half-backward. CRAMFS should not depend on either MTD nor
> CRAMFS_MTD.
Well, I would view this the same as all the other cases.. the CRAMFS
module has an optional ability consume symbols from MTD. Here that is
controlled by another 'CRAMFS_MTD' selection, but it should still
settle it out the same way as other cases like this - ie CRAMFS is
restricted to m if MTD is m
Arnd's point that kconfig is acyclic does kill it though :(
> It is CRAMFS_MTD that needs both CRAMFS and MTD.
> Furthermore CRAMFS_MTD can't be built-in if MTD is modular.
CRAMFS_MTD is a bool feature flag for the CRAMFS tristate - it is
CRAMFS that can't be built in if MTD is modular.
Jason
On Wed, 22 Apr 2020, Randy Dunlap wrote:
> On 4/22/20 2:13 PM, Nicolas Pitre wrote:
> > On Wed, 22 Apr 2020, Jani Nikula wrote:
> >
> >> On Tue, 21 Apr 2020, Nicolas Pitre <[email protected]> wrote:
> >>> This is really a conditional dependency. That's all this is about.
> >>> So why not simply making it so rather than fooling ourselves? All that
> >>> is required is an extension that would allow:
> >>>
> >>> depends on (expression) if (expression)
> >>>
> >>> This construct should be obvious even without reading the doc, is
> >>> already used extensively for other things already, and is flexible
> >>> enough to cover all sort of cases in addition to this particular one.
> >>
> >> Okay, you convinced me. Now you only need to convince whoever is doing
> >> the actual work of implementing this stuff. ;)
> >
> > What about this:
> >
> > ----- >8
> > Subject: [PATCH] kconfig: allow for conditional dependencies
> >
> > This might appear to be a strange concept, but sometimes we want
> > a dependency to be conditionally applied. One such case is currently
> > expressed with:
> >
> > depends on FOO || !FOO
> >
> > This pattern is strange enough to give one's pause. Given that it is
> > also frequent, let's make the intent more obvious with some syntaxic
> > sugar by effectively making dependencies optionally conditional.
> > This also makes the kconfig language more uniform.
> >
> > Signed-off-by: Nicolas Pitre <[email protected]>
>
> Hi,
>
> If we must do something here, I prefer this one.
>
> Nicolas, would you do another example, specifically for
> CRAMFS_MTD in fs/cramfs/Kconfig, please?
I don't see how that one can be helped. The MTD dependency is not
optional.
Nicolas
On Thu, Apr 23, 2020 at 11:11:46AM -0400, Nicolas Pitre wrote:
> On Thu, 23 Apr 2020, Jason Gunthorpe wrote:
>
> > On Thu, Apr 23, 2020 at 11:01:40AM -0400, Nicolas Pitre wrote:
> > > On Wed, 22 Apr 2020, Randy Dunlap wrote:
> > >
> > > > On 4/22/20 2:13 PM, Nicolas Pitre wrote:
> > > > > On Wed, 22 Apr 2020, Jani Nikula wrote:
> > > > >
> > > > >> On Tue, 21 Apr 2020, Nicolas Pitre <[email protected]> wrote:
> > > > >>> This is really a conditional dependency. That's all this is about.
> > > > >>> So why not simply making it so rather than fooling ourselves? All that
> > > > >>> is required is an extension that would allow:
> > > > >>>
> > > > >>> depends on (expression) if (expression)
> > > > >>>
> > > > >>> This construct should be obvious even without reading the doc, is
> > > > >>> already used extensively for other things already, and is flexible
> > > > >>> enough to cover all sort of cases in addition to this particular one.
> > > > >>
> > > > >> Okay, you convinced me. Now you only need to convince whoever is doing
> > > > >> the actual work of implementing this stuff. ;)
> > > > >
> > > > > What about this:
> > > > >
> > > > > Subject: [PATCH] kconfig: allow for conditional dependencies
> > > > >
> > > > > This might appear to be a strange concept, but sometimes we want
> > > > > a dependency to be conditionally applied. One such case is currently
> > > > > expressed with:
> > > > >
> > > > > depends on FOO || !FOO
> > > > >
> > > > > This pattern is strange enough to give one's pause. Given that it is
> > > > > also frequent, let's make the intent more obvious with some syntaxic
> > > > > sugar by effectively making dependencies optionally conditional.
> > > > > This also makes the kconfig language more uniform.
> > > > >
> > > > > Signed-off-by: Nicolas Pitre <[email protected]>
> > > >
> > > > Hi,
> > > >
> > > > If we must do something here, I prefer this one.
> > > >
> > > > Nicolas, would you do another example, specifically for
> > > > CRAMFS_MTD in fs/cramfs/Kconfig, please?
> > >
> > > I don't see how that one can be helped. The MTD dependency is not
> > > optional.
> >
> > Could it be done as
> >
> > config MTD
> > depends on CRAMFS if CRAMFS_MTD
> >
> > ?
>
> No. There is no logic in restricting MTD usage based on CRAMFS or
> CRAMFS_MTD.
Ah, I got it backwards, maybe this:
config CRAMFS
depends on MTD if CRAMFS_MTD
?
Jason
On Thu, Apr 23, 2020 at 5:16 PM Jason Gunthorpe <[email protected]> wrote:
> On Thu, Apr 23, 2020 at 11:11:46AM -0400, Nicolas Pitre wrote:
> > > > I don't see how that one can be helped. The MTD dependency is not
> > > > optional.
> > >
> > > Could it be done as
> > >
> > > config MTD
> > > depends on CRAMFS if CRAMFS_MTD
> > >
> > > ?
> >
> > No. There is no logic in restricting MTD usage based on CRAMFS or
> > CRAMFS_MTD.
>
> Ah, I got it backwards, maybe this:
>
> config CRAMFS
> depends on MTD if CRAMFS_MTD
I'm not sure this can work if you also have the requirement that 'CRAMFS_MTD
depends on CRAMFS': dependencies in Kconfig generally cannot have
loops in them.
Arnd
On Thu, 23 Apr 2020, Jason Gunthorpe wrote:
> On Thu, Apr 23, 2020 at 11:11:46AM -0400, Nicolas Pitre wrote:
> > On Thu, 23 Apr 2020, Jason Gunthorpe wrote:
> >
> > > On Thu, Apr 23, 2020 at 11:01:40AM -0400, Nicolas Pitre wrote:
> > > > On Wed, 22 Apr 2020, Randy Dunlap wrote:
> > > >
> > > > > On 4/22/20 2:13 PM, Nicolas Pitre wrote:
> > > > > > On Wed, 22 Apr 2020, Jani Nikula wrote:
> > > > > >
> > > > > >> On Tue, 21 Apr 2020, Nicolas Pitre <[email protected]> wrote:
> > > > > >>> This is really a conditional dependency. That's all this is about.
> > > > > >>> So why not simply making it so rather than fooling ourselves? All that
> > > > > >>> is required is an extension that would allow:
> > > > > >>>
> > > > > >>> depends on (expression) if (expression)
> > > > > >>>
> > > > > >>> This construct should be obvious even without reading the doc, is
> > > > > >>> already used extensively for other things already, and is flexible
> > > > > >>> enough to cover all sort of cases in addition to this particular one.
> > > > > >>
> > > > > >> Okay, you convinced me. Now you only need to convince whoever is doing
> > > > > >> the actual work of implementing this stuff. ;)
> > > > > >
> > > > > > What about this:
> > > > > >
> > > > > > Subject: [PATCH] kconfig: allow for conditional dependencies
> > > > > >
> > > > > > This might appear to be a strange concept, but sometimes we want
> > > > > > a dependency to be conditionally applied. One such case is currently
> > > > > > expressed with:
> > > > > >
> > > > > > depends on FOO || !FOO
> > > > > >
> > > > > > This pattern is strange enough to give one's pause. Given that it is
> > > > > > also frequent, let's make the intent more obvious with some syntaxic
> > > > > > sugar by effectively making dependencies optionally conditional.
> > > > > > This also makes the kconfig language more uniform.
> > > > > >
> > > > > > Signed-off-by: Nicolas Pitre <[email protected]>
> > > > >
> > > > > Hi,
> > > > >
> > > > > If we must do something here, I prefer this one.
> > > > >
> > > > > Nicolas, would you do another example, specifically for
> > > > > CRAMFS_MTD in fs/cramfs/Kconfig, please?
> > > >
> > > > I don't see how that one can be helped. The MTD dependency is not
> > > > optional.
> > >
> > > Could it be done as
> > >
> > > config MTD
> > > depends on CRAMFS if CRAMFS_MTD
> > >
> > > ?
> >
> > No. There is no logic in restricting MTD usage based on CRAMFS or
> > CRAMFS_MTD.
>
> Ah, I got it backwards, maybe this:
>
> config CRAMFS
> depends on MTD if CRAMFS_MTD
>
> ?
Still half-backward. CRAMFS should not depend on either MTD nor
CRAMFS_MTD.
It is CRAMFS_MTD that needs both CRAMFS and MTD.
Furthermore CRAMFS_MTD can't be built-in if MTD is modular.
Nicolas
On Thu, 23 Apr 2020, Jason Gunthorpe wrote:
> On Thu, Apr 23, 2020 at 11:33:33AM -0400, Nicolas Pitre wrote:
> > > > No. There is no logic in restricting MTD usage based on CRAMFS or
> > > > CRAMFS_MTD.
> > >
> > > Ah, I got it backwards, maybe this:
> > >
> > > config CRAMFS
> > > depends on MTD if CRAMFS_MTD
> > >
> > > ?
> >
> > Still half-backward. CRAMFS should not depend on either MTD nor
> > CRAMFS_MTD.
>
> Well, I would view this the same as all the other cases.. the CRAMFS
> module has an optional ability consume symbols from MTD. Here that is
> controlled by another 'CRAMFS_MTD' selection, but it should still
> settle it out the same way as other cases like this - ie CRAMFS is
> restricted to m if MTD is m
>
> Arnd's point that kconfig is acyclic does kill it though :(
>
> > It is CRAMFS_MTD that needs both CRAMFS and MTD.
> > Furthermore CRAMFS_MTD can't be built-in if MTD is modular.
>
> CRAMFS_MTD is a bool feature flag for the CRAMFS tristate - it is
> CRAMFS that can't be built in if MTD is modular.
Not exactly. CRAMFS still can be built irrespective of MTD. It is only
the XIP part of CRAMFS relying on MTD that is restricted.
Nicolas