[U-Boot] [PATCH v11 1/6] arm: add Faraday ARMv5TE cores support

Wolfgang Denk
2014-03-26T06:47:39Z

Dear Kuo-Jung Su,

In message <1395813799-3672-2-git-send-email-dantesu@gmail.com> you wrote:
> From: Kuo-Jung Su <dantesu@faraday-tech.com>
>
> Here is the list of verified cores:
>
> 1. FA606TE (ARMv5TE, no mmu)
> 2. FA626TE (ARMv5TE)
...
> diff --git a/include/configs/faraday-common.h b/include/configs/faraday-common.h
> new file mode 100644
> index 0000000..546c0a9
> --- /dev/null
> +++ b/include/configs/faraday-common.h
...
> +#ifndef CONFIG_ETHADDR
> +#define CONFIG_ETHADDR 00:41:71:00:00:50
> +#endif
> +
> +#ifndef CONFIG_IPADDR
> +#define CONFIG_IPADDR 10.0.0.192
> +#endif
> +
> +#ifndef CONFIG_NETMASK
> +#define CONFIG_NETMASK 255.255.255.0
> +#endif
> +
> +#ifndef CONFIG_SERVERIP
> +#define CONFIG_SERVERIP 10.0.0.128
> +#endif

We do not allow such static network configuration. Especially
assigning the same MAC address to all devices is deadly. Also,
the address is not an officially assigned nor a local one.
Please remove all this code.

> +# endif
> +# ifndef CONFIG_G_DNL_VENDOR_NUM
> +# define CONFIG_G_DNL_VENDOR_NUM 0x1d50 /* OpenMoko */
> +# endif

This looks wrong to me?

> +# ifndef CONFIG_G_DNL_PRODUCT_NUM
> +# define CONFIG_G_DNL_PRODUCT_NUM 0x5119
> +# endif

Is this a valid ID?

> +/* Console I/O Buffer Size */
> +#define CONFIG_SYS_CBSIZE 256
> +
> +/* Max number of command args */
> +#define CONFIG_SYS_MAXARGS 32

You use a large number of args with a tiny console buffer? This looks
suspicious. Please check.

> +#define CONFIG_CMD_AUTOSCRIPT /* support autoscript */

This has been removed years ago. Please use CONFIG_CMD_SOURCE
instead.

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
To program is to be.