[U-Boot] [PATCH] mpc8xxx: improve LAW error messages when setting up DDR

[U-Boot] [PATCH] mpc8xxx: improve LAW error messages when setting up DDR

[Paul Gortmaker]

When setting up the LAWs for the DDR, if there was an error,
you got the not-so-helpful error text "ERROR" and nothing
else.  Not only is it non-informative, but it is also
pretty frustrating trying to grep for "ERROR" in the source.

Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
---
 cpu/mpc8xxx/ddr/util.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/cpu/mpc8xxx/ddr/util.c b/cpu/mpc8xxx/ddr/util.c
index 4451989..d0f61a8 100644
--- a/cpu/mpc8xxx/ddr/util.c
+++ b/cpu/mpc8xxx/ddr/util.c
@@ -89,16 +89,16 @@ __fsl_ddr_set_lawbar(const common_timing_params_t *memctl_common_params,
 			? LAW_TRGT_IF_DDR_INTRLV : LAW_TRGT_IF_DDR_1;
 
 		if (set_ddr_laws(base, size, lawbar1_target_id) < 0) {
-			printf("ERROR\n");
+			printf("set_lawbar: ERROR (%d)\n", memctl_interleaved);
 			return ;
 		}
 	} else if (ctrl_num == 1) {
 		if (set_ddr_laws(base, size, LAW_TRGT_IF_DDR_2) < 0) {
-			printf("ERROR\n");
+			printf("set_lawbar: ERROR (ctrl #2)\n");
 			return ;
 		}
 	} else {
-		printf("unexpected controller number %u in %s\n",
+		printf("set_lawbar: unexpected controller number %u in %s\n",
 			ctrl_num, __FUNCTION__);
 	}
 }
-- 
1.6.5.rc1

[U-Boot] [PATCH] mpc8xxx: improve LAW error messages when setting up DDR

[Peter Tyser]

Hi Paul,

> diff --git a/cpu/mpc8xxx/ddr/util.c b/cpu/mpc8xxx/ddr/util.c
> index 4451989..d0f61a8 100644
> --- a/cpu/mpc8xxx/ddr/util.c
> +++ b/cpu/mpc8xxx/ddr/util.c
> @@ -89,16 +89,16 @@ __fsl_ddr_set_lawbar(const common_timing_params_t *memctl_common_params,
>  			? LAW_TRGT_IF_DDR_INTRLV : LAW_TRGT_IF_DDR_1;
>  
>  		if (set_ddr_laws(base, size, lawbar1_target_id) < 0) {
> -			printf("ERROR\n");
> +			printf("set_lawbar: ERROR (%d)\n", memctl_interleaved);
>  			return ;
>  		}
>  	} else if (ctrl_num == 1) {
>  		if (set_ddr_laws(base, size, LAW_TRGT_IF_DDR_2) < 0) {
> -			printf("ERROR\n");
> +			printf("set_lawbar: ERROR (ctrl #2)\n");

This error would print out #2 for the 2nd controller...

>  			return ;
>  		}
>  	} else {
> -		printf("unexpected controller number %u in %s\n",
> +		printf("set_lawbar: unexpected controller number %u in %s\n",
>  			ctrl_num, __FUNCTION__);

But this error would print out 2 for the 3rd controller.  Either
convention is going to be confusing, but it'd be nice if they were at
least consistent.

__func__ is preferred over __FUNCTION__, maybe you could update it also?

Wouldn't this message look at bit funny with the title being
"set_lawbar:" but then also including the full "__fsl_ddr_set_lawbar" in
the same message?  And neither of the other errors include the printing
of __func__?  Hopefully I'll never see the errors, so proceed as you see
fit:)

Best,
Peter

[U-Boot] [PATCH] mpc8xxx: improve LAW error messages when setting up DDR

[Paul Gortmaker]

Peter Tyser wrote:
> Hi Paul,
> 
>> diff --git a/cpu/mpc8xxx/ddr/util.c b/cpu/mpc8xxx/ddr/util.c
>> index 4451989..d0f61a8 100644
>> --- a/cpu/mpc8xxx/ddr/util.c
>> +++ b/cpu/mpc8xxx/ddr/util.c
>> @@ -89,16 +89,16 @@ __fsl_ddr_set_lawbar(const common_timing_params_t *memctl_common_params,
>>  			? LAW_TRGT_IF_DDR_INTRLV : LAW_TRGT_IF_DDR_1;
>>  
>>  		if (set_ddr_laws(base, size, lawbar1_target_id) < 0) {
>> -			printf("ERROR\n");
>> +			printf("set_lawbar: ERROR (%d)\n", memctl_interleaved);
>>  			return ;
>>  		}
>>  	} else if (ctrl_num == 1) {
>>  		if (set_ddr_laws(base, size, LAW_TRGT_IF_DDR_2) < 0) {
>> -			printf("ERROR\n");
>> +			printf("set_lawbar: ERROR (ctrl #2)\n");
> 
> This error would print out #2 for the 2nd controller...

I was thinking 1 based counting for the messages presented to the
end user instead of the internal zero based, but...

> 
>>  			return ;
>>  		}
>>  	} else {
>> -		printf("unexpected controller number %u in %s\n",
>> +		printf("set_lawbar: unexpected controller number %u in %s\n",
>>  			ctrl_num, __FUNCTION__);
> 
> But this error would print out 2 for the 3rd controller.  Either

...as you point out, it then is inconsistent.  I'll fix that.

> convention is going to be confusing, but it'd be nice if they were at
> least consistent.
> 
> __func__ is preferred over __FUNCTION__, maybe you could update it also?
> 
> Wouldn't this message look at bit funny with the title being
> "set_lawbar:" but then also including the full "__fsl_ddr_set_lawbar" in
> the same message?  And neither of the other errors include the printing
> of __func__?  Hopefully I'll never see the errors, so proceed as you see
> fit:)

I never got to see this last one either, just the "ERROR" ones,
fortunately (?) but you make a good point - while in there, they
might as well all be standardized on func.  I'll do that too.

Thanks,
Paul.

> 
> Best,
> Peter
> 

[U-Boot] [PATCH v2] mpc8xxx: improve LAW error messages when setting up DDR

[Paul Gortmaker]

When setting up the LAWs for the DDR, if there was an error,
you got the not-so-helpful error text "ERROR" and nothing
else.  Not only is it non-informative, but it is also
pretty frustrating trying to grep for "ERROR" in the source.

Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
---

v2: sync ctrl #'s; use __func__ as per Peter's comments.

 cpu/mpc8xxx/ddr/util.c |    9 +++++----
 1 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/cpu/mpc8xxx/ddr/util.c b/cpu/mpc8xxx/ddr/util.c
index 4451989..1e2d921 100644
--- a/cpu/mpc8xxx/ddr/util.c
+++ b/cpu/mpc8xxx/ddr/util.c
@@ -89,17 +89,18 @@ __fsl_ddr_set_lawbar(const common_timing_params_t *memctl_common_params,
 			? LAW_TRGT_IF_DDR_INTRLV : LAW_TRGT_IF_DDR_1;
 
 		if (set_ddr_laws(base, size, lawbar1_target_id) < 0) {
-			printf("ERROR\n");
+			printf("%s: ERROR (ctrl #0, intrlv=%d)\n", __func__,
+				memctl_interleaved);
 			return ;
 		}
 	} else if (ctrl_num == 1) {
 		if (set_ddr_laws(base, size, LAW_TRGT_IF_DDR_2) < 0) {
-			printf("ERROR\n");
+			printf("%s: ERROR (ctrl #1)\n", __func__);
 			return ;
 		}
 	} else {
-		printf("unexpected controller number %u in %s\n",
-			ctrl_num, __FUNCTION__);
+		printf("%s: unexpected DDR controller number (%u)\n", __func__,
+			ctrl_num);
 	}
 }
 
-- 
1.6.5.rc1

[U-Boot] [PATCH v2] mpc8xxx: improve LAW error messages when setting up DDR

[Kumar Gala]

On Oct 7, 2009, at 3:34 PM, Paul Gortmaker wrote:

> When setting up the LAWs for the DDR, if there was an error,
> you got the not-so-helpful error text "ERROR" and nothing
> else.  Not only is it non-informative, but it is also
> pretty frustrating trying to grep for "ERROR" in the source.
>
> Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
> ---
>
> v2: sync ctrl #'s; use __func__ as per Peter's comments.
>
> cpu/mpc8xxx/ddr/util.c |    9 +++++----
> 1 files changed, 5 insertions(+), 4 deletions(-)

applied to 85xx

-k