mvgbe: remove setting of ethaddr within the driver

PATCH New Michael Walle 7 participants 41 messages Nov 7, 2011 – Dec 20, 2011 Network drivers

Share & tools

Michael Walle

author

2011-11-07 22:08 UTC #0 lore raw

PATCH 1/1 mvgbe: remove setting of ethaddr within the driver

A network driver should not touch the environment at all. This patch fixes this behaviour by removing the code for setting a default/randomized MAC address.

Instead a board should either set CONFIG_ETHADDR, CONFIG_ETH1ADDR etc. or use some specific code within the board files, eg. if randomization is needed.

All boards which have CONFIG_SKIP_LOCAL_MAC_RANDOMIZATION set (actually its only one board, edminiv2) are converted to use CONFIG_ETHADDR. Please check your boards whether you really need randomization support or the CONFIG_ETHADDR macro is enough.

Signed-off-by Michael Walle <[email protected]>

Cc Mike Frysinger <[email protected]>

Cc Valentin Longchamp <[email protected]>

Cc Eric Cooper <[email protected]>

Cc Jason Cooper <[email protected]>

Cc Siddarth Gore <[email protected]>

Cc Albert ARIBAUD <[email protected]>

Cc Prafulla Wadaskar <[email protected]>

Cc Simon Guinot <[email protected]>

---
 drivers/net/mvgbe.c        |   23 -----------------------
 include/configs/edminiv2.h |    2 +-
 2 files changed, 1 insertions(+), 24 deletions(-)

drivers/net/mvgbe.c +0 ~~-23~~

index c7f7446..21be642 100644
@@ -647,7 +647,6 @@ int mvgbe_initialize(bd_t *bis)
 	struct mvgbe_device *dmvgbe;
 	struct eth_device *dev;
 	int devnum;
-	char *s;
 	u8 used_ports[MAX_MVGBE_DEVS] = CONFIG_MVGBE_PORTS;
 
 	for (devnum = 0; devnum < MAX_MVGBE_DEVS; devnum++) {

@@ -702,16 +701,13 @@ error1:
 		/* must be less than NAMESIZE (16) */
 		sprintf(dev->name, "egiga%d", devnum);
 
-		/* Extract the MAC address from the environment */
 		switch (devnum) {
 		case 0:
 			dmvgbe->regs = (void *)MVGBE0_BASE;
-			s = "ethaddr";
 			break;
 #if defined(MVGBE1_BASE)
 		case 1:
 			dmvgbe->regs = (void *)MVGBE1_BASE;
-			s = "eth1addr";
 			break;
 #endif
 		default:	/* this should never happen */

@@ -720,25 +716,6 @@ error1:
 			return -1;
 		}
 
-		while (!eth_getenv_enetaddr(s, dev->enetaddr)) {
-			/* Generate Private MAC addr if not set */
-			dev->enetaddr[0] = 0x02;
-			dev->enetaddr[1] = 0x50;
-			dev->enetaddr[2] = 0x43;
-#if defined (CONFIG_SKIP_LOCAL_MAC_RANDOMIZATION)
-			/* Generate fixed lower MAC half using devnum */
-			dev->enetaddr[3] = 0;
-			dev->enetaddr[4] = 0;
-			dev->enetaddr[5] = devnum;
-#else
-			/* Generate random lower MAC half */
-			dev->enetaddr[3] = get_random_hex();
-			dev->enetaddr[4] = get_random_hex();
-			dev->enetaddr[5] = get_random_hex();
-#endif
-			eth_setenv_enetaddr(s, dev->enetaddr);
-		}
-
 		dev->init = (void *)mvgbe_init;
 		dev->halt = (void *)mvgbe_halt;
 		dev->send = (void *)mvgbe_send;

include/configs/edminiv2.h +1 -1

index 88d32b2..2b96cb3 100644
@@ -142,7 +142,7 @@
 #ifdef CONFIG_CMD_NET
 #define CONFIG_MVGBE				/* Enable Marvell GbE Driver */
 #define CONFIG_MVGBE_PORTS	{1}		/* enable port 0 only */
-#define CONFIG_SKIP_LOCAL_MAC_RANDOMIZATION	/* don't randomize MAC */
+#define CONFIG_ETH_ADDR 02:50:43:00:00:01
 #define CONFIG_PHY_BASE_ADR	0x8
 #define CONFIG_RESET_PHY_R	/* use reset_phy() to init mv8831116 PHY */
 #define CONFIG_NETCONSOLE	/* include NetConsole support   */

Signature · 1 line

1.7.2.5

Prafulla Wadaskar

2011-11-08 07:18 UTC #1 lore raw

PATCH 1/1 mvgbe: remove setting of ethaddr within the driver

Quoted 18 lines

-----Original Message-----
From: Michael Walle [mailto:michael at walle.cc]
Sent: Tuesday, November 08, 2011 3:38 AM
To: u-boot at lists.denx.de
Cc: Michael Walle; Mike Frysinger; Valentin Longchamp; Eric Cooper; Jason Cooper; Siddarth Gore; Albert ARIBAUD; Prafulla Wadaskar; Simon Guinot
Subject: [PATCH] mvgbe: remove setting of ethaddr within the driver
A network driver should not touch the environment at all. This patch fixes
this behaviour by removing the code for setting a default/randomized MAC
address.

Quoted 5 lines

Instead a board should either set CONFIG_ETHADDR, CONFIG_ETH1ADDR etc. or
use some specific code within the board files, eg. if randomization is
needed.

Dear Michael

Yes, it is needed on few boards.
This patch removes this support for these boards, it is good to remove it from driver code, but this functionality should not be broken, the same should be supported in board specific code.

So NAK for this patch.

I had suggested to add some common framework like common/hush.c,

Here are my suggestions-
1. commond/random_mac.c should be added
2. call back function should be provided to generate random number, those should be defined in arch specific code (for Kirkwood arch-kirkwood/cpu.c)
3. mac randomization should be enabled by CONFIG_SYS_LOCAL_MAC_RANDOMIZATION macro
4. For mvgbe uses it should be enabled by default in include/configs/mv-common.h.
5. for corner case like edminiv2, in should be undefed in board config file
6. Some documentation should be supported for this generic framework.

Regards..
Prafulla . .

Mike Frysinger

2011-11-08 13:58 UTC #2 lore raw

PATCH 1/1 mvgbe: remove setting of ethaddr within the driver

Quoted 9 lines · On Tuesday 08 November 2011 02:18:20 Prafulla Wadaskar wrote:

Here are my suggestions-
1. commond/random_mac.c should be added
2. call back function should be provided to generate random number, those should be defined in arch specific code (for Kirkwood arch-kirkwood/cpu.c)
3. mac randomization should be enabled by CONFIG_SYS_LOCAL_MAC_RANDOMIZATION macro 4. For mvgbe uses it should be enabled by default in include/configs/mv-common.h. 5. for corner case like edminiv2, in should be undefed in board config file 6. Some documentation should be supported for this generic framework.

i think Wolfgang already NAK-ed this idea. but he'd have to clarify. -mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20111108/71a48366/attachment.pgp

Wolfgang Denk

2011-11-08 14:10 UTC #3 lore raw

PATCH 1/1 mvgbe: remove setting of ethaddr within the driver

Dear Mike Frysinger,

Quoted 8 lines · In message <[email protected]> you wrote:

Quoted 6 lines
3. mac randomization should be enabled by CONFIG_SYS_LOCAL_MAC_RANDOMIZATION macro 4. For mvgbe uses it should be enabled by default in include/configs/mv-common.h. 5. for corner case like edminiv2, in should be undefed in board config file 6. Some documentation should be supported for this generic framework.
i think Wolfgang already NAK-ed this idea. but he'd have to clarify.

Indeed I did NAK this. And I tried to provide an explantion why I consider this an inherently broken approach.

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
"Love your country but never trust its government."
- from a hand-painted road sign in central Pennsylvania

Mike Frysinger

2011-11-08 13:57 UTC #4 lore raw

PATCH 1/1 mvgbe: remove setting of ethaddr within the driver

In reply to Michael Walle

Quoted 1 line · On Monday 07 November 2011 17:08:09 Michael Walle wrote:

 drivers/net/mvgbe.c        |   23 -----------------------

ACK to the changes to this file

include/configs/edminiv2.h

Quoted 4 lines

include/configs/edminiv2.h +1 -0
 
+#define CONFIG_ETH_ADDR 02:50:43:00:00:01

NAK to this. board configs are not allow to hardcode MACs.
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20111108/8a1aec41/attachment.pgp

Michael Walle

author

2011-11-08 13:45 UTC #5 lore raw

PATCH 1/1 mvgbe: remove setting of ethaddr within the driver

Am Di, 8.11.2011, 14:57 schrieb Mike Frysinger:

Quoted 11 lines · On Monday 07 November 2011 17:08:09 Michael Walle wrote:

Quoted 1 line
 drivers/net/mvgbe.c        |   23 -----------------------
ACK to the changes to this file
include/configs/edminiv2.h
Quoted 4 lines
include/configs/edminiv2.h +1 -0
 
+#define CONFIG_ETH_ADDR 02:50:43:00:00:01
NAK to this. board configs are not allow to hardcode MACs.

mh, so why is there a CONFIG_ETH_ADDR macro? is it deprecated? README.enetaddr explicitly refers to that config option.

Signature · 1 line

michael

Mike Frysinger

2011-11-08 14:54 UTC #6 lore raw

PATCH 1/1 mvgbe: remove setting of ethaddr within the driver

Quoted 11 lines · On Tuesday 08 November 2011 08:45:27 Michael Walle wrote:

Am Di, 8.11.2011, 14:57 schrieb Mike Frysinger:
Quoted 7 lines · On Monday 07 November 2011 17:08:09 Michael Walle wrote:
include/configs/edminiv2.h
Quoted 4 lines
include/configs/edminiv2.h +1 -0
 
+#define CONFIG_ETH_ADDR 02:50:43:00:00:01
NAK to this. board configs are not allow to hardcode MACs.
mh, so why is there a CONFIG_ETH_ADDR macro? is it deprecated? README.enetaddr explicitly refers to that config option.

it's a useful debug knob
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20111108/5f715a2d/attachment.pgp

Wolfgang Denk

2011-11-08 15:17 UTC #7 lore raw

PATCH 1/1 mvgbe: remove setting of ethaddr within the driver

In reply to Michael Walle

Dear "Michael Walle",

Quoted 6 lines · In message <[email protected]> you…

Quoted 4 lines
Quoted 2 lines
+#define CONFIG_ETH_ADDR 02:50:43:00:00:01
NAK to this. board configs are not allow to hardcode MACs.
mh, so why is there a CONFIG_ETH_ADDR macro? is it deprecated? README.enetaddr explicitly refers to that config option.

There are situations where it cannot be helped, like when programming the U-Boot image into a ROM (yes, a real ROM that cannot be changed any more by software alone).

But in almost all normal situations this must not be used. In any case, we will not accept it into mainline.

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
Be kind to unkind people - they need it the most.

Michael Walle

author

2011-11-08 17:34 UTC #8 lore raw

PATCH 1/1 mvgbe: remove setting of ethaddr within the driver

Hi Wolfgang, Hi Mike,

Am Di, 8.11.2011, 16:17 schrieb Wolfgang Denk:

Quoted 11 lines

Dear "Michael Walle",
In message <[email protected]>
Quoted 6 lines · you
Quoted 4 lines
Quoted 2 lines
+#define CONFIG_ETH_ADDR 02:50:43:00:00:01
NAK to this. board configs are not allow to hardcode MACs.
mh, so why is there a CONFIG_ETH_ADDR macro? is it deprecated? README.enetaddr explicitly refers to that config option.

Quoted 6 lines

There are situations where it cannot be helped, like when programming the U-Boot image into a ROM (yes, a real ROM that cannot be changed any more by software alone).
But in almost all normal situations this must not be used. In any case, we will not accept it into mainline.

I have a Buffalo Linkstation Pro LS-XHL for which i plan to submit a BSP. On this board, the MAC address is saved within the environment, which is in turn stored in flash. Additionally this board does not have a serial port, thus the only available console is the network console.

If someone will mess up the environment, it is possible to clear it by holding a button upon powerup. Atm i have the CONFIG_ETH_ADDR set to a default MAC address to be able to use the network console with the default environment. To restore the original MAC address, a user have to manually replace ethaddr with the one printed on the case.

But now, if i can't use the CONFIG_ETH_ADDR, i think i'm stuck :) Or do i miss something? (Yeah there may be the MAC randomization, if accepted).

Signature · 1 line

Michael

Wolfgang Denk

2011-11-08 19:28 UTC #9 lore raw

PATCH 1/1 mvgbe: remove setting of ethaddr within the driver

Dear "Michael Walle",

Quoted 9 lines · In message <[email protected]> you…

If someone will mess up the environment, it is possible to clear it by holding a button upon powerup. Atm i have the CONFIG_ETH_ADDR set to a default MAC address to be able to use the network console with the default environment. To restore the original MAC address, a user have to manually replace ethaddr with the one printed on the case.
But now, if i can't use the CONFIG_ETH_ADDR, i think i'm stuck :) Or do i miss something? (Yeah there may be the MAC randomization, if accepted).

There are more clever ways than just to clear the environment. We have "env export" and "env import", so you can create one or more backup copies or user proviles. Instead of clearing the environment, you could also load one such previously saved user profile, which then would contain the MAC address.

Best regards,

Wolfgang Denk

Signature · 6 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
The night sky over the planet Krikkit is the least interesting  sight
in the entire Universe.
                 - Douglas Adams _Life, the Universe, and Everything_

Michael Walle

author

2011-11-08 22:30 UTC #10 lore raw

PATCH 1/1 mvgbe: remove setting of ethaddr within the driver

Am Dienstag 08 November 2011, 20:28:19 schrieb Wolfgang Denk:

Quoted 3 lines

Dear "Michael Walle",
In message <[email protected]>

Quoted 6 lines · you

Quoted 6 lines
If someone will mess up the environment, it is possible to clear it by holding a button upon powerup. Atm i have the CONFIG_ETH_ADDR set to a default MAC address to be able to use the network console with the default environment. To restore the original MAC address, a user have to manually replace ethaddr with the one printed on the case.

Quoted 8 lines

Quoted 2 lines
But now, if i can't use the CONFIG_ETH_ADDR, i think i'm stuck :) Or do i miss something? (Yeah there may be the MAC randomization, if accepted).
There are more clever ways than just to clear the environment. We have "env export" and "env import", so you can create one or more backup copies or user proviles. Instead of clearing the environment, you could also load one such previously saved user profile, which then would contain the MAC address.

thanks for the hint, but there is only 512kb flash on this board (with 64k sectors). I don't think there is enough room for a second copy of the environment.

and in some way, the mac address has to find its way into one of these environments.

i think there are more boards like this, esp. since there is this mac randomization code within the mvgbe driver.

Signature · 1 line

Michael

Wolfgang Denk

2011-11-08 22:52 UTC #11 lore raw

PATCH 1/1 mvgbe: remove setting of ethaddr within the driver

Dear Michael Walle,

Quoted 10 lines · In message <[email protected]> you wrote:

Quoted 6 lines
There are more clever ways than just to clear the environment. We have "env export" and "env import", so you can create one or more backup copies or user proviles. Instead of clearing the environment, you could also load one such previously saved user profile, which then would contain the MAC address.
thanks for the hint, but there is only 512kb flash on this board (with 64k sectors). I don't think there is enough room for a second copy of the environment.

You don't need a separate sector. How bis is your environment/ I bet if fits easily in 4 KiB or so - just find an unused part of a sector, and put it there. Yes, upating it will be more complicated, as you hav eto read that sector into RAM, replace the larst part, and write it back, but resource-restricted systems require efficient use of memory.

Quoted 2 lines

i think there are more boards like this, esp. since there is this mac randomization code within the mvgbe driver.

I don't want to have that in mainline.

Best regards,

Wolfgang Denk

Signature · 7 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
The Buddha, the Godhead, resides quite as comfortably in the circuits
of a digital computer or the gears of a cycle transmission as he does
at the top of a mountain or in the petals of a flower.
            - R.  Pirsig, "Zen and the Art of Motorcycle Maintenance"

Michael Walle

author

2011-11-08 23:26 UTC #12 lore raw

PATCH 1/1 mvgbe: remove setting of ethaddr within the driver

Am Dienstag 08 November 2011, 23:52:56 schrieb Wolfgang Denk:

Quoted 9 lines

Quoted 3 lines
thanks for the hint, but there is only 512kb flash on this board (with 64k sectors). I don't think there is enough room for a second copy of the environment.
You don't need a separate sector. How bis is your environment/ I bet if fits easily in 4 KiB or so - just find an unused part of a sector, and put it there. Yes, upating it will be more complicated, as you hav eto read that sector into RAM, replace the larst part, and write it back, but resource-restricted systems require efficient use of memory.

may be possible, but then there is a window of opportunity where the mac is already erased but the not yet written with a new one.

and there is this very easy recovery procedure.. just erase the environment, let the network works in some way and manually set the mac address, which apparently use some boards.

Quoted 4 lines

Quoted 2 lines
i think there are more boards like this, esp. since there is this mac randomization code within the mvgbe driver.
I don't want to have that in mainline.

well it is...

atm there is the mvgbe driver:
- which has a hardcoded mac address (or if enabled, a randomized mac address) and
- touches the environment

trying to fix this, it will be nak'ed because the boards in mainline have to behave identically before and after the patch. but without CONFIG_ETHADDR or generic randomization support, which will be nak'ed, too, this is not possible. sounds to me like a dead end.

additionally, there are these kind of boards without non-volatile storage other than one tiny flash, which holds uboot and its environment and therefore has the mac address stored within the environment. to make it more complicated some of these boards have to have a working ethernet connectivity with the default environment.

Signature · 1 line

Michael

Wolfgang Denk

2011-11-08 23:38 UTC #13 lore raw

PATCH 1/1 mvgbe: remove setting of ethaddr within the driver

Dear Michael Walle,

Quoted 3 lines · In message <[email protected]> you wrote:

may be possible, but then there is a window of opportunity where the mac is already erased but the not yet written with a new one.

Sure. If you look hard enough you will always find a way to brick a device. And if you don't look, it just takes an ingeniuos idiot to find one.

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
"The one charm of marriage is that it makes a  life  of  deception  a
neccessity."                                            - Oscar Wilde

Michael Walle

author

2011-11-09 08:31 UTC #14 lore raw

PATCH 1/1 mvgbe: remove setting of ethaddr within the driver

In reply to Michael Walle

Am Mi, 9.11.2011, 00:26 schrieb Michael Walle:

Quoted 5 lines

atm there is the mvgbe driver:
- which has a hardcoded mac address (or if enabled, a randomized mac
address)
and
- touches the environment

oh and i've forgot to mention, that the driver is broken for boards with

#define CONFIG_MVGBE_PORTS {0,1}

which is the reason why i initially submitted the patch.

Signature · 1 line

Michael

Michael Walle

author

2011-11-09 17:34 UTC #15 lore raw

PATCH 1/1 mvgbe: remove setting of ethaddr within the driver

Quoted 6 lines

Quoted 5 lines
atm there is the mvgbe driver:
- which has a hardcoded mac address (or if enabled, a randomized mac
address)
and
- touches the environment

Quoted 6 lines

oh and i've forgot to mention, that the driver is broken for boards with
#define CONFIG_MVGBE_PORTS {0,1}
which is the reason why i initially submitted the patch.

Hi Wolfgang, Hi Mike, Hi Prafulla,

So was there any conclusion, now? Never touch mvgbe.c anymore?

Signature · 1 line

Michael

Wolfgang Denk

2011-11-09 19:27 UTC #16 lore raw

PATCH 1/1 mvgbe: remove setting of ethaddr within the driver

Dear "Michael Walle",

Quoted 2 lines · In message <[email protected]> you…

So was there any conclusion, now? Never touch mvgbe.c anymore?

Let's remove this broken code.

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
"If you are afraid of loneliness, don't marry."             - Chekhov

Michael Walle

author

2011-11-10 09:15 UTC #17 lore raw

PATCH 1/1 mvgbe: remove setting of ethaddr within the driver

Hi Wolfgang,

Quoted 2 lines

Quoted 1 line
So was there any conclusion, now? Never touch mvgbe.c anymore?
Let's remove this broken code.

This is exactly part of this patch ([PATCH] mvgbe: remove setting of ethaddr within the driver). This part, while ACK'ed by Mike, was NAK'ed by Prafulla.

Signature · 1 line

Michael

Simon Guinot

2011-11-10 11:06 UTC #18 lore raw

PATCH 1/1 mvgbe: remove setting of ethaddr within the driver

Hi Michael,

Quoted 9 lines · Michael Walle

Hi Wolfgang,
Quoted 2 lines
Quoted 1 line
So was there any conclusion, now? Never touch mvgbe.c anymore?
Let's remove this broken code.
This is exactly part of this patch ([PATCH] mvgbe: remove setting of ethaddr within the driver). This part, while ACK'ed by Mike, was NAK'ed by Prafulla.

As some boards are relying on this broken code, maybe you should provide an alternative ? I could be wrong, but it seems to me that just removing the code is not enough.

Simon
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
Url : http://lists.denx.de/pipermail/u-boot/attachments/20111110/a2ee012d/attachment.pgp

Wolfgang Denk

2011-11-10 12:01 UTC #19 lore raw

PATCH 1/1 mvgbe: remove setting of ethaddr within the driver

Dear Simon Guinot,

Quoted 10 lines · In message <[email protected]> you wrote:

Quoted 6 lines
Quoted 2 lines
Let's remove this broken code.
This is exactly part of this patch ([PATCH] mvgbe: remove setting of ethaddr within the driver). This part, while ACK'ed by Mike, was NAK'ed by Prafulla.
As some boards are relying on this broken code, maybe you should provide an alternative ? I could be wrong, but it seems to me that just removing the code is not enough.

Which exact boards are these? And how exactly are these relying on the broken code?

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
Why is an average signature file longer than an average Perl script??

Simon Guinot

2011-11-10 14:21 UTC #20 lore raw

PATCH 1/1 mvgbe: remove setting of ethaddr within the driver

Hi Wolfgang,

Quoted 10 lines · Wolfgang Denk

Dear Simon Guinot,
Quoted 7 lines · In message <[email protected]> you wrote:
Quoted 6 lines
Quoted 2 lines
Let's remove this broken code.
This is exactly part of this patch ([PATCH] mvgbe: remove setting of ethaddr within the driver). This part, while ACK'ed by Mike, was NAK'ed by Prafulla.

Quoted 6 lines

Quoted 3 lines
As some boards are relying on this broken code, maybe you should provide an alternative ? I could be wrong, but it seems to me that just removing the code is not enough.
Which exact boards are these? And how exactly are these relying on the broken code?

I can't list this boards exactly. For sure, I can say that the LaCie boards (netspace_v2 and cie) are concerned.

For this boards a random MAC address is used and the computation is performed by the driver mvgbe. If this code is simply removed, the network initialization for this boards will break.

I suspect a lot of Marvell boards from being in the same case.

Regards,

Wolfgang Denk

2011-11-10 15:53 UTC #21 lore raw

PATCH 1/1 mvgbe: remove setting of ethaddr within the driver

Dear Simon Guinot,

Quoted 9 lines · In message <[email protected]> you wrote:

I can't list this boards exactly. For sure, I can say that the LaCie boards (netspace_v2 and cie) are concerned.
For this boards a random MAC address is used and the computation is performed by the driver mvgbe. If this code is simply removed, the network initialization for this boards will break.
I suspect a lot of Marvell boards from being in the same case.

Argh... I was not aware that the scope of damage already done was that big.

But no matter how we twist and turn it, this is a real problem that needs to be fixed, and the sooner we do it the better.

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
       There is enough for the need of everyone in this world,
       but not for the greed of everyone.     - Mahatma Gandhi

Valentin Longchamp

2011-11-10 16:30 UTC #22 lore raw

PATCH 1/1 mvgbe: remove setting of ethaddr within the driver

Quoted 7 lines · Wolfgang Denk

Quoted 2 lines · In message <[email protected]> you wrote:
I suspect a lot of Marvell boards from being in the same case.
Argh... I was not aware that the scope of damage already done was that big.

On the Keymile boards (for marvell boards, km_kirkwood) we have an EEPROM that stores the ethaddr for each board, so we should not be affected by the removal of this "feature" (the driver setting ethaddr).

Cheers

Valentin

Holger Brunck

2011-11-10 17:00 UTC #23 lore raw

PATCH 1/1 mvgbe: remove setting of ethaddr within the driver

Quoted 5 lines · Valentin Longchamp

Quoted 4 lines · Wolfgang Denk
Quoted 2 lines · In message <[email protected]> you wrote:
I suspect a lot of Marvell boards from being in the same case.

Quoted 8 lines

Quoted 3 lines
Argh... I was not aware that the scope of damage already done was that big.
On the Keymile boards (for marvell boards, km_kirkwood) we have an EEPROM that stores the ethaddr for each board, so we should not be affected by the removal of this "feature" (the driver setting ethaddr).

I gave it a try on km_kirkwood. What happens on keymile boards is that we get the warning:
Warning: failed to set MAC address
if we initially start the board without an environment saved previously.

And this is better than writing a random mac adress into the HW, I guess...

Fortunately our boards in case of an initial boot are configuring different things e.g. the correct ethaddr, do a saveenv and a reboot. And afterwards everything is ok.

So yes for our boards it's ok to remove this code.

Best regards
Holger

Mike Frysinger

2011-11-10 17:08 UTC #24 lore raw

PATCH 1/1 mvgbe: remove setting of ethaddr within the driver

In reply to Valentin Longchamp

Quoted 10 lines · On Thursday 10 November 2011 11:30:13 Valentin Longchamp wrote:

Quoted 5 lines · Wolfgang Denk
Quoted 1 line · In message <[email protected]> you wrote:
I suspect a lot of Marvell boards from being in the same case.
Argh... I was not aware that the scope of damage already done was that big.
On the Keymile boards (for marvell boards, km_kirkwood) we have an EEPROM that stores the ethaddr for each board, so we should not be affected by the removal of this "feature" (the driver setting ethaddr).

curious, but is it an eeprom on an external bus, or is it connected to the mvgbe block ?
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20111110/f3f92d08/attachment.pgp

Mike Frysinger

2011-11-10 17:09 UTC #25 lore raw

PATCH 1/1 mvgbe: remove setting of ethaddr within the driver

In reply to Simon Guinot

Quoted 15 lines · On Thursday 10 November 2011 09:21:53 Simon Guinot wrote:

Quoted 7 lines · Wolfgang Denk
Quoted 6 lines · Simon Guinot
Quoted 5 lines
Quoted 1 line
Let's remove this broken code.
This is exactly part of this patch ([PATCH] mvgbe: remove setting of ethaddr within the driver). This part, while ACK'ed by Mike, was NAK'ed by Prafulla.
Quoted 6 lines
Quoted 3 lines
As some boards are relying on this broken code, maybe you should provide an alternative ? I could be wrong, but it seems to me that just removing the code is not enough.
Which exact boards are these? And how exactly are these relying on the broken code?

Quoted 6 lines

I can't list this boards exactly. For sure, I can say that the LaCie boards (netspace_v2 and cie) are concerned.
For this boards a random MAC address is used and the computation is performed by the driver mvgbe. If this code is simply removed, the network initialization for this boards will break.

it will be broken until you set "ethaddr" in the env right ? i think that's currently the expected behavior ...
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20111110/6dc92555/attachment.pgp

Simon Guinot

2011-11-10 17:54 UTC #26 lore raw

PATCH 1/1 mvgbe: remove setting of ethaddr within the driver

Quoted 19 lines · Mike Frysinger

Quoted 18 lines · On Thursday 10 November 2011 09:21:53 Simon Guinot wrote:
Quoted 7 lines · Wolfgang Denk
Quoted 6 lines · Simon Guinot
Quoted 5 lines
Quoted 1 line
Let's remove this broken code.
This is exactly part of this patch ([PATCH] mvgbe: remove setting of ethaddr within the driver). This part, while ACK'ed by Mike, was NAK'ed by Prafulla.
Quoted 6 lines
Quoted 3 lines
As some boards are relying on this broken code, maybe you should provide an alternative ? I could be wrong, but it seems to me that just removing the code is not enough.
Which exact boards are these? And how exactly are these relying on the broken code?
I can't list this boards exactly. For sure, I can say that the LaCie boards (netspace_v2 and cie) are concerned.

Quoted 6 lines

Quoted 3 lines
For this boards a random MAC address is used and the computation is performed by the driver mvgbe. If this code is simply removed, the network initialization for this boards will break.
it will be broken until you set "ethaddr" in the env right ? i think that's currently the expected behavior ...

Yes it is right. I guess if someone is able to write U-Boot into a flash, he is also able to set the environement variable "ethaddr".

An extra operation would be needed to setup U-Boot, but clearly it is not a problem.

Michael Walle

author

2011-11-10 17:46 UTC #27 lore raw

PATCH 1/1 mvgbe: remove setting of ethaddr within the driver

In reply to Simon Guinot

Am Donnerstag 10 November 2011, 15:21:53 schrieb Simon Guinot:

Quoted 3 lines

For this boards a random MAC address is used and the computation is performed by the driver mvgbe. If this code is simply removed, the network initialization for this boards will break.

Moving this feature into some common code was NAK'ed by Wolfgang.

Signature · 1 line

Michael

Wolfgang Denk

2011-11-10 11:44 UTC #28 lore raw

PATCH 1/1 mvgbe: remove setting of ethaddr within the driver

In reply to Michael Walle

Dear Prafulla,

Quoted 7 lines · In message <[email protected]>…

Quoted 3 lines
Quoted 2 lines
So was there any conclusion, now? Never touch mvgbe.c anymore?
Let's remove this broken code.
This is exactly part of this patch ([PATCH] mvgbe: remove setting of ethaddr within the driver). This part, while ACK'ed by Mike, was NAK'ed by Prafulla.

Could you please revise your assessment? I would really appreciate if we could clean this up.

Thanks.

Wolfgang Denk

Signature · 7 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
Yes, it's a technical challenge, and  you  have  to  kind  of  admire
people  who go to the lengths of actually implementing it, but at the
same time you wonder about their IQ...
         --  Linus Torvalds in <[email protected]>

Michael Walle

author

2011-11-16 20:15 UTC #29 lore raw

PATCH 1/1 mvgbe: remove setting of ethaddr within the driver

Hi Wolfgang,

Am Donnerstag 10 November 2011, 12:44:26 schrieb Wolfgang Denk:

Quoted 3 lines

Dear Prafulla,
In message <[email protected]>

Quoted 10 lines · Michael Walle

Quoted 7 lines
Quoted 3 lines
Quoted 1 line
So was there any conclusion, now? Never touch mvgbe.c anymore?
Let's remove this broken code.
This is exactly part of this patch ([PATCH] mvgbe: remove setting of ethaddr within the driver). This part, while ACK'ed by Mike, was NAK'ed by Prafulla.
Could you please revise your assessment? I would really appreciate if we could clean this up.

Regarding Prafulla's NAK?

Let me sum up this thread once again:
- consens is found that the driver should be cleaned up (removing the randomization code)
- at least the keymile boards are ok with that. but there may be plenty of other boards which rely on this (broken?) behaviour.
- besides the fact that a driver must not touch the environment, the mvgbe driver also fails to do this correctly for boards which has only the second port enabled.
- Prafulla NAKed because only removing the code will leave some boards in a non-working state.

i'm willing make a patch to move the randomization functionality to some generic place, but only if it is not NAKed by you beforehand. i dont have any time to waste ;)

Signature · 1 line

Michael

Wolfgang Denk

2011-11-16 20:26 UTC #30 lore raw

PATCH 1/1 mvgbe: remove setting of ethaddr within the driver

Dear Michael Walle,

Quoted 4 lines · In message <[email protected]> you wrote:

Quoted 3 lines
Could you please revise your assessment? I would really appreciate if we could clean this up.
Regarding Prafulla's NAK?

Yes - I'd really appreciate if Prafulla woudl withdraw his NAK, and agree on a cleanup. But he remains silent for a week or so...

Quoted 5 lines

Let me sum up this thread once again:
- consens is found that the driver should be cleaned up (removing the randomization code)
- at least the keymile boards are ok with that. but there may be plenty of other boards which rely on this (broken?) behaviour.

The problem is this "may be". We don't know it, and we will never find out. And even if we find such configurations, we have no information if any user actually needs this feature. The only way to really find out is by removing this stuff now. If anybody complains then, we can discuss with him approaches to fix this again, in a clean way.

Quoted 9 lines

- besides the fact that a driver must not touch the environment, the mvgbe driver also fails to do this correctly for boards which has only the second port enabled.
- Prafulla NAKed because only removing the code will leave some boards in a non-working state.
i'm willing make a patch to move the randomization functionality to some generic place, but only if it is not NAKed by you beforehand. i dont have any time to waste ;)

Please don't move the broken code around. Let's remove it.

Let's give Prafulla another 2 or 3 days time to respond. If we don;t hear from him, we go ahead.

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
Be careful what you wish for. You never know who will be listening.
                                      - Terry Pratchett, _Soul Music_

Prafulla Wadaskar

2011-11-17 08:58 UTC #31 lore raw

PATCH 1/1 mvgbe: remove setting of ethaddr within the driver

Quoted 13 lines

-----Original Message-----
From: Wolfgang Denk [mailto:wd at denx.de]
Sent: Thursday, November 17, 2011 1:57 AM
To: Michael Walle
Cc: Prafulla Wadaskar; Valentin Longchamp; Siddarth Gore; Simon Guinot; u-boot at lists.denx.de
Subject: Re: [U-Boot] [PATCH] mvgbe: remove setting of ethaddr within the driver
Dear Michael Walle,

Quoted 8 lines

Quoted 2 lines · In message <[email protected]> you wrote:
Quoted 2 lines
Could you please revise your assessment? I would really
appreciate if
Quoted 2 lines
Quoted 1 line
we could clean this up.
Regarding Prafulla's NAK?
Yes - I'd really appreciate if Prafulla woudl withdraw his NAK,

Hi All
I withdraw my NAK for this patch.
Let's remove mac randomization support for mvgbe driver. If needed, for any particular board, it will be addressed separately.

Regards..
Prafulla . . .

Wolfgang Denk

2011-11-17 10:45 UTC #32 lore raw

PATCH 1/1 mvgbe: remove setting of ethaddr within the driver

Dear Prafulla,

Quoted 4 lines · In message <[email protected]>…

I withdraw my NAK for this patch.
Let's remove mac randomization support for mvgbe driver. If needed, for any particular board, it will be addressed separately.

Thanks a lot - highly appreciated.

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
I'm a programmer: I don't buy software, I write it.
                                                  -- Tom Christiansen

Michael Walle

author

2011-11-17 22:52 UTC #33 lore raw

PATCH v2 1/1 mvgbe: remove setting of ethaddr within the driver

In reply to Prafulla Wadaskar

A network driver should not touch the environment at all. This patch fixes this behaviour by removing the code for setting a default/randomized MAC address.

Signed-off-by Michael Walle <[email protected]>

Acked-by Mike Frysinger <[email protected]>

Acked-by Prafulla Wadaskar <[email protected]>

Cc Mike Frysinger <[email protected]>

Cc Prafulla Wadaskar <[email protected]>

Cc Valentin Longchamp <[email protected]>

Cc Eric Cooper <[email protected]>

Cc Jason Cooper <[email protected]>

Cc Siddarth Gore <[email protected]>

Cc Albert ARIBAUD <[email protected]>

Cc Simon Guinot <[email protected]>

---
 drivers/net/mvgbe.c |   23 -----------------------
 1 files changed, 0 insertions(+), 23 deletions(-)

drivers/net/mvgbe.c +0 ~~-23~~

index c7f7446..21be642 100644
@@ -647,7 +647,6 @@ int mvgbe_initialize(bd_t *bis)
 	struct mvgbe_device *dmvgbe;
 	struct eth_device *dev;
 	int devnum;
-	char *s;
 	u8 used_ports[MAX_MVGBE_DEVS] = CONFIG_MVGBE_PORTS;
 
 	for (devnum = 0; devnum < MAX_MVGBE_DEVS; devnum++) {

@@ -702,16 +701,13 @@ error1:
 		/* must be less than NAMESIZE (16) */
 		sprintf(dev->name, "egiga%d", devnum);
 
-		/* Extract the MAC address from the environment */
 		switch (devnum) {
 		case 0:
 			dmvgbe->regs = (void *)MVGBE0_BASE;
-			s = "ethaddr";
 			break;
 #if defined(MVGBE1_BASE)
 		case 1:
 			dmvgbe->regs = (void *)MVGBE1_BASE;
-			s = "eth1addr";
 			break;
 #endif
 		default:	/* this should never happen */

@@ -720,25 +716,6 @@ error1:
 			return -1;
 		}
 
-		while (!eth_getenv_enetaddr(s, dev->enetaddr)) {
-			/* Generate Private MAC addr if not set */
-			dev->enetaddr[0] = 0x02;
-			dev->enetaddr[1] = 0x50;
-			dev->enetaddr[2] = 0x43;
-#if defined (CONFIG_SKIP_LOCAL_MAC_RANDOMIZATION)
-			/* Generate fixed lower MAC half using devnum */
-			dev->enetaddr[3] = 0;
-			dev->enetaddr[4] = 0;
-			dev->enetaddr[5] = devnum;
-#else
-			/* Generate random lower MAC half */
-			dev->enetaddr[3] = get_random_hex();
-			dev->enetaddr[4] = get_random_hex();
-			dev->enetaddr[5] = get_random_hex();
-#endif
-			eth_setenv_enetaddr(s, dev->enetaddr);
-		}
-
 		dev->init = (void *)mvgbe_init;
 		dev->halt = (void *)mvgbe_halt;
 		dev->send = (void *)mvgbe_send;

Signature · 1 line

1.7.2.5

Mike Frysinger

2011-11-19 03:58 UTC #34 lore raw

PATCH v2 1/1 mvgbe: remove setting of ethaddr within the driver

drivers/net/mvgbe.c error1: · line 706

Quoted 6 lines · On Thursday 17 November 2011 17:52:29 Michael Walle wrote:

drivers/net/mvgbe.c +0 -0

 
 		switch (devnum) {
 		case 0:
 			dmvgbe->regs = (void *)MVGBE0_BASE;

unrelated, but usually this is pushed into an option to the init func ... the caller passes in the base address for the regs rather than the func trying to decode things itself.
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20111118/6f67a448/attachment.pgp>

Prafulla Wadaskar

2011-11-21 04:39 UTC #35 lore raw

PATCH v2 1/1 mvgbe: remove setting of ethaddr within the driver

In reply to Michael Walle

Quoted 89 lines

-----Original Message-----
From: Michael Walle [mailto:michael at walle.cc]
Sent: Friday, November 18, 2011 4:22 AM
To: u-boot at lists.denx.de
Cc: Wolfgang Denk; Michael Walle; Mike Frysinger; Prafulla Wadaskar; Valentin Longchamp; Eric Cooper; Jason Cooper; Siddarth Gore; Albert ARIBAUD; Simon Guinot
Subject: [PATCH v2] mvgbe: remove setting of ethaddr within the driver
A network driver should not touch the environment at all. This patch fixes
this behaviour by removing the code for setting a default/randomized MAC
address.
Signed-off-by Michael Walle <[email protected]>
Acked-by Mike Frysinger <[email protected]>
Acked-by Prafulla Wadaskar <[email protected]>
Cc Mike Frysinger <[email protected]>
Cc Prafulla Wadaskar <[email protected]>
Cc Valentin Longchamp <[email protected]>
Cc Eric Cooper <[email protected]>
Cc Jason Cooper <[email protected]>
Cc Siddarth Gore <[email protected]>
Cc Albert ARIBAUD <[email protected]>
Cc Simon Guinot <[email protected]>
---
 drivers/net/mvgbe.c |   23 -----------------------
 1 files changed, 0 insertions(+), 23 deletions(-)
drivers/net/mvgbe.c +0 ~~-24~~
index c7f7446..21be642 100644
@@ -647,7 +647,6 @@ int mvgbe_initialize(bd_t *bis)
 	struct mvgbe_device *dmvgbe;
 	struct eth_device *dev;
 	int devnum;
-	char *s;
 	u8 used_ports[MAX_MVGBE_DEVS] = CONFIG_MVGBE_PORTS;
 
 	for (devnum = 0; devnum < MAX_MVGBE_DEVS; devnum++) {
@@ -702,16 +701,13 @@ error1:
 		/* must be less than NAMESIZE (16) */
 		sprintf(dev->name, "egiga%d", devnum);
 
-		/* Extract the MAC address from the environment */
 		switch (devnum) {
 		case 0:
 			dmvgbe->regs = (void *)MVGBE0_BASE;
-			s = "ethaddr";
 			break;
 #if defined(MVGBE1_BASE)
 		case 1:
 			dmvgbe->regs = (void *)MVGBE1_BASE;
-			s = "eth1addr";
 			break;
 #endif
 		default:	/* this should never happen */
@@ -720,25 +716,6 @@ error1:
 			return -1;
 		}
 
-		while (!eth_getenv_enetaddr(s, dev->enetaddr)) {
-			/* Generate Private MAC addr if not set */
-			dev->enetaddr[0] = 0x02;
-			dev->enetaddr[1] = 0x50;
-			dev->enetaddr[2] = 0x43;
-#if defined (CONFIG_SKIP_LOCAL_MAC_RANDOMIZATION)
-			/* Generate fixed lower MAC half using devnum */
-			dev->enetaddr[3] = 0;
-			dev->enetaddr[4] = 0;
-			dev->enetaddr[5] = devnum;
-#else
-			/* Generate random lower MAC half */
-			dev->enetaddr[3] = get_random_hex();
-			dev->enetaddr[4] = get_random_hex();
-			dev->enetaddr[5] = get_random_hex();
-#endif
-			eth_setenv_enetaddr(s, dev->enetaddr);
-		}
-
 		dev->init = (void *)mvgbe_init;
 		dev->halt = (void *)mvgbe_halt;
 		dev->send = (void *)mvgbe_send;
--

Quoted 1 line

1.7.2.5

Ack for this patch

Acked-by Prafulla Wadaskar <[email protected]>

Regards..
Prafulla . . .

Wolfgang Denk

Applied

2011-12-20 21:55 UTC #36 lore raw

PATCH v2 1/1 mvgbe: remove setting of ethaddr within the driver

In reply to Michael Walle

Dear Michael Walle,

Quoted 15 lines · In message <[email protected]> you wrote:

A network driver should not touch the environment at all. This patch fixes this behaviour by removing the code for setting a default/randomized MAC address.
Signed-off-by Michael Walle <[email protected]>
Acked-by Mike Frysinger <[email protected]>
Acked-by Prafulla Wadaskar <[email protected]>
Cc Mike Frysinger <[email protected]>
Cc Prafulla Wadaskar <[email protected]>
Cc Valentin Longchamp <[email protected]>
Cc Eric Cooper <[email protected]>
Cc Jason Cooper <[email protected]>
Cc Siddarth Gore <[email protected]>
Cc Albert ARIBAUD <[email protected]>
Cc Simon Guinot <[email protected]>

Quoted 3 lines

---
 drivers/net/mvgbe.c |   23 -----------------------
 1 files changed, 0 insertions(+), 23 deletions(-)

Applied to "next" branch, thanks.

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
Any sufficiently advanced  technology  is  indistinguishable  from  a
rigged demo.                              - Andy Finkel, computer guy

Wolfgang Denk

Applied

2011-12-20 21:57 UTC #37 lore raw

PATCH v2 1/1 mvgbe: remove setting of ethaddr within the driver

In reply to Michael Walle

Dear Michael Walle,

Quoted 15 lines · In message <[email protected]> you wrote:

A network driver should not touch the environment at all. This patch fixes this behaviour by removing the code for setting a default/randomized MAC address.
Signed-off-by Michael Walle <[email protected]>
Acked-by Mike Frysinger <[email protected]>
Acked-by Prafulla Wadaskar <[email protected]>
Cc Mike Frysinger <[email protected]>
Cc Prafulla Wadaskar <[email protected]>
Cc Valentin Longchamp <[email protected]>
Cc Eric Cooper <[email protected]>
Cc Jason Cooper <[email protected]>
Cc Siddarth Gore <[email protected]>
Cc Albert ARIBAUD <[email protected]>
Cc Simon Guinot <[email protected]>

Quoted 3 lines

---
 drivers/net/mvgbe.c |   23 -----------------------
 1 files changed, 0 insertions(+), 23 deletions(-)

Applied to "next" branch, thanks.

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
It is a good thing for an uneducated man to read books of quotations.
                        - Sir Winston Churchill _My Early Life_ ch. 9

Mike Frysinger

2011-11-08 20:49 UTC #38 lore raw

PATCH 1/1 mvgbe: remove setting of ethaddr within the driver

In reply to Michael Walle

Quoted 21 lines · On Tuesday 08 November 2011 12:34:17 Michael Walle wrote:

Am Di, 8.11.2011, 16:17 schrieb Wolfgang Denk:
Quoted 8 lines
Quoted 6 lines · Michael Walle
Quoted 3 lines
Quoted 1 line
+#define CONFIG_ETH_ADDR 02:50:43:00:00:01
NAK to this. board configs are not allow to hardcode MACs.
mh, so why is there a CONFIG_ETH_ADDR macro? is it deprecated? README.enetaddr explicitly refers to that config option.
Quoted 6 lines
There are situations where it cannot be helped, like when programming the U-Boot image into a ROM (yes, a real ROM that cannot be changed any more by software alone).
But in almost all normal situations this must not be used. In any case, we will not accept it into mainline.
I have a Buffalo Linkstation Pro LS-XHL for which i plan to submit a BSP. On this board, the MAC address is saved within the environment, which is in turn stored in flash. Additionally this board does not have a serial port, thus the only available console is the network console.

Quoted 8 lines

If someone will mess up the environment, it is possible to clear it by holding a button upon powerup. Atm i have the CONFIG_ETH_ADDR set to a default MAC address to be able to use the network console with the default environment. To restore the original MAC address, a user have to manually replace ethaddr with the one printed on the case.
But now, if i can't use the CONFIG_ETH_ADDR, i think i'm stuck :) Or do i miss something? (Yeah there may be the MAC randomization, if accepted).

the "default" MAC address would collide with other Buffalo stations which have the "default" MAC address

by default, ethaddr is not writable, so people can't simply "set ethaddr xxx" to screw it up.
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20111108/df8cdb1b/attachment.pgp

Michael Walle

author

2011-11-08 22:45 UTC #39 lore raw

PATCH 1/1 mvgbe: remove setting of ethaddr within the driver

Am Dienstag 08 November 2011, 21:49:49 schrieb Mike Frysinger:

Quoted 28 lines · On Tuesday 08 November 2011 12:34:17 Michael Walle wrote:

Quoted 27 lines
Am Di, 8.11.2011, 16:17 schrieb Wolfgang Denk:
Quoted 8 lines
Quoted 6 lines · Michael Walle
Quoted 3 lines
Quoted 1 line
+#define CONFIG_ETH_ADDR 02:50:43:00:00:01
NAK to this. board configs are not allow to hardcode MACs.
mh, so why is there a CONFIG_ETH_ADDR macro? is it deprecated? README.enetaddr explicitly refers to that config option.
Quoted 6 lines
There are situations where it cannot be helped, like when programming the U-Boot image into a ROM (yes, a real ROM that cannot be changed any more by software alone).
But in almost all normal situations this must not be used. In any case, we will not accept it into mainline.
I have a Buffalo Linkstation Pro LS-XHL for which i plan to submit a BSP. On this board, the MAC address is saved within the environment, which is in turn stored in flash. Additionally this board does not have a serial port, thus the only available console is the network console.
If someone will mess up the environment, it is possible to clear it by holding a button upon powerup. Atm i have the CONFIG_ETH_ADDR set to a default MAC address to be able to use the network console with the default environment. To restore the original MAC address, a user have to manually replace ethaddr with the one printed on the case.

Quoted 5 lines

Quoted 2 lines
But now, if i can't use the CONFIG_ETH_ADDR, i think i'm stuck :) Or do i miss something? (Yeah there may be the MAC randomization, if accepted).
the "default" MAC address would collide with other Buffalo stations which have the "default" MAC address

yeah of course, but it is only a recovery mechanism. i would be happy with a generated mac address, too.

Quoted 2 lines

by default, ethaddr is not writable, so people can't simply "set ethaddr xxx" to screw it up.

well a user could screw it up in many other ways too ;)

there are plenty of boards which has CONFIG_ETHADDR and CONFIG_OVERWRITE_ETHADDR_ONCE set. This should exactly be the configuration i need, too.

btw only ethaddr is protected and not ethNaddr, which is a bug imho ;)

Signature · 1 line

Michael

Mike Frysinger

2011-11-09 05:36 UTC #40 lore raw

PATCH 1/1 mvgbe: remove setting of ethaddr within the driver

Quoted 1 line · On Tuesday 08 November 2011 17:45:12 Michael Walle wrote:

btw only ethaddr is protected and not ethNaddr, which is a bug imho ;)

yeah, it is, but i've ignored it as all my boards have only had 1 eth dev. feel free to post a patch :).
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20111109/d37c8cfc/attachment.pgp