Re: [PATCH 08/19] ArmPlatformPkg: PL11LcdArmVExpressLib: Improvement conditional

[Leif Lindholm <leif.lindholm@linaro.org>]

On Tue, Sep 26, 2017 at 09:15:18PM +0100, evan.lloyd@arm.com wrote:
> From: Girish Pathak <girish.pathak@arm.com>
> 
> PL111_CLCD_SITE and ARM_VE_MOTHERBOARD_SITE are both constants and
> available at build time. Use conditional compilation to process the code
> based on the value of PL111_CLCD_SITE, instead of selecting code in a
> switch statement at runtime.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Girish Pathak <girish.pathak@arm.com>
> Signed-off-by: Evan Lloyd <evan.lloyd@arm.com>

Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>

> ---
>  ArmPlatformPkg/ArmVExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpress.c | 40 +++++++-------------
>  1 file changed, 14 insertions(+), 26 deletions(-)
> 
> diff --git a/ArmPlatformPkg/ArmVExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpress.c b/ArmPlatformPkg/ArmVExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpress.c
> index 59006c4cfd771fdd7ca1dab91728d4dafe2fe831..aa5a1ff9c1c25c51796b75230ca149ae3a92db4a 100644
> --- a/ArmPlatformPkg/ArmVExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpress.c
> +++ b/ArmPlatformPkg/ArmVExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpress.c
> @@ -209,16 +209,11 @@ LcdPlatformGetVram (
>      return EFI_INVALID_PARAMETER;
>    }
>  
> -  // Is it on the motherboard or on the daughterboard?
> -  switch (PL111_CLCD_SITE) {
> -
> -  case ARM_VE_MOTHERBOARD_SITE:
> +#if (PL111_CLCD_SITE == ARM_VE_MOTHERBOARD_SITE)
>      *VramBaseAddress = (EFI_PHYSICAL_ADDRESS)PL111_CLCD_VRAM_MOTHERBOARD_BASE;
>      *VramSize = LCD_VRAM_SIZE;
>      Status = EFI_SUCCESS;
> -    break;
> -
> -  case ARM_VE_DAUGHTERBOARD_1_SITE:
> +#elif (PL111_CLCD_SITE == ARM_VE_DAUGHTERBOARD_1_SITE)
>      *VramBaseAddress = (EFI_PHYSICAL_ADDRESS)LCD_VRAM_CORE_TILE_BASE;
>      *VramSize = LCD_VRAM_SIZE;
>  
> @@ -245,13 +240,9 @@ LcdPlatformGetVram (
>        ASSERT_EFI_ERROR (Status);
>        gBS->FreePages (*VramBaseAddress, EFI_SIZE_TO_PAGES (*VramSize));
>      }
> -    break;
> -
> -  default:
> -    // Unsupported site
> -    Status = EFI_UNSUPPORTED;
> -    break;
> -  }
> +#else
> +#error PL111LcdVExpressLib: Unsupported PL111_CLCD_SITE
> +#endif // (PL111_CLCD_SITE == ARM_VE_MOTHERBOARD_SITE)
>  
>    return Status;
>  }
> @@ -303,18 +294,15 @@ LcdPlatformSetMode (
>      return EFI_INVALID_PARAMETER;
>    }
>  
> -  switch (PL111_CLCD_SITE) {
> -  case ARM_VE_MOTHERBOARD_SITE:
> -    Function = SYS_CFG_OSC;
> -    OscillatorId = PL111_CLCD_MOTHERBOARD_VIDEO_MODE_OSC_ID;
> -    break;
> -  case ARM_VE_DAUGHTERBOARD_1_SITE:
> -    Function = SYS_CFG_OSC_SITE1;
> -    OscillatorId = FixedPcdGet32 (PcdPL111LcdVideoModeOscId);
> -    break;
> -  default:
> -    return EFI_UNSUPPORTED;
> -  }
> +#if (PL111_CLCD_SITE == ARM_VE_MOTHERBOARD_SITE)
> +  Function = SYS_CFG_OSC;
> +  OscillatorId = PL111_CLCD_MOTHERBOARD_VIDEO_MODE_OSC_ID;
> +#elif (PL111_CLCD_SITE == ARM_VE_DAUGHTERBOARD_1_SITE)
> +  Function = SYS_CFG_OSC_SITE1;
> +  OscillatorId = FixedPcdGet32 (PcdPL111LcdVideoModeOscId);
> +#else
> +#error PL111LcdVExpressLib: Unsupported PL111_CLCD_SITE
> +#endif // (PL111_CLCD_SITE == ARM_VE_MOTHERBOARD_SITE)
>  
>    // Set the video mode oscillator
>    Status = ArmPlatformSysConfigSetDevice (
> -- 
> Guid("CE165669-3EF3-493F-B85D-6190EE5B9759")
> 
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel