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
)
{
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
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
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")
>
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Hi Ard.
Response inline below
> -----Original Message-----
> From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
> Sent: 12 October 2017 20:47
> 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 03/19] ArmPlatformPkg: PL111 and HDLCD: add const
> qualifier
>
> 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/HdLcdAr
> mVExpress.c | 34 ++++++++++----------
> >
> ArmPlatformPkg/ArmVExpressPkg/Library/PL111LcdArmVExpressLib/PL11
> 1LcdArmVExpress.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/HdLcd
> ArmVE
> > xpress.c
> >
> b/ArmPlatformPkg/ArmVExpressPkg/Library/HdLcdArmVExpressLib/HdLcd
> ArmVE
> > xpress.c index
> >
> cfe3259d3c737de240350e8c3eab867b80c40948..b9859a56988f7e5be0ad
> baa49048
> > a683fe586bfe 100644
> > ---
> >
> a/ArmPlatformPkg/ArmVExpressPkg/Library/HdLcdArmVExpressLib/HdLcd
> ArmVE
> > xpress.c
> > +++
> b/ArmPlatformPkg/ArmVExpressPkg/Library/HdLcdArmVExpressLib/HdLcd
> A
> > +++ rmVExpress.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.
>
[[Evan Lloyd]] The style is explicitly sanctioned by the Edk2 CCS § 5.6.2.4.2
" Constant pointer to variable: UINTN * CONST ConstPointer;
ConstPointer is a constant pointer to a variable UINTN."
The real benefit is that it clearly identifies the pointer as not changed in the function.
In this specific instance that also makes it obvious that the OUT parameters are not array bases, just pointers to individual values.
On a broader note - why would you ever not have a const where something is not modified?
As another view, the "unidiomatic for EDK2" argument implies you have a very high opinion of the existing ArmPlatformPkg code quality.
The "we have always done it that way" argument does not encourage quality improvements.
Regards,
Evan
> > )
...
> > --
> > Guid("CE165669-3EF3-493F-B85D-6190EE5B9759")
> >
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
On 1 December 2017 at 16:17, Evan Lloyd <Evan.Lloyd@arm.com> wrote:
> Hi Ard.
> Response inline below
>
>> -----Original Message-----
>> From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
>> Sent: 12 October 2017 20:47
>> 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 03/19] ArmPlatformPkg: PL111 and HDLCD: add const
>> qualifier
>>
>> 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/HdLcdAr
>> mVExpress.c | 34 ++++++++++----------
>> >
>> ArmPlatformPkg/ArmVExpressPkg/Library/PL111LcdArmVExpressLib/PL11
>> 1LcdArmVExpress.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/HdLcd
>> ArmVE
>> > xpress.c
>> >
>> b/ArmPlatformPkg/ArmVExpressPkg/Library/HdLcdArmVExpressLib/HdLcd
>> ArmVE
>> > xpress.c index
>> >
>> cfe3259d3c737de240350e8c3eab867b80c40948..b9859a56988f7e5be0ad
>> baa49048
>> > a683fe586bfe 100644
>> > ---
>> >
>> a/ArmPlatformPkg/ArmVExpressPkg/Library/HdLcdArmVExpressLib/HdLcd
>> ArmVE
>> > xpress.c
>> > +++
>> b/ArmPlatformPkg/ArmVExpressPkg/Library/HdLcdArmVExpressLib/HdLcd
>> A
>> > +++ rmVExpress.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.
>>
> [[Evan Lloyd]] The style is explicitly sanctioned by the Edk2 CCS § 5.6.2.4.2
> " Constant pointer to variable: UINTN * CONST ConstPointer;
> ConstPointer is a constant pointer to a variable UINTN."
>
That paragraph is not about function prototypes, but about constant
pointers in general.
> The real benefit is that it clearly identifies the pointer as not changed in the function.
> In this specific instance that also makes it obvious that the OUT parameters are not array bases, just pointers to individual values.
>
> On a broader note - why would you ever not have a const where something is not modified?
>
> As another view, the "unidiomatic for EDK2" argument implies you have a very high opinion of the existing ArmPlatformPkg code quality.
> The "we have always done it that way" argument does not encourage quality improvements.
>
This may all be true. But the fact remains that 99% of the EDK2 code
does not constify its function parameters, and I was simply asking why
we should deviate from that in this driver.
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
> -----Original Message-----
> From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
> Sent: 01 December 2017 17:32
> To: Evan Lloyd <Evan.Lloyd@arm.com>
> Cc: edk2-devel@lists.01.org; ard.biesheuvel@linaro.org@arm.com
> <"ard.biesheuvel@linaro.org"@arm.com>;
> leif.lindholm@linaro.org@arm.com <"leif.lindholm@linaro.org"@arm.com>;
> Matteo.Carlini@arm.com@arm.com
> <"Matteo.Carlini@arm.com"@arm.com>; nd@arm.com@arm.com
> <"nd@arm.com"@arm.com>
> Subject: Re: [PATCH 03/19] ArmPlatformPkg: PL111 and HDLCD: add const
> qualifier
>
> On 1 December 2017 at 16:17, Evan Lloyd <Evan.Lloyd@arm.com> wrote:
> > Hi Ard.
> > Response inline below
> >
> >> -----Original Message-----
> >> From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
> >> Sent: 12 October 2017 20:47
> >> 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 03/19] ArmPlatformPkg: PL111 and HDLCD: add
> const
> >> qualifier
> >>
> >> 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/HdLcdAr
> >> mVExpress.c | 34 ++++++++++----------
> >> >
> >>
> ArmPlatformPkg/ArmVExpressPkg/Library/PL111LcdArmVExpressLib/PL11
> >> 1LcdArmVExpress.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/HdLcd
> >> ArmVE
> >> > xpress.c
> >> >
> >>
> b/ArmPlatformPkg/ArmVExpressPkg/Library/HdLcdArmVExpressLib/HdLcd
> >> ArmVE
> >> > xpress.c index
> >> >
> >>
> cfe3259d3c737de240350e8c3eab867b80c40948..b9859a56988f7e5be0ad
> >> baa49048
> >> > a683fe586bfe 100644
> >> > ---
> >> >
> >>
> a/ArmPlatformPkg/ArmVExpressPkg/Library/HdLcdArmVExpressLib/HdLcd
> >> ArmVE
> >> > xpress.c
> >> > +++
> >>
> b/ArmPlatformPkg/ArmVExpressPkg/Library/HdLcdArmVExpressLib/HdLcd
> >> A
> >> > +++ rmVExpress.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.
> >>
> > [[Evan Lloyd]] The style is explicitly sanctioned by the Edk2 CCS §
> > 5.6.2.4.2 " Constant pointer to variable: UINTN * CONST ConstPointer;
> > ConstPointer is a constant pointer to a variable UINTN."
> >
>
> That paragraph is not about function prototypes, but about constant
> pointers in general.
>
> > The real benefit is that it clearly identifies the pointer as not changed in
> the function.
> > In this specific instance that also makes it obvious that the OUT
> parameters are not array bases, just pointers to individual values.
> >
> > On a broader note - why would you ever not have a const where
> something is not modified?
> >
> > As another view, the "unidiomatic for EDK2" argument implies you have a
> very high opinion of the existing ArmPlatformPkg code quality.
> > The "we have always done it that way" argument does not encourage
> quality improvements.
> >
>
> This may all be true. But the fact remains that 99% of the EDK2 code does
> not constify its function parameters, and I was simply asking why we should
> deviate from that in this driver.
[[Evan Lloyd]] And the simple answer is that it is good practice, providing cleaner, clearer, safer code. As support for that viewpoint: the CCS references MISRA-C (strangely without making use of the reference, but ...), and MISRA-C has, for example:
"Rule 8.13: A pointer should point to a const qualified type where possible"
(NOTE: I don't think it realistic to move edk2 to MISRA-C rules, but they do provide a useful guide to safer practice.)
A further point is that it certainly does no harm, and there is negligible benefit and some cost to reducing the quality of tested code.
I am also impelled to ask: if 99% of the code is so superb, why are there so many commits from Ard Biesheuvel? (Please note attempt at ironic humour here.)
Regards,
Evan
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
On 5 December 2017 at 20:35, Evan Lloyd <Evan.Lloyd@arm.com> wrote:
>
>
>> -----Original Message-----
>> From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
>> Sent: 01 December 2017 17:32
>> To: Evan Lloyd <Evan.Lloyd@arm.com>
>> Cc: edk2-devel@lists.01.org; ard.biesheuvel@linaro.org@arm.com
>> <"ard.biesheuvel@linaro.org"@arm.com>;
>> leif.lindholm@linaro.org@arm.com <"leif.lindholm@linaro.org"@arm.com>;
>> Matteo.Carlini@arm.com@arm.com
>> <"Matteo.Carlini@arm.com"@arm.com>; nd@arm.com@arm.com
>> <"nd@arm.com"@arm.com>
>> Subject: Re: [PATCH 03/19] ArmPlatformPkg: PL111 and HDLCD: add const
>> qualifier
>>
>> On 1 December 2017 at 16:17, Evan Lloyd <Evan.Lloyd@arm.com> wrote:
>> > Hi Ard.
>> > Response inline below
>> >
>> >> -----Original Message-----
>> >> From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
>> >> Sent: 12 October 2017 20:47
>> >> 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 03/19] ArmPlatformPkg: PL111 and HDLCD: add
>> const
>> >> qualifier
>> >>
>> >> 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/HdLcdAr
>> >> mVExpress.c | 34 ++++++++++----------
>> >> >
>> >>
>> ArmPlatformPkg/ArmVExpressPkg/Library/PL111LcdArmVExpressLib/PL11
>> >> 1LcdArmVExpress.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/HdLcd
>> >> ArmVE
>> >> > xpress.c
>> >> >
>> >>
>> b/ArmPlatformPkg/ArmVExpressPkg/Library/HdLcdArmVExpressLib/HdLcd
>> >> ArmVE
>> >> > xpress.c index
>> >> >
>> >>
>> cfe3259d3c737de240350e8c3eab867b80c40948..b9859a56988f7e5be0ad
>> >> baa49048
>> >> > a683fe586bfe 100644
>> >> > ---
>> >> >
>> >>
>> a/ArmPlatformPkg/ArmVExpressPkg/Library/HdLcdArmVExpressLib/HdLcd
>> >> ArmVE
>> >> > xpress.c
>> >> > +++
>> >>
>> b/ArmPlatformPkg/ArmVExpressPkg/Library/HdLcdArmVExpressLib/HdLcd
>> >> A
>> >> > +++ rmVExpress.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.
>> >>
>> > [[Evan Lloyd]] The style is explicitly sanctioned by the Edk2 CCS §
>> > 5.6.2.4.2 " Constant pointer to variable: UINTN * CONST ConstPointer;
>> > ConstPointer is a constant pointer to a variable UINTN."
>> >
>>
>> That paragraph is not about function prototypes, but about constant
>> pointers in general.
>>
>> > The real benefit is that it clearly identifies the pointer as not changed in
>> the function.
>> > In this specific instance that also makes it obvious that the OUT
>> parameters are not array bases, just pointers to individual values.
>> >
>> > On a broader note - why would you ever not have a const where
>> something is not modified?
>> >
>> > As another view, the "unidiomatic for EDK2" argument implies you have a
>> very high opinion of the existing ArmPlatformPkg code quality.
>> > The "we have always done it that way" argument does not encourage
>> quality improvements.
>> >
>>
>> This may all be true. But the fact remains that 99% of the EDK2 code does
>> not constify its function parameters, and I was simply asking why we should
>> deviate from that in this driver.
> [[Evan Lloyd]] And the simple answer is that it is good practice, providing cleaner, clearer, safer code. As support for that viewpoint: the CCS references MISRA-C (strangely without making use of the reference, but ...), and MISRA-C has, for example:
> "Rule 8.13: A pointer should point to a const qualified type where possible"
>
Should *point to* which means
UINTN CONST *Var
not
UINTN * CONST Var
as you are proposing, so this reference is completely irrelevant.
> (NOTE: I don't think it realistic to move edk2 to MISRA-C rules, but they do provide a useful guide to safer practice.)
>
Well, for the firmware in my ABS brake system, yes, but for higher
level stuff it is overly restrictive imo.
> A further point is that it certainly does no harm, and there is negligible benefit and some cost to reducing the quality of tested code.
>
I have read that sentence a couple of times, and I don't understand
what you are trying to say here - sorry. Could you rephrase that?
> I am also impelled to ask: if 99% of the code is so superb, why are there so many commits from Ard Biesheuvel? (Please note attempt at ironic humour here.)
>
No amount of drugs or alcohol could ever elicit such a statement from
me. I was merely pointing out that constifying function arguments is
very uncommon, in any project I am involved in. (EDK2, Linux, Qemu)
I agree that it may help catch programming errors, but beyond that, I
think the value is limited. I won't object to new code that does it,
but changing existing code is just churn imo.
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
On Tue, Sep 26, 2017 at 09:15:13PM +0100, 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.
If you delete the line above:
Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
(These changes are highly likely to affect code generation, so I still
consider it a functional change - if not an algorithmic one.)
/
Leif
> 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
> )
> {
> 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
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
© 2016 - 2024 Red Hat, Inc.