Skip to contentSkip to conversation

fdt_support: Fixup 'ethernet' aliases not ending in digits

PATCH New Tuomas Tynkkynen 2 participants 5 messages Mar 17, 2017 – Mar 18, 2017

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 fdt_support: Fixup 'ethernet' aliases not ending in digits

The Raspberry Pi device tree files since Linux v4.9 have a "ethernet" alias pointing to the on-board Ethernet device node. However, U-Boot's fdt_fixup_ethernet() (and the kernel's of_alias_scan()) only looks at ethernet aliases ending in digits. Make it also check the "ethernet" alias.

Without this Linux isn't told of the MAC address provided by the RPI firmware and the ethernet device is always assigned a random MAC address.

The device trees themselves can't be fixed as someone is already depending on the "ethernet" alias:
https://github.com/raspberrypi/firmware/issues/613

---
 common/fdt_support.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)
common/fdt_support.c +9 -1
index 55d4d6f6d4..6e509b38e5 100644
@@ -496,7 +496,15 @@ void fdt_fixup_ethernet(void *fdt)
 
 		path = fdt_getprop_by_offset(fdt, offset, &name, NULL);
 		if (!strncmp(name, "ethernet", len)) {
-			i = trailing_strtol(name);
+			/*
+			 * Some device trees (bcm2835) have an alias without
+			 * the number at end. Support that as well.
+			 */
+			if (!strcmp(name, "ethernet"))
+				i = 0;
+			else
+				i = trailing_strtol(name);
+
 			if (i != -1) {
 				if (i == 0)
 					strcpy(mac, "ethaddr");
Signature · 1 line
2.11.1
#1 lore raw

PATCH 1/1 fdt_support: Fixup 'ethernet' aliases not ending in digits

Quoted 10 lines · Tuomas Tynkkynen

The Raspberry Pi device tree files since Linux v4.9 have a "ethernet" alias pointing to the on-board Ethernet device node. However, U-Boot's fdt_fixup_ethernet() (and the kernel's of_alias_scan()) only looks at ethernet aliases ending in digits. Make it also check the "ethernet" alias.

Without this Linux isn't told of the MAC address provided by the RPI firmware and the ethernet device is always assigned a random MAC address.

Quoted 5 lines

The device trees themselves can't be fixed as someone is already depending on the "ethernet" alias:
https://github.com/raspberrypi/firmware/issues/613

I looked into this a bit and there's a few other (much older) examples of 'ethernet' rather than 'ethernet0' and the spec doesn't mandate aliases end in a number.

That said, looking at the code, I think we can do this in a more clear manner. Can you test this please?

common/fdt_support.c +6 -10
index 55d4d6f6d444..80186a95baf0 100644
@@ -482,7 +482,6 @@ void fdt_fixup_ethernet(void *fdt)
 	/* Cycle through all aliases */
 	for (prop = 0; ; prop++) {
 		const char *name;
-		int len = strlen("ethernet");
 
 		/* FDT might have been edited, recompute the offset */
 		offset = fdt_first_property_offset(fdt,
@@ -495,16 +494,13 @@ void fdt_fixup_ethernet(void *fdt)
 			break;
 
 		path = fdt_getprop_by_offset(fdt, offset, &name, NULL);
-		if (!strncmp(name, "ethernet", len)) {
+		if (!strncmp(name, "ethernet", 8)) {
 			i = trailing_strtol(name);
-			if (i != -1) {
-				if (i == 0)
-					strcpy(mac, "ethaddr");
-				else
-					sprintf(mac, "eth%daddr", i);
-			} else {
-				continue;
-			}
+			/* Handle 'ethernet0' (0) and 'ethernet' (-1) */
+			if (i <= 0)
+				strcpy(mac, "ethaddr");
+			else
+				sprintf(mac, "eth%daddr", i);
 			tmp = getenv(mac);
 			if (!tmp)
 				continue;
Signature · 8 lines
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20170317/10c2a6fc/attachment.sig>
#2 lore raw

PATCH 1/1 fdt_support: Fixup 'ethernet' aliases not ending in digits

Quoted 16 lines · On Fri, 17 Mar 2017 21:26:51 -0400 Tom Rini <[email protected]> wrote:
Quoted 14 lines · Tuomas Tynkkynen

The Raspberry Pi device tree files since Linux v4.9 have a "ethernet" alias pointing to the on-board Ethernet device node. However, U-Boot's fdt_fixup_ethernet() (and the kernel's of_alias_scan()) only looks at ethernet aliases ending in digits. Make it also check the "ethernet" alias.

Without this Linux isn't told of the MAC address provided by the RPI firmware and the ethernet device is always assigned a random MAC address.

The device trees themselves can't be fixed as someone is already depending on the "ethernet" alias:
https://github.com/raspberrypi/firmware/issues/613

Quoted 5 lines
Quoted 1 line

I looked into this a bit and there's a few other (much older) examples of 'ethernet' rather than 'ethernet0' and the spec doesn't mandate aliases end in a number.

Ah, good to know.

Quoted 29 lines

That said, looking at the code, I think we can do this in a more clear manner. Can you test this please?

common/fdt_support.c +1 -8
index 55d4d6f6d444..80186a95baf0 100644
@@ -482,7 +482,6 @@ void fdt_fixup_ethernet(void *fdt)
 	/* Cycle through all aliases */
 	for (prop = 0; ; prop++) {
 		const char *name;
-		int len = strlen("ethernet");
 
 		/* FDT might have been edited, recompute the offset */
 		offset = fdt_first_property_offset(fdt,
@@ -495,16 +494,13 @@ void fdt_fixup_ethernet(void *fdt)
 			break;
 
 		path = fdt_getprop_by_offset(fdt, offset, &name, NULL);
-		if (!strncmp(name, "ethernet", len)) {
+		if (!strncmp(name, "ethernet", 8)) {
 			i = trailing_strtol(name);
-			if (i != -1) {
-				if (i == 0)
-					strcpy(mac, "ethaddr");
-				else
-					sprintf(mac, "eth%daddr", i);
-			} else {

common/fdt_support.c void fdt_fixup_ethernet(void *fdt) · line 506

Quoted 10 lines
common/fdt_support.c +5 -2
-				continue;
-			}
+			/* Handle 'ethernet0' (0) and 'ethernet' (-1) */
+			if (i <= 0)
+				strcpy(mac, "ethaddr");
+			else
+				sprintf(mac, "eth%daddr", i);
 			tmp = getenv(mac);
 			if (!tmp)
 				continue;

That one accepts everything starting with "ethernet", which sounds undesirable if one wants to have e.g. an "ethernet-phy0" alias for some different purpose.

#3 lore raw

PATCH 1/1 fdt_support: Fixup 'ethernet' aliases not ending in digits

Quoted 61 lines · Tuomas Tynkkynen
Quoted 16 lines · On Fri, 17 Mar 2017 21:26:51 -0400 Tom Rini <[email protected]> wrote:
Quoted 14 lines · Tuomas Tynkkynen

The Raspberry Pi device tree files since Linux v4.9 have a "ethernet" alias pointing to the on-board Ethernet device node. However, U-Boot's fdt_fixup_ethernet() (and the kernel's of_alias_scan()) only looks at ethernet aliases ending in digits. Make it also check the "ethernet" alias.

Without this Linux isn't told of the MAC address provided by the RPI firmware and the ethernet device is always assigned a random MAC address.

The device trees themselves can't be fixed as someone is already depending on the "ethernet" alias:
https://github.com/raspberrypi/firmware/issues/613

Quoted 5 lines
Quoted 1 line

I looked into this a bit and there's a few other (much older) examples of 'ethernet' rather than 'ethernet0' and the spec doesn't mandate aliases end in a number.

Ah, good to know.

Quoted 34 lines

That said, looking at the code, I think we can do this in a more clear manner. Can you test this please?

common/fdt_support.c +5 -10
index 55d4d6f6d444..80186a95baf0 100644
@@ -482,7 +482,6 @@ void fdt_fixup_ethernet(void *fdt)
 	/* Cycle through all aliases */
 	for (prop = 0; ; prop++) {
 		const char *name;
-		int len = strlen("ethernet");
 
 		/* FDT might have been edited, recompute the offset */
 		offset = fdt_first_property_offset(fdt,
@@ -495,16 +494,13 @@ void fdt_fixup_ethernet(void *fdt)
 			break;
 
 		path = fdt_getprop_by_offset(fdt, offset, &name, NULL);
-		if (!strncmp(name, "ethernet", len)) {
+		if (!strncmp(name, "ethernet", 8)) {
 			i = trailing_strtol(name);
-			if (i != -1) {
-				if (i == 0)
-					strcpy(mac, "ethaddr");
-				else
-					sprintf(mac, "eth%daddr", i);
-			} else {
-				continue;
-			}
+			/* Handle 'ethernet0' (0) and 'ethernet' (-1) */
+			if (i <= 0)
+				strcpy(mac, "ethaddr");
+			else
Quoted 8 lines

common/fdt_support.c void fdt_fixup_ethernet(void *fdt) · line 506

Quoted 5 lines
common/fdt_support.c +1 -0
+				sprintf(mac, "eth%daddr", i);
 			tmp = getenv(mac);
 			if (!tmp)
 				continue;

That one accepts everything starting with "ethernet", which sounds undesirable if one wants to have e.g. an "ethernet-phy0" alias for some different purpose.

True. Would you mind doing a v2 of your patch then (and grab my part to just use the length of ethernet in the strncmp) where we don't add a comment implying RPi is "wrong" here and just that we sometimes have "ethernet" as the alias for the first and only ethernet device? Thanks!

Signature · 8 lines
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20170318/fe72c3df/attachment.sig>
#4 lore raw

PATCH 1/1 fdt_support: Fixup 'ethernet' aliases not ending in digits

Quoted 1 line · On Sat, 18 Mar 2017 14:34:34 -0400 Tom Rini <[email protected]> wrote:

On Sat, Mar 18, 2017 at 08:20:07PM +0200, Tuomas Tynkkynen wrote:

...

Quoted 9 lines
Quoted 3 lines

That one accepts everything starting with "ethernet", which sounds undesirable if one wants to have e.g. an "ethernet-phy0" alias for some different purpose.

True. Would you mind doing a v2 of your patch then (and grab my part to just use the length of ethernet in the strncmp) where we don't add a comment implying RPi is "wrong" here and just that we sometimes have "ethernet" as the alias for the first and only ethernet device? Thanks!

Sure, will do it on Monday when I have access to my Pi again!