From mboxrd@z Thu Jan  1 00:00:00 1970
From: Wolfgang Denk <wd@denx.de>
Date: Wed, 19 Jan 2011 09:49:41 +0100
Subject: [U-Boot] [PATCH 3/3 v3] iMX5: EfikaMX: Preliminary board support
In-Reply-To: <1295379040-11563-3-git-send-email-marek.vasut@gmail.com>
References: <1295379040-11563-1-git-send-email-marek.vasut@gmail.com>
	<1295379040-11563-3-git-send-email-marek.vasut@gmail.com>
Message-ID: <20110119084941.CF9F52FC@gemini.denx.de>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

Dear Marek Vasut,

In message <1295379040-11563-3-git-send-email-marek.vasut@gmail.com> you wrote:
> Supported:
> MMC
> IDE
> PMIC
> SPI flash
> LEDs
...
> +/******************************************************************************
> + * Compile-time error checking
> + ******************************************************************************/

Incorrect multiline comment style.

I really do not understand why you submit this again, after we
discussed this in lenght both by e-main and on IRC.

Please fix it globally, the code will not go in as is.


> +/******************************************************************************
> + * Board identification
> + ******************************************************************************/
> +static u32 board_rev;

Be careful!!  This way board_rev will be located in bss, and BSS data
MUST NOT be accessed before relocation, but it appears you to this.

This must be fixed.


> +int board_init(void)
> +{
> +	gd->bd->bi_arch_number = MACH_TYPE_MX51_LANGE51;
> +	gd->bd->bi_boot_params = PHYS_SDRAM_1 + 0x100;

Is this the correct MACH_TYPE for the EfikaMX board?


> +/*
> + * Bootloader Components Configuration
> + */
> +#define CONFIG_CMD_SPI
> +#define CONFIG_CMD_SF
> +#define CONFIG_CMD_MMC
> +#define	CONFIG_CMD_FAT
> +#define	CONFIG_CMD_IDE
> +#undef	CONFIG_CMD_IMLS

PLease be consistent - either always use space after #define
(recommended), or always use TAB, but do not mix both styles.

> +/*
> + * ATAG setup
> + */
> +#define CONFIG_CMDLINE_TAG		1	/* enable passing of ATAGs */
> +#define CONFIG_REVISION_TAG		1
> +#define CONFIG_SETUP_MEMORY_TAGS	1
> +#define CONFIG_INITRD_TAG		1

Please omit all these '1'.  Please fix globally...

> +#define CONFIG_HARD_SPI			1
> +#define CONFIG_MXC_SPI			1

... i.e. here and in similar places, too.


Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Calm down, it's *__only* ones and zeroes.