On Thursday, September 22, 2011 11:44:01 AM Stefano Babic wrote:
On 09/19/2011 12:42 PM, Marek Vasut wrote:
Signed-off-by: Marek Vasut marek.vasut@gmail.com Cc: Stefano Babic sbabic@denx.de
Hi Marek,
sorry for late review. I missed this patch...
Only a couple of minor issues:
+#if !defined(CONFIG_MACH_EFIKAMX) && !defined(CONFIG_MACH_EFIKASB) +#error "Missing CONFIG_MACH_EFIKAMX or CONFIG_MACH_EFIKASB in config.h!" +#endif
This seems not necessary because CONFIG_MACH_EFIKA* is set at the build time with the option in boards.cfg. With a correct boards.cfg, we cannot get this error.
Well once someone adds another efika, he can forget about it. And there's mx53 efika in the works.
@@ -284,9 +304,9 @@ int board_mmc_init(bd_t *bis)
int ret;
/* SDHC1 is used on all revisions, setup control pins first */
- mxc_request_iomux(MX51_PIN_GPIO1_0,
mxc_request_iomux(EFIKA_SD1_CD,
IOMUX_CONFIG_ALT0 | IOMUX_CONFIG_SION);
- mxc_iomux_set_pad(MX51_PIN_GPIO1_0,
mxc_iomux_set_pad(EFIKA_SD1_CD,
PAD_CTL_DRV_HIGH | PAD_CTL_HYS_ENABLE | PAD_CTL_PUE_KEEPER | PAD_CTL_100K_PU | PAD_CTL_ODE_OPENDRAIN_NONE |
@@ -298,11 +318,13 @@ int board_mmc_init(bd_t *bis)
PAD_CTL_100K_PU | PAD_CTL_ODE_OPENDRAIN_NONE | PAD_CTL_SRE_FAST);
- gpio_direction_input(IOMUX_TO_GPIO(MX51_PIN_GPIO1_0));
gpio_direction_input(IOMUX_TO_GPIO(EFIKA_SD1_CD));
gpio_direction_input(IOMUX_TO_GPIO(MX51_PIN_GPIO1_1));
+#ifndef CONFIG_MACH_EFIKASB
It is better to have the check consistent in the file. You mix #ifdef CONFIG_MACH_EFIKAMX with #ifndef CONFIG_MACH_EFIKASB, that is the same.
It expresses the intention much better IMO. And see above -- mx53 efika in the works.
/* Internal SDHC1 IOMUX + SDHC2 IOMUX on old boards */ if (get_efika_rev() < EFIKAMX_BOARD_REV_12) {
+#endif
At the moment, the #ifdef seems redundant. You hard-code the efikasb revision to zero, and then get_efika_rev() is always smaller as EFIKAMX_BOARD_REV_12. What about to introduce a macro such as board_is() to increase readability ?
Yes it would, but it'd also increase code size.
This if statement really means:
if (board_is(EFIKASB) || (board_is(EFIKAMX) && get_efika_rev() < EFIKAMX_BOARD_REV_12))
/*
- Board initialization
@@ -616,7 +661,11 @@ int board_early_init_f(void)
int board_init(void) {
+#ifdef CONFIG_MACH_EFIKAMX
gd->bd->bi_arch_number = MACH_TYPE_MX51_EFIKAMX;
+#else
- gd->bd->bi_arch_number = MACH_TYPE_MX51_EFIKASB;
+#endif
gd->bd->bi_boot_params = PHYS_SDRAM_1 + 0x100;
Can we use the new rule to set up the MACH-ID ? You can move the #ifdef inside config.h and let common code to set it.
Can we do that in a subsequent patch ?
Best regards, Stefano Babic