From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=2607:f8b0:4001:c06::232; helo=mail-io0-x232.google.com; envelope-from=ard.biesheuvel@linaro.org; receiver=edk2-devel@lists.01.org Received: from mail-io0-x232.google.com (mail-io0-x232.google.com [IPv6:2607:f8b0:4001:c06::232]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 98AC621F38822 for ; Thu, 12 Oct 2017 12:43:30 -0700 (PDT) Received: by mail-io0-x232.google.com with SMTP id i38so6679404iod.2 for ; Thu, 12 Oct 2017 12:47:01 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=PLpw+HAUZReZsuDCI+/rPhIZiyVXsfGnEOzZv1H44FI=; b=ODiyz5yaNb6VCN0rcAODrhNXrHaHe0gxpBpa9wM4RRmYH5eElLqA5le28M35IHsJ4c m9gmk0K4eAMLItXFiYpqlxhm5LiERAldTpYNKX6HyhUIqAghy9AmGTNeMuhSOTQKkmmM s/es5DSBw//OJtPoiHy9o2jWtRT85kbppqpSA= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=PLpw+HAUZReZsuDCI+/rPhIZiyVXsfGnEOzZv1H44FI=; b=CBLDbV+8k63rQYsJGzx65hXVVoZt5fHsfzq9m4aXpl5ZW9vK/lUBIewBVTBWmo6uo6 R/hgO0i2bKJmoU97zejNsfFd8G3ogXWVyL6VZKUsgBPt+mkYbX/2nPziBgODT7EbqbtC AGjwQ7sxz+uPkWlWFFXlE5nqUrRv15w+7sY1gsO6piXo2lj0laISuxDLu9aROwjqNF+Y f0Aeg0L8EARFJ2SEOhMwE/3cXUm6jN7/4PVlm4wsLogxaHqnazjdDAT/eXseM6b2AHs5 8OoSLD9zGcufFk5+X65/Qi5qNbvDLHB5dbaGZm2tR1FSDsaLQ7jhIFJk1SZgTAnS/bze wLjg== X-Gm-Message-State: AMCzsaVq0UUAqjoou55+wiWqy4OrgWrq30TcrOffFw9i2FIeSVUiqed4 IFK6mMnc1yU05WAgDrxDB9KBw3RiNRXzwv6Lr59Dng== X-Google-Smtp-Source: ABhQp+SQVEcoIYVZUzFGVRc0Df+oT5sv4vQscHyyY1zgGakVO2luXySngczvMqnuT2fYjNyLkKYHxJau/l4gyILvZPM= X-Received: by 10.107.11.27 with SMTP id v27mr4765815ioi.179.1507837620570; Thu, 12 Oct 2017 12:47:00 -0700 (PDT) MIME-Version: 1.0 Received: by 10.107.131.167 with HTTP; Thu, 12 Oct 2017 12:47:00 -0700 (PDT) In-Reply-To: <20170926201529.11644-4-evan.lloyd@arm.com> References: <20170926201529.11644-1-evan.lloyd@arm.com> <20170926201529.11644-4-evan.lloyd@arm.com> From: Ard Biesheuvel Date: Thu, 12 Oct 2017 20:47:00 +0100 Message-ID: To: Evan Lloyd 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 03/19] ArmPlatformPkg: PL111 and HDLCD: add const qualifier X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 12 Oct 2017 19:43:30 -0000 Content-Type: text/plain; charset="UTF-8" On 26 September 2017 at 21:15, wrote: > From: Girish Pathak > > 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 > Signed-off-by: Evan Lloyd > --- > 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") >