Hi Wolfgang,
Thanks for checking the patch, Please see inline.
On Fri, 09 Apr 2010 18:51:06 -0400, Wolfgang Denk wd@denx.de wrote:
Dear "David Wu",
In message op.vatgy2wjqigx4y@cyprus.local you wrote:
Signed-off-by: David Wu davidwu@arcturusnetworks.com
board/Mercury/ep2500/Makefile | 44 ++++++ board/Mercury/ep2500/config.mk | 23 +++ board/Mercury/ep2500/ep2500.c | 191 +++++++++++++++++++++++++ board/Mercury/ep2500/u-boot.lds | 140 ++++++++++++++++++ include/configs/EP2500.h | 297 +++++++++++++++++++++++++++++++++++++++ 5 files changed, 695 insertions(+), 0 deletions(-) create mode 100644 board/Mercury/ep2500/Makefile create mode 100644 board/Mercury/ep2500/config.mk create mode 100644 board/Mercury/ep2500/ep2500.c create mode 100644 board/Mercury/ep2500/u-boot.lds create mode 100644 include/configs/EP2500.h
Your Subject (= commit message tile) is WAY too long.
Let me know the size of subject line.
Don't try to press everything in a single line.
Entries to MAINTAINERS and MAKEALL are missing.
Will add them in. My patch is actually applied on top of another set of patches from TsiChung.
+void board_get_enetaddr(uchar *enet) +{
- int i;
- unsigned char buff[6] = { 0xff, 0xff, 0xff, 0xff, 0xff, 0xff };
- /* Read MAC address (6 bytes) in EEPROM */
- if (i2c_read
(CONFIG_SYS_I2C_EEPROM_ADDR, MAC_ADDRESS_OFFSET_IN_EEPROM,CONFIG_SYS_I2C_EEPROM_ADDR_LEN, buff, 6) != 0) {puts("Error reading the EEPROM.\n");- }
- /*
* When buff returns all 0xFF the EEPROM has not* been programed with a valid MAC. In this case* we set enet to all 0x00 as 0xFF is not valid* for this usage model.*/- if (buff[0] == 0xff && buff[1] == 0xff && buff[2] == 0xff &&
buff[3] == 0xff && buff[4] == 0xff && buff[5] == 0xff) {We have standard functions for such checks. Please use them.
It would be nice if you can let me know the function. I am grep'ing in u-boot and it's so hard to find it.
for (i = 0; i < 6; i++)enet[i] = 0;- } else {
for (i = 0; i < 6; i++)enet[i] = buff[i];- }
Hm... the whole code makes no sense to me, as neiter all-ones nor all-zeros are legal MAC addresses.
All ones are valid broadcast MAC address. It is just not valid for source MAC address. I want to set to all zeros instead in case of all FFs. Is this against any rules?
+int misc_init_r(void) +{ +#if defined(CONFIG_HARD_I2C) || defined(CONFIG_SOFT_I2C)
- uchar enetaddr[6];
- if (!eth_getenv_enetaddr("ethaddr", enetaddr)) {
board_get_enetaddr(enetaddr);eth_setenv_enetaddr("ethaddr", enetaddr);Please see recent discussion about this topic.
Could you let me know the thread -- I am new here.
+void i2c_init_board(void) +{
- struct fsl_i2c *dev;
- dev = (struct fsl_i2c *)(CONFIG_SYS_IMMR + CONFIG_SYS_I2C_OFFSET);
- if (readb(&dev->sr) & I2C_SR_MBB) {
writeb(0, &dev->cr);writeb(0xa0, &dev->cr);readb(&dev->dr);writeb(0x0, &dev->sr);writeb(0, &dev->cr);writeb(I2C_CR_MEN, &dev->cr);Please add comments what you are doing. Replace 0x0A by a symbolic
You mean comments for "writeb(0xa0, &dev->cr)" -- OK.
name. Use a consistent style (i. e. wither use "0x0" or "0", but don't mix both styles.
+#ifdef CONFIG_MCFFEC +# define CONFIG_ETHADDR 00:00:00:00:00:00 +# define CONFIG_IPADDR 192.168.1.2 +# define CONFIG_NETMASK 255.255.255.0 +# define CONFIG_SERVERIP 192.168.1.1 +# define CONFIG_GATEWAYIP 192.168.1.1 +# define CONFIG_OVERWRITE_ETHADDR_ONCE +/*#define CONFIG_ENV_OVERWRITE */
Please remove this, it will not be accepted.
Are you talking about the last line. Sure I can remove it. Otherwise would you mind tell me the reason? format is wrong, some defines are wrong or ifdef (or use if defined instead)?
Kind regards, David
Best regards,
Wolfgang Denk