[edk2,4/5] ArmPkg/GenericWatchdogDxe: Set Watchdog interrupt type

Message ID 20170911152335.72672-5-evan.lloyd@arm.com
State Superseded
Headers show
Series
  • Untitled series #3948
Related show

Commit Message

Evan Lloyd Sept. 11, 2017, 3:23 p.m.
From: Ard Biesheuvel <ard.biesheuvel@linaro.org>


Utilise the new HardwareInterrupt2 protocol to adjust the
Edge/Level characteristics of the Watchdog interrupt.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

Signed-off-by: Girish Pathak <girish.pathak@arm.com>

Signed-off-by: Evan Lloyd <evan.lloyd@arm.com>

Tested-by: Girish Pathak <girish.pathak@arm.com>

---
 ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.inf |  6 ++---
 ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c   | 28 ++++++++++++--------
 2 files changed, 20 insertions(+), 14 deletions(-)

-- 
Guid("CE165669-3EF3-493F-B85D-6190EE5B9759")

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Comments

Leif Lindholm Sept. 14, 2017, 5:06 p.m. | #1
On Mon, Sep 11, 2017 at 04:23:34PM +0100, evan.lloyd@arm.com wrote:
> From: Ard Biesheuvel <ard.biesheuvel@linaro.org>

> 

> Utilise the new HardwareInterrupt2 protocol to adjust the

> Edge/Level characteristics of the Watchdog interrupt.

> 

> Contributed-under: TianoCore Contribution Agreement 1.0

> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

> Signed-off-by: Girish Pathak <girish.pathak@arm.com>

> Signed-off-by: Evan Lloyd <evan.lloyd@arm.com>

> Tested-by: Girish Pathak <girish.pathak@arm.com>


Looks plausible, but I would loke to see it tested by someone not
directly involved in development.
Armada70x0 also uses this, so adding Marcin on cc.

Beyond that, I'm told in Ryan's absence Arvind, Daniil and Thomas are
"platform owners" for Juno, so if one of you could have a spin, that'd
be great.

Pending another Tested-by:
Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>


/
    Leif

> ---

>  ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.inf |  6 ++---

>  ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c   | 28 ++++++++++++--------

>  2 files changed, 20 insertions(+), 14 deletions(-)

> 

> diff --git a/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.inf b/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.inf

> index fece14cc18315cd15510680c438288687b60c018..ba0403d7fdc3589803c643c27a44918e73afa97e 100644

> --- a/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.inf

> +++ b/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.inf

> @@ -1,5 +1,5 @@

>  #

> -#  Copyright (c) 2013-2014, ARM Limited. All rights reserved.

> +#  Copyright (c) 2013-2017, ARM Limited. All rights reserved.

>  #

>  #  This program and the accompanying materials

>  #  are licensed and made available under the terms and conditions of the BSD License

> @@ -47,7 +47,7 @@ [Pcd.common]

>  

>  [Protocols]

>    gEfiWatchdogTimerArchProtocolGuid

> -  gHardwareInterruptProtocolGuid

> +  gHardwareInterrupt2ProtocolGuid

>  

>  [Depex]

> -  gHardwareInterruptProtocolGuid

> +  gHardwareInterrupt2ProtocolGuid

> diff --git a/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c b/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c

> index 6d102e25047253048ac555d6fb5de7223d78f381..69844db2e11f51907e6c8bff5c67d27ceb498150 100644

> --- a/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c

> +++ b/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c

> @@ -1,6 +1,6 @@

>  /** @file

>  *

> -*  Copyright (c) 2013-2014, ARM Limited. All rights reserved.

> +*  Copyright (c) 2013-2017, ARM Limited. All rights reserved.

>  *

>  *  This program and the accompanying materials

>  *  are licensed and made available under the terms and conditions of the BSD

> @@ -24,8 +24,8 @@

>  #include <Library/UefiLib.h>

>  #include <Library/ArmGenericTimerCounterLib.h>

>  

> +#include <Protocol/HardwareInterrupt2.h>

>  #include <Protocol/WatchdogTimer.h>

> -#include <Protocol/HardwareInterrupt.h>

>  

>  #include "GenericWatchdog.h"

>  

> @@ -41,7 +41,7 @@ UINTN mTimerFrequencyHz = 0;

>  // It is therefore stored here. 0 means the timer is not running.

>  UINT64 mNumTimerTicks = 0;

>  

> -EFI_HARDWARE_INTERRUPT_PROTOCOL *mInterruptProtocol;

> +EFI_HARDWARE_INTERRUPT2_PROTOCOL *mInterruptProtocol;

>  

>  EFI_STATUS

>  WatchdogWriteOffsetRegister (

> @@ -320,7 +320,7 @@ GenericWatchdogEntry (

>    if (!EFI_ERROR (Status)) {

>      // Install interrupt handler

>      Status = gBS->LocateProtocol (

> -                    &gHardwareInterruptProtocolGuid,

> +                    &gHardwareInterrupt2ProtocolGuid,

>                      NULL,

>                      (VOID **)&mInterruptProtocol

>                      );

> @@ -331,13 +331,19 @@ GenericWatchdogEntry (

>                                      WatchdogInterruptHandler

>                                      );

>        if (!EFI_ERROR (Status)) {

> -        // Install the Timer Architectural Protocol onto a new handle

> -        Handle = NULL;

> -        Status = gBS->InstallMultipleProtocolInterfaces (

> -                        &Handle,

> -                        &gEfiWatchdogTimerArchProtocolGuid, &gWatchdogTimer,

> -                        NULL

> -                        );

> +        Status = mInterruptProtocol->SetTriggerType (

> +                                    mInterruptProtocol,

> +                                    FixedPcdGet32 (PcdGenericWatchdogEl2IntrNum),

> +                                    EFI_HARDWARE_INTERRUPT2_TRIGGER_EDGE_RISING);

> +        if (!EFI_ERROR (Status)) {

> +          // Install the Timer Architectural Protocol onto a new handle

> +          Handle = NULL;

> +          Status = gBS->InstallMultipleProtocolInterfaces (

> +                          &Handle,

> +                          &gEfiWatchdogTimerArchProtocolGuid, &gWatchdogTimer,

> +                          NULL

> +                          );

> +        }

>        }

>      }

>    }

> -- 

> Guid("CE165669-3EF3-493F-B85D-6190EE5B9759")

> 

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Patch

diff --git a/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.inf b/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.inf
index fece14cc18315cd15510680c438288687b60c018..ba0403d7fdc3589803c643c27a44918e73afa97e 100644
--- a/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.inf
+++ b/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.inf
@@ -1,5 +1,5 @@ 
 #
-#  Copyright (c) 2013-2014, ARM Limited. All rights reserved.
+#  Copyright (c) 2013-2017, ARM Limited. All rights reserved.
 #
 #  This program and the accompanying materials
 #  are licensed and made available under the terms and conditions of the BSD License
@@ -47,7 +47,7 @@  [Pcd.common]
 
 [Protocols]
   gEfiWatchdogTimerArchProtocolGuid
-  gHardwareInterruptProtocolGuid
+  gHardwareInterrupt2ProtocolGuid
 
 [Depex]
-  gHardwareInterruptProtocolGuid
+  gHardwareInterrupt2ProtocolGuid
diff --git a/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c b/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c
index 6d102e25047253048ac555d6fb5de7223d78f381..69844db2e11f51907e6c8bff5c67d27ceb498150 100644
--- a/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c
+++ b/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c
@@ -1,6 +1,6 @@ 
 /** @file
 *
-*  Copyright (c) 2013-2014, ARM Limited. All rights reserved.
+*  Copyright (c) 2013-2017, ARM Limited. All rights reserved.
 *
 *  This program and the accompanying materials
 *  are licensed and made available under the terms and conditions of the BSD
@@ -24,8 +24,8 @@ 
 #include <Library/UefiLib.h>
 #include <Library/ArmGenericTimerCounterLib.h>
 
+#include <Protocol/HardwareInterrupt2.h>
 #include <Protocol/WatchdogTimer.h>
-#include <Protocol/HardwareInterrupt.h>
 
 #include "GenericWatchdog.h"
 
@@ -41,7 +41,7 @@  UINTN mTimerFrequencyHz = 0;
 // It is therefore stored here. 0 means the timer is not running.
 UINT64 mNumTimerTicks = 0;
 
-EFI_HARDWARE_INTERRUPT_PROTOCOL *mInterruptProtocol;
+EFI_HARDWARE_INTERRUPT2_PROTOCOL *mInterruptProtocol;
 
 EFI_STATUS
 WatchdogWriteOffsetRegister (
@@ -320,7 +320,7 @@  GenericWatchdogEntry (
   if (!EFI_ERROR (Status)) {
     // Install interrupt handler
     Status = gBS->LocateProtocol (
-                    &gHardwareInterruptProtocolGuid,
+                    &gHardwareInterrupt2ProtocolGuid,
                     NULL,
                     (VOID **)&mInterruptProtocol
                     );
@@ -331,13 +331,19 @@  GenericWatchdogEntry (
                                     WatchdogInterruptHandler
                                     );
       if (!EFI_ERROR (Status)) {
-        // Install the Timer Architectural Protocol onto a new handle
-        Handle = NULL;
-        Status = gBS->InstallMultipleProtocolInterfaces (
-                        &Handle,
-                        &gEfiWatchdogTimerArchProtocolGuid, &gWatchdogTimer,
-                        NULL
-                        );
+        Status = mInterruptProtocol->SetTriggerType (
+                                    mInterruptProtocol,
+                                    FixedPcdGet32 (PcdGenericWatchdogEl2IntrNum),
+                                    EFI_HARDWARE_INTERRUPT2_TRIGGER_EDGE_RISING);
+        if (!EFI_ERROR (Status)) {
+          // Install the Timer Architectural Protocol onto a new handle
+          Handle = NULL;
+          Status = gBS->InstallMultipleProtocolInterfaces (
+                          &Handle,
+                          &gEfiWatchdogTimerArchProtocolGuid, &gWatchdogTimer,
+                          NULL
+                          );
+        }
       }
     }
   }