From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
To: Evan Lloyd <evan.lloyd@arm.com>
Cc: "edk2-devel@lists.01.org" <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
Date: Thu, 12 Oct 2017 20:47:00 +0100 [thread overview]
Message-ID: <CAKv+Gu_Gzm=BT_Xt9R_0c+OqaaFvAxdHRawJywhooTEdnF-J9Q@mail.gmail.com> (raw)
In-Reply-To: <20170926201529.11644-4-evan.lloyd@arm.com>
On 26 September 2017 at 21:15, <evan.lloyd@arm.com> wrote:
> From: Girish Pathak <girish.pathak@arm.com>
>
> 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 <girish.pathak@arm.com>
> Signed-off-by: Evan Lloyd <evan.lloyd@arm.com>
> ---
> 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")
>
next prev parent reply other threads:[~2017-10-12 19:43 UTC|newest]
Thread overview: 69+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-09-26 20:15 [PATCH 00/19] ArmPlatformPkg: Update GOP evan.lloyd
2017-09-26 20:15 ` [PATCH 01/19] ArmPlatformPkg: Tidy LcdGraphicsOutputDxe code: Coding standard evan.lloyd
2017-10-12 18:45 ` Leif Lindholm
2017-09-26 20:15 ` [PATCH 02/19] ArmPlatformPkg: Tidy LcdGraphicsOutputDxe code: Added comments evan.lloyd
2017-10-12 19:02 ` Leif Lindholm
2017-12-05 18:55 ` Evan Lloyd
2017-12-05 19:58 ` Leif Lindholm
2017-12-05 22:06 ` Evan Lloyd
2017-09-26 20:15 ` [PATCH 03/19] ArmPlatformPkg: PL111 and HDLCD: add const qualifier evan.lloyd
2017-10-12 19:07 ` Leif Lindholm
2017-10-12 19:47 ` Ard Biesheuvel [this message]
2017-12-01 16:17 ` Evan Lloyd
2017-12-01 17:31 ` Ard Biesheuvel
2017-12-05 20:35 ` Evan Lloyd
2017-12-05 20:54 ` Ard Biesheuvel
2017-09-26 20:15 ` [PATCH 04/19] ArmPlatformPkg: LcdGraphicsOurputDxe: Add debug asserts evan.lloyd
2017-10-12 19:32 ` Leif Lindholm
2017-10-13 7:33 ` Ard Biesheuvel
2017-12-01 16:33 ` Evan Lloyd
2017-12-01 17:34 ` Ard Biesheuvel
2017-12-01 17:58 ` Leif Lindholm
2017-12-05 20:46 ` Evan Lloyd
2017-12-07 14:55 ` Alexei Fedorov
2017-12-07 15:10 ` Ard Biesheuvel
2017-12-07 16:53 ` Alexei Fedorov
2017-12-08 21:39 ` Ard Biesheuvel
2017-09-26 20:15 ` [PATCH 05/19] ArmPlatformPkg: PL111LcdArmVExpressLib: Minor code cleanup evan.lloyd
2017-10-12 19:33 ` Leif Lindholm
2017-09-26 20:15 ` [PATCH 06/19] ArmPlatformPkg: PL111Lcd: Replace magic number with macro evan.lloyd
2017-10-12 19:34 ` Leif Lindholm
2017-09-26 20:15 ` [PATCH 07/19] ArmPlatformPkg: PL111LcdArmVExpressLib: Use FixedPcdGet32 evan.lloyd
2017-10-12 19:35 ` Leif Lindholm
2017-09-26 20:15 ` [PATCH 08/19] ArmPlatformPkg: PL11LcdArmVExpressLib: Improvement conditional evan.lloyd
2017-10-12 19:36 ` Leif Lindholm
2017-09-26 20:15 ` [PATCH 09/19] ArmPlatformPkg: HdLcdArmVExpressLib: Use FixedPcdGet32 evan.lloyd
2017-10-12 19:38 ` Leif Lindholm
2017-09-26 20:15 ` [PATCH 10/19] ArmPlatformPkg: HdLcdArmVExpressLib: Remove status check EFI_TIMEOUT evan.lloyd
2017-10-12 19:40 ` Leif Lindholm
2017-09-26 20:15 ` [PATCH 11/19] ArmPlatformPkg: Implement LcdIdentify function for HDLCD GOP evan.lloyd
2017-10-12 19:43 ` Leif Lindholm
2017-09-26 20:15 ` [PATCH 12/19] ArmPlatformPkg: Redefine LcdPlatformGetTimings function evan.lloyd
2017-10-13 7:49 ` Leif Lindholm
2017-09-26 20:15 ` [PATCH 13/19] ArmPlatformPkg: HdLcd Remove redundant Bpp evan.lloyd
2017-10-13 7:53 ` Leif Lindholm
2017-10-17 14:32 ` Evan Lloyd
2017-10-17 15:40 ` Leif Lindholm
2017-09-26 20:15 ` [PATCH 14/19] ArmPlatformPkg: Add PCD to select pixel format evan.lloyd
2017-10-25 14:27 ` Leif Lindholm
2017-09-26 20:15 ` [PATCH 15/19] ArmPlatformPkg: PCD to swap red/blue format for HDLCD evan.lloyd
2017-10-25 14:33 ` Leif Lindholm
2017-09-26 20:15 ` [PATCH 16/19] ArmPlatformPkg: Reorganize Lcd Graphics Output evan.lloyd
2017-10-25 14:44 ` Leif Lindholm
2017-09-26 20:15 ` [PATCH 17/19] ArmPlatformPkg: Additional display modes evan.lloyd
2017-10-25 14:45 ` Leif Lindholm
2017-09-26 20:15 ` [PATCH 18/19] ArmPlatformPkg: Reserving framebuffer at build evan.lloyd
2017-10-25 14:51 ` Leif Lindholm
2017-10-25 18:10 ` Ard Biesheuvel
2017-12-01 16:56 ` Evan Lloyd
2017-12-01 17:38 ` Ard Biesheuvel
2017-09-26 20:15 ` [PATCH 19/19] ArmPlatformPkg: New DP500/DP550/DP650 GOP driver evan.lloyd
2017-10-25 15:31 ` Leif Lindholm
2017-11-28 18:17 ` Ard Biesheuvel
2017-12-01 13:12 ` Evan Lloyd
2017-12-01 17:18 ` Ard Biesheuvel
2017-12-05 20:03 ` Evan Lloyd
2017-12-05 21:27 ` Ard Biesheuvel
2017-12-07 20:21 ` Evan Lloyd
2017-12-07 21:10 ` Ard Biesheuvel
2017-12-01 17:29 ` Leif Lindholm
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CAKv+Gu_Gzm=BT_Xt9R_0c+OqaaFvAxdHRawJywhooTEdnF-J9Q@mail.gmail.com' \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox