[U-Boot] Remove global variable env_t *env_ptr ?

Joakim Tjernlund Joakim.Tjernlund at infinera.com
Mon Apr 3 12:19:32 UTC 2017


I am looking at adding support for runtime sizing of CONFIG_ENV_ADDR as
we need to replace out flash but we don't want to create a new u-boot binairy
just for this simple change.

While converting env_flash.c I noted the global variable
 env_t *env_ptr = (env_t *)CONFIG_ENV_ADDR;
which cannot be runtime decided.
Looking at users of this variable I only find one in pmc405de.c(not sure what that board is doing)
and for what I can tell this variable is not correct for redundant env. either.

Anyhow, I am faced wit two choices, either remove the env_ptr or
convert it to a function call.

What do fellow u-booters think about env_ptr?

 Jocke

PS. Adding my work so far here:
>From 5d3791099fb6a2c503b83298ac1f6135331466dc Mon Sep 17 00:00:00 2001
From: David Gounaris <david.gounaris at infinera.com>
Date: Fri, 31 Mar 2017 11:31:40 +0200
Subject: [PATCH 2/4] env_flash.c: Support dynamic sector size

This lay the ground work for supporting a non constant
sector size environment data.
---
 common/env_flash.c | 152 +++++++++++++++++++++++++++++------------------------
 1 file changed, 83 insertions(+), 69 deletions(-)

diff --git a/common/env_flash.c b/common/env_flash.c
index 004e884..306ef42 100644
--- a/common/env_flash.c
+++ b/common/env_flash.c
@@ -36,31 +36,20 @@ char *env_name_spec = "Flash";
 #ifdef ENV_IS_EMBEDDED
 env_t *env_ptr = &environment;
 
-static env_t *flash_addr = (env_t *)CONFIG_ENV_ADDR;
-
 #else /* ! ENV_IS_EMBEDDED */
 
 env_t *env_ptr = (env_t *)CONFIG_ENV_ADDR;
-static env_t *flash_addr = (env_t *)CONFIG_ENV_ADDR;
 #endif /* ENV_IS_EMBEDDED */
 
-#if defined(CMD_SAVEENV) || defined(CONFIG_ENV_ADDR_REDUND)
-/* CONFIG_ENV_ADDR is supposed to be on sector boundary */
-static ulong end_addr = CONFIG_ENV_ADDR + CONFIG_ENV_SECT_SIZE - 1;
-#endif
-
-#ifdef CONFIG_ENV_ADDR_REDUND
-static env_t *flash_addr_new = (env_t *)CONFIG_ENV_ADDR_REDUND;
-
-/* CONFIG_ENV_ADDR_REDUND is supposed to be on sector boundary */
-static ulong end_addr_new = CONFIG_ENV_ADDR_REDUND + CONFIG_ENV_SECT_SIZE - 1;
-#endif /* CONFIG_ENV_ADDR_REDUND */
-
-
 #ifdef CONFIG_ENV_ADDR_REDUND
 int env_init(void)
 {
 	int crc1_ok = 0, crc2_ok = 0;
+	env_t *flash_addr;
+	env_t *flash_addr_new;
+
+	flash_addr = (env_t *)CONFIG_ENV_ADDR;
+	flash_addr_new = (env_t *)CONFIG_ENV_ADDR_REDUND;
 
 	uchar flag1 = flash_addr->flags;
 	uchar flag2 = flash_addr_new->flags;
@@ -109,9 +98,28 @@ int saveenv(void)
 	char	*saved_data = NULL;
 	char	flag = OBSOLETE_FLAG, new_flag = ACTIVE_FLAG;
 	int	rc = 1;
-#if CONFIG_ENV_SECT_SIZE > CONFIG_ENV_SIZE
-	ulong	up_data = 0;
-#endif
+	ulong   up_data = 0;
+	env_t *flash_addr;
+	env_t *flash_addr_new;
+	ulong end_addr;
+	ulong end_addr_new;
+
+	flash_addr = (env_t *)CONFIG_ENV_ADDR;
+	flash_addr_new = (env_t *)CONFIG_ENV_ADDR_REDUND;
+	end_addr = CONFIG_ENV_ADDR + CONFIG_ENV_SECT_SIZE - 1;
+	end_addr_new = CONFIG_ENV_ADDR_REDUND + CONFIG_ENV_SECT_SIZE - 1;
+
+	if (gd->env_addr != (ulong)&(flash_addr->data)) {
+		/* Swap new and old ptrs */
+		env_t *etmp = flash_addr;
+		ulong ltmp = end_addr;
+
+		flash_addr = flash_addr_new;
+		flash_addr_new = etmp;
+
+		end_addr = end_addr_new;
+		end_addr_new = ltmp;
+	}
 
 	debug("Protect off %08lX ... %08lX\n", (ulong)flash_addr, end_addr);
 
@@ -129,24 +137,25 @@ int saveenv(void)
 		return rc;
 	env_new.flags	= new_flag;
 
-#if CONFIG_ENV_SECT_SIZE > CONFIG_ENV_SIZE
-	up_data = end_addr_new + 1 - ((long)flash_addr_new + CONFIG_ENV_SIZE);
-	debug("Data to save 0x%lX\n", up_data);
-	if (up_data) {
-		saved_data = malloc(up_data);
-		if (saved_data == NULL) {
-			printf("Unable to save the rest of sector (%ld)\n",
-				up_data);
-			goto done;
+	if(CONFIG_ENV_SECT_SIZE > CONFIG_ENV_SIZE) {
+		up_data = end_addr_new + 1 - ((long)flash_addr_new + CONFIG_ENV_SIZE);
+		debug("Data to save 0x%lX\n", up_data);
+		if (up_data) {
+			saved_data = malloc(up_data);
+			if (saved_data == NULL) {
+				printf("Unable to save the rest of sector (%ld)\n",
+				       up_data);
+				goto done;
+			}
+			memcpy(saved_data,
+			       (void *)((long)flash_addr_new + CONFIG_ENV_SIZE),
+			       up_data);
+			debug("Data (start 0x%lX, len 0x%lX) saved at 0x%p\n",
+			      (long)flash_addr_new + CONFIG_ENV_SIZE,
+			      up_data, saved_data);
 		}
-		memcpy(saved_data,
-			(void *)((long)flash_addr_new + CONFIG_ENV_SIZE),
-			up_data);
-		debug("Data (start 0x%lX, len 0x%lX) saved at 0x%p\n",
-			(long)flash_addr_new + CONFIG_ENV_SIZE,
-			up_data, saved_data);
 	}
-#endif
+
 	puts("Erasing Flash...");
 	debug(" %08lX ... %08lX ...", (ulong)flash_addr_new, end_addr_new);
 
@@ -167,7 +176,6 @@ int saveenv(void)
 	if (rc)
 		goto perror;
 
-#if CONFIG_ENV_SECT_SIZE > CONFIG_ENV_SIZE
 	if (up_data) { /* restore the rest of sector */
 		debug("Restoring the rest of data to 0x%lX len 0x%lX\n",
 			(long)flash_addr_new + CONFIG_ENV_SIZE, up_data);
@@ -176,19 +184,8 @@ int saveenv(void)
 				up_data))
 			goto perror;
 	}
-#endif
-	puts("done\n");
-
-	{
-		env_t *etmp = flash_addr;
-		ulong ltmp = end_addr;
-
-		flash_addr = flash_addr_new;
-		flash_addr_new = etmp;
 
-		end_addr = end_addr_new;
-		end_addr_new = ltmp;
-	}
+	puts("done\n");
 
 	rc = 0;
 	goto done;
@@ -209,8 +206,11 @@ done:
 
 int env_init(void)
 {
-	if (crc32(0, env_ptr->data, ENV_SIZE) == env_ptr->crc) {
-		gd->env_addr	= (ulong)&(env_ptr->data);
+	env_t *flash_addr;
+
+	flash_addr = (env_t *)CONFIG_ENV_ADDR;
+	if (crc32(0, flash_addr->data, ENV_SIZE) == flash_addr->crc) {
+		gd->env_addr	= (ulong)&(flash_addr->data);
 		gd->env_valid	= 1;
 		return 0;
 	}
@@ -226,26 +226,31 @@ int saveenv(void)
 	env_t	env_new;
 	int	rc = 1;
 	char	*saved_data = NULL;
-#if CONFIG_ENV_SECT_SIZE > CONFIG_ENV_SIZE
 	ulong	up_data = 0;
-
-	up_data = end_addr + 1 - ((long)flash_addr + CONFIG_ENV_SIZE);
-	debug("Data to save 0x%lx\n", up_data);
-	if (up_data) {
-		saved_data = malloc(up_data);
-		if (saved_data == NULL) {
-			printf("Unable to save the rest of sector (%ld)\n",
-				up_data);
-			goto done;
+	env_t *flash_addr;
+	ulong end_addr;
+
+	flash_addr = (env_t *)CONFIG_ENV_ADDR;
+	end_addr = CONFIG_ENV_ADDR + CONFIG_ENV_SECT_SIZE - 1;
+
+	if(CONFIG_ENV_SECT_SIZE > CONFIG_ENV_SIZE) {
+		up_data = end_addr + 1 - ((long)flash_addr + CONFIG_ENV_SIZE);
+		debug("Data to save 0x%lx\n", up_data);
+		if (up_data) {
+			saved_data = malloc(up_data);
+			if (saved_data == NULL) {
+				printf("Unable to save the rest of sector (%ld)\n",
+				       up_data);
+				goto done;
+			}
+			memcpy(saved_data,
+			       (void *)((long)flash_addr + CONFIG_ENV_SIZE), up_data);
+			debug("Data (start 0x%lx, len 0x%lx) saved at 0x%lx\n",
+			      (ulong)flash_addr + CONFIG_ENV_SIZE,
+			      up_data,
+			      (ulong)saved_data);
 		}
-		memcpy(saved_data,
-			(void *)((long)flash_addr + CONFIG_ENV_SIZE), up_data);
-		debug("Data (start 0x%lx, len 0x%lx) saved at 0x%lx\n",
-			(ulong)flash_addr + CONFIG_ENV_SIZE,
-			up_data,
-			(ulong)saved_data);
 	}
-#endif	/* CONFIG_ENV_SECT_SIZE */
 
 	debug("Protect off %08lX ... %08lX\n", (ulong)flash_addr, end_addr);
 
@@ -265,7 +270,6 @@ int saveenv(void)
 	if (rc != 0)
 		goto perror;
 
-#if CONFIG_ENV_SECT_SIZE > CONFIG_ENV_SIZE
 	if (up_data) {	/* restore the rest of sector */
 		debug("Restoring the rest of data to 0x%lx len 0x%lx\n",
 			(ulong)flash_addr + CONFIG_ENV_SIZE, up_data);
@@ -274,7 +278,7 @@ int saveenv(void)
 				up_data))
 			goto perror;
 	}
-#endif
+
 	puts("done\n");
 	rc = 0;
 	goto done;
@@ -294,6 +298,16 @@ done:
 void env_relocate_spec(void)
 {
 #ifdef CONFIG_ENV_ADDR_REDUND
+	env_t *flash_addr;
+	env_t *flash_addr_new;
+	ulong end_addr;
+	ulong end_addr_new;
+
+	flash_addr = (env_t *)CONFIG_ENV_ADDR;
+	flash_addr_new = (env_t *)CONFIG_ENV_ADDR_REDUND;
+	end_addr = CONFIG_ENV_ADDR + CONFIG_ENV_SECT_SIZE - 1;
+	end_addr_new = CONFIG_ENV_ADDR_REDUND + CONFIG_ENV_SECT_SIZE - 1;
+
 	if (gd->env_addr != (ulong)&(flash_addr->data)) {
 		env_t *etmp = flash_addr;
 		ulong ltmp = end_addr;
-- 
2.10.2


More information about the U-Boot mailing list