Cleanup and extend env vars

Commit 5e724ca did the same thing for env_common and env_embedded, but forgot fw_env.

Signed-off-by Beno?t Th?baudeau <[email protected]>

Cc Wolfgang Denk <[email protected]>

The ethprime env var was missing from env_common.

Signed-off-by Beno?t Th?baudeau <[email protected]>

Cc Wolfgang Denk <[email protected]>

Signed-off-by Beno?t Th?baudeau <[email protected]>

Cc Wolfgang Denk <[email protected]>

Acked-by Mike Frysinger <[email protected]>

README The following options need to be configured: · line 910

Quoted 7 lines · On Friday 10 August 2012 13:45:57 Beno?t Th?baudeau wrote:

README +0 -1

 
 		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

Quoted 8 lines

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.

Dear Wolfgang Denk,

Quoted 7 lines

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

Commit 155cb01 replaced the read-only property of the ver env var with an auto-restoring behavior. Update the README file accordingly.

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 <[email protected]>

Cc Wolfgang Denk <[email protected]>

---
 {u-boot-4d3c95f.orig => u-boot-4d3c95f}/README |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

README The following options need to be configured: · line 910

Quoted 2 lines

README +0 -1

-		This variable is readonly.

why don't we fix it to be read-only ?

The board revision can be a useful env var, like its serial number.

Quoted 1 line · 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

I have searched such a usage in the tree, but did not find any, so this should not break anything.

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.

Quoted 3 lines

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.

Quoted 7 lines

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

Quoted 8 lines

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.

Quoted 7 lines

Quoted 4 lines

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.

Quoted 21 lines

Quoted 12 lines

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,

Quoted 8 lines

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.

Quoted 4 lines

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.

Quoted 1 line

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.

Dear =?utf-8?Q?Beno=C3=AEt_Th=C3=A9baudeau?=,

Quoted 4 lines · In message <[email protected]> you…

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.

Dear Wolfgang Denk,

Quoted 6 lines

Quoted 3 lines

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.

Quoted 5 lines

Quoted 5 lines

Quoted 4 lines

I have searched such a usage in the tree, but did not find any, so
this should
not break anything.

Quoted 8 lines

Quoted 2 lines

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?

Quoted 6 lines

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.

Quoted 5 lines

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.

Quoted 6 lines

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

Quoted 8 lines

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.

Quoted 2 lines

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?

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.

Quoted 3 lines

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.

The board revision can be a useful env var, like its serial number.