diff mbox

[1/4] kconfig: introduce the "imply" keyword

Message ID 1476920573-14384-2-git-send-email-nicolas.pitre@linaro.org (mailing list archive)
State New, archived
Headers show

Commit Message

Nicolas Pitre Oct. 19, 2016, 11:42 p.m. UTC
The "imply" keyword is a weak version of "select" where the target
config symbol can still be turned off, avoiding those pitfalls that come
with the "select" keyword.

This is useful e.g. with multiple drivers that want to indicate their
ability to hook into a given subsystem while still being able to
configure that subsystem out and keep those drivers selected.

Currently, the same effect can almost be achieved with:

config DRIVER_A
	tristate

config DRIVER_B
	tristate

config DRIVER_C
	tristate

config DRIVER_D
	tristate

[...]

config SUBSYSTEM_X
	tristate
	default DRIVER_A || DRIVER_B || DRIVER_C || DRIVER_D || [...]

This is unwieldly to maintain especially with a large number of drivers.
Furthermore, there is no easy way to restrict the choice for SUBSYSTEM_X
to y or n, excluding m, when some drivers are built-in. The "select"
keyword allows for excluding m, but it excludes n as well. Hence
this "imply" keyword.  The above becomes:

config DRIVER_A
	tristate
	imply SUBSYSTEM_X

config DRIVER_B
	tristate
	imply SUBSYSTEM_X

[...]

config SUBSYSTEM_X
	tristate

This is much cleaner, and way more flexible than "select". SUBSYSTEM_X
can still be configured out, and it can be set as a module when none of
the drivers are selected or all of them are also modular.

Signed-off-by: Nicolas Pitre <nico@linaro.org>
---
 Documentation/kbuild/kconfig-language.txt | 28 ++++++++++++++++
 scripts/kconfig/expr.h                    |  2 ++
 scripts/kconfig/menu.c                    | 55 ++++++++++++++++++++++---------
 scripts/kconfig/symbol.c                  | 26 ++++++++++++++-
 scripts/kconfig/zconf.gperf               |  1 +
 scripts/kconfig/zconf.y                   | 16 +++++++--
 6 files changed, 109 insertions(+), 19 deletions(-)

Comments

Masahiro Yamada Oct. 20, 2016, 6:53 a.m. UTC | #1
2016-10-20 8:42 GMT+09:00 Nicolas Pitre <nicolas.pitre@linaro.org>:
> diff --git a/Documentation/kbuild/kconfig-language.txt b/Documentation/kbuild/kconfig-language.txt
> index 069fcb3eef..c96127f648 100644
> --- a/Documentation/kbuild/kconfig-language.txt
> +++ b/Documentation/kbuild/kconfig-language.txt
> @@ -113,6 +113,33 @@ applicable everywhere (see syntax).
>         That will limit the usefulness but on the other hand avoid
>         the illegal configurations all over.
>
> +- weak reverse dependencies: "imply" <symbol> ["if" <expr>]
> +  This is similar to "select" as it enforces a lower limit on another
> +  symbol except that the "implied" config symbol's value may still be
> +  set to n from a direct dependency or with a visible prompt.
> +  Given the following example:
> +
> +  config FOO
> +       tristate
> +       imply BAZ
> +
> +  config BAZ
> +       tristate
> +       depends on BAr


s/BAr/BAR/  ?
Edward Cree Oct. 20, 2016, 2:52 p.m. UTC | #2
On 20/10/16 00:42, Nicolas Pitre wrote:
> diff --git a/Documentation/kbuild/kconfig-language.txt b/Documentation/kbuild/kconfig-language.txt
> index 069fcb3eef..c96127f648 100644
> --- a/Documentation/kbuild/kconfig-language.txt
> +++ b/Documentation/kbuild/kconfig-language.txt
> @@ -113,6 +113,33 @@ applicable everywhere (see syntax).
>       That will limit the usefulness but on the other hand avoid
>       the illegal configurations all over.
>
> +- weak reverse dependencies: "imply" <symbol> ["if" <expr>]
> +  This is similar to "select" as it enforces a lower limit on another
> +  symbol except that the "implied" config symbol's value may still be
> +  set to n from a direct dependency or with a visible prompt.
> +  Given the following example:
> +
> +  config FOO
> +     tristate
> +     imply BAZ
> +
> +  config BAZ
> +     tristate
> +     depends on BAr
> +
> +  The following values are possible:
> +
> +     FOO             BAR             BAR's default   choice for BAZ
Should the third column not be "BAZ's default"?
> +     --------------- --------------- --------------- --------------
> +     n               y               n               N/m/y
> +     m               y               m               M/y/n
> +     y               y               y               Y/n
> +     y               n               *               N
Also, I don't think having any FOO=y should preclude BAZ=m.  Suppose both
FOO and FOO2 imply BAZ, FOO=y and FOO2=m.  Then if BAZ-features are only
desired for driver FOO2, BAz=m makes sense.
There is also the case of drivers with the ability to detect at runtime
whether BAZ is present, rather than making the decision at build time, but
I'm not sure how common that is.

-Ed
The information contained in this message is confidential and is intended for the addressee(s) only. If you have received this message in error, please notify the sender immediately and delete the message. Unless you are an addressee (or authorized to receive for an addressee), you may not use, copy or disclose to anyone this message or any information contained in this message. The unauthorized use, disclosure, copying or alteration of this message is strictly prohibited.
--
To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Josh Triplett Oct. 20, 2016, 3:38 p.m. UTC | #3
On Thu, Oct 20, 2016 at 03:52:04PM +0100, Edward Cree wrote:
> On 20/10/16 00:42, Nicolas Pitre wrote:
> > diff --git a/Documentation/kbuild/kconfig-language.txt b/Documentation/kbuild/kconfig-language.txt
> > index 069fcb3eef..c96127f648 100644
> > --- a/Documentation/kbuild/kconfig-language.txt
> > +++ b/Documentation/kbuild/kconfig-language.txt
> > @@ -113,6 +113,33 @@ applicable everywhere (see syntax).
> >       That will limit the usefulness but on the other hand avoid
> >       the illegal configurations all over.
> >
> > +- weak reverse dependencies: "imply" <symbol> ["if" <expr>]
> > +  This is similar to "select" as it enforces a lower limit on another
> > +  symbol except that the "implied" config symbol's value may still be
> > +  set to n from a direct dependency or with a visible prompt.
> > +  Given the following example:
> > +
> > +  config FOO
> > +     tristate
> > +     imply BAZ
> > +
> > +  config BAZ
> > +     tristate
> > +     depends on BAr
> > +
> > +  The following values are possible:
> > +
> > +     FOO             BAR             BAR's default   choice for BAZ
> Should the third column not be "BAZ's default"?
> > +     --------------- --------------- --------------- --------------
> > +     n               y               n               N/m/y
> > +     m               y               m               M/y/n
> > +     y               y               y               Y/n
> > +     y               n               *               N
> Also, I don't think having any FOO=y should preclude BAZ=m.  Suppose both
> FOO and FOO2 imply BAZ, FOO=y and FOO2=m.  Then if BAZ-features are only
> desired for driver FOO2, BAz=m makes sense.

That's exactly the problem that motivated "imply" in the first place:
while that's *possible*, it means the user needs to know that they're
breaking BAZ support for driver FOO.

In theory, someone could extend the UI to note the symbols with an
"imply" for a given symbol and provide additional help for the implied
symbol that explains the implications.  In that case, it might make
sense to allow the user to explicitly mark a symbol as 'm', with
appropriate explanations of the implications.  But in the absence of
that, the simple solution seems like preventing 'm' for a symbol implied
by a symbol marked as 'y'.
--
To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Nicolas Pitre Oct. 20, 2016, 5:04 p.m. UTC | #4
On Thu, 20 Oct 2016, Edward Cree wrote:

> On 20/10/16 00:42, Nicolas Pitre wrote:
> > diff --git a/Documentation/kbuild/kconfig-language.txt b/Documentation/kbuild/kconfig-language.txt
> > index 069fcb3eef..c96127f648 100644
> > --- a/Documentation/kbuild/kconfig-language.txt
> > +++ b/Documentation/kbuild/kconfig-language.txt
> > @@ -113,6 +113,33 @@ applicable everywhere (see syntax).
> >       That will limit the usefulness but on the other hand avoid
> >       the illegal configurations all over.
> >
> > +- weak reverse dependencies: "imply" <symbol> ["if" <expr>]
> > +  This is similar to "select" as it enforces a lower limit on another
> > +  symbol except that the "implied" config symbol's value may still be
> > +  set to n from a direct dependency or with a visible prompt.
> > +  Given the following example:
> > +
> > +  config FOO
> > +     tristate
> > +     imply BAZ
> > +
> > +  config BAZ
> > +     tristate
> > +     depends on BAr
> > +
> > +  The following values are possible:
> > +
> > +     FOO             BAR             BAR's default   choice for BAZ
> Should the third column not be "BAZ's default"?

Indeed.  Good catch.

> > +     --------------- --------------- --------------- --------------
> > +     n               y               n               N/m/y
> > +     m               y               m               M/y/n
> > +     y               y               y               Y/n
> > +     y               n               *               N
> Also, I don't think having any FOO=y should preclude BAZ=m.  Suppose both
> FOO and FOO2 imply BAZ, FOO=y and FOO2=m.

Some people didn't like the fact that you could turn a driver from m to 
y and silently lose some features if they were provided by a subsystem 
that also used to be m, which arguably is not the same as being 
explicitly disabled.  With "select" this is not a problem as the target 
symbol is also promoted to y in that case, so I wanted to preserve that 
property.

> Then if BAZ-features are only
> desired for driver FOO2, BAz=m makes sense.

In that case it would make more sense to add a config option related to 
FOO asking if BAZ features are desired for that driver (there is already 
one occurrence of that with PTP).  Or you could simply drop the "imply" 
statement from the FOO config entry.

> There is also the case of drivers with the ability to detect at runtime
> whether BAZ is present, rather than making the decision at build time, but
> I'm not sure how common that is.

Right now that's how PTP support is done.  Drivers can optimize things 
at build time, but most of them simply cope with a NULL return from 
ptp_clock_register().  Hence the imply statement becomes a big 
configuration hint rather than some hard build dependency.


Nicolas
--
To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Edward Cree Oct. 20, 2016, 5:41 p.m. UTC | #5
On 20/10/16 18:04, Nicolas Pitre wrote:
> On Thu, 20 Oct 2016, Edward Cree wrote:
>> Also, I don't think having any FOO=y should preclude BAZ=m.  Suppose both
>> FOO and FOO2 imply BAZ, FOO=y and FOO2=m.
> Some people didn't like the fact that you could turn a driver from m to
> y and silently lose some features if they were provided by a subsystem
> that also used to be m, which arguably is not the same as being
> explicitly disabled.  With "select" this is not a problem as the target
> symbol is also promoted to y in that case, so I wanted to preserve that
> property.
Right, but that's an argument for pushing the subsystem's default to y,
not for preventing changing the subsystem back to m afterwards.
>> Then if BAZ-features are only
>> desired for driver FOO2, BAz=m makes sense.
> In that case it would make more sense to add a config option related to
> FOO asking if BAZ features are desired for that driver (there is already
> one occurrence of that with PTP).  Or you could simply drop the "imply"
> statement from the FOO config entry.
But the desire is a property of the user, not of the driver.  If you're
willing to add CONFIG_FOO_BAZ to every combination of (driver, subsystem)
then "imply" becomes unnecessary, doesn't it?  Conversely, if you *don't*
want to have to do that, then "imply" needs to only ever deal in defaults,
not in limitations.
>> There is also the case of drivers with the ability to detect at runtime
>> whether BAZ is present, rather than making the decision at build time, but
>> I'm not sure how common that is.
> Right now that's how PTP support is done.  Drivers can optimize things
> at build time, but most of them simply cope with a NULL return from
> ptp_clock_register().  Hence the imply statement becomes a big
> configuration hint rather than some hard build dependency.
Right, so those drivers can use PTP if they're y and PTP is m, as long as
the PTP module is loaded when they probe.  But current "imply" semantics
won't allow that...

I think that Josh's suggestion (have the UI warn you if you set BAZ to m
while FOO=y) is the right approach, but I also think it should be done
now rather than at some unspecified future time.  Otherwise you forbid
potentially valid configs.

-Ed
The information contained in this message is confidential and is intended for the addressee(s) only. If you have received this message in error, please notify the sender immediately and delete the message. Unless you are an addressee (or authorized to receive for an addressee), you may not use, copy or disclose to anyone this message or any information contained in this message. The unauthorized use, disclosure, copying or alteration of this message is strictly prohibited.
--
To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Nicolas Pitre Oct. 20, 2016, 6:29 p.m. UTC | #6
On Thu, 20 Oct 2016, Edward Cree wrote:

> On 20/10/16 18:04, Nicolas Pitre wrote:
> > On Thu, 20 Oct 2016, Edward Cree wrote:
> >> Also, I don't think having any FOO=y should preclude BAZ=m.  Suppose both
> >> FOO and FOO2 imply BAZ, FOO=y and FOO2=m.
> > Some people didn't like the fact that you could turn a driver from m to
> > y and silently lose some features if they were provided by a subsystem
> > that also used to be m, which arguably is not the same as being
> > explicitly disabled.  With "select" this is not a problem as the target
> > symbol is also promoted to y in that case, so I wanted to preserve that
> > property.
> Right, but that's an argument for pushing the subsystem's default to y,
> not for preventing changing the subsystem back to m afterwards.
> >> Then if BAZ-features are only
> >> desired for driver FOO2, BAz=m makes sense.
> > In that case it would make more sense to add a config option related to
> > FOO asking if BAZ features are desired for that driver (there is already
> > one occurrence of that with PTP).  Or you could simply drop the "imply"
> > statement from the FOO config entry.
> But the desire is a property of the user, not of the driver.  If you're
> willing to add CONFIG_FOO_BAZ to every combination of (driver, subsystem)
> then "imply" becomes unnecessary, doesn't it?

Absolutely.  And if that's something that inspires you please be my 
guest.  So far, though, this apparently didn't inspire the majority of 
driver authors who preferred to have a smaller set of config options and 
forcefully pull in the BAZ features with a "select".  But "select" comes 
with its set of evils which "imply" is meant to overcome.

> Conversely, if you *don't*
> want to have to do that, then "imply" needs to only ever deal in defaults,
> not in limitations.

As I explained, It still has to prevent BAZ=m if FOO moves from m to y 
otherwise this would effectively have the same result as BAZ=n in 
practice and that is not what people expect if BAZ actually isn't n in 
your .config file.  That's why "select" also has that particular 
semantic.

Here "imply" is meant to be a weaker form of "select".  If you prefer 
not to have that limitation imposed by either "select" and "imply" then 
simply don't use them at all.  Nothing forces you to use any of them if 
your code can cope with any config combination.

In those cases where "imply" is used, you could drop it altogether 
already. But that's for driver authors to decide. If they went with 
"select" in the first place, there might be a reason, and "imply" is 
there to preserve that reason, semantically at least, without the 
handcuff effect that "select" imposes on the whole thing.

> >> There is also the case of drivers with the ability to detect at runtime
> >> whether BAZ is present, rather than making the decision at build time, but
> >> I'm not sure how common that is.
> > Right now that's how PTP support is done.  Drivers can optimize things
> > at build time, but most of them simply cope with a NULL return from
> > ptp_clock_register().  Hence the imply statement becomes a big
> > configuration hint rather than some hard build dependency.
> Right, so those drivers can use PTP if they're y and PTP is m, as long 
> as the PTP module is loaded when they probe.

Not at the moment. There is no way for PTP to dynamically signal to 
interested drivers its presence at run time.  And drivers, when 
built-in, typically probe their hardware during the boot process even 
before you have the chance to load any module. If that ever changes, 
then the imply or select statement could simply be dropped.

> But current "imply" semantics won't allow that...

And that's on purpose.

> I think that Josh's suggestion (have the UI warn you if you set BAZ to m
> while FOO=y) is the right approach, but I also think it should be done
> now rather than at some unspecified future time.

Please advocate this with kconfig UI authors.  My recursion stack is 
already quite deep.

> Otherwise you forbid
> potentially valid configs.

Like I said, if FOO=y and BAZ=m is a valid config, all you have to do is 
omit "imply BAZ" or "select BAZ" from the FOO config entry.  It's as 
simple as that.


Nicolas
--
To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Edward Cree Oct. 20, 2016, 7:09 p.m. UTC | #7
On 20/10/16 19:29, Nicolas Pitre wrote:
> On Thu, 20 Oct 2016, Edward Cree wrote:
>> But the desire is a property of the user, not of the driver.  If you're
>> willing to add CONFIG_FOO_BAZ to every combination of (driver, subsystem)
>> then "imply" becomes unnecessary, doesn't it?
> Absolutely.  And if that's something that inspires you please be my
> guest.  So far, though, this apparently didn't inspire the majority of
> driver authors who preferred to have a smaller set of config options and
> forcefully pull in the BAZ features with a "select".  But "select" comes
> with its set of evils which "imply" is meant to overcome.
It really doesn't inspire me either ;)
I was using it as a way to set up the converse, rather than as any kind of
serious suggestion.
And I agree that "imply", as it stands, is an improvement over "select" for
these kinds of cases.  I just think it could be further improved.
>> Conversely, if you *don't*
>> want to have to do that, then "imply" needs to only ever deal in defaults,
>> not in limitations.
> As I explained, It still has to prevent BAZ=m if FOO moves from m to y
> otherwise this would effectively have the same result as BAZ=n in
> practice and that is not what people expect if BAZ actually isn't n in
> your .config file.  That's why "select" also has that particular
> semantic.
If FOO "moves from" m to y (by which I'm assuming you mean a call to
sym_set_tristate_value()), then by all means set BAZ=y.  But the user
should then still be allowed to move BAZ from y to m without having to
change FOO (though hopefully they will get warned about it somehow).
> Here "imply" is meant to be a weaker form of "select".  If you prefer
> not to have that limitation imposed by either "select" and "imply" then
> simply don't use them at all.  Nothing forces you to use any of them if
> your code can cope with any config combination.
I'm interpreting "imply" as being more a way of saying "if you want FOO you
probably want BAZ as well".  But maybe that should be yet another new
keyword if it's so different from what you want "imply" to be.  "suggests",
perhaps.
>> Right, so those drivers can use PTP if they're y and PTP is m, as long
>> as the PTP module is loaded when they probe.
> Not at the moment. There is no way for PTP to dynamically signal to
> interested drivers its presence at run time.  And drivers, when
> built-in, typically probe their hardware during the boot process even
> before you have the chance to load any module. If that ever changes,
> then the imply or select statement could simply be dropped.
At least for PCIe devices, the driver can be un- and re-bound (despite
being built-in) through sysfs.  So you (already) can re-probe them after
loading PTP.  So driver=y && PTP=m is valid, today.
>> I think that Josh's suggestion (have the UI warn you if you set BAZ to m
>> while FOO=y) is the right approach, but I also think it should be done
>> now rather than at some unspecified future time.
> Please advocate this with kconfig UI authors.  My recursion stack is
> already quite deep.
If I'm reading your patch correctly, your symbol.c additions enforce this
restriction, and AFAIK the UI can't override that by saying "Yeah I warned
the user and he said it was fine".
The kconfig UI API would need to change; sym_set_tristate_value() could
grow an 'override-imply' flag, for instance.

But I'm not married to the idea; I don't have a real-world use-case this
interferes with, so if you still think I'm wrong, feel free to ignore me :)

-Ed
The information contained in this message is confidential and is intended for the addressee(s) only. If you have received this message in error, please notify the sender immediately and delete the message. Unless you are an addressee (or authorized to receive for an addressee), you may not use, copy or disclose to anyone this message or any information contained in this message. The unauthorized use, disclosure, copying or alteration of this message is strictly prohibited.
--
To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Nicolas Pitre Oct. 20, 2016, 8:10 p.m. UTC | #8
On Thu, 20 Oct 2016, Edward Cree wrote:

> On 20/10/16 19:29, Nicolas Pitre wrote:
> > On Thu, 20 Oct 2016, Edward Cree wrote:
> >> But the desire is a property of the user, not of the driver.  If you're
> >> willing to add CONFIG_FOO_BAZ to every combination of (driver, subsystem)
> >> then "imply" becomes unnecessary, doesn't it?
> > Absolutely.  And if that's something that inspires you please be my
> > guest.  So far, though, this apparently didn't inspire the majority of
> > driver authors who preferred to have a smaller set of config options and
> > forcefully pull in the BAZ features with a "select".  But "select" comes
> > with its set of evils which "imply" is meant to overcome.
> It really doesn't inspire me either ;)
> I was using it as a way to set up the converse, rather than as any kind of
> serious suggestion.
> And I agree that "imply", as it stands, is an improvement over "select" for
> these kinds of cases.  I just think it could be further improved.
> >> Conversely, if you *don't*
> >> want to have to do that, then "imply" needs to only ever deal in defaults,
> >> not in limitations.
> > As I explained, It still has to prevent BAZ=m if FOO moves from m to y
> > otherwise this would effectively have the same result as BAZ=n in
> > practice and that is not what people expect if BAZ actually isn't n in
> > your .config file.  That's why "select" also has that particular
> > semantic.
> If FOO "moves from" m to y (by which I'm assuming you mean a call to
> sym_set_tristate_value()), then by all means set BAZ=y.  But the user
> should then still be allowed to move BAZ from y to m without having to
> change FOO (though hopefully they will get warned about it somehow).

The case I'm most worried about is:

- open .config in $EDITOR
- change "CONFIG_FOO=m" to "CONFIG_FOO=y"
- save and exit
- make

The warning is most likely to be missed in that case, and if .config 
doesn't list CONFIG_BAZ as unset then this is likely to confuse people.

> > Here "imply" is meant to be a weaker form of "select".  If you prefer
> > not to have that limitation imposed by either "select" and "imply" then
> > simply don't use them at all.  Nothing forces you to use any of them if
> > your code can cope with any config combination.
> I'm interpreting "imply" as being more a way of saying "if you want FOO you
> probably want BAZ as well".  But maybe that should be yet another new
> keyword if it's so different from what you want "imply" to be.  "suggests",
> perhaps.

Indeed. That's exactly the keyword that came to my mind after I sent my 
previous reply.

> >> Right, so those drivers can use PTP if they're y and PTP is m, as long
> >> as the PTP module is loaded when they probe.
> > Not at the moment. There is no way for PTP to dynamically signal to
> > interested drivers its presence at run time.  And drivers, when
> > built-in, typically probe their hardware during the boot process even
> > before you have the chance to load any module. If that ever changes,
> > then the imply or select statement could simply be dropped.
> At least for PCIe devices, the driver can be un- and re-bound (despite
> being built-in) through sysfs.  So you (already) can re-probe them after
> loading PTP.  So driver=y && PTP=m is valid, today.

I agree with the principle, but the implementation just doesn't allow it 
at the moment. In a simplified form, what's there now in ptp.h is:

#if IS_REACHABLE(CONFIG_PTP)
extern struct ptp_clock *ptp_clock_register(...);
#else
static inline struct ptp_clock *ptp_clock_register(...) { return NULL; }
#endif

i.e. drivers may cope at runtime with PTP being absent, but there is no 
support for PTP to be loaded after drivers referring to it. Hence the 
use of "select" that I simply converted to "imply".

> >> I think that Josh's suggestion (have the UI warn you if you set BAZ to m
> >> while FOO=y) is the right approach, but I also think it should be done
> >> now rather than at some unspecified future time.
> > Please advocate this with kconfig UI authors.  My recursion stack is
> > already quite deep.
> If I'm reading your patch correctly, your symbol.c additions enforce this
> restriction, and AFAIK the UI can't override that by saying "Yeah I warned
> the user and he said it was fine".
> The kconfig UI API would need to change; sym_set_tristate_value() could
> grow an 'override-imply' flag, for instance.

The UI may tell the user about the restriction and suggest a way to 
overcome it by also changing the "impliers" to m.

Otherwise I much prefer to have a kconfig keyword that expresses the 
fact that it is actually OK to override it, like this "suggests" idea 
could do.

Sidenote: the kconfig language isn't coherent wrt english grammar as 
there is "depends on" but "select" rather than "selects". I interpreted 
"select" as using the imperative mood, hence the addition of "imply" 
rather than "implies".


Nicolas
--
To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/Documentation/kbuild/kconfig-language.txt b/Documentation/kbuild/kconfig-language.txt
index 069fcb3eef..c96127f648 100644
--- a/Documentation/kbuild/kconfig-language.txt
+++ b/Documentation/kbuild/kconfig-language.txt
@@ -113,6 +113,33 @@  applicable everywhere (see syntax).
 	That will limit the usefulness but on the other hand avoid
 	the illegal configurations all over.
 
+- weak reverse dependencies: "imply" <symbol> ["if" <expr>]
+  This is similar to "select" as it enforces a lower limit on another
+  symbol except that the "implied" config symbol's value may still be
+  set to n from a direct dependency or with a visible prompt.
+  Given the following example:
+
+  config FOO
+	tristate
+	imply BAZ
+
+  config BAZ
+	tristate
+	depends on BAr
+
+  The following values are possible:
+
+	FOO		BAR		BAR's default	choice for BAZ
+	---------------	---------------	---------------	--------------
+	n		y		n		N/m/y
+	m		y		m		M/y/n
+	y		y		y		Y/n
+	y		n		*		N
+
+  This is useful e.g. with multiple drivers that want to indicate their
+  ability to hook into a given subsystem while still being able to
+  configure that subsystem out and keep those drivers selected.
+
 - limiting menu display: "visible if" <expr>
   This attribute is only applicable to menu blocks, if the condition is
   false, the menu block is not displayed to the user (the symbols
@@ -481,6 +508,7 @@  historical issues resolved through these different solutions.
   b) Match dependency semantics:
 	b1) Swap all "select FOO" to "depends on FOO" or,
 	b2) Swap all "depends on FOO" to "select FOO"
+  c) Consider the use of "imply" instead of "select"
 
 The resolution to a) can be tested with the sample Kconfig file
 Documentation/kbuild/Kconfig.recursion-issue-01 through the removal
diff --git a/scripts/kconfig/expr.h b/scripts/kconfig/expr.h
index 973b6f7333..a73f762c48 100644
--- a/scripts/kconfig/expr.h
+++ b/scripts/kconfig/expr.h
@@ -85,6 +85,7 @@  struct symbol {
 	struct property *prop;
 	struct expr_value dir_dep;
 	struct expr_value rev_dep;
+	struct expr_value implied;
 };
 
 #define for_all_symbols(i, sym) for (i = 0; i < SYMBOL_HASHSIZE; i++) for (sym = symbol_hash[i]; sym; sym = sym->next) if (sym->type != S_OTHER)
@@ -136,6 +137,7 @@  enum prop_type {
 	P_DEFAULT,  /* default y */
 	P_CHOICE,   /* choice value */
 	P_SELECT,   /* select BAR */
+	P_IMPLY,    /* imply BAR */
 	P_RANGE,    /* range 7..100 (for a symbol) */
 	P_ENV,      /* value from environment variable */
 	P_SYMBOL,   /* where a symbol is defined */
diff --git a/scripts/kconfig/menu.c b/scripts/kconfig/menu.c
index aed678e8a7..e9357931b4 100644
--- a/scripts/kconfig/menu.c
+++ b/scripts/kconfig/menu.c
@@ -233,6 +233,8 @@  static void sym_check_prop(struct symbol *sym)
 {
 	struct property *prop;
 	struct symbol *sym2;
+	char *use;
+
 	for (prop = sym->prop; prop; prop = prop->next) {
 		switch (prop->type) {
 		case P_DEFAULT:
@@ -252,18 +254,20 @@  static void sym_check_prop(struct symbol *sym)
 			}
 			break;
 		case P_SELECT:
+		case P_IMPLY:
+			use = prop->type == P_SELECT ? "select" : "imply";
 			sym2 = prop_get_symbol(prop);
 			if (sym->type != S_BOOLEAN && sym->type != S_TRISTATE)
 				prop_warn(prop,
-				    "config symbol '%s' uses select, but is "
-				    "not boolean or tristate", sym->name);
+				    "config symbol '%s' uses %s, but is "
+				    "not boolean or tristate", sym->name, use);
 			else if (sym2->type != S_UNKNOWN &&
 				 sym2->type != S_BOOLEAN &&
 				 sym2->type != S_TRISTATE)
 				prop_warn(prop,
-				    "'%s' has wrong type. 'select' only "
+				    "'%s' has wrong type. '%s' only "
 				    "accept arguments of boolean and "
-				    "tristate type", sym2->name);
+				    "tristate type", sym2->name, use);
 			break;
 		case P_RANGE:
 			if (sym->type != S_INT && sym->type != S_HEX)
@@ -333,6 +337,10 @@  void menu_finalize(struct menu *parent)
 					struct symbol *es = prop_get_symbol(prop);
 					es->rev_dep.expr = expr_alloc_or(es->rev_dep.expr,
 							expr_alloc_and(expr_alloc_symbol(menu->sym), expr_copy(dep)));
+				} else if (prop->type == P_IMPLY) {
+					struct symbol *es = prop_get_symbol(prop);
+					es->implied.expr = expr_alloc_or(es->implied.expr,
+							expr_alloc_and(expr_alloc_symbol(menu->sym), expr_copy(dep)));
 				}
 			}
 		}
@@ -612,13 +620,30 @@  static struct property *get_symbol_prop(struct symbol *sym)
 	return prop;
 }
 
+static void get_symbol_props_str(struct gstr *r, struct symbol *sym,
+				 enum prop_type tok, const char *prefix)
+{
+	bool hit = false;
+	struct property *prop;
+
+	for_all_properties(sym, prop, tok) {
+		if (!hit) {
+			str_append(r, prefix);
+			hit = true;
+		} else
+			str_printf(r, " && ");
+		expr_gstr_print(prop->expr, r);
+	}
+	if (hit)
+		str_append(r, "\n");
+}
+
 /*
  * head is optional and may be NULL
  */
 static void get_symbol_str(struct gstr *r, struct symbol *sym,
 		    struct list_head *head)
 {
-	bool hit;
 	struct property *prop;
 
 	if (sym && sym->name) {
@@ -648,22 +673,20 @@  static void get_symbol_str(struct gstr *r, struct symbol *sym,
 		}
 	}
 
-	hit = false;
-	for_all_properties(sym, prop, P_SELECT) {
-		if (!hit) {
-			str_append(r, "  Selects: ");
-			hit = true;
-		} else
-			str_printf(r, " && ");
-		expr_gstr_print(prop->expr, r);
-	}
-	if (hit)
-		str_append(r, "\n");
+	get_symbol_props_str(r, sym, P_SELECT, _("  Selects: "));
 	if (sym->rev_dep.expr) {
 		str_append(r, _("  Selected by: "));
 		expr_gstr_print(sym->rev_dep.expr, r);
 		str_append(r, "\n");
 	}
+
+	get_symbol_props_str(r, sym, P_IMPLY, _("  Implies: "));
+	if (sym->implied.expr) {
+		str_append(r, _("  Implied by: "));
+		expr_gstr_print(sym->implied.expr, r);
+		str_append(r, "\n");
+	}
+
 	str_append(r, "\n\n");
 }
 
diff --git a/scripts/kconfig/symbol.c b/scripts/kconfig/symbol.c
index 2432298487..074fb66d9a 100644
--- a/scripts/kconfig/symbol.c
+++ b/scripts/kconfig/symbol.c
@@ -258,6 +258,15 @@  static void sym_calc_visibility(struct symbol *sym)
 		sym->rev_dep.tri = tri;
 		sym_set_changed(sym);
 	}
+	tri = no;
+	if (sym->implied.expr)
+		tri = expr_calc_value(sym->implied.expr);
+	if (tri == mod && sym_get_type(sym) == S_BOOLEAN)
+		tri = yes;
+	if (sym->implied.tri != tri) {
+		sym->implied.tri = tri;
+		sym_set_changed(sym);
+	}
 }
 
 /*
@@ -397,6 +406,12 @@  void sym_calc_value(struct symbol *sym)
 					newval.tri = EXPR_AND(expr_calc_value(prop->expr),
 							      prop->visible.tri);
 				}
+				if (sym->dir_dep.tri != no &&
+				    sym->implied.tri != no) {
+					/* implied symbols have implied defaults */
+					sym->flags |= SYMBOL_WRITE;
+					newval.tri = EXPR_OR(newval.tri, sym->implied.tri);
+				}
 			}
 		calc_newval:
 			if (sym->dir_dep.tri == no && sym->rev_dep.tri != no) {
@@ -413,7 +428,8 @@  void sym_calc_value(struct symbol *sym)
 			}
 			newval.tri = EXPR_OR(newval.tri, sym->rev_dep.tri);
 		}
-		if (newval.tri == mod && sym_get_type(sym) == S_BOOLEAN)
+		if (newval.tri == mod &&
+		    (sym_get_type(sym) == S_BOOLEAN || sym->implied.tri == yes))
 			newval.tri = yes;
 		break;
 	case S_STRING:
@@ -498,6 +514,8 @@  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;
@@ -750,6 +768,10 @@  const char *sym_get_string_default(struct symbol *sym)
 	if (sym->type == S_BOOLEAN && val == mod)
 		val = yes;
 
+	/* adjust the default value if this symbol is implied by another */
+	if (val < sym->implied.tri)
+		val = sym->implied.tri;
+
 	switch (sym->type) {
 	case S_BOOLEAN:
 	case S_TRISTATE:
@@ -1352,6 +1374,8 @@  const char *prop_get_type_name(enum prop_type type)
 		return "choice";
 	case P_SELECT:
 		return "select";
+	case P_IMPLY:
+		return "imply";
 	case P_RANGE:
 		return "range";
 	case P_SYMBOL:
diff --git a/scripts/kconfig/zconf.gperf b/scripts/kconfig/zconf.gperf
index ac498f01b4..ead02edec9 100644
--- a/scripts/kconfig/zconf.gperf
+++ b/scripts/kconfig/zconf.gperf
@@ -38,6 +38,7 @@  int,		T_TYPE,		TF_COMMAND, S_INT
 hex,		T_TYPE,		TF_COMMAND, S_HEX
 string,		T_TYPE,		TF_COMMAND, S_STRING
 select,		T_SELECT,	TF_COMMAND
+imply,		T_IMPLY,	TF_COMMAND
 range,		T_RANGE,	TF_COMMAND
 visible,	T_VISIBLE,	TF_COMMAND
 option,		T_OPTION,	TF_COMMAND
diff --git a/scripts/kconfig/zconf.y b/scripts/kconfig/zconf.y
index 71bf8bff69..001305fa08 100644
--- a/scripts/kconfig/zconf.y
+++ b/scripts/kconfig/zconf.y
@@ -31,7 +31,7 @@  struct symbol *symbol_hash[SYMBOL_HASHSIZE];
 static struct menu *current_menu, *current_entry;
 
 %}
-%expect 30
+%expect 32
 
 %union
 {
@@ -62,6 +62,7 @@  static struct menu *current_menu, *current_entry;
 %token <id>T_TYPE
 %token <id>T_DEFAULT
 %token <id>T_SELECT
+%token <id>T_IMPLY
 %token <id>T_RANGE
 %token <id>T_VISIBLE
 %token <id>T_OPTION
@@ -124,7 +125,7 @@  stmt_list:
 ;
 
 option_name:
-	T_DEPENDS | T_PROMPT | T_TYPE | T_SELECT | T_OPTIONAL | T_RANGE | T_DEFAULT | T_VISIBLE
+	T_DEPENDS | T_PROMPT | T_TYPE | T_SELECT | T_IMPLY | T_OPTIONAL | T_RANGE | T_DEFAULT | T_VISIBLE
 ;
 
 common_stmt:
@@ -216,6 +217,12 @@  config_option: T_SELECT T_WORD if_expr T_EOL
 	printd(DEBUG_PARSE, "%s:%d:select\n", zconf_curname(), zconf_lineno());
 };
 
+config_option: T_IMPLY T_WORD if_expr T_EOL
+{
+	menu_add_symbol(P_IMPLY, sym_lookup($2, 0), $3);
+	printd(DEBUG_PARSE, "%s:%d:imply\n", zconf_curname(), zconf_lineno());
+};
+
 config_option: T_RANGE symbol symbol if_expr T_EOL
 {
 	menu_add_expr(P_RANGE, expr_alloc_comp(E_RANGE,$2, $3), $4);
@@ -664,6 +671,11 @@  static void print_symbol(FILE *out, struct menu *menu)
 			expr_fprint(prop->expr, out);
 			fputc('\n', out);
 			break;
+		case P_IMPLY:
+			fputs( "  imply ", out);
+			expr_fprint(prop->expr, out);
+			fputc('\n', out);
+			break;
 		case P_RANGE:
 			fputs( "  range ", out);
 			expr_fprint(prop->expr, out);