diff mbox

[U-Boot,09/12] da850: read MAC address from I2C EEPROM on AM18xx EVM

Message ID 1312299792-16415-10-git-send-email-nagabhushana.netagunte@ti.com
State Changes Requested
Headers show

Commit Message

nagabhushana.netagunte@ti.com Aug. 2, 2011, 3:43 p.m. UTC 
From: Nagabhushana Netagunte <nagabhushana.netagunte@ti.com>

The AM18xx EVM contains MAC address in I2C EEPROM compared
da850/omap-l138 Logic PD EVM which maintains in SPI flash. So this
patch tries to read MAC address from I2C EEPROM, in failure case reads
from SPI flash assuming board to be da850/omap-l138 Logic PDS EVM.

Signed-off-by: Manjunathappa, Prakash <prakash.pm@ti.com>
Signed-off-by: Nagabhushana Netagunte <nagabhushana.netagunte@ti.com>
---
 board/davinci/da8xxevm/da850evm.c |   14 ++++++++++----
 1 files changed, 10 insertions(+), 4 deletions(-)

Comments

Wolfgang Denk Aug. 2, 2011, 4:30 p.m. UTC | #1 
Dear nagabhushana.netagunte@ti.com,

In message <1312299792-16415-10-git-send-email-nagabhushana.netagunte@ti.com> you wrote:
> From: Nagabhushana Netagunte <nagabhushana.netagunte@ti.com>
> 
> The AM18xx EVM contains MAC address in I2C EEPROM compared
> da850/omap-l138 Logic PD EVM which maintains in SPI flash. ...

Sorry, I cannot parse this. Please reformulate.

>  	if (!eth_getenv_enetaddr("ethaddr", enetaddr)) {
> -		/* Set Ethernet MAC address from EEPROM */
> -		ret = get_mac_addr_spi(addr);
> -		if (ret != 0)
> -			return -EINVAL;
> +		/* Read Ethernet MAC address from EEPROM */
> +		if (dvevm_read_mac_address(addr)) {
> +			/* Set Ethernet MAC address from EEPROM */
> +			davinci_sync_env_enetaddr(addr);
> +		} else {
> +			/* Set Ethernet MAC address from SPI flash */
> +			ret = get_mac_addr_spi(addr);
> +			if (ret != 0)
> +				return -EINVAL;
> +		}

This is a pretty bad idea, as it will slow down booting and may cause
undefined behaviour if anybody decides to put an EEPROm on a board
where you assume there is none.

Please find a different way to test which board you have (or provide a
separate configuration).  Implement this test just once, in early init
code, so that not each and every driver or feature as to test this
himself again and again.

Best regards,

Wolfgang Denk
nagabhushana.netagunte@ti.com Aug. 9, 2011, 1:47 p.m. UTC | #2 
Denk,

Thanks for comments.

I agree with you that booting will slowdown with added calls.
I will drop this patch from this series. I will device effective
Patch later and submit. 

Regards,
Nag

On Tue, Aug 02, 2011 at 22:00:36, Wolfgang Denk wrote:
> Dear nagabhushana.netagunte@ti.com,
> 
> In message <1312299792-16415-10-git-send-email-nagabhushana.netagunte@ti.com> you wrote:
> > From: Nagabhushana Netagunte <nagabhushana.netagunte@ti.com>
> > 
> > The AM18xx EVM contains MAC address in I2C EEPROM compared
> > da850/omap-l138 Logic PD EVM which maintains in SPI flash. ...
> 
> Sorry, I cannot parse this. Please reformulate.
> 
> >  	if (!eth_getenv_enetaddr("ethaddr", enetaddr)) {
> > -		/* Set Ethernet MAC address from EEPROM */
> > -		ret = get_mac_addr_spi(addr);
> > -		if (ret != 0)
> > -			return -EINVAL;
> > +		/* Read Ethernet MAC address from EEPROM */
> > +		if (dvevm_read_mac_address(addr)) {
> > +			/* Set Ethernet MAC address from EEPROM */
> > +			davinci_sync_env_enetaddr(addr);
> > +		} else {
> > +			/* Set Ethernet MAC address from SPI flash */
> > +			ret = get_mac_addr_spi(addr);
> > +			if (ret != 0)
> > +				return -EINVAL;
> > +		}
> 
> This is a pretty bad idea, as it will slow down booting and may cause undefined behaviour if anybody decides to put an EEPROm on a board where you assume there is none.
> 
> Please find a different way to test which board you have (or provide a separate configuration).  Implement this test just once, in early init code, so that not each and every driver or feature as to test this himself again and again.
> 
> 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 "Spock, did you see the looks on their faces?"
> "Yes, Captain, a sort of vacant contentment."
>
nagabhushana.netagunte@ti.com Sept. 15, 2011, 10:50 a.m. UTC | #3 
Denk,

I comments are in-lined.

Regards,
Nag
On Tue, Aug 02, 2011 at 22:00:36, Wolfgang Denk wrote:
> Dear nagabhushana.netagunte@ti.com,
> 
> In message <1312299792-16415-10-git-send-email-nagabhushana.netagunte@ti.com> you wrote:
> > From: Nagabhushana Netagunte <nagabhushana.netagunte@ti.com>
> > 
> > The AM18xx EVM contains MAC address in I2C EEPROM compared
> > da850/omap-l138 Logic PD EVM which maintains in SPI flash. ...
> 
> Sorry, I cannot parse this. Please reformulate.
> 
> >  	if (!eth_getenv_enetaddr("ethaddr", enetaddr)) {
> > -		/* Set Ethernet MAC address from EEPROM */
> > -		ret = get_mac_addr_spi(addr);
> > -		if (ret != 0)
> > -			return -EINVAL;
> > +		/* Read Ethernet MAC address from EEPROM */
> > +		if (dvevm_read_mac_address(addr)) {
> > +			/* Set Ethernet MAC address from EEPROM */
> > +			davinci_sync_env_enetaddr(addr);
> > +		} else {
> > +			/* Set Ethernet MAC address from SPI flash */
> > +			ret = get_mac_addr_spi(addr);
> > +			if (ret != 0)
> > +				return -EINVAL;
> > +		}
> 
> This is a pretty bad idea, as it will slow down booting and may cause undefined behaviour if anybody decides to put an EEPROm on a board where you assume there is none.

I understand the concern. I will introduce new config file based on board (manufacturer). Then, based on configuration, can I access EEPROM to read MAC address? OR do you think we should not read MAC address from EEPROM at all? Your suggestions will be helpful.
> 
> Please find a different way to test which board you have (or provide a separate configuration).  Implement this test just once, in early init code, so that not each and every driver or feature as to test this himself again and again.

Since we can't figure out board type by any other means, I will try to introduce new config files for each board type.
> 
> 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 "Spock, did you see the looks on their faces?"
> "Yes, Captain, a sort of vacant contentment."
>
diff mbox

Patch

diff --git a/board/davinci/da8xxevm/da850evm.c b/board/davinci/da8xxevm/da850evm.c
index eb9c915..64ff566 100644
--- a/board/davinci/da8xxevm/da850evm.c
+++ b/board/davinci/da8xxevm/da850evm.c
@@ -494,10 +494,16 @@  int misc_init_r(void)
 	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;
+		/* Read Ethernet MAC address from EEPROM */
+		if (dvevm_read_mac_address(addr)) {
+			/* Set Ethernet MAC address from EEPROM */
+			davinci_sync_env_enetaddr(addr);
+		} else {
+			/* Set Ethernet MAC address from SPI flash */
+			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");