[U-Boot] [PATCH 6/9] SPEAr300 SoC support added

[Wolfgang Denk]

Dear Vipin KUMAR,

In message <1260955110-5656-7-git-send-email-vipin.kumar at st.com> you wrote:
> 
> Signed-off-by: Vipin <vipin.kumar at st.com>
...
> diff --git a/board/spear/spear300/config.mk b/board/spear/spear300/config.mk
> new file mode 100755
> index 0000000..90a6cf9
> --- /dev/null
> +++ b/board/spear/spear300/config.mk
...
> +PLATFORM_RELFLAGS += -DCONFIG_SPEAR_USBTTY
> +endif
> +

Please do not add trailing empty lines. [Please check globally.]

> diff --git a/board/spear/spear300/spr300_board.c b/board/spear/spear300/spr300_board.c
> new file mode 100755
> index 0000000..97de0be
> --- /dev/null
> +++ b/board/spear/spear300/spr300_board.c

Usually the file name is the same as the board config name.

> +int board_nand_init(struct nand_chip *nand)
> +{
> +	struct misc_regs *const misc_regs_p =
> +		(struct misc_regs *)CONFIG_SPEAR_MISCBASE;
> +
> +	if (((readl(&misc_regs_p->auto_cfg_reg) & MISC_SOCCFGMSK) ==
> +				MISC_SOCCFG30) ||
> +	   ((readl(&misc_regs_p->auto_cfg_reg) & MISC_SOCCFGMSK) ==
> +				MISC_SOCCFG31)) {
> +
> +		return spear_nand_init(nand);
> +	} else {
> +		return -1;

"else" not needed. Unindent the "return -1;".

> diff --git a/include/configs/spear300.h b/include/configs/spear300.h
> new file mode 100755
> index 0000000..59b1f45
> --- /dev/null
> +++ b/include/configs/spear300.h
...
> +/*
> + * Ethernet MAC driver configuration
> + */
> +/* #define CONFIG_SPEARMAC */
> +/* #define CONFIG_ETHAUTONEG */
> +/* #define CONFIG_ETHDEBUG */

Don't add dead code.

> +#define CONFIG_SYS_FIFO_BASE			(0xE1000800)
> +#define CONFIG_USBD_PRODUCT_NAME		"SPEAr SoC"
> +#define CONFIG_USBD_MANUFACTURER		"ST Microelectronics"
> +
> +#define CONFIG_EXTRA_ENV_SETTINGS               "usbtty=cdc_acm"

Vertial alignment by TABs only, please.

> +/* #define CONFIG_SYS_I2C_MULTI_EEPROMS */
> +/* #define CONFIG_SYS_I2C_EEPROM_ADDR_LEN        0x1 */

Don't add dead code. Please fix globally.

> +/*
> + * SMI driver configuration
> + */
> +#define CONFIG_SPEARSMI
> +#undef  CONFIG_SYS_NO_FLASH

Don't undef what is not defined anyway.

> +#define CONFIG_SYS_HZ				(26000)

CONFIG_SYS_HZ _must_ be defined as 1000.

> +/* #define CONFIG_ETHADDR			00:11:22:33:44:55 */
> +
> +/* #define CONFIG_NETMASK			255.255.255.0 */
> +/* #define CONFIG_IPADDR			192.168.1.10 */
> +/* #define CONFIG_SERVERIP			192.168.1.1 */
> +/* #define CONFIG_GATEWAYIP			192.168.1.1 */

Get rid of all this stuff.

> +#define CONFIG_ENV_ADDR				(CONFIG_SYS_MONITOR_BASE + \
> +						CONFIG_SYS_MONITOR_LEN)
> +
> +#define CONFIG_BOOTARGS				"console=ttyS0 mem=128M "  \

Lines too long.

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
But it's real. And if it's real it can be affected ... we may not  be
able  to break it, but, I'll bet you credits to Navy Beans we can put
a dent in it.
	-- deSalle, "Catspaw", stardate 3018.2