From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wr1-f66.google.com (mail-wr1-f66.google.com [209.85.221.66]) by mx.groups.io with SMTP id smtpd.web09.19152.1583174045633790835 for ; Mon, 02 Mar 2020 10:34:06 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@akeo-ie.20150623.gappssmtp.com header.s=20150623 header.b=ksPG+thm; spf=none, err=permanent DNS error (domain: akeo.ie, ip: 209.85.221.66, mailfrom: pete@akeo.ie) Received: by mail-wr1-f66.google.com with SMTP id r17so1028387wrj.7 for ; Mon, 02 Mar 2020 10:34:05 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=akeo-ie.20150623.gappssmtp.com; s=20150623; h=subject:to:cc:references:from:message-id:date:user-agent :mime-version:in-reply-to:content-language:content-transfer-encoding; bh=tTYXZfu9RdzRMpltl2RsGw4uJJB3HlvhoZ3mVM/T6FU=; b=ksPG+thmGJ9iL5tp5QpnJardagaflzQlkX4Tvbf2MvlKoxbZEMHkC/RwtdgvmN1FWz 6wSMhLqhhb9ilH8pZUYvJfVvBomdaAQ2gHyh+I38mtTU+/k72J1go1h6ruYTNWrWMZhL X3/uQ9oZYosDll2xUX7kWyj08+PF+OIWLh/hrjCIevs6ZBJ5duy/NpcgWasVXiW2VB3W 1kWwRg4sz2EPsBQNUd1LfI0/tdeTZO+qh0WLuCyQ2+7Nxw7u3TOCdsfzwbelmR4Rfl1d i/J2qvceaKG1neMCFgjqs801aOPtq00dTW62IbqmWUmL1Ex1SqVQL2IHOcweurqI8wAH xQeA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:cc:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=tTYXZfu9RdzRMpltl2RsGw4uJJB3HlvhoZ3mVM/T6FU=; b=hbkhHyLGB6DFO8BirisywtUDpAQs6NsWmr/dZlCxETFO9xhmr+ZbDWrTxz9LDEoybZ cwILkunNcKV1IiFPQiRfGnwFEP2B0+5ZQ8SGoLw3jCO84qCOhEMnif669hgzASxYTT6z GsRnauLzlITC/uuhi1VSEUtmkm7tfWiVPREPSuS/x+suZTra++yBSlVrBlxMmO1ow924 gdLwYOx6Zt9Z/Fef09L+mWl0Hj50wOxFUsGcY4u9BuxecqDptxVGUHmVercObmknUqQW dpAd2ph/ncdA18kxhvIQy1juZiNRY7rjEN/MCOyglP6UR0fOzwM+piiekY+TykJW+8u/ pZDQ== X-Gm-Message-State: ANhLgQ2Xdp7VUtipJ4ObpMflszSKho/Bcw7x8thLK2jpabfC6FX0PV0z KZMh9Op4QGx0hrjIEXuLGYuEog== X-Google-Smtp-Source: ADFU+vtOfMt1bltZY5FrGJGm/MYlL0KlRfW8V2N8WyoIIXigREiijvzFSnf5ngLFKRMu91hZdJZb/Q== X-Received: by 2002:a5d:4984:: with SMTP id r4mr813226wrq.137.1583174044104; Mon, 02 Mar 2020 10:34:04 -0800 (PST) Return-Path: Received: from [10.0.0.122] ([84.203.56.244]) by smtp.googlemail.com with ESMTPSA id w206sm384012wmg.11.2020.03.02.10.34.02 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 02 Mar 2020 10:34:03 -0800 (PST) Subject: Re: [edk2-devel][PATCH 1/1] Platform/RPi: Move away from AcpiPlatformDxe for loading ACPI tables To: Ard Biesheuvel Cc: edk2-devel-groups-io , Leif Lindholm , =?UTF-8?Q?Philippe_Mathieu-Daud=c3=a9?= , Andrei Warkentin References: <20200302172113.6260-1-pete@akeo.ie> From: "Pete Batard" Message-ID: <4b7ef9ee-0824-149e-32f3-750cffedd69a@akeo.ie> Date: Mon, 2 Mar 2020 18:34:01 +0000 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:68.0) Gecko/20100101 Thunderbird/68.5.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-GB Content-Transfer-Encoding: 7bit Points noted. Will look to avoid these in future patches. Regards, /Pete On 2020.03.02 18:30, Ard Biesheuvel wrote: > On Mon, 2 Mar 2020 at 18:21, Pete Batard wrote: >> >> From: Andrei Warkentin >> >> Instead use ConfigDxe. This will allow selective loading/patching >> to enable different SBBR/EBBR profiles. >> > > For future patches, please don't break sentences in between the title > and the commit log body. In my email window, this starts with 'Instead > use ConfigDxe', and it takes me more cycles than necessary to figure > out what this patch is about. The commit log should make sense by > itself, and the title is just a summary of that. > > > > >> Signed-off-by: Pete Batard >> --- >> Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c | 10 ++++++++++ >> Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf | 2 ++ >> Platform/RaspberryPi/RPi3/RPi3.dsc | 2 +- >> Platform/RaspberryPi/RPi3/RPi3.fdf | 1 - >> Platform/RaspberryPi/RPi4/RPi4.dsc | 2 +- >> Platform/RaspberryPi/RPi4/RPi4.fdf | 1 - >> 6 files changed, 14 insertions(+), 4 deletions(-) >> >> diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c >> index f92ac709a3d8..5c86b6dd12b1 100644 >> --- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c >> +++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c >> @@ -11,6 +11,7 @@ >> #include >> #include >> #include >> +#include > > I know this is not ordered to begin with, but we usually try not to > make things worse. I moved this include to the top. > >> #include >> #include >> #include >> @@ -26,6 +27,12 @@ extern UINT8 ConfigDxeStrings[]; >> >> STATIC RASPBERRY_PI_FIRMWARE_PROTOCOL *mFwProtocol; >> >> +/* >> + * The GUID inside Platform/RaspberryPi/RPi3/AcpiTables/AcpiTables.inf and >> + * Platform/RaspberryPi/RPi4/AcpiTables/AcpiTables.inf _must_ match below. >> + */ >> +STATIC CONST EFI_GUID mAcpiTableFile = { 0x7E374E25, 0x8E01, 0x4FEE, { 0x87, 0xf2, 0x39, 0x0C, 0x23, 0xC6, 0x06, 0xCD } }; > > This line is 122 characters long. I fixed that up for you. > >> + >> typedef struct { >> VENDOR_DEVICE_PATH VendorDevicePath; >> EFI_DEVICE_PATH_PROTOCOL End; >> @@ -408,5 +415,8 @@ ConfigInitialize ( >> DEBUG ((DEBUG_ERROR, "Couldn't install ConfigDxe configuration pages: %r\n", Status)); >> } >> >> + Status = LocateAndInstallAcpiFromFv (&mAcpiTableFile); >> + ASSERT_EFI_ERROR (Status); >> + >> return EFI_SUCCESS; >> } >> diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf >> index 817cb98c1933..dc726cc6d934 100644 >> --- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf >> +++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf >> @@ -35,6 +35,7 @@ [Packages] >> MdeModulePkg/MdeModulePkg.dec >> Silicon/Broadcom/Bcm283x/Bcm283x.dec >> Platform/RaspberryPi/RaspberryPi.dec >> + EmbeddedPkg/EmbeddedPkg.dec >> >> [LibraryClasses] >> BaseLib >> @@ -46,6 +47,7 @@ [LibraryClasses] >> UefiDriverEntryPoint >> HiiLib >> GpioLib >> + AcpiLib >> >> [Guids] >> gConfigDxeFormSetGuid >> diff --git a/Platform/RaspberryPi/RPi3/RPi3.dsc b/Platform/RaspberryPi/RPi3/RPi3.dsc >> index 304bc3dfeadf..df5b246af1f8 100644 >> --- a/Platform/RaspberryPi/RPi3/RPi3.dsc >> +++ b/Platform/RaspberryPi/RPi3/RPi3.dsc >> @@ -346,6 +346,7 @@ [LibraryClasses.common] >> PlatformBootManagerLib|Platform/RaspberryPi/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf >> CustomizedDisplayLib|MdeModulePkg/Library/CustomizedDisplayLib/CustomizedDisplayLib.inf >> FileExplorerLib|MdeModulePkg/Library/FileExplorerLib/FileExplorerLib.inf >> + AcpiLib|EmbeddedPkg/Library/AcpiLib/AcpiLib.inf >> >> [LibraryClasses.common.UEFI_DRIVER] >> UefiScsiLib|MdePkg/Library/UefiScsiLib/UefiScsiLib.inf >> @@ -548,7 +549,6 @@ [Components.common] >> # ACPI Support >> # >> MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableDxe.inf >> - MdeModulePkg/Universal/Acpi/AcpiPlatformDxe/AcpiPlatformDxe.inf >> MdeModulePkg/Universal/Acpi/BootGraphicsResourceTableDxe/BootGraphicsResourceTableDxe.inf >> Platform/RaspberryPi/AcpiTables/AcpiTables.inf >> >> diff --git a/Platform/RaspberryPi/RPi3/RPi3.fdf b/Platform/RaspberryPi/RPi3/RPi3.fdf >> index ec3742c83729..66c2cbada59b 100644 >> --- a/Platform/RaspberryPi/RPi3/RPi3.fdf >> +++ b/Platform/RaspberryPi/RPi3/RPi3.fdf >> @@ -240,7 +240,6 @@ [FV.FvMain] >> # ACPI Support >> # >> INF MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableDxe.inf >> - INF MdeModulePkg/Universal/Acpi/AcpiPlatformDxe/AcpiPlatformDxe.inf >> INF MdeModulePkg/Universal/Acpi/BootGraphicsResourceTableDxe/BootGraphicsResourceTableDxe.inf >> INF RuleOverride = ACPITABLE Platform/RaspberryPi/AcpiTables/AcpiTables.inf >> >> diff --git a/Platform/RaspberryPi/RPi4/RPi4.dsc b/Platform/RaspberryPi/RPi4/RPi4.dsc >> index c039f6df2eb4..94e0d91ede2f 100644 >> --- a/Platform/RaspberryPi/RPi4/RPi4.dsc >> +++ b/Platform/RaspberryPi/RPi4/RPi4.dsc >> @@ -355,6 +355,7 @@ [LibraryClasses.common] >> PlatformBootManagerLib|Platform/RaspberryPi/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf >> CustomizedDisplayLib|MdeModulePkg/Library/CustomizedDisplayLib/CustomizedDisplayLib.inf >> FileExplorerLib|MdeModulePkg/Library/FileExplorerLib/FileExplorerLib.inf >> + AcpiLib|EmbeddedPkg/Library/AcpiLib/AcpiLib.inf >> >> [LibraryClasses.common.UEFI_DRIVER] >> UefiScsiLib|MdePkg/Library/UefiScsiLib/UefiScsiLib.inf >> @@ -574,7 +575,6 @@ [Components.common] >> # ACPI Support >> # >> MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableDxe.inf >> - MdeModulePkg/Universal/Acpi/AcpiPlatformDxe/AcpiPlatformDxe.inf >> MdeModulePkg/Universal/Acpi/BootGraphicsResourceTableDxe/BootGraphicsResourceTableDxe.inf >> Platform/RaspberryPi/AcpiTables/AcpiTables.inf >> >> diff --git a/Platform/RaspberryPi/RPi4/RPi4.fdf b/Platform/RaspberryPi/RPi4/RPi4.fdf >> index b2a6ac9e6c66..ee57cc0dac89 100644 >> --- a/Platform/RaspberryPi/RPi4/RPi4.fdf >> +++ b/Platform/RaspberryPi/RPi4/RPi4.fdf >> @@ -236,7 +236,6 @@ [FV.FvMain] >> # ACPI Support >> # >> INF MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableDxe.inf >> - INF MdeModulePkg/Universal/Acpi/AcpiPlatformDxe/AcpiPlatformDxe.inf >> INF MdeModulePkg/Universal/Acpi/BootGraphicsResourceTableDxe/BootGraphicsResourceTableDxe.inf >> INF RuleOverride = ACPITABLE Platform/RaspberryPi/AcpiTables/AcpiTables.inf >> > > Pushed as 07cc442f7212..cd6474cbed30 > > Thanks, >