[U-Boot] [PATCH 0/5] Cleanup and extend env vars

[U-Boot] [PATCH 0/5] Cleanup and extend env vars

[Benoît Thébaudeau]

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

[U-Boot] [PATCH 1/5] fw_env: Add env vars describing U-Boot target board

[Benoît Thébaudeau]

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

[U-Boot] [U-Boot, 1/5] fw_env: Add env vars describing U-Boot target board

[Tom Rini]

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!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20120918/d3894d65/attachment.pgp>

[U-Boot] [PATCH 2/5] env_common: Add missing ethprime

[Benoît Thébaudeau]

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

[U-Boot] [U-Boot,2/5] env_common: Add missing ethprime

[Tom Rini]

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!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20120918/e3b1d6e0/attachment.pgp>

[U-Boot] [PATCH 3/5] env import/export: Remove from help if disabled

[Benoît Thébaudeau]

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"

[U-Boot] [PATCH 3/5] env import/export: Remove from help if disabled

[Mike Frysinger]

Acked-by: Mike Frysinger <vapier@gentoo.org>
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20120811/98103ab1/attachment.pgp>

[U-Boot] [U-Boot, 3/5] env import/export: Remove from help if disabled

[Tom Rini]

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!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20120918/b75feb97/attachment.pgp>

[U-Boot] [PATCH 4/5] README: The ver env var is not read-only

[Benoît Thébaudeau]

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:
 

[U-Boot] [PATCH 4/5] README: The ver env var is not read-only

[Mike Frysinger]

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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20120811/01ae7021/attachment.pgp>

[U-Boot] [PATCH 4/5] README: The ver env var is not read-only

[Benoît Thébaudeau]

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

[U-Boot] [PATCH 4/5] README: The ver env var is not read-only

[Wolfgang Denk]

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

-- 
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
A Vulcan can  no  sooner  be  disloyal  than  he  can  exist  without
breathing.
	-- Kirk, "The Menagerie", stardate 3012.4

[U-Boot] [PATCH 4/5] README: The ver env var is not read-only

[Benoît Thébaudeau]

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

[U-Boot] [PATCH 4/5] README: The ver env var is not read-only

[Mike Frysinger]

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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20120812/bfc3ff48/attachment.pgp>

[U-Boot] [PATCH] README: Update ver env var description

[Benoît Thébaudeau]

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:
 

[U-Boot] [PATCH] README: Update ver env var description

[Mike Frysinger]

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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20120817/68d866ab/attachment.pgp>

[U-Boot] [PATCH] README: Update ver env var description

[Wolfgang Denk]

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

-- 
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
"We shall reach greater and greater platitudes of achievement."
- Richard J. Daley

[U-Boot] [PATCH 4/5] README: The ver env var is not read-only

[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

-- 
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
"How is this place run - is it an anarchy?"
"No, I wouldn't say so; it is not that well organised..."

[U-Boot] [PATCH 5/5] Add env var giving the board revision

[Benoît Thébaudeau]

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
  *
  */

[U-Boot] [PATCH 5/5] Add env var giving the board revision

[Mike Frysinger]

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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20120811/60097306/attachment.pgp>

[U-Boot] [PATCH 5/5] Add env var giving the board revision

[Benoît Thébaudeau]

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

[U-Boot] [PATCH 5/5] Add env var giving the board revision

[Wolfgang Denk]

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

-- 
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
PoB = "Prisoner of Bill" -- those held captive, unwillingly or other-
wise, by the contemptible Microsoft monopoly.
         -- Tom Christiansen in <6abo45$3lc$2@csnews.cs.colorado.edu>

[U-Boot] [PATCH 5/5] Add env var giving the board revision

[Benoît Thébaudeau]

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

[U-Boot] [PATCH 5/5] Add env var giving the board revision

[Benoît Thébaudeau]

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

[U-Boot] [PATCH 5/5] Add env var giving the board revision

[Wolfgang Denk]

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

-- 
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
The number you have dialed is imaginary. Please divide by 0  and  try
again.

[U-Boot] [PATCH 5/5] Add env var giving the board revision

[Jeroen Hofstee]

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

[U-Boot] [PATCH 5/5] Add env var giving the board revision

[Mike Frysinger]

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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20120812/0bcab542/attachment.pgp>

[U-Boot] [PATCH 5/5] Add env var giving the board revision

[Benoît Thébaudeau]

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

[U-Boot] [PATCH 5/5] Add env var giving the board revision

[Wolfgang Denk]

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

-- 
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
backups: always in season, never out of style.

[U-Boot] [PATCH 5/5] Add env var giving the board revision

[Benoît Thébaudeau]

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

[U-Boot] [PATCH 5/5] Add env var giving the board revision

[Wolfgang Denk]

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

-- 
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
G's Third Law:             In spite of all evidence  to  the  contra-
ry,  the  entire  universe  is composed of only two basic substances:
magic and bullshit.
H's Dictum:                There is no magic ...

[U-Boot] [PATCH 5/5] Add env var giving the board revision

[Mike Frysinger]

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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20120812/b5a58243/attachment.pgp>

[U-Boot] [PATCH 5/5] Add env var giving the board revision

[Wolfgang Denk]

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

-- 
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
Q:  Why do mountain climbers rope themselves together?
A:  To prevent the sensible ones from going home.

[U-Boot] [PATCH 5/5] Add env var giving the board revision

[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

-- 
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
"We have the right to survive!"
"Not be killing others."
	-- Deela and Kirk, "Wink of An Eye", stardate 5710.5