Hi, Stefano,

2011/4/22 Stefano Babic <sba...@denx.de>:
> On 04/22/2011 07:45 AM, Jason Hui wrote:
>> Hi, Stefano,
>
> Hi Jason,
>
>>>>  int checkboard(void)
>>>>  {
>>>>       u32 system_rev = get_cpu_rev();
>>>> -     u32 cause;
>>>> -     struct src *src_regs = (struct src *)SRC_BASE_ADDR;
>>>
>>> This seems to me not the best solution. If we have now factored out code
>>> to print the reset cause and the silicon version (inside print_cpuinfo),
>>> why do we need to repeat this code for each board ? Calling get_cpu_rev
>>> seems to me redundant (then each board should only set
>>> CONFIG_DISPLAY_CPUINFO). And then the CPU revision is printed again, and
>>> this is redundant.
>>
>> The purpose for this patch is to remove the boot cause code and and don't 
>> change
>> any cpu rev code. The cpu rev part of code is as it is as before.
>
> However, it seems to me a half-way clean up. As we have already
> factorize function for printing the cpu revision and the reset cause, I
> will see in the checkboard only board related information. If there is
> no revision number for board, printing only the board name as you make
> for the LOCO is correct.
>
> Taking as example the efika board (all boards make the same):
>
>>
>>>
>>>>
>>>>       puts("Board: Efika MX ");
>>>>
>>>>       switch (system_rev & 0xff) {
>
> The only new information is the board name. If I am not wrong,
> system_rev & 0xff contains only the cpu revision, and the switch prints
> out the silicon version. Everything already done in print_cpuinfo as well.
>
>>
>> Ditto, as I only remove the boot cause part of code as the patch tile said.
>
> Yes, but this it is redundant with the print_cpuinfo(). As you plan to
> clean up the code, this part should be cleaned up as well. Or do you
> think there is something not covered by the common code ?
>
>>> I think we need more clean up, removing all part related to CPU revision
>>> and leaving (if any) only the output related to the board revision.
>>
>> If that, I need change the patch tile, and include more clean up in the patch
>> and send again.
>
> Agree.

OK, I will re-send the patch with more clean-up.

Jason

>
> Best regards,
> Stefano Babic
>
> --
> =====================================================================
> DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: off...@denx.de
> =====================================================================
> _______________________________________________
> U-Boot mailing list
> U-Boot@lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
>
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Reply via email to