The 'imply' keyword restricts a symbol to y or n, excluding m
when it is implied by y. This is the original behavior since
commit 237e3ad0f195 ("Kconfig: Introduce the "imply" keyword").
However, the author of the 'imply' keyword, Nicolas Pitre, stated
that the 'imply' keyword should not impose any restrictions. [1]
I agree, and want to get rid of this tricky behavior.
[1]: https://lkml.org/lkml/2020/2/19/714
Suggested-by: Nicolas Pitre <[email protected]>
Signed-off-by: Masahiro Yamada <[email protected]>
---
Documentation/kbuild/kconfig-language.rst | 12 +++++++++++-
scripts/kconfig/symbol.c | 5 +----
2 files changed, 12 insertions(+), 5 deletions(-)
diff --git a/Documentation/kbuild/kconfig-language.rst b/Documentation/kbuild/kconfig-language.rst
index d0111dd26410..d4d988aea679 100644
--- a/Documentation/kbuild/kconfig-language.rst
+++ b/Documentation/kbuild/kconfig-language.rst
@@ -173,7 +173,7 @@ applicable everywhere (see syntax).
=== === ============= ==============
n y n N/m/y
m y m M/y/n
- y y y Y/n
+ y y y Y/m/n
y n * N
=== === ============= ==============
@@ -181,6 +181,16 @@ applicable everywhere (see syntax).
ability to hook into a secondary subsystem while allowing the user to
configure that subsystem out without also having to unset these drivers.
+ Note: If the combination of FOO=y and BAR=m causes a link error,
+ you can guard the function call with IS_REACHABLE()::
+
+ foo_init()
+ {
+ if (IS_REACHABLE(CONFIG_BAZ))
+ baz_register(&foo);
+ ...
+ }
+
- limiting menu display: "visible if" <expr>
This attribute is only applicable to menu blocks, if the condition is
diff --git a/scripts/kconfig/symbol.c b/scripts/kconfig/symbol.c
index 8d38b700b314..b101ef3c377a 100644
--- a/scripts/kconfig/symbol.c
+++ b/scripts/kconfig/symbol.c
@@ -401,8 +401,7 @@ void sym_calc_value(struct symbol *sym)
sym_warn_unmet_dep(sym);
newval.tri = EXPR_OR(newval.tri, sym->rev_dep.tri);
}
- if (newval.tri == mod &&
- (sym_get_type(sym) == S_BOOLEAN || sym->implied.tri == yes))
+ if (newval.tri == mod && sym_get_type(sym) == S_BOOLEAN)
newval.tri = yes;
break;
case S_STRING:
@@ -484,8 +483,6 @@ bool sym_tristate_within_range(struct symbol *sym, tristate val)
return false;
if (sym->visible <= sym->rev_dep.tri)
return false;
- if (sym->implied.tri == yes && val == mod)
- return false;
if (sym_is_choice_value(sym) && sym->visible == yes)
return val == yes;
return val >= sym->rev_dep.tri && val <= sym->visible;
--
2.17.1
The 'imply' statement may create unmet direct dependency when the
implied symbol depends on m.
[Test Code]
config FOO
tristate "foo"
imply BAZ
config BAZ
tristate "baz"
depends on BAR
config BAR
def_tristate m
config MODULES
def_bool y
option modules
If you set FOO=y, BAZ is also promoted to y, which results in the
following .config file:
CONFIG_FOO=y
CONFIG_BAZ=y
CONFIG_BAR=m
CONFIG_MODULES=y
This does not meet the dependency 'BAZ depends on BAR'.
Unlike 'select', what is worse, Kconfig never shows the
'WARNING: unmet direct dependencies detected for ...' for this case.
Because 'imply' is considered to be weaker than 'depends on', Kconfig
should take the direct dependency into account.
For clarification, describe this case in kconfig-language.rst too.
Signed-off-by: Masahiro Yamada <[email protected]>
---
Documentation/kbuild/kconfig-language.rst | 7 +++++--
scripts/kconfig/symbol.c | 4 +++-
2 files changed, 8 insertions(+), 3 deletions(-)
diff --git a/Documentation/kbuild/kconfig-language.rst b/Documentation/kbuild/kconfig-language.rst
index d4d988aea679..68719e78ff85 100644
--- a/Documentation/kbuild/kconfig-language.rst
+++ b/Documentation/kbuild/kconfig-language.rst
@@ -159,11 +159,11 @@ applicable everywhere (see syntax).
Given the following example::
config FOO
- tristate
+ tristate "foo"
imply BAZ
config BAZ
- tristate
+ tristate "baz"
depends on BAR
The following values are possible:
@@ -174,6 +174,9 @@ applicable everywhere (see syntax).
n y n N/m/y
m y m M/y/n
y y y Y/m/n
+ n m n N/m
+ m m m M/n
+ y m n M/n
y n * N
=== === ============= ==============
diff --git a/scripts/kconfig/symbol.c b/scripts/kconfig/symbol.c
index b101ef3c377a..3dc81397d003 100644
--- a/scripts/kconfig/symbol.c
+++ b/scripts/kconfig/symbol.c
@@ -221,7 +221,7 @@ static void sym_calc_visibility(struct symbol *sym)
sym_set_changed(sym);
}
tri = no;
- if (sym->implied.expr && sym->dir_dep.tri != no)
+ if (sym->implied.expr)
tri = expr_calc_value(sym->implied.expr);
if (tri == mod && sym_get_type(sym) == S_BOOLEAN)
tri = yes;
@@ -394,6 +394,8 @@ void sym_calc_value(struct symbol *sym)
if (sym->implied.tri != no) {
sym->flags |= SYMBOL_WRITE;
newval.tri = EXPR_OR(newval.tri, sym->implied.tri);
+ newval.tri = EXPR_AND(newval.tri,
+ sym->dir_dep.tri);
}
}
calc_newval:
--
2.17.1
On Mon, 2 Mar 2020, Masahiro Yamada wrote:
> The 'imply' keyword restricts a symbol to y or n, excluding m
> when it is implied by y. This is the original behavior since
> commit 237e3ad0f195 ("Kconfig: Introduce the "imply" keyword").
>
> However, the author of the 'imply' keyword, Nicolas Pitre, stated
> that the 'imply' keyword should not impose any restrictions. [1]
>
> I agree, and want to get rid of this tricky behavior.
>
> [1]: https://lkml.org/lkml/2020/2/19/714
>
> Suggested-by: Nicolas Pitre <[email protected]>
> Signed-off-by: Masahiro Yamada <[email protected]>
Acked-by: Nicolas Pitre <[email protected]>
In addition to the IS_REACHABLE() note, it might be a good idea to
suggest adding an "imply" reference to the dependency if the feature
provided by BAZ is highli desirable, e.g.:
config FOO
tristate
imply BAR
imply BAZ
> ---
>
> Documentation/kbuild/kconfig-language.rst | 12 +++++++++++-
> scripts/kconfig/symbol.c | 5 +----
> 2 files changed, 12 insertions(+), 5 deletions(-)
>
> diff --git a/Documentation/kbuild/kconfig-language.rst b/Documentation/kbuild/kconfig-language.rst
> index d0111dd26410..d4d988aea679 100644
> --- a/Documentation/kbuild/kconfig-language.rst
> +++ b/Documentation/kbuild/kconfig-language.rst
> @@ -173,7 +173,7 @@ applicable everywhere (see syntax).
> === === ============= ==============
> n y n N/m/y
> m y m M/y/n
> - y y y Y/n
> + y y y Y/m/n
> y n * N
> === === ============= ==============
>
> @@ -181,6 +181,16 @@ applicable everywhere (see syntax).
> ability to hook into a secondary subsystem while allowing the user to
> configure that subsystem out without also having to unset these drivers.
>
> + Note: If the combination of FOO=y and BAR=m causes a link error,
> + you can guard the function call with IS_REACHABLE()::
> +
> + foo_init()
> + {
> + if (IS_REACHABLE(CONFIG_BAZ))
> + baz_register(&foo);
> + ...
> + }
> +
> - limiting menu display: "visible if" <expr>
>
> This attribute is only applicable to menu blocks, if the condition is
> diff --git a/scripts/kconfig/symbol.c b/scripts/kconfig/symbol.c
> index 8d38b700b314..b101ef3c377a 100644
> --- a/scripts/kconfig/symbol.c
> +++ b/scripts/kconfig/symbol.c
> @@ -401,8 +401,7 @@ void sym_calc_value(struct symbol *sym)
> sym_warn_unmet_dep(sym);
> newval.tri = EXPR_OR(newval.tri, sym->rev_dep.tri);
> }
> - if (newval.tri == mod &&
> - (sym_get_type(sym) == S_BOOLEAN || sym->implied.tri == yes))
> + if (newval.tri == mod && sym_get_type(sym) == S_BOOLEAN)
> newval.tri = yes;
> break;
> case S_STRING:
> @@ -484,8 +483,6 @@ bool sym_tristate_within_range(struct symbol *sym, tristate val)
> return false;
> if (sym->visible <= sym->rev_dep.tri)
> return false;
> - if (sym->implied.tri == yes && val == mod)
> - return false;
> if (sym_is_choice_value(sym) && sym->visible == yes)
> return val == yes;
> return val >= sym->rev_dep.tri && val <= sym->visible;
> --
> 2.17.1
>
>
On Mon, 2 Mar 2020, Masahiro Yamada wrote:
> The 'imply' statement may create unmet direct dependency when the
> implied symbol depends on m.
>
> [Test Code]
>
> config FOO
> tristate "foo"
> imply BAZ
>
> config BAZ
> tristate "baz"
> depends on BAR
>
> config BAR
> def_tristate m
>
> config MODULES
> def_bool y
> option modules
>
> If you set FOO=y, BAZ is also promoted to y, which results in the
> following .config file:
>
> CONFIG_FOO=y
> CONFIG_BAZ=y
> CONFIG_BAR=m
> CONFIG_MODULES=y
>
> This does not meet the dependency 'BAZ depends on BAR'.
>
> Unlike 'select', what is worse, Kconfig never shows the
> 'WARNING: unmet direct dependencies detected for ...' for this case.
>
> Because 'imply' is considered to be weaker than 'depends on', Kconfig
> should take the direct dependency into account.
>
> For clarification, describe this case in kconfig-language.rst too.
>
> Signed-off-by: Masahiro Yamada <[email protected]>
Acked-by: Nicolas Pitre <[email protected]>
> ---
>
> Documentation/kbuild/kconfig-language.rst | 7 +++++--
> scripts/kconfig/symbol.c | 4 +++-
> 2 files changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/kbuild/kconfig-language.rst b/Documentation/kbuild/kconfig-language.rst
> index d4d988aea679..68719e78ff85 100644
> --- a/Documentation/kbuild/kconfig-language.rst
> +++ b/Documentation/kbuild/kconfig-language.rst
> @@ -159,11 +159,11 @@ applicable everywhere (see syntax).
> Given the following example::
>
> config FOO
> - tristate
> + tristate "foo"
> imply BAZ
>
> config BAZ
> - tristate
> + tristate "baz"
> depends on BAR
>
> The following values are possible:
> @@ -174,6 +174,9 @@ applicable everywhere (see syntax).
> n y n N/m/y
> m y m M/y/n
> y y y Y/m/n
> + n m n N/m
> + m m m M/n
> + y m n M/n
> y n * N
> === === ============= ==============
>
> diff --git a/scripts/kconfig/symbol.c b/scripts/kconfig/symbol.c
> index b101ef3c377a..3dc81397d003 100644
> --- a/scripts/kconfig/symbol.c
> +++ b/scripts/kconfig/symbol.c
> @@ -221,7 +221,7 @@ static void sym_calc_visibility(struct symbol *sym)
> sym_set_changed(sym);
> }
> tri = no;
> - if (sym->implied.expr && sym->dir_dep.tri != no)
> + if (sym->implied.expr)
> tri = expr_calc_value(sym->implied.expr);
> if (tri == mod && sym_get_type(sym) == S_BOOLEAN)
> tri = yes;
> @@ -394,6 +394,8 @@ void sym_calc_value(struct symbol *sym)
> if (sym->implied.tri != no) {
> sym->flags |= SYMBOL_WRITE;
> newval.tri = EXPR_OR(newval.tri, sym->implied.tri);
> + newval.tri = EXPR_AND(newval.tri,
> + sym->dir_dep.tri);
> }
> }
> calc_newval:
> --
> 2.17.1
>
>
Hi Yamada-san,
On Mon, Mar 2, 2020 at 7:24 AM Masahiro Yamada <[email protected]> wrote:
> The 'imply' statement may create unmet direct dependency when the
> implied symbol depends on m.
>
> [Test Code]
>
> config FOO
> tristate "foo"
> imply BAZ
>
> config BAZ
> tristate "baz"
> depends on BAR
>
> config BAR
> def_tristate m
>
> config MODULES
> def_bool y
> option modules
>
> If you set FOO=y, BAZ is also promoted to y, which results in the
> following .config file:
>
> CONFIG_FOO=y
> CONFIG_BAZ=y
> CONFIG_BAR=m
> CONFIG_MODULES=y
>
> This does not meet the dependency 'BAZ depends on BAR'.
>
> Unlike 'select', what is worse, Kconfig never shows the
> 'WARNING: unmet direct dependencies detected for ...' for this case.
>
> Because 'imply' is considered to be weaker than 'depends on', Kconfig
> should take the direct dependency into account.
>
> For clarification, describe this case in kconfig-language.rst too.
>
> Signed-off-by: Masahiro Yamada <[email protected]>
This fixes some issue with "imply SND_SOC_WCD934X".
Tested-by: Geert Uytterhoeven <[email protected]>
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Hi Nicolas,
On Tue, Mar 3, 2020 at 12:31 AM Nicolas Pitre <[email protected]> wrote:
>
> On Mon, 2 Mar 2020, Masahiro Yamada wrote:
>
> > The 'imply' keyword restricts a symbol to y or n, excluding m
> > when it is implied by y. This is the original behavior since
> > commit 237e3ad0f195 ("Kconfig: Introduce the "imply" keyword").
> >
> > However, the author of the 'imply' keyword, Nicolas Pitre, stated
> > that the 'imply' keyword should not impose any restrictions. [1]
> >
> > I agree, and want to get rid of this tricky behavior.
> >
> > [1]: https://lkml.org/lkml/2020/2/19/714
> >
> > Suggested-by: Nicolas Pitre <[email protected]>
> > Signed-off-by: Masahiro Yamada <[email protected]>
>
> Acked-by: Nicolas Pitre <[email protected]>
>
> In addition to the IS_REACHABLE() note, it might be a good idea to
> suggest adding an "imply" reference to the dependency if the feature
> provided by BAZ is highli desirable, e.g.:
>
> config FOO
> tristate
> imply BAR
> imply BAZ
>
OK, this info is more relevant to the 2nd patch.
I folded this following to 2/2.
diff --git a/Documentation/kbuild/kconfig-language.rst
b/Documentation/kbuild/kconfig-language.rst
index 68719e78ff85..a1601ec3317b 100644
--- a/Documentation/kbuild/kconfig-language.rst
+++ b/Documentation/kbuild/kconfig-language.rst
@@ -194,6 +194,14 @@ applicable everywhere (see syntax).
...
}
+ Note: If the feature provided by BAZ is highly desirable for FOO,
+ FOO should imply not only BAZ, but also its dependency BAR::
+
+ config FOO
+ tristate "foo"
+ imply BAR
+ imply BAZ
+
- limiting menu display: "visible if" <expr>
This attribute is only applicable to menu blocks, if the condition is
Both applied.
> > ---
> >
> > Documentation/kbuild/kconfig-language.rst | 12 +++++++++++-
> > scripts/kconfig/symbol.c | 5 +----
> > 2 files changed, 12 insertions(+), 5 deletions(-)
> >
> > diff --git a/Documentation/kbuild/kconfig-language.rst b/Documentation/kbuild/kconfig-language.rst
> > index d0111dd26410..d4d988aea679 100644
> > --- a/Documentation/kbuild/kconfig-language.rst
> > +++ b/Documentation/kbuild/kconfig-language.rst
> > @@ -173,7 +173,7 @@ applicable everywhere (see syntax).
> > === === ============= ==============
> > n y n N/m/y
> > m y m M/y/n
> > - y y y Y/n
> > + y y y Y/m/n
> > y n * N
> > === === ============= ==============
> >
> > @@ -181,6 +181,16 @@ applicable everywhere (see syntax).
> > ability to hook into a secondary subsystem while allowing the user to
> > configure that subsystem out without also having to unset these drivers.
> >
> > + Note: If the combination of FOO=y and BAR=m causes a link error,
> > + you can guard the function call with IS_REACHABLE()::
> > +
> > + foo_init()
> > + {
> > + if (IS_REACHABLE(CONFIG_BAZ))
> > + baz_register(&foo);
> > + ...
> > + }
> > +
> > - limiting menu display: "visible if" <expr>
> >
> > This attribute is only applicable to menu blocks, if the condition is
> > diff --git a/scripts/kconfig/symbol.c b/scripts/kconfig/symbol.c
> > index 8d38b700b314..b101ef3c377a 100644
> > --- a/scripts/kconfig/symbol.c
> > +++ b/scripts/kconfig/symbol.c
> > @@ -401,8 +401,7 @@ void sym_calc_value(struct symbol *sym)
> > sym_warn_unmet_dep(sym);
> > newval.tri = EXPR_OR(newval.tri, sym->rev_dep.tri);
> > }
> > - if (newval.tri == mod &&
> > - (sym_get_type(sym) == S_BOOLEAN || sym->implied.tri == yes))
> > + if (newval.tri == mod && sym_get_type(sym) == S_BOOLEAN)
> > newval.tri = yes;
> > break;
> > case S_STRING:
> > @@ -484,8 +483,6 @@ bool sym_tristate_within_range(struct symbol *sym, tristate val)
> > return false;
> > if (sym->visible <= sym->rev_dep.tri)
> > return false;
> > - if (sym->implied.tri == yes && val == mod)
> > - return false;
> > if (sym_is_choice_value(sym) && sym->visible == yes)
> > return val == yes;
> > return val >= sym->rev_dep.tri && val <= sym->visible;
> > --
> > 2.17.1
> >
> >
--
Best Regards
Masahiro Yamada