[U-Boot] [PATCH 2/2] TT-01: add basic board support for HALE TT-01

From: Wolfgang Denk <wd@denx.de>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH 2/2] TT-01: add basic board support for HALE TT-01
Date: Thu, 22 Sep 2011 16:08:33 +0200	[thread overview]
Message-ID: <20110922140833.CF0B6140796D@gemini.denx.de> (raw)
In-Reply-To: <1316693575-20726-3-git-send-email-helmut.raiger@hale.at>

Dear Helmut Raiger,

In message <1316693575-20726-3-git-send-email-helmut.raiger@hale.at> you wrote:
> This adds basic board support for TT-01 based on
> the bluetechnix i.MX31 SOM. Currently only NOR-Flash
> boot is supported.
> 
> Signed-off-by: Helmut Raiger <helmut.raiger@hale.at>
> ---
>  Makefile                  |    4 +
>  board/hale/tt01/Makefile  |   53 +++++++++
>  board/hale/tt01/config.mk |    1 +
>  board/hale/tt01/tt01.c    |   98 ++++++++++++++++
>  include/configs/tt01.h    |  283 +++++++++++++++++++++++++++++++++++++++++++++
>  5 files changed, 439 insertions(+), 0 deletions(-)
>  create mode 100644 board/hale/tt01/Makefile
>  create mode 100644 board/hale/tt01/config.mk
>  create mode 100644 board/hale/tt01/tt01.c
>  create mode 100644 include/configs/tt01.h

Entry to MAINTAINERS missing.

> diff --git a/Makefile b/Makefile
> index e9ba6a4..aaed9e6 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -908,6 +908,10 @@ mx31pdk_nand_config	: unconfig
>  	fi
>  	@$(MKCONFIG) -n $@ -a mx31pdk arm arm1136 mx31pdk freescale mx31
>  
> +tt01_config : unconfig
> +	@mkdir -p $(obj)include
> +	@$(MKCONFIG) -n $@ -a tt01 arm arm1136 tt01 hale mx31

We don't add boards to Makefile any more.  Please add instead to
boards.cfg 

...
> +	/* start CSPI3 clock (3 = always on except if PLL off) */
> +	writel(readl(CCM_CGR0) | (3 << 16), CCM_CGR0);

Please use setbits() instead.

> +int board_early_init_f(void)
> +{
> +	/* CS4: FPGA incl. network controller */
> +	__REG(CSCR_U(4)) = 0x0000dcf6;
> +	__REG(CSCR_L(4)) = 0x444A4541;
> +	__REG(CSCR_A(4)) = 0x44443302;

We don't allow such accesses any more.  Please use proper I/O
accessors instead.

And please introduce some symbolic constants for the magic numbers,
and add comments whatthese are.

> +int board_init(void)
> +{
> +	/* board id for linux, not in mach-types.h yet */
> +	gd->bd->bi_arch_number = 3726;		/* MACH_TYPE_TT01 = 3726 */

NAK.  Please use standard approach for handling the mach_id.

> +/*-----------------------------------------------------------------------
> + * Physical Memory Map:
...

Incorrect multiline comment style.  Please fix globally.

...
> +#define CONFIG_SYS_MONITOR_BASE	CONFIG_SYS_FLASH_BASE
> +#define CONFIG_SYS_MONITOR_LEN		(1024 * 1024)

Are you really, really sure about this?  I doubt it...

> +#define	CONFIG_ENV_IS_IN_FLASH	1

Please do not define values for macros that select features only.
Please fix globally.

> +#include <config_cmd_default.h>
> +
> +/* this is currently not supported, mxc_nand.c is too incomplete for it */
> +#undef CONFIG_CMD_MTDPARTS

Please do not undef what is not defined in the first place.

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
You go slow, be gentle. It's no one-way street -- you  know  how  you
feel and that's all. It's how the girl feels too. Don't press. If the
girl feels anything for you at all, you'll know.
	-- Kirk, "Charlie X", stardate 1535.8