[U-Boot] [PATCH] fdt: Rewrite the logic in fdt_fixup_ethernet()
Currently in fdt_fixup_ethernet() the MAC address fix up is handled in a loop of which the exit condition is to test the "eth%daddr" env is not NULL. However this creates unnecessary constrains that those "eth%daddr" env variables must be sequential even if "ethernet%d" does not start from 0 in the "/aliases" node. For example, with "/aliases" node below:
aliases { ethernet3 = &enet3; ethernet4 = &enet4; };
"ethaddr", "eth1addr", "eth2addr" must exist in order to fix up ethernet3's MAC address successfully.
Now we change the loop logic to iterate the properties in the "/aliases" node. For each property, test if it is in a format of "ethernet%d", then get its MAC address from corresponding "eth%daddr" env and fix it up in the dtb.
Signed-off-by: Bin Meng bmeng.cn@gmail.com ---
common/fdt_support.c | 53 ++++++++++++++++++++++++++++++++-------------------- 1 file changed, 33 insertions(+), 20 deletions(-)
diff --git a/common/fdt_support.c b/common/fdt_support.c index f86365e..5cdf185 100644 --- a/common/fdt_support.c +++ b/common/fdt_support.c @@ -476,10 +476,11 @@ int fdt_fixup_memory(void *blob, u64 start, u64 size) void fdt_fixup_ethernet(void *fdt) { int node, i, j; - char enet[16], *tmp, *end; + char *tmp, *end; char mac[16]; const char *path; unsigned char mac_addr[6]; + int offset;
node = fdt_path_offset(fdt, "/aliases"); if (node < 0) @@ -496,27 +497,39 @@ void fdt_fixup_ethernet(void *fdt) strcpy(mac, "ethaddr"); }
- i = 0; - while ((tmp = getenv(mac)) != NULL) { - sprintf(enet, "ethernet%d", i); - path = fdt_getprop(fdt, node, enet, NULL); - if (!path) { - debug("No alias for %s\n", enet); - sprintf(mac, "eth%daddr", ++i); - continue; - } + for (offset = fdt_first_property_offset(fdt, node); + offset > 0; + offset = fdt_next_property_offset(fdt, offset)) { + const char *name; + int len = strlen("ethernet"); + + path = fdt_getprop_by_offset(fdt, offset, &name, NULL); + if (!strncmp(name, "ethernet", len)) { + i = trailing_strtol(name); + if (i != -1) { + if (i == 0) + strcpy(mac, "ethaddr"); + else + sprintf(mac, "eth%daddr", i); + } else { + continue; + } + tmp = getenv(mac); + if (!tmp) + continue; + + for (j = 0; j < 6; j++) { + mac_addr[j] = tmp ? + simple_strtoul(tmp, &end, 16) : 0; + if (tmp) + tmp = (*end) ? end + 1 : end; + }
- for (j = 0; j < 6; j++) { - mac_addr[j] = tmp ? simple_strtoul(tmp, &end, 16) : 0; - if (tmp) - tmp = (*end) ? end+1 : end; + do_fixup_by_path(fdt, path, "mac-address", + &mac_addr, 6, 0); + do_fixup_by_path(fdt, path, "local-mac-address", + &mac_addr, 6, 1); } - - do_fixup_by_path(fdt, path, "mac-address", &mac_addr, 6, 0); - do_fixup_by_path(fdt, path, "local-mac-address", - &mac_addr, 6, 1); - - sprintf(mac, "eth%daddr", ++i); } }
Hi Bin,
On Tue, Oct 27, 2015 at 11:10 AM, Bin Meng bmeng.cn@gmail.com wrote:
Currently in fdt_fixup_ethernet() the MAC address fix up is handled in a loop of which the exit condition is to test the "eth%daddr" env is not NULL. However this creates unnecessary constrains that those "eth%daddr" env variables must be sequential even if "ethernet%d" does not start from 0 in the "/aliases" node. For example, with "/aliases" node below:
aliases { ethernet3 = &enet3; ethernet4 = &enet4; };"ethaddr", "eth1addr", "eth2addr" must exist in order to fix up ethernet3's MAC address successfully.
Now we change the loop logic to iterate the properties in the "/aliases" node. For each property, test if it is in a format of "ethernet%d", then get its MAC address from corresponding "eth%daddr" env and fix it up in the dtb.
Signed-off-by: Bin Meng bmeng.cn@gmail.com
Acked-by: Joe Hershberger joe.hershberger@ni.com
Hi Joe,
On Thu, Oct 29, 2015 at 4:52 AM, Joe Hershberger joe.hershberger@gmail.com wrote:
Hi Bin,
On Tue, Oct 27, 2015 at 11:10 AM, Bin Meng bmeng.cn@gmail.com wrote:
Currently in fdt_fixup_ethernet() the MAC address fix up is handled in a loop of which the exit condition is to test the "eth%daddr" env is not NULL. However this creates unnecessary constrains that those "eth%daddr" env variables must be sequential even if "ethernet%d" does not start from 0 in the "/aliases" node. For example, with "/aliases" node below:
aliases { ethernet3 = &enet3; ethernet4 = &enet4; };"ethaddr", "eth1addr", "eth2addr" must exist in order to fix up ethernet3's MAC address successfully.
Now we change the loop logic to iterate the properties in the "/aliases" node. For each property, test if it is in a format of "ethernet%d", then get its MAC address from corresponding "eth%daddr" env and fix it up in the dtb.
Signed-off-by: Bin Meng bmeng.cn@gmail.com
Acked-by: Joe Hershberger joe.hershberger@ni.com
Thanks! Do you have any comments regarding to "usbethaddr" env that I raised here [1]? I originally wanted to remove "usbethaddr" handling completely in fdt_fixup_ethernet(), but did not do that when I submitted this patch.
[1] http://lists.denx.de/pipermail/u-boot/2015-October/231791.html
Regards, Bin
On Thu, Oct 29, 2015 at 09:40:43AM +0800, Bin Meng wrote:
Hi Joe,
On Thu, Oct 29, 2015 at 4:52 AM, Joe Hershberger joe.hershberger@gmail.com wrote:
Hi Bin,
On Tue, Oct 27, 2015 at 11:10 AM, Bin Meng bmeng.cn@gmail.com wrote:
Currently in fdt_fixup_ethernet() the MAC address fix up is handled in a loop of which the exit condition is to test the "eth%daddr" env is not NULL. However this creates unnecessary constrains that those "eth%daddr" env variables must be sequential even if "ethernet%d" does not start from 0 in the "/aliases" node. For example, with "/aliases" node below:
aliases { ethernet3 = &enet3; ethernet4 = &enet4; };"ethaddr", "eth1addr", "eth2addr" must exist in order to fix up ethernet3's MAC address successfully.
Now we change the loop logic to iterate the properties in the "/aliases" node. For each property, test if it is in a format of "ethernet%d", then get its MAC address from corresponding "eth%daddr" env and fix it up in the dtb.
Signed-off-by: Bin Meng bmeng.cn@gmail.com
Acked-by: Joe Hershberger joe.hershberger@ni.com
Thanks! Do you have any comments regarding to "usbethaddr" env that I raised here [1]? I originally wanted to remove "usbethaddr" handling completely in fdt_fixup_ethernet(), but did not do that when I submitted this patch.
I need to think about that post still, sorry, but we can't remove it, it would break various systems that have on-board USB eth (pandaboard, others).
Hi,
On 28 October 2015 at 19:40, Bin Meng bmeng.cn@gmail.com wrote:
Hi Joe,
On Thu, Oct 29, 2015 at 4:52 AM, Joe Hershberger joe.hershberger@gmail.com wrote:
Hi Bin,
On Tue, Oct 27, 2015 at 11:10 AM, Bin Meng bmeng.cn@gmail.com wrote:
Currently in fdt_fixup_ethernet() the MAC address fix up is handled in a loop of which the exit condition is to test the "eth%daddr" env is not NULL. However this creates unnecessary constrains that those "eth%daddr" env variables must be sequential even if "ethernet%d" does not start from 0 in the "/aliases" node. For example, with "/aliases" node below:
aliases { ethernet3 = &enet3; ethernet4 = &enet4; };"ethaddr", "eth1addr", "eth2addr" must exist in order to fix up ethernet3's MAC address successfully.
Now we change the loop logic to iterate the properties in the "/aliases" node. For each property, test if it is in a format of "ethernet%d", then get its MAC address from corresponding "eth%daddr" env and fix it up in the dtb.
Signed-off-by: Bin Meng bmeng.cn@gmail.com
Acked-by: Joe Hershberger joe.hershberger@ni.com
Thanks! Do you have any comments regarding to "usbethaddr" env that I raised here [1]? I originally wanted to remove "usbethaddr" handling completely in fdt_fixup_ethernet(), but did not do that when I submitted this patch.
[1] http://lists.denx.de/pipermail/u-boot/2015-October/231791.html
My understanding is that we were going to drop the usbethaddr variables and just use ethaddr.
Regards, Simon
Hi Simon,
On Thu, Oct 29, 2015 at 9:53 AM, Simon Glass sjg@chromium.org wrote:
Hi,
On 28 October 2015 at 19:40, Bin Meng bmeng.cn@gmail.com wrote:
Hi Joe,
On Thu, Oct 29, 2015 at 4:52 AM, Joe Hershberger joe.hershberger@gmail.com wrote:
Hi Bin,
On Tue, Oct 27, 2015 at 11:10 AM, Bin Meng bmeng.cn@gmail.com wrote:
Currently in fdt_fixup_ethernet() the MAC address fix up is handled in a loop of which the exit condition is to test the "eth%daddr" env is not NULL. However this creates unnecessary constrains that those "eth%daddr" env variables must be sequential even if "ethernet%d" does not start from 0 in the "/aliases" node. For example, with "/aliases" node below:
aliases { ethernet3 = &enet3; ethernet4 = &enet4; };"ethaddr", "eth1addr", "eth2addr" must exist in order to fix up ethernet3's MAC address successfully.
Now we change the loop logic to iterate the properties in the "/aliases" node. For each property, test if it is in a format of "ethernet%d", then get its MAC address from corresponding "eth%daddr" env and fix it up in the dtb.
Signed-off-by: Bin Meng bmeng.cn@gmail.com
Acked-by: Joe Hershberger joe.hershberger@ni.com
Thanks! Do you have any comments regarding to "usbethaddr" env that I raised here [1]? I originally wanted to remove "usbethaddr" handling completely in fdt_fixup_ethernet(), but did not do that when I submitted this patch.
[1] http://lists.denx.de/pipermail/u-boot/2015-October/231791.html
My understanding is that we were going to drop the usbethaddr variables and just use ethaddr.
Yep, I agree, and driver model eth device only uses "ethaddr" as the MAC address source. I guess when we introduced "usbethaddr" to U-Boot at the beginning we didn't think about MAC address fix up. Later commit b1f49ab was added to only handle "usbethaddr" assuming that there is only one usb ethernet on the board, which is not necessarily the case. Using two sources ("usbethaddr" and "ethaddr") just causes trouble in the logic in fdt_fixup_ethernet().
Regards, Bin
Hi Bin,
On Wed, Oct 28, 2015 at 8:40 PM, Bin Meng bmeng.cn@gmail.com wrote:
Hi Joe,
On Thu, Oct 29, 2015 at 4:52 AM, Joe Hershberger joe.hershberger@gmail.com wrote:
Hi Bin,
On Tue, Oct 27, 2015 at 11:10 AM, Bin Meng bmeng.cn@gmail.com wrote:
Currently in fdt_fixup_ethernet() the MAC address fix up is handled in a loop of which the exit condition is to test the "eth%daddr" env is not NULL. However this creates unnecessary constrains that those "eth%daddr" env variables must be sequential even if "ethernet%d" does not start from 0 in the "/aliases" node. For example, with "/aliases" node below:
aliases { ethernet3 = &enet3; ethernet4 = &enet4; };"ethaddr", "eth1addr", "eth2addr" must exist in order to fix up ethernet3's MAC address successfully.
Now we change the loop logic to iterate the properties in the "/aliases" node. For each property, test if it is in a format of "ethernet%d", then get its MAC address from corresponding "eth%daddr" env and fix it up in the dtb.
Signed-off-by: Bin Meng bmeng.cn@gmail.com
Acked-by: Joe Hershberger joe.hershberger@ni.com
Thanks! Do you have any comments regarding to "usbethaddr" env that I raised here [1]? I originally wanted to remove "usbethaddr" handling completely in fdt_fixup_ethernet(), but did not do that when I submitted this patch.
[1] http://lists.denx.de/pipermail/u-boot/2015-October/231791.html
I'm in agreement with you. See here: http://lists.denx.de/pipermail/u-boot/2015-July/218601.html
It would be great if you completely remove it.
Cheers, -Joe
Hi Joe,
On Fri, Oct 30, 2015 at 3:38 AM, Joe Hershberger joe.hershberger@gmail.com wrote:
Hi Bin,
On Wed, Oct 28, 2015 at 8:40 PM, Bin Meng bmeng.cn@gmail.com wrote:
Hi Joe,
On Thu, Oct 29, 2015 at 4:52 AM, Joe Hershberger joe.hershberger@gmail.com wrote:
Hi Bin,
On Tue, Oct 27, 2015 at 11:10 AM, Bin Meng bmeng.cn@gmail.com wrote:
Currently in fdt_fixup_ethernet() the MAC address fix up is handled in a loop of which the exit condition is to test the "eth%daddr" env is not NULL. However this creates unnecessary constrains that those "eth%daddr" env variables must be sequential even if "ethernet%d" does not start from 0 in the "/aliases" node. For example, with "/aliases" node below:
aliases { ethernet3 = &enet3; ethernet4 = &enet4; };"ethaddr", "eth1addr", "eth2addr" must exist in order to fix up ethernet3's MAC address successfully.
Now we change the loop logic to iterate the properties in the "/aliases" node. For each property, test if it is in a format of "ethernet%d", then get its MAC address from corresponding "eth%daddr" env and fix it up in the dtb.
Signed-off-by: Bin Meng bmeng.cn@gmail.com
Acked-by: Joe Hershberger joe.hershberger@ni.com
Thanks! Do you have any comments regarding to "usbethaddr" env that I raised here [1]? I originally wanted to remove "usbethaddr" handling completely in fdt_fixup_ethernet(), but did not do that when I submitted this patch.
[1] http://lists.denx.de/pipermail/u-boot/2015-October/231791.html
I'm in agreement with you. See here: http://lists.denx.de/pipermail/u-boot/2015-July/218601.html
It would be great if you completely remove it.
Good. I will prepare a patch to remove "usbethaddr" in fdt_fixup_ethernet().
Regards, Bin
On Fri, Oct 30, 2015 at 10:16:49AM +0800, Bin Meng wrote:
Hi Joe,
On Fri, Oct 30, 2015 at 3:38 AM, Joe Hershberger joe.hershberger@gmail.com wrote:
Hi Bin,
On Wed, Oct 28, 2015 at 8:40 PM, Bin Meng bmeng.cn@gmail.com wrote:
Hi Joe,
On Thu, Oct 29, 2015 at 4:52 AM, Joe Hershberger joe.hershberger@gmail.com wrote:
Hi Bin,
On Tue, Oct 27, 2015 at 11:10 AM, Bin Meng bmeng.cn@gmail.com wrote:
Currently in fdt_fixup_ethernet() the MAC address fix up is handled in a loop of which the exit condition is to test the "eth%daddr" env is not NULL. However this creates unnecessary constrains that those "eth%daddr" env variables must be sequential even if "ethernet%d" does not start from 0 in the "/aliases" node. For example, with "/aliases" node below:
aliases { ethernet3 = &enet3; ethernet4 = &enet4; };"ethaddr", "eth1addr", "eth2addr" must exist in order to fix up ethernet3's MAC address successfully.
Now we change the loop logic to iterate the properties in the "/aliases" node. For each property, test if it is in a format of "ethernet%d", then get its MAC address from corresponding "eth%daddr" env and fix it up in the dtb.
Signed-off-by: Bin Meng bmeng.cn@gmail.com
Acked-by: Joe Hershberger joe.hershberger@ni.com
Thanks! Do you have any comments regarding to "usbethaddr" env that I raised here [1]? I originally wanted to remove "usbethaddr" handling completely in fdt_fixup_ethernet(), but did not do that when I submitted this patch.
[1] http://lists.denx.de/pipermail/u-boot/2015-October/231791.html
I'm in agreement with you. See here: http://lists.denx.de/pipermail/u-boot/2015-July/218601.html
It would be great if you completely remove it.
Good. I will prepare a patch to remove "usbethaddr" in fdt_fixup_ethernet().
So long as you don't break the boards that need it :)
Hi Tom,
On Fri, Oct 30, 2015 at 10:25 AM, Tom Rini trini@konsulko.com wrote:
On Fri, Oct 30, 2015 at 10:16:49AM +0800, Bin Meng wrote:
Hi Joe,
On Fri, Oct 30, 2015 at 3:38 AM, Joe Hershberger joe.hershberger@gmail.com wrote:
Hi Bin,
On Wed, Oct 28, 2015 at 8:40 PM, Bin Meng bmeng.cn@gmail.com wrote:
Hi Joe,
On Thu, Oct 29, 2015 at 4:52 AM, Joe Hershberger joe.hershberger@gmail.com wrote:
Hi Bin,
On Tue, Oct 27, 2015 at 11:10 AM, Bin Meng bmeng.cn@gmail.com wrote:
Currently in fdt_fixup_ethernet() the MAC address fix up is handled in a loop of which the exit condition is to test the "eth%daddr" env is not NULL. However this creates unnecessary constrains that those "eth%daddr" env variables must be sequential even if "ethernet%d" does not start from 0 in the "/aliases" node. For example, with "/aliases" node below:
aliases { ethernet3 = &enet3; ethernet4 = &enet4; };"ethaddr", "eth1addr", "eth2addr" must exist in order to fix up ethernet3's MAC address successfully.
Now we change the loop logic to iterate the properties in the "/aliases" node. For each property, test if it is in a format of "ethernet%d", then get its MAC address from corresponding "eth%daddr" env and fix it up in the dtb.
Signed-off-by: Bin Meng bmeng.cn@gmail.com
Acked-by: Joe Hershberger joe.hershberger@ni.com
Thanks! Do you have any comments regarding to "usbethaddr" env that I raised here [1]? I originally wanted to remove "usbethaddr" handling completely in fdt_fixup_ethernet(), but did not do that when I submitted this patch.
[1] http://lists.denx.de/pipermail/u-boot/2015-October/231791.html
I'm in agreement with you. See here: http://lists.denx.de/pipermail/u-boot/2015-July/218601.html
It would be great if you completely remove it.
Good. I will prepare a patch to remove "usbethaddr" in fdt_fixup_ethernet().
So long as you don't break the boards that need it :)
I will only remove the MAC address fix up in fdt_fixup_ethernet(). This needs these affected boards to configure "ethaddr" in their environment. For other instances of "usbethaddr" in U-Boot, they should be removed when these drivers/boards have been converted to driver model.
Regards, Bin
On Tue, Oct 27, 2015 at 09:10:39AM -0700, Bin Meng wrote:
Currently in fdt_fixup_ethernet() the MAC address fix up is handled in a loop of which the exit condition is to test the "eth%daddr" env is not NULL. However this creates unnecessary constrains that those "eth%daddr" env variables must be sequential even if "ethernet%d" does not start from 0 in the "/aliases" node. For example, with "/aliases" node below:
aliases { ethernet3 = &enet3; ethernet4 = &enet4; };"ethaddr", "eth1addr", "eth2addr" must exist in order to fix up ethernet3's MAC address successfully.
Now we change the loop logic to iterate the properties in the "/aliases" node. For each property, test if it is in a format of "ethernet%d", then get its MAC address from corresponding "eth%daddr" env and fix it up in the dtb.
Signed-off-by: Bin Meng bmeng.cn@gmail.com
Reviewed-by: Tom Rini trini@konsulko.com
participants (4)
-
Bin Meng -
Joe Hershberger -
Simon Glass -
Tom Rini