From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wr1-f68.google.com (mail-wr1-f68.google.com [209.85.221.68]) by mx.groups.io with SMTP id smtpd.web12.7256.1581182672016524763 for ; Sat, 08 Feb 2020 09:24:32 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@linaro.org header.s=google header.b=SzmgZCaU; spf=pass (domain: linaro.org, ip: 209.85.221.68, mailfrom: ard.biesheuvel@linaro.org) Received: by mail-wr1-f68.google.com with SMTP id m16so2490596wrx.11 for ; Sat, 08 Feb 2020 09:24:31 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=CnYP8PoR/x22wWRMuEfcSJBiQy3Y7dEHs261j0uemmA=; b=SzmgZCaUvCUsKh+Wa+Ujgr7XFM5D21MIlrmIBvCmMB6MZsx/KcTL7u5LyrMy/0nb2W 0QyHtzBwuTGEZghLnTqMvGfg0aaOurkV3RrXXPPvNXELgC3WJ66fNcUWt63aSz2HWPp0 +Ad/8DuwMyd/oFbsweHZdb7yooSIe0IqQIUzVdY/ZEgcc3T2tFsrsx3qrOAr6AA7JPzE 5XerIh+868n7V6SPJVkGmF2RQkYlf53yc7vPiBMAMAHjTa2EYU6tr+dxZAO8x9fI3VA8 3O98Ih7KtxfWrcOHMM1WrscMDmem37ndjuRXbjK7Jx8Ro+jaJyQMFP7uJgTpKgxXbvo0 f6MA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=CnYP8PoR/x22wWRMuEfcSJBiQy3Y7dEHs261j0uemmA=; b=WKdkjrXiB5gUI2tkKkpGtmgML2NPSVtVxTnrqVYNHueEZQFCfzALKkhHC4w4OZchGT 78rgIjtYGCWQqVr2YBRo1F6a2tCVNC0MFiGXOKisTdn8W6k+sZFyNpIEcv8hP58h6sln qQlz6anqfK3HHO/IUHzutXDy38L8qkEQLVFNfUvtuWFvwL+AIf5OkOJhR++q7mUDynlH z3WxNTwiM+jivWKNP8SY5X4+0NoZ2uSzIzp/hN/0M6Y3PrYvvyBoq6weCKQQQJbV3jVs wIy7jGWyqwWBDWivvxgn0JRFtqNjmkEOfib40FFLhg5QTW3mzm6JkvgDypq9CWuYRrI2 M4yA== X-Gm-Message-State: APjAAAWbhvulYUOIoP7i7J3m4+utxGwoq6pcDmkOm+8kunSFRnZMkYNd 7UvWjAxySX6WDKRiZNT3yTRVeBpXD1Mj9q8uhHfRnQ== X-Google-Smtp-Source: APXvYqyeERnKDaHrUZ1Enk9M5FResPDktkYj6hzw0XPHliZx9/p/1gFCV11Kl4UMuI3mXRS5ZJfNNVqK8PpYfbmC8dM= X-Received: by 2002:a5d:42c6:: with SMTP id t6mr6157884wrr.151.1581182670471; Sat, 08 Feb 2020 09:24:30 -0800 (PST) MIME-Version: 1.0 References: <20200203134010.12144-1-pete@akeo.ie> In-Reply-To: From: "Ard Biesheuvel" Date: Sat, 8 Feb 2020 18:24:19 +0100 Message-ID: Subject: Re: [edk2-platforms][PATCH 1/1] Platform/RPi4: Add ACPI entry for Genet network interface To: Pete Batard Cc: edk2-devel-groups-io , Leif Lindholm , =?UTF-8?Q?Philippe_Mathieu=2DDaud=C3=A9?= , Jeremy Linton Content-Type: text/plain; charset="UTF-8" On Sat, 8 Feb 2020 at 15:19, Pete Batard wrote: > > On 2020.02.08 08:52, Ard Biesheuvel wrote: > > On Mon, 3 Feb 2020 at 13:40, Pete Batard wrote: > >> > >> The Raspberry Pi 4 platforms uses a Broadcom Genet network interface, for > >> which we need ACPI entries in order to make it usable under Linux. > >> > >> This patch adds these entries, including a max-dma-burst-size DSD attribute > >> aimed at simplifying support for Genet on distros that use older kernels, > >> such as Debian. > >> > >> Note that we ran these settings through someone working for Broadcom, who > >> okayed the proposed values including ownership of max-dma-burst-size (which > >> we expect to also require for Device Tree usage on older kernels, hence the > >> requirement for a designated owner). > >> > >> Signed-off-by: Pete Batard > >> --- > >> Platform/RaspberryPi/RPi4/AcpiTables/Dsdt.asl | 29 ++++++++++++++++++++ > >> 1 file changed, 29 insertions(+) > >> > >> diff --git a/Platform/RaspberryPi/RPi4/AcpiTables/Dsdt.asl b/Platform/RaspberryPi/RPi4/AcpiTables/Dsdt.asl > >> index b2f1d3439211..12c3967fa9e1 100644 > >> --- a/Platform/RaspberryPi/RPi4/AcpiTables/Dsdt.asl > >> +++ b/Platform/RaspberryPi/RPi4/AcpiTables/Dsdt.asl > >> @@ -267,6 +267,35 @@ DefinitionBlock ("Dsdt.aml", "DSDT", 5, "MSFT", "EDK2", 2) > >> } > >> } > >> > >> + Device (ETH0) > >> + { > >> + Name (_HID, "BCM6E4E") > >> + Name (_CID, "BCM6E4E") > > > > What is the point of having identical _HID and _CID? Is that a Windows thing? > > > >> + Name (_UID, 0x0) > >> + Name (_CCA, 0x0) > >> + Method (_STA) > >> + { > >> + Return (0xf) > >> + } > >> + Method (_CRS, 0x0, Serialized) > >> + { > >> + Name (RBUF, ResourceTemplate () > >> + { > >> + Memory32Fixed (ReadWrite, 0xfd580000, 0x10000, ) > > > > We could grab that base address from a PCD as well, no? > > Well, we don't seem to be using PCDs in any of the .asl's we have (we > only do so on .aslc) and, also, that couldn't really be done without the > previous series having been integrated. > > I don't mind going for a PCD here, but do you really want to do that > just for ETH0? It'll just look weird because it's going to be the only > .asl address that we'll pick from a PCD. > > If we're going to move to using PCDs in .asl's, I'd rather do that > across the board and craft a patch that addresses all, including > Uart.asl and so on... Else, it's a bit inconsistent as far as I'm concerned. > That's a fair point. Let's leave it for now. > >> + Interrupt (ResourceConsumer, Level, ActiveHigh, Exclusive) { 0xBD } > >> + Interrupt (ResourceConsumer, Level, ActiveHigh, Exclusive) { 0xBE } > > > > Why not > > > > Interrupt (ResourceConsumer, Level, ActiveHigh, Exclusive) { 0xBD, 0xBE } > > Likewise: Consistency (be it ill placed or not). > > We've simply followed what GPIO had here. > > Again, I don't mind making the .asl cleaner, but if we're going to do > so, I'd rather not be piecemeal about it, and do that in a separate > patch that harmonises the whole thing. > > If you're okay with that approach, and not require a v2 for this, I can > send a separate patch that cleans up the .asl's and uses PCDs early next > week. > I'll just merge the patch as-is. These are mostly nitpicks anyway, and the current version has been tested and is know to work (right?) so no point in doing a respin.