Dear John Rigby,
In message <1301761196-26072-5-git-send-email-john.ri...@linaro.org> you wrote:
> Based on ST-Ericsson private git repo.
> Plus changes to use arm_pl180_mmci driver.
>
> This board support requires the vexpress mmc driver patch set from
> Matt Waddel.
All these comments are not really useful as commit message. YOu might
add these as comments.
For the commit message, I'd rather like to see a short description of
the board, and which board features are supported by this port.
...
> +extern int prcmu_i2c_read(u8 reg, u16 slave);
> +extern int prcmu_i2c_write(u8 reg, u16 slave, u8 reg_data);
Please move prototype declarations to approproate header file.
> +int board_mmc_init(bd_t *bd)
> +{
> + if (u8500_mmci_board_init())
> + return -ENODEV;
> +
> + arm_pl180_mmci_init();
Error. You ignore the return code from arm_pl180_mmci_init() here.
Please fix.
...
> + /* check ARM clock source */
> + reg = readl(PRCM_ARM_CHGCLKREQ_REG);
> + printf("A9 running on ");
> + if (reg & 1)
> + printf("external clock");
> + else
> + printf("ARM PLL");
> + printf("\n");
Something like
printf("A9 running on %s\n",
(reg & 1) ? "external clock" : "ARM PLL";
would probably result in smaller (and faster) code.
The same applies to other places, too.
...
> +#define CONFIG_SYS_MEMTEST_START 0x00000000
> +#define CONFIG_SYS_MEMTEST_END 0x1FFFFFFF
Has this actually been tested?
> +#define CONFIG_BOARD_EARLY_INIT_F 1
> +#define BOARD_LATE_INIT 1
...
> +#define CONFIG_MMC 1
> +#define CONFIG_GENERIC_MMC 1
> +#define CONFIG_DOS_PARTITION 1
Please omit the '1' in all defines that select features only.
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: w...@denx.de
"More software projects have gone awry for lack of calendar time than
for all other causes combined."
- Fred Brooks, Jr., _The Mythical Man Month_
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
