From mboxrd@z Thu Jan 1 00:00:00 1970 From: Stefano Babic Date: Thu, 22 Sep 2011 11:44:01 +0200 Subject: [U-Boot] [PATCH 2/2] EfikaSB: Add preliminary EfikaSB support In-Reply-To: <1316428941-8957-3-git-send-email-marek.vasut@gmail.com> References: <1316428941-8957-1-git-send-email-marek.vasut@gmail.com> <1316428941-8957-3-git-send-email-marek.vasut@gmail.com> Message-ID: <4E7B0361.8000309@denx.de> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de On 09/19/2011 12:42 PM, Marek Vasut wrote: > Signed-off-by: Marek Vasut > Cc: Stefano Babic > --- 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. > @@ -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. > /* 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 ? 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. Best regards, Stefano Babic -- ===================================================================== DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: office at denx.de =====================================================================