[U-Boot] [PATCH v2 3/3] MX5:MX53: add initial support for MX53EVK board

[Stefano Babic <sbabic@denx.de>]

On 12/28/2010 08:45 AM, Jason Liu wrote:

>>> +     /* Set VDDA to 1.25V */
>>> +     val = pmic_reg_read(REG_SW_2);
>>> +     val = (val & (~0x1F)) | 0x1A;
>>
>> I have already commented this. Please drop all fix constants and use
>> already provided #define or add new ones if needed.
> 
> Do you think that both 0x1F and 0x1A should be used with #define?

Yes, of course. I see you have not included any specific pmic header in
the file (take a look at the mc13892.h file). Nobody knows what 0x1F and
0x1A mean, and everybody relies only on the comment. However, not always
a comment is updated...

> Do you think that both 0x1F and 0x1C should be used with #define?

Yes

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: office at denx.de
=====================================================================