All of lore.kernel.org
 help / color / mirror / Atom feed
* [U-Boot] [PATCH] pxe: get ethaddr from the current device instead of env
@ 2011-12-06  0:04 Rob Herring
  2011-12-06  3:50 ` Mike Frysinger
                   ` (3 more replies)
  0 siblings, 4 replies; 14+ messages in thread
From: Rob Herring @ 2011-12-06  0:04 UTC (permalink / raw)
  To: u-boot

From: Rob Herring <rob.herring@calxeda.com>

The env variable "ethaddr" may not be set, so get the address from the
eth_device struct instead. This also enables pxe for secondary ethernet
devices.

Signed-off-by: Rob Herring <rob.herring@calxeda.com>
---
 common/cmd_pxe.c |   31 ++++++++++++-------------------
 1 files changed, 12 insertions(+), 19 deletions(-)

diff --git a/common/cmd_pxe.c b/common/cmd_pxe.c
index 3efd700..f14ef89 100644
--- a/common/cmd_pxe.c
+++ b/common/cmd_pxe.c
@@ -56,38 +56,31 @@ static char *from_env(char *envvar)
  */
 static int format_mac_pxe(char *outbuf, size_t outbuf_len)
 {
-	size_t ethaddr_len;
-	char *p, *ethaddr;
-
-	ethaddr = from_env("ethaddr");
-
-	if (!ethaddr)
-		return -ENOENT;
-
-	ethaddr_len = strlen(ethaddr);
+	char *p;
+	struct eth_device *dev;
 
 	/*
 	 * ethaddr_len + 4 gives room for "01-", ethaddr, and a NUL byte at
 	 * the end.
 	 */
-	if (outbuf_len < ethaddr_len + 4) {
-		printf("outbuf is too small (%d < %d)\n",
-				outbuf_len, ethaddr_len + 4);
-
+	if (outbuf_len < 21) {
+		printf("outbuf is too small (%d < 21)\n", outbuf_len);
 		return -EINVAL;
 	}
 
-	strcpy(outbuf, "01-");
+	eth_set_current();
+	dev = eth_get_dev();
+	if (!dev)
+		return -ENODEV;
 
-	for (p = outbuf + 3; *ethaddr; ethaddr++, p++) {
-		if (*ethaddr == ':')
+	sprintf(outbuf, "01-%pM", dev->enetaddr);
+	for (p = outbuf + 3; *p; p++) {
+		if (*p == ':')
 			*p = '-';
 		else
-			*p = tolower(*ethaddr);
+			*p = tolower(*p);
 	}
 
-	*p = '\0';
-
 	return 1;
 }
 
-- 
1.7.5.4

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [U-Boot] [PATCH] pxe: get ethaddr from the current device instead of env
  2011-12-06  0:04 [U-Boot] [PATCH] pxe: get ethaddr from the current device instead of env Rob Herring
@ 2011-12-06  3:50 ` Mike Frysinger
  2011-12-06 10:41 ` Wolfgang Denk
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 14+ messages in thread
From: Mike Frysinger @ 2011-12-06  3:50 UTC (permalink / raw)
  To: u-boot

On Monday 05 December 2011 19:04:33 Rob Herring wrote:
> The env variable "ethaddr" may not be set, so get the address from the
> eth_device struct instead. This also enables pxe for secondary ethernet
> devices.

NAK: this won't work either ;).  the API contract is that the env is the 
canonical storage, and eth_device->enetaddr is there only for the network 
device drivers themselves: they seed it during registration, and then use it 
when starting up as the common layer has taken care of syncing from the env.

what you probably want to do is update net/eth.c's eth_register() to see if 
dev->enetaddr is valid (is_valid_ether_addr), and if the env addr is not set 
for that device, and if not, set the env with it.

then you can assume in cmd_pxe.c that the env is setup.
-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/20111205/71a33330/attachment.pgp>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [U-Boot] [PATCH] pxe: get ethaddr from the current device instead of env
  2011-12-06  0:04 [U-Boot] [PATCH] pxe: get ethaddr from the current device instead of env Rob Herring
  2011-12-06  3:50 ` Mike Frysinger
@ 2011-12-06 10:41 ` Wolfgang Denk
  2011-12-06 19:24 ` [U-Boot] [PATCH v2] net: allow setting env enetaddr from net device setting Rob Herring
  2012-04-15  4:06 ` [U-Boot] [PATCH v5] " Rob Herring
  3 siblings, 0 replies; 14+ messages in thread
From: Wolfgang Denk @ 2011-12-06 10:41 UTC (permalink / raw)
  To: u-boot

Dear Rob Herring,

In message <1323129873-8786-1-git-send-email-robherring2@gmail.com> you wrote:
> 
> The env variable "ethaddr" may not be set, so get the address from the
> eth_device struct instead. This also enables pxe for secondary ethernet
> devices.

This is not correct. The "ethaddr" environment variable shall always
take precedence. Feel free to _additionally_ test other places that
store this information.  If the "ethaddr" environment variable is not
set, then this alternative value can be used as default. In this case
a warning should be printed.  If both settings exist and differ, the
"ethaddr" environment variable shall be used, and a warning be
printed, too.

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 at denx.de
The shortest unit of time in the multiverse is the News York  Second,
defined  as  the  period  of  time between the traffic lights turning
green and the cab behind you honking.
                                - Terry Pratchett, _Lords and Ladies_

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [U-Boot] [PATCH v2] net: allow setting env enetaddr from net device setting
  2011-12-06  0:04 [U-Boot] [PATCH] pxe: get ethaddr from the current device instead of env Rob Herring
  2011-12-06  3:50 ` Mike Frysinger
  2011-12-06 10:41 ` Wolfgang Denk
@ 2011-12-06 19:24 ` Rob Herring
  2012-01-13 20:36   ` Wolfgang Denk
  2012-02-01 23:27   ` [U-Boot] [PATCH v3] " Rob Herring
  2012-04-15  4:06 ` [U-Boot] [PATCH v5] " Rob Herring
  3 siblings, 2 replies; 14+ messages in thread
From: Rob Herring @ 2011-12-06 19:24 UTC (permalink / raw)
  To: u-boot

From: Rob Herring <rob.herring@calxeda.com>

If the net driver has setup a valid ethernet address and an ethernet
address is not set in the environment already, then set the environment
variables from the net driver setting

This enables pxe booting on boards which don't set ethaddr env variable.

Signed-off-by: Rob Herring <rob.herring@calxeda.com>
---
v2:
- Re-wrote to always setup ethaddr env variables

 doc/README.enetaddr |    4 +++-
 net/eth.c           |   19 ++++++++++++++++---
 2 files changed, 19 insertions(+), 4 deletions(-)

diff --git a/doc/README.enetaddr b/doc/README.enetaddr
index 2d8e24f..6c61817 100644
--- a/doc/README.enetaddr
+++ b/doc/README.enetaddr
@@ -32,7 +32,9 @@ Correct flow of setting up the MAC address (summarized):
 
 1. Read from hardware in initialize() function
 2. Read from environment in net/eth.c after initialize()
-3. Give priority to the value in the environment if a conflict
+3. Write value to environment if setup in struct eth_device->enetaddr by driver
+   initialize() function. Give priority to the value in the environment if a
+   conflict.
 4. Program the address into hardware if the following conditions are met:
 	a) The relevant driver has a 'write_addr' function
 	b) The user hasn't set an 'ethmacskip' environment variable
diff --git a/net/eth.c b/net/eth.c
index 4280d6d..e3d8777 100644
--- a/net/eth.c
+++ b/net/eth.c
@@ -62,6 +62,15 @@ int eth_getenv_enetaddr_by_index(const char *base_name, int index,
 	return eth_getenv_enetaddr(enetvar, enetaddr);
 }
 
+int eth_setenv_enetaddr_by_index(const char *base_name, int index,
+				 uchar *enetaddr)
+{
+	char enetvar[32];
+	sprintf(enetvar, index ? "%s%daddr" : "%saddr", base_name, index);
+	return eth_setenv_enetaddr(enetvar, enetaddr);
+}
+
+
 static int eth_mac_skip(int index)
 {
 	char enetvar[15];
@@ -190,9 +199,13 @@ int eth_write_hwaddr(struct eth_device *dev, const char *base_name,
 	unsigned char env_enetaddr[6];
 	int ret = 0;
 
-	if (!eth_getenv_enetaddr_by_index(base_name, eth_number, env_enetaddr))
-		return -1;
-
+	if (!eth_getenv_enetaddr_by_index(base_name, eth_number, env_enetaddr)) {
+		if (!is_valid_ether_addr(dev->enetaddr))
+			return -1;
+		eth_setenv_enetaddr_by_index(base_name, eth_number,
+					     dev->enetaddr);
+		memcpy(env_enetaddr, dev->enetaddr, 6);
+	}
 	if (memcmp(env_enetaddr, "\0\0\0\0\0\0", 6)) {
 		if (memcmp(dev->enetaddr, "\0\0\0\0\0\0", 6) &&
 			memcmp(dev->enetaddr, env_enetaddr, 6)) {
-- 
1.7.5.4

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [U-Boot] [PATCH v2] net: allow setting env enetaddr from net device setting
  2011-12-06 19:24 ` [U-Boot] [PATCH v2] net: allow setting env enetaddr from net device setting Rob Herring
@ 2012-01-13 20:36   ` Wolfgang Denk
  2012-02-01 23:27   ` [U-Boot] [PATCH v3] " Rob Herring
  1 sibling, 0 replies; 14+ messages in thread
From: Wolfgang Denk @ 2012-01-13 20:36 UTC (permalink / raw)
  To: u-boot

Dear Rob Herring,

In message <1323199478-21001-1-git-send-email-robherring2@gmail.com> you wrote:
> From: Rob Herring <rob.herring@calxeda.com>
> 
> If the net driver has setup a valid ethernet address and an ethernet
> address is not set in the environment already, then set the environment
> variables from the net driver setting
> 
> This enables pxe booting on boards which don't set ethaddr env variable.
> 
> Signed-off-by: Rob Herring <rob.herring@calxeda.com>
> ---
> v2:
> - Re-wrote to always setup ethaddr env variables
> 
>  doc/README.enetaddr |    4 +++-
>  net/eth.c           |   19 ++++++++++++++++---
>  2 files changed, 19 insertions(+), 4 deletions(-)

I asked before:

... If the "ethaddr" environment variable is not
set, then this alternative value can be used as default. In this case
a warning should be printed.  If both settings exist and differ, the
"ethaddr" environment variable shall be used, and a warning be
printed, too.

I do not see any wanings in your patch?

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 at denx.de
The only person who always got his work done by Friday
                                                 was Robinson Crusoe.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [U-Boot] [PATCH v3] net: allow setting env enetaddr from net device setting
  2011-12-06 19:24 ` [U-Boot] [PATCH v2] net: allow setting env enetaddr from net device setting Rob Herring
  2012-01-13 20:36   ` Wolfgang Denk
@ 2012-02-01 23:27   ` Rob Herring
  2012-03-06 13:37     ` Rob Herring
                       ` (2 more replies)
  1 sibling, 3 replies; 14+ messages in thread
From: Rob Herring @ 2012-02-01 23:27 UTC (permalink / raw)
  To: u-boot

From: Rob Herring <rob.herring@calxeda.com>

If the net driver has setup a valid ethernet address and an ethernet
address is not set in the environment already, then set the environment
variables from the net driver setting.

This enables pxe booting on boards which don't set ethaddr env variable.

Signed-off-by: Rob Herring <rob.herring@calxeda.com>
---
Wolfgang,

This now prints a warning. If both env and device mac's are set, then the
behavior is unchanged and the env setting is used.

v3:
- print a warning if using mac address from the net device

v2:
- Re-wrote to always setup ethaddr env variables

Rob

 doc/README.enetaddr |    4 +++-
 net/eth.c           |   21 ++++++++++++++++++---
 2 files changed, 21 insertions(+), 4 deletions(-)

diff --git a/doc/README.enetaddr b/doc/README.enetaddr
index 2d8e24f..6c61817 100644
--- a/doc/README.enetaddr
+++ b/doc/README.enetaddr
@@ -32,7 +32,9 @@ Correct flow of setting up the MAC address (summarized):
 
 1. Read from hardware in initialize() function
 2. Read from environment in net/eth.c after initialize()
-3. Give priority to the value in the environment if a conflict
+3. Write value to environment if setup in struct eth_device->enetaddr by driver
+   initialize() function. Give priority to the value in the environment if a
+   conflict.
 4. Program the address into hardware if the following conditions are met:
 	a) The relevant driver has a 'write_addr' function
 	b) The user hasn't set an 'ethmacskip' environment variable
diff --git a/net/eth.c b/net/eth.c
index b4b9b43..f75a944 100644
--- a/net/eth.c
+++ b/net/eth.c
@@ -62,6 +62,15 @@ int eth_getenv_enetaddr_by_index(const char *base_name, int index,
 	return eth_getenv_enetaddr(enetvar, enetaddr);
 }
 
+int eth_setenv_enetaddr_by_index(const char *base_name, int index,
+				 uchar *enetaddr)
+{
+	char enetvar[32];
+	sprintf(enetvar, index ? "%s%daddr" : "%saddr", base_name, index);
+	return eth_setenv_enetaddr(enetvar, enetaddr);
+}
+
+
 static int eth_mac_skip(int index)
 {
 	char enetvar[15];
@@ -175,9 +184,15 @@ int eth_write_hwaddr(struct eth_device *dev, const char *base_name,
 	unsigned char env_enetaddr[6];
 	int ret = 0;
 
-	if (!eth_getenv_enetaddr_by_index(base_name, eth_number, env_enetaddr))
-		return -1;
-
+	if (!eth_getenv_enetaddr_by_index(base_name, eth_number, env_enetaddr)) {
+		if (!is_valid_ether_addr(dev->enetaddr))
+			return -1;
+		eth_setenv_enetaddr_by_index(base_name, eth_number,
+					     dev->enetaddr);
+		printf("\nWarning: %s using MAC address from net device\n",
+			dev->name);
+		return 0;
+	}
 	if (memcmp(env_enetaddr, "\0\0\0\0\0\0", 6)) {
 		if (memcmp(dev->enetaddr, "\0\0\0\0\0\0", 6) &&
 			memcmp(dev->enetaddr, env_enetaddr, 6)) {
-- 
1.7.5.4

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [U-Boot] [PATCH v3] net: allow setting env enetaddr from net device setting
  2012-02-01 23:27   ` [U-Boot] [PATCH v3] " Rob Herring
@ 2012-03-06 13:37     ` Rob Herring
  2012-03-06 19:30     ` Wolfgang Denk
  2012-03-07  3:03     ` [U-Boot] [PATCH v4] " Rob Herring
  2 siblings, 0 replies; 14+ messages in thread
From: Rob Herring @ 2012-03-06 13:37 UTC (permalink / raw)
  To: u-boot

Wolfgang, Mike,

On 02/01/2012 05:27 PM, Rob Herring wrote:
> From: Rob Herring <rob.herring@calxeda.com>
> 
> If the net driver has setup a valid ethernet address and an ethernet
> address is not set in the environment already, then set the environment
> variables from the net driver setting.
> 
> This enables pxe booting on boards which don't set ethaddr env variable.
> 
> Signed-off-by: Rob Herring <rob.herring@calxeda.com>
> ---
> Wolfgang,
> 
> This now prints a warning. If both env and device mac's are set, then the
> behavior is unchanged and the env setting is used.
> 
> v3:
> - print a warning if using mac address from the net device
> 
> v2:
> - Re-wrote to always setup ethaddr env variables
> 

Any comments on this?

Rob

> Rob
> 
>  doc/README.enetaddr |    4 +++-
>  net/eth.c           |   21 ++++++++++++++++++---
>  2 files changed, 21 insertions(+), 4 deletions(-)
> 
> diff --git a/doc/README.enetaddr b/doc/README.enetaddr
> index 2d8e24f..6c61817 100644
> --- a/doc/README.enetaddr
> +++ b/doc/README.enetaddr
> @@ -32,7 +32,9 @@ Correct flow of setting up the MAC address (summarized):
>  
>  1. Read from hardware in initialize() function
>  2. Read from environment in net/eth.c after initialize()
> -3. Give priority to the value in the environment if a conflict
> +3. Write value to environment if setup in struct eth_device->enetaddr by driver
> +   initialize() function. Give priority to the value in the environment if a
> +   conflict.
>  4. Program the address into hardware if the following conditions are met:
>  	a) The relevant driver has a 'write_addr' function
>  	b) The user hasn't set an 'ethmacskip' environment variable
> diff --git a/net/eth.c b/net/eth.c
> index b4b9b43..f75a944 100644
> --- a/net/eth.c
> +++ b/net/eth.c
> @@ -62,6 +62,15 @@ int eth_getenv_enetaddr_by_index(const char *base_name, int index,
>  	return eth_getenv_enetaddr(enetvar, enetaddr);
>  }
>  
> +int eth_setenv_enetaddr_by_index(const char *base_name, int index,
> +				 uchar *enetaddr)
> +{
> +	char enetvar[32];
> +	sprintf(enetvar, index ? "%s%daddr" : "%saddr", base_name, index);
> +	return eth_setenv_enetaddr(enetvar, enetaddr);
> +}
> +
> +
>  static int eth_mac_skip(int index)
>  {
>  	char enetvar[15];
> @@ -175,9 +184,15 @@ int eth_write_hwaddr(struct eth_device *dev, const char *base_name,
>  	unsigned char env_enetaddr[6];
>  	int ret = 0;
>  
> -	if (!eth_getenv_enetaddr_by_index(base_name, eth_number, env_enetaddr))
> -		return -1;
> -
> +	if (!eth_getenv_enetaddr_by_index(base_name, eth_number, env_enetaddr)) {
> +		if (!is_valid_ether_addr(dev->enetaddr))
> +			return -1;
> +		eth_setenv_enetaddr_by_index(base_name, eth_number,
> +					     dev->enetaddr);
> +		printf("\nWarning: %s using MAC address from net device\n",
> +			dev->name);
> +		return 0;
> +	}
>  	if (memcmp(env_enetaddr, "\0\0\0\0\0\0", 6)) {
>  		if (memcmp(dev->enetaddr, "\0\0\0\0\0\0", 6) &&
>  			memcmp(dev->enetaddr, env_enetaddr, 6)) {

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [U-Boot] [PATCH v3] net: allow setting env enetaddr from net device setting
  2012-02-01 23:27   ` [U-Boot] [PATCH v3] " Rob Herring
  2012-03-06 13:37     ` Rob Herring
@ 2012-03-06 19:30     ` Wolfgang Denk
  2012-03-06 20:01       ` Rob Herring
  2012-03-07  3:03     ` [U-Boot] [PATCH v4] " Rob Herring
  2 siblings, 1 reply; 14+ messages in thread
From: Wolfgang Denk @ 2012-03-06 19:30 UTC (permalink / raw)
  To: u-boot

Dear Rob Herring,

In message <1328138854-28612-1-git-send-email-robherring2@gmail.com> you wrote:
>
> --- a/doc/README.enetaddr
> +++ b/doc/README.enetaddr
> @@ -32,7 +32,9 @@ Correct flow of setting up the MAC address (summarized):
>  
>  1. Read from hardware in initialize() function
>  2. Read from environment in net/eth.c after initialize()
> -3. Give priority to the value in the environment if a conflict
> +3. Write value to environment if setup in struct eth_device->enetaddr by driver
> +   initialize() function. Give priority to the value in the environment if a
> +   conflict.

Sorry, but this description is not correct.  You say here that the
environment variable should always be written, but this is not the
case.  Only if it does not exist it shall be set.  If it exists, it
shall only be read, and in case of inconsistencies a warning shall be
printed.


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 at denx.de
Too many people are ready to carry the stool when the piano needs  to
be moved.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [U-Boot] [PATCH v3] net: allow setting env enetaddr from net device setting
  2012-03-06 19:30     ` Wolfgang Denk
@ 2012-03-06 20:01       ` Rob Herring
  2012-03-06 20:33         ` Wolfgang Denk
  0 siblings, 1 reply; 14+ messages in thread
From: Rob Herring @ 2012-03-06 20:01 UTC (permalink / raw)
  To: u-boot

On 03/06/2012 01:30 PM, Wolfgang Denk wrote:
> Dear Rob Herring,
> 
> In message <1328138854-28612-1-git-send-email-robherring2@gmail.com> you wrote:
>>
>> --- a/doc/README.enetaddr
>> +++ b/doc/README.enetaddr
>> @@ -32,7 +32,9 @@ Correct flow of setting up the MAC address (summarized):
>>  
>>  1. Read from hardware in initialize() function
>>  2. Read from environment in net/eth.c after initialize()
>> -3. Give priority to the value in the environment if a conflict
>> +3. Write value to environment if setup in struct eth_device->enetaddr by driver
>> +   initialize() function. Give priority to the value in the environment if a
>> +   conflict.
> 
> Sorry, but this description is not correct.  You say here that the
> environment variable should always be written, but this is not the
> case.  Only if it does not exist it shall be set.  If it exists, it
> shall only be read, and in case of inconsistencies a warning shall be
> printed.
> 

How about this:

3. Always use the value in the environment if there is a conflict. If
the environment variable is not set and the driver initialized struct
eth_device->enetaddr, then print a warning and set the environment
variable to initialized by the driver.

Rob

> 
> Best regards,
> 
> Wolfgang Denk
> 

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [U-Boot] [PATCH v3] net: allow setting env enetaddr from net device setting
  2012-03-06 20:01       ` Rob Herring
@ 2012-03-06 20:33         ` Wolfgang Denk
  0 siblings, 0 replies; 14+ messages in thread
From: Wolfgang Denk @ 2012-03-06 20:33 UTC (permalink / raw)
  To: u-boot

Dear Rob,

In message <4F566D05.5020809@gmail.com> you wrote:
>
> >> +3. Write value to environment if setup in struct eth_device->enetaddr by driver
> >> +   initialize() function. Give priority to the value in the environment if a
> >> +   conflict.
> > 
> > Sorry, but this description is not correct.  You say here that the
> > environment variable should always be written, but this is not the
> > case.  Only if it does not exist it shall be set.  If it exists, it
> > shall only be read, and in case of inconsistencies a warning shall be
> > printed.
> 
> How about this:
> 
> 3. Always use the value in the environment if there is a conflict. If
> the environment variable is not set and the driver initialized struct
> eth_device->enetaddr, then print a warning and set the environment
> variable to initialized by the driver.

I find you make it difficult to read without need by explaining it
backwards.

	The environment variable will be compared to the driver
	initialized struct eth_device->enetaddr. If they differ, a
	warning is printed, an the environment variable will be used
	unchanged.

	If the environment variable is not set, it will be initialized
	from eth_device->enetaddr, and a warning will be printed.

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 at denx.de
Ninety-Ninety Rule of Project Schedules:
        The first ninety percent of the task takes ninety percent of
the time, and the last ten percent takes the other ninety percent.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [U-Boot] [PATCH v4] net: allow setting env enetaddr from net device setting
  2012-02-01 23:27   ` [U-Boot] [PATCH v3] " Rob Herring
  2012-03-06 13:37     ` Rob Herring
  2012-03-06 19:30     ` Wolfgang Denk
@ 2012-03-07  3:03     ` Rob Herring
  2012-04-04 15:06       ` Joe Hershberger
  2 siblings, 1 reply; 14+ messages in thread
From: Rob Herring @ 2012-03-07  3:03 UTC (permalink / raw)
  To: u-boot

From: Rob Herring <rob.herring@calxeda.com>

If the net driver has setup a valid ethernet address and an ethernet
address is not set in the environment already, then set the environment
variables from the net driver setting.

This enables pxe booting on boards which don't set ethaddr env variable.

Signed-off-by: Rob Herring <rob.herring@calxeda.com>
---
v4:
- rewrite of documentation from Wolfgang

v3:
- print a warning if using mac address from the net device

v2:
- Re-wrote to always setup ethaddr env variables

 doc/README.enetaddr |    6 +++++-
 net/eth.c           |   21 ++++++++++++++++++---
 2 files changed, 23 insertions(+), 4 deletions(-)

diff --git a/doc/README.enetaddr b/doc/README.enetaddr
index 2d8e24f..24b25f7 100644
--- a/doc/README.enetaddr
+++ b/doc/README.enetaddr
@@ -32,7 +32,11 @@ Correct flow of setting up the MAC address (summarized):
 
 1. Read from hardware in initialize() function
 2. Read from environment in net/eth.c after initialize()
-3. Give priority to the value in the environment if a conflict
+3. The environment variable will be compared to the driver initialized
+   struct eth_device->enetaddr. If they differ, a warning is printed, and the
+   environment variable will be used unchanged.
+   If the environment variable is not set, it will be initialized from 
+   eth_device->enetaddr, and a warning will be printed.
 4. Program the address into hardware if the following conditions are met:
 	a) The relevant driver has a 'write_addr' function
 	b) The user hasn't set an 'ethmacskip' environment variable
diff --git a/net/eth.c b/net/eth.c
index f14767b..22cd37d 100644
--- a/net/eth.c
+++ b/net/eth.c
@@ -62,6 +62,15 @@ int eth_getenv_enetaddr_by_index(const char *base_name, int index,
 	return eth_getenv_enetaddr(enetvar, enetaddr);
 }
 
+int eth_setenv_enetaddr_by_index(const char *base_name, int index,
+				 uchar *enetaddr)
+{
+	char enetvar[32];
+	sprintf(enetvar, index ? "%s%daddr" : "%saddr", base_name, index);
+	return eth_setenv_enetaddr(enetvar, enetaddr);
+}
+
+
 static int eth_mac_skip(int index)
 {
 	char enetvar[15];
@@ -172,9 +181,15 @@ int eth_write_hwaddr(struct eth_device *dev, const char *base_name,
 	unsigned char env_enetaddr[6];
 	int ret = 0;
 
-	if (!eth_getenv_enetaddr_by_index(base_name, eth_number, env_enetaddr))
-		return -1;
-
+	if (!eth_getenv_enetaddr_by_index(base_name, eth_number, env_enetaddr)) {
+		if (!is_valid_ether_addr(dev->enetaddr))
+			return -1;
+		eth_setenv_enetaddr_by_index(base_name, eth_number,
+					     dev->enetaddr);
+		printf("\nWarning: %s using MAC address from net device\n",
+			dev->name);
+		return 0;
+	}
 	if (memcmp(env_enetaddr, "\0\0\0\0\0\0", 6)) {
 		if (memcmp(dev->enetaddr, "\0\0\0\0\0\0", 6) &&
 			memcmp(dev->enetaddr, env_enetaddr, 6)) {
-- 
1.7.5.4

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [U-Boot] [PATCH v4] net: allow setting env enetaddr from net device setting
  2012-03-07  3:03     ` [U-Boot] [PATCH v4] " Rob Herring
@ 2012-04-04 15:06       ` Joe Hershberger
  0 siblings, 0 replies; 14+ messages in thread
From: Joe Hershberger @ 2012-04-04 15:06 UTC (permalink / raw)
  To: u-boot

Hi Rob,

On Tue, Mar 6, 2012 at 9:03 PM, Rob Herring <robherring2@gmail.com> wrote:
> From: Rob Herring <rob.herring@calxeda.com>
>
> If the net driver has setup a valid ethernet address and an ethernet
> address is not set in the environment already, then set the environment
> variables from the net driver setting.
>
> This enables pxe booting on boards which don't set ethaddr env variable.
>
> Signed-off-by: Rob Herring <rob.herring@calxeda.com>
> ---
> v4:
> - rewrite of documentation from Wolfgang
>
> v3:
> - print a warning if using mac address from the net device
>
> v2:
> - Re-wrote to always setup ethaddr env variables
>
> ?doc/README.enetaddr | ? ?6 +++++-
> ?net/eth.c ? ? ? ? ? | ? 21 ++++++++++++++++++---
> ?2 files changed, 23 insertions(+), 4 deletions(-)

checkpatch.pl failures:

-------------------------------------

ERROR: trailing whitespace
#48: FILE: doc/README.enetaddr:38:
+   If the environment variable is not set, it will be initialized from $

WARNING: line over 80 characters
#80: FILE: net/eth.c:184:
+       if (!eth_getenv_enetaddr_by_index(base_name, eth_number,
env_enetaddr)) {

total: 1 errors, 1 warnings, 45 lines checked

NOTE: whitespace errors detected, you may wish to use scripts/cleanpatch or
      scripts/cleanfile

NOTE: Ignored message types: COMPLEX_MACRO CONSIDER_KSTRTO MINMAX
MULTISTATEMENT_MACRO_USE_DO_WHILE

U-Boot-v4-net-allow-setting-env-enetaddr-from-net-device-setting.patch
has style problems, please review.

----------------------

Also, it seems that just because the enetaddr was read from dev
(possibly from an eeprom or elsewhere) doesn't mean that the MAC
doesn't need you to call write_hwaddr().  I think you shouldn't return
0 if you use the net device's addr, but rather should make the if
(memcmp... into an else if.

-Joe

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [U-Boot] [PATCH v5] net: allow setting env enetaddr from net device setting
  2011-12-06  0:04 [U-Boot] [PATCH] pxe: get ethaddr from the current device instead of env Rob Herring
                   ` (2 preceding siblings ...)
  2011-12-06 19:24 ` [U-Boot] [PATCH v2] net: allow setting env enetaddr from net device setting Rob Herring
@ 2012-04-15  4:06 ` Rob Herring
  2012-07-11 18:30   ` Joe Hershberger
  3 siblings, 1 reply; 14+ messages in thread
From: Rob Herring @ 2012-04-15  4:06 UTC (permalink / raw)
  To: u-boot

From: Rob Herring <rob.herring@calxeda.com>

If the net driver has setup a valid ethernet address and an ethernet
address is not set in the environment already, then set the environment
variables from the net driver setting.

This enables pxe booting on boards which don't set ethaddr env variable.

Signed-off-by: Rob Herring <rob.herring@calxeda.com>
---
v5:
- Based on Joe's comments and new commit 6937664426446b763 "net/eth.c: fix
  eth_write_hwaddr() to use dev->enetaddr as fall back", moved the setting
  of ethaddr and allow the driver write_hwaddr to be called.
- fixed trailing whitespace in documentation

v4:
- rewrite of documentation from Wolfgang

v3:
- print a warning if using mac address from the net device

v2:
- Re-wrote to always setup ethaddr env variables

 doc/README.enetaddr |    6 +++++-
 net/eth.c           |   14 ++++++++++++++
 2 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/doc/README.enetaddr b/doc/README.enetaddr
index 2d8e24f..1eaeaf9 100644
--- a/doc/README.enetaddr
+++ b/doc/README.enetaddr
@@ -32,7 +32,11 @@ Correct flow of setting up the MAC address (summarized):
 
 1. Read from hardware in initialize() function
 2. Read from environment in net/eth.c after initialize()
-3. Give priority to the value in the environment if a conflict
+3. The environment variable will be compared to the driver initialized
+   struct eth_device->enetaddr. If they differ, a warning is printed, and the
+   environment variable will be used unchanged.
+   If the environment variable is not set, it will be initialized from
+   eth_device->enetaddr, and a warning will be printed.
 4. Program the address into hardware if the following conditions are met:
 	a) The relevant driver has a 'write_addr' function
 	b) The user hasn't set an 'ethmacskip' environment variable
diff --git a/net/eth.c b/net/eth.c
index 3eeb908..d4b94a4 100644
--- a/net/eth.c
+++ b/net/eth.c
@@ -62,6 +62,15 @@ int eth_getenv_enetaddr_by_index(const char *base_name, int index,
 	return eth_getenv_enetaddr(enetvar, enetaddr);
 }
 
+int eth_setenv_enetaddr_by_index(const char *base_name, int index,
+				 uchar *enetaddr)
+{
+	char enetvar[32];
+	sprintf(enetvar, index ? "%s%daddr" : "%saddr", base_name, index);
+	return eth_setenv_enetaddr(enetvar, enetaddr);
+}
+
+
 static int eth_mac_skip(int index)
 {
 	char enetvar[15];
@@ -186,6 +195,11 @@ int eth_write_hwaddr(struct eth_device *dev, const char *base_name,
 		}
 
 		memcpy(dev->enetaddr, env_enetaddr, 6);
+	} else if (is_valid_ether_addr(dev->enetaddr)) {
+		eth_setenv_enetaddr_by_index(base_name, eth_number,
+					     dev->enetaddr);
+		printf("\nWarning: %s using MAC address from net device\n",
+			dev->name);
 	}
 
 	if (dev->write_hwaddr &&
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [U-Boot] [PATCH v5] net: allow setting env enetaddr from net device setting
  2012-04-15  4:06 ` [U-Boot] [PATCH v5] " Rob Herring
@ 2012-07-11 18:30   ` Joe Hershberger
  0 siblings, 0 replies; 14+ messages in thread
From: Joe Hershberger @ 2012-07-11 18:30 UTC (permalink / raw)
  To: u-boot

Hi Rob,

On Sat, Apr 14, 2012 at 11:06 PM, Rob Herring <robherring2@gmail.com> wrote:
> From: Rob Herring <rob.herring@calxeda.com>
>
> If the net driver has setup a valid ethernet address and an ethernet
> address is not set in the environment already, then set the environment
> variables from the net driver setting.
>
> This enables pxe booting on boards which don't set ethaddr env variable.
>
> Signed-off-by: Rob Herring <rob.herring@calxeda.com>
> ---

Applied to next, thanks.

-Joe

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2012-07-11 18:30 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-12-06  0:04 [U-Boot] [PATCH] pxe: get ethaddr from the current device instead of env Rob Herring
2011-12-06  3:50 ` Mike Frysinger
2011-12-06 10:41 ` Wolfgang Denk
2011-12-06 19:24 ` [U-Boot] [PATCH v2] net: allow setting env enetaddr from net device setting Rob Herring
2012-01-13 20:36   ` Wolfgang Denk
2012-02-01 23:27   ` [U-Boot] [PATCH v3] " Rob Herring
2012-03-06 13:37     ` Rob Herring
2012-03-06 19:30     ` Wolfgang Denk
2012-03-06 20:01       ` Rob Herring
2012-03-06 20:33         ` Wolfgang Denk
2012-03-07  3:03     ` [U-Boot] [PATCH v4] " Rob Herring
2012-04-04 15:06       ` Joe Hershberger
2012-04-15  4:06 ` [U-Boot] [PATCH v5] " Rob Herring
2012-07-11 18:30   ` Joe Hershberger

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.