Re: [edk2-devel] [PATCH edk2-platforms 1/2] Silicon/SynQuacer/PlatformDxe: move EMMC SSDT handling to core routine

["Philippe Mathieu-Daudé" <philmd@redhat.com>]

On 11/29/19 11:47 AM, Ard Biesheuvel via Groups.Io wrote:
> In preparation of adding support for describing the presence of OP-TEE
> via a SSDT ACPI table, refactor the existing code so we will be able to
> reuse it more easily.
> 
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
>   Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/Emmc.c        | 55 -----------------
>   Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.c | 65 ++++++++++++++++++++
>   2 files changed, 65 insertions(+), 55 deletions(-)
> 
> diff --git a/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/Emmc.c b/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/Emmc.c
> index 0d0e5edad901..90b67152c768 100644
> --- a/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/Emmc.c
> +++ b/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/Emmc.c
> @@ -53,10 +53,6 @@
>   
>   STATIC EFI_HANDLE mSdMmcControllerHandle;
>   
> -STATIC EFI_ACPI_DESCRIPTION_HEADER      *mSsdt;
> -STATIC UINTN                            mSsdtSize;
> -STATIC VOID                             *mEventRegistration;
> -
>   /**
>   
>     Override function for SDHCI capability bits
> @@ -185,31 +181,6 @@ STATIC EDKII_SD_MMC_OVERRIDE mSdMmcOverride = {
>     SynQuacerSdMmcNotifyPhase,
>   };
>   
> -STATIC
> -VOID
> -EFIAPI
> -InstallAcpiTable (
> -  IN EFI_EVENT                      Event,
> -  IN VOID*                          Context
> -  )
> -{
> -  UINTN                             TableKey;
> -  EFI_STATUS                        Status;
> -  EFI_ACPI_TABLE_PROTOCOL           *AcpiTable;
> -
> -  Status = gBS->LocateProtocol (&gEfiAcpiTableProtocolGuid, NULL,
> -                  (VOID **)&AcpiTable);
> -  if (EFI_ERROR (Status)) {
> -    return;
> -  }
> -
> -  Status = AcpiTable->InstallAcpiTable (AcpiTable, mSsdt, mSsdtSize, &TableKey);
> -  if (EFI_ERROR (Status)) {
> -    DEBUG ((DEBUG_WARN, "%a: failed to install SSDT table for eMMC - %r\n",
> -      __FUNCTION__, Status));
> -  }
> -}
> -
>   EFI_STATUS
>   EFIAPI
>   RegisterEmmc (
> @@ -218,32 +189,6 @@ RegisterEmmc (
>   {
>     EFI_STATUS                      Status;
>     EFI_HANDLE                      Handle;
> -  UINTN                           Index;
> -
> -  if (mHiiSettings->AcpiPref == ACPIPREF_ACPI) {
> -    //
> -    // Load the SSDT table from a raw section in this FFS file.
> -    //
> -    for (Index = 0;; Index++) {
> -      Status = GetSectionFromFv (&gEfiCallerIdGuid, EFI_SECTION_RAW, Index,
> -                 (VOID **)&mSsdt, &mSsdtSize);
> -      if (EFI_ERROR (Status)) {
> -        break;
> -      }
> -
> -      if (mSsdt->OemTableId != EMMC_TABLE_ID) {
> -        continue;
> -      }
> -
> -      //
> -      // Register for the ACPI table protocol
> -      //
> -      EfiCreateProtocolNotifyEvent (&gEfiAcpiTableProtocolGuid, TPL_CALLBACK,
> -        InstallAcpiTable, NULL, &mEventRegistration);
> -
> -      break;
> -    }
> -  }
>   
>     Status = RegisterNonDiscoverableMmioDevice (
>                NonDiscoverableDeviceTypeSdhci,
> diff --git a/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.c b/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.c
> index 73cc560fa8d8..c9cc37dd2478 100644
> --- a/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.c
> +++ b/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.c
> @@ -113,6 +113,11 @@ STATIC EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR mI2c1Desc[] = {
>     }
>   };
>   
> +STATIC EFI_ACPI_DESCRIPTION_HEADER      *mEmmcSsdt;
> +STATIC UINTN                            mEmmcSsdtSize;
> +
> +STATIC VOID                             *mAcpiTableEventRegistration;
> +
>   STATIC
>   EFI_STATUS
>   RegisterDevice (
> @@ -256,6 +261,32 @@ EnableSettingsForm (
>     return InstallHiiPages ();
>   }
>   
> +STATIC
> +VOID
> +EFIAPI
> +InstallAcpiTables (

Maybe rename as InstallEmmcSsdtAcpiTables?

Regardless the name chosen:
Reviewed-by: Philippe Mathieu-Daude <philmd@redhat.com>

> +  IN EFI_EVENT                      Event,
> +  IN VOID*                          Context
> +  )
> +{
> +  UINTN                             TableKey;
> +  EFI_STATUS                        Status;
> +  EFI_ACPI_TABLE_PROTOCOL           *AcpiTable;
> +
> +  Status = gBS->LocateProtocol (&gEfiAcpiTableProtocolGuid, NULL,
> +                  (VOID **)&AcpiTable);
> +  if (EFI_ERROR (Status)) {
> +    return;
> +  }
> +
> +  Status = AcpiTable->InstallAcpiTable (AcpiTable, mEmmcSsdt, mEmmcSsdtSize,
> +                        &TableKey);
> +  if (EFI_ERROR (Status)) {
> +    DEBUG ((DEBUG_WARN, "%a: failed to install SSDT table for eMMC - %r\n",
> +      __FUNCTION__, Status));
> +  }
> +}
> +
>   EFI_STATUS
>   EFIAPI
>   PlatformDxeEntryPoint (
> @@ -267,6 +298,9 @@ PlatformDxeEntryPoint (
>     VOID                            *Dtb;
>     UINTN                           DtbSize;
>     EFI_HANDLE                      Handle;
> +  EFI_ACPI_DESCRIPTION_HEADER     *Ssdt;
> +  UINTN                           SsdtSize;
> +  UINTN                           Index;
>   
>     mHiiSettingsVal = PcdGet64 (PcdPlatformSettings);
>     mHiiSettings = (SYNQUACER_PLATFORM_VARSTORE_DATA *)&mHiiSettingsVal;
> @@ -344,5 +378,36 @@ PlatformDxeEntryPoint (
>       ASSERT_EFI_ERROR (Status);
>     }
>   
> +  if (mHiiSettings->AcpiPref == ACPIPREF_ACPI) {
> +    //
> +    // Load the SSDT tables from a raw section in this FFS file.
> +    //
> +    for (Index = 0;; Index++) {
> +      Status = GetSectionFromFv (&gEfiCallerIdGuid, EFI_SECTION_RAW, Index,
> +                 (VOID **)&Ssdt, &SsdtSize);
> +      if (EFI_ERROR (Status)) {
> +        break;
> +      }
> +
> +      switch (Ssdt->OemTableId) {
> +      case EMMC_TABLE_ID:
> +        if (mHiiSettings->EnableEmmc != EMMC_ENABLED) {
> +          break;
> +        }
> +        mEmmcSsdt = Ssdt;
> +        mEmmcSsdtSize = SsdtSize;
> +        break;
> +      }
> +    }
> +
> +    if (mEmmcSsdtSize > 0) {
> +      //
> +      // Register for the ACPI table protocol if we found any SSDTs to install
> +      //
> +      EfiCreateProtocolNotifyEvent (&gEfiAcpiTableProtocolGuid, TPL_CALLBACK,
> +        InstallAcpiTables, NULL, &mAcpiTableEventRegistration);
> +    }
> +  }
> +
>     return EFI_SUCCESS;
>   }
>