[U-Boot] [PATCH v3 1/5] sandbox: Support 'env import' and 'env export'

[U-Boot] [PATCH v3 1/5] sandbox: Support 'env import' and 'env export'

[Simon Glass]

Adjust the code for these commands so that they work on sandbox.

Signed-off-by: Simon Glass <sjg@chromium.org>
---
Changes in v3: None
Changes in v2:
- Add new patch to get 'env import/export' working on sandbox

 common/cmd_nvedit.c | 31 ++++++++++++++++++-------------
 1 file changed, 18 insertions(+), 13 deletions(-)

diff --git a/common/cmd_nvedit.c b/common/cmd_nvedit.c
index ba9ba16..718d4eb 100644
--- a/common/cmd_nvedit.c
+++ b/common/cmd_nvedit.c
@@ -33,6 +33,7 @@
 #include <watchdog.h>
 #include <linux/stddef.h>
 #include <asm/byteorder.h>
+#include <asm/io.h>
 
 DECLARE_GLOBAL_DATA_PTR;
 
@@ -848,7 +849,8 @@ static int do_env_export(cmd_tbl_t *cmdtp, int flag,
 			 int argc, char * const argv[])
 {
 	char	buf[32];
-	char	*addr, *cmd, *res;
+	ulong	addr;
+	char	*ptr, *cmd, *res;
 	size_t	size = 0;
 	ssize_t	len;
 	env_t	*envp;
@@ -893,10 +895,11 @@ NXTARG:		;
 	if (argc < 1)
 		return CMD_RET_USAGE;
 
-	addr = (char *)simple_strtoul(argv[0], NULL, 16);
+	addr = simple_strtoul(argv[0], NULL, 16);
+	ptr = map_sysmem(addr, size);
 
 	if (size)
-		memset(addr, '\0', size);
+		memset(ptr, '\0', size);
 
 	argc--;
 	argv++;
@@ -904,7 +907,7 @@ NXTARG:		;
 	if (sep) {		/* export as text file */
 		len = hexport_r(&env_htab, sep,
 				H_MATCH_KEY | H_MATCH_IDENT,
-				&addr, size, argc, argv);
+				&ptr, size, argc, argv);
 		if (len < 0) {
 			error("Cannot export environment: errno = %d\n", errno);
 			return 1;
@@ -915,12 +918,12 @@ NXTARG:		;
 		return 0;
 	}
 
-	envp = (env_t *)addr;
+	envp = (env_t *)ptr;
 
 	if (chk)		/* export as checksum protected block */
 		res = (char *)envp->data;
 	else			/* export as raw binary data */
-		res = addr;
+		res = ptr;
 
 	len = hexport_r(&env_htab, '\0',
 			H_MATCH_KEY | H_MATCH_IDENT,
@@ -962,7 +965,8 @@ sep_err:
 static int do_env_import(cmd_tbl_t *cmdtp, int flag,
 			 int argc, char * const argv[])
 {
-	char	*cmd, *addr;
+	ulong	addr;
+	char	*cmd, *ptr;
 	char	sep = '\n';
 	int	chk = 0;
 	int	fmt = 0;
@@ -1006,12 +1010,13 @@ static int do_env_import(cmd_tbl_t *cmdtp, int flag,
 	if (!fmt)
 		printf("## Warning: defaulting to text format\n");
 
-	addr = (char *)simple_strtoul(argv[0], NULL, 16);
+	addr = simple_strtoul(argv[0], NULL, 16);
+	ptr = map_sysmem(addr, 0);
 
 	if (argc == 2) {
 		size = simple_strtoul(argv[1], NULL, 16);
 	} else {
-		char *s = addr;
+		char *s = ptr;
 
 		size = 0;
 
@@ -1031,7 +1036,7 @@ static int do_env_import(cmd_tbl_t *cmdtp, int flag,
 
 	if (chk) {
 		uint32_t crc;
-		env_t *ep = (env_t *)addr;
+		env_t *ep = (env_t *)ptr;
 
 		size -= offsetof(env_t, data);
 		memcpy(&crc, &ep->crc, sizeof(crc));
@@ -1040,11 +1045,11 @@ static int do_env_import(cmd_tbl_t *cmdtp, int flag,
 			puts("## Error: bad CRC, import failed\n");
 			return 1;
 		}
-		addr = (char *)ep->data;
+		ptr = (char *)ep->data;
 	}
 
-	if (himport_r(&env_htab, addr, size, sep, del ? 0 : H_NOCLEAR,
-			0, NULL) == 0) {
+	if (himport_r(&env_htab, ptr, size, sep, del ? 0 : H_NOCLEAR, 0,
+		      NULL) == 0) {
 		error("Environment import failed: errno = %d\n", errno);
 		return 1;
 	}
-- 
1.8.4.1

[U-Boot] [PATCH v3 2/5] Make 'run' use run_command_list() instead of run_command()

[Simon Glass]

In the case where an environment variable spans multiple lines, we should
use run_command_list() so that all lines are executed. This shold be
backwards compatible with existing behaviour for existing scripts.

Signed-off-by: Simon Glass <sjg@chromium.org>
---
Changes in v3: None
Changes in v2:
- Add new patch to adjust 'run' command to better support testing

 common/main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/common/main.c b/common/main.c
index 6f475f0..6650293 100644
--- a/common/main.c
+++ b/common/main.c
@@ -1544,7 +1544,7 @@ int do_run (cmd_tbl_t * cmdtp, int flag, int argc, char * const argv[])
 			return 1;
 		}
 
-		if (run_command(arg, flag) != 0)
+		if (run_command_list(arg, -1, flag) != 0)
 			return 1;
 	}
 	return 0;
-- 
1.8.4.1

[U-Boot] [PATCH v3 3/5] Allow U-Boot scripts to be placed in a .env file

[Simon Glass]

At present U-Boot environment variables, and thus scripts, are defined
by CONFIG_EXTRA_ENV_SETTINGS. It is painful to add large amounts of text
to this file and dealing with quoting and newlines is harder than it
should be. It would be better if we could just type the script into a
text file and have it included by U-Boot.

Add a feature that brings in a .env file associated with the board
config, if present. To use it, create a file in a board/<vendor>/env
directory called <board>.env (or common.env if you want the same
environment for all boards).

The environment variables should be of the form "var=value". Values can
extend to multiple lines. See the README under 'Environment Variables:'
for more information and an example.

Comments are not permitted in the environment with this commit.

Signed-off-by: Simon Glass <sjg@chromium.org>
---
Changes in v3:
- Add more detail in the README about the format of .env files
- Adjust Makefile to generate the .inc and .h files in separate fules
- Correctly terminate environment files with \n
- Improve the comment about " in the awk script

Changes in v2:
- Add dependency rule so that the environment is rebuilt when it changes
- Add information and updated example script to README
- Move .env file from include/configs to board/
- Use awk script to process environment since it is much easier on the brain

 Makefile                     | 35 +++++++++++++++++++++++++++++++-
 README                       | 38 +++++++++++++++++++++++++++++++++++
 common/env_embedded.c        |  1 +
 config.mk                    |  2 ++
 include/env_default.h        |  2 ++
 mkconfig                     |  7 +++++++
 tools/scripts/env2string.awk | 48 ++++++++++++++++++++++++++++++++++++++++++++
 7 files changed, 132 insertions(+), 1 deletion(-)
 create mode 100644 tools/scripts/env2string.awk

diff --git a/Makefile b/Makefile
index 2d18d27..a9b4b9e 100644
--- a/Makefile
+++ b/Makefile
@@ -708,7 +708,7 @@ $(obj)include/autoconf.mk.dep: $(obj)include/config.h include/common.h
 	$(CC) -x c -DDO_DEPS_ONLY -M $(CFLAGS) $(CPPFLAGS) \
 		-MQ $(obj)include/autoconf.mk include/common.h > $@
 
-$(obj)include/autoconf.mk: $(obj)include/config.h
+$(obj)include/generated/autoconf.mk.base: $(obj)include/config.h
 	@$(XECHO) Generating $@ ; \
 	set -e ; \
 	: Extract the config macros ; \
@@ -734,6 +734,39 @@ $(obj)include/spl-autoconf.mk: $(obj)include/config.h
 	sed -n -f tools/scripts/define2mk.sed > $@.tmp && \
 	mv $@.tmp $@
 
+# We expect '<board>.env' but failing that will use 'common.env'
+ENV_HEADER := $(obj)include/generated/environment.h
+ENV_DIR := $(if $(VENDOR),$(VENDOR)/env,$(BOARD)/env)
+ENV_FILE_BOARD := $(src)board/${ENV_DIR}/$(BOARD).env
+ENV_FILE_COMMON := $(src)board/${ENV_DIR}/common.env
+ENV_FILE := $(if $(wildcard $(ENV_FILE_BOARD)),$(ENV_FILE_BOARD),$(ENV_FILE_COMMON))
+
+# Regenerate the environment if it changes
+$(obj)include/generated/environment.in: $(obj)include/generated/autoconf.mk.base \
+		$(wildcard $(ENV_FILE))
+	if [ -f "$(ENV_FILE)" ]; then \
+		cat $(ENV_FILE) >$@ ; \
+	else \
+		echo -n >$@ ; \
+	fi
+
+$(obj)include/generated/environment.inc: $(obj)include/generated/environment.in
+	@$(XECHO) Generating $@ ; \
+	set -e ; \
+	: Process the environment file ; \
+	echo -n "CONFIG_EXTRA_ENV_TEXT=" >$@ ; \
+	awk -f tools/scripts/env2string.awk $< >>$@
+
+$(ENV_HEADER): $(obj)include/generated/environment.in
+	@$(XECHO) Generating $@ ; \
+	set -e ; \
+	: Process the environment file ; \
+	echo -n "#define CONFIG_EXTRA_ENV_TEXT " >$@ ; \
+	awk -f tools/scripts/env2string.awk $< >>$@
+
+$(obj)include/autoconf.mk: $(obj)include/generated/environment.inc $(ENV_HEADER)
+	cat $(obj)include/generated/autoconf.mk.base $< >$@
+
 $(obj)include/generated/generic-asm-offsets.h:	$(obj)include/autoconf.mk.dep \
 	$(obj)include/spl-autoconf.mk \
 	$(obj)include/tpl-autoconf.mk \
diff --git a/README b/README
index f0eedbb..3146a2d 100644
--- a/README
+++ b/README
@@ -4556,6 +4556,44 @@ environment. As long as you don't save the environment you are
 working with an in-memory copy. In case the Flash area containing the
 environment is erased by accident, a default environment is provided.
 
+The default environment is created in include/env_default.h, and can be
+augmented by various CONFIG defines. See that file for details. In
+particular you can define CONFIG_EXTRA_ENV_SETTINGS in your board file
+to add environment variables (see 'CONFIG_EXTRA_ENV_SETTINGS' above
+for details).
+
+It is also possible to create an environment file with the name
+board/<vendor>/env/<board>.env for your board. If that file is not present
+then U-Boot will look for board/<vendor>/env/common.env so that you can
+have a common environment for all vendor boards.
+
+This is a plain text file where you can type your environment variables in
+the form 'var=value'. Blank lines and multi-line variables are supported.
+The conversion script looks for a line that starts with a letter or number
+and has an equals sign immediately afterwards. Spaces before the = are not
+permitted. It is a good idea to indent your scripts so that only the 'var='
+appears at the start of a line.
+
+For example, for snapper9260 you would create a text file called
+board/bluewater/env/snapper9260.env containing the environment text.
+
+>>>
+bootcmd=
+	if [ -z ${tftpserverip} ]; then
+		echo "Use 'setenv tftpserverip a.b.c.d' to set IP address."
+	fi
+
+	usb start; setenv autoload n; bootp;
+	tftpboot ${tftpserverip}:
+	bootm
+failed=
+	echo boot failed - please check your image
+<<<
+
+The resulting environment can be exported and importing using the
+'env export/import -t' commands.
+
+
 Some configuration options can be set using Environment Variables.
 
 List of environment variables (most likely not complete):
diff --git a/common/env_embedded.c b/common/env_embedded.c
index 1c4f915..1d7fe2a 100644
--- a/common/env_embedded.c
+++ b/common/env_embedded.c
@@ -74,6 +74,7 @@
 #endif
 
 #define DEFAULT_ENV_INSTANCE_EMBEDDED
+#include <config.h>
 #include <env_default.h>
 
 #ifdef CONFIG_ENV_ADDR_REDUND
diff --git a/config.mk b/config.mk
index 91a8f24..a9f3317 100644
--- a/config.mk
+++ b/config.mk
@@ -190,8 +190,10 @@ sinclude $(TOPDIR)/$(CPUDIR)/$(SOC)/config.mk	# include  SoC	specific rules
 endif
 ifdef	VENDOR
 BOARDDIR = $(VENDOR)/$(BOARD)
+ENVDIR=${vendor}/env
 else
 BOARDDIR = $(BOARD)
+ENVDIR=${board}/env
 endif
 ifdef	BOARD
 sinclude $(TOPDIR)/board/$(BOARDDIR)/config.mk	# include board specific rules
diff --git a/include/env_default.h b/include/env_default.h
index 90431be..b88b1e4 100644
--- a/include/env_default.h
+++ b/include/env_default.h
@@ -121,6 +121,8 @@ const uchar default_environment[] = {
 #ifdef	CONFIG_EXTRA_ENV_SETTINGS
 	CONFIG_EXTRA_ENV_SETTINGS
 #endif
+	/* This is created in the Makefile */
+	CONFIG_EXTRA_ENV_TEXT
 	"\0"
 #ifdef DEFAULT_ENV_INSTANCE_EMBEDDED
 	}
diff --git a/mkconfig b/mkconfig
index 1d06c8e..8240431 100755
--- a/mkconfig
+++ b/mkconfig
@@ -144,8 +144,10 @@ fi
 # Assign board directory to BOARDIR variable
 if [ -z "${vendor}" ] ; then
     BOARDDIR=${board}
+    ENVDIR=${board}/env
 else
     BOARDDIR=${vendor}/${board}
+    ENVDIR=${vendor}/env
 fi
 
 #
@@ -174,10 +176,15 @@ echo "#define CONFIG_SYS_BOARD \"${board}\"" >> config.h
 
 cat << EOF >> config.h
 #define CONFIG_BOARDDIR board/$BOARDDIR
+#define CONFIG_ENVDIR board/$ENVDIR
 #include <config_cmd_defaults.h>
 #include <config_defaults.h>
 #include <configs/${CONFIG_NAME}.h>
 #include <asm/config.h>
+#if !defined(DO_DEPS_ONLY) && !defined(D__UBOOT_CONFIG__) && \
+	!defined(__ASSEMBLY__)
+#include <generated/environment.h>
+#endif
 #include <config_fallbacks.h>
 #include <config_uncmd_spl.h>
 EOF
diff --git a/tools/scripts/env2string.awk b/tools/scripts/env2string.awk
new file mode 100644
index 0000000..2a86494
--- /dev/null
+++ b/tools/scripts/env2string.awk
@@ -0,0 +1,48 @@
+#
+# (C) Copyright 2013 Google, Inc
+#
+# SPDX-License-Identifier:	GPL-2.0+
+#
+# Sed script to parse a text file containing an environment and convert it
+# to a C string which can be compiled into U-Boot.
+#
+
+# We output a double quote before starting, and again when we finish so that
+# all output is quoted.
+BEGIN {
+	# env holds the env variable we are currently processing
+	env = "";
+	ORS = ""
+	print "\""
+}
+
+{
+	# Quote quotes
+	gsub("\"", "\\\"")
+
+	# Is this the start of a new environment variable?
+	if (match($0, "^([^ =][^ =]*)=(.*)", arr)) {
+		if (length(env) != 0) {
+			# Record the value of the variable now completed
+			vars[var] = env
+		}
+		var = arr[1]
+		env = arr[2]
+	} else {
+		# Change newline to \n
+		env = env "\\n" $0;
+	}
+}
+
+END {
+	# Record the value of the variable now completed
+	if (length(env) != 0) {
+		vars[var] = env
+	}
+
+	# Print out all the variables
+	for (var in vars) {
+		print var "=" vars[var] "\\0";
+	}
+	print "\"\n"
+}
-- 
1.8.4.1

[U-Boot] [PATCH v3 4/5] env: Allow environment files to use the C preprocessor

[Simon Glass]

In many cases environment variables need access to the U-Boot CONFIG
variables to select different options. Enable this so that the environment
scripts can be as useful as the ones currently in the board config files.

Signed-off-by: Simon Glass <sjg@chromium.org>
---
Changes in v3:
- Define __UBOOT_CONFIG__ when collecting environment files

Changes in v2:
- Add separate patch to enable C preprocessor for environment files
- Enable var+=value form to simplify composing variables in multiple steps

 Makefile                     |  4 +++-
 README                       | 18 +++++++++++++++++-
 tools/scripts/env2string.awk |  6 ++++++
 3 files changed, 26 insertions(+), 2 deletions(-)

diff --git a/Makefile b/Makefile
index a9b4b9e..8c96310 100644
--- a/Makefile
+++ b/Makefile
@@ -745,7 +745,9 @@ ENV_FILE := $(if $(wildcard $(ENV_FILE_BOARD)),$(ENV_FILE_BOARD),$(ENV_FILE_COMM
 $(obj)include/generated/environment.in: $(obj)include/generated/autoconf.mk.base \
 		$(wildcard $(ENV_FILE))
 	if [ -f "$(ENV_FILE)" ]; then \
-		cat $(ENV_FILE) >$@ ; \
+		$(CPP) -P $(CFLAGS) -x assembler-with-cpp -D__ASSEMBLY__ \
+			-D__UBOOT_CONFIG__ -include $(obj)include/config.h \
+			$(ENV_FILE) -o $@; \
 	else \
 		echo -n >$@ ; \
 	fi
diff --git a/README b/README
index 3146a2d..7428b0c 100644
--- a/README
+++ b/README
@@ -4574,11 +4574,25 @@ and has an equals sign immediately afterwards. Spaces before the = are not
 permitted. It is a good idea to indent your scripts so that only the 'var='
 appears at the start of a line.
 
+To add additional text to a variable you can use var+=value. This text is
+merged into the variable during the make process and made available as a
+single value to U-Boot.
+
 For example, for snapper9260 you would create a text file called
 board/bluewater/env/snapper9260.env containing the environment text.
 
+This file can include C-style comments. Blank lines and multi-line
+variables are supported, and you can use normal C preprocessor directives
+and CONFIG defines from your board config also.
+
 >>>
+stdout=serial
+#ifdef CONFIG_LCD
+stdout+=,lcd
+#endif
 bootcmd=
+	/* U-Boot script for booting */
+
 	if [ -z ${tftpserverip} ]; then
 		echo "Use 'setenv tftpserverip a.b.c.d' to set IP address."
 	fi
@@ -4587,7 +4601,9 @@ bootcmd=
 	tftpboot ${tftpserverip}:
 	bootm
 failed=
-	echo boot failed - please check your image
+	/* Print a message when boot fails */
+	echo CONFIG_SYS_BOARD boot failed - please check your image
+	echo Load address is CONFIG_SYS_LOAD_ADDR
 <<<
 
 The resulting environment can be exported and importing using the
diff --git a/tools/scripts/env2string.awk b/tools/scripts/env2string.awk
index 2a86494..f2b6d1a 100644
--- a/tools/scripts/env2string.awk
+++ b/tools/scripts/env2string.awk
@@ -28,6 +28,12 @@ BEGIN {
 		}
 		var = arr[1]
 		env = arr[2]
+
+		# Deal with +=
+		if (match(var, "(.*)[+]$", var_arr)) {
+			var = var_arr[1]
+			env = vars[var] env
+		}
 	} else {
 		# Change newline to \n
 		env = env "\\n" $0;
-- 
1.8.4.1

[U-Boot] [PATCH v3 5/5] RFC: tegra: Convert to using environment files

[Simon Glass]

This seems more intuitive that the current #define way of doing things.
The resulting code is shorter, avoids the quoting and line continuation
pain, and also improves the clumsy way that stdio variables are created:

 #ifdef CONFIG_VIDEO_TEGRA
 #define STDOUT_LCD ",lcd"
 #else
 #define STDOUT_LCD ""
 #endif

...
 #define TEGRA_DEVICE_SETTINGS \
	"stdout=serial" STDOUT_LCD "\0" \
	...

The MEM_LAYOUT_ENV_SETTINGS variable is left in the header files, since
it depends on the SOC type and we probably don't want to add .emv files
for each board at this stage.

Signed-off-by: Simon Glass <sjg@chromium.org>
---
Changes in v3: None
Changes in v2:
- Add new patch to illustrate the impact on Tegra environment

 board/nvidia/env/common.env         |  79 ++++++++++++++++++++++++
 include/configs/tegra-common-post.h | 120 +-----------------------------------
 2 files changed, 80 insertions(+), 119 deletions(-)
 create mode 100644 board/nvidia/env/common.env

diff --git a/board/nvidia/env/common.env b/board/nvidia/env/common.env
new file mode 100644
index 0000000..aca36a5
--- /dev/null
+++ b/board/nvidia/env/common.env
@@ -0,0 +1,79 @@
+#ifndef CONFIG_BOOTCOMMAND
+rootpart=1
+script_boot=
+	if load ${devtype} ${devnum}:${rootpart}
+			${scriptaddr} ${prefix}${script}; then
+		echo ${script} found! Executing ...
+		source ${scriptaddr}
+	fi
+
+scan_boot=
+	echo Scanning ${devtype} ${devnum}...
+	for prefix in ${boot_prefixes}; do
+		for script in ${boot_scripts}; do
+			run script_boot
+		done
+	done
+
+boot_targets=
+boot_prefixes=/ /boot/
+boot_scripts=boot.scr.uimg boot.scr
+
+#ifdef CONFIG_CMD_MMC
+mmc_boot=
+	setenv devtype mmc
+		if mmc dev ${devnum}; then
+			run scan_boot
+		fi
+bootcmd_mmc0=setenv devnum 0; run mmc_boot
+bootcmd_mmc1=setenv devnum 1; run mmc_booxt
+boot_targets+= mmc1 mmc0
+#endif
+
+#ifdef CONFIG_CMD_USB
+#define BOOTCMD_INIT_USB run usb_init
+usb_init=
+	if ${usb_need_init}; then
+		set usb_need_init false
+		usb start 0
+	fi
+usb_boot=
+	setenv devtype usb
+	BOOTCMD_INIT_USB
+	if usb dev ${devnum}; then
+		run scan_boot
+	fi
+bootcmd_usb0=setenv devnum 0; run usb_boot
+boot_targets+= usb0
+#else
+#define BOOTCMD_INIT_USB
+#endif
+
+#ifdef CONFIG_CMD_DHCP
+bootcmd_dhcp=
+	BOOTCMD_INIT_USB
+	if dhcp ${scriptaddr} boot.scr.uimg; then
+		source ${scriptaddr}
+	fi
+boot_targets+= dhcp
+#endif
+
+bootcmd=for target in ${boot_targets}; do run bootcmd_${target}; done
+#endif
+
+/* Decide on values for stdio */
+stdin=serial
+stdout=serial
+stderr=serial
+
+#ifdef CONFIG_TEGRA_KEYBOARD
+stdin+=,tegra-kbc
+#endif
+
+#ifdef CONFIG_USB_KEYBOARD
+stdin+=,usbkbd
+#endif
+
+#ifdef CONFIG_VIDEO_TEGRA
+stdout+=,lcd
+#endif
diff --git a/include/configs/tegra-common-post.h b/include/configs/tegra-common-post.h
index a3242fe..8465a1e 100644
--- a/include/configs/tegra-common-post.h
+++ b/include/configs/tegra-common-post.h
@@ -8,131 +8,13 @@
 #ifndef __TEGRA_COMMON_POST_H
 #define __TEGRA_COMMON_POST_H
 
-#ifdef CONFIG_BOOTCOMMAND
-
-#define BOOTCMDS_COMMON ""
-
-#else
-
-#ifdef CONFIG_CMD_MMC
-#define BOOTCMDS_MMC \
-	"mmc_boot=" \
-		"setenv devtype mmc; " \
-		"if mmc dev ${devnum}; then " \
-			"run scan_boot; " \
-		"fi\0" \
-	"bootcmd_mmc0=setenv devnum 0; run mmc_boot;\0" \
-	"bootcmd_mmc1=setenv devnum 1; run mmc_boot;\0"
-#define BOOT_TARGETS_MMC "mmc1 mmc0"
-#else
-#define BOOTCMDS_MMC ""
-#define BOOT_TARGETS_MMC ""
-#endif
-
-#ifdef CONFIG_CMD_USB
-#define BOOTCMD_INIT_USB "run usb_init; "
-#define BOOTCMDS_USB \
-	"usb_init=" \
-		"if ${usb_need_init}; then " \
-			"set usb_need_init false; " \
-			"usb start 0; " \
-		"fi\0" \
-	\
-	"usb_boot=" \
-		"setenv devtype usb; " \
-		BOOTCMD_INIT_USB \
-		"if usb dev ${devnum}; then " \
-			"run scan_boot; " \
-		"fi\0" \
-	\
-	"bootcmd_usb0=setenv devnum 0; run usb_boot;\0"
-#define BOOT_TARGETS_USB "usb0"
-#else
-#define BOOTCMD_INIT_USB ""
-#define BOOTCMDS_USB ""
-#define BOOT_TARGETS_USB ""
-#endif
-
-#ifdef CONFIG_CMD_DHCP
-#define BOOTCMDS_DHCP \
-	"bootcmd_dhcp=" \
-		BOOTCMD_INIT_USB \
-		"if dhcp ${scriptaddr} boot.scr.uimg; then "\
-			"source ${scriptaddr}; " \
-		"fi\0"
-#define BOOT_TARGETS_DHCP "dhcp"
-#else
-#define BOOTCMDS_DHCP ""
-#define BOOT_TARGETS_DHCP ""
-#endif
-
-#define BOOTCMDS_COMMON \
-	"rootpart=1\0" \
-	\
-	"script_boot="                                                    \
-		"if load ${devtype} ${devnum}:${rootpart} "               \
-				"${scriptaddr} ${prefix}${script}; then " \
-			"echo ${script} found! Executing ...;"            \
-			"source ${scriptaddr};"                           \
-		"fi;\0"                                                   \
-	\
-	"scan_boot="                                                      \
-		"echo Scanning ${devtype} ${devnum}...; "                 \
-		"for prefix in ${boot_prefixes}; do "                     \
-			"for script in ${boot_scripts}; do "              \
-				"run script_boot; "                       \
-			"done; "                                          \
-		"done;\0"                                                 \
-	\
-	"boot_targets=" \
-		BOOT_TARGETS_MMC " " \
-		BOOT_TARGETS_USB " " \
-		BOOT_TARGETS_DHCP " " \
-		"\0" \
-	\
-	"boot_prefixes=/ /boot/\0" \
-	\
-	"boot_scripts=boot.scr.uimg boot.scr\0" \
-	\
-	BOOTCMDS_MMC \
-	BOOTCMDS_USB \
-	BOOTCMDS_DHCP
-
-#define CONFIG_BOOTCOMMAND \
-	"for target in ${boot_targets}; do run bootcmd_${target}; done"
-
-#endif
-
-#ifdef CONFIG_TEGRA_KEYBOARD
-#define STDIN_KBD_KBC ",tegra-kbc"
-#else
-#define STDIN_KBD_KBC ""
-#endif
-
 #ifdef CONFIG_USB_KEYBOARD
-#define STDIN_KBD_USB ",usbkbd"
 #define CONFIG_SYS_USB_EVENT_POLL
 #define CONFIG_PREBOOT			"usb start"
-#else
-#define STDIN_KBD_USB ""
 #endif
 
-#ifdef CONFIG_VIDEO_TEGRA
-#define STDOUT_LCD ",lcd"
-#else
-#define STDOUT_LCD ""
-#endif
-
-#define TEGRA_DEVICE_SETTINGS \
-	"stdin=serial" STDIN_KBD_KBC STDIN_KBD_USB "\0" \
-	"stdout=serial" STDOUT_LCD "\0" \
-	"stderr=serial" STDOUT_LCD "\0" \
-	""
-
 #define CONFIG_EXTRA_ENV_SETTINGS \
-	TEGRA_DEVICE_SETTINGS \
-	MEM_LAYOUT_ENV_SETTINGS \
-	BOOTCMDS_COMMON
+	MEM_LAYOUT_ENV_SETTINGS
 
 #if defined(CONFIG_TEGRA20_SFLASH) || defined(CONFIG_TEGRA20_SLINK) || defined(CONFIG_TEGRA114_SPI)
 #define CONFIG_FDT_SPI
-- 
1.8.4.1

[U-Boot] [PATCH v3 3/5] Allow U-Boot scripts to be placed in a .env file

[Wolfgang Denk]

Dear Simon,

In message <1382763695-2849-4-git-send-email-sjg@chromium.org> you wrote:
>
> +For example, for snapper9260 you would create a text file called
> +board/bluewater/env/snapper9260.env containing the environment text.
> +
> +>>>
> +bootcmd=
> +	if [ -z ${tftpserverip} ]; then
> +		echo "Use 'setenv tftpserverip a.b.c.d' to set IP address."
> +	fi
> +
> +	usb start; setenv autoload n; bootp;
> +	tftpboot ${tftpserverip}:
> +	bootm
> +failed=
> +	echo boot failed - please check your image
> +<<<
> +
> +The resulting environment can be exported and importing using the
> +'env export/import -t' commands.

I think this statement is misleading.  It reads as if thois text coul
actually be imported using "env import -t", which is not correct.  And
the result of an "env export -t" of equivalent command settings will
look pretty much different, too.

I can see why you like such a "beautified" text format, but I don;t
think you are doing anybody a favour here.  Can we not rather use
_exactly_ the same text format as U-Boot uses with it's import /
export commands?

This would make it _much_ easier to experiment on a system and modify
the environment until it fits all the requirements and passes all
tests, and then export it as a text file and use this directly
(without editing) for the input needed here?

...

> --- /dev/null
> +++ b/tools/scripts/env2string.awk
> @@ -0,0 +1,48 @@
> +#
> +# (C) Copyright 2013 Google, Inc
> +#
> +# SPDX-License-Identifier:	GPL-2.0+
> +#
> +# Sed script to parse a text file containing an environment and convert it
> +# to a C string which can be compiled into U-Boot.

That doesn't look like sed to me, looks more like awk :-)

> +	# Is this the start of a new environment variable?
> +	if (match($0, "^([^ =][^ =]*)=(.*)", arr)) {

I think this is a bit naive...

Example (using notation as exported by U-Boot using "env export -t"):

	foo=setenv xxx\
	one=1;setenv yyy\
	two=2;setenv zzz\
	three=3

> +	# Print out all the variables
> +	for (var in vars) {
> +		print var "=" vars[var] "\\0";
> +	}

I think it should not be difficult to find examples that would result
incorrect output.

I guess this needs more work - but then - why define a new format at
all?  Why not use what U-Boot uses itself?

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
One essential to success is that you desire be an all-obsessing  one,
your thoughts and aims be co-ordinated, and your energy be concentra-
ted and applied without letup.

[U-Boot] [PATCH v3 3/5] Allow U-Boot scripts to be placed in a .env file

[Otavio Salvador]

Hello,

On Sat, Oct 26, 2013 at 3:01 AM, Simon Glass <sjg@chromium.org> wrote:
> At present U-Boot environment variables, and thus scripts, are defined
> by CONFIG_EXTRA_ENV_SETTINGS. It is painful to add large amounts of text
> to this file and dealing with quoting and newlines is harder than it
> should be. It would be better if we could just type the script into a
> text file and have it included by U-Boot.
>
> Add a feature that brings in a .env file associated with the board
> config, if present. To use it, create a file in a board/<vendor>/env
> directory called <board>.env (or common.env if you want the same
> environment for all boards).
>
> The environment variables should be of the form "var=value". Values can
> extend to multiple lines. See the README under 'Environment Variables:'
> for more information and an example.
>
> Comments are not permitted in the environment with this commit.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>

I think people (or myself) are misunderstanding what this patch does.

My understand is:

 - it allow moving environment setting to a .env file
 - it needs to include the environment at /build/ time

This does improve a lot the current status (and I really appreciate
it); one missing thing (or I missed it completely) is a way to:

 - build u-boot (binary)
 - generate a binary version of the environment
 - glue both together in a way the new environment /replaces/ the
default environment originally written inside u-boot (binary)

Am I missing something?


-- 
Otavio Salvador                             O.S. Systems
http://www.ossystems.com.br        http://code.ossystems.com.br
Mobile: +55 (53) 9981-7854            Mobile: +1 (347) 903-9750

[U-Boot] [PATCH v3 5/5] RFC: tegra: Convert to using environment files

[Stephen Warren]

On 10/25/2013 11:01 PM, Simon Glass wrote:
> This seems more intuitive that the current #define way of doing things.
> The resulting code is shorter, avoids the quoting and line continuation
> pain, and also improves the clumsy way that stdio variables are created:
> 
>  #ifdef CONFIG_VIDEO_TEGRA
>  #define STDOUT_LCD ",lcd"
>  #else
>  #define STDOUT_LCD ""
>  #endif
> 
> ...
>  #define TEGRA_DEVICE_SETTINGS \
> 	"stdout=serial" STDOUT_LCD "\0" \
> 	...
> 
> The MEM_LAYOUT_ENV_SETTINGS variable is left in the header files, since
> it depends on the SOC type and we probably don't want to add .emv files
> for each board at this stage.

I guess I'm fine with this as long as e.g.
board/compulab/env/trimslice.env can contain:

#include "../../../board/nvidia/env/common.env"

... or perhaps the include path is set up to include board/nvidia/env
already, so it could just contain:

#include <common-nvidia.env>

> diff --git a/board/nvidia/env/common.env b/board/nvidia/env/common.env

> +bootcmd_mmc0=setenv devnum 0; run mmc_boot
> +bootcmd_mmc1=setenv devnum 1; run mmc_booxt
> +boot_targets+= mmc1 mmc0

I still don't see why = needs no space before/after, but += needs no
space before, but a space after. That simply looks like a typo to me,
and I'd be inclined to fix it were I editing this file. If a sed script
can't handle more flexible white-space, perhaps use Python or perhaps
Perl instead?

[U-Boot] [PATCH v3 3/5] Allow U-Boot scripts to be placed in a .env file

[Simon Glass]

Hi Wolfgang,

On Sat, Oct 26, 2013 at 2:26 PM, Wolfgang Denk <wd@denx.de> wrote:
> Dear Simon,
>
> In message <1382763695-2849-4-git-send-email-sjg@chromium.org> you wrote:
>>
>> +For example, for snapper9260 you would create a text file called
>> +board/bluewater/env/snapper9260.env containing the environment text.
>> +
>> +>>>
>> +bootcmd=
>> +     if [ -z ${tftpserverip} ]; then
>> +             echo "Use 'setenv tftpserverip a.b.c.d' to set IP address."
>> +     fi
>> +
>> +     usb start; setenv autoload n; bootp;
>> +     tftpboot ${tftpserverip}:
>> +     bootm
>> +failed=
>> +     echo boot failed - please check your image
>> +<<<
>> +
>> +The resulting environment can be exported and importing using the
>> +'env export/import -t' commands.
>
> I think this statement is misleading.  It reads as if thois text coul
> actually be imported using "env import -t", which is not correct.  And
> the result of an "env export -t" of equivalent command settings will
> look pretty much different, too.

The point here is that it is possible to export the default
environment that has been created at build time. Agreed this should be
worded better.

>
> I can see why you like such a "beautified" text format, but I don;t
> think you are doing anybody a favour here.  Can we not rather use
> _exactly_ the same text format as U-Boot uses with it's import /
> export commands?

That would be nice, but how to we handle newlines? Some scripts are
quite long. Do we need to put \ at the end of every line? That feels a
bit painful to me.

Also how do we handle #define? Without it I don't think this feature
is useful, since the existing build system often sticks CONFIG
variables into the environment.

>
> This would make it _much_ easier to experiment on a system and modify
> the environment until it fits all the requirements and passes all
> tests, and then export it as a text file and use this directly
> (without editing) for the input needed here?

I believe that is already possible - you should be able to take the
output of 'env export -t' and put it in the .env file. I have not
tried it though.

>
> ...
>
>> --- /dev/null
>> +++ b/tools/scripts/env2string.awk
>> @@ -0,0 +1,48 @@
>> +#
>> +# (C) Copyright 2013 Google, Inc
>> +#
>> +# SPDX-License-Identifier:   GPL-2.0+
>> +#
>> +# Sed script to parse a text file containing an environment and convert it
>> +# to a C string which can be compiled into U-Boot.
>
> That doesn't look like sed to me, looks more like awk :-)

Hmm, yes I gave up on sed after a while. Will fix.

>
>> +     # Is this the start of a new environment variable?
>> +     if (match($0, "^([^ =][^ =]*)=(.*)", arr)) {
>
> I think this is a bit naive...
>
> Example (using notation as exported by U-Boot using "env export -t"):
>
>         foo=setenv xxx\
>         one=1;setenv yyy\
>         two=2;setenv zzz\
>         three=3
>
>> +     # Print out all the variables
>> +     for (var in vars) {
>> +             print var "=" vars[var] "\\0";
>> +     }
>
> I think it should not be difficult to find examples that would result
> incorrect output.

I will take a look at this - here it is the \ at the end of line which
needs to be handled.

>
> I guess this needs more work - but then - why define a new format at
> all?  Why not use what U-Boot uses itself?

See above, would be good to resolve this issue before going any further.

Regards,
Simon

[U-Boot] [PATCH v3 3/5] Allow U-Boot scripts to be placed in a .env file

[Simon Glass]

Hi Otavio,

On Mon, Oct 28, 2013 at 7:34 AM, Otavio Salvador
<otavio@ossystems.com.br> wrote:
> Hello,
>
> On Sat, Oct 26, 2013 at 3:01 AM, Simon Glass <sjg@chromium.org> wrote:
>> At present U-Boot environment variables, and thus scripts, are defined
>> by CONFIG_EXTRA_ENV_SETTINGS. It is painful to add large amounts of text
>> to this file and dealing with quoting and newlines is harder than it
>> should be. It would be better if we could just type the script into a
>> text file and have it included by U-Boot.
>>
>> Add a feature that brings in a .env file associated with the board
>> config, if present. To use it, create a file in a board/<vendor>/env
>> directory called <board>.env (or common.env if you want the same
>> environment for all boards).
>>
>> The environment variables should be of the form "var=value". Values can
>> extend to multiple lines. See the README under 'Environment Variables:'
>> for more information and an example.
>>
>> Comments are not permitted in the environment with this commit.
>>
>> Signed-off-by: Simon Glass <sjg@chromium.org>
>
> I think people (or myself) are misunderstanding what this patch does.

Possibly, I'm not sure.

>
> My understand is:
>
>  - it allow moving environment setting to a .env file
>  - it needs to include the environment at /build/ time

Yes

>
> This does improve a lot the current status (and I really appreciate
> it); one missing thing (or I missed it completely) is a way to:
>
>  - build u-boot (binary)
>  - generate a binary version of the environment
>  - glue both together in a way the new environment /replaces/ the
> default environment originally written inside u-boot (binary)
>
> Am I missing something?

This is something that Wolfgang would like to see (as would I), but I
think it is a separate feature. This feature is just trying to
simplify creation of the build-time default environment.

Regards,
Simon

[U-Boot] [PATCH v3 5/5] RFC: tegra: Convert to using environment files

[Simon Glass]

Hi Stephen,

On Mon, Oct 28, 2013 at 1:59 PM, Stephen Warren <swarren@wwwdotorg.org> wrote:
> On 10/25/2013 11:01 PM, Simon Glass wrote:
>> This seems more intuitive that the current #define way of doing things.
>> The resulting code is shorter, avoids the quoting and line continuation
>> pain, and also improves the clumsy way that stdio variables are created:
>>
>>  #ifdef CONFIG_VIDEO_TEGRA
>>  #define STDOUT_LCD ",lcd"
>>  #else
>>  #define STDOUT_LCD ""
>>  #endif
>>
>> ...
>>  #define TEGRA_DEVICE_SETTINGS \
>>       "stdout=serial" STDOUT_LCD "\0" \
>>       ...
>>
>> The MEM_LAYOUT_ENV_SETTINGS variable is left in the header files, since
>> it depends on the SOC type and we probably don't want to add .emv files
>> for each board at this stage.
>
> I guess I'm fine with this as long as e.g.
> board/compulab/env/trimslice.env can contain:
>
> #include "../../../board/nvidia/env/common.env"
>
> ... or perhaps the include path is set up to include board/nvidia/env
> already, so it could just contain:
>
> #include <common-nvidia.env>
>
>> diff --git a/board/nvidia/env/common.env b/board/nvidia/env/common.env
>
>> +bootcmd_mmc0=setenv devnum 0; run mmc_boot
>> +bootcmd_mmc1=setenv devnum 1; run mmc_booxt
>> +boot_targets+= mmc1 mmc0
>
> I still don't see why = needs no space before/after, but += needs no
> space before, but a space after. That simply looks like a typo to me,
> and I'd be inclined to fix it were I editing this file. If a sed script
> can't handle more flexible white-space, perhaps use Python or perhaps
> Perl instead?

The old code was similar, in that it had a space after the quote.

We need the string to contain "mmc0 mmc1 usb0 dhcp" or perhaps "mmc0
mmc1". I chose to add a space at the start of each string, but
certainly we need a space somewhere, or we get "mmc0mmc1usb0dhcp".

Regards,
Simon

[U-Boot] [PATCH v3 5/5] RFC: tegra: Convert to using environment files

[Stephen Warren]

On 10/28/2013 02:34 PM, Simon Glass wrote:
> Hi Stephen,
> 
> On Mon, Oct 28, 2013 at 1:59 PM, Stephen Warren <swarren@wwwdotorg.org> wrote:
>> On 10/25/2013 11:01 PM, Simon Glass wrote:
>>> This seems more intuitive that the current #define way of doing things.
>>> The resulting code is shorter, avoids the quoting and line continuation
>>> pain, and also improves the clumsy way that stdio variables are created:
>>>
>>>  #ifdef CONFIG_VIDEO_TEGRA
>>>  #define STDOUT_LCD ",lcd"
>>>  #else
>>>  #define STDOUT_LCD ""
>>>  #endif
>>>
>>> ...
>>>  #define TEGRA_DEVICE_SETTINGS \
>>>       "stdout=serial" STDOUT_LCD "\0" \
>>>       ...
>>>
>>> The MEM_LAYOUT_ENV_SETTINGS variable is left in the header files, since
>>> it depends on the SOC type and we probably don't want to add .emv files
>>> for each board at this stage.
>>
>> I guess I'm fine with this as long as e.g.
>> board/compulab/env/trimslice.env can contain:
>>
>> #include "../../../board/nvidia/env/common.env"
>>
>> ... or perhaps the include path is set up to include board/nvidia/env
>> already, so it could just contain:
>>
>> #include <common-nvidia.env>
>>
>>> diff --git a/board/nvidia/env/common.env b/board/nvidia/env/common.env
>>
>>> +bootcmd_mmc0=setenv devnum 0; run mmc_boot
>>> +bootcmd_mmc1=setenv devnum 1; run mmc_booxt
>>> +boot_targets+= mmc1 mmc0
>>
>> I still don't see why = needs no space before/after, but += needs no
>> space before, but a space after. That simply looks like a typo to me,
>> and I'd be inclined to fix it were I editing this file. If a sed script
>> can't handle more flexible white-space, perhaps use Python or perhaps
>> Perl instead?
> 
> The old code was similar, in that it had a space after the quote.
> 
> We need the string to contain "mmc0 mmc1 usb0 dhcp" or perhaps "mmc0
> mmc1". I chose to add a space at the start of each string, but
> certainly we need a space somewhere, or we get "mmc0mmc1usb0dhcp".

Oh, I see. I thought the space was part of the += syntax, not the value.
Perhaps to make that more obvious, you could allow:

# No space added to value
var+=value
var+= value
var +=value
var += value

var+=value1 value2
var+= value1 value2
var +=value1 value2
var += value1 value2

var+="value1 value2"
var+= "value1 value2"
var +="value1 value2"
var += "value1 value2"

# One space included at start of addition to value
var+=" value1 value2"
var+= " value1 value2"
var +=" value1 value2"
var += " value1 value2"

[U-Boot] [PATCH v3 5/5] RFC: tegra: Convert to using environment files

[Simon Glass]

Hi Stephen,

On Mon, Oct 28, 2013 at 2:41 PM, Stephen Warren <swarren@wwwdotorg.org> wrote:
> On 10/28/2013 02:34 PM, Simon Glass wrote:
>> Hi Stephen,
>>
>> On Mon, Oct 28, 2013 at 1:59 PM, Stephen Warren <swarren@wwwdotorg.org> wrote:
>>> On 10/25/2013 11:01 PM, Simon Glass wrote:
>>>> This seems more intuitive that the current #define way of doing things.
>>>> The resulting code is shorter, avoids the quoting and line continuation
>>>> pain, and also improves the clumsy way that stdio variables are created:
>>>>
>>>>  #ifdef CONFIG_VIDEO_TEGRA
>>>>  #define STDOUT_LCD ",lcd"
>>>>  #else
>>>>  #define STDOUT_LCD ""
>>>>  #endif
>>>>
>>>> ...
>>>>  #define TEGRA_DEVICE_SETTINGS \
>>>>       "stdout=serial" STDOUT_LCD "\0" \
>>>>       ...
>>>>
>>>> The MEM_LAYOUT_ENV_SETTINGS variable is left in the header files, since
>>>> it depends on the SOC type and we probably don't want to add .emv files
>>>> for each board at this stage.
>>>
>>> I guess I'm fine with this as long as e.g.
>>> board/compulab/env/trimslice.env can contain:
>>>
>>> #include "../../../board/nvidia/env/common.env"
>>>
>>> ... or perhaps the include path is set up to include board/nvidia/env
>>> already, so it could just contain:
>>>
>>> #include <common-nvidia.env>
>>>
>>>> diff --git a/board/nvidia/env/common.env b/board/nvidia/env/common.env
>>>
>>>> +bootcmd_mmc0=setenv devnum 0; run mmc_boot
>>>> +bootcmd_mmc1=setenv devnum 1; run mmc_booxt
>>>> +boot_targets+= mmc1 mmc0
>>>
>>> I still don't see why = needs no space before/after, but += needs no
>>> space before, but a space after. That simply looks like a typo to me,
>>> and I'd be inclined to fix it were I editing this file. If a sed script
>>> can't handle more flexible white-space, perhaps use Python or perhaps
>>> Perl instead?
>>
>> The old code was similar, in that it had a space after the quote.
>>
>> We need the string to contain "mmc0 mmc1 usb0 dhcp" or perhaps "mmc0
>> mmc1". I chose to add a space at the start of each string, but
>> certainly we need a space somewhere, or we get "mmc0mmc1usb0dhcp".
>
> Oh, I see. I thought the space was part of the += syntax, not the value.
> Perhaps to make that more obvious, you could allow:
>
> # No space added to value
> var+=value
> var+= value
> var +=value
> var += value
>
> var+=value1 value2
> var+= value1 value2
> var +=value1 value2
> var += value1 value2
>
> var+="value1 value2"
> var+= "value1 value2"
> var +="value1 value2"
> var += "value1 value2"
>
> # One space included at start of addition to value
> var+=" value1 value2"
> var+= " value1 value2"
> var +=" value1 value2"
> var += " value1 value2"
>

I was deliberately trying to avoid using quotes, since then it is
really hard when you actually mean 'quote'.

For example at present you can put this in an env script at present,
but how would you do it if quotes are special?

echo "This is a test; or it might not be"

Regards,
Simon

[U-Boot] [PATCH v3 3/5] Allow U-Boot scripts to be placed in a .env file

[Wolfgang Denk]

Dear Simon,

In message <CAPnjgZ0+35Zd6_o=G0=G-YL_mR-GiTW+MeYqK7BSxU_nVkvv1w@mail.gmail.com> you wrote:
> 
> > I can see why you like such a "beautified" text format, but I don;t
> > think you are doing anybody a favour here.  Can we not rather use
> > _exactly_ the same text format as U-Boot uses with it's import /
> > export commands?
> 
> That would be nice, but how to we handle newlines? Some scripts are
> quite long. Do we need to put \ at the end of every line? That feels a
> bit painful to me.

Agreed.  But that's what "env export -t" creates anyway (and I cannot
think of a much better presentation of multiline variable content if
you don't want to run in ambiguities).

> Also how do we handle #define? Without it I don't think this feature
> is useful, since the existing build system often sticks CONFIG
> variables into the environment.

I don't understand this question here.  I agree that we should be able
to run the file through the preprocessor such that it can resolve such
macro definitions.  But this should be independent of the actual text
format, or am I missing something?

> I believe that is already possible - you should be able to take the
> output of 'env export -t' and put it in the .env file. I have not
> tried it though.

Hm... I really think we should agree on a common format her e- one
that is easy to work with on both sides.

> > Example (using notation as exported by U-Boot using "env export -t"):
> >
> >         foo=setenv xxx\
> >         one=1;setenv yyy\
> >         two=2;setenv zzz\
> >         three=3
> >
> >> +     # Print out all the variables
> >> +     for (var in vars) {
> >> +             print var "=" vars[var] "\\0";
> >> +     }
> >
> > I think it should not be difficult to find examples that would result
> > incorrect output.
> 
> I will take a look at this - here it is the \ at the end of line which
> needs to be handled.

Well, I can't see how you avoid such ambiguities in your proposed
format.  You need a clear termination for the value of a variable.
If it is spread across multiple lines, you have to find a way to mark
continuation lines.  The "accepted standard" way to do so is by using
a backslash at the end of the line.  It appears you want to use
indentation instead - this prevents users from using a free text
format, and quickly becomes messy.  And it is incompatible to
(current) U-Boot code.

> > I guess this needs more work - but then - why define a new format at
> > all?  Why not use what U-Boot uses itself?
> 
> See above, would be good to resolve this issue before going any further.

"Above" I cannot see any explanation why you define a new format
except for the fact that the backslash as marker for continuation
lines is "painful".   I'm open on discussing a new format for text
representation (which then also should be used by "env print" and "env
export -t" ?).  But I'd like to see a description of that format
first (and not just a few examples I can guess from).

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Only two things are infinite,  the universe and human stupidity,  and
I'm not sure about the former.                     -- Albert Einstein

[U-Boot] [PATCH v3 5/5] RFC: tegra: Convert to using environment files

[Stephen Warren]

On 10/28/2013 02:50 PM, Simon Glass wrote:
> Hi Stephen,
> 
> On Mon, Oct 28, 2013 at 2:41 PM, Stephen Warren <swarren@wwwdotorg.org> wrote:
>> On 10/28/2013 02:34 PM, Simon Glass wrote:
>>> Hi Stephen,
>>>
>>> On Mon, Oct 28, 2013 at 1:59 PM, Stephen Warren <swarren@wwwdotorg.org> wrote:
>>>> On 10/25/2013 11:01 PM, Simon Glass wrote:
>>>>> This seems more intuitive that the current #define way of doing things.
>>>>> The resulting code is shorter, avoids the quoting and line continuation
>>>>> pain, and also improves the clumsy way that stdio variables are created:

>>>>> diff --git a/board/nvidia/env/common.env b/board/nvidia/env/common.env
>>>>
>>>>> +bootcmd_mmc0=setenv devnum 0; run mmc_boot
>>>>> +bootcmd_mmc1=setenv devnum 1; run mmc_booxt
>>>>> +boot_targets+= mmc1 mmc0
>>>>
>>>> I still don't see why = needs no space before/after, but += needs no
>>>> space before, but a space after. That simply looks like a typo to me,
>>>> and I'd be inclined to fix it were I editing this file. If a sed script
>>>> can't handle more flexible white-space, perhaps use Python or perhaps
>>>> Perl instead?
>>>
>>> The old code was similar, in that it had a space after the quote.
>>>
>>> We need the string to contain "mmc0 mmc1 usb0 dhcp" or perhaps "mmc0
>>> mmc1". I chose to add a space at the start of each string, but
>>> certainly we need a space somewhere, or we get "mmc0mmc1usb0dhcp".
>>
>> Oh, I see. I thought the space was part of the += syntax, not the value.
>> Perhaps to make that more obvious, you could allow:
>>
>> # No space added to value
>> var+=value
...
>> var += "value1 value2"
>>
>> # One space included at start of addition to value
>> var+=" value1 value2"
>> var+= " value1 value2"
>> var +=" value1 value2"
>> var += " value1 value2"
> 
> I was deliberately trying to avoid using quotes, since then it is
> really hard when you actually mean 'quote'.

Hmm. On the other hand, quoting is standard syntax in any scripting
language.

> For example at present you can put this in an env script at present,
> but how would you do it if quotes are special?

Just escape it; " goes around the string and \" or "" within the string.
This seems pretty common...

[U-Boot] [PATCH v3 5/5] RFC: tegra: Convert to using environment files

[Simon Glass]

Hi Stephen,

On Mon, Oct 28, 2013 at 3:20 PM, Stephen Warren <swarren@wwwdotorg.org> wrote:
> On 10/28/2013 02:50 PM, Simon Glass wrote:
>> Hi Stephen,
>>
>> On Mon, Oct 28, 2013 at 2:41 PM, Stephen Warren <swarren@wwwdotorg.org> wrote:
>>> On 10/28/2013 02:34 PM, Simon Glass wrote:
>>>> Hi Stephen,
>>>>
>>>> On Mon, Oct 28, 2013 at 1:59 PM, Stephen Warren <swarren@wwwdotorg.org> wrote:
>>>>> On 10/25/2013 11:01 PM, Simon Glass wrote:
>>>>>> This seems more intuitive that the current #define way of doing things.
>>>>>> The resulting code is shorter, avoids the quoting and line continuation
>>>>>> pain, and also improves the clumsy way that stdio variables are created:
>
>>>>>> diff --git a/board/nvidia/env/common.env b/board/nvidia/env/common.env
>>>>>
>>>>>> +bootcmd_mmc0=setenv devnum 0; run mmc_boot
>>>>>> +bootcmd_mmc1=setenv devnum 1; run mmc_booxt
>>>>>> +boot_targets+= mmc1 mmc0
>>>>>
>>>>> I still don't see why = needs no space before/after, but += needs no
>>>>> space before, but a space after. That simply looks like a typo to me,
>>>>> and I'd be inclined to fix it were I editing this file. If a sed script
>>>>> can't handle more flexible white-space, perhaps use Python or perhaps
>>>>> Perl instead?
>>>>
>>>> The old code was similar, in that it had a space after the quote.
>>>>
>>>> We need the string to contain "mmc0 mmc1 usb0 dhcp" or perhaps "mmc0
>>>> mmc1". I chose to add a space at the start of each string, but
>>>> certainly we need a space somewhere, or we get "mmc0mmc1usb0dhcp".
>>>
>>> Oh, I see. I thought the space was part of the += syntax, not the value.
>>> Perhaps to make that more obvious, you could allow:
>>>
>>> # No space added to value
>>> var+=value
> ...
>>> var += "value1 value2"
>>>
>>> # One space included at start of addition to value
>>> var+=" value1 value2"
>>> var+= " value1 value2"
>>> var +=" value1 value2"
>>> var += " value1 value2"
>>
>> I was deliberately trying to avoid using quotes, since then it is
>> really hard when you actually mean 'quote'.
>
> Hmm. On the other hand, quoting is standard syntax in any scripting
> language.
>
>> For example at present you can put this in an env script at present,
>> but how would you do it if quotes are special?
>
> Just escape it; " goes around the string and \" or "" within the string.
> This seems pretty common...

Quoting quotes is currently needed for the header file. So how would
my feature actually improve things?

Between this and Wolfgang's \ at newline I am wondering if this
feature will actually improve anything? It we are really going to
insist on making the .env file like a C string then I'm not sure what
we gain.

Regards,
Simon

[U-Boot] [PATCH v3 3/5] Allow U-Boot scripts to be placed in a .env file

[Simon Glass]

Hi Wolfgang,

On Mon, Oct 28, 2013 at 3:16 PM, Wolfgang Denk <wd@denx.de> wrote:
> Dear Simon,
>
> In message <CAPnjgZ0+35Zd6_o=G0=
G-YL_mR-GiTW+MeYqK7BSxU_nVkvv1w@mail.gmail.com> you wrote:
>>
>> > I can see why you like such a "beautified" text format, but I don;t
>> > think you are doing anybody a favour here.  Can we not rather use
>> > _exactly_ the same text format as U-Boot uses with it's import /
>> > export commands?
>>
>> That would be nice, but how to we handle newlines? Some scripts are
>> quite long. Do we need to put \ at the end of every line? That feels a
>> bit painful to me.
>
> Agreed.  But that's what "env export -t" creates anyway (and I cannot
> think of a much better presentation of multiline variable content if
> you don't want to run in ambiguities).

I believe the ambiguities are pretty rare. And people tend to indent
multi-line scripts anyway, right?

>
>> Also how do we handle #define? Without it I don't think this feature
>> is useful, since the existing build system often sticks CONFIG
>> variables into the environment.
>
> I don't understand this question here.  I agree that we should be able
> to run the file through the preprocessor such that it can resolve such
> macro definitions.  But this should be independent of the actual text
> format, or am I missing something?

Yes we can, agreed. I was thinking you would not want the preprocessor
since 'env export -t' doesn't understand it.

>
>> I believe that is already possible - you should be able to take the
>> output of 'env export -t' and put it in the .env file. I have not
>> tried it though.
>
> Hm... I really think we should agree on a common format her e- one
> that is easy to work with on both sides.

Agreed.

>
>> > Example (using notation as exported by U-Boot using "env export -t"):
>> >
>> >         foo=setenv xxx\
>> >         one=1;setenv yyy\
>> >         two=2;setenv zzz\
>> >         three=3
>> >
>> >> +     # Print out all the variables
>> >> +     for (var in vars) {
>> >> +             print var "=" vars[var] "\\0";
>> >> +     }
>> >
>> > I think it should not be difficult to find examples that would result
>> > incorrect output.
>>
>> I will take a look at this - here it is the \ at the end of line which
>> needs to be handled.
>
> Well, I can't see how you avoid such ambiguities in your proposed
> format.  You need a clear termination for the value of a variable.
> If it is spread across multiple lines, you have to find a way to mark
> continuation lines.  The "accepted standard" way to do so is by using
> a backslash at the end of the line.  It appears you want to use
> indentation instead - this prevents users from using a free text
> format, and quickly becomes messy.  And it is incompatible to
> (current) U-Boot code.

Indentation is pretty normal in code, so I don't feel embarrassed about
asking for it. I think the primary problem with my feature is that it is
different from 'env export -t', and thus potentially introduces another
format. My argument against that interpretation is that I am in fact
replacing a C header file definition with a text file, so perhaps I'm not
really increasing the number of formats?

>
>> > I guess this needs more work - but then - why define a new format at
>> > all?  Why not use what U-Boot uses itself?
>>
>> See above, would be good to resolve this issue before going any further.
>
> "Above" I cannot see any explanation why you define a new format
> except for the fact that the backslash as marker for continuation
> lines is "painful".   I'm open on discussing a new format for text
> representation (which then also should be used by "env print" and "env
> export -t" ?).  But I'd like to see a description of that format
> first (and not just a few examples I can guess from).

[U-Boot] [PATCH v3 3/5] Allow U-Boot scripts to be placed in a .env file

[Wolfgang Denk]

Dear Simon,

In message <CAPnjgZ3tGwRK2ytm59EKUmAiPD5kAq39Mu9BityL0Y6d0H2C1w@mail.gmail.com> you wrote:
>
> > Agreed.  But that's what "env export -t" creates anyway (and I cannot
> > think of a much better presentation of multiline variable content if
> > you don't want to run in ambiguities).
> 
> I believe the ambiguities are pretty rare. And people tend to indent
> multi-line scripts anyway, right?

Do they? In the current include/config/*.h files, there is basically
zero structuring of the environment as nobody has been using
multi-line variables yet.   Yes, there is indentation in the header
files, but this is not visible at all in the environment variables.

> > I don't understand this question here.  I agree that we should be able
> > to run the file through the preprocessor such that it can resolve such
> > macro definitions.  But this should be independent of the actual text
> > format, or am I missing something?
> 
> Yes we can, agreed. I was thinking you would not want the preprocessor
> since 'env export -t' doesn't understand it.

Well, of course it deosn't, as it cannot invert the operation of the C
preprocessor...  But we can still use the prepro to create output that
is digestable to "env import -t", right?

> > Well, I can't see how you avoid such ambiguities in your proposed
> > format.  You need a clear termination for the value of a variable.
> > If it is spread across multiple lines, you have to find a way to mark
> > continuation lines.  The "accepted standard" way to do so is by using
> > a backslash at the end of the line.  It appears you want to use
> > indentation instead - this prevents users from using a free text
> > format, and quickly becomes messy.  And it is incompatible to
> > (current) U-Boot code.
> 
> Indentation is pretty normal in code, so I don't feel embarrassed about
> asking for it. I think the primary problem with my feature is that it is
> different from 'env export -t', and thus potentially introduces another
> format. My argument against that interpretation is that I am in fact
> replacing a C header file definition with a text file, so perhaps I'm not
> really increasing the number of formats?

Well, I'm afraid you have not yet formally defined the syntax of your
format, so I may just misunderstand what you mean.

> Well yes I could adjust 'env export -t' to use the same format - is that a
> good idea, or not? I can see down-sides. We can of course convert existing
> files brought in by 'env import -t' (by making the import flexible) but I
> worry that there might be external tools that users have which expect the
> format to be a certain way.

Are there any such tools?  Anybodyu who is using such please speak up
now and here! ;-)

> I agree creating a new format is not ideal - it's just that the existing C
> header format is so painful...

Yes, I agree on that, and I'm more than willing to get rid of it.  But
it has to be something that is actually working, and that is easy to
work with.

> --485b3970d160c06fb304e9d48797
> Content-Type: text/html; charset=ISO-8859-1
> Content-Transfer-Encoding: quoted-printable
> 
> <div dir=3D"ltr">Hi Wolfgang,<br><br>On Mon, Oct 28, 2013 at 3:16 PM, Wolfg=
> ang Denk &lt;<a href=3D"mailto:wd@denx.de">wd at denx.de</a>&gt; wrote:<br>&gt=
...


Argh... can you please turn this off?

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
I have never understood the female capacity to avoid a direct  answer
to any question.
	-- Spock, "This Side of Paradise", stardate 3417.3

[U-Boot] [PATCH v3 5/5] RFC: tegra: Convert to using environment files

[Stephen Warren]

On 10/28/2013 04:15 PM, Simon Glass wrote:
> Hi Stephen,
> 
> On Mon, Oct 28, 2013 at 3:20 PM, Stephen Warren <swarren@wwwdotorg.org> wrote:
>> On 10/28/2013 02:50 PM, Simon Glass wrote:
>>> Hi Stephen,
>>>
>>> On Mon, Oct 28, 2013 at 2:41 PM, Stephen Warren <swarren@wwwdotorg.org> wrote:
>>>> On 10/28/2013 02:34 PM, Simon Glass wrote:
>>>>> Hi Stephen,
>>>>>
>>>>> On Mon, Oct 28, 2013 at 1:59 PM, Stephen Warren <swarren@wwwdotorg.org> wrote:
>>>>>> On 10/25/2013 11:01 PM, Simon Glass wrote:
>>>>>>> This seems more intuitive that the current #define way of doing things.
>>>>>>> The resulting code is shorter, avoids the quoting and line continuation
>>>>>>> pain, and also improves the clumsy way that stdio variables are created:
>>
>>>>>>> diff --git a/board/nvidia/env/common.env b/board/nvidia/env/common.env
>>>>>>
>>>>>>> +bootcmd_mmc0=setenv devnum 0; run mmc_boot
>>>>>>> +bootcmd_mmc1=setenv devnum 1; run mmc_booxt
>>>>>>> +boot_targets+= mmc1 mmc0
>>>>>>
>>>>>> I still don't see why = needs no space before/after, but += needs no
>>>>>> space before, but a space after. That simply looks like a typo to me,
>>>>>> and I'd be inclined to fix it were I editing this file. If a sed script
>>>>>> can't handle more flexible white-space, perhaps use Python or perhaps
>>>>>> Perl instead?
>>>>>
>>>>> The old code was similar, in that it had a space after the quote.
>>>>>
>>>>> We need the string to contain "mmc0 mmc1 usb0 dhcp" or perhaps "mmc0
>>>>> mmc1". I chose to add a space at the start of each string, but
>>>>> certainly we need a space somewhere, or we get "mmc0mmc1usb0dhcp".
>>>>
>>>> Oh, I see. I thought the space was part of the += syntax, not the value.
>>>> Perhaps to make that more obvious, you could allow:
>>>>
>>>> # No space added to value
>>>> var+=value
>> ...
>>>> var += "value1 value2"
>>>>
>>>> # One space included at start of addition to value
>>>> var+=" value1 value2"
>>>> var+= " value1 value2"
>>>> var +=" value1 value2"
>>>> var += " value1 value2"
>>>
>>> I was deliberately trying to avoid using quotes, since then it is
>>> really hard when you actually mean 'quote'.
>>
>> Hmm. On the other hand, quoting is standard syntax in any scripting
>> language.
>>
>>> For example at present you can put this in an env script at present,
>>> but how would you do it if quotes are special?
>>
>> Just escape it; " goes around the string and \" or "" within the string.
>> This seems pretty common...
> 
> Quoting quotes is currently needed for the header file. So how would
> my feature actually improve things?
> 
> Between this and Wolfgang's \ at newline I am wondering if this
> feature will actually improve anything? It we are really going to
> insist on making the .env file like a C string then I'm not sure what
> we gain.

I guess I don't really see much wrong with the current header file
approach. The benefits this patch provide are the ability to use += and
omit the \0. Maintaining a more typical " and \ based quoting system
seem fine though. I guess you could avoid the need for \ by using either
shell-style here documents:

var += <<<ENDOFHERE
   data
   more data
ENDOFHERE

or Python-style triple-quotes:

var += """
   data
   more data"""

either of which would degenerate to allowing the following when
multi-line and leading/trailing spaces weren't required:

var += value

... although I like the idea of requiring quotes, thus making that:

var += "value"