[U-Boot] [PATCH 2/2] EfikaSB: Add preliminary EfikaSB support

Stefano Babic
2011-09-22T09:44:01Z

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.

> @@ -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
=====================================================================