[U-Boot] [PATCH] Bad Block Capable environment for OneNAND

[Scott Wood]

On Mon, Apr 13, 2009 at 06:34:52PM +0530, apgmoorthy wrote:
> +	/* Protect boot-loader and environment variables from badblock testing */
> +	while (start < CONFIG_SYS_MONITOR_LEN + CONFIG_ENV_SIZE + badblocklen) {
> +		if (mtd->block_isbad(&onenand_mtd, start))
> +			badblocklen += blocksize;
> +		start += blocksize;
> +	}

You need to start at zero to determine the number of bad blocks used by
the boot image -- otherwise, if start points in the middle of the image,
it is assuming that there were no bad blocks prior to that location.

Plus, if "start" was not block-aligned (is that allowed?), this should
round up to the next block rather than jumping to the middle of the next
block.

> +#define ONENAND_ENV_END   (CONFIG_SYS_MONITOR_LEN + CONFIG_ENV_SIZE)
> +
>  char *env_name_spec = "OneNAND";
>  
>  #ifdef ENV_IS_EMBEDDED
> @@ -58,23 +68,31 @@ uchar env_get_char_spec(int index)
>  
>  void env_relocate_spec(void)
>  {
> -	unsigned long env_addr;
> -	int use_default = 0;
> +	unsigned long env_addr = CONFIG_SYS_MONITOR_LEN;

As above, you have to check for bad blocks from the beginning.  You do
this in saveenv...

> +	int use_default = 1;
>  	size_t retlen;
> +	int blocksize = onenand_mtd.erasesize;
>  
> -	env_addr = CONFIG_ENV_ADDR;
> +	/* Find environment block. */
> +	while (onenand_mtd.writesize && (env_addr < ONENAND_ENV_END)) {
> +		if (onenand_block_isbad(&onenand_mtd, env_addr)) {
> +			printf("Skip bad block at 0x%x\n", (u32)env_addr);

Why not just use %lx?

> +	 /* update crc */
> +	 env_ptr->crc =
> +	     crc32(0, env_ptr->data, ONENAND_ENV_SIZE(onenand_mtd));

There's an extra space in the above indent.

> +	 return 0;

Here too.

<record state="broken">
Any consolidation we can do between env_nand and env_onenand would be
appreciated... even if they don't share code yet, let's keep the logic
similar.
</record>

-Scott