fw_env: use vars from the board config

it is quite odd that fw_printenv/fw_setenv does not use the settings from include/configs but instead redefines things.

This patch uses the variables from the config file The edit in fw_env.c is only needed to resolve a name clash

Signed-off-by Frans Meulenbroeks <[email protected]>

---

Note: this is more intended to get some feedback. (also to see if I am on the right track)
I did test the changes locally.

(and yes, I know there are some more things that could be cleaned up).

---
 tools/env/fw_env.c |   20 ++++++++++----------
 tools/env/fw_env.h |   36 ++++++++++++++++--------------------
 2 files changed, 26 insertions(+), 30 deletions(-)

tools/env/fw_env.c +0 -0

index 996682e..6597fbf 100644

tools/env/fw_env.c +3 -3

 #define ENV_SIZE      getenvsize()
 

@@ -216,7 +216,7 @@ static int get_config (char *);
 #endif
 static inline ulong getenvsize (void)
 {
-	ulong rc = CONFIG_ENV_SIZE - sizeof (long);
+	ulong rc = CFG_ENV_SIZE - sizeof (long);
 
 	if (HaveRedundEnv)
 		rc -= sizeof (char);

@@ -425,7 +425,7 @@ int fw_env_write(char *name, char *value)
 		++env;
 	/*
 	 * Overflow when:
-	 * "name" + "=" + "val" +"\0\0"  > CONFIG_ENV_SIZE - (env-environment)
+	 * "name" + "=" + "val" +"\0\0"  > CFG_ENV_SIZE - (env-environment)
 	 */
 	len = strlen (name) + 2;
 	/* add '=' for first arg, ' ' for all others */

@@ -941,7 +941,7 @@ static int flash_write (int fd_current, int fd_target,
 int dev_target) DEVOFFSET (dev_target), DEVNAME (dev_target));
 #endif
 	rc = flash_write_buf (dev_target, fd_target, environment.image,
-			      CONFIG_ENV_SIZE, DEVOFFSET (dev_target),
+			      CFG_ENV_SIZE, DEVOFFSET (dev_target),
 			      DEVTYPE(dev_target));
 	if (rc < 0)
 		return rc;

tools/env/fw_env.c +4 -3

+	rc = flash_read_buf (dev_current, fd, environment.image, CFG_ENV_SIZE,
 			     DEVOFFSET (dev_current), mtdinfo.type);
 
-	return (rc != CONFIG_ENV_SIZE) ? -1 : 0;
+	return (rc != CFG_ENV_SIZE) ? -1 : 0;
 }
 
 static int flash_io (int mode)

@@ -1080,11 +1080,11 @@ int fw_env_open(void)
 	if (parse_config ())		/* should fill envdevices */
 		return -1;
 
-	addr0 = calloc (1, CONFIG_ENV_SIZE);
+	addr0 = calloc (1, CFG_ENV_SIZE);
 	if (addr0 == NULL) {
 		fprintf (stderr,
 			"Not enough memory for environment (%ld bytes)\n",
-			CONFIG_ENV_SIZE);
+			CFG_ENV_SIZE);
 		return -1;
 	}
 

tools/env/fw_env.c +1 -1

-				CONFIG_ENV_SIZE);
+				CFG_ENV_SIZE);
 			return -1;
 		}
 		redundant = addr1;

tools/env/fw_env.h +2 -8

index 2dcb373..8fcc5bd 100644
@@ -24,30 +24,26 @@
 /*
  * To build the utility with the run-time configuration
  * uncomment the next line.
- * See included "fw_env.config" sample file
+ * See included "fw_env.config" sample file (TRAB board)
  * for notes on configuration.
  */
-#define CONFIG_FILE     "/etc/fw_env.config"
+//#define CONFIG_FILE     "/etc/fw_env.config"
 
-#define HAVE_REDUND /* For systems with 2 env sectors */
-#define DEVICE1_NAME      "/dev/mtd1"
-#define DEVICE2_NAME      "/dev/mtd2"
-#define DEVICE1_OFFSET    0x0000
-#define ENV1_SIZE         0x4000
-#define DEVICE1_ESIZE     0x4000

tools/env/fw_env.h +13 -1

-	"bootm"
+#define DEVICE1_NAME      "/dev/mtd0"
+#define DEVICE2_NAME      "/dev/mtd0"
+#define DEVICE1_OFFSET    CONFIG_ENV_ADDR_REDUND
+#define ENV1_SIZE         CONFIG_ENV_SIZE
+#define DEVICE1_ESIZE     CONFIG_ENV_SECT_SIZE
+#define DEVICE1_ENVSECTORS     1
+#ifdef CONFIG_ENV_ADDR_REDUND
+#define HAVE_REDUND /* For systems with 2 env sectors */
+#define DEVICE2_OFFSET    CONFIG_ENV_ADDR_REDUND
+#define ENV2_SIZE         CONFIG_ENV_SIZE_REDUND
+#define DEVICE2_ESIZE     CONFIG_ENV_SECT_SIZE
+#define DEVICE2_ENVSECTORS     1
+#endif
 
 extern int   fw_printenv(int argc, char *argv[]);
 extern char *fw_getenv  (char *name);

2012/1/2 Marek Vasut <[email protected]>

Quoted 31 lines

Quoted 31 lines

it is quite odd that fw_printenv/fw_setenv does not use the settings from include/configs but instead redefines things.

This patch uses the variables from the config file The edit in fw_env.c is only needed to resolve a name clash

Signed-off-by Frans Meulenbroeks <[email protected]>

---

Note: this is more intended to get some feedback. (also to see if I am on the right track)
I did test the changes locally.

(and yes, I know there are some more things that could be cleaned up).
---

 tools/env/fw_env.c |   20 ++++++++++----------
 tools/env/fw_env.h |   36 ++++++++++++++++--------------------
 2 files changed, 26 insertions(+), 30 deletions(-)

tools/env/fw_env.c +0 -0

index 996682e..6597fbf 100644
@@ -79,7 +79,7 @@ static int dev_current;
 
 #define ENVSECTORS(i) envdevices[(i)].env_sectors
 #define DEVTYPE(i)    envdevices[(i)].mtd_type
 

Quoted 6 lines

tools/env/fw_env.c static int dev_current; · line 82

Quoted 2 lines

tools/env/fw_env.c +1 -1

-#define CONFIG_ENV_SIZE ENVSIZE(dev_current)
+#define CFG_ENV_SIZE ENVSIZE(dev_current)

NAK, don't change it to CFG_... for no reason! Why did you change it ? Just use
ENVSIZE(dev_current) instead.

I agree with that.
As I wrote in the comment of the patch, this was mainly to get some feedback that I am on the right track.
I've seen a number of places where the code of fw_env.c could be improved, but opted for minimal change for now, as for now I am mostly solicitating feedback on the changes in fw_env.h
(and the actual reason for the change is that CONFIG_ENV_SIZE is defined here, but also in config.h, resulting in a naming config, a quick rename was the simplest way forward for now)

And actually I feel that lines like:
#define ENV1_SIZE CONFIG_ENV_SIZE
in fw_env.h are somewhat pointless and it would be better to eliminate ENV1_SIZE completely.

There are more cases like that. I'm happy to spent time on this, but only if it is felt to be useful and has any chance on being accepted.

Best regards, Frans