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 mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
