[U-Boot] [PATCH 0/5] Cleanup and extend env vars
Hi all,
This series does some cleanup regarding env vars, and it adds a new env var for the board revision.
Best regards, Benoît
Commit 5e724ca did the same thing for env_common and env_embedded, but forgot fw_env.
Signed-off-by: Benoît Thébaudeau benoit.thebaudeau@advansee.com Cc: Wolfgang Denk wd@denx.de --- .../tools/env/fw_env.c | 11 +++++++++++ 1 file changed, 11 insertions(+)
diff --git u-boot-4d3c95f.orig/tools/env/fw_env.c u-boot-4d3c95f/tools/env/fw_env.c index e292d2b..1a2c227 100644 --- u-boot-4d3c95f.orig/tools/env/fw_env.c +++ u-boot-4d3c95f/tools/env/fw_env.c @@ -203,6 +203,17 @@ static char default_environment[] = { #if defined(CONFIG_PCI_BOOTDELAY) && (CONFIG_PCI_BOOTDELAY > 0) "pcidelay=" MK_STR (CONFIG_PCI_BOOTDELAY) "\0" #endif +#ifdef CONFIG_ENV_VARS_UBOOT_CONFIG + "arch=" CONFIG_SYS_ARCH "\0" + "cpu=" CONFIG_SYS_CPU "\0" + "board=" CONFIG_SYS_BOARD "\0" +#ifdef CONFIG_SYS_VENDOR + "vendor=" CONFIG_SYS_VENDOR "\0" +#endif +#ifdef CONFIG_SYS_SOC + "soc=" CONFIG_SYS_SOC "\0" +#endif +#endif #ifdef CONFIG_EXTRA_ENV_SETTINGS CONFIG_EXTRA_ENV_SETTINGS #endif
On Fri, Aug 10, 2012 at 07:45:15AM -0000, Beno?t Th?baudeau wrote:
Commit 5e724ca did the same thing for env_common and env_embedded, but forgot fw_env.
Signed-off-by: Beno?t Th?baudeau benoit.thebaudeau@advansee.com Cc: Wolfgang Denk wd@denx.de
Applied to u-boot/master, thanks!
The ethprime env var was missing from env_common.
Signed-off-by: Benoît Thébaudeau benoit.thebaudeau@advansee.com Cc: Wolfgang Denk wd@denx.de --- .../common/env_common.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git u-boot-4d3c95f.orig/common/env_common.c u-boot-4d3c95f/common/env_common.c index d9e990d..911a6af 100644 --- u-boot-4d3c95f.orig/common/env_common.c +++ u-boot-4d3c95f/common/env_common.c @@ -80,6 +80,9 @@ const uchar default_environment[] = { #ifdef CONFIG_ETH5ADDR "eth5addr=" MK_STR(CONFIG_ETH5ADDR) "\0" #endif +#ifdef CONFIG_ETHPRIME + "ethprime=" CONFIG_ETHPRIME "\0" +#endif #ifdef CONFIG_IPADDR "ipaddr=" MK_STR(CONFIG_IPADDR) "\0" #endif
On Fri, Aug 10, 2012 at 07:45:31AM -0000, Beno?t Th?baudeau wrote:
The ethprime env var was missing from env_common.
Signed-off-by: Beno?t Th?baudeau benoit.thebaudeau@advansee.com Cc: Wolfgang Denk wd@denx.de
Applied to u-boot/master, thanks!
Signed-off-by: Benoît Thébaudeau benoit.thebaudeau@advansee.com Cc: Wolfgang Denk wd@denx.de --- .../common/cmd_nvedit.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git u-boot-4d3c95f.orig/common/cmd_nvedit.c u-boot-4d3c95f/common/cmd_nvedit.c index fd05e72..0f320cc 100644 --- u-boot-4d3c95f.orig/common/cmd_nvedit.c +++ u-boot-4d3c95f/common/cmd_nvedit.c @@ -954,11 +954,15 @@ U_BOOT_CMD( #if defined(CONFIG_CMD_EDITENV) "env edit name - edit environment variable\n" #endif +#if defined(CONFIG_CMD_EXPORTENV) "env export [-t | -b | -c] [-s size] addr [var ...] - export environment\n" +#endif #if defined(CONFIG_CMD_GREPENV) "env grep string [...] - search environment\n" #endif +#if defined(CONFIG_CMD_IMPORTENV) "env import [-d] [-t | -b | -c] addr [size] - import environment\n" +#endif "env print [name ...] - print environment\n" #if defined(CONFIG_CMD_RUN) "env run var [...] - run commands in an environment variable\n"
Acked-by: Mike Frysinger vapier@gentoo.org -mike
On Fri, Aug 10, 2012 at 07:45:44AM -0000, Beno?t Th?baudeau wrote:
Signed-off-by: Beno?t Th?baudeau benoit.thebaudeau@advansee.com Cc: Wolfgang Denk wd@denx.de Acked-by: Mike Frysinger vapier@gentoo.org
Applied to u-boot/master, thanks!
Signed-off-by: Benoît Thébaudeau benoit.thebaudeau@advansee.com Cc: Wolfgang Denk wd@denx.de --- {u-boot-4d3c95f.orig => u-boot-4d3c95f}/README | 1 - 1 file changed, 1 deletion(-)
diff --git u-boot-4d3c95f.orig/README u-boot-4d3c95f/README index fb9d904..369ea9c 100644 --- u-boot-4d3c95f.orig/README +++ u-boot-4d3c95f/README @@ -907,7 +907,6 @@ The following options need to be configured: If this variable is defined, an environment variable named "ver" is created by U-Boot showing the U-Boot version as printed by the "version" command. - This variable is readonly.
- Real-Time Clock:
On Friday 10 August 2012 13:45:57 Benoît Thébaudeau wrote:
--- u-boot-4d3c95f.orig/README +++ u-boot-4d3c95f/README
If this variable is defined, an environment variable named "ver" is created by U-Boot showing the U-Boot version as printed by the "version" command.
This variable is readonly.
why don't we fix it to be read-only ? -mike
On Saturday 11 August 2012 19:48:24 Mike Frysinger wrote:
On Friday 10 August 2012 13:45:57 Benoît Thébaudeau wrote:
--- u-boot-4d3c95f.orig/README +++ u-boot-4d3c95f/README
If this variable is defined, an environment variable named "ver" is created by U-Boot showing the U-Boot version as printed by the "version" command.
This variable is readonly.why don't we fix it to be read-only ? -mike
I had thought about that, but there is an issue. main_loop() sets this env var, so if ver is made read-only and the env is stored somewhere (NVRAM, etc.), then after an update of U-Boot with a newer version (stored env untouched), ver will still indicate the older version. See commit 155cb01, which forgot to update the README file, which my patch does.
Best regards, Benoît
Dear =?utf-8?Q?Beno=C3=AEt_Th=C3=A9baudeau?=,
In message 2098801045.2303154.1344708456703.JavaMail.root@advansee.com you wrote:
I had thought about that, but there is an issue. main_loop() sets this env var, so if ver is made read-only and the env is stored somewhere (NVRAM, etc.), then after an update of U-Boot with a newer version (stored env untouched), ver will still indicate the older version. See commit 155cb01, which forgot to update the
No. main_loop() will always set this variable to the right value, no matter what might be stored in the environment. Only if you then change i later you may (temporarily) see a different value. But again only until you reset the board.
The correct fix for this would be the introduction of variable types, including flags like "read-only" (as for serial# or ethaddr) or "volatile" (i. e. not included in saveenv, as for filesize etc.)
I have been thinking about this for a long time already, I just didn't find time yet to implement it.
Best regards,
Wolfgang Denk
Dear Wolfgang Denk,
I had thought about that, but there is an issue. main_loop() sets this env var, so if ver is made read-only and the env is stored somewhere (NVRAM, etc.), then after an update of U-Boot with a newer version (stored env untouched), ver will still indicate the older version. See commit 155cb01, which forgot to update the
No. main_loop() will always set this variable to the right value, no matter what might be stored in the environment. Only if you then change i later you may (temporarily) see a different value. But again only until you reset the board.
Yes, I agree. The behavior that I described is what would occur _if_ commit 155cb01 were reverted in order to make ver truly read-only like Mike asked for. The current behavior is not too bad for now.
The correct fix for this would be the introduction of variable types, including flags like "read-only" (as for serial# or ethaddr) or "volatile" (i. e. not included in saveenv, as for filesize etc.)
I have been thinking about this for a long time already, I just didn't find time yet to implement it.
OK. Don't you think that the README file should be updated in some way in the meantime to reflect this, since ver is neither read-only nor "normal"?
Best regards, Benoît
On Sunday 12 August 2012 09:58:24 Benoît Thébaudeau wrote:
Dear Wolfgang Denk,
The correct fix for this would be the introduction of variable types, including flags like "read-only" (as for serial# or ethaddr) or "volatile" (i. e. not included in saveenv, as for filesize etc.)
I have been thinking about this for a long time already, I just didn't find time yet to implement it.
OK. Don't you think that the README file should be updated in some way in the meantime to reflect this, since ver is neither read-only nor "normal"?
sure, just mention any updates to it will be lost -mike
Commit 155cb01 replaced the read-only property of the ver env var with an auto-restoring behavior. Update the README file accordingly.
Signed-off-by: Benoît Thébaudeau benoit.thebaudeau@advansee.com Cc: Wolfgang Denk wd@denx.de --- {u-boot-4d3c95f.orig => u-boot-4d3c95f}/README | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git u-boot-4d3c95f.orig/README u-boot-4d3c95f/README index fb9d904..5f64588 100644 --- u-boot-4d3c95f.orig/README +++ u-boot-4d3c95f/README @@ -907,7 +907,8 @@ The following options need to be configured: If this variable is defined, an environment variable named "ver" is created by U-Boot showing the U-Boot version as printed by the "version" command. - This variable is readonly. + Any change to this variable will be reverted at the + next reset.
- Real-Time Clock:
On Monday 13 August 2012 09:01:14 Benoît Thébaudeau wrote:
Commit 155cb01 replaced the read-only property of the ver env var with an auto-restoring behavior. Update the README file accordingly.
Acked-by: Mike Frysinger vapier@gentoo.org -mike
Dear Benoît Thébaudeau,
In message 791653575.2357512.1344862874565.JavaMail.root@advansee.com you wrote:
Commit 155cb01 replaced the read-only property of the ver env var with an auto-restoring behavior. Update the README file accordingly.
Signed-off-by: Benoît Thébaudeau benoit.thebaudeau@advansee.com Cc: Wolfgang Denk wd@denx.de
{u-boot-4d3c95f.orig => u-boot-4d3c95f}/README | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
Applied, thanks.
Best regards,
Wolfgang Denk
Dear Mike Frysinger,
In message 201208111348.25632.vapier@gentoo.org you wrote:
This variable is readonly.why don't we fix it to be read-only ?
Actually it is "auto-restoring". Any value written to it will be lost at the next reset, when the variable will be siigned it's original value again.
Best regards,
Wolfgang Denk
The board revision can be a useful env var, like its serial number.
Signed-off-by: Benoît Thébaudeau benoit.thebaudeau@advansee.com Cc: Wolfgang Denk wd@denx.de --- {u-boot-4d3c95f.orig => u-boot-4d3c95f}/README | 21 ++++++++++---------- .../common/cmd_nvedit.c | 5 +++-- .../tools/env/fw_env.c | 5 +++-- 3 files changed, 17 insertions(+), 14 deletions(-)
diff --git u-boot-4d3c95f.orig/README u-boot-4d3c95f/README index 369ea9c..2ea48cf 100644 --- u-boot-4d3c95f.orig/README +++ u-boot-4d3c95f/README @@ -2073,13 +2073,13 @@ The following options need to be configured: - Vendor Parameter Protection:
U-Boot considers the values of the environment - variables "serial#" (Board Serial Number) and - "ethaddr" (Ethernet Address) to be parameters that - are set once by the board vendor / manufacturer, and - protects these variables from casual modification by - the user. Once set, these variables are read-only, - and write or delete attempts are rejected. You can - change this behaviour: + variables "serial#" (Board Serial Number), "rev" + (Board Revision) and "ethaddr" (Ethernet Address) + to be parameters that are set once by the board + vendor / manufacturer, and protects these variables + from casual modification by the user. Once set, + these variables are read-only, and write or delete + attempts are rejected. You can change this behaviour:
If CONFIG_ENV_OVERWRITE is #defined in your config file, the write protection for vendor parameters is @@ -2090,8 +2090,8 @@ The following options need to be configured: _and_ CONFIG_OVERWRITE_ETHADDR_ONCE, a default Ethernet address is installed in the environment, which can be changed exactly ONCE by the user. [The - serial# is unaffected by this, i. e. it remains - read-only.] + serial# and rev are unaffected by this, i. e. they + remain read-only.]
- Protected RAM: CONFIG_PRAM @@ -3968,10 +3968,11 @@ depending the information provided by your boot server: serverip - see above
-There are two special Environment Variables: +There are three special Environment Variables:
serial# - contains hardware identification information such as type string and/or serial number + rev - hardware revision ethaddr - Ethernet address
These variables can be set only once (usually during manufacturing of diff --git u-boot-4d3c95f.orig/common/cmd_nvedit.c u-boot-4d3c95f/common/cmd_nvedit.c index 0f320cc..d16aeb6 100644 --- u-boot-4d3c95f.orig/common/cmd_nvedit.c +++ u-boot-4d3c95f/common/cmd_nvedit.c @@ -255,12 +255,13 @@ int _do_env_set(int flag, int argc, char * const argv[]) }
/* - * Some variables like "ethaddr" and "serial#" can be set only - * once and cannot be deleted; also, "ver" is readonly. + * Some variables like "ethaddr", "serial#" and "rev" can be set only + * once and cannot be deleted. */ if (ep) { /* variable exists */ #ifndef CONFIG_ENV_OVERWRITE if (strcmp(name, "serial#") == 0 || + strcmp(name, "rev") == 0 || (strcmp(name, "ethaddr") == 0 #if defined(CONFIG_OVERWRITE_ETHADDR_ONCE) && defined(CONFIG_ETHADDR) && strcmp(ep->data, MK_STR(CONFIG_ETHADDR)) != 0 diff --git u-boot-4d3c95f.orig/tools/env/fw_env.c u-boot-4d3c95f/tools/env/fw_env.c index 1a2c227..b5aa3aa 100644 --- u-boot-4d3c95f.orig/tools/env/fw_env.c +++ u-boot-4d3c95f/tools/env/fw_env.c @@ -405,10 +405,11 @@ int fw_env_write(char *name, char *value) if (oldval) { #ifndef CONFIG_ENV_OVERWRITE /* - * Ethernet Address and serial# can be set only once + * Ethernet Address, serial# and rev can be set only once */ if ( (strcmp(name, "serial#") == 0) || + (strcmp (name, "rev") == 0) || ((strcmp(name, "ethaddr") == 0) #if defined(CONFIG_OVERWRITE_ETHADDR_ONCE) && defined(CONFIG_ETHADDR) && (strcmp(oldval, MK_STR(CONFIG_ETHADDR)) != 0) @@ -474,7 +475,7 @@ int fw_env_write(char *name, char *value) * Deletes or sets environment variables. Returns -1 and sets errno error codes: * 0 - OK * EINVAL - need at least 1 argument - * EROFS - certain variables ("ethaddr", "serial#") cannot be + * EROFS - certain variables ("ethaddr", "serial#", "rev") cannot be * modified or deleted * */
On Friday 10 August 2012 13:46:08 Benoît Thébaudeau wrote:
The board revision can be a useful env var, like its serial number.
unless i missed something, there is no standard "rev" variable today, which means this change can easily break anyone who happens to already be using a variable named "rev".
i also don't see value here in hardcoding another variable that: - no one is setting - is way too generic (rev of *what* ? cpu ? board ? u-boot ? something else ?) - adds nothing on top of the existing "serial#"
so NAK from me -mike
Hi Mike,
On Saturday 11 August 2012 19:48:33 Mike Frysinger wrote:
On Friday 10 August 2012 13:46:08 Benoît Thébaudeau wrote:
The board revision can be a useful env var, like its serial number.
unless i missed something, there is no standard "rev" variable today, which means this change can easily break anyone who happens to already be using a variable named "rev".
I have searched such a usage in the tree, but did not find any, so this should not break anything.
i also don't see value here in hardcoding another variable that:
- no one is setting
Well, I am setting it for my local boards that are not yet ready for upstream, so I thought that it could also be useful for others.
- is way too generic (rev of *what* ? cpu ? board ? u-boot ?
something else ?)
It could be renamed to hwrev, board_rev or whatever you like. This is not really an issue. Its purpose is the board hardware revision. The CPU revision can often be read from the CPU and is printed upon startup. U-Boot's revision already has the ver env var and the version command. On the contrary, the board revision can not always be determined by analyzing the hardware (OTP, fuses, EEPROM, GPIOs, etc.), so it can be useful to have an official env var to store it in the backed up env, exactly like for the serial# env var that can not always be stored in some dedicated hardware location.
- adds nothing on top of the existing "serial#"
This is all the contrary. If you think that a rev variable would be useless, then you can also remove the serial# variable. And if you mean that a rev variable would duplicate the serial# information, this is wrong: The serial# variable gives a UID for a board, while the rev variable would give the hardware revision of that board. These are completely different things, and the board revision can not always be easily derived from its serial number.
so NAK from me -mike
Best regards, Benoît
Dear =?utf-8?Q?Beno=C3=AEt_Th=C3=A9baudeau?=,
In message 1666183421.2304101.1344712292461.JavaMail.root@advansee.com you wrote:
I have searched such a usage in the tree, but did not find any, so this should not break anything.
You cannot expect to see the real, production environments in the mainline source tree.
It could be renamed to hwrev, board_rev or whatever you like. This is not really an issue. Its purpose is the board hardware revision. The CPU revision can often be read from the CPU and is printed upon startup. U-Boot's revision already has the ver env var and the version command. On the contrary, the board revision can not always be determined by analyzing the hardware (OTP, fuses, EEPROM, GPIOs, etc.), so it can be useful to have an official env var to store it in the backed up env, exactly like for the serial# env var that can not always be stored in some dedicated hardware location.
As mentioned before, I don't see need for such a thing in general. Any such use is highly board specific, and vendors use different ways to address this.
I don't intend to apply this patch, sorry.
Best regards,
Wolfgang Denk
Dear Wolfgang Denk,
I have searched such a usage in the tree, but did not find any, so this should not break anything.
You cannot expect to see the real, production environments in the mainline source tree.
Right, but the same applied to serial# and ethaddr when they were added, except if U-Boot deployment was not large enough at that time to worry you.
It could be renamed to hwrev, board_rev or whatever you like. This is not really an issue. Its purpose is the board hardware revision. The CPU revision can often be read from the CPU and is printed upon startup. U-Boot's revision already has the ver env var and the version command. On the contrary, the board revision can not always be determined by analyzing the hardware (OTP, fuses, EEPROM, GPIOs, etc.), so it can be useful to have an official env var to store it in the backed up env, exactly like for the serial# env var that can not always be stored in some dedicated hardware location.
As mentioned before, I don't see need for such a thing in general. Any such use is highly board specific, and vendors use different ways to address this.
The same applies to serial# again.
I don't intend to apply this patch, sorry.
OK.
Best regards, Benoît
Dear Wolfgang Denk,
I have searched such a usage in the tree, but did not find any, so this should not break anything.
You cannot expect to see the real, production environments in the mainline source tree.
Right, but the same applied to serial# and ethaddr when they were added, except if U-Boot deployment was not large enough at that time to worry you.
It could be renamed to hwrev, board_rev or whatever you like. This is not really an issue. Its purpose is the board hardware revision. The CPU revision can often be read from the CPU and is printed upon startup. U-Boot's revision already has the ver env var and the version command. On the contrary, the board revision can not always be determined by analyzing the hardware (OTP, fuses, EEPROM, GPIOs, etc.), so it can be useful to have an official env var to store it in the backed up env, exactly like for the serial# env var that can not always be stored in some dedicated hardware location.
As mentioned before, I don't see need for such a thing in general. Any such use is highly board specific, and vendors use different ways to address this.
The same applies to serial# again.
I don't intend to apply this patch, sorry.
OK.
Anyway, when you will have implemented read-only and volatile flags for env vars, this patch will no longer be needed. But with the current code, there is no way board-specific code can create a board revision env var and make it read-only, except with this patch.
Best regards, Benoît
Dear =?utf-8?Q?Beno=C3=AEt_Th=C3=A9baudeau?=,
In message 1323086777.2324156.1344780597072.JavaMail.root@advansee.com you wrote:
Anyway, when you will have implemented read-only and volatile flags for env vars, this patch will no longer be needed. But with the current code, there is no way board-specific code can create a board revision env var and make it read-only, except with this patch.
Correct, so far we don't support custom read-only variables. But if we add these, then in a generic way, and definitely not for a single, specific variable. This is a lesson we learned form experience.
Best regards,
Wolfgang Denk
On 08/12/2012 11:06 PM, Wolfgang Denk wrote:
Dear =?utf-8?Q?Beno=C3=AEt_Th=C3=A9baudeau?=,
In message 1323086777.2324156.1344780597072.JavaMail.root@advansee.com you wrote:
Anyway, when you will have implemented read-only and volatile flags for env vars, this patch will no longer be needed. But with the current code, there is no way board-specific code can create a board revision env var and make it read-only, except with this patch.
Correct, so far we don't support custom read-only variables. But if we add these, then in a generic way, and definitely not for a single, specific variable. This is a lesson we learned form experience.
I didn't follow this thread, but would it be an option to create a command for these kind of things, instead of read-only variables? e.g. some like
if get revision out; then; echo $out; else; echo revision not supported fi;
Just a though, regards,
Jeroen
On Sunday 12 August 2012 10:02:48 Benoît Thébaudeau wrote:
Dear Wolfgang Denk,
I have searched such a usage in the tree, but did not find any, so this should not break anything.
You cannot expect to see the real, production environments in the mainline source tree.
Right, but the same applied to serial# and ethaddr when they were added, except if U-Boot deployment was not large enough at that time to worry you.
which makes all the difference in the world. those two variables were set up this way before 2002 (at least, that's according to the git history, and that's when the source code was first imported, so i can't easily check just how far back it goes). as the project grows up, policies evolve. -mike
Dear Mike Frysinger,
I have searched such a usage in the tree, but did not find any, so this should not break anything.
You cannot expect to see the real, production environments in the mainline source tree.
Right, but the same applied to serial# and ethaddr when they were added, except if U-Boot deployment was not large enough at that time to worry you.
which makes all the difference in the world. those two variables were set up this way before 2002 (at least, that's according to the git history, and that's when the source code was first imported, so i can't easily check just how far back it goes). as the project grows up, policies evolve. -mike
OK. Actually, the only reason for which I need this patch is to make a variable read-only, and the only reason for which you reject it is because you fear that it breaks something.
So we could add a config like CONFIG_BOARD_REV_RO_VARIABLE to enable the code in my patch. But I think you won't like that either because you will find it too specific.
What about adding a config like CONFIG_READONLY_VARS that would be an array initializer containing the names of the board-specific variables to make read-only? _do_env_set() and fw_env_write() would use it besides the hard-coded serial# and the like. That would give something like: #define CONFIG_READONLY_VARS {"my_ro_var1", "my_ro_var2", "board_rev"} That would be a very simple solution to make everyone happy before Wolfgang implements a more sophisticated solution with read-only and volatile flags. What do you think?
Best regards, Benoît
Dear =?utf-8?Q?Beno=C3=AEt_Th=C3=A9baudeau?=,
In message 383502175.2331918.1344791463023.JavaMail.root@advansee.com you wrote:
OK. Actually, the only reason for which I need this patch is to make a variable read-only, and the only reason for which you reject it is because you fear that it breaks something.
So we could add a config like CONFIG_BOARD_REV_RO_VARIABLE to enable the code in my patch. But I think you won't like that either because you will find it too specific.
No, this may solve your problem, but will never scale for any real life use.
What about adding a config like CONFIG_READONLY_VARS that would be an array initializer containing the names of the board-specific variables to make read-only? _do_env_set() and fw_env_write() would use it besides the hard-coded serial# and the like. That would give something like: #define CONFIG_READONLY_VARS {"my_ro_var1", "my_ro_var2", "board_rev"} That would be a very simple solution to make everyone happy before Wolfgang implements a more sophisticated solution with read-only and volatile flags. What do you think?
Please feel free to add this to your local code.
Best regards,
Wolfgang Denk
Dear Wolfgang Denk,
OK. Actually, the only reason for which I need this patch is to make a variable read-only, and the only reason for which you reject it is because you fear that it breaks something.
So we could add a config like CONFIG_BOARD_REV_RO_VARIABLE to enable the code in my patch. But I think you won't like that either because you will find it too specific.
No, this may solve your problem, but will never scale for any real life use.
OK.
What about adding a config like CONFIG_READONLY_VARS that would be an array initializer containing the names of the board-specific variables to make read-only? _do_env_set() and fw_env_write() would use it besides the hard-coded serial# and the like. That would give something like: #define CONFIG_READONLY_VARS {"my_ro_var1", "my_ro_var2", "board_rev"} That would be a very simple solution to make everyone happy before Wolfgang implements a more sophisticated solution with read-only and volatile flags. What do you think?
Please feel free to add this to your local code.
By "local", do you mean that this new suggestion would still not be generic enough for you to be interested in it for a patch?
Best regards, Benoît
Dear =?utf-8?Q?Beno=C3=AEt_Th=C3=A9baudeau?=,
In message 1012612599.2335651.1344807326593.JavaMail.root@advansee.com you wrote:
Please feel free to add this to your local code.
By "local", do you mean that this new suggestion would still not be generic enough for you to be interested in it for a patch?
Correct.
Best regards,
Wolfgang Denk
On Sunday 12 August 2012 13:11:03 Benoît Thébaudeau wrote:
OK. Actually, the only reason for which I need this patch is to make a variable read-only, and the only reason for which you reject it is because you fear that it breaks something.
and because it bloats the codebase for 0 gain for the vast majority of boards (every one currently in the tree). -mike
Dear =?utf-8?Q?Beno=C3=AEt_Th=C3=A9baudeau?=,
In message 1642597694.2324115.1344780168514.JavaMail.root@advansee.com you wrote:
You cannot expect to see the real, production environments in the mainline source tree.
Right, but the same applied to serial# and ethaddr when they were added, except if U-Boot deployment was not large enough at that time to worry you.
Did you check when these were added? This was long before the project even was called U-Boot... IIRC they were there right from the beginning when I implemented the first version of the environment...
Best regards,
Wolfgang Denk
Dear =?utf-8?Q?Beno=C3=AEt_Th=C3=A9baudeau?=,
In message 2028294275.2280404.1344620768178.JavaMail.root@advansee.com you wrote:
The board revision can be a useful env var, like its serial number.
It can be useful, but it appears not many boards needed it so far, so I suggest we leave as is; boards that want such a thing can implement it easily in their board specific code.
Best regards,
Wolfgang Denk
participants (5)
-
Benoît Thébaudeau -
Jeroen Hofstee -
Mike Frysinger -
Tom Rini -
Wolfgang Denk