Message ID | 20100713114322.57c8b166.sfr@canb.auug.org.au (mailing list archive) |
---|---|
State | Not Applicable |
Headers | show |
On 07/13/2010 03:43 AM, Stephen Rothwell wrote: > After this change, doing a "make xxx_defconfig" will check first for > a file called arch/<arch>/configs/Kconfig.xxx and use that to generate > the .config (effectively starting from an allnoconfig). If that file > doesn't exist, it will use arch/<ARCH>/configs/xxx_defconfig as now. > > Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au> > --- > scripts/kconfig/Makefile | 14 +++++++++++++- > 1 files changed, 13 insertions(+), 1 deletions(-) > > Hi Linus, > > Is this more the direction you want to take? > > There are still 2 main problems with is approach: > > - there are some config options that are globally and > unconditionally enabled that some platforms may not want. The only way > currently to turn them off is to reproduce the config entry with the > different default. I am not sure if we need a wa to turn them off or to > just change them to being neede to be selected by those that do want them. > - we have no way to select options that are neither bool or > tristate to suitable values. Again the only way to do that currently is > to reproduce the config entry with a different default value. Hi Stephen, how are these Kconfig.xxx files going look like? A list of 'select FOO' and 'include "Kconfig.other"' statements? What about adding support for an include statement to the .config file format and using that in the defconfigs instead? The VARIABLE=VALUE grammar seems much more suitable for this than the kconfig language. Michal
[cc'ing rmk and linux-arm-kernel] On Mon, Jul 12, 2010 at 7:43 PM, Stephen Rothwell <sfr@canb.auug.org.au> wrote: > After this change, doing a "make xxx_defconfig" will check first for > a file called arch/<arch>/configs/Kconfig.xxx and use that to generate > the .config (effectively starting from an allnoconfig). If that file > doesn't exist, it will use arch/<ARCH>/configs/xxx_defconfig as now. Oops, I hadn't seen this patch when I wrote mine this afternoon[1]. :-) A few minor differences, but essentially the same solution. [1] http://patchwork.ozlabs.org/patch/58823/ I chose to use -D /dev/null (defconfig from an empty file) instead of -n (allnoconfig) so that default values in Kconfig would get respected. For the benefit of everyone else, here's an excerpt from our IRC conversation this afternoon: 19:49 < gcl> sfr: [...] Your patch and my patch are essentially doing exactly the same thing, except that I used '-d' and you used '-n'. 19:50 < gcl> s/-d/-D/ 19:55 < sfr> right 19:55 < sfr> Linus wanted us to use -n 19:55 < sfr> because that way you get what you asked for, not what the defaults say ... 19:58 < gcl> I suppose we don't currently have a way to say "select FOO=n", so I suppose that makes sense 19:58 < gcl> although I think using the defaults unless told not to is a better approach in the long run 19:59 < gcl> most of the time I *don't want* to ask for something in the defconfig. :-) 20:00 < gcl> I just want TheBestOrCorrectAnswer to be chosen for me 20:04 < sfr> gcl: go read Linus' vision :-) > Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au> > --- > scripts/kconfig/Makefile | 14 +++++++++++++- > 1 files changed, 13 insertions(+), 1 deletions(-) > > Hi Linus, > > Is this more the direction you want to take? > > There are still 2 main problems with is approach: > > - there are some config options that are globally and > unconditionally enabled that some platforms may not want. The only way > currently to turn them off is to reproduce the config entry with the > different default. I am not sure if we need a wa to turn them off or to > just change them to being neede to be selected by those that do want them. > - we have no way to select options that are neither bool or > tristate to suitable values. Again the only way to do that currently is > to reproduce the config entry with a different default value. For both of the above problems, what if we added syntax like the following to Kconfig? config FOO bool select BAR = n select FOO_VALUE = 54 > I am currently working towards using this to recreate the PowerPC > defconfigs, but it is a slow process as they have some much stuff enabled > in them and some of it is probably actually not relevant. If the trimmed configs are merged, then there is no rush on this. We can keep them around and switch them over as people want to make changes. > This process is made easier by the recent commit "kbuild: Warn on > selecting symbols with unmet direct dependencies" that is in the kbuild > tree (and linux-next). Ah, I didn't know that change was being merged. That indeed makes things easier, and eliminates the post-test that I do to make sure that the resulting config is actually valid. > diff --git a/scripts/kconfig/Makefile b/scripts/kconfig/Makefile > index 7ea649d..1ab8f45 100644 > --- a/scripts/kconfig/Makefile > +++ b/scripts/kconfig/Makefile > @@ -117,9 +117,21 @@ else > $(Q)$< -D arch/$(SRCARCH)/configs/$(KBUILD_DEFCONFIG) $(Kconfig) > endif > > -%_defconfig: $(obj)/conf > +configs_dir := $(srctree)/arch/$(SRCARCH)/configs > +# We check a level of sub directories because arch/powerpc > +# has its defconfig files arranged that way > +old_defconfigs := $(patsubst $(configs_dir)/%,%,\ > + $(wildcard $(configs_dir)/*_defconfig) \ > + $(wildcard $(configs_dir)/*/*_defconfig)) > +defconfigs := $(patsubst $(configs_dir)/Kconfig.%,%_defconfig,\ > + $(wildcard $(configs_dir)/Kconfig.*)) > + Ugh. My first impression is that all this shouldn't be necessary, and it should be okay to just make the two following rules include a pattern dependency for the source file. However, as I play with it, I cannot seem to get the rules right to handle the sub directories. The $(defconfigs) patsubst at least could be done solely with dependencies on the target rule if the files were named *.Kconfig instead of Kconfig.* (because subdirectories are not handled in that case). For example: %_defconfig: $(obj)/conf arch/$(SRCARCH)/configs/%.Kconfig $(Q)$< -n arch/$(SRCARCH)/configs/$*.Kconfig > +$(old_defconfigs): %_defconfig: $(obj)/conf > $(Q)$< -D arch/$(SRCARCH)/configs/$@ $(Kconfig) > > +$(defconfigs): %_defconfig: $(obj)/conf > + $(Q)$< -n arch/$(SRCARCH)/configs/Kconfig.$* > + > # Help text used by make help > help: > @echo ' config - Update current config utilising a line-oriented program' Cheers, g.
On Tue, Jul 13, 2010 at 7:26 PM, Grant Likely <grant.likely@secretlab.ca> wrote: > > I chose to use -D /dev/null (defconfig from an empty file) instead of > -n (allnoconfig) so that default values in Kconfig would get > respected. For the benefit of everyone else, here's an excerpt from > our IRC conversation this afternoon: > > 19:49 < gcl> sfr: [...] Your patch and my patch are > essentially doing exactly the same thing, except that I used '-d' > and you used '-n'. > 19:50 < gcl> s/-d/-D/ > 19:55 < sfr> right > 19:55 < sfr> Linus wanted us to use -n Just a note: Linus doesn't really care. IOW, I used -n not because of any fundamental belief that it is correct, but just because ti happened to be how I happened to decide to solve it. It's entirely possible that starting from the Kconfig defaults (rather than "no") is the right way to go. I think either approach is likely fine. The -D /dev/null approach would presumably give smaller Kconfig.xyz files, assuming our defaults are sane (an maybe that could be at least a partial validation of the defaults we do have). While the -n approach is in some ways more stable, in that the resulting Kconfig.xyz entires would presumably be more stand-alone, and there wouldn't be any subtle interactions when somebody changes a default value int he Kconfig file. So I can see advantages to either model. And either model clearly would want the improvements to "select" that are independent (ie warn about unsatisfied 'depends on' constraints, and select recursively. Maybe we already fixed the recursive select thing, I forget). I also think we need to allow setting of actual values. I don't know what the syntax would be. A "set" statement that overrides a default in the Kconfig file, so that you can do set NODES_SHIFT 10 which would basically be equivalent to a "select" of a tristate variable, but instead set the actual value? I dunno. And quite frankly, maybe somebody comes up with a better model entirely. I like the Kconfig.xyz model, in that it should be human-readable/writable and shouldn't introduce any fundamentally new concepts (except the fairly simple extensions to the Kconfig language), but maybe there are better models. Regardless, I don't have anything against either set of patches (Grant's or Stephen's). Linus
On Tue, Jul 13, 2010 at 10:04 PM, Linus Torvalds <torvalds@linux-foundation.org> wrote: > On Tue, Jul 13, 2010 at 7:26 PM, Grant Likely <grant.likely@secretlab.ca> wrote: >> >> I chose to use -D /dev/null (defconfig from an empty file) instead of >> -n (allnoconfig) so that default values in Kconfig would get >> respected. For the benefit of everyone else, here's an excerpt from >> our IRC conversation this afternoon: >> >> 19:49 < gcl> sfr: [...] Your patch and my patch are >> essentially doing exactly the same thing, except that I used '-d' >> and you used '-n'. >> 19:50 < gcl> s/-d/-D/ >> 19:55 < sfr> right >> 19:55 < sfr> Linus wanted us to use -n > > Just a note: Linus doesn't really care. > > IOW, I used -n not because of any fundamental belief that it is > correct, but just because ti happened to be how I happened to decide > to solve it. It's entirely possible that starting from the Kconfig > defaults (rather than "no") is the right way to go. > > I think either approach is likely fine. The -D /dev/null approach > would presumably give smaller Kconfig.xyz files, assuming our defaults > are sane (an maybe that could be at least a partial validation of the > defaults we do have). While the -n approach is in some ways more > stable, in that the resulting Kconfig.xyz entires would presumably be > more stand-alone, and there wouldn't be any subtle interactions when > somebody changes a default value int he Kconfig file. Okay, well I advocate for the -D /dev/null approach then. I think that validating our defaults, and looking for the subtle interactions are exactly what we want to be doing when it comes to defconfigs. The fact that a defconfig does *not* want the default value is exactly what the defconfigs should be capturing. > So I can see advantages to either model. And either model clearly > would want the improvements to "select" that are independent (ie warn > about unsatisfied 'depends on' constraints, and select recursively. > Maybe we already fixed the recursive select thing, I forget). > > I also think we need to allow setting of actual values. I don't know > what the syntax would be. A "set" statement that overrides a default > in the Kconfig file, so that you can do > > set NODES_SHIFT 10 > > which would basically be equivalent to a "select" of a tristate > variable, but instead set the actual value? I dunno. I'm partial to extending select statements myself because it fits nicely into the existing grammer; but I can see value in having a set statement too. It would eliminate the temporary config options that both my and Stephen's patch would add. > And quite frankly, maybe somebody comes up with a better model > entirely. I like the Kconfig.xyz model, in that it should be > human-readable/writable and shouldn't introduce any fundamentally new > concepts (except the fairly simple extensions to the Kconfig > language), but maybe there are better models. Perhaps, but I can't think of anything and this one is simple, elegant, and easy to implement. > Regardless, I don't have anything against either set of patches > (Grant's or Stephen's). I think we should run with this. Since the patch has been merged to warn on unmet Kconfig dependences, the only major hole left is being able to do negative selects and to select specific values. Stephen, I'm happy to either keep working on this, or drop my patch in favor of yours. Whichever you prefer. I'll try to find some time to look at the Kconfig grammer. The solver would also be useful and could further reduce the size of the Kconfig fragments, but it isn't necessary so we don't need to wait for it to get implemented to take this approach.. Cheers, g.
On Tue, Jul 13, 2010 at 10:47 PM, Grant Likely <grant.likely@secretlab.ca> wrote: > > Okay, well I advocate for the -D /dev/null approach then. I think > that validating our defaults, and looking for the subtle interactions > are exactly what we want to be doing when it comes to defconfigs. The > fact that a defconfig does *not* want the default value is exactly > what the defconfigs should be capturing. Sure. That does have the downside that it absolutely _requires_ changes to the Kconfig language. Because currently "select" can only set a config variable, it cannot clear it or downgrade it from the defaults - so if you start from anything but a noconfig, you can't unset anything. But that's not a big downside, since honestly either approach will want it anyway (even -n wants it for setting integer or string options - never mind the whole issue of making it easier/cleaner to write the files in the first place). >> set NODES_SHIFT 10 > > I'm partial to extending select statements myself because it fits > nicely into the existing grammer; but I can see value in having a set > statement too. If it turns out easier to just implement the "set value" as a "select with a value", that's fine by me too. If the syntax for setting a particular value ends up being select NODES_SHIFT 10 that still works (with or without a "=" there). It looks a bit odd to me, but that's probably just because it's new. It doesn't look _wrong_ by any stretch. > It would eliminate the temporary config options that > both my and Stephen's patch would add. I don't think there is anything wrong with having the extra config option just to drive the "select" syntax. In fact, in many cases I think it would be "documentation", because I think the extra config option should generally be named by the platform that you're actually selecting for - even if that name may not be used for anything else. So having the basic syntax for a Kconfig.xyz file be something like config SIDEWINDER_PLATFORM def_bool y select ... where that initial config/def_bool is strictly not needed for the actual end result, but is used to get to the select statements, and to include that "SIDEWINDER_PLATFORM=y" in the resulting .config as a kind of top-level comment all looks fine. Linus
different default. I am not sure if we need a wa to turn them off or to just change them to being neede to be selected by those that do want them. - we have no way to select options that are neither bool or tristate to suitable values. Again the only way to do that currently is to reproduce the config entry with a different default value. I am currently working towards using this to recreate the PowerPC defconfigs, but it is a slow process as they have some much stuff enabled in them and some of it is probably actually not relevant. This process is made easier by the recent commit "kbuild: Warn on selecting symbols with unmet direct dependencies" that is in the kbuild tree (and linux-next). diff --git a/scripts/kconfig/Makefile b/scripts/kconfig/Makefile index 7ea649d..1ab8f45 100644 --- a/scripts/kconfig/Makefile +++ b/scripts/kconfig/Makefile @@ -117,9 +117,21 @@ else $(Q)$< -D arch/$(SRCARCH)/configs/$(KBUILD_DEFCONFIG) $(Kconfig) endif -%_defconfig: $(obj)/conf +configs_dir := $(srctree)/arch/$(SRCARCH)/configs +# We check a level of sub directories because arch/powerpc +# has its defconfig files arranged that way +old_defconfigs := $(patsubst $(configs_dir)/%,%,\ + $(wildcard $(configs_dir)/*_defconfig) \ + $(wildcard $(configs_dir)/*/*_defconfig)) +defconfigs := $(patsubst $(configs_dir)/Kconfig.%,%_defconfig,\ + $(wildcard $(configs_dir)/Kconfig.*)) + +$(old_defconfigs): %_defconfig: $(obj)/conf $(Q)$< -D arch/$(SRCARCH)/configs/$@ $(Kconfig) +$(defconfigs): %_defconfig: $(obj)/conf + $(Q)$< -n arch/$(SRCARCH)/configs/Kconfig.$* + # Help text used by make help help: @echo ' config - Update current config utilising a line-oriented program'
After this change, doing a "make xxx_defconfig" will check first for a file called arch/<arch>/configs/Kconfig.xxx and use that to generate the .config (effectively starting from an allnoconfig). If that file doesn't exist, it will use arch/<ARCH>/configs/xxx_defconfig as now. Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au> --- scripts/kconfig/Makefile | 14 +++++++++++++- 1 files changed, 13 insertions(+), 1 deletions(-) Hi Linus, Is this more the direction you want to take? There are still 2 main problems with is approach: - there are some config options that are globally and unconditionally enabled that some platforms may not want. The only way currently to turn them off is to reproduce the config entry with the