2007-11-10 20:39:14

by Sam Ravnborg

[permalink] [raw]
Subject: [PATCH 0/5] introduce K64BIT=y and backward compatibility ARCH={i386,x86_64} for x86

As discussed in another thread the right thing is to add a generic solution
to select between 32 and 64 bit - useable for powerpc, s390, ppc et al.

First step was to teach kconfig how to force 64BIT to a specific value.
The x86 Kconfig file needed a small twist to use 64BIT as the symbol
to seelct 32 or 64 bit.
Then it was simple to add backward compatibility ARCH= settings.

The patchset is not yet pushed out - I will await a bit feedback first.

Shortlog and diffstat.
kconfig: factor out code in confdata.c
kconfig: use $K64BIT to set 64BIT with all*config targets
x86: Use CONFIG_64BIT to select between 32 and 64 bit in Kconfig
kconfig: document make K64BIT=y in README
x86: introduce ARCH=i386,ARCH=x86_64 to select 32/64 bit


Makefile | 16 ++++-
README | 2 +
arch/x86/Kconfig | 26 +++-----
arch/x86/Makefile | 10 ++-
scripts/kconfig/Makefile | 2 +-
scripts/kconfig/conf.c | 1 +
scripts/kconfig/confdata.c | 146 +++++++++++++++++++++++++++----------------
scripts/kconfig/lkc_proto.h | 1 +
8 files changed, 124 insertions(+), 80 deletions(-)

The majority of the diffstat is the code refactoring of confdata.c
The rest is simple changes.

Patches follows...
Patches are on top of the patchset to introduce "make ARCH=x86"

Sam


2007-11-10 20:41:59

by Sam Ravnborg

[permalink] [raw]
Subject: [PATCH] x86: Use CONFIG_64BIT to select between 32 and 64 bit in Kconfig

This change allow us to use the new syntax:
make K64BIT={n,y} to select between 32 and 64 bit.

Signed-off-by: Sam Ravnborg <[email protected]>
---
arch/x86/Kconfig | 26 ++++++++------------------
1 files changed, 8 insertions(+), 18 deletions(-)

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 153c26c..0d86611 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -1,34 +1,24 @@
# x86 configuration

# Select 32 or 64 bit
-choice
- bool "Select 32 or 64 bit"
- default X86_32
+config 64BIT
+ bool "64 Bit kernel"
+ default n
+ help
+ Say yes to build a 64 bit kernel - formerly known as x86_64
+ Say no to build a 32 bit kernel - formerly known as i386

config X86_32
- bool "32 bit (former ARCH=i386)"
- help
- This is Linux's home port. Linux was originally native to the Intel
- 386, and runs on all the later x86 processors including the Intel
- 486, 586, Pentiums, and various instruction-set-compatible chips by
- AMD, Cyrix, and others.
+ def_bool !64BIT

config X86_64
- bool "64 bit (former ARCH=x86_64)"
- help
- Port to the x86-64 architecture. x86-64 is a 64-bit extension to the
- classical 32-bit x86 architecture. For details see
- <http://www.x86-64.org/>.
-endchoice
+ def_bool 64BIT

### Arch settings
config X86
bool
default y

-config 64BIT
- def_bool X86_64
-
config GENERIC_TIME
bool
default y
--
1.5.3.4.1157.g0e74-dirty

2007-11-10 20:42:20

by Sam Ravnborg

[permalink] [raw]
Subject: [PATCH] kconfig: factor out code in confdata.c

This patch introduce no functional changes.

Signed-off-by: Sam Ravnborg <[email protected]>
Cc: Roman Zippel <[email protected]>
---
scripts/kconfig/confdata.c | 119 +++++++++++++++++++++++--------------------
1 files changed, 64 insertions(+), 55 deletions(-)

diff --git a/scripts/kconfig/confdata.c b/scripts/kconfig/confdata.c
index b2913e9..e0f402f 100644
--- a/scripts/kconfig/confdata.c
+++ b/scripts/kconfig/confdata.c
@@ -83,6 +83,68 @@ char *conf_get_default_confname(void)
return name;
}

+static int conf_set_sym_val(struct symbol *sym, int def, int def_flags, char *p)
+{
+ char *p2;
+
+ switch (sym->type) {
+ case S_TRISTATE:
+ if (p[0] == 'm') {
+ sym->def[def].tri = mod;
+ sym->flags |= def_flags;
+ break;
+ }
+ case S_BOOLEAN:
+ if (p[0] == 'y') {
+ sym->def[def].tri = yes;
+ sym->flags |= def_flags;
+ break;
+ }
+ if (p[0] == 'n') {
+ sym->def[def].tri = no;
+ sym->flags |= def_flags;
+ break;
+ }
+ conf_warning("symbol value '%s' invalid for %s", p, sym->name);
+ break;
+ case S_OTHER:
+ if (*p != '"') {
+ for (p2 = p; *p2 && !isspace(*p2); p2++)
+ ;
+ sym->type = S_STRING;
+ goto done;
+ }
+ case S_STRING:
+ if (*p++ != '"')
+ break;
+ for (p2 = p; (p2 = strpbrk(p2, "\"\\")); p2++) {
+ if (*p2 == '"') {
+ *p2 = 0;
+ break;
+ }
+ memmove(p2, p2 + 1, strlen(p2));
+ }
+ if (!p2) {
+ conf_warning("invalid string found");
+ return 1;
+ }
+ case S_INT:
+ case S_HEX:
+ done:
+ if (sym_string_valid(sym, p)) {
+ sym->def[def].val = strdup(p);
+ sym->flags |= def_flags;
+ } else {
+ conf_warning("symbol value '%s' invalid for %s", p, sym->name);
+ return 1;
+ }
+ break;
+ default:
+ ;
+ }
+ return 0;
+}
+
int conf_read_simple(const char *name, int def)
{
FILE *in = NULL;
@@ -213,61 +275,8 @@ load:
conf_warning("trying to reassign symbol %s", sym->name);
break;
}
- switch (sym->type) {
- case S_TRISTATE:
- if (p[0] == 'm') {
- sym->def[def].tri = mod;
- sym->flags |= def_flags;
- break;
- }
- case S_BOOLEAN:
- if (p[0] == 'y') {
- sym->def[def].tri = yes;
- sym->flags |= def_flags;
- break;
- }
- if (p[0] == 'n') {
- sym->def[def].tri = no;
- sym->flags |= def_flags;
- break;
- }
- conf_warning("symbol value '%s' invalid for %s", p, sym->name);
- break;
- case S_OTHER:
- if (*p != '"') {
- for (p2 = p; *p2 && !isspace(*p2); p2++)
- ;
- sym->type = S_STRING;
- goto done;
- }
- case S_STRING:
- if (*p++ != '"')
- break;
- for (p2 = p; (p2 = strpbrk(p2, "\"\\")); p2++) {
- if (*p2 == '"') {
- *p2 = 0;
- break;
- }
- memmove(p2, p2 + 1, strlen(p2));
- }
- if (!p2) {
- conf_warning("invalid string found");
- continue;
- }
- case S_INT:
- case S_HEX:
- done:
- if (sym_string_valid(sym, p)) {
- sym->def[def].val = strdup(p);
- sym->flags |= def_flags;
- } else {
- conf_warning("symbol value '%s' invalid for %s", p, sym->name);
- continue;
- }
- break;
- default:
- ;
- }
+ if (conf_set_sym_val(sym, def, def_flags, p))
+ continue;
break;
case '\r':
case '\n':
--
1.5.3.4.1157.g0e74-dirty

2007-11-10 20:42:33

by Sam Ravnborg

[permalink] [raw]
Subject: [PATCH] x86: introduce ARCH=i386,ARCH=x86_64 to select 32/64 bit

Using the newly added infrastructure is is now simple
to add addition ARCH= symbols to select between 32 and 64 bit.
Do this for x86.

Signed-off-by: Sam Ravnborg <[email protected]>
---
Makefile | 16 +++++++++++++---
arch/x86/Makefile | 10 +++++++---
scripts/kconfig/Makefile | 2 +-
3 files changed, 21 insertions(+), 7 deletions(-)

diff --git a/Makefile b/Makefile
index 902082b..5efee80 100644
--- a/Makefile
+++ b/Makefile
@@ -165,8 +165,7 @@ export srctree objtree VPATH TOPDIR
# then ARCH is assigned, getting whatever value it gets normally, and
# SUBARCH is subsequently ignored.

-SUBARCH := $(shell uname -m | sed -e s/i.86/x86/ -e s/x86_64/x86/ \
- -e s/sun4u/sparc64/ \
+SUBARCH := $(shell uname -m | sed -e s/i.86/i386/ -e s/sun4u/sparc64/ \
-e s/arm.*/arm/ -e s/sa110/arm/ \
-e s/s390x/s390/ -e s/parisc64/parisc/ \
-e s/ppc.*/powerpc/ -e s/mips.*/mips/ \
@@ -198,8 +197,19 @@ CROSS_COMPILE ?=
UTS_MACHINE := $(ARCH)
SRCARCH := $(ARCH)

+ifeq ($(ARCH),i386)
+ K64BIT := n
+ SRCARCH := x86
+endif
+ifeq ($(ARCH),x86_64)
+ K64BIT := y
+ SRCARCH := x86
+endif
+export K64BIT
+
+
# Sanity check the specified ARCH
-ifeq ($(wildcard $(srctree)/arch/$(ARCH)/Kconfig),)
+ifeq ($(wildcard $(srctree)/arch/$(SRCARCH)/Kconfig),)
$(error "ERROR: ARCH ($(ARCH)) does not exist (for i386 and x86_64 use ARCH=x86)")
endif

diff --git a/arch/x86/Makefile b/arch/x86/Makefile
index ee94224..feba761 100644
--- a/arch/x86/Makefile
+++ b/arch/x86/Makefile
@@ -1,9 +1,13 @@
# Unified Makefile for i386 and x86_64

# select i386 defconfig file as default config
-KBUILD_DEFCONFIG := i386_defconfig
-
-# # No need to remake these files
+ifeq ($(ARCH),x86)
+ KBUILD_DEFCONFIG := i386_defconfig
+else
+ KBUILD_DEFCONFIG := $(ARCH)_defconfig
+endif
+
+# No need to remake these files
$(srctree)/arch/x86/Makefile%: ;

ifeq ($(CONFIG_X86_32),y)
diff --git a/scripts/kconfig/Makefile b/scripts/kconfig/Makefile
index 3c9db07..1ad6f7f 100644
--- a/scripts/kconfig/Makefile
+++ b/scripts/kconfig/Makefile
@@ -4,7 +4,7 @@

PHONY += oldconfig xconfig gconfig menuconfig config silentoldconfig update-po-config

-Kconfig := arch/$(ARCH)/Kconfig
+Kconfig := arch/$(SRCARCH)/Kconfig

xconfig: $(obj)/qconf
$< $(Kconfig)
--
1.5.3.4.1157.g0e74-dirty

2007-11-10 20:42:49

by Sam Ravnborg

[permalink] [raw]
Subject: [PATCH] kconfig: document make K64BIT=y in README

Signed-off-by: Sam Ravnborg <[email protected]>
---
README | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/README b/README
index 159912c..6622ba1 100644
--- a/README
+++ b/README
@@ -194,6 +194,8 @@ CONFIGURING the kernel:
"make *config" checks for a file named "all{yes/mod/no/random}.config"
for symbol values that are to be forced. If this file is not found,
it checks for a file named "all.config" to contain forced values.
+ Finally it checks the environment variable K64BIT and set the config
+ symbol "64BIT" to the value of the K64BIT variable.

NOTES on "make config":
- having unnecessary drivers will make the kernel bigger, and can
--
1.5.3.4.1157.g0e74-dirty

2007-11-10 20:43:08

by Sam Ravnborg

[permalink] [raw]
Subject: [PATCH] kconfig: use $K64BIT to set 64BIT with all*config targets

The variable K64BIT can now be used to select the
value of CONFIG_64BIT.

This is for example useful for powerpc to generate
allmodconfig for both bit sizes - like this:
make ARCH=powerpc K64BIT=y
make ARCH=powerpc K64BIT=n

To use this the Kconfig file must use "64BIT" as the
config value to select between 32 and 64 bit.

Signed-off-by: Sam Ravnborg <[email protected]>
---
scripts/kconfig/conf.c | 1 +
scripts/kconfig/confdata.c | 27 +++++++++++++++++++++++++++
scripts/kconfig/lkc_proto.h | 1 +
3 files changed, 29 insertions(+), 0 deletions(-)

diff --git a/scripts/kconfig/conf.c b/scripts/kconfig/conf.c
index a38787a..c6bee85 100644
--- a/scripts/kconfig/conf.c
+++ b/scripts/kconfig/conf.c
@@ -591,6 +591,7 @@ int main(int ac, char **av)
conf_read_simple(name, S_DEF_USER);
else if (!stat("all.config", &tmpstat))
conf_read_simple("all.config", S_DEF_USER);
+ conf_set_env_sym("K64BIT", "64BIT", S_DEF_USER);
break;
default:
break;
diff --git a/scripts/kconfig/confdata.c b/scripts/kconfig/confdata.c
index e0f402f..0cb7555 100644
--- a/scripts/kconfig/confdata.c
+++ b/scripts/kconfig/confdata.c
@@ -145,6 +145,33 @@ static int conf_set_sym_val(struct symbol *sym, int def, int def_flags, char *p)
return 0;
}

+/* Read an environment variable and assign the vaule to the symbol */
+int conf_set_env_sym(const char *env, const char *symname, int def)
+{
+ struct symbol *sym;
+ char *p;
+ int def_flags;
+
+ p = getenv(env);
+ if (p) {
+ char warning[100];
+ sprintf(warning, "Environment variable (%s = \"%s\")", env, p);
+ conf_filename = warning;
+ def_flags = SYMBOL_DEF << def;
+ if (def == S_DEF_USER) {
+ sym = sym_find(symname);
+ if (!sym)
+ return 1;
+ } else {
+ sym = sym_lookup(symname, 0);
+ if (sym->type == S_UNKNOWN)
+ sym->type = S_OTHER;
+ }
+ conf_set_sym_val(sym, def, def_flags, p);
+ }
+ return 0;
+}
+
int conf_read_simple(const char *name, int def)
{
FILE *in = NULL;
diff --git a/scripts/kconfig/lkc_proto.h b/scripts/kconfig/lkc_proto.h
index 4d09f6d..dca294e 100644
--- a/scripts/kconfig/lkc_proto.h
+++ b/scripts/kconfig/lkc_proto.h
@@ -1,6 +1,7 @@

/* confdata.c */
P(conf_parse,void,(const char *name));
+P(conf_set_env_sym,int,(const char *envname, const char *symname, int def));
P(conf_read,int,(const char *name));
P(conf_read_simple,int,(const char *name, int));
P(conf_write,int,(const char *name));
--
1.5.3.4.1157.g0e74-dirty

2007-11-10 20:55:22

by Guillaume Chazarain

[permalink] [raw]
Subject: Re: [PATCH] kconfig: use $K64BIT to set 64BIT with all*config targets

Hi,

On 11/10/07, Sam Ravnborg <[email protected]> wrote:
> The variable K64BIT can now be used to select the
> value of CONFIG_64BIT.

Why not calling the environment variable CONFIG_64BIT,
in preparation of the day when all CONFIG_ variables can
be passed by environment variables?

--
Guillaume

2007-11-10 22:17:49

by Randy Dunlap

[permalink] [raw]
Subject: Re: [PATCH] kconfig: use $K64BIT to set 64BIT with all*config targets

On Sat, 10 Nov 2007 21:43:26 +0100 Sam Ravnborg wrote:

> The variable K64BIT can now be used to select the
> value of CONFIG_64BIT.
>
> This is for example useful for powerpc to generate
> allmodconfig for both bit sizes - like this:
> make ARCH=powerpc K64BIT=y
> make ARCH=powerpc K64BIT=n
>
> To use this the Kconfig file must use "64BIT" as the
> config value to select between 32 and 64 bit.
>
> Signed-off-by: Sam Ravnborg <[email protected]>
> ---
> scripts/kconfig/conf.c | 1 +
> scripts/kconfig/confdata.c | 27 +++++++++++++++++++++++++++
> scripts/kconfig/lkc_proto.h | 1 +
> 3 files changed, 29 insertions(+), 0 deletions(-)
>
> diff --git a/scripts/kconfig/conf.c b/scripts/kconfig/conf.c
> index a38787a..c6bee85 100644
> --- a/scripts/kconfig/conf.c
> +++ b/scripts/kconfig/conf.c
> @@ -591,6 +591,7 @@ int main(int ac, char **av)
> conf_read_simple(name, S_DEF_USER);
> else if (!stat("all.config", &tmpstat))
> conf_read_simple("all.config", S_DEF_USER);
> + conf_set_env_sym("K64BIT", "64BIT", S_DEF_USER);
> break;
> default:
> break;
> diff --git a/scripts/kconfig/confdata.c b/scripts/kconfig/confdata.c
> index e0f402f..0cb7555 100644
> --- a/scripts/kconfig/confdata.c
> +++ b/scripts/kconfig/confdata.c
> @@ -145,6 +145,33 @@ static int conf_set_sym_val(struct symbol *sym, int def, int def_flags, char *p)
> return 0;
> }
>
> +/* Read an environment variable and assign the vaule to the symbol */

value

> +int conf_set_env_sym(const char *env, const char *symname, int def)
> +{
> + struct symbol *sym;
> + char *p;
> + int def_flags;
> +
> + p = getenv(env);
> + if (p) {
> + char warning[100];
> + sprintf(warning, "Environment variable (%s = \"%s\")", env, p);
> + conf_filename = warning;

What's with <warning> and <conf_filename> here? I don't see how
they are used in this function or in the caller of this function.

> + def_flags = SYMBOL_DEF << def;
> + if (def == S_DEF_USER) {
> + sym = sym_find(symname);
> + if (!sym)
> + return 1;
> + } else {
> + sym = sym_lookup(symname, 0);
> + if (sym->type == S_UNKNOWN)
> + sym->type = S_OTHER;
> + }
> + conf_set_sym_val(sym, def, def_flags, p);
> + }
> + return 0;
> +}
> +
> int conf_read_simple(const char *name, int def)
> {
> FILE *in = NULL;

---
~Randy

2007-11-10 22:19:56

by Randy Dunlap

[permalink] [raw]
Subject: Re: [PATCH] x86: Use CONFIG_64BIT to select between 32 and 64 bit in Kconfig

On Sat, 10 Nov 2007 21:43:27 +0100 Sam Ravnborg wrote:

> This change allow us to use the new syntax:
> make K64BIT={n,y} to select between 32 and 64 bit.
>
> Signed-off-by: Sam Ravnborg <[email protected]>
> ---
> arch/x86/Kconfig | 26 ++++++++------------------
> 1 files changed, 8 insertions(+), 18 deletions(-)
>
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index 153c26c..0d86611 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -1,34 +1,24 @@
> # x86 configuration
>
> # Select 32 or 64 bit
> -choice
> - bool "Select 32 or 64 bit"
> - default X86_32
> +config 64BIT
> + bool "64 Bit kernel"

bool "64-bit kernel"

> + default n
> + help
> + Say yes to build a 64 bit kernel - formerly known as x86_64

64-bit

> + Say no to build a 32 bit kernel - formerly known as i386

32-bit

>
> config X86_32
> - bool "32 bit (former ARCH=i386)"
> - help
> - This is Linux's home port. Linux was originally native to the Intel
> - 386, and runs on all the later x86 processors including the Intel
> - 486, 586, Pentiums, and various instruction-set-compatible chips by
> - AMD, Cyrix, and others.
> + def_bool !64BIT
>
> config X86_64
> - bool "64 bit (former ARCH=x86_64)"
> - help
> - Port to the x86-64 architecture. x86-64 is a 64-bit extension to the
> - classical 32-bit x86 architecture. For details see
> - <http://www.x86-64.org/>.
> -endchoice
> + def_bool 64BIT
>
> ### Arch settings
> config X86
> bool
> default y
>
> -config 64BIT
> - def_bool X86_64
> -
> config GENERIC_TIME
> bool
> default y
> --


---
~Randy

2007-11-10 22:23:52

by Randy Dunlap

[permalink] [raw]
Subject: Re: [PATCH] kconfig: document make K64BIT=y in README

On Sat, 10 Nov 2007 21:43:28 +0100 Sam Ravnborg wrote:

> Signed-off-by: Sam Ravnborg <[email protected]>
> ---
> README | 2 ++
> 1 files changed, 2 insertions(+), 0 deletions(-)
>
> diff --git a/README b/README
> index 159912c..6622ba1 100644
> --- a/README
> +++ b/README
> @@ -194,6 +194,8 @@ CONFIGURING the kernel:
> "make *config" checks for a file named "all{yes/mod/no/random}.config"
> for symbol values that are to be forced. If this file is not found,
> it checks for a file named "all.config" to contain forced values.
> + Finally it checks the environment variable K64BIT and set the config

and if found, sets
the config symbol "64BIT" ...


> + symbol "64BIT" to the value of the K64BIT variable.
>
> NOTES on "make config":
> - having unnecessary drivers will make the kernel bigger, and can
> --

---
~Randy

2007-11-10 22:30:14

by Sam Ravnborg

[permalink] [raw]
Subject: Re: [PATCH] kconfig: use $K64BIT to set 64BIT with all*config targets

On Sat, Nov 10, 2007 at 02:16:19PM -0800, Randy Dunlap wrote:
> On Sat, 10 Nov 2007 21:43:26 +0100 Sam Ravnborg wrote:
>
> > The variable K64BIT can now be used to select the
> > value of CONFIG_64BIT.
> >
> > This is for example useful for powerpc to generate
> > allmodconfig for both bit sizes - like this:
> > make ARCH=powerpc K64BIT=y
> > make ARCH=powerpc K64BIT=n
> >
> > To use this the Kconfig file must use "64BIT" as the
> > config value to select between 32 and 64 bit.
> >
> > Signed-off-by: Sam Ravnborg <[email protected]>
> > ---
> > scripts/kconfig/conf.c | 1 +
> > scripts/kconfig/confdata.c | 27 +++++++++++++++++++++++++++
> > scripts/kconfig/lkc_proto.h | 1 +
> > 3 files changed, 29 insertions(+), 0 deletions(-)
> >
> > diff --git a/scripts/kconfig/conf.c b/scripts/kconfig/conf.c
> > index a38787a..c6bee85 100644
> > --- a/scripts/kconfig/conf.c
> > +++ b/scripts/kconfig/conf.c
> > @@ -591,6 +591,7 @@ int main(int ac, char **av)
> > conf_read_simple(name, S_DEF_USER);
> > else if (!stat("all.config", &tmpstat))
> > conf_read_simple("all.config", S_DEF_USER);
> > + conf_set_env_sym("K64BIT", "64BIT", S_DEF_USER);
> > break;
> > default:
> > break;
> > diff --git a/scripts/kconfig/confdata.c b/scripts/kconfig/confdata.c
> > index e0f402f..0cb7555 100644
> > --- a/scripts/kconfig/confdata.c
> > +++ b/scripts/kconfig/confdata.c
> > @@ -145,6 +145,33 @@ static int conf_set_sym_val(struct symbol *sym, int def, int def_flags, char *p)
> > return 0;
> > }
> >
> > +/* Read an environment variable and assign the vaule to the symbol */
>
> value
>
> > +int conf_set_env_sym(const char *env, const char *symname, int def)
> > +{
> > + struct symbol *sym;
> > + char *p;
> > + int def_flags;
> > +
> > + p = getenv(env);
> > + if (p) {
> > + char warning[100];
> > + sprintf(warning, "Environment variable (%s = \"%s\")", env, p);
> > + conf_filename = warning;
>
> What's with <warning> and <conf_filename> here? I don't see how
> they are used in this function or in the caller of this function.

Added to allow conf_warning to print out something usefull.
Like in the following example:

make K64BIT=randy allnoconfig

Environment variable (K64BIT = "randy"):0:warning: symbol value 'randy' invalid for 64BIT


This could look better - but I preferred this version for the
less readable:
(null):0:warning: symbol value 'randy' invalid for 64BIT

Thanks for the other inputs. I am correcting them in
my local patch-set. Before resubmit I willawait further comments.

Sam

2007-11-10 22:35:00

by Randy Dunlap

[permalink] [raw]
Subject: Re: [PATCH 0/5] introduce K64BIT=y and backward compatibility ARCH={i386,x86_64} for x86

On Sat, 10 Nov 2007 21:40:38 +0100 Sam Ravnborg wrote:

> As discussed in another thread the right thing is to add a generic solution
> to select between 32 and 64 bit - useable for powerpc, s390, ppc et al.
>
> First step was to teach kconfig how to force 64BIT to a specific value.
> The x86 Kconfig file needed a small twist to use 64BIT as the symbol
> to seelct 32 or 64 bit.
> Then it was simple to add backward compatibility ARCH= settings.
>
> The patchset is not yet pushed out - I will await a bit feedback first.
>
> Shortlog and diffstat.
> kconfig: factor out code in confdata.c
> kconfig: use $K64BIT to set 64BIT with all*config targets
> x86: Use CONFIG_64BIT to select between 32 and 64 bit in Kconfig
> kconfig: document make K64BIT=y in README
> x86: introduce ARCH=i386,ARCH=x86_64 to select 32/64 bit

Hi Sam,
Looks good to me and should satisfy our habits^w usages models.

The one minor question is the environment variable name (K64BIT or
something else, like Guillaume brought up). Personally I don't
care how it's spelled. IOW, K64BIT is sufficient, but if there is a
goal to be able to place any CONFIG_symbol on the command line or
as an env. variable, they might as well all be named with CONFIG_*.

Thanks,
---
~Randy

2007-11-10 22:48:31

by Sam Ravnborg

[permalink] [raw]
Subject: Re: [PATCH 0/5] introduce K64BIT=y and backward compatibility ARCH={i386,x86_64} for x86

On Sat, Nov 10, 2007 at 02:33:02PM -0800, Randy Dunlap wrote:
> On Sat, 10 Nov 2007 21:40:38 +0100 Sam Ravnborg wrote:
>
> > As discussed in another thread the right thing is to add a generic solution
> > to select between 32 and 64 bit - useable for powerpc, s390, ppc et al.
> >
> > First step was to teach kconfig how to force 64BIT to a specific value.
> > The x86 Kconfig file needed a small twist to use 64BIT as the symbol
> > to seelct 32 or 64 bit.
> > Then it was simple to add backward compatibility ARCH= settings.
> >
> > The patchset is not yet pushed out - I will await a bit feedback first.
> >
> > Shortlog and diffstat.
> > kconfig: factor out code in confdata.c
> > kconfig: use $K64BIT to set 64BIT with all*config targets
> > x86: Use CONFIG_64BIT to select between 32 and 64 bit in Kconfig
> > kconfig: document make K64BIT=y in README
> > x86: introduce ARCH=i386,ARCH=x86_64 to select 32/64 bit
>
> Hi Sam,
> Looks good to me and should satisfy our habits^w usages models.
>
> The one minor question is the environment variable name (K64BIT or
> something else, like Guillaume brought up). Personally I don't
> care how it's spelled. IOW, K64BIT is sufficient, but if there is a
> goal to be able to place any CONFIG_symbol on the command line or
> as an env. variable, they might as well all be named with CONFIG_*.

The K64BIT varibale came up because it is more unique than CONFIG_64BIT.
Google turned up less than 20 hits[1] on K64BIT but 14900 on CONFIG_64BIT.
And I dunno if some people source their .config or other crazy stuff.

And then K64BIT was easier to type...
But if we assume this is internal stuff only then we could go
for the longer version and I will then suggest to prefix
the CONFIG_ symbol with K like in KCONFIG_64BIT which
has no hits with google right now!

I do not see this extended to 'any' CONFIG_ symbol
but maybe the 4 most important ones.
[64BIT, SMP, PREEMPT, ?]

[1] before my posting - now we are up to ~80.

Sam

2007-11-11 05:10:23

by Adrian Bunk

[permalink] [raw]
Subject: Re: [PATCH 0/5] introduce K64BIT=y and backward compatibility ARCH={i386,x86_64} for x86

On Sat, Nov 10, 2007 at 09:40:38PM +0100, Sam Ravnborg wrote:

> As discussed in another thread the right thing is to add a generic solution
> to select between 32 and 64 bit - useable for powerpc, s390, ppc et al.
>...

I seriously question this would be "the right thing".

32/64bit isn't that special that this and only this option would require
special casing, and the KISS principle of having only one way to specify
something like this has it's advantages.

> Sam

cu
Adrian

--

"Is there not promise of rain?" Ling Tan asked suddenly out
of the darkness. There had been need of rain for many days.
"Only a promise," Lao Er said.
Pearl S. Buck - Dragon Seed

2007-11-11 05:14:39

by Adrian Bunk

[permalink] [raw]
Subject: Re: [PATCH] kconfig: use $K64BIT to set 64BIT with all*config targets

On Sat, Nov 10, 2007 at 09:55:06PM +0100, Guillaume Chazarain wrote:
> Hi,
>
> On 11/10/07, Sam Ravnborg <[email protected]> wrote:
> > The variable K64BIT can now be used to select the
> > value of CONFIG_64BIT.
>
> Why not calling the environment variable CONFIG_64BIT,
> in preparation of the day when all CONFIG_ variables can
> be passed by environment variables?

What exactly are the use cases where someone would need this?

> Guillaume

cu
Adrian

--

"Is there not promise of rain?" Ling Tan asked suddenly out
of the darkness. There had been need of rain for many days.
"Only a promise," Lao Er said.
Pearl S. Buck - Dragon Seed

2007-11-11 11:53:25

by Sam Ravnborg

[permalink] [raw]
Subject: Re: [PATCH 0/5] introduce K64BIT=y and backward compatibility ARCH={i386,x86_64} for x86

On Sun, Nov 11, 2007 at 06:09:45AM +0100, Adrian Bunk wrote:
> On Sat, Nov 10, 2007 at 09:40:38PM +0100, Sam Ravnborg wrote:
>
> > As discussed in another thread the right thing is to add a generic solution
> > to select between 32 and 64 bit - useable for powerpc, s390, ppc et al.
> >...
>
> I seriously question this would be "the right thing".
>
> 32/64bit isn't that special that this and only this option would require
> special casing, and the KISS principle of having only one way to specify
> something like this has it's advantages.

"The right thing" in the correct context.
It was discussed to keep ARCH={i386,x86_64} and the point I have
is that if we are going to extend ARCH=... to be useable to
specify kernel bit size then it should be done in a generic way
and not like it was done before on x86.

I do not consider the discussion about keeping/dropping
ARCH={i386,x86_64} as concluded.

And if we decide on keeping ARCH={i386,x86_64} then I have
questioned the semantics. Clear opinions are missing..

ARCH= semantic

Impact before now
================================================
32/64 bit yes yes
bzImage location yes no
different Kconfig files yes no
decide defconfig yes yes
asm symlink no no
build option yes no [1]

[did I miss anything? I think I did]

[1] ARCH=... select 32/64-bit during configuration.
There is no difference between ARCH={x86,i386,x86_64}
when building the kernel because the 32/64 bit
choice is done at configuration time.

The table above reflect the [now] semantics with the
patches that is present at lkml.

And the patch needed to implment the above
semantic (after the preparational stuff which
is generic) are:

$ git diff --stat HEAD~1..HEAD
Makefile | 18 ++++++++++++++----
arch/x86/Makefile | 8 ++++++--
scripts/kconfig/Makefile | 2 +-
3 files changed, 21 insertions(+), 7 deletions(-)


The scripts/kconfig/Makefile change is a bugfix that maybe
should be included in another patch. It is not x86 specific.

So 19 additional lines and 5 deleted lines to introduce the
ARCH= semantics above.

Sam

2007-11-11 12:43:40

by Guillaume Chazarain

[permalink] [raw]
Subject: Re: [PATCH] kconfig: use $K64BIT to set 64BIT with all*config targets

Hi Adrian,

On 11/11/07, Adrian Bunk <[email protected]> wrote:
> What exactly are the use cases where someone would need this?

Glad you asked. Today, when I want to recompile a kernel while
changing a CONFIG_ option, I manually edit the .config,
remove the appropriate line and then run make oldconfig.
I'd like to be able to do: make oldconfig CONFIG_FOO=bar.

Also, when working on a specific feature of the kernel, I tend to
install both a kernel with the CONFIG_ option set and one with
the option unset. Scripts to do that can twiddle the .config file,
but it would be more convenient if kbuild could avoid that.

As you see, I'm more interested in make oldconfig than
make all*config.

Cheers.

--
Guillaume

2007-11-11 13:07:48

by Adrian Bunk

[permalink] [raw]
Subject: Re: [PATCH] kconfig: use $K64BIT to set 64BIT with all*config targets

On Sun, Nov 11, 2007 at 01:43:28PM +0100, Guillaume Chazarain wrote:
> Hi Adrian,

Hi Guillaume,

> On 11/11/07, Adrian Bunk <[email protected]> wrote:
> > What exactly are the use cases where someone would need this?
>
> Glad you asked. Today, when I want to recompile a kernel while
> changing a CONFIG_ option, I manually edit the .config,
> remove the appropriate line and then run make oldconfig.
> I'd like to be able to do: make oldconfig CONFIG_FOO=bar.

first of all it's obvious that there can't be any guarantee that your
CONFIG_FOO variable will actually get the value bar since dependencies
might enable or disable it despite you wanting the opposite.

Another important point is that users that know about and see CONFIG_*
variables are kernel hackers, not the normal kconfig users.

> Also, when working on a specific feature of the kernel, I tend to
> install both a kernel with the CONFIG_ option set and one with
> the option unset. Scripts to do that can twiddle the .config file,
> but it would be more convenient if kbuild could avoid that.

I'm wondering why you don't use two different O= output directories
instead?

Depending on the CONFIG_ option in question this might even greatly
reduce your compile times.

And you won't upgrade the kernel you work against that
often compared to working on and testing of your own changes when
developing code.

> As you see, I'm more interested in make oldconfig than
> make all*config.

That's clear.

> Cheers.
> Guillaume

cu
Adrian

--

"Is there not promise of rain?" Ling Tan asked suddenly out
of the darkness. There had been need of rain for many days.
"Only a promise," Lao Er said.
Pearl S. Buck - Dragon Seed

2007-11-11 15:00:10

by Guillaume Chazarain

[permalink] [raw]
Subject: Re: [PATCH] kconfig: use $K64BIT to set 64BIT with all*config targets

On 11/11/07, Adrian Bunk <[email protected]> wrote:
> Another important point is that users that know about and see CONFIG_*
> variables are kernel hackers, not the normal kconfig users.

But kconfig is mainly for kernel hackers, otherwise it would be
called CML2 ;-)

> > Also, when working on a specific feature of the kernel, I tend to
> > install both a kernel with the CONFIG_ option set and one with
> > the option unset. Scripts to do that can twiddle the .config file,
> > but it would be more convenient if kbuild could avoid that.
>
> I'm wondering why you don't use two different O= output directories
> instead?
>
> Depending on the CONFIG_ option in question this might even greatly
> reduce your compile times.

/me is filled with wonder at the discovery that .config is saved in the O=
directory. Thanks a lot Adrian for this time saver. So it's not strictly an
output directory, more a build directory.

I still think "make oldconfig CONFIG_FOO=bar" is useful for the occasional
config change, but thanks again for this great tip.

--
Guillaume

2007-11-11 15:29:20

by Sam Ravnborg

[permalink] [raw]
Subject: Re: [PATCH] kconfig: use $K64BIT to set 64BIT with all*config targets

On Sun, Nov 11, 2007 at 03:59:54PM +0100, Guillaume Chazarain wrote:
> > I'm wondering why you don't use two different O= output directories
> > instead?
> >
> > Depending on the CONFIG_ option in question this might even greatly
> > reduce your compile times.
>
> /me is filled with wonder at the discovery that .config is saved in the O=
> directory. Thanks a lot Adrian for this time saver. So it's not strictly an
> output directory, more a build directory.
The opposite....
All output is placed there - including the configuration generated by
the *config frontends.
It is not limited to kernel-build output.

One of the use cases for "make O=.." is a setup where the kernel source
is located in a RO location (CDROM, restricted permissions etc).

Sam

2007-11-11 15:56:08

by Guillaume Chazarain

[permalink] [raw]
Subject: Re: [PATCH] kconfig: use $K64BIT to set 64BIT with all*config targets

On 11/11/07, Sam Ravnborg <[email protected]> wrote:
> > So it's not strictly an
> > output directory, more a build directory.
> The opposite....
> All output is placed there - including the configuration generated by
> the *config frontends.

I meant, it's not strictly an output directory as if I do

make O=dir oldconfig

it will _read_ dir/.config, so the O= directory is also used for input.
And yes, I was splitting hairs ;-)

Sorry for the confusion.

--
Guillaume

2007-11-12 02:48:22

by Roman Zippel

[permalink] [raw]
Subject: Re: [PATCH 0/5] introduce K64BIT=y and backward compatibility ARCH={i386,x86_64} for x86

Hi,

On Sat, 10 Nov 2007, Sam Ravnborg wrote:

> As discussed in another thread the right thing is to add a generic solution
> to select between 32 and 64 bit - useable for powerpc, s390, ppc et al.

Could you please point me to this discussion?
Thanks.

bye, Roman

2007-11-12 05:22:24

by Sam Ravnborg

[permalink] [raw]
Subject: Re: [PATCH 0/5] introduce K64BIT=y and backward compatibility ARCH={i386,x86_64} for x86

On Mon, Nov 12, 2007 at 03:47:02AM +0100, Roman Zippel wrote:
> Hi,
>
> On Sat, 10 Nov 2007, Sam Ravnborg wrote:
>
> > As discussed in another thread the right thing is to add a generic solution
> > to select between 32 and 64 bit - useable for powerpc, s390, ppc et al.
>
> Could you please point me to this discussion?

It starts with this post from Jeff Garzik:
http://lkml.org/lkml/2007/11/9/274

Sam

2007-11-14 20:57:52

by Roman Zippel

[permalink] [raw]
Subject: Re: [PATCH] kconfig: use $K64BIT to set 64BIT with all*config targets

Hi,

On Sat, 10 Nov 2007, Sam Ravnborg wrote:

> + if (p) {
> + char warning[100];
> + sprintf(warning, "Environment variable (%s = \"%s\")", env, p);
> + conf_filename = warning;
> + def_flags = SYMBOL_DEF << def;
> + if (def == S_DEF_USER) {
> + sym = sym_find(symname);
> + if (!sym)
> + return 1;
> + } else {
> + sym = sym_lookup(symname, 0);
> + if (sym->type == S_UNKNOWN)
> + sym->type = S_OTHER;
> + }
> + conf_set_sym_val(sym, def, def_flags, p);
> + }
> + return 0;

This is more complex than necessary, this should be enough:

sym = sym_find(symname);
if (sym)
sym_set_string_value(sym, p);

This is not a direct user interface, so the potential stricter error
checking is not really needed.

In general I think it's problematic that this is only checked, when the
config system is called, i.e. with a configured kernel adding ARCH would
have no effect, what makes this more confusing is that one can later omit
the ARCH variable, since it's saved in the .config. I think it would be
better to check for this directly in the Makefile and then use a separate
tool to set the variable directly (which could be simply sed or a simple
helper program).

bye, Roman

2007-11-14 22:07:11

by Sam Ravnborg

[permalink] [raw]
Subject: Re: [PATCH] kconfig: use $K64BIT to set 64BIT with all*config targets

Hi Roman

On Wed, Nov 14, 2007 at 09:57:32PM +0100, Roman Zippel wrote:
> Hi,
>
> On Sat, 10 Nov 2007, Sam Ravnborg wrote:
>
> > + if (p) {
> > + char warning[100];
> > + sprintf(warning, "Environment variable (%s = \"%s\")", env, p);
> > + conf_filename = warning;
> > + def_flags = SYMBOL_DEF << def;
> > + if (def == S_DEF_USER) {
> > + sym = sym_find(symname);
> > + if (!sym)
> > + return 1;
> > + } else {
> > + sym = sym_lookup(symname, 0);
> > + if (sym->type == S_UNKNOWN)
> > + sym->type = S_OTHER;
> > + }
> > + conf_set_sym_val(sym, def, def_flags, p);
> > + }
> > + return 0;
>
> This is more complex than necessary, this should be enough:
>
> sym = sym_find(symname);
> if (sym)
> sym_set_string_value(sym, p);
>
> This is not a direct user interface, so the potential stricter error
> checking is not really needed.
The value can be supplied on the command-line so we need to validate input.
The code is a copy of what happen when reading a all.config file and
the functionality should be equal.
Can we make that part simpler too?

By the way - I have never understood the purpose of the flags (S_DEF_USER etc.)
Can we have a few comments added to their definition?

> In general I think it's problematic that this is only checked, when the
> config system is called, i.e. with a configured kernel adding ARCH would
> have no effect, what makes this more confusing is that one can later omit
> the ARCH variable, since it's saved in the .config.

The future plan is to set the architecture as part of the configuration step
and not by ARCH=. The ARCH= notation will continue to be supported
but will merely be a hint about the desired architecture.

The only place where it will play a real effect is when generating
allnoconfig, allyesconfig, allmodconfig, randconfig.

Some consistency check will likely be added - but a pure 'make' will
build the kernel with the configured ARCH and the configured CROSS_COMPILE
setting.

One of the blockers are that kconfig does not support more than one prompt
for a choice symbol. Is this something you can fix - or sketch how to fix it?

Sam

2007-11-15 15:43:18

by Roman Zippel

[permalink] [raw]
Subject: Re: [PATCH] kconfig: use $K64BIT to set 64BIT with all*config targets

Hi,

On Wed, 14 Nov 2007, Sam Ravnborg wrote:

> The value can be supplied on the command-line so we need to validate input.

Is there a need for this?
BTW ARCH was already available as a value in the Kconfig files, so setting
the 64BIT option was already possible without any changes the kconfig
system, e.g.:

config 64BIT
bool "64 Bit kernel" if ARCH!="i386" && ARCH!="x86_64"
default ARCH="x86_64"

The patch below adds some features to it:
- it allows to import any environment variable by specifying "option env=..."
- it generates a dependency on it, so the kernel config is updated if it
changes.

Please revert the K64BIT changes and use this instead.

> The code is a copy of what happen when reading a all.config file and
> the functionality should be equal.
> Can we make that part simpler too?

These are two different uses, when reading a .config only the basic syntax
is checked, but not the value itself.

> By the way - I have never understood the purpose of the flags (S_DEF_USER etc.)
> Can we have a few comments added to their definition?

It allows to hold multiple configs, a user of it is conf_split_config()
which loads another config and compares to the current config and updates
the files under include/config as needed.
It could also be used by front ends to display what actually changed
compared to e.g. the saved config.

> One of the blockers are that kconfig does not support more than one prompt
> for a choice symbol. Is this something you can fix - or sketch how to fix it?

The basic idea is to add a name to the choice, so multiple choices can be
grouped together. This requires some changes to the dependency check to
make sure one choice option doesn't depend on another (which is currently
enforced by the syntax).

bye, Roman


Signed-off-by: Roman Zippel <[email protected]>

---
init/Kconfig | 4 ++
scripts/kconfig/expr.c | 16 +++++-----
scripts/kconfig/expr.h | 5 +--
scripts/kconfig/lkc.h | 5 +++
scripts/kconfig/menu.c | 7 +++-
scripts/kconfig/qconf.cc | 15 ++-------
scripts/kconfig/symbol.c | 53 +++++++++++++++++++++++++++++------
scripts/kconfig/util.c | 25 +++++++++++++++-
scripts/kconfig/zconf.gperf | 1
scripts/kconfig/zconf.hash.c_shipped | 45 ++++++++++++++++-------------
10 files changed, 123 insertions(+), 53 deletions(-)

Index: linux-2.6/init/Kconfig
===================================================================
--- linux-2.6.orig/init/Kconfig
+++ linux-2.6/init/Kconfig
@@ -7,6 +7,10 @@ config DEFCONFIG_LIST
default "/boot/config-$UNAME_RELEASE"
default "arch/$ARCH/defconfig"

+config ARCH
+ string
+ option env="ARCH"
+
menu "General setup"

config EXPERIMENTAL
Index: linux-2.6/scripts/kconfig/expr.c
===================================================================
--- linux-2.6.orig/scripts/kconfig/expr.c
+++ linux-2.6/scripts/kconfig/expr.c
@@ -87,7 +87,7 @@ struct expr *expr_copy(struct expr *org)
break;
case E_AND:
case E_OR:
- case E_CHOICE:
+ case E_LIST:
e->left.expr = expr_copy(org->left.expr);
e->right.expr = expr_copy(org->right.expr);
break;
@@ -217,7 +217,7 @@ int expr_eq(struct expr *e1, struct expr
expr_free(e2);
trans_count = old_count;
return res;
- case E_CHOICE:
+ case E_LIST:
case E_RANGE:
case E_NONE:
/* panic */;
@@ -648,7 +648,7 @@ struct expr *expr_transform(struct expr
case E_EQUAL:
case E_UNEQUAL:
case E_SYMBOL:
- case E_CHOICE:
+ case E_LIST:
break;
default:
e->left.expr = expr_transform(e->left.expr);
@@ -932,7 +932,7 @@ struct expr *expr_trans_compare(struct e
break;
case E_SYMBOL:
return expr_alloc_comp(type, e->left.sym, sym);
- case E_CHOICE:
+ case E_LIST:
case E_RANGE:
case E_NONE:
/* panic */;
@@ -1000,9 +1000,9 @@ int expr_compare_type(enum expr_type t1,
if (t2 == E_OR)
return 1;
case E_OR:
- if (t2 == E_CHOICE)
+ if (t2 == E_LIST)
return 1;
- case E_CHOICE:
+ case E_LIST:
if (t2 == 0)
return 1;
default:
@@ -1053,11 +1053,11 @@ void expr_print(struct expr *e, void (*f
fn(data, NULL, " && ");
expr_print(e->right.expr, fn, data, E_AND);
break;
- case E_CHOICE:
+ case E_LIST:
fn(data, e->right.sym, e->right.sym->name);
if (e->left.expr) {
fn(data, NULL, " ^ ");
- expr_print(e->left.expr, fn, data, E_CHOICE);
+ expr_print(e->left.expr, fn, data, E_LIST);
}
break;
case E_RANGE:
Index: linux-2.6/scripts/kconfig/expr.h
===================================================================
--- linux-2.6.orig/scripts/kconfig/expr.h
+++ linux-2.6/scripts/kconfig/expr.h
@@ -32,7 +32,7 @@ typedef enum tristate {
} tristate;

enum expr_type {
- E_NONE, E_OR, E_AND, E_NOT, E_EQUAL, E_UNEQUAL, E_CHOICE, E_SYMBOL, E_RANGE
+ E_NONE, E_OR, E_AND, E_NOT, E_EQUAL, E_UNEQUAL, E_LIST, E_SYMBOL, E_RANGE
};

union expr_data {
@@ -105,7 +105,8 @@ struct symbol {
#define SYMBOL_HASHMASK 0xff

enum prop_type {
- P_UNKNOWN, P_PROMPT, P_COMMENT, P_MENU, P_DEFAULT, P_CHOICE, P_SELECT, P_RANGE
+ P_UNKNOWN, P_PROMPT, P_COMMENT, P_MENU, P_DEFAULT, P_CHOICE,
+ P_SELECT, P_RANGE, P_ENV
};

struct property {
Index: linux-2.6/scripts/kconfig/lkc.h
===================================================================
--- linux-2.6.orig/scripts/kconfig/lkc.h
+++ linux-2.6/scripts/kconfig/lkc.h
@@ -44,6 +44,7 @@ extern "C" {

#define T_OPT_MODULES 1
#define T_OPT_DEFCONFIG_LIST 2
+#define T_OPT_ENV 3

struct kconf_id {
int name;
@@ -74,6 +75,7 @@ void kconfig_load(void);

/* menu.c */
void menu_init(void);
+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);
@@ -103,6 +105,8 @@ void str_printf(struct gstr *gs, const c
const char *str_get(struct gstr *gs);

/* symbol.c */
+struct expr *sym_env_list;
+
void sym_init(void);
void sym_clear_all_valid(void);
void sym_set_all_changed(void);
@@ -110,6 +114,7 @@ void sym_set_changed(struct symbol *sym)
struct symbol *sym_check_deps(struct symbol *sym);
struct property *prop_alloc(enum prop_type type, struct symbol *sym);
struct symbol *prop_get_symbol(struct property *prop);
+struct property *sym_get_env_prop(struct symbol *sym);

static inline tristate sym_get_tristate_value(struct symbol *sym)
{
Index: linux-2.6/scripts/kconfig/menu.c
===================================================================
--- linux-2.6.orig/scripts/kconfig/menu.c
+++ linux-2.6/scripts/kconfig/menu.c
@@ -15,7 +15,7 @@ static struct menu **last_entry_ptr;
struct file *file_list;
struct file *current_file;

-static void menu_warn(struct menu *menu, const char *fmt, ...)
+void menu_warn(struct menu *menu, const char *fmt, ...)
{
va_list ap;
va_start(ap, fmt);
@@ -172,6 +172,9 @@ void menu_add_option(int token, char *ar
else if (sym_defconfig_list != current_entry->sym)
zconf_error("trying to redefine defconfig symbol");
break;
+ case T_OPT_ENV:
+ prop_add_env(arg);
+ break;
}
}

@@ -331,7 +334,7 @@ void menu_finalize(struct menu *parent)
prop = sym_get_choice_prop(sym);
for (ep = &prop->expr; *ep; ep = &(*ep)->left.expr)
;
- *ep = expr_alloc_one(E_CHOICE, NULL);
+ *ep = expr_alloc_one(E_LIST, NULL);
(*ep)->right.sym = menu->sym;
}
if (menu->list && (!menu->prompt || !menu->prompt->text)) {
Index: linux-2.6/scripts/kconfig/qconf.cc
===================================================================
--- linux-2.6.orig/scripts/kconfig/qconf.cc
+++ linux-2.6/scripts/kconfig/qconf.cc
@@ -1083,7 +1083,10 @@ QString ConfigInfoView::debug_info(struc
debug += "</a><br>";
break;
case P_DEFAULT:
- debug += "default: ";
+ case P_SELECT:
+ case P_ENV:
+ debug += prop_get_type_name(prop->type);
+ debug += ": ";
expr_print(prop->expr, expr_print_help, &debug, E_NONE);
debug += "<br>";
break;
@@ -1094,16 +1097,6 @@ QString ConfigInfoView::debug_info(struc
debug += "<br>";
}
break;
- case P_SELECT:
- debug += "select: ";
- expr_print(prop->expr, expr_print_help, &debug, E_NONE);
- debug += "<br>";
- break;
- case P_RANGE:
- debug += "range: ";
- expr_print(prop->expr, expr_print_help, &debug, E_NONE);
- debug += "<br>";
- break;
default:
debug += "unknown property: ";
debug += prop_get_type_name(prop->type);
Index: linux-2.6/scripts/kconfig/symbol.c
===================================================================
--- linux-2.6.orig/scripts/kconfig/symbol.c
+++ linux-2.6/scripts/kconfig/symbol.c
@@ -34,6 +34,8 @@ struct symbol *sym_defconfig_list;
struct symbol *modules_sym;
tristate modules_val;

+struct expr *sym_env_list;
+
void sym_add_default(struct symbol *sym, const char *def)
{
struct property *prop = prop_alloc(P_DEFAULT, sym);
@@ -54,13 +56,6 @@ void sym_init(void)

uname(&uts);

- sym = sym_lookup("ARCH", 0);
- sym->type = S_STRING;
- sym->flags |= SYMBOL_AUTO;
- p = getenv("ARCH");
- if (p)
- sym_add_default(sym, p);
-
sym = sym_lookup("KERNELVERSION", 0);
sym->type = S_STRING;
sym->flags |= SYMBOL_AUTO;
@@ -117,6 +112,15 @@ struct property *sym_get_choice_prop(str
return NULL;
}

+struct property *sym_get_env_prop(struct symbol *sym)
+{
+ struct property *prop;
+
+ for_all_properties(sym, prop, P_ENV)
+ return prop;
+ return NULL;
+}
+
struct property *sym_get_default_prop(struct symbol *sym)
{
struct property *prop;
@@ -347,6 +351,9 @@ void sym_calc_value(struct symbol *sym)
;
}

+ if (sym->flags & SYMBOL_AUTO)
+ sym->flags &= ~SYMBOL_WRITE;
+
sym->curr = newval;
if (sym_is_choice(sym) && newval.tri == yes)
sym->curr.val = sym_calc_choice(sym);
@@ -849,7 +856,7 @@ struct property *prop_alloc(enum prop_ty
struct symbol *prop_get_symbol(struct property *prop)
{
if (prop->expr && (prop->expr->type == E_SYMBOL ||
- prop->expr->type == E_CHOICE))
+ prop->expr->type == E_LIST))
return prop->expr->left.sym;
return NULL;
}
@@ -859,6 +866,8 @@ const char *prop_get_type_name(enum prop
switch (type) {
case P_PROMPT:
return "prompt";
+ case P_ENV:
+ return "env";
case P_COMMENT:
return "comment";
case P_MENU:
@@ -876,3 +885,31 @@ const char *prop_get_type_name(enum prop
}
return "unknown";
}
+
+void prop_add_env(const char *env)
+{
+ struct symbol *sym, *sym2;
+ struct property *prop;
+ char *p;
+
+ sym = current_entry->sym;
+ sym->flags |= SYMBOL_AUTO;
+ for_all_properties(sym, prop, P_ENV) {
+ sym2 = prop_get_symbol(prop);
+ if (strcmp(sym2->name, env))
+ menu_warn(current_entry, "Redefining environment symbol");
+ return;
+ }
+
+ prop = prop_alloc(P_ENV, sym);
+ prop->expr = expr_alloc_symbol(sym_lookup(env, 1));
+
+ sym_env_list = expr_alloc_one(E_LIST, sym_env_list);
+ sym_env_list->right.sym = sym;
+
+ p = getenv(env);
+ if (p)
+ sym_add_default(sym, p);
+ else
+ menu_warn(current_entry, "environment variable %s undefined", sym->name);
+}
Index: linux-2.6/scripts/kconfig/util.c
===================================================================
--- linux-2.6.orig/scripts/kconfig/util.c
+++ linux-2.6/scripts/kconfig/util.c
@@ -29,6 +29,7 @@ struct file *file_lookup(const char *nam
/* write a dependency file as used by kbuild to track dependencies */
int file_write_dep(const char *name)
{
+ struct expr *e;
struct file *file;
FILE *out;

@@ -45,8 +46,28 @@ int file_write_dep(const char *name)
fprintf(out, "\t%s\n", file->name);
}
fprintf(out, "\ninclude/config/auto.conf: \\\n"
- "\t$(deps_config)\n\n"
- "$(deps_config): ;\n");
+ "\t$(deps_config)\n\n");
+
+ for (e = sym_env_list; e; e = e->left.expr) {
+ struct property *p;
+ struct symbol *sym, *env_sym;
+ const char *value;
+
+ sym = e->right.sym;
+ p = sym_get_env_prop(sym);
+ env_sym = prop_get_symbol(p);
+ if (!env_sym)
+ continue;
+ value = sym_get_string_value(sym);
+ if ((sym->type == S_BOOLEAN || sym->type == S_TRISTATE) &&
+ sym_get_tristate_value(sym) == no)
+ value = "";
+ fprintf(out, "ifneq \"$(%s)\" \"%s\"\n", env_sym->name, value);
+ fprintf(out, "include/config/auto.conf: FORCE\n");
+ fprintf(out, "endif\n");
+ }
+
+ fprintf(out, "\n$(deps_config): ;\n");
fclose(out);
rename("..config.tmp", name);
return 0;
Index: linux-2.6/scripts/kconfig/zconf.gperf
===================================================================
--- linux-2.6.orig/scripts/kconfig/zconf.gperf
+++ linux-2.6/scripts/kconfig/zconf.gperf
@@ -41,4 +41,5 @@ option, T_OPTION, TF_COMMAND
on, T_ON, TF_PARAM
modules, T_OPT_MODULES, TF_OPTION
defconfig_list, T_OPT_DEFCONFIG_LIST,TF_OPTION
+env, T_OPT_ENV, TF_OPTION
%%
Index: linux-2.6/scripts/kconfig/zconf.hash.c_shipped
===================================================================
--- linux-2.6.orig/scripts/kconfig/zconf.hash.c_shipped
+++ linux-2.6/scripts/kconfig/zconf.hash.c_shipped
@@ -53,10 +53,10 @@ kconf_id_hash (register const char *str,
49, 49, 49, 49, 49, 49, 49, 49, 49, 49,
49, 49, 49, 49, 49, 49, 49, 49, 49, 49,
49, 49, 49, 49, 49, 49, 49, 49, 49, 49,
- 49, 49, 49, 49, 49, 49, 49, 18, 11, 5,
+ 49, 49, 49, 49, 49, 49, 49, 35, 35, 5,
0, 0, 5, 49, 5, 20, 49, 49, 5, 20,
- 5, 0, 30, 49, 0, 15, 0, 10, 49, 49,
- 25, 49, 49, 49, 49, 49, 49, 49, 49, 49,
+ 5, 0, 30, 49, 0, 15, 0, 10, 0, 49,
+ 10, 49, 49, 49, 49, 49, 49, 49, 49, 49,
49, 49, 49, 49, 49, 49, 49, 49, 49, 49,
49, 49, 49, 49, 49, 49, 49, 49, 49, 49,
49, 49, 49, 49, 49, 49, 49, 49, 49, 49,
@@ -89,6 +89,7 @@ kconf_id_hash (register const char *str,
struct kconf_id_strings_t
{
char kconf_id_strings_str2[sizeof("on")];
+ char kconf_id_strings_str3[sizeof("env")];
char kconf_id_strings_str5[sizeof("endif")];
char kconf_id_strings_str6[sizeof("option")];
char kconf_id_strings_str7[sizeof("endmenu")];
@@ -99,30 +100,31 @@ struct kconf_id_strings_t
char kconf_id_strings_str12[sizeof("default")];
char kconf_id_strings_str13[sizeof("def_bool")];
char kconf_id_strings_str14[sizeof("help")];
- char kconf_id_strings_str15[sizeof("bool")];
char kconf_id_strings_str16[sizeof("config")];
char kconf_id_strings_str17[sizeof("def_tristate")];
- char kconf_id_strings_str18[sizeof("boolean")];
+ char kconf_id_strings_str18[sizeof("hex")];
char kconf_id_strings_str19[sizeof("defconfig_list")];
char kconf_id_strings_str21[sizeof("string")];
char kconf_id_strings_str22[sizeof("if")];
char kconf_id_strings_str23[sizeof("int")];
- char kconf_id_strings_str24[sizeof("enable")];
char kconf_id_strings_str26[sizeof("select")];
char kconf_id_strings_str27[sizeof("modules")];
char kconf_id_strings_str28[sizeof("tristate")];
char kconf_id_strings_str29[sizeof("menu")];
char kconf_id_strings_str31[sizeof("source")];
char kconf_id_strings_str32[sizeof("comment")];
- char kconf_id_strings_str33[sizeof("hex")];
char kconf_id_strings_str35[sizeof("menuconfig")];
char kconf_id_strings_str36[sizeof("prompt")];
char kconf_id_strings_str37[sizeof("depends")];
+ char kconf_id_strings_str39[sizeof("bool")];
+ char kconf_id_strings_str41[sizeof("enable")];
+ char kconf_id_strings_str42[sizeof("boolean")];
char kconf_id_strings_str48[sizeof("mainmenu")];
};
static struct kconf_id_strings_t kconf_id_strings_contents =
{
"on",
+ "env",
"endif",
"option",
"endmenu",
@@ -133,25 +135,25 @@ static struct kconf_id_strings_t kconf_i
"default",
"def_bool",
"help",
- "bool",
"config",
"def_tristate",
- "boolean",
+ "hex",
"defconfig_list",
"string",
"if",
"int",
- "enable",
"select",
"modules",
"tristate",
"menu",
"source",
"comment",
- "hex",
"menuconfig",
"prompt",
"depends",
+ "bool",
+ "enable",
+ "boolean",
"mainmenu"
};
#define kconf_id_strings ((const char *) &kconf_id_strings_contents)
@@ -163,7 +165,7 @@ kconf_id_lookup (register const char *st
{
enum
{
- TOTAL_KEYWORDS = 31,
+ TOTAL_KEYWORDS = 32,
MIN_WORD_LENGTH = 2,
MAX_WORD_LENGTH = 14,
MIN_HASH_VALUE = 2,
@@ -174,7 +176,8 @@ kconf_id_lookup (register const char *st
{
{-1}, {-1},
{(int)(long)&((struct kconf_id_strings_t *)0)->kconf_id_strings_str2, T_ON, TF_PARAM},
- {-1}, {-1},
+ {(int)(long)&((struct kconf_id_strings_t *)0)->kconf_id_strings_str3, T_OPT_ENV, TF_OPTION},
+ {-1},
{(int)(long)&((struct kconf_id_strings_t *)0)->kconf_id_strings_str5, T_ENDIF, TF_COMMAND},
{(int)(long)&((struct kconf_id_strings_t *)0)->kconf_id_strings_str6, T_OPTION, TF_COMMAND},
{(int)(long)&((struct kconf_id_strings_t *)0)->kconf_id_strings_str7, T_ENDMENU, TF_COMMAND},
@@ -185,17 +188,16 @@ kconf_id_lookup (register const char *st
{(int)(long)&((struct kconf_id_strings_t *)0)->kconf_id_strings_str12, T_DEFAULT, TF_COMMAND, S_UNKNOWN},
{(int)(long)&((struct kconf_id_strings_t *)0)->kconf_id_strings_str13, T_DEFAULT, TF_COMMAND, S_BOOLEAN},
{(int)(long)&((struct kconf_id_strings_t *)0)->kconf_id_strings_str14, T_HELP, TF_COMMAND},
- {(int)(long)&((struct kconf_id_strings_t *)0)->kconf_id_strings_str15, T_TYPE, TF_COMMAND, S_BOOLEAN},
+ {-1},
{(int)(long)&((struct kconf_id_strings_t *)0)->kconf_id_strings_str16, T_CONFIG, TF_COMMAND},
{(int)(long)&((struct kconf_id_strings_t *)0)->kconf_id_strings_str17, T_DEFAULT, TF_COMMAND, S_TRISTATE},
- {(int)(long)&((struct kconf_id_strings_t *)0)->kconf_id_strings_str18, T_TYPE, TF_COMMAND, S_BOOLEAN},
+ {(int)(long)&((struct kconf_id_strings_t *)0)->kconf_id_strings_str18, T_TYPE, TF_COMMAND, S_HEX},
{(int)(long)&((struct kconf_id_strings_t *)0)->kconf_id_strings_str19, T_OPT_DEFCONFIG_LIST,TF_OPTION},
{-1},
{(int)(long)&((struct kconf_id_strings_t *)0)->kconf_id_strings_str21, T_TYPE, TF_COMMAND, S_STRING},
{(int)(long)&((struct kconf_id_strings_t *)0)->kconf_id_strings_str22, T_IF, TF_COMMAND|TF_PARAM},
{(int)(long)&((struct kconf_id_strings_t *)0)->kconf_id_strings_str23, T_TYPE, TF_COMMAND, S_INT},
- {(int)(long)&((struct kconf_id_strings_t *)0)->kconf_id_strings_str24, T_SELECT, TF_COMMAND},
- {-1},
+ {-1}, {-1},
{(int)(long)&((struct kconf_id_strings_t *)0)->kconf_id_strings_str26, T_SELECT, TF_COMMAND},
{(int)(long)&((struct kconf_id_strings_t *)0)->kconf_id_strings_str27, T_OPT_MODULES, TF_OPTION},
{(int)(long)&((struct kconf_id_strings_t *)0)->kconf_id_strings_str28, T_TYPE, TF_COMMAND, S_TRISTATE},
@@ -203,13 +205,16 @@ kconf_id_lookup (register const char *st
{-1},
{(int)(long)&((struct kconf_id_strings_t *)0)->kconf_id_strings_str31, T_SOURCE, TF_COMMAND},
{(int)(long)&((struct kconf_id_strings_t *)0)->kconf_id_strings_str32, T_COMMENT, TF_COMMAND},
- {(int)(long)&((struct kconf_id_strings_t *)0)->kconf_id_strings_str33, T_TYPE, TF_COMMAND, S_HEX},
- {-1},
+ {-1}, {-1},
{(int)(long)&((struct kconf_id_strings_t *)0)->kconf_id_strings_str35, T_MENUCONFIG, TF_COMMAND},
{(int)(long)&((struct kconf_id_strings_t *)0)->kconf_id_strings_str36, T_PROMPT, TF_COMMAND},
{(int)(long)&((struct kconf_id_strings_t *)0)->kconf_id_strings_str37, T_DEPENDS, TF_COMMAND},
- {-1}, {-1}, {-1}, {-1}, {-1}, {-1}, {-1}, {-1}, {-1},
{-1},
+ {(int)(long)&((struct kconf_id_strings_t *)0)->kconf_id_strings_str39, T_TYPE, TF_COMMAND, S_BOOLEAN},
+ {-1},
+ {(int)(long)&((struct kconf_id_strings_t *)0)->kconf_id_strings_str41, T_SELECT, TF_COMMAND},
+ {(int)(long)&((struct kconf_id_strings_t *)0)->kconf_id_strings_str42, T_TYPE, TF_COMMAND, S_BOOLEAN},
+ {-1}, {-1}, {-1}, {-1}, {-1},
{(int)(long)&((struct kconf_id_strings_t *)0)->kconf_id_strings_str48, T_MAINMENU, TF_COMMAND}
};

2007-11-15 19:24:24

by Sam Ravnborg

[permalink] [raw]
Subject: Re: [PATCH] kconfig: use $K64BIT to set 64BIT with all*config targets

On Thu, Nov 15, 2007 at 04:43:03PM +0100, Roman Zippel wrote:
> Hi,
>
> On Wed, 14 Nov 2007, Sam Ravnborg wrote:
>
> > The value can be supplied on the command-line so we need to validate input.
>
> Is there a need for this?
Yes. We would like to set 64BIT or not in other than x86 cases.
And way forward was not to override ARCH as in the x86 case.


> BTW ARCH was already available as a value in the Kconfig files, so setting
> the 64BIT option was already possible without any changes the kconfig
> system, e.g.:
>
> config 64BIT
> bool "64 Bit kernel" if ARCH!="i386" && ARCH!="x86_64"
> default ARCH="x86_64"

I thought this was not possible but it must have been the limitation
of choice symbols I have hit when I played with it.


> The patch below adds some features to it:
> - it allows to import any environment variable by specifying "option env=..."
> - it generates a dependency on it, so the kernel config is updated if it
> changes.
>
> Please revert the K64BIT changes and use this instead.

I will finish up your patch and target it for next merge window.
Thanks.

>
> > The code is a copy of what happen when reading a all.config file and
> > the functionality should be equal.
> > Can we make that part simpler too?
>
> These are two different uses, when reading a .config only the basic syntax
> is checked, but not the value itself.
This is wrong considering the amount of people that hand edit the .config file.


> > By the way - I have never understood the purpose of the flags (S_DEF_USER etc.)
> > Can we have a few comments added to their definition?
>
> It allows to hold multiple configs, a user of it is conf_split_config()
> which loads another config and compares to the current config and updates
> the files under include/config as needed.
> It could also be used by front ends to display what actually changed
> compared to e.g. the saved config.

Took a deeper look.
So we can have 4 different set of vlaues where the value indexed by S_DEF_USER
is the one that is actually used and the other three can be used to hold values.

I will try to document this in expr.h
Patch will be sent to you for review.

> > One of the blockers are that kconfig does not support more than one prompt
> > for a choice symbol. Is this something you can fix - or sketch how to fix it?
>
> The basic idea is to add a name to the choice, so multiple choices can be
> grouped together. This requires some changes to the dependency check to
> make sure one choice option doesn't depend on another (which is currently
> enforced by the syntax).
Do you have any suggestions how to properly fix this?

Sam

2007-11-15 19:43:39

by Roman Zippel

[permalink] [raw]
Subject: Re: [PATCH] kconfig: use $K64BIT to set 64BIT with all*config targets

Hi,

On Thu, 15 Nov 2007, Sam Ravnborg wrote:

> > > The value can be supplied on the command-line so we need to validate input.
> >
> > Is there a need for this?
> Yes. We would like to set 64BIT or not in other than x86 cases.
> And way forward was not to override ARCH as in the x86 case.

Can we please can get some consistency in this?
We have a .config file for a reason, what's wrong with using it?

> > Please revert the K64BIT changes and use this instead.
>
> I will finish up your patch and target it for next merge window.

Why can't this be fixed properly now? You don't even need this patch, just
use ARCH to set 64BIT in the Kconfig as I've shown.

> > These are two different uses, when reading a .config only the basic syntax
> > is checked, but not the value itself.
> This is wrong considering the amount of people that hand edit the .config file.

It's not, the actual symbol value is set later depending on the dependency
constraints.

bye, Roman

2007-11-15 20:43:52

by Sam Ravnborg

[permalink] [raw]
Subject: Re: [PATCH] kconfig: use $K64BIT to set 64BIT with all*config targets

Hi Roman.

> > > > The value can be supplied on the command-line so we need to validate input.
> > >
> > > Is there a need for this?
> > Yes. We would like to set 64BIT or not in other than x86 cases.
> > And way forward was not to override ARCH as in the x86 case.
>
> Can we please can get some consistency in this?
> We have a .config file for a reason, what's wrong with using it?

We need to set a selected few values in a few cases where we do
not have a .config file.
allmodconfig for x86 for instance. We would like to generate a
32-bit and a 64-bit version.

>
> > > Please revert the K64BIT changes and use this instead.
> >
> > I will finish up your patch and target it for next merge window.
>
> Why can't this be fixed properly now? You don't even need this patch, just
> use ARCH to set 64BIT in the Kconfig as I've shown.
Because the patch is in mainline and has been tested by a lot of people
during the last week. And as the functionality is almost equal I do not
see it as a big deal to have the less-perfect solution in one kernel release.

And the only reason the patch were applied to mainline was to fix the build
of the merged x86 architecute - otherwise it was in no way -rc material.
>
> > > These are two different uses, when reading a .config only the basic syntax
> > > is checked, but not the value itself.
> > This is wrong considering the amount of people that hand edit the .config file.
>
> It's not, the actual symbol value is set later depending on the dependency
> constraints.
My point is that the .config file is handedited so the syntax should be checked
to best possible extent. If someone specify CONFIG_64BIT=64 we should error out.

Sam

2007-11-15 21:24:22

by Roman Zippel

[permalink] [raw]
Subject: Re: [PATCH] kconfig: use $K64BIT to set 64BIT with all*config targets

Hi,

On Thu, 15 Nov 2007, Sam Ravnborg wrote:

> > Can we please can get some consistency in this?
> > We have a .config file for a reason, what's wrong with using it?
>
> We need to set a selected few values in a few cases where we do
> not have a .config file.
> allmodconfig for x86 for instance. We would like to generate a
> 32-bit and a 64-bit version.

This can already be set via all.config/allmod.config.
Where is this need coming from? The point is that I don't like to add an
interface, which is maybe used by two people, who should be perfectly
capable using an existing superior mechanism.

> > > > Please revert the K64BIT changes and use this instead.
> > >
> > > I will finish up your patch and target it for next merge window.
> >
> > Why can't this be fixed properly now? You don't even need this patch, just
> > use ARCH to set 64BIT in the Kconfig as I've shown.
> Because the patch is in mainline and has been tested by a lot of people
> during the last week. And as the functionality is almost equal I do not
> see it as a big deal to have the less-perfect solution in one kernel release.
>
> And the only reason the patch were applied to mainline was to fix the build
> of the merged x86 architecute - otherwise it was in no way -rc material.

I showed you a solution, which requires no patch at all, while your patch
adds extra functionality which is questionable.
Why is a quick hack preferable over a proper solution?
Either explain why my solution isn't usable or _please_ revert the K64BIT
changes.

> > > > These are two different uses, when reading a .config only the basic syntax
> > > > is checked, but not the value itself.
> > > This is wrong considering the amount of people that hand edit the .config file.
> >
> > It's not, the actual symbol value is set later depending on the dependency
> > constraints.
> My point is that the .config file is handedited so the syntax should be checked
> to best possible extent. If someone specify CONFIG_64BIT=64 we should error out.

The other function doesn't complain about it either. There is already
only limited error checking, e.g. there is no complaint that the value
isn't really set (because it was selected by something else), otherwise
the .config check during a kernel upgrades would be a lot noisier than it
already is. Anyone directly editing .config should know what he is doing.

bye, Roman

2007-11-15 22:05:37

by Sam Ravnborg

[permalink] [raw]
Subject: Re: [PATCH] kconfig: use $K64BIT to set 64BIT with all*config targets

On Thu, Nov 15, 2007 at 10:24:05PM +0100, Roman Zippel wrote:
> Hi,
>
> On Thu, 15 Nov 2007, Sam Ravnborg wrote:
>
> > > Can we please can get some consistency in this?
> > > We have a .config file for a reason, what's wrong with using it?
> >
> > We need to set a selected few values in a few cases where we do
> > not have a .config file.
> > allmodconfig for x86 for instance. We would like to generate a
> > 32-bit and a 64-bit version.
>
> This can already be set via all.config/allmod.config.
> Where is this need coming from? The point is that I don't like to add an
> interface, which is maybe used by two people, who should be perfectly
> capable using an existing superior mechanism.

When you say "two people" I am afraid we are not talking about the same case.

>
> > > > > Please revert the K64BIT changes and use this instead.
> > > >
> > > > I will finish up your patch and target it for next merge window.
> > >
> > > Why can't this be fixed properly now? You don't even need this patch, just
> > > use ARCH to set 64BIT in the Kconfig as I've shown.
> > Because the patch is in mainline and has been tested by a lot of people
> > during the last week. And as the functionality is almost equal I do not
> > see it as a big deal to have the less-perfect solution in one kernel release.
> >
> > And the only reason the patch were applied to mainline was to fix the build
> > of the merged x86 architecute - otherwise it was in no way -rc material.
>
> I showed you a solution, which requires no patch at all, while your patch
> adds extra functionality which is questionable.
> Why is a quick hack preferable over a proper solution?
> Either explain why my solution isn't usable or _please_ revert the K64BIT
> changes.
You suggest just to check ARCH value and not apply your patch. This was
not my initial understanding as was hopefully obvious from my reply.

So you suggest to make the ARCH= setting on the command line mandatory
again even for a configured kernel which is a step backward.

I assume your patch also has this drawback - no?

If user did NOT specify ARCH we should use the kernel configuration - which
your solution fail to do.

If user did specify ARCH and the kernel is configured then what?
-> Ignore the ARCH= setting
-> Warn if they do not match
-> Always adhere to the ARCH setting

Accepting the solution where we check the ARCH as you suggested and
loose the benefits of letting the configuration be the master should
be OK for upcoming kernel release.

Sam

2007-11-16 01:28:23

by Roman Zippel

[permalink] [raw]
Subject: Re: [PATCH] kconfig: use $K64BIT to set 64BIT with all*config targets

Hi,

On Thu, 15 Nov 2007, Sam Ravnborg wrote:

> You suggest just to check ARCH value and not apply your patch. This was
> not my initial understanding as was hopefully obvious from my reply.

This patch only adds some extra features.

> If user did NOT specify ARCH we should use the kernel configuration - which
> your solution fail to do.

To make this easy I attached the patch which reverts the problematic
changes and then you only need this simple change to force the 64BIT value
for ARCH={i386,x86_64}, otherwise it's set by the user:

bye, Roman

Signed-off-by: Roman Zippel <[email protected]>

---
Makefile | 3 ++-
arch/x86/Kconfig | 4 ++--
2 files changed, 4 insertions(+), 3 deletions(-)

Index: linux-2.6/Makefile
===================================================================
--- linux-2.6.orig/Makefile
+++ linux-2.6/Makefile
@@ -165,7 +165,8 @@ export srctree objtree VPATH TOPDIR
# then ARCH is assigned, getting whatever value it gets normally, and
# SUBARCH is subsequently ignored.

-SUBARCH := $(shell uname -m | sed -e s/i.86/i386/ -e s/sun4u/sparc64/ \
+SUBARCH := $(shell uname -m | sed -e s/i.86/x86/ -e s/x86_64/x86/ \
+ -e s/sun4u/sparc64/ \
-e s/arm.*/arm/ -e s/sa110/arm/ \
-e s/s390x/s390/ -e s/parisc64/parisc/ \
-e s/ppc.*/powerpc/ -e s/mips.*/mips/ \
Index: linux-2.6/arch/x86/Kconfig
===================================================================
--- linux-2.6.orig/arch/x86/Kconfig
+++ linux-2.6/arch/x86/Kconfig
@@ -3,8 +3,8 @@ mainmenu "Linux Kernel Configuration for

# Select 32 or 64 bit
config 64BIT
- bool "64-bit kernel"
- default n
+ bool "64-bit kernel" if ARCH="x86"
+ default ARCH="x86_64"
help
Say yes to build a 64-bit kernel - formerly known as x86_64
Say no to build a 32-bit kernel - formerly known as i386


Attachments:
revert_k64bit (6.69 kB)

2007-11-16 03:44:46

by Randy Dunlap

[permalink] [raw]
Subject: Re: [PATCH] kconfig: use $K64BIT to set 64BIT with all*config targets

On Fri, 16 Nov 2007 02:28:09 +0100 (CET) Roman Zippel wrote:

> Hi,
>
> On Thu, 15 Nov 2007, Sam Ravnborg wrote:
>
> > You suggest just to check ARCH value and not apply your patch. This was
> > not my initial understanding as was hopefully obvious from my reply.
>
> This patch only adds some extra features.
>
> > If user did NOT specify ARCH we should use the kernel configuration - which
> > your solution fail to do.
>
> To make this easy I attached the patch which reverts the problematic
> changes and then you only need this simple change to force the 64BIT value
> for ARCH={i386,x86_64}, otherwise it's set by the user:

Roman,

This all began (AFAIK) because some of us want to continue to be
able to specify ARCH={i386,x86_64} on the (make) command line --
not by using a .config file. Taking away ARCH= on the command line
is a regression (in some minds, at least), so Sam provided that
capability. Is that capability still present after this patch?

Thanks.


> bye, Roman
>
> Signed-off-by: Roman Zippel <[email protected]>
>
> ---
> Makefile | 3 ++-
> arch/x86/Kconfig | 4 ++--
> 2 files changed, 4 insertions(+), 3 deletions(-)
>
> Index: linux-2.6/Makefile
> ===================================================================
> --- linux-2.6.orig/Makefile
> +++ linux-2.6/Makefile
> @@ -165,7 +165,8 @@ export srctree objtree VPATH TOPDIR
> # then ARCH is assigned, getting whatever value it gets normally, and
> # SUBARCH is subsequently ignored.
>
> -SUBARCH := $(shell uname -m | sed -e s/i.86/i386/ -e s/sun4u/sparc64/ \
> +SUBARCH := $(shell uname -m | sed -e s/i.86/x86/ -e s/x86_64/x86/ \
> + -e s/sun4u/sparc64/ \
> -e s/arm.*/arm/ -e s/sa110/arm/ \
> -e s/s390x/s390/ -e s/parisc64/parisc/ \
> -e s/ppc.*/powerpc/ -e s/mips.*/mips/ \
> Index: linux-2.6/arch/x86/Kconfig
> ===================================================================
> --- linux-2.6.orig/arch/x86/Kconfig
> +++ linux-2.6/arch/x86/Kconfig
> @@ -3,8 +3,8 @@ mainmenu "Linux Kernel Configuration for
>
> # Select 32 or 64 bit
> config 64BIT
> - bool "64-bit kernel"
> - default n
> + bool "64-bit kernel" if ARCH="x86"
> + default ARCH="x86_64"
> help
> Say yes to build a 64-bit kernel - formerly known as x86_64
> Say no to build a 32-bit kernel - formerly known as i386


---
~Randy

2007-11-16 05:39:55

by Sam Ravnborg

[permalink] [raw]
Subject: Re: [PATCH] kconfig: use $K64BIT to set 64BIT with all*config targets

Hi Roman.
> > If user did NOT specify ARCH we should use the kernel configuration - which
> > your solution fail to do.
>
> To make this easy I attached the patch which reverts the problematic
> changes and then you only need this simple change to force the 64BIT value
> for ARCH={i386,x86_64}, otherwise it's set by the user:

Just eyeballing your patch I made following observations:
1) make all*config, randconfig, defconfig is broken on 64-bit boxes
2) A pure code refactoring patch is reverted for no obvious reason
3) Behavioral changes are not documented:
- 32-bit/64-bit can only be selected in config is you specify ARCH=x86
- ARCH= takes precedence over kernel config for a configured kernel
4) The changelogs miss title on reverted patches

All points are trivial to fix so I do not say your approach is
bad - just that the supplied patch is not good enough.

I will fix it up tonight and test it.

Sam

2007-11-16 12:54:17

by Roman Zippel

[permalink] [raw]
Subject: Re: [PATCH] kconfig: use $K64BIT to set 64BIT with all*config targets

Hi,

On Fri, 16 Nov 2007, Sam Ravnborg wrote:

> 1) make all*config, randconfig, defconfig is broken on 64-bit boxes

With your approach you made it impossible to set 64BIT from all*.config,
which is the proper way to set the defaults.

> 2) A pure code refactoring patch is reverted for no obvious reason

It was done for the wrong reasons, otherwise the warning before it should
have been included as well and then the function could have been reused
for the "# ... is not set" case as well.

> 3) Behavioral changes are not documented:
> - 32-bit/64-bit can only be selected in config is you specify ARCH=x86

Which is now the default and thus it behaves more like other archs.

> - ARCH= takes precedence over kernel config for a configured kernel

What point is there in being able to specify ARCH=x86_64 and then still
produce a 32bit kernel?

> 4) The changelogs miss title on reverted patches

Seriously?

bye, Roman

2007-11-16 13:03:06

by Roman Zippel

[permalink] [raw]
Subject: Re: [PATCH] kconfig: use $K64BIT to set 64BIT with all*config targets

Hi,

On Thu, 15 Nov 2007, Randy Dunlap wrote:

> This all began (AFAIK) because some of us want to continue to be
> able to specify ARCH={i386,x86_64} on the (make) command line --
> not by using a .config file. Taking away ARCH= on the command line
> is a regression (in some minds, at least), so Sam provided that
> capability. Is that capability still present after this patch?

My patch does exactly that.
Sam's patch only made a subtle difference for all*config, but otherwise it
would have made little difference, i.e. you still had to edit .config.

The maybe only remaining issue is which default config to use for
defconfig, when ARCH isn't specified, but that is purely a Kbuild issue.

bye, Roman

2008-01-06 13:26:18

by Sam Ravnborg

[permalink] [raw]
Subject: kconfig: support option env="" [Was: kconfig: use $K64BIT to set 64BIT with all*config targets]

Hi Roman.

Some time ago you sent the following patch which I have started
to review properly and test.
But it triggers a few questions / comments.

For reference (since it is so long ago I have kept most
of the patch but only a few places are commented.

Please get back to me so we can finsih this patch and have it applied.
I will split the patch in two btw.
One where option env= is introduced
and a second patch that kill the three hardcoded variables
in symbol.c (ARCH, KERNELVERSION and UNAME_RELEASE).

Sam

> The patch below adds some features to it:
> - it allows to import any environment variable by specifying "option env=..."
> - it generates a dependency on it, so the kernel config is updated if it
> changes.
>
>
> Signed-off-by: Roman Zippel <[email protected]>
>
> ---
> init/Kconfig | 4 ++
> scripts/kconfig/expr.c | 16 +++++-----
> scripts/kconfig/expr.h | 5 +--
> scripts/kconfig/lkc.h | 5 +++
> scripts/kconfig/menu.c | 7 +++-
> scripts/kconfig/qconf.cc | 15 ++-------
> scripts/kconfig/symbol.c | 53 +++++++++++++++++++++++++++++------
> scripts/kconfig/util.c | 25 +++++++++++++++-
> scripts/kconfig/zconf.gperf | 1
> scripts/kconfig/zconf.hash.c_shipped | 45 ++++++++++++++++-------------
> 10 files changed, 123 insertions(+), 53 deletions(-)
>
> Index: linux-2.6/init/Kconfig
> ===================================================================
> --- linux-2.6.orig/init/Kconfig
> +++ linux-2.6/init/Kconfig
> @@ -7,6 +7,10 @@ config DEFCONFIG_LIST
> default "/boot/config-$UNAME_RELEASE"
> default "arch/$ARCH/defconfig"
>
> +config ARCH
> + string
> + option env="ARCH"
> +
> menu "General setup"
>
> config EXPERIMENTAL
> Index: linux-2.6/scripts/kconfig/expr.c
> ===================================================================
> --- linux-2.6.orig/scripts/kconfig/expr.c
> +++ linux-2.6/scripts/kconfig/expr.c
> @@ -87,7 +87,7 @@ struct expr *expr_copy(struct expr *org)
> break;
> case E_AND:
> case E_OR:
> - case E_CHOICE:
> + case E_LIST:
> e->left.expr = expr_copy(org->left.expr);
> e->right.expr = expr_copy(org->right.expr);
> break;
> @@ -217,7 +217,7 @@ int expr_eq(struct expr *e1, struct expr
> expr_free(e2);
> trans_count = old_count;
> return res;
> - case E_CHOICE:
> + case E_LIST:
> case E_RANGE:
> case E_NONE:
> /* panic */;
> @@ -648,7 +648,7 @@ struct expr *expr_transform(struct expr
> case E_EQUAL:
> case E_UNEQUAL:
> case E_SYMBOL:
> - case E_CHOICE:
> + case E_LIST:
> break;
> default:
> e->left.expr = expr_transform(e->left.expr);
> @@ -932,7 +932,7 @@ struct expr *expr_trans_compare(struct e
> break;
> case E_SYMBOL:
> return expr_alloc_comp(type, e->left.sym, sym);
> - case E_CHOICE:
> + case E_LIST:
> case E_RANGE:
> case E_NONE:
> /* panic */;
> @@ -1000,9 +1000,9 @@ int expr_compare_type(enum expr_type t1,
> if (t2 == E_OR)
> return 1;
> case E_OR:
> - if (t2 == E_CHOICE)
> + if (t2 == E_LIST)
> return 1;
> - case E_CHOICE:
> + case E_LIST:
> if (t2 == 0)
> return 1;
> default:
> @@ -1053,11 +1053,11 @@ void expr_print(struct expr *e, void (*f
> fn(data, NULL, " && ");
> expr_print(e->right.expr, fn, data, E_AND);
> break;
> - case E_CHOICE:
> + case E_LIST:
> fn(data, e->right.sym, e->right.sym->name);
> if (e->left.expr) {
> fn(data, NULL, " ^ ");
> - expr_print(e->left.expr, fn, data, E_CHOICE);
> + expr_print(e->left.expr, fn, data, E_LIST);
> }
> break;
> case E_RANGE:
> Index: linux-2.6/scripts/kconfig/expr.h
> ===================================================================
> --- linux-2.6.orig/scripts/kconfig/expr.h
> +++ linux-2.6/scripts/kconfig/expr.h
> @@ -32,7 +32,7 @@ typedef enum tristate {
> } tristate;
>
> enum expr_type {
> - E_NONE, E_OR, E_AND, E_NOT, E_EQUAL, E_UNEQUAL, E_CHOICE, E_SYMBOL, E_RANGE
> + E_NONE, E_OR, E_AND, E_NOT, E_EQUAL, E_UNEQUAL, E_LIST, E_SYMBOL, E_RANGE
> };
>
> union expr_data {
> @@ -105,7 +105,8 @@ struct symbol {
> #define SYMBOL_HASHMASK 0xff
>
> enum prop_type {
> - P_UNKNOWN, P_PROMPT, P_COMMENT, P_MENU, P_DEFAULT, P_CHOICE, P_SELECT, P_RANGE
> + P_UNKNOWN, P_PROMPT, P_COMMENT, P_MENU, P_DEFAULT, P_CHOICE,
> + P_SELECT, P_RANGE, P_ENV
> };
>
> struct property {
> Index: linux-2.6/scripts/kconfig/lkc.h
> ===================================================================
> --- linux-2.6.orig/scripts/kconfig/lkc.h
> +++ linux-2.6/scripts/kconfig/lkc.h
> @@ -44,6 +44,7 @@ extern "C" {
>
> #define T_OPT_MODULES 1
> #define T_OPT_DEFCONFIG_LIST 2
> +#define T_OPT_ENV 3
>
> struct kconf_id {
> int name;
> @@ -74,6 +75,7 @@ void kconfig_load(void);
>
> /* menu.c */
> void menu_init(void);
> +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);
> @@ -103,6 +105,8 @@ void str_printf(struct gstr *gs, const c
> const char *str_get(struct gstr *gs);
>
> /* symbol.c */
> +struct expr *sym_env_list;

As this is in a .h file I assume it should be extern.
So I did:
> +extern struct expr *sym_env_list;
> +
> void sym_init(void);
> void sym_clear_all_valid(void);
> void sym_set_all_changed(void);
> @@ -110,6 +114,7 @@ void sym_set_changed(struct symbol *sym)
> struct symbol *sym_check_deps(struct symbol *sym);
> struct property *prop_alloc(enum prop_type type, struct symbol *sym);
> struct symbol *prop_get_symbol(struct property *prop);
> +struct property *sym_get_env_prop(struct symbol *sym);
>
> static inline tristate sym_get_tristate_value(struct symbol *sym)
> {
> Index: linux-2.6/scripts/kconfig/menu.c
> ===================================================================
> --- linux-2.6.orig/scripts/kconfig/menu.c
> +++ linux-2.6/scripts/kconfig/menu.c
> @@ -15,7 +15,7 @@ static struct menu **last_entry_ptr;
> struct file *file_list;
> struct file *current_file;
>
> -static void menu_warn(struct menu *menu, const char *fmt, ...)
> +void menu_warn(struct menu *menu, const char *fmt, ...)
> {
> va_list ap;
> va_start(ap, fmt);
> @@ -172,6 +172,9 @@ void menu_add_option(int token, char *ar
> else if (sym_defconfig_list != current_entry->sym)
> zconf_error("trying to redefine defconfig symbol");
> break;
> + case T_OPT_ENV:
> + prop_add_env(arg);
> + break;
> }
> }
>
> @@ -331,7 +334,7 @@ void menu_finalize(struct menu *parent)
> prop = sym_get_choice_prop(sym);
> for (ep = &prop->expr; *ep; ep = &(*ep)->left.expr)
> ;
> - *ep = expr_alloc_one(E_CHOICE, NULL);
> + *ep = expr_alloc_one(E_LIST, NULL);
> (*ep)->right.sym = menu->sym;
> }
> if (menu->list && (!menu->prompt || !menu->prompt->text)) {
> Index: linux-2.6/scripts/kconfig/qconf.cc
> ===================================================================
> --- linux-2.6.orig/scripts/kconfig/qconf.cc
> +++ linux-2.6/scripts/kconfig/qconf.cc
> @@ -1083,7 +1083,10 @@ QString ConfigInfoView::debug_info(struc
> debug += "</a><br>";
> break;
> case P_DEFAULT:
> - debug += "default: ";
> + case P_SELECT:
> + case P_ENV:
> + debug += prop_get_type_name(prop->type);
> + debug += ": ";
> expr_print(prop->expr, expr_print_help, &debug, E_NONE);
> debug += "<br>";
> break;
> @@ -1094,16 +1097,6 @@ QString ConfigInfoView::debug_info(struc
> debug += "<br>";
> }
> break;
> - case P_SELECT:
> - debug += "select: ";
> - expr_print(prop->expr, expr_print_help, &debug, E_NONE);
> - debug += "<br>";
> - break;
This part looks OK - did not test it throughly though.

> - case P_RANGE:
> - debug += "range: ";
> - expr_print(prop->expr, expr_print_help, &debug, E_NONE);
> - debug += "<br>";
> - break;
But this parts looks wrong. I have added back the case P_RANGE.
Testing revealed that this was needed/OK.

> default:
> debug += "unknown property: ";
> debug += prop_get_type_name(prop->type);
> Index: linux-2.6/scripts/kconfig/symbol.c
> ===================================================================
> --- linux-2.6.orig/scripts/kconfig/symbol.c
> +++ linux-2.6/scripts/kconfig/symbol.c
> @@ -34,6 +34,8 @@ struct symbol *sym_defconfig_list;
> struct symbol *modules_sym;
> tristate modules_val;
>
> +struct expr *sym_env_list;
> +
> void sym_add_default(struct symbol *sym, const char *def)
> {
> struct property *prop = prop_alloc(P_DEFAULT, sym);
> @@ -54,13 +56,6 @@ void sym_init(void)
>
> uname(&uts);
>
> - sym = sym_lookup("ARCH", 0);
> - sym->type = S_STRING;
> - sym->flags |= SYMBOL_AUTO;
> - p = getenv("ARCH");
> - if (p)
> - sym_add_default(sym, p);
> -
> sym = sym_lookup("KERNELVERSION", 0);
> sym->type = S_STRING;
> sym->flags |= SYMBOL_AUTO;
> @@ -117,6 +112,15 @@ struct property *sym_get_choice_prop(str
> return NULL;
> }
>
> +struct property *sym_get_env_prop(struct symbol *sym)
> +{
> + struct property *prop;
> +
> + for_all_properties(sym, prop, P_ENV)
> + return prop;
> + return NULL;
> +}
> +
> struct property *sym_get_default_prop(struct symbol *sym)
> {
> struct property *prop;
> @@ -347,6 +351,9 @@ void sym_calc_value(struct symbol *sym)
> ;
> }
>
> + if (sym->flags & SYMBOL_AUTO)
> + sym->flags &= ~SYMBOL_WRITE;
> +

Why is this change needed?
It is non-obvious to me so please explain and I will add a comment.

> sym->curr = newval;
> if (sym_is_choice(sym) && newval.tri == yes)
> sym->curr.val = sym_calc_choice(sym);
> @@ -849,7 +856,7 @@ struct property *prop_alloc(enum prop_ty
> struct symbol *prop_get_symbol(struct property *prop)
> {
> if (prop->expr && (prop->expr->type == E_SYMBOL ||
> - prop->expr->type == E_CHOICE))
> + prop->expr->type == E_LIST))
> return prop->expr->left.sym;
> return NULL;
> }
> @@ -859,6 +866,8 @@ const char *prop_get_type_name(enum prop
> switch (type) {
> case P_PROMPT:
> return "prompt";
> + case P_ENV:
> + return "env";
> case P_COMMENT:
> return "comment";
> case P_MENU:
> @@ -876,3 +885,31 @@ const char *prop_get_type_name(enum prop
> }
> return "unknown";
> }
> +
> +void prop_add_env(const char *env)
> +{
> + struct symbol *sym, *sym2;
> + struct property *prop;
> + char *p;
> +
> + sym = current_entry->sym;
> + sym->flags |= SYMBOL_AUTO;
> + for_all_properties(sym, prop, P_ENV) {
> + sym2 = prop_get_symbol(prop);
> + if (strcmp(sym2->name, env))
> + menu_warn(current_entry, "Redefining environment symbol");
I did it like this:
menu_warn(current_entry,
"config %s: redefining environment symbol from '%s' to '%s'",
sym->name, env, sym2->name);

> + return;
> + }
> +
> + prop = prop_alloc(P_ENV, sym);
> + prop->expr = expr_alloc_symbol(sym_lookup(env, 1));
> +
> + sym_env_list = expr_alloc_one(E_LIST, sym_env_list);
> + sym_env_list->right.sym = sym;
> +
> + p = getenv(env);
> + if (p)
> + sym_add_default(sym, p);
> + else
> + menu_warn(current_entry, "environment variable %s undefined", sym->name);
And like this:
menu_warn(current_entry,
"config %s: environment variable '%s' undefined",
sym->name, env);

> +}
> Index: linux-2.6/scripts/kconfig/util.c
> ===================================================================
> --- linux-2.6.orig/scripts/kconfig/util.c
> +++ linux-2.6/scripts/kconfig/util.c
> @@ -29,6 +29,7 @@ struct file *file_lookup(const char *nam
> /* write a dependency file as used by kbuild to track dependencies */
> int file_write_dep(const char *name)
> {
> + struct expr *e;
> struct file *file;
> FILE *out;
>
> @@ -45,8 +46,28 @@ int file_write_dep(const char *name)
> fprintf(out, "\t%s\n", file->name);
> }
> fprintf(out, "\ninclude/config/auto.conf: \\\n"
> - "\t$(deps_config)\n\n"
> - "$(deps_config): ;\n");
> + "\t$(deps_config)\n\n");
> +
> + for (e = sym_env_list; e; e = e->left.expr) {
> + struct property *p;
> + struct symbol *sym, *env_sym;
> + const char *value;
> +
> + sym = e->right.sym;
> + p = sym_get_env_prop(sym);
> + env_sym = prop_get_symbol(p);
> + if (!env_sym)
> + continue;
> + value = sym_get_string_value(sym);
> + if ((sym->type == S_BOOLEAN || sym->type == S_TRISTATE) &&
> + sym_get_tristate_value(sym) == no)
> + value = "";
> + fprintf(out, "ifneq \"$(%s)\" \"%s\"\n", env_sym->name, value);
> + fprintf(out, "include/config/auto.conf: FORCE\n");
> + fprintf(out, "endif\n");
> + }
> +
> + fprintf(out, "\n$(deps_config): ;\n");
> fclose(out);
> rename("..config.tmp", name);
> return 0;
> Index: linux-2.6/scripts/kconfig/zconf.gperf
> ===================================================================
> --- linux-2.6.orig/scripts/kconfig/zconf.gperf
> +++ linux-2.6/scripts/kconfig/zconf.gperf
> @@ -41,4 +41,5 @@ option, T_OPTION, TF_COMMAND
> on, T_ON, TF_PARAM
> modules, T_OPT_MODULES, TF_OPTION
> defconfig_list, T_OPT_DEFCONFIG_LIST,TF_OPTION
> +env, T_OPT_ENV, TF_OPTION
> %%

2008-01-14 03:50:07

by Roman Zippel

[permalink] [raw]
Subject: Re: kconfig: support option env="" [Was: kconfig: use $K64BIT to set 64BIT with all*config targets]

Hi,

On Sun, 6 Jan 2008, Sam Ravnborg wrote:

> Please get back to me so we can finsih this patch and have it applied.
> I will split the patch in two btw.

I reworked the patch a little and split it into three.

> > + if (sym->flags & SYMBOL_AUTO)
> > + sym->flags &= ~SYMBOL_WRITE;
> > +
>
> Why is this change needed?
> It is non-obvious to me so please explain and I will add a comment.

Automatically generated symbols are not saved, this was previously not
needed as they weren't in the menu structure.

> I did it like this:
> menu_warn(current_entry,
> "config %s: redefining environment symbol from '%s' to '%s'",
> sym->name, env, sym2->name);

I omitted the prefix, it's inconsistent with other warnings.

bye, Roman

2008-01-14 03:50:37

by Roman Zippel

[permalink] [raw]
Subject: [PATCH 1/3] explicitly introduce expression list


Rename E_CHOICE to E_LIST to explicitly add support for expression
lists. Add a helper macro expr_list_for_each_sym to more easily iterate
over the list.

Signed-off-by: Roman Zippel <[email protected]>

---
scripts/kconfig/confdata.c | 8 ++++----
scripts/kconfig/expr.c | 16 ++++++++--------
scripts/kconfig/expr.h | 5 ++++-
scripts/kconfig/menu.c | 2 +-
scripts/kconfig/symbol.c | 13 +++++++------
5 files changed, 24 insertions(+), 20 deletions(-)

Index: linux-2.6/scripts/kconfig/confdata.c
===================================================================
--- linux-2.6.orig/scripts/kconfig/confdata.c
+++ linux-2.6/scripts/kconfig/confdata.c
@@ -316,7 +316,7 @@ load:

int conf_read(const char *name)
{
- struct symbol *sym;
+ struct symbol *sym, *choice_sym;
struct property *prop;
struct expr *e;
int i, flags;
@@ -357,9 +357,9 @@ int conf_read(const char *name)
*/
prop = sym_get_choice_prop(sym);
flags = sym->flags;
- for (e = prop->expr; e; e = e->left.expr)
- if (e->right.sym->visible != no)
- flags &= e->right.sym->flags;
+ expr_list_for_each_sym(prop->expr, e, choice_sym)
+ if (choice_sym->visible != no)
+ flags &= choice_sym->flags;
sym->flags &= flags | ~SYMBOL_DEF_USER;
}

Index: linux-2.6/scripts/kconfig/expr.c
===================================================================
--- linux-2.6.orig/scripts/kconfig/expr.c
+++ linux-2.6/scripts/kconfig/expr.c
@@ -87,7 +87,7 @@ struct expr *expr_copy(struct expr *org)
break;
case E_AND:
case E_OR:
- case E_CHOICE:
+ case E_LIST:
e->left.expr = expr_copy(org->left.expr);
e->right.expr = expr_copy(org->right.expr);
break;
@@ -217,7 +217,7 @@ int expr_eq(struct expr *e1, struct expr
expr_free(e2);
trans_count = old_count;
return res;
- case E_CHOICE:
+ case E_LIST:
case E_RANGE:
case E_NONE:
/* panic */;
@@ -648,7 +648,7 @@ struct expr *expr_transform(struct expr
case E_EQUAL:
case E_UNEQUAL:
case E_SYMBOL:
- case E_CHOICE:
+ case E_LIST:
break;
default:
e->left.expr = expr_transform(e->left.expr);
@@ -932,7 +932,7 @@ struct expr *expr_trans_compare(struct e
break;
case E_SYMBOL:
return expr_alloc_comp(type, e->left.sym, sym);
- case E_CHOICE:
+ case E_LIST:
case E_RANGE:
case E_NONE:
/* panic */;
@@ -1000,9 +1000,9 @@ int expr_compare_type(enum expr_type t1,
if (t2 == E_OR)
return 1;
case E_OR:
- if (t2 == E_CHOICE)
+ if (t2 == E_LIST)
return 1;
- case E_CHOICE:
+ case E_LIST:
if (t2 == 0)
return 1;
default:
@@ -1053,11 +1053,11 @@ void expr_print(struct expr *e, void (*f
fn(data, NULL, " && ");
expr_print(e->right.expr, fn, data, E_AND);
break;
- case E_CHOICE:
+ case E_LIST:
fn(data, e->right.sym, e->right.sym->name);
if (e->left.expr) {
fn(data, NULL, " ^ ");
- expr_print(e->left.expr, fn, data, E_CHOICE);
+ expr_print(e->left.expr, fn, data, E_LIST);
}
break;
case E_RANGE:
Index: linux-2.6/scripts/kconfig/expr.h
===================================================================
--- linux-2.6.orig/scripts/kconfig/expr.h
+++ linux-2.6/scripts/kconfig/expr.h
@@ -32,7 +32,7 @@ typedef enum tristate {
} tristate;

enum expr_type {
- E_NONE, E_OR, E_AND, E_NOT, E_EQUAL, E_UNEQUAL, E_CHOICE, E_SYMBOL, E_RANGE
+ E_NONE, E_OR, E_AND, E_NOT, E_EQUAL, E_UNEQUAL, E_LIST, E_SYMBOL, E_RANGE
};

union expr_data {
@@ -49,6 +49,9 @@ struct expr {
#define E_AND(dep1, dep2) (((dep1)<(dep2))?(dep1):(dep2))
#define E_NOT(dep) (2-(dep))

+#define expr_list_for_each_sym(l, e, s) \
+ for (e = (l); e && (s = e->right.sym); e = e->left.expr)
+
struct expr_value {
struct expr *expr;
tristate tri;
Index: linux-2.6/scripts/kconfig/menu.c
===================================================================
--- linux-2.6.orig/scripts/kconfig/menu.c
+++ linux-2.6/scripts/kconfig/menu.c
@@ -331,7 +331,7 @@ void menu_finalize(struct menu *parent)
prop = sym_get_choice_prop(sym);
for (ep = &prop->expr; *ep; ep = &(*ep)->left.expr)
;
- *ep = expr_alloc_one(E_CHOICE, NULL);
+ *ep = expr_alloc_one(E_LIST, NULL);
(*ep)->right.sym = menu->sym;
}
if (menu->list && (!menu->prompt || !menu->prompt->text)) {
Index: linux-2.6/scripts/kconfig/symbol.c
===================================================================
--- linux-2.6.orig/scripts/kconfig/symbol.c
+++ linux-2.6/scripts/kconfig/symbol.c
@@ -247,8 +247,7 @@ static struct symbol *sym_calc_choice(st

/* just get the first visible value */
prop = sym_get_choice_prop(sym);
- for (e = prop->expr; e; e = e->left.expr) {
- def_sym = e->right.sym;
+ expr_list_for_each_sym(prop->expr, e, def_sym) {
sym_calc_visibility(def_sym);
if (def_sym->visible != no)
return def_sym;
@@ -361,12 +360,14 @@ void sym_calc_value(struct symbol *sym)
}

if (sym_is_choice(sym)) {
+ struct symbol *choice_sym;
int flags = sym->flags & (SYMBOL_CHANGED | SYMBOL_WRITE);
+
prop = sym_get_choice_prop(sym);
- for (e = prop->expr; e; e = e->left.expr) {
- e->right.sym->flags |= flags;
+ expr_list_for_each_sym(prop->expr, e, choice_sym) {
+ choice_sym->flags |= flags;
if (flags & SYMBOL_CHANGED)
- sym_set_changed(e->right.sym);
+ sym_set_changed(choice_sym);
}
}
}
@@ -849,7 +850,7 @@ struct property *prop_alloc(enum prop_ty
struct symbol *prop_get_symbol(struct property *prop)
{
if (prop->expr && (prop->expr->type == E_SYMBOL ||
- prop->expr->type == E_CHOICE))
+ prop->expr->type == E_LIST))
return prop->expr->left.sym;
return NULL;
}

2008-01-14 03:51:28

by Roman Zippel

[permalink] [raw]
Subject: [PATCH 2/3] environment symbol support


Add the possibility to import a value from the environment into kconfig
via the option syntax. Beside flexibility this has the advantage
providing proper dependencies.

Signed-off-by: Roman Zippel <[email protected]>

---
Documentation/kbuild/kconfig-language.txt | 21 ++++++++++++++
scripts/kconfig/expr.h | 3 +-
scripts/kconfig/lkc.h | 5 +++
scripts/kconfig/menu.c | 5 ++-
scripts/kconfig/qconf.cc | 16 +++-------
scripts/kconfig/symbol.c | 45 ++++++++++++++++++++++++++++++
scripts/kconfig/util.c | 23 ++++++++++++++-
scripts/kconfig/zconf.gperf | 1
scripts/kconfig/zconf.hash.c_shipped | 45 ++++++++++++++++--------------
9 files changed, 129 insertions(+), 35 deletions(-)

Index: linux-2.6/Documentation/kbuild/kconfig-language.txt
===================================================================
--- linux-2.6.orig/Documentation/kbuild/kconfig-language.txt
+++ linux-2.6/Documentation/kbuild/kconfig-language.txt
@@ -127,6 +127,27 @@ applicable everywhere (see syntax).
used to help visually separate configuration logic from help within
the file as an aid to developers.

+- misc options: "option" <symbol>[=<value>]
+ Various less common options can be defined via this option syntax,
+ which can modify the behaviour of the menu entry and its config
+ symbol. These options are currently possible:
+
+ - "defconfig_list"
+ This declares a list of default entries which can be used when
+ looking for the default configuration (which is used when the main
+ .config doesn't exists yet.)
+
+ - "modules"
+ This declares the symbol to be used as the MODULES symbol, which
+ enables the third modular state for all config symbols.
+
+ - "env"=<value>
+ This imports the environment variable into Kconfig. It behaves like
+ a default, except that the value comes from the environment, this
+ also means that the behaviour when mixing it with normal defaults is
+ undefined at this point. The symbol is currently not exported back
+ to the build environment (if this is desired, it can be done via
+ another symbol).

Menu dependencies
-----------------
Index: linux-2.6/scripts/kconfig/expr.h
===================================================================
--- linux-2.6.orig/scripts/kconfig/expr.h
+++ linux-2.6/scripts/kconfig/expr.h
@@ -108,7 +108,8 @@ struct symbol {
#define SYMBOL_HASHMASK 0xff

enum prop_type {
- P_UNKNOWN, P_PROMPT, P_COMMENT, P_MENU, P_DEFAULT, P_CHOICE, P_SELECT, P_RANGE
+ P_UNKNOWN, P_PROMPT, P_COMMENT, P_MENU, P_DEFAULT, P_CHOICE,
+ P_SELECT, P_RANGE, P_ENV
};

struct property {
Index: linux-2.6/scripts/kconfig/lkc.h
===================================================================
--- linux-2.6.orig/scripts/kconfig/lkc.h
+++ linux-2.6/scripts/kconfig/lkc.h
@@ -44,6 +44,7 @@ extern "C" {

#define T_OPT_MODULES 1
#define T_OPT_DEFCONFIG_LIST 2
+#define T_OPT_ENV 3

struct kconf_id {
int name;
@@ -74,6 +75,7 @@ void kconfig_load(void);

/* menu.c */
void menu_init(void);
+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);
@@ -103,6 +105,8 @@ void str_printf(struct gstr *gs, const c
const char *str_get(struct gstr *gs);

/* symbol.c */
+extern struct expr *sym_env_list;
+
void sym_init(void);
void sym_clear_all_valid(void);
void sym_set_all_changed(void);
@@ -110,6 +114,7 @@ void sym_set_changed(struct symbol *sym)
struct symbol *sym_check_deps(struct symbol *sym);
struct property *prop_alloc(enum prop_type type, struct symbol *sym);
struct symbol *prop_get_symbol(struct property *prop);
+struct property *sym_get_env_prop(struct symbol *sym);

static inline tristate sym_get_tristate_value(struct symbol *sym)
{
Index: linux-2.6/scripts/kconfig/menu.c
===================================================================
--- linux-2.6.orig/scripts/kconfig/menu.c
+++ linux-2.6/scripts/kconfig/menu.c
@@ -15,7 +15,7 @@ static struct menu **last_entry_ptr;
struct file *file_list;
struct file *current_file;

-static void menu_warn(struct menu *menu, const char *fmt, ...)
+void menu_warn(struct menu *menu, const char *fmt, ...)
{
va_list ap;
va_start(ap, fmt);
@@ -172,6 +172,9 @@ void menu_add_option(int token, char *ar
else if (sym_defconfig_list != current_entry->sym)
zconf_error("trying to redefine defconfig symbol");
break;
+ case T_OPT_ENV:
+ prop_add_env(arg);
+ break;
}
}

Index: linux-2.6/scripts/kconfig/qconf.cc
===================================================================
--- linux-2.6.orig/scripts/kconfig/qconf.cc
+++ linux-2.6/scripts/kconfig/qconf.cc
@@ -1083,7 +1083,11 @@ QString ConfigInfoView::debug_info(struc
debug += "</a><br>";
break;
case P_DEFAULT:
- debug += "default: ";
+ case P_SELECT:
+ case P_RANGE:
+ case P_ENV:
+ debug += prop_get_type_name(prop->type);
+ debug += ": ";
expr_print(prop->expr, expr_print_help, &debug, E_NONE);
debug += "<br>";
break;
@@ -1094,16 +1098,6 @@ QString ConfigInfoView::debug_info(struc
debug += "<br>";
}
break;
- case P_SELECT:
- debug += "select: ";
- expr_print(prop->expr, expr_print_help, &debug, E_NONE);
- debug += "<br>";
- break;
- case P_RANGE:
- debug += "range: ";
- expr_print(prop->expr, expr_print_help, &debug, E_NONE);
- debug += "<br>";
- break;
default:
debug += "unknown property: ";
debug += prop_get_type_name(prop->type);
Index: linux-2.6/scripts/kconfig/symbol.c
===================================================================
--- linux-2.6.orig/scripts/kconfig/symbol.c
+++ linux-2.6/scripts/kconfig/symbol.c
@@ -34,6 +34,8 @@ struct symbol *sym_defconfig_list;
struct symbol *modules_sym;
tristate modules_val;

+struct expr *sym_env_list;
+
void sym_add_default(struct symbol *sym, const char *def)
{
struct property *prop = prop_alloc(P_DEFAULT, sym);
@@ -117,6 +119,15 @@ struct property *sym_get_choice_prop(str
return NULL;
}

+struct property *sym_get_env_prop(struct symbol *sym)
+{
+ struct property *prop;
+
+ for_all_properties(sym, prop, P_ENV)
+ return prop;
+ return NULL;
+}
+
struct property *sym_get_default_prop(struct symbol *sym)
{
struct property *prop;
@@ -346,6 +357,9 @@ void sym_calc_value(struct symbol *sym)
;
}

+ if (sym->flags & SYMBOL_AUTO)
+ sym->flags &= ~SYMBOL_WRITE;
+
sym->curr = newval;
if (sym_is_choice(sym) && newval.tri == yes)
sym->curr.val = sym_calc_choice(sym);
@@ -860,6 +874,8 @@ const char *prop_get_type_name(enum prop
switch (type) {
case P_PROMPT:
return "prompt";
+ case P_ENV:
+ return "env";
case P_COMMENT:
return "comment";
case P_MENU:
@@ -877,3 +893,32 @@ const char *prop_get_type_name(enum prop
}
return "unknown";
}
+
+void prop_add_env(const char *env)
+{
+ struct symbol *sym, *sym2;
+ struct property *prop;
+ char *p;
+
+ sym = current_entry->sym;
+ sym->flags |= SYMBOL_AUTO;
+ for_all_properties(sym, prop, P_ENV) {
+ sym2 = prop_get_symbol(prop);
+ if (strcmp(sym2->name, env))
+ menu_warn(current_entry, "redefining environment symbol from %s",
+ sym2->name);
+ return;
+ }
+
+ prop = prop_alloc(P_ENV, sym);
+ prop->expr = expr_alloc_symbol(sym_lookup(env, 1));
+
+ sym_env_list = expr_alloc_one(E_LIST, sym_env_list);
+ sym_env_list->right.sym = sym;
+
+ p = getenv(env);
+ if (p)
+ sym_add_default(sym, p);
+ else
+ menu_warn(current_entry, "environment variable %s undefined", env);
+}
Index: linux-2.6/scripts/kconfig/util.c
===================================================================
--- linux-2.6.orig/scripts/kconfig/util.c
+++ linux-2.6/scripts/kconfig/util.c
@@ -29,6 +29,8 @@ struct file *file_lookup(const char *nam
/* write a dependency file as used by kbuild to track dependencies */
int file_write_dep(const char *name)
{
+ struct symbol *sym, *env_sym;
+ struct expr *e;
struct file *file;
FILE *out;

@@ -45,8 +47,25 @@ int file_write_dep(const char *name)
fprintf(out, "\t%s\n", file->name);
}
fprintf(out, "\ninclude/config/auto.conf: \\\n"
- "\t$(deps_config)\n\n"
- "$(deps_config): ;\n");
+ "\t$(deps_config)\n\n");
+
+ expr_list_for_each_sym(sym_env_list, e, sym) {
+ struct property *prop;
+ const char *value;
+
+ prop = sym_get_env_prop(sym);
+ env_sym = prop_get_symbol(prop);
+ if (!env_sym)
+ continue;
+ value = getenv(env_sym->name);
+ if (!value)
+ value = "";
+ fprintf(out, "ifneq \"$(%s)\" \"%s\"\n", env_sym->name, value);
+ fprintf(out, "include/config/auto.conf: FORCE\n");
+ fprintf(out, "endif\n");
+ }
+
+ fprintf(out, "\n$(deps_config): ;\n");
fclose(out);
rename("..config.tmp", name);
return 0;
Index: linux-2.6/scripts/kconfig/zconf.gperf
===================================================================
--- linux-2.6.orig/scripts/kconfig/zconf.gperf
+++ linux-2.6/scripts/kconfig/zconf.gperf
@@ -41,4 +41,5 @@ option, T_OPTION, TF_COMMAND
on, T_ON, TF_PARAM
modules, T_OPT_MODULES, TF_OPTION
defconfig_list, T_OPT_DEFCONFIG_LIST,TF_OPTION
+env, T_OPT_ENV, TF_OPTION
%%
Index: linux-2.6/scripts/kconfig/zconf.hash.c_shipped
===================================================================
--- linux-2.6.orig/scripts/kconfig/zconf.hash.c_shipped
+++ linux-2.6/scripts/kconfig/zconf.hash.c_shipped
@@ -53,10 +53,10 @@ kconf_id_hash (register const char *str,
49, 49, 49, 49, 49, 49, 49, 49, 49, 49,
49, 49, 49, 49, 49, 49, 49, 49, 49, 49,
49, 49, 49, 49, 49, 49, 49, 49, 49, 49,
- 49, 49, 49, 49, 49, 49, 49, 18, 11, 5,
+ 49, 49, 49, 49, 49, 49, 49, 35, 35, 5,
0, 0, 5, 49, 5, 20, 49, 49, 5, 20,
- 5, 0, 30, 49, 0, 15, 0, 10, 49, 49,
- 25, 49, 49, 49, 49, 49, 49, 49, 49, 49,
+ 5, 0, 30, 49, 0, 15, 0, 10, 0, 49,
+ 10, 49, 49, 49, 49, 49, 49, 49, 49, 49,
49, 49, 49, 49, 49, 49, 49, 49, 49, 49,
49, 49, 49, 49, 49, 49, 49, 49, 49, 49,
49, 49, 49, 49, 49, 49, 49, 49, 49, 49,
@@ -89,6 +89,7 @@ kconf_id_hash (register const char *str,
struct kconf_id_strings_t
{
char kconf_id_strings_str2[sizeof("on")];
+ char kconf_id_strings_str3[sizeof("env")];
char kconf_id_strings_str5[sizeof("endif")];
char kconf_id_strings_str6[sizeof("option")];
char kconf_id_strings_str7[sizeof("endmenu")];
@@ -99,30 +100,31 @@ struct kconf_id_strings_t
char kconf_id_strings_str12[sizeof("default")];
char kconf_id_strings_str13[sizeof("def_bool")];
char kconf_id_strings_str14[sizeof("help")];
- char kconf_id_strings_str15[sizeof("bool")];
char kconf_id_strings_str16[sizeof("config")];
char kconf_id_strings_str17[sizeof("def_tristate")];
- char kconf_id_strings_str18[sizeof("boolean")];
+ char kconf_id_strings_str18[sizeof("hex")];
char kconf_id_strings_str19[sizeof("defconfig_list")];
char kconf_id_strings_str21[sizeof("string")];
char kconf_id_strings_str22[sizeof("if")];
char kconf_id_strings_str23[sizeof("int")];
- char kconf_id_strings_str24[sizeof("enable")];
char kconf_id_strings_str26[sizeof("select")];
char kconf_id_strings_str27[sizeof("modules")];
char kconf_id_strings_str28[sizeof("tristate")];
char kconf_id_strings_str29[sizeof("menu")];
char kconf_id_strings_str31[sizeof("source")];
char kconf_id_strings_str32[sizeof("comment")];
- char kconf_id_strings_str33[sizeof("hex")];
char kconf_id_strings_str35[sizeof("menuconfig")];
char kconf_id_strings_str36[sizeof("prompt")];
char kconf_id_strings_str37[sizeof("depends")];
+ char kconf_id_strings_str39[sizeof("bool")];
+ char kconf_id_strings_str41[sizeof("enable")];
+ char kconf_id_strings_str42[sizeof("boolean")];
char kconf_id_strings_str48[sizeof("mainmenu")];
};
static struct kconf_id_strings_t kconf_id_strings_contents =
{
"on",
+ "env",
"endif",
"option",
"endmenu",
@@ -133,25 +135,25 @@ static struct kconf_id_strings_t kconf_i
"default",
"def_bool",
"help",
- "bool",
"config",
"def_tristate",
- "boolean",
+ "hex",
"defconfig_list",
"string",
"if",
"int",
- "enable",
"select",
"modules",
"tristate",
"menu",
"source",
"comment",
- "hex",
"menuconfig",
"prompt",
"depends",
+ "bool",
+ "enable",
+ "boolean",
"mainmenu"
};
#define kconf_id_strings ((const char *) &kconf_id_strings_contents)
@@ -163,7 +165,7 @@ kconf_id_lookup (register const char *st
{
enum
{
- TOTAL_KEYWORDS = 31,
+ TOTAL_KEYWORDS = 32,
MIN_WORD_LENGTH = 2,
MAX_WORD_LENGTH = 14,
MIN_HASH_VALUE = 2,
@@ -174,7 +176,8 @@ kconf_id_lookup (register const char *st
{
{-1}, {-1},
{(int)(long)&((struct kconf_id_strings_t *)0)->kconf_id_strings_str2, T_ON, TF_PARAM},
- {-1}, {-1},
+ {(int)(long)&((struct kconf_id_strings_t *)0)->kconf_id_strings_str3, T_OPT_ENV, TF_OPTION},
+ {-1},
{(int)(long)&((struct kconf_id_strings_t *)0)->kconf_id_strings_str5, T_ENDIF, TF_COMMAND},
{(int)(long)&((struct kconf_id_strings_t *)0)->kconf_id_strings_str6, T_OPTION, TF_COMMAND},
{(int)(long)&((struct kconf_id_strings_t *)0)->kconf_id_strings_str7, T_ENDMENU, TF_COMMAND},
@@ -185,17 +188,16 @@ kconf_id_lookup (register const char *st
{(int)(long)&((struct kconf_id_strings_t *)0)->kconf_id_strings_str12, T_DEFAULT, TF_COMMAND, S_UNKNOWN},
{(int)(long)&((struct kconf_id_strings_t *)0)->kconf_id_strings_str13, T_DEFAULT, TF_COMMAND, S_BOOLEAN},
{(int)(long)&((struct kconf_id_strings_t *)0)->kconf_id_strings_str14, T_HELP, TF_COMMAND},
- {(int)(long)&((struct kconf_id_strings_t *)0)->kconf_id_strings_str15, T_TYPE, TF_COMMAND, S_BOOLEAN},
+ {-1},
{(int)(long)&((struct kconf_id_strings_t *)0)->kconf_id_strings_str16, T_CONFIG, TF_COMMAND},
{(int)(long)&((struct kconf_id_strings_t *)0)->kconf_id_strings_str17, T_DEFAULT, TF_COMMAND, S_TRISTATE},
- {(int)(long)&((struct kconf_id_strings_t *)0)->kconf_id_strings_str18, T_TYPE, TF_COMMAND, S_BOOLEAN},
+ {(int)(long)&((struct kconf_id_strings_t *)0)->kconf_id_strings_str18, T_TYPE, TF_COMMAND, S_HEX},
{(int)(long)&((struct kconf_id_strings_t *)0)->kconf_id_strings_str19, T_OPT_DEFCONFIG_LIST,TF_OPTION},
{-1},
{(int)(long)&((struct kconf_id_strings_t *)0)->kconf_id_strings_str21, T_TYPE, TF_COMMAND, S_STRING},
{(int)(long)&((struct kconf_id_strings_t *)0)->kconf_id_strings_str22, T_IF, TF_COMMAND|TF_PARAM},
{(int)(long)&((struct kconf_id_strings_t *)0)->kconf_id_strings_str23, T_TYPE, TF_COMMAND, S_INT},
- {(int)(long)&((struct kconf_id_strings_t *)0)->kconf_id_strings_str24, T_SELECT, TF_COMMAND},
- {-1},
+ {-1}, {-1},
{(int)(long)&((struct kconf_id_strings_t *)0)->kconf_id_strings_str26, T_SELECT, TF_COMMAND},
{(int)(long)&((struct kconf_id_strings_t *)0)->kconf_id_strings_str27, T_OPT_MODULES, TF_OPTION},
{(int)(long)&((struct kconf_id_strings_t *)0)->kconf_id_strings_str28, T_TYPE, TF_COMMAND, S_TRISTATE},
@@ -203,13 +205,16 @@ kconf_id_lookup (register const char *st
{-1},
{(int)(long)&((struct kconf_id_strings_t *)0)->kconf_id_strings_str31, T_SOURCE, TF_COMMAND},
{(int)(long)&((struct kconf_id_strings_t *)0)->kconf_id_strings_str32, T_COMMENT, TF_COMMAND},
- {(int)(long)&((struct kconf_id_strings_t *)0)->kconf_id_strings_str33, T_TYPE, TF_COMMAND, S_HEX},
- {-1},
+ {-1}, {-1},
{(int)(long)&((struct kconf_id_strings_t *)0)->kconf_id_strings_str35, T_MENUCONFIG, TF_COMMAND},
{(int)(long)&((struct kconf_id_strings_t *)0)->kconf_id_strings_str36, T_PROMPT, TF_COMMAND},
{(int)(long)&((struct kconf_id_strings_t *)0)->kconf_id_strings_str37, T_DEPENDS, TF_COMMAND},
- {-1}, {-1}, {-1}, {-1}, {-1}, {-1}, {-1}, {-1}, {-1},
{-1},
+ {(int)(long)&((struct kconf_id_strings_t *)0)->kconf_id_strings_str39, T_TYPE, TF_COMMAND, S_BOOLEAN},
+ {-1},
+ {(int)(long)&((struct kconf_id_strings_t *)0)->kconf_id_strings_str41, T_SELECT, TF_COMMAND},
+ {(int)(long)&((struct kconf_id_strings_t *)0)->kconf_id_strings_str42, T_TYPE, TF_COMMAND, S_BOOLEAN},
+ {-1}, {-1}, {-1}, {-1}, {-1},
{(int)(long)&((struct kconf_id_strings_t *)0)->kconf_id_strings_str48, T_MAINMENU, TF_COMMAND}
};

2008-01-14 03:51:42

by Roman Zippel

[permalink] [raw]
Subject: [PATCH 3/3] use environment option


Use the environment option to provide the ARCH symbol.
Remove the unused KERNELVERSION symbol.

Signed-off-by: Roman Zippel <[email protected]>

---
init/Kconfig | 4 ++++
scripts/kconfig/symbol.c | 14 --------------
2 files changed, 4 insertions(+), 14 deletions(-)

Index: linux-2.6/init/Kconfig
===================================================================
--- linux-2.6.orig/init/Kconfig
+++ linux-2.6/init/Kconfig
@@ -1,3 +1,7 @@
+config ARCH
+ string
+ option env="ARCH"
+
config DEFCONFIG_LIST
string
depends on !UML
Index: linux-2.6/scripts/kconfig/symbol.c
===================================================================
--- linux-2.6.orig/scripts/kconfig/symbol.c
+++ linux-2.6/scripts/kconfig/symbol.c
@@ -56,20 +56,6 @@ void sym_init(void)

uname(&uts);

- sym = sym_lookup("ARCH", 0);
- sym->type = S_STRING;
- sym->flags |= SYMBOL_AUTO;
- p = getenv("ARCH");
- if (p)
- sym_add_default(sym, p);
-
- sym = sym_lookup("KERNELVERSION", 0);
- sym->type = S_STRING;
- sym->flags |= SYMBOL_AUTO;
- p = getenv("KERNELVERSION");
- if (p)
- sym_add_default(sym, p);
-
sym = sym_lookup("UNAME_RELEASE", 0);
sym->type = S_STRING;
sym->flags |= SYMBOL_AUTO;

2008-01-14 05:58:54

by Sam Ravnborg

[permalink] [raw]
Subject: Re: kconfig: support option env="" [Was: kconfig: use $K64BIT to set 64BIT with all*config targets]

On Mon, Jan 14, 2008 at 04:49:48AM +0100, Roman Zippel wrote:
> Hi,
>
> On Sun, 6 Jan 2008, Sam Ravnborg wrote:
>
> > Please get back to me so we can finsih this patch and have it applied.
> > I will split the patch in two btw.
>
> I reworked the patch a little and split it into three.

Thanks Roman.
I will test and apply tonight.

Removal of KERNELVERSION in patch #3 is wrong as the frontend
uses KERNELVERSION to display the kernel version in their title.
I will drop the deletion before I apply the patch.


>
> > > + if (sym->flags & SYMBOL_AUTO)
> > > + sym->flags &= ~SYMBOL_WRITE;
> > > +
> >
> > Why is this change needed?
> > It is non-obvious to me so please explain and I will add a comment.
>
> Automatically generated symbols are not saved, this was previously not
> needed as they weren't in the menu structure.
OK

>
> > I did it like this:
> > menu_warn(current_entry,
> > "config %s: redefining environment symbol from '%s' to '%s'",
> > sym->name, env, sym2->name);
>
> I omitted the prefix, it's inconsistent with other warnings.
OK

Thanks,
Sam