On 26 September 2017 at 21:15,  <[email protected]> wrote:
> From: Girish Pathak <[email protected]>
>
> This change adds some STATIC and CONST qualifiers (mainly to arguments
> of  functions) in PL111 and HdLcd modules.
>
> It doesn't add or modify any functionality.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Girish Pathak <[email protected]>
> Signed-off-by: Evan Lloyd <[email protected]>
> ---
>  ArmPlatformPkg/ArmVExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpress.c 
>       | 34 ++++++++++----------
>  
> ArmPlatformPkg/ArmVExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpress.c
>  | 34 ++++++++++----------
>  ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/HdLcd.c                          
>       |  4 +--
>  ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/PL111Lcd.c                       
>       |  4 +--
>  4 files changed, 38 insertions(+), 38 deletions(-)
>
> diff --git 
> a/ArmPlatformPkg/ArmVExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpress.c
>  
> b/ArmPlatformPkg/ArmVExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpress.c
> index 
> cfe3259d3c737de240350e8c3eab867b80c40948..b9859a56988f7e5be0adbaa49048a683fe586bfe
>  100644
> --- 
> a/ArmPlatformPkg/ArmVExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpress.c
> +++ 
> b/ArmPlatformPkg/ArmVExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpress.c
> @@ -46,7 +46,7 @@ typedef struct {
>
>  /** The display modes supported by the platform.
>  **/
> -LCD_RESOLUTION mResolutions[] = {
> +STATIC CONST LCD_RESOLUTION mResolutions[] = {
>    { // Mode 0 : VGA : 640 x 480 x 24 bpp
>      VGA, VGA_H_RES_PIXELS, VGA_V_RES_PIXELS, LCD_BITS_PER_PIXEL_24,
>      VGA_OSC_FREQUENCY,
> @@ -144,8 +144,8 @@ LcdPlatformInitializeDisplay (
>  **/
>  EFI_STATUS
>  LcdPlatformGetVram (
> -  OUT EFI_PHYSICAL_ADDRESS*  VramBaseAddress,
> -  OUT UINTN*                 VramSize
> +  OUT EFI_PHYSICAL_ADDRESS * CONST  VramBaseAddress,
> +  OUT UINTN * CONST                 VramSize

What is the point of this CONST (and all the other occurrences in this patch)

In all cases [AFAICT] the CONST applies to the argument itself, not to
the object it points to, which means the variable is CONST in the
scope of the function, but can still be dereferenced to assign the OUT
value.

This means your change is technically correct, but it is extremely
unidiomatic for EDK2, so an explanation why this driver needs this
would be highly appreciated.

>    )
>  {
>    EFI_STATUS              Status;
> @@ -209,7 +209,7 @@ LcdPlatformGetMaxMode(VOID)
>  **/
>  EFI_STATUS
>  LcdPlatformSetMode (
> -  IN UINT32                         ModeNumber
> +  IN CONST UINT32                         ModeNumber
>    )
>  {
>    EFI_STATUS            Status;
> @@ -267,8 +267,8 @@ LcdPlatformSetMode (
>  **/
>  EFI_STATUS
>  LcdPlatformQueryMode (
> -  IN  UINT32                                ModeNumber,
> -  OUT EFI_GRAPHICS_OUTPUT_MODE_INFORMATION  *Info
> +  IN CONST UINT32                                   ModeNumber,
> +  OUT EFI_GRAPHICS_OUTPUT_MODE_INFORMATION * CONST  Info
>    )
>  {
>    if (ModeNumber >= LcdPlatformGetMaxMode ()) {
> @@ -322,15 +322,15 @@ LcdPlatformQueryMode (
>  **/
>  EFI_STATUS
>  LcdPlatformGetTimings (
> -  IN  UINT32                              ModeNumber,
> -  OUT UINT32*                             HRes,
> -  OUT UINT32*                             HSync,
> -  OUT UINT32*                             HBackPorch,
> -  OUT UINT32*                             HFrontPorch,
> -  OUT UINT32*                             VRes,
> -  OUT UINT32*                             VSync,
> -  OUT UINT32*                             VBackPorch,
> -  OUT UINT32*                             VFrontPorch
> +  IN  CONST UINT32                              ModeNumber,
> +  OUT UINT32 * CONST                            HRes,
> +  OUT UINT32 * CONST                            HSync,
> +  OUT UINT32 * CONST                            HBackPorch,
> +  OUT UINT32 * CONST                            HFrontPorch,
> +  OUT UINT32 * CONST                            VRes,
> +  OUT UINT32 * CONST                            VSync,
> +  OUT UINT32 * CONST                            VBackPorch,
> +  OUT UINT32 * CONST                            VFrontPorch
>    )
>  {
>    if (ModeNumber >= LcdPlatformGetMaxMode ()) {
> @@ -359,8 +359,8 @@ LcdPlatformGetTimings (
>  **/
>  EFI_STATUS
>  LcdPlatformGetBpp (
> -  IN  UINT32                              ModeNumber,
> -  OUT LCD_BPP  *                          Bpp
> +  IN CONST UINT32                        ModeNumber,
> +  OUT LCD_BPP * CONST                    Bpp
>    )
>  {
>    if (ModeNumber >= LcdPlatformGetMaxMode ()) {
> diff --git 
> a/ArmPlatformPkg/ArmVExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpress.c
>  
> b/ArmPlatformPkg/ArmVExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpress.c
> index 
> 84880e5fd1dfe6f824b27e53926f9bb32ff6cdf7..6ae13f06d8b396ea1c67f0bcd735a9d70f476400
>  100644
> --- 
> a/ArmPlatformPkg/ArmVExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpress.c
> +++ 
> b/ArmPlatformPkg/ArmVExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpress.c
> @@ -43,7 +43,7 @@ typedef struct {
>
>  /** The display modes supported by the platform.
>  **/
> -LCD_RESOLUTION mResolutions[] = {
> +STATIC CONST LCD_RESOLUTION mResolutions[] = {
>    {   // Mode 0 : VGA : 640 x 480 x 24 bpp
>        VGA, VGA_H_RES_PIXELS, VGA_V_RES_PIXELS, LCD_BITS_PER_PIXEL_24,
>        VGA_OSC_FREQUENCY,
> @@ -195,8 +195,8 @@ LcdPlatformInitializeDisplay (
>  **/
>  EFI_STATUS
>  LcdPlatformGetVram (
> -  OUT EFI_PHYSICAL_ADDRESS*  VramBaseAddress,
> -  OUT UINTN*                 VramSize
> +  OUT EFI_PHYSICAL_ADDRESS * CONST VramBaseAddress,
> +  OUT UINTN * CONST                VramSize
>    )
>  {
>    EFI_STATUS              Status;
> @@ -283,7 +283,7 @@ LcdPlatformGetMaxMode(VOID)
>  **/
>  EFI_STATUS
>  LcdPlatformSetMode (
> -  IN UINT32                         ModeNumber
> +  IN CONST UINT32                         ModeNumber
>    )
>  {
>    EFI_STATUS            Status;
> @@ -363,8 +363,8 @@ LcdPlatformSetMode (
>  **/
>  EFI_STATUS
>  LcdPlatformQueryMode (
> -  IN  UINT32                                ModeNumber,
> -  OUT EFI_GRAPHICS_OUTPUT_MODE_INFORMATION  *Info
> +  IN CONST UINT32                                  ModeNumber,
> +  OUT EFI_GRAPHICS_OUTPUT_MODE_INFORMATION * CONST Info
>    )
>  {
>    if (ModeNumber >= LcdPlatformGetMaxMode ()) {
> @@ -418,15 +418,15 @@ LcdPlatformQueryMode (
>  **/
>  EFI_STATUS
>  LcdPlatformGetTimings (
> -  IN  UINT32                              ModeNumber,
> -  OUT UINT32*                             HRes,
> -  OUT UINT32*                             HSync,
> -  OUT UINT32*                             HBackPorch,
> -  OUT UINT32*                             HFrontPorch,
> -  OUT UINT32*                             VRes,
> -  OUT UINT32*                             VSync,
> -  OUT UINT32*                             VBackPorch,
> -  OUT UINT32*                             VFrontPorch
> +  IN  CONST UINT32                        ModeNumber,
> +  OUT UINT32 * CONST                      HRes,
> +  OUT UINT32 * CONST                      HSync,
> +  OUT UINT32 * CONST                      HBackPorch,
> +  OUT UINT32 * CONST                      HFrontPorch,
> +  OUT UINT32 * CONST                      VRes,
> +  OUT UINT32 * CONST                      VSync,
> +  OUT UINT32 * CONST                      VBackPorch,
> +  OUT UINT32 * CONST                      VFrontPorch
>    )
>  {
>    if (ModeNumber >= LcdPlatformGetMaxMode ()) {
> @@ -455,8 +455,8 @@ LcdPlatformGetTimings (
>  **/
>  EFI_STATUS
>  LcdPlatformGetBpp (
> -  IN  UINT32                              ModeNumber,
> -  OUT LCD_BPP  *                          Bpp
> +  IN  CONST UINT32                        ModeNumber,
> +  OUT LCD_BPP * CONST                     Bpp
>    )
>  {
>    if (ModeNumber >= LcdPlatformGetMaxMode ()) {
> diff --git a/ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/HdLcd.c 
> b/ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/HdLcd.c
> index 
> 744dd3d556b5071defc6bcad5a9a30881bcb4b6f..5f950579720fb69e0a481f697a5cc4038158b409
>  100644
> --- a/ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/HdLcd.c
> +++ b/ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/HdLcd.c
> @@ -37,7 +37,7 @@
>  **/
>  EFI_STATUS
>  LcdInitialize (
> -  IN EFI_PHYSICAL_ADDRESS   VramBaseAddress
> +  IN CONST EFI_PHYSICAL_ADDRESS   VramBaseAddress
>    )
>  {
>    // Disable the controller
> @@ -81,7 +81,7 @@ LcdInitialize (
>  **/
>  EFI_STATUS
>  LcdSetMode (
> -  IN UINT32  ModeNumber
> +  IN CONST UINT32  ModeNumber
>    )
>  {
>    EFI_STATUS        Status;
> diff --git a/ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/PL111Lcd.c 
> b/ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/PL111Lcd.c
> index 
> f8a3c1f8266c0a11f111c3747688defc0d49877c..386e6140a69b045f77ee7fa60c4587d8bf4e7d54
>  100644
> --- a/ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/PL111Lcd.c
> +++ b/ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/PL111Lcd.c
> @@ -62,7 +62,7 @@ LcdIdentify (
>  **/
>  EFI_STATUS
>  LcdInitialize (
> -  IN EFI_PHYSICAL_ADDRESS   VramBaseAddress
> +  IN CONST EFI_PHYSICAL_ADDRESS   VramBaseAddress
>    )
>  {
>    // Define start of the VRAM. This never changes for any graphics mode
> @@ -83,7 +83,7 @@ LcdInitialize (
>  **/
>  EFI_STATUS
>  LcdSetMode (
> -  IN UINT32  ModeNumber
> +  IN CONST UINT32  ModeNumber
>    )
>  {
>    EFI_STATUS        Status;
> --
> Guid("CE165669-3EF3-493F-B85D-6190EE5B9759")
>
_______________________________________________
edk2-devel mailing list
[email protected]
https://lists.01.org/mailman/listinfo/edk2-devel

Reply via email to