[U-Boot] env_flash.c:saveenv() broken when env is smaller than a sector
Mike Frysinger
vapier at gentoo.org
Thu Dec 30 02:53:17 CET 2010
On Saturday, July 17, 2010 15:45:48 Wolfgang Denk wrote:
> --- a/common/env_flash.c
> +++ b/common/env_flash.c
> #ifdef CMD_SAVEENV
> int saveenv(void)
> {
> - char *saved_data = NULL;
> - int rc = 1;
> - char flag = OBSOLETE_FLAG, new_flag = ACTIVE_FLAG;
> + env_t env_new;
> + ssize_t len;
> + char *saved_data = NULL;
> + char *res;
> + int rc = 1;
> + char flag = OBSOLETE_FLAG, new_flag = ACTIVE_FLAG;
> #if CONFIG_ENV_SECT_SIZE > CONFIG_ENV_SIZE
> - ulong up_data = 0;
> + ulong up_data = 0;
> #endif
>
> - debug ("Protect off %08lX ... %08lX\n",
> + debug("Protect off %08lX ... %08lX\n",
> (ulong)flash_addr, end_addr);
>
> - if (flash_sect_protect (0, (ulong)flash_addr, end_addr)) {
> - goto Done;
> + if (flash_sect_protect(0, (ulong)flash_addr, end_addr)) {
> + goto done;
> }
>
> - debug ("Protect off %08lX ... %08lX\n",
> + debug("Protect off %08lX ... %08lX\n",
> (ulong)flash_addr_new, end_addr_new);
>
> - if (flash_sect_protect (0, (ulong)flash_addr_new, end_addr_new)) {
> - goto Done;
> + if (flash_sect_protect(0, (ulong)flash_addr_new, end_addr_new)) {
> + goto done;
> + }
> +
> + res = (char *)&env_new.data;
> + len = hexport('\0', &res, ENV_SIZE);
> + if (len < 0) {
> + error("Cannot export environment: errno = %d\n", errno);
> + goto done;
> }
> + env_new.crc = crc32(0, env_new.data, ENV_SIZE);
> + 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%x\n", up_data);
> + debug("Data to save 0x%lX\n", up_data);
> if (up_data) {
> if ((saved_data = malloc(up_data)) == NULL) {
> printf("Unable to save the rest of sector (%ld)\n",
> up_data);
> - goto Done;
> + goto done;
> }
> memcpy(saved_data,
> (void *)((long)flash_addr_new + CONFIG_ENV_SIZE), up_data);
> - debug ("Data (start 0x%x, len 0x%x) saved at 0x%x\n",
> - (long)flash_addr_new + CONFIG_ENV_SIZE,
> - up_data, saved_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 ...",
> + puts("Erasing Flash...");
> + debug(" %08lX ... %08lX ...",
> (ulong)flash_addr_new, end_addr_new);
>
> - if (flash_sect_erase ((ulong)flash_addr_new, end_addr_new)) {
> - goto Done;
> + if (flash_sect_erase((ulong)flash_addr_new, end_addr_new)) {
> + goto done;
> }
>
> - puts ("Writing to Flash... ");
> - debug (" %08lX ... %08lX ...",
> + puts("Writing to Flash... ");
> + debug(" %08lX ... %08lX ...",
> (ulong)&(flash_addr_new->data),
> sizeof(env_ptr->data)+(ulong)&(flash_addr_new->data));
> - if ((rc = flash_write((char *)env_ptr->data,
> - (ulong)&(flash_addr_new->data),
> - sizeof(env_ptr->data))) ||
> - (rc = flash_write((char *)&(env_ptr->crc),
> - (ulong)&(flash_addr_new->crc),
> - sizeof(env_ptr->crc))) ||
> + if ((rc = flash_write((char *)&env_new,
> + (ulong)flash_addr_new,
> + sizeof(env_new))) ||
> (rc = flash_write(&flag,
> (ulong)&(flash_addr->flags),
> - sizeof(flash_addr->flags))) ||
> - (rc = flash_write(&new_flag,
> - (ulong)&(flash_addr_new->flags),
> - sizeof(flash_addr_new->flags))))
> - {
> - flash_perror (rc);
> - goto Done;
> + sizeof(flash_addr->flags))) ) {
> + flash_perror(rc);
> + goto done;
> }
> - puts ("done\n");
>
> #if CONFIG_ENV_SECT_SIZE > CONFIG_ENV_SIZE
> if (up_data) { /* restore the rest of sector */
> - debug ("Restoring the rest of data to 0x%x len 0x%x\n",
> - (long)flash_addr_new + CONFIG_ENV_SIZE, up_data);
> + debug("Restoring the rest of data to 0x%lX len 0x%lX\n",
> + (long)flash_addr_new + CONFIG_ENV_SIZE, up_data);
> if (flash_write(saved_data,
> (long)flash_addr_new + CONFIG_ENV_SIZE,
> up_data)) {
> flash_perror(rc);
> - goto Done;
> + goto done;
> }
> }
> #endif
> + puts("done\n");
> +
> {
> env_t * etmp = flash_addr;
> ulong ltmp = end_addr;
> @@ -220,13 +230,12 @@ int saveenv(void)
> }
>
> rc = 0;
> -Done:
> -
> +done:
> if (saved_data)
> - free (saved_data);
> + free(saved_data);
> /* try to re-protect */
> - (void) flash_sect_protect (1, (ulong)flash_addr, end_addr);
> - (void) flash_sect_protect (1, (ulong)flash_addr_new, end_addr_new);
> + (void) flash_sect_protect(1, (ulong)flash_addr, end_addr);
> + (void) flash_sect_protect(1, (ulong)flash_addr_new, end_addr_new);
>
> return rc;
> }
> @@ -244,83 +253,93 @@ int env_init(void)
>
> gd->env_addr = (ulong)&default_environment[0];
> gd->env_valid = 0;
> - return (0);
> + return 0;
> }
>
> #ifdef CMD_SAVEENV
>
> int saveenv(void)
> {
> - int len, rc;
> - ulong end_addr;
> - ulong flash_sect_addr;
> -#if defined(CONFIG_ENV_SECT_SIZE) && (CONFIG_ENV_SECT_SIZE >
> CONFIG_ENV_SIZE) - ulong flash_offset;
> - uchar env_buffer[CONFIG_ENV_SECT_SIZE];
> -#else
> - uchar *env_buffer = (uchar *)env_ptr;
> -#endif /* CONFIG_ENV_SECT_SIZE */
> - int rcode = 0;
> -
> -#if defined(CONFIG_ENV_SECT_SIZE) && (CONFIG_ENV_SECT_SIZE >
> CONFIG_ENV_SIZE) -
> - flash_offset = ((ulong)flash_addr) & (CONFIG_ENV_SECT_SIZE-1);
> - flash_sect_addr = ((ulong)flash_addr) & ~(CONFIG_ENV_SECT_SIZE-1);
> -
> - debug ( "copy old content: "
> - "sect_addr: %08lX env_addr: %08lX offset: %08lX\n",
> - flash_sect_addr, (ulong)flash_addr, flash_offset);
> -
> - /* copy old contents to temporary buffer */
> - memcpy (env_buffer, (void *)flash_sect_addr, CONFIG_ENV_SECT_SIZE);
> -
> - /* copy current environment to temporary buffer */
> - memcpy ((uchar *)((unsigned long)env_buffer + flash_offset),
> - env_ptr,
> - CONFIG_ENV_SIZE);
> + env_t env_new;
> + ssize_t len;
> + int rc = 1;
> + char *res;
> + char *saved_data = NULL;
> +#if CONFIG_ENV_SECT_SIZE > CONFIG_ENV_SIZE
> + ulong up_data = 0;
>
> - len = CONFIG_ENV_SECT_SIZE;
> -#else
> - flash_sect_addr = (ulong)flash_addr;
> - len = 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) {
> + if ((saved_data = malloc(up_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);
> + }
> #endif /* CONFIG_ENV_SECT_SIZE */
>
> - end_addr = flash_sect_addr + len - 1;
> + debug("Protect off %08lX ... %08lX\n",
> + (ulong)flash_addr, end_addr);
>
> - debug ("Protect off %08lX ... %08lX\n",
> - (ulong)flash_sect_addr, end_addr);
> + if (flash_sect_protect(0, (long)flash_addr, end_addr))
> + goto done;
>
> - if (flash_sect_protect (0, flash_sect_addr, end_addr))
> - return 1;
> + res = (char *)&env_new.data;
> + len = hexport('\0', &res, ENV_SIZE);
> + if (len < 0) {
> + error("Cannot export environment: errno = %d\n", errno);
> + goto done;
> + }
> + env_new.crc = crc32(0, env_new.data, ENV_SIZE);
>
> - puts ("Erasing Flash...");
> - if (flash_sect_erase (flash_sect_addr, end_addr))
> - return 1;
> + puts("Erasing Flash...");
> + if (flash_sect_erase((long)flash_addr, end_addr))
> + goto done;
>
> - puts ("Writing to Flash... ");
> - rc = flash_write((char *)env_buffer, flash_sect_addr, len);
> + puts("Writing to Flash... ");
> + rc = flash_write((char *)&env_new, (long)flash_addr, CONFIG_ENV_SIZE);
> if (rc != 0) {
> - flash_perror (rc);
> - rcode = 1;
> - } else {
> - puts ("done\n");
> + flash_perror(rc);
> + goto done;
> }
> -
> +#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);
> + if (flash_write(saved_data,
> + (long)flash_addr + CONFIG_ENV_SIZE,
> + up_data)) {
> + flash_perror(rc);
> + goto done;
> + }
> + }
> +#endif
> + puts("done\n");
> + rc = 0;
> +done:
> + if (saved_data)
> + free(saved_data);
> /* try to re-protect */
> - (void) flash_sect_protect (1, flash_sect_addr, end_addr);
> - return rcode;
> + (void) flash_sect_protect(1, (long)flash_addr, end_addr);
> + return rc;
> }
it would seem that somewhere nestled in this rewrite, support for embedded env
was broken (CONFIG_ENV_SIZE < CONFIG_ENV_SECT_SIZE). on a bf548-ezkit for
example, i see this behavior:
(v2010.09 / before this commit):
bfin> save
Saving Environment to Flash...
. done
Un-Protected 1 sectors
Erasing Flash...
. done
Erased 1 sectors
Writing to Flash... done
. done
Protected 1 sectors
(after this commit / v2010.12):
bfin> save
Saving Environment to Flash...
Error: start address not on sector boundary
Error: start address not on sector boundary
not sure if anyone has noticed/fixed this yet and would save me from the
trouble of finding/fixing the problem ...
-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/20101229/14330289/attachment.pgp
More information about the U-Boot
mailing list