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

Hi Joakim,

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.

Please correct me if I did not understand your use case correctly.

Other boards have separate regions in flash to store ENV variables - even redundancy is supported (from ./include/mccmon6.h)

/* Envs are stored in NOR flash */ #define CONFIG_ENV_IS_IN_FLASH #define CONFIG_ENV_SECT_SIZE (SZ_128K) #define CONFIG_ENV_ADDR (CONFIG_SYS_FLASH_BASE + 0x40000)

#define CONFIG_SYS_REDUNDAND_ENVIRONMENT #define CONFIG_ENV_ADDR_REDUND (CONFIG_SYS_FLASH_BASE + 0x60000) #define CONFIG_ENV_SIZE_REDUND CONFIG_ENV_SIZE

You can extract the ENV variables by using following script:

scripts/get_default_envs.sh > default_envs.txt

and then create updated env image (with [*]) to be stored on flash.

Note:

[*] ./tools/mkenvimage -s 131072 -o ${UBOOT_ENVS_DEFAULT} default_envs.txt

Best regards, Łukasz Majewski

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@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;

Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de