[U-Boot] [PATCH 1/1] ARM Add New Board GEC2410 rev2

[Wolfgang Denk]

Dear "Hui.Tang",

In message <1257476600-13264-1-git-send-email-zetalabs at gmail.com> you wrote:
...
> --- a/Makefile
> +++ b/Makefile
...
> +gec2410_config	:	unconfig
> +	@mkdir -p $(obj)include $(obj)board/gec/gec2410
> +	@mkdir -p $(obj)nand_spl/board/gec/gec2410
> +	@echo "RAM_TEXT = 0x33e00000" >> $(obj)board/gec/gec2410/config.tmp
> +	@$(MKCONFIG) $(@:_config=) arm arm920t gec2410 gec s3c24x0
> +	@echo "CONFIG_NAND_U_BOOT = y" >> $(obj)include/config.mk

As there is only a single Make target, please add RAM_TEXT and
CONFIG_NAND_U_BOOT unconditionally to your config.mk and simplify the
Makefile entry here.

...
> --- /dev/null
> +++ b/board/gec/gec2410/config.mk
...
> +sinclude $(OBJTREE)/board/$(BOARDDIR)/config.tmp
> +
> +ifndef CONFIG_NAND_SPL
> +TEXT_BASE = $(RAM_TEXT)
> +else
> +TEXT_BASE = 0
> +endif

All this seems to be not needed since you have a single, static
configuration anyway.

> +LDSCRIPT := $(SRCTREE)/board/$(BOARDDIR)/u-boot-nand.lds
> +
> +PLATFORM_CPPFLAGS += -I$(TOPDIR)/board/gec/gec2410

This seems to be not needed as only board/gec/gec2410/lowlevel_init.S
will include any file from there, and then they are in the same
directory.

> diff --git a/board/gec/gec2410/gec2410.c b/board/gec/gec2410/gec2410.c
> new file mode 100644
> index 0000000..1f941ad
> --- /dev/null
> +++ b/board/gec/gec2410/gec2410.c
...
> +#define FCLK_SPEED 1
> +
> +#if FCLK_SPEED == 0		/* Fout = 203MHz, Fin = 12MHz for Audio */
> +#define M_MDIV	0xC3
> +#define M_PDIV	0x4
> +#define M_SDIV	0x1
> +#elif FCLK_SPEED == 1		/* Fout = 202.8MHz */
> +#define M_MDIV	0xA1
> +#define M_PDIV	0x3
> +#define M_SDIV	0x1
> +#endif
> +
> +#define USB_CLOCK 1
> +
> +#if USB_CLOCK == 0
> +#define U_M_MDIV	0xA1
> +#define U_M_PDIV	0x3
> +#define U_M_SDIV	0x1
> +#elif USB_CLOCK == 1
> +#define U_M_MDIV	0x48
> +#define U_M_PDIV	0x3
> +#define U_M_SDIV	0x2
> +#endif

Such #defines have no place in the source code. Please move to
appropriate header file.

> +static inline void delay(unsigned long loops)
> +{
> +	__asm__ volatile ("1:\n"
> +	  "subs %0, %1, #1\n"
> +	  "bne 1b" : "=r" (loops) : "0" (loops));
> +}

You probably do not want to do this, or at least not call it delay().

...
> +	/* set up the I/O ports */
> +	/* configure GPA port */
> +	/*====================================*
> +	 * PIN		FUNCTION	VALUE *
> +	 *====================================*

Incorrect multiline comment style. Please check globally.

...
> +SMRDATA:
> +    .word (0+(B1_BWSCON<<4)+(B2_BWSCON<<8)+(B3_BWSCON<<12)	\
> +	+(B4_BWSCON<<16)+(B5_BWSCON<<20)+(B6_BWSCON<<24)+(B7_BWSCON<<28))
> +    .word ((B0_Tacs<<13)+(B0_Tcos<<11)+(B0_Tacc<<8)	\
> +	+(B0_Tcoh<<6)+(B0_Tah<<4)+(B0_Tacp<<2)+(B0_PMC))
> +    .word ((B1_Tacs<<13)+(B1_Tcos<<11)+(B1_Tacc<<8)	\
> +	+(B1_Tcoh<<6)+(B1_Tah<<4)+(B1_Tacp<<2)+(B1_PMC))
> +    .word ((B2_Tacs<<13)+(B2_Tcos<<11)+(B2_Tacc<<8)	\
> +	+(B2_Tcoh<<6)+(B2_Tah<<4)+(B2_Tacp<<2)+(B2_PMC))
> +    .word ((B3_Tacs<<13)+(B3_Tcos<<11)+(B3_Tacc<<8)	\
> +	+(B3_Tcoh<<6)+(B3_Tah<<4)+(B3_Tacp<<2)+(B3_PMC))
> +    .word ((B4_Tacs<<13)+(B4_Tcos<<11)+(B4_Tacc<<8)	\
> +	+(B4_Tcoh<<6)+(B4_Tah<<4)+(B4_Tacp<<2)+(B4_PMC))
> +    .word ((B5_Tacs<<13)+(B5_Tcos<<11)+(B5_Tacc<<8)	\
> +	+(B5_Tcoh<<6)+(B5_Tah<<4)+(B5_Tacp<<2)+(B5_PMC))

Please fix indentation.

> diff --git a/cpu/arm920t/s3c24x0/timer.c b/cpu/arm920t/s3c24x0/timer.c
> index 20cedd4..d463a03 100644
> --- a/cpu/arm920t/s3c24x0/timer.c
> +++ b/cpu/arm920t/s3c24x0/timer.c
> @@ -188,7 +188,8 @@ ulong get_tbclk(void)
>  	tbclk = timer_load_val * 100;
>  #elif defined(CONFIG_SBC2410X) || \
>        defined(CONFIG_SMDK2410) || \
> -      defined(CONFIG_VCMA9)
> +      defined(CONFIG_VCMA9) || \
> +      defined(CONFIG_GEC2410)

Please keep list sorted.

> --- /dev/null
> +++ b/nand_spl/board/gec/gec2410/Makefile
...
> +LDFLAGS	= -Bstatic -T $(nandobj)u-boot.lds -Ttext $(TEXT_BASE) $(PLATFORM_LDFLAGS)

Line too long. Please check globally.

> --- /dev/null
> +++ b/nand_spl/board/gec/gec2410/config.mk
...
> +ifeq ($(debug),1)
> +PLATFORM_CPPFLAGS += -DDEBUG
> +endif

Please remove.


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
It's hard to make a program foolproof because fools are so ingenious.