[05/14] env: Make it explicit where we're loading our environment from

Message ID 45c910bdc2ad56e4ab10612ca801692812b3bbfc.1511864667.git-series.maxime.ripard@free-electrons.com
State New
Headers show
Series
  • env: Multiple env support and env transition for sunxi
Related show

Commit Message

Maxime Ripard Nov. 28, 2017, 10:24 a.m.
Since we can have multiple environments now, it's better to provide a
decent indication on what environments were tried and which were the one to
fail and succeed.

Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 env/env.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

Comments

Andre Przywara Dec. 5, 2017, 10:09 a.m. | #1
On 28/11/17 10:24, Maxime Ripard wrote:
> Since we can have multiple environments now, it's better to provide a
> decent indication on what environments were tried and which were the one to
> fail and succeed.
> 
> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> ---
>  env/env.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/env/env.c b/env/env.c
> index 1d13220aa79b..44f9908e2c2d 100644
> --- a/env/env.c
> +++ b/env/env.c
> @@ -109,12 +109,11 @@ int env_load(void)
>  		if (!drv->load)
>  			continue;
>  
> +		printf("Loading Environment from %s... ", drv->name);
>  		ret = drv->load();
> +		printf("%s\n", ret ? "Failed" : "OK");

This looses the error code that was printed with debug() below. Might be
worth to keep it? Either always or as a debug?
		printf("%s (%d)\n", ...);

Cheers,
Andre.

>  		if (!ret)
>  			return 0;
> -
> -		debug("%s: Environment %s failed to load (err=%d)\n", __func__,
> -		      drv->name, ret);
>  	}
>  
>  	return -ENODEV;
>
Simon Glass Dec. 29, 2017, 3:13 a.m. | #2
On 5 December 2017 at 03:09, Andre Przywara <andre.przywara@arm.com> wrote:
> On 28/11/17 10:24, Maxime Ripard wrote:
>> Since we can have multiple environments now, it's better to provide a
>> decent indication on what environments were tried and which were the one to
>> fail and succeed.
>>
>> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
>> ---
>>  env/env.c | 5 ++---
>>  1 file changed, 2 insertions(+), 3 deletions(-)
>>
>> diff --git a/env/env.c b/env/env.c
>> index 1d13220aa79b..44f9908e2c2d 100644
>> --- a/env/env.c
>> +++ b/env/env.c
>> @@ -109,12 +109,11 @@ int env_load(void)
>>               if (!drv->load)
>>                       continue;
>>
>> +             printf("Loading Environment from %s... ", drv->name);
>>               ret = drv->load();
>> +             printf("%s\n", ret ? "Failed" : "OK");
>
> This looses the error code that was printed with debug() below. Might be
> worth to keep it? Either always or as a debug?
>                 printf("%s (%d)\n", ...);
>

Seems like a good idea. Other than that:

Reviewed-by: Simon Glass <sjg@chromium.org>


> Cheers,
> Andre.
>
>>               if (!ret)
>>                       return 0;
>> -
>> -             debug("%s: Environment %s failed to load (err=%d)\n", __func__,
>> -                   drv->name, ret);
>>       }
>>
>>       return -ENODEV;
>>
>
> _______________________________________________
> U-Boot mailing list
> U-Boot@lists.denx.de
> https://lists.denx.de/listinfo/u-boot

Patch

diff --git a/env/env.c b/env/env.c
index 1d13220aa79b..44f9908e2c2d 100644
--- a/env/env.c
+++ b/env/env.c
@@ -109,12 +109,11 @@  int env_load(void)
 		if (!drv->load)
 			continue;
 
+		printf("Loading Environment from %s... ", drv->name);
 		ret = drv->load();
+		printf("%s\n", ret ? "Failed" : "OK");
 		if (!ret)
 			return 0;
-
-		debug("%s: Environment %s failed to load (err=%d)\n", __func__,
-		      drv->name, ret);
 	}
 
 	return -ENODEV;