[U-Boot] [PATCH] mpc8xxx: improve LAW error messages when setting up DDR
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__); } }
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 ;
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
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 ;
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
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); } }
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
participants (3)
-
Kumar Gala -
Paul Gortmaker -
Peter Tyser