| Message ID | 1312299792-16415-6-git-send-email-nagabhushana.netagunte@ti.com |
|---|---|
| State | Changes Requested |
| Headers | show |
Dear nagabhushana.netagunte@ti.com, In message <1312299792-16415-6-git-send-email-nagabhushana.netagunte@ti.com> you wrote: > From: Nagabhushana Netagunte <nagabhushana.netagunte@ti.com> > > add misc_int_r function to read the mac address from SPI flash > if env variable ethaddr is not set. > > Signed-off-by: Prakash PM <prakash.pm@ti.com> > Signed-off-by: Nagabhushana Netagunte <nagabhushana.netagunte@ti.com> > --- > board/davinci/da8xxevm/da850evm.c | 66 +++++++++++++++++++++++++++++++++++++ > include/configs/da850evm.h | 1 + > 2 files changed, 67 insertions(+), 0 deletions(-) Ummm... I remember I have seent his code before. Right: http://article.gmane.org/gmane.comp.boot-loaders.u-boot/104458 So this is a repost, but you do not mark it as such (no "v2" or similar in the Subject), nor do you provide any kind of change log or explanations what has been changed compared to the previous version. Please follow the rules when posting patches! Nobody here on the list has the time and resources to compare different patch versions just because the poster does not bother to provide the mandatory information. Normally, all such patches are simply NAKed ! Best regards, Wolfgang Denk
Dear nagabhushana.netagunte@ti.com, In message <1312299792-16415-6-git-send-email-nagabhushana.netagunte@ti.com> you wrote: > From: Nagabhushana Netagunte <nagabhushana.netagunte@ti.com> > > add misc_int_r function to read the mac address from SPI flash > if env variable ethaddr is not set. > > Signed-off-by: Prakash PM <prakash.pm@ti.com> > Signed-off-by: Nagabhushana Netagunte <nagabhushana.netagunte@ti.com> ... > +err_read: > + /* cannot call free currently since the free function calls free() for > + * spi_flash structure though it is not directly allocated through > + * malloc() > + */ Incorrect multiline comment style. > +int misc_init_r(void) > +{ > + uint8_t addr[10]; > + uchar enetaddr[6]; > + int ret; > + > + printf("ARM Clock : %d Hz\n", clk_get(DAVINCI_ARM_CLKID)); Please do not add unneeded output to the a;lways printed boot messages. If you want tto provide this information to the user then add it as part of the "clock" command. > + sprintf((char *)enetaddr, "%02x:%02x:%02x:%02x:%02x:%02x", > + addr[0], addr[1], addr[2], addr[3], addr[4], addr[5]); Why don't you simply use "%pM" here? Best regards, Wolfgang Denk
Denk, Sorry for missing upstream syntax here, will follow relevant rules. Regards, Nag On Tue, Aug 02, 2011 at 21:50:02, Wolfgang Denk wrote: > Dear nagabhushana.netagunte@ti.com, > > In message <1312299792-16415-6-git-send-email-nagabhushana.netagunte@ti.com> you wrote: > > From: Nagabhushana Netagunte <nagabhushana.netagunte@ti.com> > > > > add misc_int_r function to read the mac address from SPI flash if env > > variable ethaddr is not set. > > > > Signed-off-by: Prakash PM <prakash.pm@ti.com> > > Signed-off-by: Nagabhushana Netagunte <nagabhushana.netagunte@ti.com> > > --- > > board/davinci/da8xxevm/da850evm.c | 66 +++++++++++++++++++++++++++++++++++++ > > include/configs/da850evm.h | 1 + > > 2 files changed, 67 insertions(+), 0 deletions(-) > > Ummm... I remember I have seent his code before. Right: > > http://article.gmane.org/gmane.comp.boot-loaders.u-boot/104458 > > > So this is a repost, but you do not mark it as such (no "v2" or similar in the Subject), nor do you provide any kind of change log or explanations what has been changed compared to the previous version. > > > Please follow the rules when posting patches! Nobody here on the list has the time and resources to compare different patch versions just because the poster does not bother to provide the mandatory information. Normally, all such patches are simply NAKed ! > > > 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@denx.de Diplomacy is the art of saying "nice doggy" until you can find a rock. >
Denk, Thanks for your response. My response is in-lined. Regards, Nag On Tue, Aug 02, 2011 at 21:53:42, Wolfgang Denk wrote: > Dear nagabhushana.netagunte@ti.com, > > In message <1312299792-16415-6-git-send-email-nagabhushana.netagunte@ti.com> you wrote: > > From: Nagabhushana Netagunte <nagabhushana.netagunte@ti.com> > > > > add misc_int_r function to read the mac address from SPI flash if env > > variable ethaddr is not set. > > > > Signed-off-by: Prakash PM <prakash.pm@ti.com> > > Signed-off-by: Nagabhushana Netagunte <nagabhushana.netagunte@ti.com> > ... > > +err_read: > > + /* cannot call free currently since the free function calls free() for > > + * spi_flash structure though it is not directly allocated through > > + * malloc() > > + */ > > Incorrect multiline comment style. > > > +int misc_init_r(void) > > +{ > > + uint8_t addr[10]; > > + uchar enetaddr[6]; > > + int ret; > > + > > + printf("ARM Clock : %d Hz\n", clk_get(DAVINCI_ARM_CLKID)); > > Please do not add unneeded output to the a;lways printed boot messages. If you want tto provide this information to the user then add it as part of the "clock" command. > Agree with you. It will be great if you can point me to the command you are referring to as I failed to locate the same. > > + sprintf((char *)enetaddr, "%02x:%02x:%02x:%02x:%02x:%02x", > > + addr[0], addr[1], addr[2], addr[3], addr[4], addr[5]); > > Why don't you simply use "%pM" here? > Will make suggested change. > > 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@denx.de The flow chart is a most thoroughly oversold piece of program docu- > mentation. -- Frederick Brooks, "The Mythical Man Month" >
diff --git a/board/davinci/da8xxevm/da850evm.c b/board/davinci/da8xxevm/da850evm.c index d3d965c..ffded78 100644 --- a/board/davinci/da8xxevm/da850evm.c +++ b/board/davinci/da8xxevm/da850evm.c @@ -25,10 +25,13 @@ #include <i2c.h> #include <net.h> #include <netdev.h> +#include <spi.h> +#include <spi_flash.h> #include <asm/arch/hardware.h> #include <asm/arch/emif_defs.h> #include <asm/arch/emac_defs.h> #include <asm/io.h> +#include <asm/errno.h> #include <asm/arch/davinci_misc.h> DECLARE_GLOBAL_DATA_PTR; @@ -384,4 +387,67 @@ int board_eth_init(bd_t *bis) return 0; } + #endif /* CONFIG_DRIVER_TI_EMAC */ + +#define CFG_MAC_ADDR_SPI_BUS 0 +#define CFG_MAC_ADDR_SPI_CS 0 +#define CFG_MAC_ADDR_SPI_MAX_HZ CONFIG_SF_DEFAULT_SPEED +#define CFG_MAC_ADDR_SPI_MODE SPI_MODE_3 + +#define CFG_MAC_ADDR_OFFSET (flash->size - SZ_64K) + +static int get_mac_addr_spi(u8 *addr) +{ + int ret; + struct spi_flash *flash; + + flash = spi_flash_probe(CFG_MAC_ADDR_SPI_BUS, CFG_MAC_ADDR_SPI_CS, + CFG_MAC_ADDR_SPI_MAX_HZ, CFG_MAC_ADDR_SPI_MODE); + if (!flash) { + printf(" Error - unable to probe SPI flash.\n"); + ret = -1; + goto err_probe; + } + + ret = spi_flash_read(flash, CFG_MAC_ADDR_OFFSET, 6, addr); + if (ret) { + printf("Error - unable to read MAC address from SPI flash.\n"); + goto err_read; + } + +err_read: + /* cannot call free currently since the free function calls free() for + * spi_flash structure though it is not directly allocated through + * malloc() + */ +err_probe: + return ret; +} + +int misc_init_r(void) +{ + uint8_t addr[10]; + uchar enetaddr[6]; + int ret; + + printf("ARM Clock : %d Hz\n", clk_get(DAVINCI_ARM_CLKID)); + + if (!eth_getenv_enetaddr("ethaddr", enetaddr)) { + /* Set Ethernet MAC address from EEPROM */ + ret = get_mac_addr_spi(addr); + if (ret != 0) + return -EINVAL; + + if (is_multicast_ether_addr(addr) || is_zero_ether_addr(addr)) { + printf("Invalid MAC address read.\n"); + return -EINVAL; + } + sprintf((char *)enetaddr, "%02x:%02x:%02x:%02x:%02x:%02x", + addr[0], addr[1], addr[2], addr[3], addr[4], addr[5]); + + eth_setenv_enetaddr("ethaddr", enetaddr); + } + + return 0; +} diff --git a/include/configs/da850evm.h b/include/configs/da850evm.h index de22ffa..ede428b 100644 --- a/include/configs/da850evm.h +++ b/include/configs/da850evm.h @@ -161,6 +161,7 @@ /* * U-Boot general configuration */ +#define CONFIG_MISC_INIT_R #define CONFIG_BOOTFILE "uImage" /* Boot file name */ #define CONFIG_SYS_PROMPT "U-Boot > " /* Command Prompt */ #define CONFIG_SYS_CBSIZE 1024 /* Console I/O Buffer Size */