Re: [U-Boot] [PATCH] ARM: fdt support: Add usbethaddr as an acceptable MAC

2013-10-09 Thread Dan Murphy
All

On 10/02/2013 03:21 PM, Dan Murphy wrote:
 Tom

 On 10/02/2013 03:19 PM, Tom Rini wrote:
 On Wed, Oct 02, 2013 at 03:14:26PM -0500, Dan Murphy wrote:
 Tom

 On 10/02/2013 02:19 PM, Tom Rini wrote:
 On Wed, Oct 02, 2013 at 02:00:15PM -0500, Dan Murphy wrote:

 A board that has a USB ethernet device only may set the usbetheraddr
 and not the ethaddr.
 ethaddr will be the default MAC address that is chosen and if that
 is not populated then the usbethaddr is looked at.  If neither are set
 then then device tree blob is not modified.

 Signed-off-by: Dan Murphy dmur...@ti.com
 ---
  common/fdt_support.c |   12 +++-
  1 file changed, 11 insertions(+), 1 deletion(-)

 diff --git a/common/fdt_support.c b/common/fdt_support.c
 index b034c98..fef7e60 100644
 --- a/common/fdt_support.c
 +++ b/common/fdt_support.c
 @@ -450,8 +450,18 @@ void fdt_fixup_ethernet(void *fdt)
   if (node  0)
   return;
  
 + if (!getenv(ethaddr)) {
 + if (getenv(usbethaddr)) {
 + strcpy(mac, usbethaddr);
 + } else {
 + debug(No ethernet MAC Address defined\n);
 + return;
 + }
 + } else {
 + strcpy(mac, ethaddr);
 + }
 +
   i = 0;
 - strcpy(mac, ethaddr);
   while ((tmp = getenv(mac)) != NULL) {
   sprintf(enet, ethernet%d, i);
   path = fdt_getprop(fdt, node, enet, NULL);
 The problem is we may well have both.  I think we need to re-work the
 function slightly to be:
 while ((tmp = getenv(mac)) != NULL) {
   do_fdt_fixup_ethernet_x(tmp, fdt, node, enet, i)
 }
 if (getenv(usbethaddr))
   do_fdt_fixup_ethernet_x(usbethaddr, fdt, ...)

 Where the name of the new function, and parameter order also makes sense
 and is complete of course.  Thanks!

 One issue with this approach is that we don't know which inteface in
 the dt is usb ethernet and which is not.  So correctly assigning the
 MAC to the correct inteface will be tricky.
 Oh, that's true...  Maybe I should withdraw my objection, since we're
 unlikely to really see both in production cases.

 But the patch is flawed in the affect is that it does not take into
 account multiple usbethaddr either.  I will rework it for this but I
 am not sure about the other.
 But we don't support that either, did a quick grep before posting.

 Actually we do.  If you setenv eth1addr in the enviroment it will try to set 
 it in the dtb for the next ethernet instance.

 The other option is to set the ethaddr and then have usbethaddr reference 
 ethaddr and leave this code alone.

 Dan


Are there additional comments?
I currently do not have a V2 so I am assuming I have addressed all comments.

Please let me know.

Dan

-- 
--
Dan Murphy

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


[U-Boot] [PATCH] ARM: fdt support: Add usbethaddr as an acceptable MAC

2013-10-02 Thread Dan Murphy
A board that has a USB ethernet device only may set the usbetheraddr
and not the ethaddr.
ethaddr will be the default MAC address that is chosen and if that
is not populated then the usbethaddr is looked at.  If neither are set
then then device tree blob is not modified.

Signed-off-by: Dan Murphy dmur...@ti.com
---
 common/fdt_support.c |   12 +++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/common/fdt_support.c b/common/fdt_support.c
index b034c98..fef7e60 100644
--- a/common/fdt_support.c
+++ b/common/fdt_support.c
@@ -450,8 +450,18 @@ void fdt_fixup_ethernet(void *fdt)
if (node  0)
return;
 
+   if (!getenv(ethaddr)) {
+   if (getenv(usbethaddr)) {
+   strcpy(mac, usbethaddr);
+   } else {
+   debug(No ethernet MAC Address defined\n);
+   return;
+   }
+   } else {
+   strcpy(mac, ethaddr);
+   }
+
i = 0;
-   strcpy(mac, ethaddr);
while ((tmp = getenv(mac)) != NULL) {
sprintf(enet, ethernet%d, i);
path = fdt_getprop(fdt, node, enet, NULL);
-- 
1.7.9.5

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH] ARM: fdt support: Add usbethaddr as an acceptable MAC

2013-10-02 Thread Tom Rini
On Wed, Oct 02, 2013 at 02:00:15PM -0500, Dan Murphy wrote:

 A board that has a USB ethernet device only may set the usbetheraddr
 and not the ethaddr.
 ethaddr will be the default MAC address that is chosen and if that
 is not populated then the usbethaddr is looked at.  If neither are set
 then then device tree blob is not modified.
 
 Signed-off-by: Dan Murphy dmur...@ti.com
 ---
  common/fdt_support.c |   12 +++-
  1 file changed, 11 insertions(+), 1 deletion(-)
 
 diff --git a/common/fdt_support.c b/common/fdt_support.c
 index b034c98..fef7e60 100644
 --- a/common/fdt_support.c
 +++ b/common/fdt_support.c
 @@ -450,8 +450,18 @@ void fdt_fixup_ethernet(void *fdt)
   if (node  0)
   return;
  
 + if (!getenv(ethaddr)) {
 + if (getenv(usbethaddr)) {
 + strcpy(mac, usbethaddr);
 + } else {
 + debug(No ethernet MAC Address defined\n);
 + return;
 + }
 + } else {
 + strcpy(mac, ethaddr);
 + }
 +
   i = 0;
 - strcpy(mac, ethaddr);
   while ((tmp = getenv(mac)) != NULL) {
   sprintf(enet, ethernet%d, i);
   path = fdt_getprop(fdt, node, enet, NULL);

The problem is we may well have both.  I think we need to re-work the
function slightly to be:
while ((tmp = getenv(mac)) != NULL) {
  do_fdt_fixup_ethernet_x(tmp, fdt, node, enet, i)
}
if (getenv(usbethaddr))
  do_fdt_fixup_ethernet_x(usbethaddr, fdt, ...)

Where the name of the new function, and parameter order also makes sense
and is complete of course.  Thanks!

-- 
Tom


signature.asc
Description: Digital signature
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH] ARM: fdt support: Add usbethaddr as an acceptable MAC

2013-10-02 Thread Dan Murphy
Tom

On 10/02/2013 02:19 PM, Tom Rini wrote:
 On Wed, Oct 02, 2013 at 02:00:15PM -0500, Dan Murphy wrote:

 A board that has a USB ethernet device only may set the usbetheraddr
 and not the ethaddr.
 ethaddr will be the default MAC address that is chosen and if that
 is not populated then the usbethaddr is looked at.  If neither are set
 then then device tree blob is not modified.

 Signed-off-by: Dan Murphy dmur...@ti.com
 ---
  common/fdt_support.c |   12 +++-
  1 file changed, 11 insertions(+), 1 deletion(-)

 diff --git a/common/fdt_support.c b/common/fdt_support.c
 index b034c98..fef7e60 100644
 --- a/common/fdt_support.c
 +++ b/common/fdt_support.c
 @@ -450,8 +450,18 @@ void fdt_fixup_ethernet(void *fdt)
  if (node  0)
  return;
  
 +if (!getenv(ethaddr)) {
 +if (getenv(usbethaddr)) {
 +strcpy(mac, usbethaddr);
 +} else {
 +debug(No ethernet MAC Address defined\n);
 +return;
 +}
 +} else {
 +strcpy(mac, ethaddr);
 +}
 +
  i = 0;
 -strcpy(mac, ethaddr);
  while ((tmp = getenv(mac)) != NULL) {
  sprintf(enet, ethernet%d, i);
  path = fdt_getprop(fdt, node, enet, NULL);
 The problem is we may well have both.  I think we need to re-work the
 function slightly to be:
 while ((tmp = getenv(mac)) != NULL) {
   do_fdt_fixup_ethernet_x(tmp, fdt, node, enet, i)
 }
 if (getenv(usbethaddr))
   do_fdt_fixup_ethernet_x(usbethaddr, fdt, ...)

 Where the name of the new function, and parameter order also makes sense
 and is complete of course.  Thanks!


One issue with this approach is that we don't know which inteface in the dt is 
usb ethernet and which is not.
So correctly assigning the MAC to the correct inteface will be tricky.

But the patch is flawed in the affect is that it does not take into account 
multiple usbethaddr either.  I will rework it for this but I am not sure about 
the other.

Dan

-- 
--
Dan Murphy

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH] ARM: fdt support: Add usbethaddr as an acceptable MAC

2013-10-02 Thread Tom Rini
On Wed, Oct 02, 2013 at 03:14:26PM -0500, Dan Murphy wrote:
 Tom
 
 On 10/02/2013 02:19 PM, Tom Rini wrote:
  On Wed, Oct 02, 2013 at 02:00:15PM -0500, Dan Murphy wrote:
 
  A board that has a USB ethernet device only may set the usbetheraddr
  and not the ethaddr.
  ethaddr will be the default MAC address that is chosen and if that
  is not populated then the usbethaddr is looked at.  If neither are set
  then then device tree blob is not modified.
 
  Signed-off-by: Dan Murphy dmur...@ti.com
  ---
   common/fdt_support.c |   12 +++-
   1 file changed, 11 insertions(+), 1 deletion(-)
 
  diff --git a/common/fdt_support.c b/common/fdt_support.c
  index b034c98..fef7e60 100644
  --- a/common/fdt_support.c
  +++ b/common/fdt_support.c
  @@ -450,8 +450,18 @@ void fdt_fixup_ethernet(void *fdt)
 if (node  0)
 return;
   
  +  if (!getenv(ethaddr)) {
  +  if (getenv(usbethaddr)) {
  +  strcpy(mac, usbethaddr);
  +  } else {
  +  debug(No ethernet MAC Address defined\n);
  +  return;
  +  }
  +  } else {
  +  strcpy(mac, ethaddr);
  +  }
  +
 i = 0;
  -  strcpy(mac, ethaddr);
 while ((tmp = getenv(mac)) != NULL) {
 sprintf(enet, ethernet%d, i);
 path = fdt_getprop(fdt, node, enet, NULL);
  The problem is we may well have both.  I think we need to re-work the
  function slightly to be:
  while ((tmp = getenv(mac)) != NULL) {
do_fdt_fixup_ethernet_x(tmp, fdt, node, enet, i)
  }
  if (getenv(usbethaddr))
do_fdt_fixup_ethernet_x(usbethaddr, fdt, ...)
 
  Where the name of the new function, and parameter order also makes sense
  and is complete of course.  Thanks!
 
 
 One issue with this approach is that we don't know which inteface in
 the dt is usb ethernet and which is not.  So correctly assigning the
 MAC to the correct inteface will be tricky.

Oh, that's true...  Maybe I should withdraw my objection, since we're
unlikely to really see both in production cases.

 But the patch is flawed in the affect is that it does not take into
 account multiple usbethaddr either.  I will rework it for this but I
 am not sure about the other.

But we don't support that either, did a quick grep before posting.

-- 
Tom


signature.asc
Description: Digital signature
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH] ARM: fdt support: Add usbethaddr as an acceptable MAC

2013-10-02 Thread Dan Murphy
Tom

On 10/02/2013 03:19 PM, Tom Rini wrote:
 On Wed, Oct 02, 2013 at 03:14:26PM -0500, Dan Murphy wrote:
 Tom

 On 10/02/2013 02:19 PM, Tom Rini wrote:
 On Wed, Oct 02, 2013 at 02:00:15PM -0500, Dan Murphy wrote:

 A board that has a USB ethernet device only may set the usbetheraddr
 and not the ethaddr.
 ethaddr will be the default MAC address that is chosen and if that
 is not populated then the usbethaddr is looked at.  If neither are set
 then then device tree blob is not modified.

 Signed-off-by: Dan Murphy dmur...@ti.com
 ---
  common/fdt_support.c |   12 +++-
  1 file changed, 11 insertions(+), 1 deletion(-)

 diff --git a/common/fdt_support.c b/common/fdt_support.c
 index b034c98..fef7e60 100644
 --- a/common/fdt_support.c
 +++ b/common/fdt_support.c
 @@ -450,8 +450,18 @@ void fdt_fixup_ethernet(void *fdt)
if (node  0)
return;
  
 +  if (!getenv(ethaddr)) {
 +  if (getenv(usbethaddr)) {
 +  strcpy(mac, usbethaddr);
 +  } else {
 +  debug(No ethernet MAC Address defined\n);
 +  return;
 +  }
 +  } else {
 +  strcpy(mac, ethaddr);
 +  }
 +
i = 0;
 -  strcpy(mac, ethaddr);
while ((tmp = getenv(mac)) != NULL) {
sprintf(enet, ethernet%d, i);
path = fdt_getprop(fdt, node, enet, NULL);
 The problem is we may well have both.  I think we need to re-work the
 function slightly to be:
 while ((tmp = getenv(mac)) != NULL) {
   do_fdt_fixup_ethernet_x(tmp, fdt, node, enet, i)
 }
 if (getenv(usbethaddr))
   do_fdt_fixup_ethernet_x(usbethaddr, fdt, ...)

 Where the name of the new function, and parameter order also makes sense
 and is complete of course.  Thanks!

 One issue with this approach is that we don't know which inteface in
 the dt is usb ethernet and which is not.  So correctly assigning the
 MAC to the correct inteface will be tricky.
 Oh, that's true...  Maybe I should withdraw my objection, since we're
 unlikely to really see both in production cases.

 But the patch is flawed in the affect is that it does not take into
 account multiple usbethaddr either.  I will rework it for this but I
 am not sure about the other.
 But we don't support that either, did a quick grep before posting.


Actually we do.  If you setenv eth1addr in the enviroment it will try to set it 
in the dtb for the next ethernet instance.

The other option is to set the ethaddr and then have usbethaddr reference 
ethaddr and leave this code alone.

Dan

-- 
--
Dan Murphy

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot