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

Kuo-Jung Su
2014-03-26T07:22:56Z

2014-03-26 14:47 GMT+08:00 Wolfgang Denk <wd@denx.de>:
> 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.
>

Got it, thanks

>> +# 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?
>

By stealing the OpenMoko's vendor id & product id, I could directly use the
OpenMoko dfu-util without any modifications.

And unfortunately we don't have a registered OUI for Faraday Technology, so
I'll remove this ID in the next release.

>> +/* 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.
>

Got it, thanks

>> +#define CONFIG_CMD_AUTOSCRIPT /* support autoscript */
>
> This has been removed years ago. Please use CONFIG_CMD_SOURCE
> instead.
>

Got it, thanks

--
Best wishes,
Kuo-Jung Su