On Wednesday, December 29, 2010 20:53:17 Mike Frysinger wrote:
> On Saturday, July 17, 2010 15:45:48 Wolfgang Denk wrote:
> > --- a/common/env_flash.c
> > +++ b/common/env_flash.c
> > 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 */
>
> 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 ...
seems the new saveenv code in the non-redund case has been rewritten
completely and doesn't support the same feature set as it used to.
old behavior:
- load up the entire sector
- overlay new env into sector
- round env start addr down to sector start
- round env end addr up to sector end
- protect/erase/save/protect whole sector
new behavior:
- load up the sector data after the env
- overlay new env into sector
- set env end addr to start of env plus one sector
- protect/erase/save/protect whole sector
so the difference is that the new code no longer supports envs which do not
start on a sector boundary. it does this so that it doesn't memcpy() as much
data out of the flash at the expense of doing more CPU bound math operations
(mostly bit twiddling).
so the question is whether the old behavior should be restored. if it not,
the documentation should be tweaked to note these requirements and some simple
CPP checks added to environment.h so that this issue turns into a build
failure. after all, it's easy to detect an env offset that isnt at the start
of the sector:
#if (CONFIG_ENV_OFFSET & (CONFIG_ENV_SECT_SIZE - 1))
# error env offset not sector aligned
#endif
-mike
signature.asc
Description: This is a digitally signed message part.
_______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
