* [U-Boot] [PATCH] fdt_support: Fixup 'ethernet' aliases not ending in digits
@ 2017-03-17 19:44 Tuomas Tynkkynen
2017-03-18 1:26 ` Tom Rini
0 siblings, 1 reply; 5+ messages in thread
From: Tuomas Tynkkynen @ 2017-03-17 19:44 UTC (permalink / raw)
To: u-boot
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
Signed-off-by: Tuomas Tynkkynen <tuomas@tuxera.com>
---
common/fdt_support.c | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/common/fdt_support.c b/common/fdt_support.c
index 55d4d6f6d4..6e509b38e5 100644
--- a/common/fdt_support.c
+++ b/common/fdt_support.c
@@ -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");
--
2.11.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [U-Boot] [PATCH] fdt_support: Fixup 'ethernet' aliases not ending in digits
2017-03-17 19:44 [U-Boot] [PATCH] fdt_support: Fixup 'ethernet' aliases not ending in digits Tuomas Tynkkynen
@ 2017-03-18 1:26 ` Tom Rini
2017-03-18 18:20 ` Tuomas Tynkkynen
0 siblings, 1 reply; 5+ messages in thread
From: Tom Rini @ 2017-03-18 1:26 UTC (permalink / raw)
To: u-boot
On Fri, Mar 17, 2017 at 09:44:59PM +0200, Tuomas Tynkkynen wrote:
> 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
>
> Signed-off-by: Tuomas Tynkkynen <tuomas@tuxera.com>
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?
diff --git a/common/fdt_support.c b/common/fdt_support.c
index 55d4d6f6d444..80186a95baf0 100644
--- a/common/fdt_support.c
+++ b/common/fdt_support.c
@@ -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;
--
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>
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [U-Boot] [PATCH] fdt_support: Fixup 'ethernet' aliases not ending in digits
2017-03-18 1:26 ` Tom Rini
@ 2017-03-18 18:20 ` Tuomas Tynkkynen
2017-03-18 18:34 ` Tom Rini
0 siblings, 1 reply; 5+ messages in thread
From: Tuomas Tynkkynen @ 2017-03-18 18:20 UTC (permalink / raw)
To: u-boot
On Fri, 17 Mar 2017 21:26:51 -0400
Tom Rini <trini@konsulko.com> wrote:
> On Fri, Mar 17, 2017 at 09:44:59PM +0200, Tuomas Tynkkynen wrote:
>
> > 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
> >
> > Signed-off-by: Tuomas Tynkkynen <tuomas@tuxera.com>
>
> 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.
> That said, looking at the code, I think we can do this in a more clear
> manner. Can you test this please?
>
> diff --git a/common/fdt_support.c b/common/fdt_support.c
> index 55d4d6f6d444..80186a95baf0 100644
> --- a/common/fdt_support.c
> +++ b/common/fdt_support.c
> @@ -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;
>
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.
^ permalink raw reply [flat|nested] 5+ messages in thread
* [U-Boot] [PATCH] fdt_support: Fixup 'ethernet' aliases not ending in digits
2017-03-18 18:20 ` Tuomas Tynkkynen
@ 2017-03-18 18:34 ` Tom Rini
2017-03-18 23:17 ` Tuomas Tynkkynen
0 siblings, 1 reply; 5+ messages in thread
From: Tom Rini @ 2017-03-18 18:34 UTC (permalink / raw)
To: u-boot
On Sat, Mar 18, 2017 at 08:20:07PM +0200, Tuomas Tynkkynen wrote:
> On Fri, 17 Mar 2017 21:26:51 -0400
> Tom Rini <trini@konsulko.com> wrote:
>
> > On Fri, Mar 17, 2017 at 09:44:59PM +0200, Tuomas Tynkkynen wrote:
> >
> > > 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
> > >
> > > Signed-off-by: Tuomas Tynkkynen <tuomas@tuxera.com>
> >
> > 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.
>
> > That said, looking at the code, I think we can do this in a more clear
> > manner. Can you test this please?
> >
> > diff --git a/common/fdt_support.c b/common/fdt_support.c
> > index 55d4d6f6d444..80186a95baf0 100644
> > --- a/common/fdt_support.c
> > +++ b/common/fdt_support.c
> > @@ -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;
> >
>
> 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!
--
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>
^ permalink raw reply [flat|nested] 5+ messages in thread
* [U-Boot] [PATCH] fdt_support: Fixup 'ethernet' aliases not ending in digits
2017-03-18 18:34 ` Tom Rini
@ 2017-03-18 23:17 ` Tuomas Tynkkynen
0 siblings, 0 replies; 5+ messages in thread
From: Tuomas Tynkkynen @ 2017-03-18 23:17 UTC (permalink / raw)
To: u-boot
On Sat, 18 Mar 2017 14:34:34 -0400
Tom Rini <trini@konsulko.com> wrote:
> On Sat, Mar 18, 2017 at 08:20:07PM +0200, Tuomas Tynkkynen wrote:
...
> >
> > 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!
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2017-03-18 23:17 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-03-17 19:44 [U-Boot] [PATCH] fdt_support: Fixup 'ethernet' aliases not ending in digits Tuomas Tynkkynen
2017-03-18 1:26 ` Tom Rini
2017-03-18 18:20 ` Tuomas Tynkkynen
2017-03-18 18:34 ` Tom Rini
2017-03-18 23:17 ` Tuomas Tynkkynen
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.