Re: [PATCH 3/4] ArmPkg/GenericWatchdogDxe: Set Watchdog interrupt type

[Leif Lindholm <leif.lindholm@linaro.org>]

On Thu, Feb 09, 2017 at 07:26:22PM +0000, evan.lloyd@arm.com wrote:
> From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> 
> Utilise the new HardwareInterrupt2 protocol to adjust the
> Edje/Level characteristics of the Watchdog interrupt.

Edge.

> 
> 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 |  4 +--
>  ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c   | 27 ++++++++++++--------
>  2 files changed, 19 insertions(+), 12 deletions(-)
> 
> diff --git a/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.inf b/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.inf
> index fece14cc18315cd15510680c438288687b60c018..51d5c0042d84333b9fe66547c99a8d8ed987f175 100644
> --- a/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.inf
> +++ b/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.inf
> @@ -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 54a1625a32137556b58fa93ddf7fbe4d0f22c786..9ba9bf79c961b52dc9b448039a9186e069fc29f7 100644
> --- a/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c
> +++ b/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c
> @@ -25,7 +25,8 @@
>  #include <Library/ArmGenericTimerCounterLib.h>
>  
>  #include <Protocol/WatchdogTimer.h>
> -#include <Protocol/HardwareInterrupt.h>
> +
> +#include <Protocol/HardwareInterrupt2.h>

If Protocol/HardwareInterrupt.h is dropped, can we move this before
Protocol/WatchdogTimer.h?

>  
>  #include "GenericWatchdog.h"
>  
> @@ -41,7 +42,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 +321,7 @@ GenericWatchdogEntry (
>    if (!EFI_ERROR (Status)) {
>      // Install interrupt handler
>      Status = gBS->LocateProtocol (
> -                    &gHardwareInterruptProtocolGuid,
> +                    &gHardwareInterrupt2ProtocolGuid,
>                      NULL,
>                      (VOID **)&mInterruptProtocol
>                      );
> @@ -331,13 +332,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,

I believe this is an incorrect level of indentation.

> +                                    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")
>