Re: [edk2-devel][PATCH 1/1] Platform/RPi: Move away from AcpiPlatformDxe for loading ACPI tables

["Ard Biesheuvel" <ard.biesheuvel@linaro.org>]

On Mon, 2 Mar 2020 at 18:21, Pete Batard <pete@akeo.ie> wrote:
>
> From: Andrei Warkentin <andrey.warkentin@gmail.com>
>
> 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 <pete@akeo.ie>
> ---
>  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 <Library/HiiLib.h>
>  #include <Library/DebugLib.h>
>  #include <Library/IoLib.h>
> +#include <Library/AcpiLib.h>

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 <Library/UefiBootServicesTableLib.h>
>  #include <Library/UefiRuntimeServicesTableLib.h>
>  #include <Library/DevicePathLib.h>
> @@ -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,