[U-Boot] [PATCH v3 3/3] IGEP v2 support

Thu May 6 00:33:23 CEST 2010

Dear Enric Balletbo i Serra,

In message <1271234190-2432-4-git-send-email-eballetbo at gmail.com> you wrote:
> From: Enric Balletbo i Serra <eballetbo at iseebcn.com>
> 
> This patch adds support for the IGEP v2 board.
> 
> The IGEP v2 board is a low-cost, fan-less and industrial temperature
> range single board computer that unleashes laptop-like performance and
> expandability without the bulk, expense, or noise of typical desktop
> machines. Its architecture shares much in common with other OMAP3 boards.
> 
> Signed-off-by: Enric Balletbo i Serra <eballetbo at iseebcn.com>
> ---
>  MAINTAINERS                      |    4 +
>  Makefile                         |    3 +
>  board/isee/igep0020/Makefile     |   49 +++++
>  board/isee/igep0020/config.mk    |   34 ++++
>  board/isee/igep0020/igep0020.c   |  134 +++++++++++++
>  board/isee/igep0020/igep0020.h   |  399 ++++++++++++++++++++++++++++++++++++++
>  include/configs/omap3_igep0020.h |  245 +++++++++++++++++++++++
>  7 files changed, 868 insertions(+), 0 deletions(-)
>  create mode 100644 board/isee/igep0020/Makefile
>  create mode 100644 board/isee/igep0020/config.mk
>  create mode 100644 board/isee/igep0020/igep0020.c
>  create mode 100644 board/isee/igep0020/igep0020.h
>  create mode 100644 include/configs/omap3_igep0020.h

MAKEALL entry missing.

> diff --git a/MAINTAINERS b/MAINTAINERS
> index 94839ce..19da604 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -525,6 +525,10 @@ Stefano Babic <sbabic at denx.de>
>  	polaris		xscale
>  	trizepsiv	xscale
>  
> +Enric Balletbo i Serra <eballetbo at gmail.com>
> +
> +	omap3_igep0020	ARM CORTEX-A8 (OMAP3530 SoC)
> +
>  Dirk Behme <dirk.behme at gmail.com>
>  
>  	omap3_beagle	ARM CORTEX-A8 (OMAP3530 SoC)
> diff --git a/Makefile b/Makefile
> index 412e359..90f8c29 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -3161,6 +3161,9 @@ omap3_overo_config :	unconfig
>  omap3_evm_config :	unconfig
>  	@$(MKCONFIG) $(@:_config=) arm arm_cortexa8 evm ti omap3
>  
> +omap3_igep0020_config :	unconfig
> +	@$(MKCONFIG) $(@:_config=) arm arm_cortexa8 igep0020 isee omap3
> +
>  omap3_pandora_config :	unconfig
>  	@$(MKCONFIG) $(@:_config=) arm arm_cortexa8 pandora NULL omap3

Make target names shall be identical to board names. Please use
"igep0020_config" here. [Yes, I know, bad examples exist. Please
ignore these.]

> diff --git a/board/isee/igep0020/config.mk b/board/isee/igep0020/config.mk
> new file mode 100644
> index 0000000..a05f782
> --- /dev/null
> +++ b/board/isee/igep0020/config.mk
...
> +# For use with external or internal boots.
> +TEXT_BASE = 0x8ff00000
> +

No trailing empty lines, please.

> diff --git a/board/isee/igep0020/igep0020.c b/board/isee/igep0020/igep0020.c
> new file mode 100644
> index 0000000..a9d9deb
> --- /dev/null
> +++ b/board/isee/igep0020/igep0020.c
> @@ -0,0 +1,134 @@
...
> +/*
> + * Routine: board_init
> + * Description: Early hardware init.
> + */
> +int board_init(void)
> +{
> +	DECLARE_GLOBAL_DATA_PTR;

This will not work. DECLARE_GLOBAL_DATA_PTR must be on file levcel,
not on function level.

> diff --git a/board/isee/igep0020/igep0020.h b/board/isee/igep0020/igep0020.h
> new file mode 100644
> index 0000000..e5e0ac9
> --- /dev/null
> +++ b/board/isee/igep0020/igep0020.h
...
> +/* MUX_VAL(CP(MMC2_DAT3),		(IEN  | PTD | DIS | M4)) GPIO_135 (GPIO-Based CS) */\
> + \
> + MUX_VAL(CP(CAM_HS),		(IDIS | PTD | DIS | M4)) /* GPIO_94 - PDN (Rev. B) */\
> + MUX_VAL(CP(CAM_VS),		(IDIS | PTD | DIS | M4)) /* GPIO_95 - RESET_N_W (Rev. B) */\
> + \
> + MUX_VAL(CP(MMC2_DAT4),		(IDIS | PTD | DIS | M4)) /* GPIO_136 */\
> + MUX_VAL(CP(MMC2_DAT5),		(IDIS | PTD | DIS | M4)) /* GPIO_137 - RESET_N_B */\
> + MUX_VAL(CP(MMC2_DAT6),		(IDIS | PTD | DIS | M4)) /* GPIO_138 - PDN (Rev. C)  */\
> + MUX_VAL(CP(MMC2_DAT7),		(IDIS | PTD | DIS | M4)) /* GPIO_139 - RESET_N_W (Rev. C) */\

Lines too long. Please fix globally.

> diff --git a/include/configs/omap3_igep0020.h b/include/configs/omap3_igep0020.h
> new file mode 100644
> index 0000000..0b2564a
> --- /dev/null
> +++ b/include/configs/omap3_igep0020.h

File name should be include/configs/igep0020.h

> +/* Clock Defines */
> +#define V_OSCK			26000000	/* Clock output from T2 */
> +#define V_SCLK			(V_OSCK >> 1)
> +
> +#undef CONFIG_USE_IRQ				/* no support for IRQs */

Please do not #undef what is not defined anyway.

> +#define CONFIG_CMD_EXT2		/* EXT2 Support			*/
> +#define CONFIG_CMD_FAT		/* FAT support			*/
> +#define CONFIG_CMD_I2C		/* I2C serial bus support	*/
> +#define CONFIG_CMD_MMC		/* MMC support			*/
> +#define CONFIG_CMD_ONENAND	/* ONENAND support		*/
> +#define CONFIG_CMD_NET		/* bootp, tftpboot, rarpboot	*/
> +#define CONFIG_CMD_DHCP
> +#define CONFIG_CMD_PING
> +#define CONFIG_CMD_NFS		/* NFS support			*/

Maybe you want to sort the list?

> +#undef CONFIG_CMD_FLASH		/* flinfo, erase, protect	*/
> +#undef CONFIG_CMD_FPGA		/* FPGA configuration Support	*/
> +#undef CONFIG_CMD_IMI		/* iminfo			*/
> +#undef CONFIG_CMD_IMLS		/* List all found images	*/
> +#undef CONFIG_CMD_JFFS2		/* JFFS2 Support		*/
> +#undef CONFIG_CMD_NAND	  	/* NAND support			*/

iminfo is a very useful command. Are you sure you want to disablke it?

Is there neither NOR nor NAND flash on your board? What are you
booting from?

> +/* Environment information */
> +#define CONFIG_BOOTDELAY		3
> +#define CONFIG_EXTRA_ENV_SETTINGS	"\0"

That's bogus. Please delete.

> +/*
> + * FLASH and environment organization
> + */
> +
> +#define PISMO1_ONEN_SIZE		GPMC_SIZE_128M /* Configure the PISMO */
> +
> +#define CONFIG_SYS_FLASH_BASE		boot_flash_base

Hm... FLASH_BASE when there is no flash there?

> +#define CONFIG_ENV_SIZE			(512 << 10)	/* 512 KiB */

Do you really need that much? That will make CRC caclulation slow...

> +/* Monitor at start of flash */
> +#define CONFIG_SYS_MONITOR_BASE		CONFIG_SYS_FLASH_BASE

Without flash? Really?

> +#ifndef __ASSEMBLY__
> +extern struct gpmc *gpmc_cfg;
> +extern unsigned int boot_flash_base;
> +extern volatile unsigned int boot_flash_env_addr;
> +extern unsigned int boot_flash_off;
> +extern unsigned int boot_flash_sec;
> +extern unsigned int boot_flash_type;
> +#endif

This has no place in the board config file. Please move elsewhere.

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
Fools ignore complexity. Pragmatists suffer it. Some  can  avoid  it.
Geniuses remove it.
     - Perlis's Programming Proverb #58, SIGPLAN Notices, Sept.  1982