On Fri, 17 Mar 2017 21:26:51 -0400
Tom Rini <[email protected]> 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 <[email protected]>  
> 
> 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.
_______________________________________________
U-Boot mailing list
[email protected]
https://lists.denx.de/listinfo/u-boot

Reply via email to