[edk2] [PATCH v2 04/13] ArmPlatformPkg: HDLCD and PL111: Update debug ASSERTS

evan.lloyd@arm.com posted 13 patches 7 years ago
[edk2] [PATCH v2 04/13] ArmPlatformPkg: HDLCD and PL111: Update debug ASSERTS
Posted by evan.lloyd@arm.com 7 years ago
From: Girish Pathak <girish.pathak at arm.com>

This change moves some ASSERTs in error handling code
to improve efficiency in DEBUG build.

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/Library/HdLcd/HdLcd.c       | 11 ++++-------
 ArmPlatformPkg/Library/PL111Lcd/PL111Lcd.c |  8 ++++----
 2 files changed, 8 insertions(+), 11 deletions(-)

diff --git a/ArmPlatformPkg/Library/HdLcd/HdLcd.c b/ArmPlatformPkg/Library/HdLcd/HdLcd.c
index a1eeabfefe7d32e6182371e5b131ac5df0dd4dd7..d71b6020dc0c4b91e74d16e96b06a60601b9628a 100644
--- a/ArmPlatformPkg/Library/HdLcd/HdLcd.c
+++ b/ArmPlatformPkg/Library/HdLcd/HdLcd.c
@@ -122,15 +122,15 @@ LcdSetMode (
              &VBackPorch,
              &VFrontPorch
              );
-  ASSERT_EFI_ERROR (Status);
   if (EFI_ERROR (Status)) {
-    return EFI_DEVICE_ERROR;
+    ASSERT (FALSE);
+    return Status;
   }
 
   Status = LcdPlatformGetBpp (ModeNumber, &LcdBpp);
-  ASSERT_EFI_ERROR (Status);
   if (EFI_ERROR (Status)) {
-    return EFI_DEVICE_ERROR;
+    ASSERT (FALSE);
+    return Status;
   }
 
   BytesPerPixel = GetBytesPerPixel (LcdBpp);
@@ -174,9 +174,6 @@ LcdShutdown (VOID)
 
   @retval EFI_SUCCESS            Returns success if platform implements a HDLCD
                                  controller.
-
-  @retval EFI_NOT_FOUND          HDLCD display controller not found on the
-                                 platform
 **/
 EFI_STATUS
 LcdIdentify (VOID)
diff --git a/ArmPlatformPkg/Library/PL111Lcd/PL111Lcd.c b/ArmPlatformPkg/Library/PL111Lcd/PL111Lcd.c
index 53b402f711ff10d70feba38671171c027a98b4ba..267c972bf795997f1df88b82acbaea5f75a7a00e 100644
--- a/ArmPlatformPkg/Library/PL111Lcd/PL111Lcd.c
+++ b/ArmPlatformPkg/Library/PL111Lcd/PL111Lcd.c
@@ -105,15 +105,15 @@ LcdSetMode (
              &VBackPorch,
              &VFrontPorch
              );
-  ASSERT_EFI_ERROR (Status);
   if (EFI_ERROR (Status)) {
-    return EFI_DEVICE_ERROR;
+    ASSERT (FALSE);
+    return Status;
   }
 
   Status = LcdPlatformGetBpp (ModeNumber, &LcdBpp);
-  ASSERT_EFI_ERROR (Status);
   if (EFI_ERROR (Status)) {
-    return EFI_DEVICE_ERROR;
+    ASSERT (FALSE);
+    return Status;
   }
 
   // Disable the CLCD_LcdEn bit
-- 
Guid("CE165669-3EF3-493F-B85D-6190EE5B9759")

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH v2 04/13] ArmPlatformPkg: HDLCD and PL111: Update debug ASSERTS
Posted by Ard Biesheuvel 7 years ago
On 22 December 2017 at 18:34,  <evan.lloyd@arm.com> wrote:
> From: Girish Pathak <girish.pathak at arm.com>
>
> This change moves some ASSERTs in error handling code
> to improve efficiency in DEBUG build.
>
> 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/Library/HdLcd/HdLcd.c       | 11 ++++-------
>  ArmPlatformPkg/Library/PL111Lcd/PL111Lcd.c |  8 ++++----
>  2 files changed, 8 insertions(+), 11 deletions(-)
>
> diff --git a/ArmPlatformPkg/Library/HdLcd/HdLcd.c b/ArmPlatformPkg/Library/HdLcd/HdLcd.c
> index a1eeabfefe7d32e6182371e5b131ac5df0dd4dd7..d71b6020dc0c4b91e74d16e96b06a60601b9628a 100644
> --- a/ArmPlatformPkg/Library/HdLcd/HdLcd.c
> +++ b/ArmPlatformPkg/Library/HdLcd/HdLcd.c
> @@ -122,15 +122,15 @@ LcdSetMode (
>               &VBackPorch,
>               &VFrontPorch
>               );
> -  ASSERT_EFI_ERROR (Status);
>    if (EFI_ERROR (Status)) {
> -    return EFI_DEVICE_ERROR;
> +    ASSERT (FALSE);

Please don't replace ASSERT_EFI_ERROR with ASSERT(FALSE). The former
will print the value of Status in the DEBUG output, while that latter
only prints '0 != 1' which is not very helpful in diagnosing the error
condition.

> +    return Status;
>    }
>
>    Status = LcdPlatformGetBpp (ModeNumber, &LcdBpp);
> -  ASSERT_EFI_ERROR (Status);
>    if (EFI_ERROR (Status)) {
> -    return EFI_DEVICE_ERROR;
> +    ASSERT (FALSE);
> +    return Status;
>    }
>
>    BytesPerPixel = GetBytesPerPixel (LcdBpp);
> @@ -174,9 +174,6 @@ LcdShutdown (VOID)
>
>    @retval EFI_SUCCESS            Returns success if platform implements a HDLCD
>                                   controller.
> -
> -  @retval EFI_NOT_FOUND          HDLCD display controller not found on the
> -                                 platform
>  **/
>  EFI_STATUS
>  LcdIdentify (VOID)
> diff --git a/ArmPlatformPkg/Library/PL111Lcd/PL111Lcd.c b/ArmPlatformPkg/Library/PL111Lcd/PL111Lcd.c
> index 53b402f711ff10d70feba38671171c027a98b4ba..267c972bf795997f1df88b82acbaea5f75a7a00e 100644
> --- a/ArmPlatformPkg/Library/PL111Lcd/PL111Lcd.c
> +++ b/ArmPlatformPkg/Library/PL111Lcd/PL111Lcd.c
> @@ -105,15 +105,15 @@ LcdSetMode (
>               &VBackPorch,
>               &VFrontPorch
>               );
> -  ASSERT_EFI_ERROR (Status);
>    if (EFI_ERROR (Status)) {
> -    return EFI_DEVICE_ERROR;
> +    ASSERT (FALSE);
> +    return Status;
>    }
>
>    Status = LcdPlatformGetBpp (ModeNumber, &LcdBpp);
> -  ASSERT_EFI_ERROR (Status);
>    if (EFI_ERROR (Status)) {
> -    return EFI_DEVICE_ERROR;
> +    ASSERT (FALSE);
> +    return Status;
>    }
>
>    // Disable the CLCD_LcdEn bit
> --
> Guid("CE165669-3EF3-493F-B85D-6190EE5B9759")
>
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel