[U-Boot] [PATCH V2 11/11] Add support for Freescale's mx35pdk board.

[Wolfgang Denk]

Dear Stefano Babic,

In message <4D381237.9040309 at denx.de> you wrote:
>
> >> +	/* Print board revision */
> >> +	puts(board_detect() ? "2.0" : "1.0");
> >> + 
> >> +	/* Print CPU revision */
> >> +	puts(" i.MX35 ");
> > 
> > I mentioned this before:  If you make board_detect() return 1 or 2, 
> > you can combine the calls to puts() for example like that:
> 
> I was unsure, it seemed to me easier to understand as it is implemented
> now, becaues board_detect() is used to detect if the PMIC is installed
> or not. It returns 0 or 1, and tell if the test for the PMIC was
> successful.
> 
> Probably the former version of the board has no pmic at all or was not
> connected to I2C. So in another part of code board_detect is used in
> boolean form:
> 
> if (board_detect()) {

This function should be called pmic_detect() or so.

> I thought that to combine the result makes some confusion. Probably it
> is clearer to use get_board_rev() instead of board_detect() and to
> extract the revision number from the returned u32:
> 
>  	printf("Board: MX35 PDK %d.0 i.MX35 ", (get_board_rev() >> 8) & 0xFF);

Agreed.


Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
It's certainly  convenient  the  way  the  crime  (or  condition)  of
stupidity   carries   with   it  its  own  punishment,  automatically
admisistered without remorse, pity, or prejudice. :-)
         -- Tom Christiansen in <559seq$ag1$1 at csnews.cs.colorado.edu>