[U-Boot] [PATCH] Add support for setting environment variable from RAM.

This is useful for allowing scripts to read environment variables from file, among other things.
This is a slightly modified version of what Alessandro submitted to the mailing list last July: http://www.mail-archive.com/u-boot-users@lists.sourceforge.net/msg07932.html
I changed the name from 'setenvram' to 'ramenv' to prevent breakage of scripts that use the abbreviation 'set' (which my handss have the habit of doing).
Signed-off-by: Eric Nelson eric.nelson@boundarydevices.com --- common/cmd_nvedit.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 46 insertions(+), 0 deletions(-)
diff --git a/common/cmd_nvedit.c b/common/cmd_nvedit.c index 1fcb4c9..bcb6d9f 100644 --- a/common/cmd_nvedit.c +++ b/common/cmd_nvedit.c @@ -494,6 +494,44 @@ int do_askenv ( cmd_tbl_t *cmdtp, int flag, int argc, char *argv[]) #endif
/************************************************************************ + * Set a new environment variable from RAM. + * Requires three arguments: the variable name, a memory address and a length. + * + * Deletes the environment variable if the length is zero. + */ +int do_ramenv(cmd_tbl_t *cmdtp, int flag, int argc, char *argv[]) +{ + unsigned long len, i; + char *addr; + + if (argc != 4) { + cmd_usage(cmdtp); + return 1; + } + addr = (char *)simple_strtol(argv[2], NULL, 16); + len = simple_strtol(argv[3], NULL, 16); + if (!addr || !len) { + cmd_usage(cmdtp); + return 1; + } + addr[len] = '\0'; + for (i = 0; i < len; i++) { + /* turn newlines into semicolon */ + if (addr[i] == '\n') + addr[i] = ';'; /* ignore dos-style newlines */ + if (addr[i] == '\r') + addr[i] = ' '; /* accept sh-comments and discard them */ + if (addr[i] == '#') { + while (addr[i] && addr[i] != '\n') + addr[i++] = ' '; + i--; + } + } + setenv(argv[1], addr); + return 0; +} + +/************************************************************************ * Look up variable from environment, * return address of storage for that variable, * or NULL if not found @@ -605,6 +643,14 @@ U_BOOT_CMD( " - delete environment variable 'name'\n" );
+U_BOOT_CMD( + ramenv, 4, 0, do_ramenv, + "ramenv - get environment variable from ram\n", + "name addr maxlen\n" + " - set environment variable 'name' from addr 'addr'\n" + " - delete environment variable if maxlen is 0\n" +); + #if defined(CONFIG_CMD_ASKENV)
U_BOOT_CMD(

Hi Eric,
On Tue, 2009-02-03 at 08:28 -0700, Eric Nelson (Boundary Devices) wrote:
/************************************************************************
- Set a new environment variable from RAM.
- Requires three arguments: the variable name, a memory address and a length.
- Deletes the environment variable if the length is zero.
- */
+int do_ramenv(cmd_tbl_t *cmdtp, int flag, int argc, char *argv[]) +{
- unsigned long len, i;
- char *addr;
- if (argc != 4) {
cmd_usage(cmdtp);
return 1;
- }
- addr = (char *)simple_strtol(argv[2], NULL, 16);
- len = simple_strtol(argv[3], NULL, 16);
- if (!addr || !len) {
cmd_usage(cmdtp);
return 1;
- }
- addr[len] = '\0';
- for (i = 0; i < len; i++) {
/* turn newlines into semicolon */
if (addr[i] == '\n')
addr[i] = ';'; /* ignore dos-style newlines */
if (addr[i] == '\r')
addr[i] = ' '; /* accept sh-comments and discard them */
if (addr[i] == '#') {
while (addr[i] && addr[i] != '\n')
addr[i++] = ' ';
i--;
}
- }
- setenv(argv[1], addr);
- return 0;
+}
+/************************************************************************
- Look up variable from environment,
- return address of storage for that variable,
- or NULL if not found
@@ -605,6 +643,14 @@ U_BOOT_CMD( " - delete environment variable 'name'\n" );
+U_BOOT_CMD(
- ramenv, 4, 0, do_ramenv,
- "ramenv - get environment variable from ram\n",
The "ramenv - " and "\n" are no longer used in the above line.
- "name addr maxlen\n"
- " - set environment variable 'name' from addr 'addr'\n"
- " - delete environment variable if maxlen is 0\n"
+);
#if defined(CONFIG_CMD_ASKENV)
U_BOOT_CMD(
In the email thread you mentioned above, Detlev mentions 2 alternatives to the "ramenv" command - loading a uImage script and running it via autoscr, or modifying autoscr to be able to raw files (non-uImages). Both of these methods seem cleaner and more flexible at a glance. Is there a specific reason using autoscr wouldn't work for your setup?
For example, what is the process to load multiple environment variables with the ramenv command? If I understand correctly, in order to load 10 environment variables you'd have to repeat the process of "load a file to RAM, run ramenv" 10 times? That seems much more difficult than loading 1 file with 10 environment variables and running autoscr once.
Best, Peter

Hello Peter,
Peter Tyser wrote:
Hi Eric,
On Tue, 2009-02-03 at 08:28 -0700, Eric Nelson (Boundary Devices) wrote:
/************************************************************************
- Set a new environment variable from RAM.
- Requires three arguments: the variable name, a memory address and a length.
- Deletes the environment variable if the length is zero.
- */
+int do_ramenv(cmd_tbl_t *cmdtp, int flag, int argc, char *argv[]) +{
- unsigned long len, i;
- char *addr;
- if (argc != 4) {
cmd_usage(cmdtp);
return 1;
- }
- addr = (char *)simple_strtol(argv[2], NULL, 16);
- len = simple_strtol(argv[3], NULL, 16);
- if (!addr || !len) {
cmd_usage(cmdtp);
return 1;
- }
- addr[len] = '\0';
- for (i = 0; i < len; i++) {
/* turn newlines into semicolon */
if (addr[i] == '\n')
addr[i] = ';'; /* ignore dos-style newlines */
if (addr[i] == '\r')
addr[i] = ' '; /* accept sh-comments and discard them */
if (addr[i] == '#') {
while (addr[i] && addr[i] != '\n')
addr[i++] = ' ';
i--;
}
- }
- setenv(argv[1], addr);
- return 0;
+}
+/************************************************************************
- Look up variable from environment,
- return address of storage for that variable,
- or NULL if not found
@@ -605,6 +643,14 @@ U_BOOT_CMD( " - delete environment variable 'name'\n" );
+U_BOOT_CMD(
- ramenv, 4, 0, do_ramenv,
- "ramenv - get environment variable from ram\n",
The "ramenv - " and "\n" are no longer used in the above line.
Oops. Can you tell I started by implementing this on an older source tree?
Re-reading it, the comment should probably also say "set environment variable from ram" instead of "get...".
If there's interest, I'll happily re-submit the patch.
- "name addr maxlen\n"
- " - set environment variable 'name' from addr 'addr'\n"
- " - delete environment variable if maxlen is 0\n"
+);
#if defined(CONFIG_CMD_ASKENV)
U_BOOT_CMD(
In the email thread you mentioned above, Detlev mentions 2 alternatives to the "ramenv" command - loading a uImage script and running it via autoscr, or modifying autoscr to be able to raw files (non-uImages). Both of these methods seem cleaner and more flexible at a glance. Is there a specific reason using autoscr wouldn't work for your setup?
The customer requesting this feature operates in a regulated environment with pretty strict rules about separation of code and data. Autoscr is kind of a big stick for what we're trying to achieve: (configuring an LCD with settings from a file on SD card).
For example, what is the process to load multiple environment variables with the ramenv command? If I understand correctly, in order to load 10 environment variables you'd have to repeat the process of "load a file to RAM, run ramenv" 10 times? That seems much more difficult than loading 1 file with 10 environment variables and running autoscr once.
That's certainly true, although once you have an environment variable you could use it for iteration...
Our particular need is just for a single environment variable, so the update works pretty well. I started by updating our 'lcdpanel' U-Boot command to read from file, but this is much more useful.
Best, Peter
Regards,
Eric

Dear Eric,
In message 4988936E.3070504@boundarydevices.com you wrote:
In the email thread you mentioned above, Detlev mentions 2 alternatives to the "ramenv" command - loading a uImage script and running it via autoscr, or modifying autoscr to be able to raw files (non-uImages). Both of these methods seem cleaner and more flexible at a glance. Is there a specific reason using autoscr wouldn't work for your setup?
The customer requesting this feature operates in a regulated environment with pretty strict rules about separation of code and data. Autoscr is kind of a big stick for what we're trying to achieve: (configuring an LCD with settings from a file on SD card).
Why is it a big stick? It has a couple of significant advantages over your implementation:
* It already exissts in mainline. * It can set several environment variables at once (and also perform any other commands you might need on your system). * It is based on text files which are easy to edit without need for new tools. * It will verify that your file has been transmitted correctly.
Our particular need is just for a single environment variable, so the update works pretty well. I started by updating our 'lcdpanel' U-Boot command to read from file, but this is much more useful.
I suggest you use autoscr instead.
I think I will reject your patch.
Best regards,
Wolfgang Denk

Wolfgang Denk wrote:
Dear Eric,
In message 4988936E.3070504@boundarydevices.com you wrote:
In the email thread you mentioned above, Detlev mentions 2 alternatives to the "ramenv" command - loading a uImage script and running it via autoscr, or modifying autoscr to be able to raw files (non-uImages). Both of these methods seem cleaner and more flexible at a glance. Is there a specific reason using autoscr wouldn't work for your setup?
The customer requesting this feature operates in a regulated environment with pretty strict rules about separation of code and data. Autoscr is kind of a big stick for what we're trying to achieve: (configuring an LCD with settings from a file on SD card).
Why is it a big stick? It has a couple of significant advantages over your implementation:
No offense intended ;) Perhaps I should have said it was a much more powerful feature.
- It already exissts in mainline.
- It can set several environment variables at once (and also perform any other commands you might need on your system).
- It is based on text files which are easy to edit without need for new tools.
- It will verify that your file has been transmitted correctly.
Don't get me wrong, I'm a big fan of autoscr.
As I mentioned, the customer requesting the feature has a regulatory requirement any time __code__ changes. They have interpreted boot loader scripts as being executable, so they have to write checks if the script changes, but not if "data" changes. They interpret a file containing a variable as data.
Of course they'll need a code release to get there, ...
Our particular need is just for a single environment variable, so the update works pretty well. I started by updating our 'lcdpanel' U-Boot command to read from file, but this is much more useful.
I suggest you use autoscr instead.
I think I will reject your patch.
That's reasonable. I don't mind carrying the patch around for this customer, and you're the one preventing U-Boot from becoming bloated over time.
I appreciate all of your efforts.
Regards,
Eric

Dear Eric Nelson,
In message 4988BBC0.4070004@boundarydevices.com you wrote:
As I mentioned, the customer requesting the feature has a regulatory requirement any time __code__ changes. They have interpreted boot loader scripts as being executable, so they have to write checks if the script changes, but not if "data" changes. They interpret a file containing a variable as data.
Hm... If we think this to a logical end that would mean that you have to disable the setenv command completely, as any variable can in fact contain a sequence of commands that might get run, thus environment data should be treated as code ;-)
I think I will reject your patch.
That's reasonable. I don't mind carrying the patch around for this customer, and you're the one preventing U-Boot from becoming bloated over time.
I appreciate all of your efforts.
Thanks.
Best regards,
Wolfgang Denk
participants (4)
-
Eric Nelson
-
Eric Nelson (Boundary Devices)
-
Peter Tyser
-
Wolfgang Denk