Skip to contentSkip to conversation

introduce K64BIT=y and backward compatibility ARCH={i386,x86_64} for x86

PATCH In review Sam Ravnborg 5 participants 42 messages Nov 10, 2007 – Jan 14, 2008

Subsystems (4)
lore
Share & tools
lore
#0 lore raw

cover 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

#1 lore raw

PATCH 1/1 kconfig: factor out code in confdata.c

This patch introduce no functional changes.

---
 scripts/kconfig/confdata.c |  119 +++++++++++++++++++++++--------------------
 1 files changed, 64 insertions(+), 55 deletions(-)
scripts/kconfig/confdata.c +64 -55
index b2913e9..e0f402f 100644
@@ -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':
Signature · 1 line
1.5.3.4.1157.g0e74-dirty
#2 lore raw

PATCH 1/1 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.

---
 scripts/kconfig/conf.c      |    1 +
 scripts/kconfig/confdata.c  |   27 +++++++++++++++++++++++++++
 scripts/kconfig/lkc_proto.h |    1 +
 3 files changed, 29 insertions(+), 0 deletions(-)
scripts/kconfig/conf.c +1 -0
index a38787a..c6bee85 100644
@@ -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;
scripts/kconfig/confdata.c +27 -0
index e0f402f..0cb7555 100644
@@ -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;
scripts/kconfig/lkc_proto.h +1 -0
index 4d09f6d..dca294e 100644
@@ -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));
Signature · 1 line
1.5.3.4.1157.g0e74-dirty
#3 lore raw

PATCH 1/1 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.

---
 arch/x86/Kconfig |   26 ++++++++------------------
 1 files changed, 8 insertions(+), 18 deletions(-)
arch/x86/Kconfig +8 -18
index 153c26c..0d86611 100644
@@ -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
Signature · 1 line
1.5.3.4.1157.g0e74-dirty
#4 lore raw

PATCH 1/1 kconfig: document make K64BIT=y in README

---
 README |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)
README +2 -0
index 159912c..6622ba1 100644
@@ -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
Signature · 1 line
1.5.3.4.1157.g0e74-dirty
#5 lore raw

PATCH 1/1 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.

---
 Makefile                 |   16 +++++++++++++---
 arch/x86/Makefile        |   10 +++++++---
 scripts/kconfig/Makefile |    2 +-
 3 files changed, 21 insertions(+), 7 deletions(-)
Makefile +13 -3
index 902082b..5efee80 100644
@@ -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
 
arch/x86/Makefile +7 -3
index ee94224..feba761 100644
@@ -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)
scripts/kconfig/Makefile +1 -1
index 3c9db07..1ad6f7f 100644
@@ -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)
Signature · 1 line
1.5.3.4.1157.g0e74-dirty
#6 lore raw

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

In reply to Sam Ravnborg
Quoted 10 lines · On Sat, 10 Nov 2007 21:43:28 +0100 Sam Ravnborg wrote:
---
 README |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)
README +0 -0
index 159912c..6622ba1 100644

README CONFIGURING the kernel: · line 197

Quoted 4 lines
README +1 -0
@@ -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" ...

README CONFIGURING the kernel: · line 201

Quoted 5 lines
README +1 -0
+   symbol "64BIT" to the value of the K64BIT variable.
 
 	NOTES on "make config":
 	- having unnecessary drivers will make the kernel bigger, and can

---
~Randy

#7 lore raw

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

In reply to Sam Ravnborg
Quoted 13 lines · 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.

---
 arch/x86/Kconfig |   26 ++++++++------------------
 1 files changed, 8 insertions(+), 18 deletions(-)
arch/x86/Kconfig +0 -0
index 153c26c..0d86611 100644

arch/x86/Kconfig line 5

Quoted 8 lines
arch/x86/Kconfig +2 -3
@@ -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"

arch/x86/Kconfig line 8

Quoted 3 lines
arch/x86/Kconfig +3 -0
+	default n
+	help
+	  Say yes to build a 64 bit kernel - formerly known as x86_64
	                     64-bit

arch/x86/Kconfig line 9

Quoted 1 line
arch/x86/Kconfig +1 -0
+	  Say no to build a 32 bit kernel - formerly known as i386
	                    32-bit
Quoted 21 lines
arch/x86/Kconfig +2 -12
 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

arch/x86/Kconfig line 24

Quoted 10 lines
arch/x86/Kconfig +0 -3
 config X86
 	bool
 	default y
 
-config 64BIT
-	def_bool X86_64
-
 config GENERIC_TIME
 	bool
 	default y

---
~Randy

#8 lore raw

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

In reply to Sam Ravnborg

Hi,

Quoted 2 lines · Sam Ravnborg

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?

Signature · 1 line
Guillaume
#9 lore raw

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

Quoted 9 lines · Guillaume Chazarain

Hi,

Quoted 2 lines · Sam Ravnborg

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?

Quoted 1 line

Guillaume

cu
Adrian

Signature · 4 lines
       "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
#10 lore raw

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

Hi Adrian,

Quoted 1 line · Adrian Bunk

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.

Signature · 1 line
Guillaume
#11 lore raw

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

Quoted 1 line · Guillaume Chazarain

Hi Adrian,

Hi Guillaume,

Quoted 7 lines · Adrian Bunk
Quoted 1 line

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.

Quoted 4 lines

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.

Quoted 2 lines

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

That's clear.

Quoted 2 lines

Cheers.
Guillaume

cu
Adrian

Signature · 4 lines
       "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
#12 lore raw

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

Quoted 2 lines · Adrian Bunk

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 ;-)

Quoted 10 lines
Quoted 4 lines

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.

Signature · 1 line
Guillaume
#13 lore raw

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

Quoted 9 lines · Guillaume Chazarain
Quoted 5 lines

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

#14 lore raw

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

Quoted 5 lines · Sam Ravnborg
Quoted 2 lines

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.

Signature · 1 line
Guillaume
#15 lore raw

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

In reply to Sam Ravnborg
Quoted 35 lines · 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.

---
 scripts/kconfig/conf.c      |    1 +
 scripts/kconfig/confdata.c  |   27 +++++++++++++++++++++++++++
 scripts/kconfig/lkc_proto.h |    1 +
 3 files changed, 29 insertions(+), 0 deletions(-)
scripts/kconfig/conf.c +1 -0
index a38787a..c6bee85 100644
@@ -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;
scripts/kconfig/confdata.c +0 -0
index e0f402f..0cb7555 100644

scripts/kconfig/confdata.c static int conf_set_sym_val(struct symbol *sym, int def, int def_flags, char *p) · line 148

Quoted 4 lines
scripts/kconfig/confdata.c +1 -0
@@ -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

scripts/kconfig/confdata.c static int conf_set_sym_val(struct symbol *sym, int def, int def_flags, char *p) · line 159

Quoted 11 lines
scripts/kconfig/confdata.c +11 -0
+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.

Quoted 8 lines
scripts/kconfig/confdata.c +8 -0
+		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)

scripts/kconfig/confdata.c static int conf_set_sym_val(struct symbol *sym, int def, int def_flags, char *p) · line 177

Quoted 10 lines
scripts/kconfig/confdata.c +7 -0
+				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

#16 lore raw

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

Quoted 44 lines · Randy Dunlap
Quoted 35 lines · 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.

---
 scripts/kconfig/conf.c      |    1 +
 scripts/kconfig/confdata.c  |   27 +++++++++++++++++++++++++++
 scripts/kconfig/lkc_proto.h |    1 +
 3 files changed, 29 insertions(+), 0 deletions(-)
scripts/kconfig/conf.c +1 -0
index a38787a..c6bee85 100644
@@ -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;
scripts/kconfig/confdata.c +0 -0
index e0f402f..0cb7555 100644

scripts/kconfig/confdata.c static int conf_set_sym_val(struct symbol *sym, int def, int def_flags, char *p) · line 148

Quoted 4 lines
scripts/kconfig/confdata.c +1 -0
@@ -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

Quoted 14 lines

scripts/kconfig/confdata.c static int conf_set_sym_val(struct symbol *sym, int def, int def_flags, char *p) · line 159

Quoted 11 lines
scripts/kconfig/confdata.c +11 -0
+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

#17 lore raw

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

In reply to Sam Ravnborg

Hi,

Quoted 7 lines · Sam Ravnborg
scripts/kconfig/confdata.c +7 -0
+	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);

scripts/kconfig/confdata.c static int conf_set_sym_val(struct symbol *sym, int def, int def_flags, char *p) · line 172

Quoted 10 lines
scripts/kconfig/confdata.c +10 -0
+			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

#18 lore raw

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

Hi Roman

Quoted 24 lines · Roman Zippel

Hi,

Quoted 7 lines · Sam Ravnborg
scripts/kconfig/confdata.c +7 -0
+	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);

scripts/kconfig/confdata.c static int conf_set_sym_val(struct symbol *sym, int def, int def_flags, char *p) · line 172

Quoted 10 lines
scripts/kconfig/confdata.c +10 -0
+			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:

Quoted 6 lines
	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?

Quoted 4 lines

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

#19 lore raw

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

Hi,

Quoted 1 line · Sam Ravnborg

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.

Quoted 3 lines

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.

Quoted 2 lines

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.

Quoted 2 lines

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

---
 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(-)
init/Kconfig +4 -0
Index: 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
scripts/kconfig/expr.c +8 -8
Index: 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:
scripts/kconfig/expr.h +3 -2
Index: 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 {
scripts/kconfig/lkc.h +5 -0
Index: 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)
 {
scripts/kconfig/menu.c +5 -2
Index: 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)) {
scripts/kconfig/qconf.cc +4 -11
Index: 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);
scripts/kconfig/symbol.c +45 -8

92 lines not on this page · raw message

scripts/kconfig/util.c +23 -2
Index: 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;
scripts/kconfig/zconf.gperf +1 -0
Index: 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
 %%
scripts/kconfig/zconf.hash.c_shipped +25 -20

140 lines not on this page · raw message

#20 lore raw

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

Quoted 7 lines · Roman Zippel

Hi,

Quoted 1 line · Sam Ravnborg

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.

Quoted 7 lines

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.

Quoted 6 lines

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.

Quoted 7 lines
Quoted 4 lines

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.

Quoted 8 lines
Quoted 2 lines

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.

Quoted 7 lines
Quoted 2 lines

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

#21 lore raw

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

Hi,

Quoted 5 lines · Sam Ravnborg
Quoted 3 lines
Quoted 1 line

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?

Quoted 3 lines
Quoted 1 line

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.

Quoted 3 lines
Quoted 2 lines

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

#22 lore raw

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

Hi Roman.

Quoted 8 lines
Quoted 5 lines
Quoted 3 lines
Quoted 1 line

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.

Quoted 7 lines
Quoted 4 lines
Quoted 2 lines

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.

Quoted 7 lines
Quoted 4 lines
Quoted 3 lines

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

#23 lore raw

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

Hi,

Quoted 7 lines · Sam Ravnborg
Quoted 2 lines

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.

Quoted 4 lines
Quoted 4 lines
Quoted 3 lines
Quoted 1 line

Please revert the K64BIT changes and use this instead.

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

Quoted 8 lines
Quoted 2 lines

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.

Quoted 8 lines
Quoted 6 lines
Quoted 3 lines
Quoted 2 lines

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

#24 lore raw

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

Quoted 12 lines · Roman Zippel

Hi,

Quoted 7 lines · Sam Ravnborg
Quoted 2 lines

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.

Quoted 4 lines

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.

Quoted 11 lines
Quoted 11 lines
Quoted 7 lines
Quoted 4 lines
Quoted 2 lines

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.

Quoted 8 lines
Quoted 2 lines

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

#25 lore raw

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

Hi,

Quoted 2 lines · Sam Ravnborg

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.

Quoted 2 lines

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

---
 Makefile         |    3 ++-
 arch/x86/Kconfig |    4 ++--
 2 files changed, 4 insertions(+), 3 deletions(-)
Makefile +2 -1
Index: 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/ \
arch/x86/Kconfig +2 -2
Index: 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

Revert

9c900a9c9d9351e55ab6a84e12e3a52c474c7c8b
0f855aa64b3f63d35a891510cf7db932a435c116
2a113281f5cd2febbab21a93c8943f8d3eece4d3

and K64BIT parts of

daa93fab824f2b8c35bd11670c7fab2f32b2de5f

---
 Makefile                    |    4 -
 README                      |    2 
 scripts/kconfig/conf.c      |    1 
 scripts/kconfig/confdata.c  |  146 ++++++++++++++++----------------------------
 scripts/kconfig/lkc_proto.h |    1 
 5 files changed, 56 insertions(+), 98 deletions(-)
Makefile +1 -3
Index: linux-2.6/Makefile
===================================================================
@@ -200,11 +200,9 @@ SRCARCH 	:= $(ARCH)
 # Additional ARCH settings for x86
 ifeq ($(ARCH),i386)
         SRCARCH := x86
-        K64BIT  := n
 endif
 ifeq ($(ARCH),x86_64)
         SRCARCH := x86
-        K64BIT  := y
 endif
 
 KCONFIG_CONFIG	?= .config
@@ -341,7 +339,7 @@ KERNELRELEASE = $(shell cat include/conf
 KERNELVERSION = $(VERSION).$(PATCHLEVEL).$(SUBLEVEL)$(EXTRAVERSION)
 
 export VERSION PATCHLEVEL SUBLEVEL KERNELRELEASE KERNELVERSION
-export ARCH SRCARCH K64BIT CONFIG_SHELL HOSTCC HOSTCFLAGS CROSS_COMPILE AS LD CC
+export ARCH SRCARCH CONFIG_SHELL HOSTCC HOSTCFLAGS CROSS_COMPILE AS LD CC
 export CPP AR NM STRIP OBJCOPY OBJDUMP MAKE AWK GENKSYMS PERL UTS_MACHINE
 export HOSTCXX HOSTCXXFLAGS LDFLAGS_MODULE CHECK CHECKFLAGS
 
README +0 -2
Index: linux-2.6/README
===================================================================
@@ -194,8 +194,6 @@ 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 if found, sets
-   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
scripts/kconfig/conf.c +0 -1
Index: linux-2.6/scripts/kconfig/conf.c
===================================================================
@@ -591,7 +591,6 @@ 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;
scripts/kconfig/confdata.c +55 -91

158 lines not on this page · raw message

scripts/kconfig/lkc_proto.h +0 -1
Index: linux-2.6/scripts/kconfig/lkc_proto.h
===================================================================
@@ -1,7 +1,6 @@
 
 /* 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));
#26 lore raw

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

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

Hi,

Quoted 2 lines · Sam Ravnborg

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.

Quoted 8 lines

This patch only adds some extra features.

Quoted 2 lines

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.

Quoted 28 lines

bye, Roman

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

Index: linux-2.6/Makefile
===================================================================

Makefile +2 -1
@@ -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
===================================================================

arch/x86/Kconfig mainmenu "Linux Kernel Configuration for · line 10

Quoted 10 lines
arch/x86/Kconfig +2 -2
@@ -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

#27 lore raw

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

Hi,

Quoted 5 lines · Randy Dunlap

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

#28 lore raw

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

In reply to Roman Zippel

Hi Roman.

Quoted 6 lines
Quoted 2 lines

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

#29 lore raw

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

Hi,

Quoted 1 line · Sam Ravnborg

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.

Quoted 1 line

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.

Quoted 2 lines

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.

Quoted 1 line

- 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?

Quoted 1 line

4) The changelogs miss title on reverted patches

Seriously?

bye, Roman

#30 lore raw

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

In reply to Roman Zippel

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

Quoted 146 lines

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.

---
 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
===================================================================

init/Kconfig +4 -0
@@ -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
===================================================================

scripts/kconfig/expr.c +8 -8
@@ -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
===================================================================

scripts/kconfig/expr.h +3 -2
@@ -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
===================================================================

scripts/kconfig/lkc.h +2 -0
@@ -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);

scripts/kconfig/lkc.h void str_printf(struct gstr *gs, const c · line 108

Quoted 4 lines
scripts/kconfig/lkc.h +1 -0
@@ -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:

Quoted 62 lines
scripts/kconfig/lkc.h +3 -0
+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
===================================================================

scripts/kconfig/menu.c +5 -2
@@ -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
===================================================================

scripts/kconfig/qconf.cc +4 -1
@@ -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;

scripts/kconfig/qconf.cc QString ConfigInfoView::debug_info(struc · line 1101

Quoted 8 lines
scripts/kconfig/qconf.cc +0 -5
@@ -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.

scripts/kconfig/qconf.cc QString ConfigInfoView::debug_info(struc · line 1106

Quoted 5 lines
scripts/kconfig/qconf.cc +0 -5
-		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.

Quoted 47 lines
scripts/kconfig/qconf.cc +0 -0
 		default:
 			debug += "unknown property: ";
 			debug += prop_get_type_name(prop->type);

Index: linux-2.6/scripts/kconfig/symbol.c
===================================================================

scripts/kconfig/symbol.c +11 -7
@@ -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;

scripts/kconfig/symbol.c void sym_calc_value(struct symbol *sym) · line 356

Quoted 6 lines
scripts/kconfig/symbol.c +3 -0
@@ -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.

Quoted 28 lines
scripts/kconfig/symbol.c +6 -1
 	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)
+{

scripts/kconfig/symbol.c const char *prop_get_type_name(enum prop · line 900

Quoted 10 lines
scripts/kconfig/symbol.c +10 -0
+	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);
Quoted 4 lines
scripts/kconfig/symbol.c +4 -0
+		return;
+	}
+
+	prop = prop_alloc(P_ENV, sym);

scripts/kconfig/symbol.c const char *prop_get_type_name(enum prop · line 914

Quoted 10 lines
scripts/kconfig/symbol.c +10 -0
+	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);
Quoted 54 lines
scripts/kconfig/symbol.c +1 -0
+}

Index: linux-2.6/scripts/kconfig/util.c
===================================================================

scripts/kconfig/util.c +23 -2
@@ -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
===================================================================

scripts/kconfig/zconf.gperf +1 -0
@@ -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
 %%
#31 lore raw

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

Hi,

Quoted 2 lines · Sam Ravnborg

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.

Quoted 6 lines

scripts/kconfig/symbol.c void sym_calc_value(struct symbol *sym) · line 356

Quoted 3 lines
scripts/kconfig/symbol.c +3 -0
+	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.

Quoted 4 lines

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

#32 lore raw

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

Quoted 8 lines · Roman Zippel

Hi,

Quoted 2 lines · Sam Ravnborg

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.

Quoted 10 lines
Quoted 7 lines

scripts/kconfig/symbol.c void sym_calc_value(struct symbol *sym) · line 356

Quoted 4 lines
scripts/kconfig/symbol.c +3 -0
+	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

Quoted 7 lines
Quoted 5 lines

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

#33 lore raw

PATCH 1/3 explicitly introduce expression list

In reply to Sam Ravnborg

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.

---
 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(-)
scripts/kconfig/confdata.c +4 -4
Index: 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;
 	}
 
scripts/kconfig/expr.c +8 -8
Index: 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:
scripts/kconfig/expr.h +4 -1
Index: 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;
scripts/kconfig/menu.c +1 -1
Index: 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)) {
scripts/kconfig/symbol.c +7 -6
Index: 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;
 }
#34 lore raw

PATCH 2/3 environment symbol support

In reply to Sam Ravnborg

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.

---
 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(-)
Documentation/kbuild/kconfig-language.txt +21 -0
Index: 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
 -----------------
scripts/kconfig/expr.h +2 -1
Index: 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 {
scripts/kconfig/lkc.h +5 -0
Index: 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)
 {
scripts/kconfig/menu.c +4 -1
Index: 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;
 	}
 }
 
scripts/kconfig/qconf.cc +5 -11
Index: 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);
scripts/kconfig/symbol.c +45 -0

72 lines not on this page · raw message

scripts/kconfig/util.c +21 -2
Index: 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;
scripts/kconfig/zconf.gperf +1 -0
Index: 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
 %%
scripts/kconfig/zconf.hash.c_shipped +25 -20

140 lines not on this page · raw message

#35 lore raw

PATCH 3/3 use environment option

In reply to Sam Ravnborg

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

---
 init/Kconfig             |    4 ++++
 scripts/kconfig/symbol.c |   14 --------------
 2 files changed, 4 insertions(+), 14 deletions(-)
init/Kconfig +4 -0
Index: linux-2.6/init/Kconfig
===================================================================
@@ -1,3 +1,7 @@
+config ARCH
+	string
+	option env="ARCH"
+
 config DEFCONFIG_LIST
 	string
 	depends on !UML
scripts/kconfig/symbol.c +0 -14
Index: 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;
#36 lore raw
In reply to Sam Ravnborg
Quoted 8 lines · 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.

Quoted 8 lines

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

#37 lore raw
Quoted 19 lines · Randy Dunlap
Quoted 8 lines · 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.

Quoted 8 lines

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

Quoted 8 lines

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

#38 lore raw
In reply to Sam Ravnborg
Quoted 3 lines · Sam Ravnborg

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.

Quoted 1 line

Sam

cu
Adrian

Signature · 4 lines
       "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
#39 lore raw
Quoted 11 lines · Adrian Bunk
Quoted 3 lines · Sam Ravnborg

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

#40 lore raw
In reply to Sam Ravnborg

Hi,

Quoted 2 lines · Sam Ravnborg

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

#41 lore raw
Quoted 8 lines · Roman Zippel

Hi,

Quoted 2 lines · Sam Ravnborg

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