[U-Boot] [PATCH v2 1/2] fdt: Deprecate "usbethaddr" usage in fdt_fixup_ethernet()

[U-Boot] [PATCH v2 1/2] fdt: Deprecate "usbethaddr" usage in fdt_fixup_ethernet()

[Bin Meng]

In fdt_fixup_ethernet() only "usbethaddr" is handled to fix up the
first usb ethernet port MAC address. Other additional usb ethernet
ports are ignored as there is no logic to handle "usbeth%daddr".

It is suggested we should use "ethaddr" for all ethernet devices.
Hence deprecate "usbethaddr" usage in fdt_fixup_ethernet(). Note
this actually reverts commit b1f49ab.

Signed-off-by: Bin Meng <bmeng.cn@gmail.com>

---

Changes in v2:
- New patch to deprecate "usbethaddr" usage in fdt_fixup_ethernet()

 common/fdt_support.c | 12 +-----------
 1 file changed, 1 insertion(+), 11 deletions(-)

diff --git a/common/fdt_support.c b/common/fdt_support.c
index a7ff2df..ec72b86 100644
--- a/common/fdt_support.c
+++ b/common/fdt_support.c
@@ -490,18 +490,8 @@ 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.8.2.1

[U-Boot] [PATCH v2 2/2] fdt: Rewrite the logic in fdt_fixup_ethernet()

[Bin Meng]

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>
Acked-by: Joe Hershberger <joe.hershberger@ni.com>

---

Changes in v2: None

 common/fdt_support.c | 54 ++++++++++++++++++++++++++++++++--------------------
 1 file changed, 33 insertions(+), 21 deletions(-)

diff --git a/common/fdt_support.c b/common/fdt_support.c
index ec72b86..f7aaf9c 100644
--- a/common/fdt_support.c
+++ b/common/fdt_support.c
@@ -481,37 +481,49 @@ 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)
 		return;
 
-	i = 0;
-	strcpy(mac, "ethaddr");
-	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);
 	}
 }
 
-- 
1.8.2.1

[U-Boot] [PATCH v2 1/2] fdt: Deprecate "usbethaddr" usage in fdt_fixup_ethernet()

[Joe Hershberger]

Hi Bin,

On Mon, Nov 2, 2015 at 1:58 AM, Bin Meng <bmeng.cn@gmail.com> wrote:
> In fdt_fixup_ethernet() only "usbethaddr" is handled to fix up the
> first usb ethernet port MAC address. Other additional usb ethernet
> ports are ignored as there is no logic to handle "usbeth%daddr".
>
> It is suggested we should use "ethaddr" for all ethernet devices.
> Hence deprecate "usbethaddr" usage in fdt_fixup_ethernet(). Note
> this actually reverts commit b1f49ab.

Please include the subject of this commit that is reverted. Also a
good idea to include the entire commit ID.

>
> Signed-off-by: Bin Meng <bmeng.cn@gmail.com>

Other than that,
Acked-by: Joe Hershberger <joe.hershberger@ni.com>

> ---
>
> Changes in v2:
> - New patch to deprecate "usbethaddr" usage in fdt_fixup_ethernet()
>
>  common/fdt_support.c | 12 +-----------
>  1 file changed, 1 insertion(+), 11 deletions(-)
>
> diff --git a/common/fdt_support.c b/common/fdt_support.c
> index a7ff2df..ec72b86 100644
> --- a/common/fdt_support.c
> +++ b/common/fdt_support.c
> @@ -490,18 +490,8 @@ 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.8.2.1
>
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH v2 1/2] fdt: Deprecate "usbethaddr" usage in fdt_fixup_ethernet()

[Tom Rini]

On Sun, Nov 01, 2015 at 11:58:14PM -0800, Bin Meng wrote:

> In fdt_fixup_ethernet() only "usbethaddr" is handled to fix up the
> first usb ethernet port MAC address. Other additional usb ethernet
> ports are ignored as there is no logic to handle "usbeth%daddr".
> 
> It is suggested we should use "ethaddr" for all ethernet devices.
> Hence deprecate "usbethaddr" usage in fdt_fixup_ethernet(). Note
> this actually reverts commit b1f49ab.
> 
> Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
> 

Reviewed-by: Tom Rini <trini@konsulko.com>
On OMAP4 Panda (+ v4.3 kernel)
Tested-by: Tom Rini <trini@konsulko.com>

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20151102/acd8b69f/attachment.sig>

[U-Boot] [PATCH v2 2/2] fdt: Rewrite the logic in fdt_fixup_ethernet()

[Tom Rini]

On Sun, Nov 01, 2015 at 11:58:15PM -0800, 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>
> Acked-by: Joe Hershberger <joe.hershberger@ni.com>
> 

Reviewed-by: Tom Rini <trini@konsulko.com>
On OMAP4 Panda (+v4.3 kernel)
Tested-by: Tom Rini <trini@konsulko.com>

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20151102/c33e7107/attachment.sig>