omap3evm: Set environment variable 'ethaddr'
PATCH 1/1 omap3evm: Set environment variable 'ethaddr'
It is now responsibility of the board specific init code to set the environment variable corresponding to the MAC address.
---
Tested on omap3evm at against latest master at:
bd061a5 : Merge branch 'master' of git://git.denx.de/u-boot-sh
board/ti/evm/evm.c | 10 ++++++++++ 1 files changed, 10 insertions(+), 0 deletions(-)
board/ti/evm/evm.c +10 -0
@@ -216,7 +216,17 @@ int board_eth_init(bd_t *bis) {int rc =0 ;#ifdef CONFIG_SMC911X +struct eth_device *dev; + rc =smc911x_initialize (0 ,CONFIG_SMC911X_BASE ); + + dev =eth_get_dev_by_index (0 ); +if (dev) { +eth_setenv_enetaddr ("ethaddr" , dev->enetaddr); + }else { +printf ("omap3evm: Couldn't get eth device\n" ); + rc =-1 ; + }#endif return rc; }
Signature · 1 line
1.7.0.4
PATCH 1/1 omap3evm: Set environment variable 'ethaddr'
Dear Sanjeev Premi,
Quoted 5 lines · In message <[email protected]> you wrote:
It is now responsibility of the board specific init code to set the environment variable corresponding to the MAC address.
Signed-off-by Sanjeev Premi <[email protected]>
This looks all wrong to me. In U-Boot, the "ethaddr" environment variable is normally the primary storage for the MAC address. Only when this variable is not set, other potential storage locations may be referenced to initialize this value.
Your patch always and unconditionally overwrites any existing "ethaddr" settings. This is not acceptable.
Best regards,
Wolfgang Denk
Signature · 5 lines
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 at denx.de Build a system that even a fool can use and only a fool will want to use it.
Premi
PATCH 1/1 omap3evm: Set environment variable 'ethaddr'
Quoted 17 lines
-----Original Message-----
From: Wolfgang Denk [mailto:wd at denx.de]
Sent: Friday, September 02, 2011 7:14 PM
To: Premi, Sanjeev
Cc: u-boot at lists.denx.de
Subject: Re: [U-Boot] [PATCH] omap3evm: Set environment variable 'ethaddr'Dear Sanjeev Premi,
Quoted 5 lines · In message <[email protected]> you wrote:
It is now responsibility of the board specific init code to set the environment variable corresponding to the MAC address.
Signed-off-by Sanjeev Premi <[email protected]>
Quoted 7 lines
This looks all wrong to me. In U-Boot, the "ethaddr" environment variable is normally the primary storage for the MAC address. Only when this variable is not set, other potential storage locations may be referenced to initialize this value.
Your patch always and unconditionally overwrites any existing "ethaddr" settings. This is not acceptable.
For the EVM, MAC address is always probed from the chip. Hence, I assumed it safe to set the ethaddr - without checking for env var. It was unlikely that someone would be setting it - to different value.
But, I see your point that I should be checking for existence of a valid ethaddr before setting/overwriting it.
~sanjeev
Quoted 12 lines
Best regards,
Wolfgang Denk
Signature · 5 lines
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 at denx.de Build a system that even a fool can use and only a fool will want to use it.
PATCH 1/1 omap3evm: Set environment variable 'ethaddr'
Dear "Premi, Sanjeev",
Quoted 6 lines · In message <[email protected]> you…
Quoted 3 lines
Your patch always and unconditionally overwrites any existing "ethaddr" settings. This is not acceptable.
For the EVM, MAC address is always probed from the chip. Hence, I assumed it safe to set the ethaddr - without checking for env var.
This is not what we do in U-Boot, so please fix that.
Quoted 2 lines
It was unlikely that someone would be setting it - to different value.
Unlikely? Not so. This is expected standard behaviour. Please don't try to invent a wheel (in a different shape).
Best regards,
Wolfgang Denk
Signature · 4 lines
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 at denx.de Save energy: Drive a smaller shell.
Premi
PATCH 1/1 omap3evm: Set environment variable 'ethaddr'
Quoted 16 lines
From: Wolfgang Denk [wd at denx.de]
Sent: Saturday, September 03, 2011 3:37 AM
To: Premi, Sanjeev
Cc: u-boot at lists.denx.de
Subject: Re: [U-Boot] [PATCH] omap3evm: Set environment variable 'ethaddr'Dear "Premi, Sanjeev",
Quoted 6 lines · In message <[email protected]> you…
Quoted 3 lines
Your patch always and unconditionally overwrites any existing "ethaddr" settings. This is not acceptable.
For the EVM, MAC address is always probed from the chip. Hence, I assumed it safe to set the ethaddr - without checking for env var.
Quoted 7 lines
This is not what we do in U-Boot, so please fix that.
Quoted 2 lines
It was unlikely that someone would be setting it - to different value.
Unlikely? Not so. This is expected standard behaviour. Please don't try to invent a wheel (in a different shape).
[sp] I agreed with your comments in the next para. Had only
described my thoughts before creating this patch.
You seem to have missed the v2 of the same patch where I believe I have taken care of this behavior:
http://marc.info/?l=u-boot&m=131497907619918&w=2
Do let me know if this is better...
~sanjeev
Quoted 4 lines
Best regards,
Wolfgang Denk