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

Wolfgang Denk
2011-01-20T12:03:19Z

Dear Stefano Babic,

In message <4D381237.9040309@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@csnews.cs.colorado.edu>