From mboxrd@z Thu Jan 1 00:00:00 1970 From: Scott Wood Date: Thu, 29 May 2008 12:30:40 -0500 Subject: [U-Boot-Users] [PATCH]env_nand.c Added bad block management for environment variables In-Reply-To: References: <20080528175704.GA3090@loki.buserror.net> Message-ID: <483EE840.6030002@freescale.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Stuart Wood wrote: > Scott, > > Here is a new version of my patch to env_nand.c. Thanks for the good comments. > Fixed a problem with the new code that allowed it to read a > environment area even > if it contained a bad block after the good environment data. Please put comments such as these that don't belong in the commit message below a --- line, with the commit message and a signed-off-by above it. > diff --git a/common/env_nand.c b/common/env_nand.c > index 49742f5..3ee42e0 100644 > --- a/common/env_nand.c > +++ b/common/env_nand.c > @@ -1,4 +1,7 @@ > /* > + * (C) Copywrite 2008 > + * Stuart Wood, Lab X Technologies > + * > * (C) Copyright 2004 It's spelled "Copyright" (as in the "right" to "copy"). Just like the one below it. :-) > * Jian Zhang, Texas Instruments, jzhang at ti.com. > > @@ -53,6 +56,10 @@ > #error CONFIG_INFERNO not supported yet > #endif > > +#ifndef CFG_ENV_RANGE > +#define CFG_ENV_RANGE CFG_ENV_SIZE > +#endif > + > int nand_legacy_rw (struct nand_chip* nand, int cmd, > size_t start, size_t len, > size_t * retlen, u_char * buf); > @@ -152,30 +159,57 @@ int env_init(void) > * nand_dev_desc + 0. This is also the behaviour using the new NAND code. > */ > #ifdef CFG_ENV_OFFSET_REDUND > +size_t erase_env(size_t offset) > +{ erase_env() should not be within this ifdef. > + size_t end; > + > + end = offset + CFG_ENV_RANGE; > + > + while (offset < end && nand_erase(&nand_info[0],offset, CFG_ENV_SIZE)) Spaces after commas. > + offset += CFG_ENV_SIZE; > + > + if (offset >= end) > + return 0; > + > + return offset; > +} What if the offset is zero? Should return -1 or something similar on error. > @@ -208,10 +250,26 @@ int saveenv(void) > #endif /* CMD_SAVEENV */ > > #ifdef CFG_ENV_OFFSET_REDUND > +int check_env_size (size_t offset) > +{ What about the non-redundant version of env_relocate_spec? Also, this isn't checking the size, so it's not really an appropriate name. > + size_t end; > + int ret_val = 0; > + end = offset + CFG_ENV_SIZE; > + > + for (; offset < end; offset += nand_info[0].erasesize) { > + if (nand_block_isbad(&nand_info[0],offset)) > + ret_val = 1; > + } > + > + return ret_val; size_t end = offset + CFG_ENV_SIZE; while (offset < end) if (nand_block_isbad(&nand_info[0], offset)) return 1; return 0; > @@ -220,10 +278,27 @@ void env_relocate_spec (void) > tmp_env1 = (env_t *) malloc(CFG_ENV_SIZE); > tmp_env2 = (env_t *) malloc(CFG_ENV_SIZE); > > - nand_read(&nand_info[0], CFG_ENV_OFFSET, &total, > - (u_char*) tmp_env1); > - nand_read(&nand_info[0], CFG_ENV_OFFSET_REDUND, &total, > - (u_char*) tmp_env2); > + offset = CFG_ENV_OFFSET; > + end = offset + CFG_ENV_RANGE; > + > + while (offset < end && check_env_size(offset)) { > + offset += CFG_ENV_SIZE; > + } > + if (offset >= end) > + puts("No Valid Environment Area Found\n"); > + else > + nand_read(&nand_info[0], offset, &total, (u_char*) tmp_env1); If the environment can span multiple blocks, we should be able to skip blocks internally rather than requiring contiguous good blocks. -Scott