Re: [edk2-platforms] [PATCH v2 2/5] Platform/ARM/SgiPkg: Use lookup table to install ACPI table

["Leif Lindholm" <leif@nuviainc.com>]

On Wed, Feb 12, 2020 at 18:59:24 +0530, Vijayenthiran Subramaniam wrote:
> Use lookup table to identify the platform and install corresponding ACPI
> tables. As the number of supported platforms grow, the existing platform
> identification using if..else does not scale well.
> 
> Signed-off-by: Vijayenthiran Subramaniam <vijayenthiran.subramaniam@arm.com>
> ---
>  Platform/ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.c | 49 ++++++++++++++++----
>  1 file changed, 39 insertions(+), 10 deletions(-)
> 
> diff --git a/Platform/ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.c b/Platform/ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.c
> index 09e00e5d538b..4ab0dd768cd1 100644
> --- a/Platform/ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.c
> +++ b/Platform/ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.c
> @@ -11,6 +11,35 @@
>  #include <Library/HobLib.h>
>  #include <SgiPlatform.h>
>  
> +typedef struct {
> +  SGI_PLATFORM_DESCRIPTOR SgiPlafromDescriptor;
> +  CONST  EFI_GUID*        AcpiTableGuid;
> +} SGI_PLATFORM_ACPI_TABLE_GUID_LOOKUP;
> +
> +// Macro to construct the SGI_PLATFORM_ACPI_TABLE_GUID_LOOKUP structure
> +#define ACPI_GUID_LOOKUP(PART_NUM, CONFIG_NUM, GUID)                           \
> +{                                                                              \
> +  {                                                                            \
> +    PART_NUM, CONFIG_NUM                                                       \
> +  },                                                                           \
> +  GUID                                                                         \
> +}                                                                              \
> +
> +STATIC SGI_PLATFORM_ACPI_TABLE_GUID_LOOKUP AcpiTableGuidLookup[] = {
> +  ACPI_GUID_LOOKUP (
> +      SGI575_PART_NUM,
> +      SGI575_CONF_NUM,
> +      &gSgi575AcpiTablesFileGuid),
> +  ACPI_GUID_LOOKUP (
> +      RD_N1E1_EDGE_PART_NUM,
> +      RD_N1_EDGE_CONF_ID,
> +      &gRdN1EdgeAcpiTablesFileGuid),
> +  ACPI_GUID_LOOKUP (
> +      RD_N1E1_EDGE_PART_NUM,
> +      RD_E1_EDGE_CONF_ID,
> +      &gRdE1EdgeAcpiTablesFileGuid),
> +};
> +
>  VOID
>  InitVirtioDevices (
>    VOID
> @@ -26,6 +55,7 @@ ArmSgiPkgEntryPoint (
>    EFI_STATUS              Status;
>    VOID                    *SystemIdHob;
>    SGI_PLATFORM_DESCRIPTOR *HobData;
> +  UINT8                   i;

Type should be UINTN.
"Index" or "Idx" would be more idiomatic.

/
    Leif

>    UINT32                  ConfigId;
>    UINT32                  PartNum;
>  
> @@ -40,16 +70,15 @@ ArmSgiPkgEntryPoint (
>    PartNum = HobData->PlatformId;
>    ConfigId = HobData->ConfigId;
>  
> -  if ((PartNum == SGI575_PART_NUM) && (ConfigId == SGI575_CONF_NUM)) {
> -    Status = LocateAndInstallAcpiFromFv (&gSgi575AcpiTablesFileGuid);
> -  } else if ((PartNum == RD_N1E1_EDGE_PART_NUM) &&
> -             (ConfigId == RD_N1_EDGE_CONF_ID)) {
> -    Status = LocateAndInstallAcpiFromFv (&gRdN1EdgeAcpiTablesFileGuid);
> -  } else if ((PartNum == RD_N1E1_EDGE_PART_NUM) &&
> -             (ConfigId == RD_E1_EDGE_CONF_ID)) {
> -    Status = LocateAndInstallAcpiFromFv (&gRdE1EdgeAcpiTablesFileGuid);
> -  } else {
> -    Status = EFI_UNSUPPORTED;
> +  Status = EFI_UNSUPPORTED;
> +
> +  // Walk through the AcpiTableGuidLookup lookup array
> +  for (i = 0; i < ARRAY_SIZE (AcpiTableGuidLookup); i++) {
> +    if ((PartNum == AcpiTableGuidLookup[i].SgiPlafromDescriptor.PlatformId) &&
> +        (ConfigId == AcpiTableGuidLookup[i].SgiPlafromDescriptor.ConfigId)) {
> +      Status = LocateAndInstallAcpiFromFv (AcpiTableGuidLookup[i].AcpiTableGuid);
> +      break;
> +    }
>    }
>  
>    if (EFI_ERROR (Status)) {
> -- 
> 2.7.4
>