Patchwork [U-Boot,05/12] da850: add support to read mac address from spi flash

login
register
mail settings
Submitter nagabhushana.netagunte@ti.com
Date Aug. 2, 2011, 3:43 p.m.
Message ID <1312299792-16415-6-git-send-email-nagabhushana.netagunte@ti.com>
Download mbox | patch
Permalink /patch/107957/
State Changes Requested
Headers show

Comments

nagabhushana.netagunte@ti.com - Aug. 2, 2011, 3:43 p.m.
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(-)
Wolfgang Denk - Aug. 2, 2011, 4:20 p.m.
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
Wolfgang Denk - Aug. 2, 2011, 4:23 p.m.
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
nagabhushana.netagunte@ti.com - Aug. 9, 2011, 2:05 p.m.
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.
>
nagabhushana.netagunte@ti.com - Aug. 9, 2011, 2:08 p.m.
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"
>

Patch

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