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

[Stefano Babic]

On 09/19/2011 12:42 PM, Marek Vasut wrote:
> Signed-off-by: Marek Vasut <marek.vasut at gmail.com>
> Cc: Stefano Babic <sbabic at 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
=====================================================================