
Dear John Rigby,
In message 1301761196-26072-5-git-send-email-john.rigby@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