Re: [PATCH v2 01/13] ArmPlatformPkg: Tidy Lcd code: Coding standard

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

(fix Leif's email address)

On 2 January 2018 at 15:21, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> On 2 January 2018 at 15:11, Evan Lloyd <Evan.Lloyd@arm.com> wrote:
>> Hi Ard.
>> One aim of these changes is to get those files we have to play with into a state where a beautifier like indent, astyle,  or clang-format can be used to help tidy our changes.  (NOTE, we do not have that fully working yet, but they do help.)  In a world where we have to play with several contradictory formatting standards (not just EDK2) then anything that can help is welcome.
>> Of the changes made:
>>         Fixing the include guards: is a small improvement.  (Ideally patchcheck should reject these.)
>>         Reducing lines to 80 columns: makes Leif (at least) happy, and aligns with formatter behaviour.
>>         Correcting Doxygen format comments: prevents Doxygen generating gibberish.
>>         Spaces before '(': Maintains consistency, and aligns with desired formatter behaviour.
>>
>
> To be honest, this is an aspect I hadn't considered at all. It would
> be excellent if we could use tooling to fix our code wrt to coding
> style, and if changes such as these bring us closer to that goal, I am
> all for it.
>
> Would it be feasible to run that on entire packages, i.e., ArmPkg and
> ArmPlatformPkg?
>
>> More below:
>>
>>> -----Original Message-----
>>> From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
>>> Sent: 23 December 2017 13:19
>>> To: Evan Lloyd <Evan.Lloyd@arm.com>
>>> Cc: edk2-devel@lists.01.org; "ard.biesheuvel@linaro.org"@arm.com;
>>> "leif.lindholm@linaro.org"@arm.com;
>>> "Matteo.Carlini@arm.com"@arm.com; "nd@arm.com"@arm.com
>>> Subject: Re: [PATCH v2 01/13] ArmPlatformPkg: Tidy Lcd code: Coding
>>> standard
>>>
>>> On 22 December 2017 at 18:34,  <evan.lloyd@arm.com> wrote:
>>> > From: Girish Pathak <girish.pathak at arm.com>
>>> >
>>> > There is no functional modification in this change As preparation for
>>> > further work, the formatting is corrected to meet the EDKII coding
>>> > standard.
>>> > Of specific note, some invalid include guards were fixed.
>>> >
>>> > 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>
>>>
>>> Hi Girish, Evan,
>>>
>>> I am sorry, but I really don't see the point of this patch. Given that the
>>> coding standard is not in line with common practice in Tianocore, changing
>>> comments to remove empty lines after // or changing one style to the
>>> other is just pointless churn. Also, changes like
>>>
>>> >  VOID
>>> > -LcdShutdown (
>>> > -  VOID
>>> > -  )
>>> > +LcdShutdown (VOID)
>>> >  {
>>>
>>> look backward to me, and so if the coding standard mandates that, we
>>> should changes the coding standard, not the code.
>>>
>>> --
>>> Ard.
>>>
>> [[Evan Lloyd]] Hi Ard.
>> The coding standard doesn't mandate this format, but permits it (5.7.1.5).
>> Our case is that whilst either format is acceptable, consistency is desirable, so we aimed (however imperfectly) to use a consistent style.
>> In this instance, though, this  would be reverted by the formatting tools, so I agree that it is pointless.
>>
>>>
>>>
>>> > ---
>>> >
>>> ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/LcdGraphicsOutputDxe.h |
>>> 10 +-
>>> >  ArmPlatformPkg/Include/Library/LcdPlatformLib.h                    |  14 +-
>>> >  ArmPlatformPkg/Library/HdLcd/HdLcd.h                               |  21 ++-
>>> >
>>> ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/LcdGraphicsOutputDxe.c |
>>> 187 +++++++++++---------
>>> >  ArmPlatformPkg/Library/HdLcd/HdLcd.c                               |  96 +++++----
>>> -
>>> >  ArmPlatformPkg/Library/PL111Lcd/PL111Lcd.c                         |  72 ++++--
>>> --
>>> >  6 files changed, 212 insertions(+), 188 deletions(-)
>>> >
>>> > diff --git
>>> a/ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/LcdGraphicsOutputDxe.
>>> h
>>> b/ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/LcdGraphicsOutputDxe.
>>> h
>>> > index
>>> b66efd34561f655b74a5ecfad8a97281cdd5929d..2b001b107927fc75317ce
>>> 39d370049d7740953a8 100644
>>> > ---
>>> a/ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/LcdGraphicsOutputDxe.
>>> h
>>> > +++
>>> b/ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/LcdGraphicsOutputDxe.
>>> h
>>> > @@ -1,6 +1,6 @@
>>> >  /** @file
>>> >
>>> > -  Copyright (c) 2011, ARM Ltd. All rights reserved.<BR>
>>> > +  Copyright (c) 2011-2017, ARM Ltd. All rights reserved.<BR>
>>> >    This program and the accompanying materials
>>> >    are licensed and made available under the terms and conditions of the
>>> BSD License
>>> >    which accompanies this distribution.  The full text of the license may be
>>> found at
>>> > @@ -11,9 +11,8 @@
>>> >
>>> >  **/
>>> >
>>> > -#ifndef __ARM_VE_GRAPHICS_DXE_H__
>>> > -#define __ARM_VE_GRAPHICS_DXE_H__
>>> > -
>>> > +#ifndef LCD_GRAPHICS_OUTPUT_DXE_H_
>>> > +#define LCD_GRAPHICS_OUTPUT_DXE_H_
>>> >
>>> >  #include <Base.h>
>>> >
>>> > @@ -25,7 +24,6 @@
>>> >
>>> >  #include <Protocol/DevicePath.h>
>>> >
>>> > -
>>> >  //
>>> >  // Device structures
>>> >  //
>>> > @@ -106,4 +104,4 @@ InitializeDisplay (
>>> >    IN LCD_INSTANCE* Instance
>>> >  );
>>> >
>>> > -#endif /* __ARM_VE_GRAPHICS_DXE_H__ */
>>> > +#endif /* LCD_GRAPHICS_OUTPUT_DXE_H_ */
>>> > diff --git a/ArmPlatformPkg/Include/Library/LcdPlatformLib.h
>>> b/ArmPlatformPkg/Include/Library/LcdPlatformLib.h
>>> > index
>>> b9bdf471e2d65dba7a0fcb0f7ecc352bd576b46b..b9316ec8de8425a83e2f6
>>> 27f5c24821ff9a2f750 100644
>>> > --- a/ArmPlatformPkg/Include/Library/LcdPlatformLib.h
>>> > +++ b/ArmPlatformPkg/Include/Library/LcdPlatformLib.h
>>> > @@ -1,6 +1,6 @@
>>> >  /** @file
>>> >
>>> > - Copyright (c) 2011, ARM Ltd. All rights reserved.<BR>
>>> > + Copyright (c) 2011-2017, ARM Ltd. All rights reserved.<BR>
>>> >   This program and the accompanying materials
>>> >   are licensed and made available under the terms and conditions of the
>>> BSD License
>>> >   which accompanies this distribution.  The full text of the license may be
>>> found at
>>> > @@ -11,8 +11,8 @@
>>> >
>>> >   **/
>>> >
>>> > -#ifndef __LCDPLATFORMLIB_H
>>> > -#define __LCDPLATFORMLIB_H
>>> > +#ifndef LCD_PLATFORM_LIB_H_
>>> > +#define LCD_PLATFORM_LIB_H_
>>> >
>>> >  #include <Protocol/GraphicsOutput.h>
>>> >
>>> > @@ -158,8 +158,9 @@
>>> >  #define LCD_12BPP_444_BLUE_MASK         0x0000000F
>>> >  #define LCD_12BPP_444_RESERVED_MASK     0x0000F000
>>> >
>>> > -
>>> > -// The enumeration indexes maps the PL111 LcdBpp values used in the
>>> LCD Control Register
>>> > +/** The enumeration indexes maps the PL111 LcdBpp values used in the
>>> LCD Control
>>> > +  Register
>>> > +**/
>>> >  typedef enum {
>>> >    LCD_BITS_PER_PIXEL_1 = 0,
>>> >    LCD_BITS_PER_PIXEL_2,
>>> > @@ -171,7 +172,6 @@ typedef enum {
>>> >    LCD_BITS_PER_PIXEL_12_444
>>> >  } LCD_BPP;
>>> >
>>> > -
>>> >  EFI_STATUS
>>> >  LcdPlatformInitializeDisplay (
>>> >    IN EFI_HANDLE   Handle
>>> > @@ -218,4 +218,4 @@ LcdPlatformGetBpp (
>>> >    OUT LCD_BPP*                              Bpp
>>> >    );
>>> >
>>> > -#endif
>>> > +#endif /* LCD_PLATFORM_LIB_H_ */
>>> > diff --git a/ArmPlatformPkg/Library/HdLcd/HdLcd.h
>>> b/ArmPlatformPkg/Library/HdLcd/HdLcd.h
>>> > index
>>> 6df97a9dfee60e9fda615cf3bea1b6a164a42333..861d3c398f7d6b9a171b4
>>> d8718c2816419d8e20a 100644
>>> > --- a/ArmPlatformPkg/Library/HdLcd/HdLcd.h
>>> > +++ b/ArmPlatformPkg/Library/HdLcd/HdLcd.h
>>> > @@ -1,6 +1,6 @@
>>> > -/** @file  HDLcd.h
>>> > +/** @file
>>> >
>>> > - Copyright (c) 2011-2012, ARM Ltd. All rights reserved.<BR>
>>> > + Copyright (c) 2011-2017, ARM Ltd. All rights reserved.<BR>
>>> >
>>> >   This program and the accompanying materials
>>> >   are licensed and made available under the terms and conditions of the
>>> BSD License
>>> > @@ -12,13 +12,10 @@
>>> >
>>> >   **/
>>> >
>>> > -#ifndef _HDLCD_H_
>>> > -#define _HDLCD_H_
>>> > +#ifndef HDLCD_H_
>>> > +#define HDLCD_H_
>>> >
>>> > -//
>>> >  // HDLCD Controller Register Offsets
>>> > -//
>>> > -
>>> >  #define HDLCD_REG_VERSION                 ((UINTN)PcdGet32
>>> (PcdArmHdLcdBase) + 0x000)
>>> >  #define HDLCD_REG_INT_RAWSTAT             ((UINTN)PcdGet32
>>> (PcdArmHdLcdBase) + 0x010)
>>> >  #define HDLCD_REG_INT_CLEAR               ((UINTN)PcdGet32
>>> (PcdArmHdLcdBase) + 0x014)
>>> > @@ -44,10 +41,7 @@
>>> >  #define HDLCD_REG_GREEN_SELECT            ((UINTN)PcdGet32
>>> (PcdArmHdLcdBase) + 0x248)
>>> >  #define HDLCD_REG_BLUE_SELECT             ((UINTN)PcdGet32
>>> (PcdArmHdLcdBase) + 0x24C)
>>> >
>>> > -
>>> > -//
>>> >  // HDLCD Values of registers
>>> > -//
>>> >
>>> >  // HDLCD Interrupt mask, clear and status register
>>> >  #define HDLCD_DMA_END                     BIT0    /* DMA has finished
>>> reading a frame */
>>> > @@ -79,6 +73,11 @@
>>> >  #define HDLCD_DATA_LOW                    0
>>> >  #define HDLCD_PXCLK_LOW                   0
>>> >
>>> > +// Default polarities
>>> > +#define HDLCD_DEFAULT_POLARITIES   HDLCD_PXCLK_LOW |
>>> HDLCD_DATA_HIGH         \
>>> > +                                   | HDLCD_DATEN_HIGH | HDLCD_HSYNC_LOW      \
>>> > +                                   | HDLCD_VSYNC_HIGH
>>> > +
>>> >  // Pixel Format
>>> >  #define HDLCD_LITTLE_ENDIAN              (0 << 31)
>>> >  #define HDLCD_BIG_ENDIAN                 (1 << 31)
>>> > @@ -86,4 +85,4 @@
>>> >  // Number of bytes per pixel
>>> >  #define HDLCD_4BYTES_PER_PIXEL           ((4 - 1) << 3)
>>> >
>>> > -#endif /* _HDLCD_H_ */
>>> > +#endif /* HDLCD_H_ */
>>> > diff --git
>>> a/ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/LcdGraphicsOutputDxe.
>>> c
>>> b/ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/LcdGraphicsOutputDxe.
>>> c
>>> > index
>>> b721061fc1df5695092e8c71da97ae0b9af46b3f..948e5692c943cdf62a10f4
>>> e6290470d3ccd334f1 100644
>>> > ---
>>> a/ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/LcdGraphicsOutputDxe.
>>> c
>>> > +++
>>> b/ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/LcdGraphicsOutputDxe.
>>> c
>>> > @@ -1,6 +1,6 @@
>>> >  /** @file
>>> >
>>> > - Copyright (c) 2011-2014, ARM Ltd. All rights reserved.<BR>
>>> > + Copyright (c) 2011-2017, ARM Ltd. All rights reserved.<BR>
>>> >   This program and the accompanying materials
>>> >   are licensed and made available under the terms and conditions of the
>>> BSD License
>>> >   which accompanies this distribution.  The full text of the license may be
>>> found at
>>> > @@ -9,7 +9,7 @@
>>> >   THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS"
>>> BASIS,
>>> >   WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER
>>> EXPRESS OR IMPLIED.
>>> >
>>> > - **/
>>> > +**/
>>> >
>>> >  #include <PiDxe.h>
>>> >  #include <Library/BaseMemoryLib.h>
>>> > @@ -22,12 +22,10 @@
>>> >
>>> >  #include "LcdGraphicsOutputDxe.h"
>>> >
>>> > -
>>> /**********************************************************
>>> ************
>>> > - *
>>> > - *  This file implements the Graphics Output protocol on
>>> ArmVersatileExpress
>>> > - *  using the Lcd controller
>>> > - *
>>> > -
>>> **********************************************************
>>> ************/
>>> > +/** This file implements the Graphics Output protocol on
>>> ArmVersatileExpress
>>> > +  using the Lcd controller
>>> > +
>>> > +**/
>>> >
>>> >  //
>>> >  // Global variables
>>> > @@ -64,7 +62,10 @@ LCD_INSTANCE mLcdTemplate = {
>>> >      {
>>> >        {
>>> >          HARDWARE_DEVICE_PATH, HW_VENDOR_DP,
>>> > -        { (UINT8) (sizeof(VENDOR_DEVICE_PATH)), (UINT8)
>>> ((sizeof(VENDOR_DEVICE_PATH)) >> 8) },
>>> > +        {
>>> > +          (UINT8)(sizeof (VENDOR_DEVICE_PATH)),
>>> > +          (UINT8)((sizeof (VENDOR_DEVICE_PATH)) >> 8)
>>> > +        },
>>> >        },
>>> >        // Hardware Device Path for Lcd
>>> >        EFI_CALLER_ID_GUID // Use the driver's GUID
>>> > @@ -73,10 +74,13 @@ LCD_INSTANCE mLcdTemplate = {
>>> >      {
>>> >        END_DEVICE_PATH_TYPE,
>>> >        END_ENTIRE_DEVICE_PATH_SUBTYPE,
>>> > -      { sizeof(EFI_DEVICE_PATH_PROTOCOL), 0 }
>>> > +      {
>>> > +        sizeof (EFI_DEVICE_PATH_PROTOCOL),
>>> > +        0
>>> > +      }
>>> >      }
>>> >    },
>>> > -  (EFI_EVENT) NULL // ExitBootServicesEvent
>>> > +  (EFI_EVENT)NULL // ExitBootServicesEvent
>>> >  };
>>> >
>>> >  EFI_STATUS
>>> > @@ -86,7 +90,7 @@ LcdInstanceContructor (
>>> >  {
>>> >    LCD_INSTANCE* Instance;
>>> >
>>> > -  Instance = AllocateCopyPool (sizeof(LCD_INSTANCE), &mLcdTemplate);
>>> > +  Instance = AllocateCopyPool (sizeof (LCD_INSTANCE), &mLcdTemplate);
>>> >    if (Instance == NULL) {
>>> >      return EFI_OUT_OF_RESOURCES;
>>> >    }
>>> > @@ -113,23 +117,23 @@ InitializeDisplay (
>>> >    UINTN                  VramSize;
>>> >
>>> >    Status = LcdPlatformGetVram (&VramBaseAddress, &VramSize);
>>> > -  if (EFI_ERROR(Status)) {
>>> > +  if (EFI_ERROR (Status)) {
>>> >      return Status;
>>> >    }
>>> >
>>> >    // Setup the LCD
>>> >    Status = LcdInitialize (VramBaseAddress);
>>> > -  if (EFI_ERROR(Status)) {
>>> > +  if (EFI_ERROR (Status)) {
>>> >      goto EXIT_ERROR_LCD_SHUTDOWN;
>>> >    }
>>> >
>>> >    Status = LcdPlatformInitializeDisplay (Instance->Handle);
>>> > -  if (EFI_ERROR(Status)) {
>>> > +  if (EFI_ERROR (Status)) {
>>> >      goto EXIT_ERROR_LCD_SHUTDOWN;
>>> >    }
>>> >
>>> >    // Setup all the relevant mode information
>>> > -  Instance->Gop.Mode->SizeOfInfo      =
>>> sizeof(EFI_GRAPHICS_OUTPUT_MODE_INFORMATION);
>>> > +  Instance->Gop.Mode->SizeOfInfo      = sizeof
>>> (EFI_GRAPHICS_OUTPUT_MODE_INFORMATION);
>>> >    Instance->Gop.Mode->FrameBufferBase = VramBaseAddress;
>>> >
>>> >    // Set the flag before changing the mode, to avoid infinite loops
>>> > @@ -139,7 +143,8 @@ InitializeDisplay (
>>> >    goto EXIT;
>>> >
>>> >  EXIT_ERROR_LCD_SHUTDOWN:
>>> > -  DEBUG((DEBUG_ERROR, "InitializeDisplay: ERROR - Can not initialise the
>>> display. Exit Status=%r\n", Status));
>>> > +  DEBUG ((DEBUG_ERROR, "InitializeDisplay: ERROR - Can not initialise
>>> the display. Exit Status=%r\n", Status));
>>> > +
>>> >    LcdShutdown ();
>>> >
>>> >  EXIT:
>>> > @@ -157,40 +162,44 @@ LcdGraphicsOutputDxeInitialize (
>>> >    LCD_INSTANCE* Instance;
>>> >
>>> >    Status = LcdIdentify ();
>>> > -  if (EFI_ERROR(Status)) {
>>> > +  if (EFI_ERROR (Status)) {
>>> >      goto EXIT;
>>> >    }
>>> >
>>> >    Status = LcdInstanceContructor (&Instance);
>>> > -  if (EFI_ERROR(Status)) {
>>> > +  if (EFI_ERROR (Status)) {
>>> >      goto EXIT;
>>> >    }
>>> >
>>> >    // Install the Graphics Output Protocol and the Device Path
>>> > -  Status = gBS->InstallMultipleProtocolInterfaces(
>>> > -            &Instance->Handle,
>>> > -            &gEfiGraphicsOutputProtocolGuid, &Instance->Gop,
>>> > -            &gEfiDevicePathProtocolGuid,     &Instance->DevicePath,
>>> > -            NULL
>>> > -            );
>>> > +  Status = gBS->InstallMultipleProtocolInterfaces (
>>> > +                  &Instance->Handle,
>>> > +                  &gEfiGraphicsOutputProtocolGuid,
>>> > +                  &Instance->Gop,
>>> > +                  &gEfiDevicePathProtocolGuid,
>>> > +                  &Instance->DevicePath,
>>> > +                  NULL
>>> > +                  );
>>> >
>>> > -  if (EFI_ERROR(Status)) {
>>> > -    DEBUG((DEBUG_ERROR, "GraphicsOutputDxeInitialize: Can not install
>>> the protocol. Exit Status=%r\n", Status));
>>> > +  if (EFI_ERROR (Status)) {
>>> > +    DEBUG ((DEBUG_ERROR, "LcdGraphicsOutputDxeInitialize: Can not
>>> install the protocol. Exit Status=%r\n", Status));
>>> >      goto EXIT;
>>> >    }
>>> >
>>> >    // Register for an ExitBootServicesEvent
>>> > -  // When ExitBootServices starts, this function here will make sure that
>>> the graphics driver will shut down properly,
>>> > -  // i.e. it will free up all allocated memory and perform any necessary
>>> hardware re-configuration.
>>> > +  // When ExitBootServices starts, this function will make sure that the
>>> > +  // graphics driver shuts down properly, i.e. it will free up all
>>> > +  // allocated memory and perform any necessary hardware re-
>>> configuration.
>>> >    Status = gBS->CreateEvent (
>>> > -            EVT_SIGNAL_EXIT_BOOT_SERVICES,
>>> > -            TPL_NOTIFY,
>>> > -            LcdGraphicsExitBootServicesEvent, NULL,
>>> > -            &Instance->ExitBootServicesEvent
>>> > -            );
>>> > +                  EVT_SIGNAL_EXIT_BOOT_SERVICES,
>>> > +                  TPL_NOTIFY,
>>> > +                  LcdGraphicsExitBootServicesEvent,
>>> > +                  NULL,
>>> > +                  &Instance->ExitBootServicesEvent
>>> > +                  );
>>> >
>>> > -  if (EFI_ERROR(Status)) {
>>> > -    DEBUG((DEBUG_ERROR, "GraphicsOutputDxeInitialize: Can not install
>>> the ExitBootServicesEvent handler. Exit Status=%r\n", Status));
>>> > +  if (EFI_ERROR (Status)) {
>>> > +    DEBUG ((DEBUG_ERROR, "LcdGraphicsOutputDxeInitialize: Can not
>>> install the ExitBootServicesEvent handler. Exit Status=%r\n", Status));
>>> >      goto EXIT_ERROR_UNINSTALL_PROTOCOL;
>>> >    }
>>> >
>>> > @@ -198,48 +207,46 @@ LcdGraphicsOutputDxeInitialize (
>>> >    goto EXIT;
>>> >
>>> >  EXIT_ERROR_UNINSTALL_PROTOCOL:
>>> > -  /* The following function could return an error message,
>>> > -   * however, to get here something must have gone wrong already,
>>> > -   * so preserve the original error, i.e. don't change
>>> > -   * the Status variable, even it fails to uninstall the protocol.
>>> > -   */
>>> > +  // The following function could return an error message,
>>> > +  // however, to get here something must have gone wrong already,
>>> > +  // so preserve the original error, i.e. don't change
>>> > +  // the Status variable, even it fails to uninstall the protocol.
>>> >    gBS->UninstallMultipleProtocolInterfaces (
>>> > -    Instance->Handle,
>>> > -    &gEfiGraphicsOutputProtocolGuid, &Instance->Gop, // Uninstall
>>> Graphics Output protocol
>>> > -    &gEfiDevicePathProtocolGuid,     &Instance->DevicePath,     // Uninstall
>>> device path
>>> > -    NULL
>>> > -    );
>>> > +         Instance->Handle,
>>> > +         &gEfiGraphicsOutputProtocolGuid,
>>> > +         &Instance->Gop, // Uninstall Graphics Output protocol
>>> > +         &gEfiDevicePathProtocolGuid,
>>> > +         &Instance->DevicePath,     // Uninstall device path
>>> > +         NULL
>>> > +         );
>>> >
>>> >  EXIT:
>>> >    return Status;
>>> > -
>>> >  }
>>> >
>>> > -/***************************************
>>> > - * This function should be called
>>> > - * on Event: ExitBootServices
>>> > - * to free up memory, stop the driver
>>> > - * and uninstall the protocols
>>> > - ***************************************/
>>> > +/** This function should be called
>>> > +  on Event: ExitBootServices
>>> > +  to free up memory, stop the driver
>>> > +  and uninstall the protocols
>>> > +**/
>>> >  VOID
>>> >  LcdGraphicsExitBootServicesEvent (
>>> >    IN EFI_EVENT  Event,
>>> >    IN VOID       *Context
>>> >    )
>>> >  {
>>> > -  // By default, this PCD is FALSE. But if a platform starts a predefined OS
>>> that
>>> > -  // does not use a framebuffer then we might want to disable the display
>>> controller
>>> > -  // to avoid to display corrupted information on the screen.
>>> > +  // By default, this PCD is FALSE. But if a platform starts a predefined OS
>>> > +  // that does not use a framebuffer then we might want to disable the
>>> display
>>> > +  // controller to avoid to display corrupted information on the screen.
>>> >    if (FeaturePcdGet (PcdGopDisableOnExitBootServices)) {
>>> >      // Turn-off the Display controller
>>> >      LcdShutdown ();
>>> >    }
>>> >  }
>>> >
>>> > -/***************************************
>>> > - * GraphicsOutput Protocol function, mapping to
>>> > - * EFI_GRAPHICS_OUTPUT_PROTOCOL.QueryMode
>>> > - ***************************************/
>>> > +/** GraphicsOutput Protocol function, mapping to
>>> > +  EFI_GRAPHICS_OUTPUT_PROTOCOL.QueryMode
>>> > +**/
>>> >  EFI_STATUS
>>> >  EFIAPI
>>> >  LcdGraphicsQueryMode (
>>> > @@ -252,19 +259,22 @@ LcdGraphicsQueryMode (
>>> >    EFI_STATUS Status = EFI_SUCCESS;
>>> >    LCD_INSTANCE *Instance;
>>> >
>>> > -  Instance = LCD_INSTANCE_FROM_GOP_THIS(This);
>>> > +  Instance = LCD_INSTANCE_FROM_GOP_THIS (This);
>>> >
>>> >    // Setup the hardware if not already done
>>> > -  if( !mDisplayInitialized ) {
>>> > -    Status = InitializeDisplay(Instance);
>>> > -    if (EFI_ERROR(Status)) {
>>> > +  if (!mDisplayInitialized) {
>>> > +    Status = InitializeDisplay (Instance);
>>> > +    if (EFI_ERROR (Status)) {
>>> >        goto EXIT;
>>> >      }
>>> >    }
>>> >
>>> >    // Error checking
>>> > -  if ( (This == NULL) || (Info == NULL) || (SizeOfInfo == NULL) ||
>>> (ModeNumber >= This->Mode->MaxMode) ) {
>>> > -    DEBUG((DEBUG_ERROR, "LcdGraphicsQueryMode: ERROR - For mode
>>> number %d : Invalid Parameter.\n", ModeNumber ));
>>> > +  if ((This == NULL)
>>> > +      || (Info == NULL)
>>> > +      || (SizeOfInfo == NULL)
>>> > +      || (ModeNumber >= This->Mode->MaxMode)) {
>>> > +    DEBUG ((DEBUG_ERROR, "LcdGraphicsQueryMode: ERROR - For mode
>>> number %d : Invalid Parameter.\n", ModeNumber));
>>> >      Status = EFI_INVALID_PARAMETER;
>>> >      goto EXIT;
>>> >    }
>>> > @@ -275,21 +285,20 @@ LcdGraphicsQueryMode (
>>> >      goto EXIT;
>>> >    }
>>> >
>>> > -  *SizeOfInfo = sizeof( EFI_GRAPHICS_OUTPUT_MODE_INFORMATION);
>>> > +  *SizeOfInfo = sizeof (EFI_GRAPHICS_OUTPUT_MODE_INFORMATION);
>>> >
>>> > -  Status = LcdPlatformQueryMode (ModeNumber,*Info);
>>> > -  if (EFI_ERROR(Status)) {
>>> > -    FreePool(*Info);
>>> > +  Status = LcdPlatformQueryMode (ModeNumber, *Info);
>>> > +  if (EFI_ERROR (Status)) {
>>> > +    FreePool (*Info);
>>> >    }
>>> >
>>> >  EXIT:
>>> >    return Status;
>>> >  }
>>> >
>>> > -/***************************************
>>> > - * GraphicsOutput Protocol function, mapping to
>>> > - * EFI_GRAPHICS_OUTPUT_PROTOCOL.SetMode
>>> > - ***************************************/
>>> > +/** GraphicsOutput Protocol function, mapping to
>>> > +  EFI_GRAPHICS_OUTPUT_PROTOCOL.SetMode
>>> > +**/
>>> >  EFI_STATUS
>>> >  EFIAPI
>>> >  LcdGraphicsSetMode (
>>> > @@ -305,47 +314,48 @@ LcdGraphicsSetMode (
>>> >    Instance = LCD_INSTANCE_FROM_GOP_THIS (This);
>>> >
>>> >    // Setup the hardware if not already done
>>> > -  if(!mDisplayInitialized) {
>>> > +  if (!mDisplayInitialized) {
>>> >      Status = InitializeDisplay (Instance);
>>> > -    if (EFI_ERROR(Status)) {
>>> > +    if (EFI_ERROR (Status)) {
>>> >        goto EXIT;
>>> >      }
>>> >    }
>>> >
>>> >    // Check if this mode is supported
>>> > -  if( ModeNumber >= This->Mode->MaxMode ) {
>>> > -    DEBUG((DEBUG_ERROR, "LcdGraphicsSetMode: ERROR - Unsupported
>>> mode number %d .\n", ModeNumber ));
>>> > +  if (ModeNumber >= This->Mode->MaxMode) {
>>> > +    DEBUG ((DEBUG_ERROR, "LcdGraphicsSetMode: ERROR - Unsupported
>>> mode number %d .\n", ModeNumber));
>>> >      Status = EFI_UNSUPPORTED;
>>> >      goto EXIT;
>>> >    }
>>> >
>>> >    // Set the oscillator frequency to support the new mode
>>> >    Status = LcdPlatformSetMode (ModeNumber);
>>> > -  if (EFI_ERROR(Status)) {
>>> > +  if (EFI_ERROR (Status)) {
>>> >      Status = EFI_DEVICE_ERROR;
>>> >      goto EXIT;
>>> >    }
>>> >
>>> >    // Update the UEFI mode information
>>> >    This->Mode->Mode = ModeNumber;
>>> > -  LcdPlatformQueryMode (ModeNumber,&Instance->ModeInfo);
>>> > -  Status = LcdPlatformGetBpp(ModeNumber, &Bpp);
>>> > -  if (EFI_ERROR(Status)) {
>>> > +  LcdPlatformQueryMode (ModeNumber, &Instance->ModeInfo);
>>> > +  Status = LcdPlatformGetBpp (ModeNumber, &Bpp);
>>> > +  if (EFI_ERROR (Status)) {
>>> >      DEBUG ((DEBUG_ERROR, "LcdGraphicsSetMode: ERROR - Couldn't get
>>> bytes per pixel, status: %r\n", Status));
>>> >      goto EXIT;
>>> >    }
>>> >    This->Mode->FrameBufferSize =  Instance->ModeInfo.VerticalResolution
>>> > -                               * Instance->ModeInfo.PixelsPerScanLine
>>> > -                               * GetBytesPerPixel(Bpp);
>>> > +                                 * Instance->ModeInfo.PixelsPerScanLine
>>> > +                                 * GetBytesPerPixel (Bpp);
>>> >
>>> >    // Set the hardware to the new mode
>>> >    Status = LcdSetMode (ModeNumber);
>>> > -  if (EFI_ERROR(Status)) {
>>> > +  if (EFI_ERROR (Status)) {
>>> >      Status = EFI_DEVICE_ERROR;
>>> >      goto EXIT;
>>> >    }
>>> >
>>> > -  // The UEFI spec requires that we now clear the visible portions of the
>>> output display to black.
>>> > +  // The UEFI spec requires that we now clear the visible portions of the
>>> > +  // output display to black.
>>> >
>>> >    // Set the fill colour to black
>>> >    SetMem (&FillColour, sizeof (EFI_GRAPHICS_OUTPUT_BLT_PIXEL), 0x0);
>>> > @@ -361,7 +371,8 @@ LcdGraphicsSetMode (
>>> >        0,
>>> >        This->Mode->Info->HorizontalResolution,
>>> >        This->Mode->Info->VerticalResolution,
>>> > -      0);
>>> > +      0
>>> > +      );
>>> >
>>> >  EXIT:
>>> >    return Status;
>>> > @@ -372,7 +383,7 @@ GetBytesPerPixel (
>>> >    IN  LCD_BPP       Bpp
>>> >    )
>>> >  {
>>> > -  switch(Bpp) {
>>> > +  switch (Bpp) {
>>> >    case LCD_BITS_PER_PIXEL_24:
>>> >      return 4;
>>> >
>>> > diff --git a/ArmPlatformPkg/Library/HdLcd/HdLcd.c
>>> b/ArmPlatformPkg/Library/HdLcd/HdLcd.c
>>> > index
>>> 24efb68f23e3393a96fc760732d978b6346a2807..f1fffb1e83173f03f044fc0
>>> f4a5ad5c0b864695b 100644
>>> > --- a/ArmPlatformPkg/Library/HdLcd/HdLcd.c
>>> > +++ b/ArmPlatformPkg/Library/HdLcd/HdLcd.c
>>> > @@ -1,6 +1,6 @@
>>> > -/** @file  Lcd.c
>>> > +/** @file
>>> >
>>> > -  Copyright (c) 2011-2012, ARM Ltd. All rights reserved.<BR>
>>> > +  Copyright (c) 2011-2017, ARM Ltd. All rights reserved.<BR>
>>> >
>>> >    This program and the accompanying materials
>>> >    are licensed and made available under the terms and conditions of the
>>> BSD License
>>> > @@ -21,12 +21,9 @@
>>> >
>>> >  #include "HdLcd.h"
>>> >
>>> > -
>>> /**********************************************************
>>> ************
>>> > - *
>>> > - *  This file contains all the bits of the Lcd that are
>>> > - *  platform independent.
>>> > - *
>>> > -
>>> **********************************************************
>>> ************/
>>> > +/** This file contains all the bits of the Lcd that are
>>> > +  platform independent.
>>> > +**/
>>> >
>>> >  STATIC
>>> >  UINTN
>>> > @@ -34,7 +31,7 @@ GetBytesPerPixel (
>>> >    IN  LCD_BPP       Bpp
>>> >    )
>>> >  {
>>> > -  switch(Bpp) {
>>> > +  switch (Bpp) {
>>> >    case LCD_BITS_PER_PIXEL_24:
>>> >      return 4;
>>> >
>>> > @@ -60,21 +57,27 @@ LcdInitialize (
>>> >    )
>>> >  {
>>> >    // Disable the controller
>>> > -  MmioWrite32(HDLCD_REG_COMMAND, HDLCD_DISABLE);
>>> > +  MmioWrite32 (HDLCD_REG_COMMAND, HDLCD_DISABLE);
>>> >
>>> >    // Disable all interrupts
>>> > -  MmioWrite32(HDLCD_REG_INT_MASK, 0);
>>> > +  MmioWrite32 (HDLCD_REG_INT_MASK, 0);
>>> >
>>> >    // Define start of the VRAM. This never changes for any graphics mode
>>> > -  MmioWrite32(HDLCD_REG_FB_BASE, (UINT32) VramBaseAddress);
>>> > +  MmioWrite32 (HDLCD_REG_FB_BASE, (UINT32)VramBaseAddress);
>>> >
>>> >    // Setup various registers that never change
>>> > -  MmioWrite32(HDLCD_REG_BUS_OPTIONS,  (4 << 8) | HDLCD_BURST_8);
>>> > -  MmioWrite32(HDLCD_REG_POLARITIES,   HDLCD_PXCLK_LOW |
>>> HDLCD_DATA_HIGH | HDLCD_DATEN_HIGH | HDLCD_HSYNC_LOW |
>>> HDLCD_VSYNC_HIGH);
>>> > -  MmioWrite32(HDLCD_REG_PIXEL_FORMAT, HDLCD_LITTLE_ENDIAN |
>>> HDLCD_4BYTES_PER_PIXEL);
>>> > -  MmioWrite32(HDLCD_REG_RED_SELECT,   (0 << 16 | 8 << 8 |  0));
>>> > -  MmioWrite32(HDLCD_REG_GREEN_SELECT, (0 << 16 | 8 << 8 |  8));
>>> > -  MmioWrite32(HDLCD_REG_BLUE_SELECT,  (0 << 16 | 8 << 8 | 16));
>>> > +  MmioWrite32 (HDLCD_REG_BUS_OPTIONS,  (4 << 8) |
>>> HDLCD_BURST_8);
>>> > +
>>> > +  MmioWrite32 (HDLCD_REG_POLARITIES, HDLCD_DEFAULT_POLARITIES);
>>> > +
>>> > +  MmioWrite32 (
>>> > +    HDLCD_REG_PIXEL_FORMAT,
>>> > +    HDLCD_LITTLE_ENDIAN | HDLCD_4BYTES_PER_PIXEL
>>> > +    );
>>> > +
>>> > +  MmioWrite32 (HDLCD_REG_RED_SELECT,   (0 << 16 | 8 << 8 | 0));
>>> > +  MmioWrite32 (HDLCD_REG_GREEN_SELECT, (0 << 16 | 8 << 8 | 8));
>>> > +  MmioWrite32 (HDLCD_REG_BLUE_SELECT,  (0 << 16 | 8 << 8 | 16));
>>> >
>>> >    return EFI_SUCCESS;
>>> >  }
>>> > @@ -96,63 +99,66 @@ LcdSetMode (
>>> >    UINT32            BytesPerPixel;
>>> >    LCD_BPP           LcdBpp;
>>> >
>>> > -
>>> >    // Set the video mode timings and other relevant information
>>> > -  Status = LcdPlatformGetTimings (ModeNumber,
>>> > -                                  &HRes,&HSync,&HBackPorch,&HFrontPorch,
>>> > -                                  &VRes,&VSync,&VBackPorch,&VFrontPorch);
>>> > +  Status = LcdPlatformGetTimings (
>>> > +             ModeNumber,
>>> > +             &HRes,
>>> > +             &HSync,
>>> > +             &HBackPorch,
>>> > +             &HFrontPorch,
>>> > +             &VRes,
>>> > +             &VSync,
>>> > +             &VBackPorch,
>>> > +             &VFrontPorch
>>> > +             );
>>> >    ASSERT_EFI_ERROR (Status);
>>> > -  if (EFI_ERROR( Status )) {
>>> > +  if (EFI_ERROR (Status)) {
>>> >      return EFI_DEVICE_ERROR;
>>> >    }
>>> >
>>> > -  Status = LcdPlatformGetBpp (ModeNumber,&LcdBpp);
>>> > +  Status = LcdPlatformGetBpp (ModeNumber, &LcdBpp);
>>> >    ASSERT_EFI_ERROR (Status);
>>> > -  if (EFI_ERROR( Status )) {
>>> > +  if (EFI_ERROR (Status)) {
>>> >      return EFI_DEVICE_ERROR;
>>> >    }
>>> >
>>> > -  BytesPerPixel = GetBytesPerPixel(LcdBpp);
>>> > +  BytesPerPixel = GetBytesPerPixel (LcdBpp);
>>> >
>>> >    // Disable the controller
>>> > -  MmioWrite32(HDLCD_REG_COMMAND, HDLCD_DISABLE);
>>> > +  MmioWrite32 (HDLCD_REG_COMMAND, HDLCD_DISABLE);
>>> >
>>> >    // Update the frame buffer information with the new settings
>>> > -  MmioWrite32(HDLCD_REG_FB_LINE_LENGTH, HRes * BytesPerPixel);
>>> > -  MmioWrite32(HDLCD_REG_FB_LINE_PITCH,  HRes * BytesPerPixel);
>>> > -  MmioWrite32(HDLCD_REG_FB_LINE_COUNT,  VRes - 1);
>>> > +  MmioWrite32 (HDLCD_REG_FB_LINE_LENGTH, HRes * BytesPerPixel);
>>> > +  MmioWrite32 (HDLCD_REG_FB_LINE_PITCH,  HRes * BytesPerPixel);
>>> > +  MmioWrite32 (HDLCD_REG_FB_LINE_COUNT,  VRes - 1);
>>> >
>>> >    // Set the vertical timing information
>>> > -  MmioWrite32(HDLCD_REG_V_SYNC,         VSync);
>>> > -  MmioWrite32(HDLCD_REG_V_BACK_PORCH,   VBackPorch);
>>> > -  MmioWrite32(HDLCD_REG_V_DATA,         VRes - 1);
>>> > -  MmioWrite32(HDLCD_REG_V_FRONT_PORCH,  VFrontPorch);
>>> > +  MmioWrite32 (HDLCD_REG_V_SYNC,         VSync);
>>> > +  MmioWrite32 (HDLCD_REG_V_BACK_PORCH,   VBackPorch);
>>> > +  MmioWrite32 (HDLCD_REG_V_DATA,         VRes - 1);
>>> > +  MmioWrite32 (HDLCD_REG_V_FRONT_PORCH,  VFrontPorch);
>>> >
>>> >    // Set the horizontal timing information
>>> > -  MmioWrite32(HDLCD_REG_H_SYNC,         HSync);
>>> > -  MmioWrite32(HDLCD_REG_H_BACK_PORCH,   HBackPorch);
>>> > -  MmioWrite32(HDLCD_REG_H_DATA,         HRes - 1);
>>> > -  MmioWrite32(HDLCD_REG_H_FRONT_PORCH,  HFrontPorch);
>>> > +  MmioWrite32 (HDLCD_REG_H_SYNC,         HSync);
>>> > +  MmioWrite32 (HDLCD_REG_H_BACK_PORCH,   HBackPorch);
>>> > +  MmioWrite32 (HDLCD_REG_H_DATA,         HRes - 1);
>>> > +  MmioWrite32 (HDLCD_REG_H_FRONT_PORCH,  HFrontPorch);
>>> >
>>> >    // Enable the controller
>>> > -  MmioWrite32(HDLCD_REG_COMMAND, HDLCD_ENABLE);
>>> > +  MmioWrite32 (HDLCD_REG_COMMAND, HDLCD_ENABLE);
>>> >
>>> >    return EFI_SUCCESS;
>>> >  }
>>> >
>>> >  VOID
>>> > -LcdShutdown (
>>> > -  VOID
>>> > -  )
>>> > +LcdShutdown (VOID)
>>> >  {
>>> >    // Disable the controller
>>> >    MmioWrite32 (HDLCD_REG_COMMAND, HDLCD_DISABLE);
>>> >  }
>>> >
>>> >  EFI_STATUS
>>> > -LcdIdentify (
>>> > -  VOID
>>> > -  )
>>> > +LcdIdentify (VOID)
>>> >  {
>>> >    return EFI_SUCCESS;
>>> >  }
>>> > diff --git a/ArmPlatformPkg/Library/PL111Lcd/PL111Lcd.c
>>> b/ArmPlatformPkg/Library/PL111Lcd/PL111Lcd.c
>>> > index
>>> 9b4a02045ab7ca170e0d4362ee0e2bcf1d275bdb..19a5f3c58c63173f6528d
>>> 7c1eedf51053dbeaf7e 100644
>>> > --- a/ArmPlatformPkg/Library/PL111Lcd/PL111Lcd.c
>>> > +++ b/ArmPlatformPkg/Library/PL111Lcd/PL111Lcd.c
>>> > @@ -1,6 +1,6 @@
>>> > -/** @file  PL111Lcd.c
>>> > +/** @file
>>> >
>>> > -  Copyright (c) 2011-2012, ARM Ltd. All rights reserved.<BR>
>>> > +  Copyright (c) 2011-2017, ARM Ltd. All rights reserved.<BR>
>>> >
>>> >    This program and the accompanying materials
>>> >    are licensed and made available under the terms and conditions of the
>>> BSD License
>>> > @@ -19,17 +19,12 @@
>>> >
>>> >  #include "PL111Lcd.h"
>>> >
>>> > -
>>> /**********************************************************
>>> ************
>>> > - *
>>> > - *  This file contains all the bits of the PL111 that are
>>> > - *  platform independent.
>>> > - *
>>> > -
>>> **********************************************************
>>> ************/
>>> > +/** This file contains all the bits of the PL111 that are
>>> > +  platform independent.
>>> >
>>> > +**/
>>> >  EFI_STATUS
>>> > -LcdIdentify (
>>> > -  VOID
>>> > -  )
>>> > +LcdIdentify (VOID)
>>> >  {
>>> >    DEBUG ((EFI_D_WARN, "Probing ID registers at 0x%lx for a PL111\n",
>>> >      PL111_REG_CLCD_PERIPH_ID_0));
>>> > @@ -54,11 +49,11 @@ LcdInitialize (
>>> >    )
>>> >  {
>>> >    // Define start of the VRAM. This never changes for any graphics mode
>>> > -  MmioWrite32(PL111_REG_LCD_UP_BASE, (UINT32) VramBaseAddress);
>>> > -  MmioWrite32(PL111_REG_LCD_LP_BASE, 0); // We are not using a
>>> double buffer
>>> > +  MmioWrite32 (PL111_REG_LCD_UP_BASE, (UINT32)VramBaseAddress);
>>> > +  MmioWrite32 (PL111_REG_LCD_LP_BASE, 0); // We are not using a
>>> double buffer
>>> >
>>> >    // Disable all interrupts from the PL111
>>> > -  MmioWrite32(PL111_REG_LCD_IMSC, 0);
>>> > +  MmioWrite32 (PL111_REG_LCD_IMSC, 0);
>>> >
>>> >    return EFI_SUCCESS;
>>> >  }
>>> > @@ -81,45 +76,60 @@ LcdSetMode (
>>> >    LCD_BPP           LcdBpp;
>>> >
>>> >    // Set the video mode timings and other relevant information
>>> > -  Status = LcdPlatformGetTimings (ModeNumber,
>>> > -                                  &HRes,&HSync,&HBackPorch,&HFrontPorch,
>>> > -                                  &VRes,&VSync,&VBackPorch,&VFrontPorch);
>>> > +  Status = LcdPlatformGetTimings (
>>> > +             ModeNumber,
>>> > +             &HRes,
>>> > +             &HSync,
>>> > +             &HBackPorch,
>>> > +             &HFrontPorch,
>>> > +             &VRes,
>>> > +             &VSync,
>>> > +             &VBackPorch,
>>> > +             &VFrontPorch
>>> > +             );
>>> >    ASSERT_EFI_ERROR (Status);
>>> > -  if (EFI_ERROR( Status )) {
>>> > +  if (EFI_ERROR (Status)) {
>>> >      return EFI_DEVICE_ERROR;
>>> >    }
>>> >
>>> > -  Status = LcdPlatformGetBpp (ModeNumber,&LcdBpp);
>>> > +  Status = LcdPlatformGetBpp (ModeNumber, &LcdBpp);
>>> >    ASSERT_EFI_ERROR (Status);
>>> > -  if (EFI_ERROR( Status )) {
>>> > +  if (EFI_ERROR (Status)) {
>>> >      return EFI_DEVICE_ERROR;
>>> >    }
>>> >
>>> >    // Disable the CLCD_LcdEn bit
>>> > -  LcdControl = MmioRead32( PL111_REG_LCD_CONTROL);
>>> > -  MmioWrite32(PL111_REG_LCD_CONTROL,  LcdControl & ~1);
>>> > +  LcdControl = MmioRead32 (PL111_REG_LCD_CONTROL);
>>> > +  MmioWrite32 (PL111_REG_LCD_CONTROL, LcdControl & ~1);
>>> >
>>> >    // Set Timings
>>> > -  MmioWrite32 (PL111_REG_LCD_TIMING_0,
>>> HOR_AXIS_PANEL(HBackPorch, HFrontPorch, HSync, HRes));
>>> > -  MmioWrite32 (PL111_REG_LCD_TIMING_1,
>>> VER_AXIS_PANEL(VBackPorch, VFrontPorch, VSync, VRes));
>>> > -  MmioWrite32 (PL111_REG_LCD_TIMING_2, CLK_SIG_POLARITY(HRes));
>>> > +  MmioWrite32 (
>>> > +    PL111_REG_LCD_TIMING_0,
>>> > +    HOR_AXIS_PANEL (HBackPorch, HFrontPorch, HSync, HRes)
>>> > +    );
>>> > +
>>> > +  MmioWrite32 (
>>> > +    PL111_REG_LCD_TIMING_1,
>>> > +    VER_AXIS_PANEL (VBackPorch, VFrontPorch, VSync, VRes)
>>> > +    );
>>> > +
>>> > +  MmioWrite32 (PL111_REG_LCD_TIMING_2, CLK_SIG_POLARITY (HRes));
>>> >    MmioWrite32 (PL111_REG_LCD_TIMING_3, 0);
>>> >
>>> >    // PL111_REG_LCD_CONTROL
>>> > -  LcdControl = PL111_CTRL_LCD_EN | PL111_CTRL_LCD_BPP(LcdBpp) |
>>> PL111_CTRL_LCD_TFT | PL111_CTRL_BGR;
>>> > -  MmioWrite32(PL111_REG_LCD_CONTROL,  LcdControl);
>>> > +  LcdControl = PL111_CTRL_LCD_EN | PL111_CTRL_LCD_BPP (LcdBpp)
>>> > +               | PL111_CTRL_LCD_TFT | PL111_CTRL_BGR;
>>> > +  MmioWrite32 (PL111_REG_LCD_CONTROL, LcdControl);
>>> >
>>> >    // Turn on power to the LCD Panel
>>> >    LcdControl |= PL111_CTRL_LCD_PWR;
>>> > -  MmioWrite32(PL111_REG_LCD_CONTROL,  LcdControl);
>>> > +  MmioWrite32 (PL111_REG_LCD_CONTROL, LcdControl);
>>> >
>>> >    return EFI_SUCCESS;
>>> >  }
>>> >
>>> >  VOID
>>> > -LcdShutdown (
>>> > -  VOID
>>> > -  )
>>> > +LcdShutdown (VOID)
>>> >  {
>>> >    // Disable the controller
>>> >    MmioAnd32 (PL111_REG_LCD_CONTROL, ~PL111_CTRL_LCD_EN);
>>> > --
>>> > Guid("CE165669-3EF3-493F-B85D-6190EE5B9759")
>>> >