From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: mx.groups.io; dkim=pass header.i=@linaro.org header.s=google header.b=KXNNIpep; spf=pass (domain: linaro.org, ip: 209.85.221.66, mailfrom: leif.lindholm@linaro.org) Received: from mail-wr1-f66.google.com (mail-wr1-f66.google.com [209.85.221.66]) by groups.io with SMTP; Wed, 24 Jul 2019 11:53:47 -0700 Received: by mail-wr1-f66.google.com with SMTP id g17so48097182wrr.5 for ; Wed, 24 Jul 2019 11:53:47 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=nbb1csuOVYcYel4xI3aaZ8RGH6Brl/+ggI8w7npcGqU=; b=KXNNIpepGL3X1i6XWUoe/wtUAGRKRgT579nJS0M2S4XfpMo8rV5OxaxiYil8+HufMX FJbEJb2dTkHxe7WlIc/QTTvTUnm7ZYmeCZkYWr/t7bxnjVoVCQ/P3ehLiCi698h6JM2x XkdTUUZoB9jZSspyQecfI2XyAdNlSRaIh4SJ3S6pwaUcp31/WEXwSPxwUA/dUixGF5Qq as9PTDBt9J/uKOhTyJyWXXcCklnETZiHzqIa9YPRn6brG8THyf37vqhi2jVWeQ2yPtTS xm+m9uCynQoI9mrhN1qb4G3Jusck5x6HKEnBatUDIPNqj3Es/zUevO53rBkGmL81PQeb rChQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=nbb1csuOVYcYel4xI3aaZ8RGH6Brl/+ggI8w7npcGqU=; b=KQgVFctMnIMtT/h1vO1EC7IhxKiKwYIY7O9hSxgSLcZq9eurmmqSBCRycuWAGuvlXi kwNpw1BBPfCUCZw40jI+W92bH0SFT2QQLAAJtzV8qy7k7bpqhGEfa5wsIq21fvKNMZy3 ts7eSdtWr0N/bTL4nla2nmioaC6L2I4//ru4skxgZymFz9USmq0U2YxeHdbbnSLN/vdm 7L5NGaUn2I/hPqd4zMddo1rfBpN1alaanCeYfSo5vvMijDfggMECQdYmwn4Rz9MwLTWT PHyeh4WRIuSapPjzr6RGXSo3xErG7jXYgQp1EhSM3/XlKb8payb3txhIelwkkK7BIbLO oUSw== X-Gm-Message-State: APjAAAWorvwAVSE7lQzEy+4nZ5WtPNL7Cqv7LFKrB5LtJ7rNncPLSsmw vQabJAERNiY+oK8H711PkM1+lQ== X-Google-Smtp-Source: APXvYqycVjO/VNnLw9X/nSBDYH3I9Fxb3JhhJ2uX9ePOs2ju/hQpckDOSF/uIRN+uuDVqPZJhRe2KQ== X-Received: by 2002:adf:de8b:: with SMTP id w11mr11605250wrl.134.1563994426032; Wed, 24 Jul 2019 11:53:46 -0700 (PDT) Return-Path: Received: from bivouac.eciton.net (bivouac.eciton.net. [2a00:1098:0:86:1000:23:0:2]) by smtp.gmail.com with ESMTPSA id w67sm59193707wma.24.2019.07.24.11.53.44 (version=TLS1_3 cipher=AEAD-AES256-GCM-SHA384 bits=256/256); Wed, 24 Jul 2019 11:53:44 -0700 (PDT) Date: Wed, 24 Jul 2019 19:53:43 +0100 From: "Leif Lindholm" To: Michael Brown Cc: devel@edk2.groups.io, ard.biesheuvel@linaro.org, pete@akeo.ie Subject: Re: [edk2-platforms: PATCHv2 1/1] Platform/RPi3: Provide "ethernet[0]" aliases in device tree Message-ID: <20190724185343.GX11541@bivouac.eciton.net> References: <20190723111727.GG11541@bivouac.eciton.net> <20190724143114.468304-2-mbrown@fensystems.co.uk> MIME-Version: 1.0 In-Reply-To: <20190724143114.468304-2-mbrown@fensystems.co.uk> User-Agent: Mutt/1.10.1 (2018-07-13) Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Hi Michael, Thanks for this. One comment below. On Wed, Jul 24, 2019 at 03:31:14PM +0100, Michael Brown wrote: > Older device trees tend to use the alias "ethernet". Newer device > trees tend to use "ethernet0" since older versions of U-Boot would > skip aliases that do not include an index number. See, for example, > Linux kernel commit 10b6c0c ("ARM: dts: bcm2835: add index to the > ethernet alias") and U-Boot commit f8e57c6 ("fdt_support: Fixup > 'ethernet' aliases not ending in digits"). > > Ensure that both "ethernet" and "ethernet0" aliases are present within > the device tree, to provide compatibility with operating systems that > expect either alias, and with device trees that contain either (or > both) aliases. > > Signed-off-by: Michael Brown > --- > .../RaspberryPi/RPi3/Drivers/FdtDxe/FdtDxe.c | 64 +++++++++++++++++++ > 1 file changed, 64 insertions(+) > > diff --git a/Platform/RaspberryPi/RPi3/Drivers/FdtDxe/FdtDxe.c b/Platform/RaspberryPi/RPi3/Drivers/FdtDxe/FdtDxe.c > index 83446e3e45..57e4bee8da 100644 > --- a/Platform/RaspberryPi/RPi3/Drivers/FdtDxe/FdtDxe.c > +++ b/Platform/RaspberryPi/RPi3/Drivers/FdtDxe/FdtDxe.c > @@ -23,6 +23,69 @@ STATIC VOID *mFdtImage; > > STATIC RASPBERRY_PI_FIRMWARE_PROTOCOL *mFwProtocol; > > +STATIC > +VOID > +FixEthernetAliases ( > + VOID > +) > +{ > + INTN Aliases; > + CONST CHAR8 *Ethernet; > + CONST CHAR8 *Ethernet0; > + CONST CHAR8 *Alias; > + UINTN CopySize; > + CHAR8 *Copy; > + INTN Retval; > + > + // > + // Look up the 'ethernet[0]' aliases > + // > + Aliases = fdt_path_offset (mFdtImage, "/aliases"); > + if (Aliases < 0) { > + DEBUG ((DEBUG_ERROR, "%a: failed to locate '/aliases'\n", __FUNCTION__)); The DEBUG statements are only included in DEBUG builds. And I think that's fine for these very detailed ones, but... > + return; > + } > + Ethernet = fdt_getprop (mFdtImage, Aliases, "ethernet", NULL); > + Ethernet0 = fdt_getprop (mFdtImage, Aliases, "ethernet0", NULL); > + Alias = Ethernet ? Ethernet : Ethernet0; > + if (!Alias) { > + DEBUG ((DEBUG_ERROR, "%a: failed to locate 'ethernet[0]' alias\n", __FUNCTION__)); > + return; > + } > + > + // > + // Create copy for fdt_setprop > + // > + CopySize = AsciiStrSize (Alias); > + Copy = AllocateCopyPool (CopySize, Alias); > + if (!Copy) { > + DEBUG ((DEBUG_ERROR, "%a: failed to copy '%a'\n", __FUNCTION__, Alias)); > + return; > + } > + > + // > + // Create missing aliases > + // > + if (!Ethernet) { > + Retval = fdt_setprop (mFdtImage, Aliases, "ethernet", Copy, CopySize); > + if (Retval != 0) { > + DEBUG ((DEBUG_ERROR, "%a: failed to create 'ethernet' alias (%d)\n", > + __FUNCTION__, Retval)); > + } > + DEBUG ((DEBUG_INFO, "%a: created 'ethernet' alias '%a'\n", __FUNCTION__, Copy)); > + } > + if (!Ethernet0) { > + Retval = fdt_setprop (mFdtImage, Aliases, "ethernet0", Copy, CopySize); > + if (Retval != 0) { > + DEBUG ((DEBUG_ERROR, "%a: failed to create 'ethernet0' alias (%d)\n", > + __FUNCTION__, Retval)); > + } > + DEBUG ((DEBUG_INFO, "%a: created 'ethernet0' alias '%a'\n", __FUNCTION__, Copy)); > + } > + > + FreePool (Copy); > +} > + > STATIC > VOID > UpdateMacAddress ( > @@ -342,6 +405,7 @@ FdtDxeInitialize ( > SanitizePSCI (); > CleanMemoryNodes (); > CleanSimpleFramebuffer (); > + FixEthernetAliases (); ...would it be worth having a return value here and Print()ing a message visible regardless of build profile if this function fails? / Leif > UpdateMacAddress (); > if (Internal) { > /* > -- > 2.21.0 >