On 22 December 2017 at 19:08, <evan.lloyd@arm.com> wrote:
> From: Girish Pathak <girish.pathak at 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>
Please drop this patch. Replacing C conditionals with CPP conditionals
makes the code more difficult to read, and reduces build time
coverage. The compiler is perfectly capable of removing the
unreachable code (and these are not hot paths anyway)
> ---
> Platform/ARM/VExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpress.c | 40 +++++++-------------
> 1 file changed, 14 insertions(+), 26 deletions(-)
>
> diff --git a/Platform/ARM/VExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpress.c b/Platform/ARM/VExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpress.c
> index 92918bb4ee6811db47791a435bc06a6dc77ae9a3..cf50b20fd9b1b44a81963655c2f88305ce6bdc67 100644
> --- a/Platform/ARM/VExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpress.c
> +++ b/Platform/ARM/VExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpress.c
> @@ -206,16 +206,11 @@ LcdPlatformGetVram (
> ASSERT (VramBaseAddress != NULL);
> ASSERT (VramSize != NULL);
>
> - // 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;
>
> @@ -242,13 +237,9 @@ LcdPlatformGetVram (
> ASSERT (FALSE);
> 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;
> }
> @@ -299,18 +290,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