Patchwork [U-Boot,3/4] env_nand.c: do warn only if really no valid environment could be loaded

login
register
mail settings
Submitter Phil Sutter
Date Nov. 21, 2012, 12:59 p.m.
Message ID <1353502761-12640-3-git-send-email-phil.sutter@viprinet.com>
Download mbox | patch
Permalink /patch/200771/
State Changes Requested
Delegated to: Scott Wood
Headers show

Comments

Phil Sutter - Nov. 21, 2012, 12:59 p.m.
The warning is misleading, since there is no equivalent success note
when reading the other copy succeeds.

Signed-off-by: Phil Sutter <phil.sutter@viprinet.com>
---
 common/env_nand.c |    9 +++++----
 1 files changed, 5 insertions(+), 4 deletions(-)
Scott Wood - Nov. 27, 2012, 10:06 p.m.
On 11/21/2012 06:59:20 AM, Phil Sutter wrote:
> The warning is misleading, since there is no equivalent success note
> when reading the other copy succeeds.
> 
> Signed-off-by: Phil Sutter <phil.sutter@viprinet.com>
> ---
>  common/env_nand.c |    9 +++++----
>  1 files changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/common/env_nand.c b/common/env_nand.c
> index 895532b..58ba558 100644
> --- a/common/env_nand.c
> +++ b/common/env_nand.c
> @@ -315,6 +315,7 @@ int get_nand_env_oob(nand_info_t *nand, unsigned  
> long *result)
>  void env_relocate_spec(void)
>  {
>  #if !defined(ENV_IS_EMBEDDED)
> +	int read1_fail = 0, read2_fail = 0;
>  	int crc1_ok = 0, crc2_ok = 0;
>  	env_t *ep, *tmp_env1, *tmp_env2;
> 
> @@ -326,11 +327,11 @@ void env_relocate_spec(void)
>  		goto done;
>  	}
> 
> -	if (readenv(CONFIG_ENV_OFFSET, (u_char *) tmp_env1))
> -		puts("No Valid Environment Area found\n");
> +	read1_fail = readenv(CONFIG_ENV_OFFSET, (u_char *) tmp_env1);
> +	read2_fail = readenv(CONFIG_ENV_OFFSET_REDUND, (u_char *)  
> tmp_env2);

Why read the redundant environment if the the main one didn't fail?

-Scott
Scott Wood - Nov. 27, 2012, 10:07 p.m.
On 11/27/2012 04:06:03 PM, Scott Wood wrote:
> On 11/21/2012 06:59:20 AM, Phil Sutter wrote:
>> The warning is misleading, since there is no equivalent success note
>> when reading the other copy succeeds.
>> 
>> Signed-off-by: Phil Sutter <phil.sutter@viprinet.com>
>> ---
>>  common/env_nand.c |    9 +++++----
>>  1 files changed, 5 insertions(+), 4 deletions(-)
>> 
>> diff --git a/common/env_nand.c b/common/env_nand.c
>> index 895532b..58ba558 100644
>> --- a/common/env_nand.c
>> +++ b/common/env_nand.c
>> @@ -315,6 +315,7 @@ int get_nand_env_oob(nand_info_t *nand, unsigned  
>> long *result)
>>  void env_relocate_spec(void)
>>  {
>>  #if !defined(ENV_IS_EMBEDDED)
>> +	int read1_fail = 0, read2_fail = 0;
>>  	int crc1_ok = 0, crc2_ok = 0;
>>  	env_t *ep, *tmp_env1, *tmp_env2;
>> 
>> @@ -326,11 +327,11 @@ void env_relocate_spec(void)
>>  		goto done;
>>  	}
>> 
>> -	if (readenv(CONFIG_ENV_OFFSET, (u_char *) tmp_env1))
>> -		puts("No Valid Environment Area found\n");
>> +	read1_fail = readenv(CONFIG_ENV_OFFSET, (u_char *) tmp_env1);
>> +	read2_fail = readenv(CONFIG_ENV_OFFSET_REDUND, (u_char *)  
>> tmp_env2);
> 
> Why read the redundant environment if the the main one didn't fail?

Never mind, misread the original code (the answer is we want to see  
which one is more recent).

-Scott

Patch

diff --git a/common/env_nand.c b/common/env_nand.c
index 895532b..58ba558 100644
--- a/common/env_nand.c
+++ b/common/env_nand.c
@@ -315,6 +315,7 @@  int get_nand_env_oob(nand_info_t *nand, unsigned long *result)
 void env_relocate_spec(void)
 {
 #if !defined(ENV_IS_EMBEDDED)
+	int read1_fail = 0, read2_fail = 0;
 	int crc1_ok = 0, crc2_ok = 0;
 	env_t *ep, *tmp_env1, *tmp_env2;
 
@@ -326,11 +327,11 @@  void env_relocate_spec(void)
 		goto done;
 	}
 
-	if (readenv(CONFIG_ENV_OFFSET, (u_char *) tmp_env1))
-		puts("No Valid Environment Area found\n");
+	read1_fail = readenv(CONFIG_ENV_OFFSET, (u_char *) tmp_env1);
+	read2_fail = readenv(CONFIG_ENV_OFFSET_REDUND, (u_char *) tmp_env2);
 
-	if (readenv(CONFIG_ENV_OFFSET_REDUND, (u_char *) tmp_env2))
-		puts("No Valid Redundant Environment Area found\n");
+	if (read1_fail && read2_fail)
+		puts("No Valid Environment Area found\n");
 
 	crc1_ok = crc32(0, tmp_env1->data, ENV_SIZE) == tmp_env1->crc;
 	crc2_ok = crc32(0, tmp_env2->data, ENV_SIZE) == tmp_env2->crc;