[U-Boot] [PATCH 03/10] ARM Add New Board GEC2410

[Wolfgang Denk]

Dear "Hui.Tang",

In message <abdf4f47f4822a58409a4b819f470dc1a540774c.1256898456.git.zetalabs at gmail.com> you wrote:
> Add GEC2410 default config.
...
> diff --git a/include/configs/gec2410.h b/include/configs/gec2410.h
> new file mode 100644
> index 0000000..98c8040
> --- /dev/null
> +++ b/include/configs/gec2410.h
...
> +#undef CONFIG_USE_IRQ			/* we don't need IRQ/FIQ stuff */
> +
> +#undef CONFIG_SKIP_RELOCATE_UBOOT

Don't #undef what is not defined anyway.


> +/*#define CONFIG_BOOTARGS	"root=ramfs devfs=mount console=ttySA0,9600" */

Don;t add dead code.

> +#define CONFIG_ETHADDR		08:00:3e:26:0a:5b
> +#define CONFIG_NETMASK          255.255.255.0
> +#define CONFIG_IPADDR		192.168.1.10
> +#define CONFIG_SERVERIP		192.168.1.254

As Ben already commented: strict NAK on this.

> +/*#define CONFIG_BOOTFILE	"elinos-lart" */
> +/*#define CONFIG_BOOTCOMMAND	"tftp; bootm" */

Don';t add dead code - please fix globally.

> +/* what's this ? it's not used anywhere */
> +#define CONFIG_KGDB_SER_INDEX	1		/* which serial port to use */

Then why do you add it?

> +/* the PWM TImer 4 uses a counter of 15625 for 10 ms, so we need */
> +/* it to wrap 100 times (total 1562500) to get 1 sec. */
> +#define CONFIG_SYS_HZ			1562500

NAK.

CONFIG_SYS_HZ is a constant and must be defined as 1000 on all
systems.


> +/* Use drivers/cfi_flash.c, even though the flash is not CFI-compliant	*/

????

Didn't you add your own driver board/gec/gec2410/flash.c ?

So does the CFI driver work, or not? And if not, then why exactly not?

> +#define CONFIG_ENV_SIZE		0x4000	/* Total Size of Environment Sector */

Wrong comment? CONFIG_ENV_SIZE is the size of the envrionment, NOT
the sector size.

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
Anything free is worth what you pay for it.