[U-Boot] [PATCHv2 2/4] env_nand.c: support falling back to redundant env when writing

[Scott Wood]

On Thu, Feb 21, 2013 at 06:21:54PM +0100, Phil Sutter wrote:
> Without this patch, when the currently chosen environment to be written
> has bad blocks, saveenv fails completely. Instead, when there is
> redundant environment fall back to the other copy. Environment reading
> needs no adjustment, as the fallback logic for incomplete writes applies
> to this case as well.
> 
> Signed-off-by: Phil Sutter <phil.sutter at viprinet.com>
> ---
>  common/env_nand.c |  103 ++++++++++++++++++++++------------------------------
>  1 files changed, 44 insertions(+), 59 deletions(-)
> 
> diff --git a/common/env_nand.c b/common/env_nand.c
> index 22e72a2..c0c985c 100644
> --- a/common/env_nand.c
> +++ b/common/env_nand.c
> @@ -168,72 +168,55 @@ int writeenv(size_t offset, u_char *buf)
>  	return 0;
>  }
>  
> -#ifdef CONFIG_ENV_OFFSET_REDUND
> -static unsigned char env_flags;
> +typedef struct env_location {
> +	const char *name;
> +	nand_erase_options_t *erase_opts;
> +	loff_t offset;
> +} env_location_t;

WARNING: do not add new typedefs
#286: FILE: common/env_nand.c:171:
+typedef struct env_location {

> +	printf("Writing to %s... ", location->name);
> +	if ((ret = writeenv(location->offset, env_new)))
>  		puts("FAILED!\n");
> -		return 1;
> -	}

ERROR: do not use assignment in if condition
#339: FILE: common/env_nand.c:187:
+	if ((ret = writeenv(location->offset, env_new)))


> -
> -	puts("done\n");
> -
> -	gd->env_valid = gd->env_valid == 2 ? 1 : 2;
> +	else
> +		puts("OK\n");
>  
>  	return ret;
>  }
> -#else /* ! CONFIG_ENV_OFFSET_REDUND */
> +
> +static unsigned char env_flags;
> +
>  int saveenv(void)
>  {
>  	int	ret = 0;
>  	ALLOC_CACHE_ALIGN_BUFFER(env_t, env_new, 1);
>  	ssize_t	len;
>  	char	*res;
> +	int	env_idx;
>  	nand_erase_options_t nand_erase_options;
> +	env_location_t location[] = {{
> +		.name = "NAND",
> +		.erase_opts = &nand_erase_options,
> +		.offset = CONFIG_ENV_OFFSET,
> +#ifdef CONFIG_ENV_OFFSET_REDUND
> +	}, {
> +		.name = "redundant NAND",
> +		.erase_opts = &nand_erase_options,
> +		.offset = CONFIG_ENV_OFFSET_REDUND,
> +#endif
> +	}};
> +

ERROR: space required after that close brace '}'
#374: FILE: common/env_nand.c:215:
+	}};

...or rather, it should be like this:

	static const struct env_location location[] = {
		{
			.name = "NAND";
			...
		},
#ifdef CONFIG_ENV_OFFSET_REDUND
		{
			.name = "redundant NAND",
			...
		},
#endif
	};

Note that without the "static" GCC will emit code to dynamically
initialize the array, which will be larger.

> +#ifdef CONFIG_ENV_OFFSET_REDUND
> +	if (ret) {
> +		env_idx = (env_idx + 1) & 1;
> +		ret = erase_and_write_env(&location[env_idx],
> +				(u_char *)env_new);
> +	} else
> +		gd->env_valid = gd->env_valid == 2 ? 1 : 2;

Braces around both halves of the if/else if one side has them.

-Scott