subject:"\[edk2\] \[PATCH 2\/4\] ArmPlatformPkg\/SP805WatchdogDxe\: switch to interrupt mode"

Re: [edk2] [PATCH 2/4] ArmPlatformPkg/SP805WatchdogDxe: switch to interrupt mode

2018-12-19 Thread Thomas Abraham

Hi Ard,

On Tue, Dec 18, 2018 at 9:59 PM Ard Biesheuvel
 wrote:
>
> On Tue, 18 Dec 2018 at 14:39, Leif Lindholm  wrote:
> >
> > On Tue, Dec 18, 2018 at 02:10:12PM +0100, Ard Biesheuvel wrote:
> > > The SP805 watchdog driver doesn't implement the PI watchdog protocol
> > > fully, but always simply resets the system if the watchdog time runs
> > > out.
> > >
> > > However, the hardware does support the intended usage model, as long
> > > as the SP805 is wired up correctly. So let's implement interrupt based
> > > mode involving a handler that is registered by the DXE core and invoked
> > > when the watchdog runs out. In the interrupt handler, we invoke the
> > > notify function if one was registered, or call the ResetSystem()
> > > runtime service otherwise (as per the UEFI spec)
> >
> > The only question mark from my end is - what happens when the
> > interrupt isn't wired up correctly? Would it be worth to bail out and
> > refuse to register the driver if PcdSP805WatchdogInterrupt is set to
> > 0?
> >
> > Thomas?
> >
>
> I have left the code in place that enables the hard reset, but the
> timeout is double the programmed value (since the countdown timer is
> restarted on an interrupt, and the hard reset is generated when it
> reaches zero the second time)
>
> This should cover both the miswired interrupt scenario, and the
> scenario where ResetSystem() (or the handler) gets stuck and never
> returns.

Yes, this would suffice. But the system would reset after twice the
amount of time than programmed, which probably is okay.

-Thomas.

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

Re: [edk2] [PATCH 2/4] ArmPlatformPkg/SP805WatchdogDxe: switch to interrupt mode

2018-12-18 Thread Ard Biesheuvel

On Tue, 18 Dec 2018 at 14:39, Leif Lindholm  wrote:
>
> On Tue, Dec 18, 2018 at 02:10:12PM +0100, Ard Biesheuvel wrote:
> > The SP805 watchdog driver doesn't implement the PI watchdog protocol
> > fully, but always simply resets the system if the watchdog time runs
> > out.
> >
> > However, the hardware does support the intended usage model, as long
> > as the SP805 is wired up correctly. So let's implement interrupt based
> > mode involving a handler that is registered by the DXE core and invoked
> > when the watchdog runs out. In the interrupt handler, we invoke the
> > notify function if one was registered, or call the ResetSystem()
> > runtime service otherwise (as per the UEFI spec)
>
> The only question mark from my end is - what happens when the
> interrupt isn't wired up correctly? Would it be worth to bail out and
> refuse to register the driver if PcdSP805WatchdogInterrupt is set to
> 0?
>
> Thomas?
>

I have left the code in place that enables the hard reset, but the
timeout is double the programmed value (since the countdown timer is
restarted on an interrupt, and the hard reset is generated when it
reaches zero the second time)

This should cover both the miswired interrupt scenario, and the
scenario where ResetSystem() (or the handler) gets stuck and never
returns.
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH 2/4] ArmPlatformPkg/SP805WatchdogDxe: switch to interrupt mode

2018-12-18 Thread Ard Biesheuvel

On Tue, 18 Dec 2018 at 16:58, Udit Kumar  wrote:
>
>
>
> > -Original Message-
> > From: Ard Biesheuvel 
> > Sent: Tuesday, December 18, 2018 6:40 PM
> > To: edk2-devel@lists.01.org
> > Cc: Ard Biesheuvel ; Leif Lindholm
> > ; Sami Mujawar ; Thomas
> > Panakamattam Abraham ; Meenakshi Aggarwal
> > ; Udit Kumar ; Matteo
> > Carlini ; Nariman Poushin
> > 
> > Subject: [PATCH 2/4] ArmPlatformPkg/SP805WatchdogDxe: switch to interrupt
> > mode
> >
> > The SP805 watchdog driver doesn't implement the PI watchdog protocol fully,
> > but always simply resets the system if the watchdog time runs out.
> >
> > However, the hardware does support the intended usage model, as long as the
> > SP805 is wired up correctly. So let's implement interrupt based mode 
> > involving a
> > handler that is registered by the DXE core and invoked when the watchdog 
> > runs
> > out. In the interrupt handler, we invoke the notify function if one was 
> > registered,
> > or call the ResetSystem() runtime service otherwise (as per the UEFI spec)
>
> Specs, says
> If no handler has been registered, or the registered handler returns, then 
> the system will be reset by calling the Runtime Service ResetSystem().
> My understanding is that we have to ResetSystem() always irrespective of 
> notify function
>

Indeed. Thanks for spotting that.

>
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Ard Biesheuvel 
> > ---
> >  ArmPlatformPkg/ArmPlatformPkg.dec|  1 +
> >  ArmPlatformPkg/Drivers/SP805WatchdogDxe/SP805Watchdog.c  | 95
> > ++--
> >  ArmPlatformPkg/Drivers/SP805WatchdogDxe/SP805WatchdogDxe.inf |  6 +-
> >  3 files changed, 75 insertions(+), 27 deletions(-)
> >
> > diff --git a/ArmPlatformPkg/ArmPlatformPkg.dec
> > b/ArmPlatformPkg/ArmPlatformPkg.dec
> > index 5f67e7415469..44c00bd0c133 100644
> > --- a/ArmPlatformPkg/ArmPlatformPkg.dec
> > +++ b/ArmPlatformPkg/ArmPlatformPkg.dec
> > @@ -70,6 +70,7 @@
> >## SP805 Watchdog
> >
> > gArmPlatformTokenSpaceGuid.PcdSP805WatchdogBase|0x0|UINT32|0x00
> > 23
> >
> > gArmPlatformTokenSpaceGuid.PcdSP805WatchdogClockFrequencyInHz|32000|
> > UINT32|0x0021
> > +
> > +
> > gArmPlatformTokenSpaceGuid.PcdSP805WatchdogInterrupt|0|UINT32|0x
> > 00
> > + 2E
> >
> >## PL011 UART
> >
> > gArmPlatformTokenSpaceGuid.PL011UartClkInHz|2400|UINT32|0x00
> > 1F
> > diff --git a/ArmPlatformPkg/Drivers/SP805WatchdogDxe/SP805Watchdog.c
> > b/ArmPlatformPkg/Drivers/SP805WatchdogDxe/SP805Watchdog.c
> > index 12c2f0a1fe49..4f09acf1fa28 100644
> > --- a/ArmPlatformPkg/Drivers/SP805WatchdogDxe/SP805Watchdog.c
> > +++ b/ArmPlatformPkg/Drivers/SP805WatchdogDxe/SP805Watchdog.c
> > @@ -21,12 +21,17 @@
> >  #include 
> >  #include 
> >  #include 
> > +#include 
> >
> > +#include 
> >  #include 
> >
> >  #include "SP805Watchdog.h"
> >
> > -STATIC EFI_EVENT  mEfiExitBootServicesEvent;
> > +STATIC EFI_EVENTmEfiExitBootServicesEvent;
> > +STATIC EFI_HARDWARE_INTERRUPT_PROTOCOL  *mInterrupt;
> > +STATIC EFI_WATCHDOG_TIMER_NOTIFYmWatchdogNotify;
> > +STATIC UINT32   mTimerPeriod;
> >
> >  /**
> >Make sure the SP805 registers are unlocked for writing.
> > @@ -65,6 +70,33 @@ SP805Lock (
> >}
> >  }
> >
> > +STATIC
> > +VOID
> > +EFIAPI
> > +SP805InterruptHandler (
> > +  IN  HARDWARE_INTERRUPT_SOURCE   Source,
> > +  IN  EFI_SYSTEM_CONTEXT  SystemContext
> > +  )
> > +{
> > +  //
> > +  // The notify function should be called with the elapsed number of
> > +ticks
> > +  // since the watchdog was armed, which should exceed the timer period.
> > +  // We don't actually know the elapsed number of ticks, so let's
> > +return
> > +  // the timer period plus 1.
> > +  //
> > +  if (mWatchdogNotify != NULL) {
> > +mWatchdogNotify (mTimerPeriod + 1);
> > +  } else {
> > +gRT->ResetSystem (EfiResetCold, EFI_TIMEOUT, 0, NULL);
> > +  }
>
> Shouldn't we ResetSystem in all cases.
>
>
> > +
> > +  SP805Unlock ();
> > +  MmioWrite32 (SP805_WDOG_INT_CLR_REG, 0); // write of any value clears
> > + the irq  SP805Lock ();
> > +
> > +  mInterrupt->EndOfInterrupt (mInterrupt, Source); }
> > +
>
> IMO, this routine should be
> 1/ Handle IT
> 2/ Do callback mWatchdogNotify
> 3/ Reset the system.
>
> >  /**
> >Stop the SP805 watchdog timer from counting down by disabling interrupts.
> >  **/
> > @@ -149,9 +181,16 @@ SP805RegisterHandler (
> >IN EFI_WATCHDOG_TIMER_NOTIFYNotifyFunction
> >)
> >  {
> > -  // ERROR: This function is not supported.
> > -  // The hardware watchdog will reset the board
> > -  return EFI_INVALID_PARAMETER;
> > +  if (mWatchdogNotify == NULL && NotifyFunction == NULL) {
> > +return EFI_INVALID_PARAMETER;
> > +  }
> > +
> > +  if (mWatchdogNotify != NULL && NotifyFunction != NULL) {
> > +return EFI_ALREADY_STARTED;
> > +  }
> > +
> > +  mWatchdogNotify = NotifyFunction;
> > +  return EFI_SUCCESS;
> >

Re: [edk2] [PATCH 2/4] ArmPlatformPkg/SP805WatchdogDxe: switch to interrupt mode

2018-12-18 Thread Udit Kumar




> -Original Message-
> From: Ard Biesheuvel 
> Sent: Tuesday, December 18, 2018 6:40 PM
> To: edk2-devel@lists.01.org
> Cc: Ard Biesheuvel ; Leif Lindholm
> ; Sami Mujawar ; Thomas
> Panakamattam Abraham ; Meenakshi Aggarwal
> ; Udit Kumar ; Matteo
> Carlini ; Nariman Poushin
> 
> Subject: [PATCH 2/4] ArmPlatformPkg/SP805WatchdogDxe: switch to interrupt
> mode
> 
> The SP805 watchdog driver doesn't implement the PI watchdog protocol fully,
> but always simply resets the system if the watchdog time runs out.
> 
> However, the hardware does support the intended usage model, as long as the
> SP805 is wired up correctly. So let's implement interrupt based mode 
> involving a
> handler that is registered by the DXE core and invoked when the watchdog runs
> out. In the interrupt handler, we invoke the notify function if one was 
> registered,
> or call the ResetSystem() runtime service otherwise (as per the UEFI spec)

Specs, says 
If no handler has been registered, or the registered handler returns, then the 
system will be reset by calling the Runtime Service ResetSystem().
My understanding is that we have to ResetSystem() always irrespective of notify 
function

 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel 
> ---
>  ArmPlatformPkg/ArmPlatformPkg.dec|  1 +
>  ArmPlatformPkg/Drivers/SP805WatchdogDxe/SP805Watchdog.c  | 95
> ++--
>  ArmPlatformPkg/Drivers/SP805WatchdogDxe/SP805WatchdogDxe.inf |  6 +-
>  3 files changed, 75 insertions(+), 27 deletions(-)
> 
> diff --git a/ArmPlatformPkg/ArmPlatformPkg.dec
> b/ArmPlatformPkg/ArmPlatformPkg.dec
> index 5f67e7415469..44c00bd0c133 100644
> --- a/ArmPlatformPkg/ArmPlatformPkg.dec
> +++ b/ArmPlatformPkg/ArmPlatformPkg.dec
> @@ -70,6 +70,7 @@
>## SP805 Watchdog
> 
> gArmPlatformTokenSpaceGuid.PcdSP805WatchdogBase|0x0|UINT32|0x00
> 23
> 
> gArmPlatformTokenSpaceGuid.PcdSP805WatchdogClockFrequencyInHz|32000|
> UINT32|0x0021
> +
> +
> gArmPlatformTokenSpaceGuid.PcdSP805WatchdogInterrupt|0|UINT32|0x
> 00
> + 2E
> 
>## PL011 UART
> 
> gArmPlatformTokenSpaceGuid.PL011UartClkInHz|2400|UINT32|0x00
> 1F
> diff --git a/ArmPlatformPkg/Drivers/SP805WatchdogDxe/SP805Watchdog.c
> b/ArmPlatformPkg/Drivers/SP805WatchdogDxe/SP805Watchdog.c
> index 12c2f0a1fe49..4f09acf1fa28 100644
> --- a/ArmPlatformPkg/Drivers/SP805WatchdogDxe/SP805Watchdog.c
> +++ b/ArmPlatformPkg/Drivers/SP805WatchdogDxe/SP805Watchdog.c
> @@ -21,12 +21,17 @@
>  #include 
>  #include 
>  #include 
> +#include 
> 
> +#include 
>  #include 
> 
>  #include "SP805Watchdog.h"
> 
> -STATIC EFI_EVENT  mEfiExitBootServicesEvent;
> +STATIC EFI_EVENTmEfiExitBootServicesEvent;
> +STATIC EFI_HARDWARE_INTERRUPT_PROTOCOL  *mInterrupt;
> +STATIC EFI_WATCHDOG_TIMER_NOTIFYmWatchdogNotify;
> +STATIC UINT32   mTimerPeriod;
> 
>  /**
>Make sure the SP805 registers are unlocked for writing.
> @@ -65,6 +70,33 @@ SP805Lock (
>}
>  }
> 
> +STATIC
> +VOID
> +EFIAPI
> +SP805InterruptHandler (
> +  IN  HARDWARE_INTERRUPT_SOURCE   Source,
> +  IN  EFI_SYSTEM_CONTEXT  SystemContext
> +  )
> +{
> +  //
> +  // The notify function should be called with the elapsed number of
> +ticks
> +  // since the watchdog was armed, which should exceed the timer period.
> +  // We don't actually know the elapsed number of ticks, so let's
> +return
> +  // the timer period plus 1.
> +  //
> +  if (mWatchdogNotify != NULL) {
> +mWatchdogNotify (mTimerPeriod + 1);
> +  } else {
> +gRT->ResetSystem (EfiResetCold, EFI_TIMEOUT, 0, NULL);
> +  }

Shouldn't we ResetSystem in all cases.
 

> +
> +  SP805Unlock ();
> +  MmioWrite32 (SP805_WDOG_INT_CLR_REG, 0); // write of any value clears
> + the irq  SP805Lock ();
> +
> +  mInterrupt->EndOfInterrupt (mInterrupt, Source); }
> +

IMO, this routine should be 
1/ Handle IT
2/ Do callback mWatchdogNotify
3/ Reset the system.

>  /**
>Stop the SP805 watchdog timer from counting down by disabling interrupts.
>  **/
> @@ -149,9 +181,16 @@ SP805RegisterHandler (
>IN EFI_WATCHDOG_TIMER_NOTIFYNotifyFunction
>)
>  {
> -  // ERROR: This function is not supported.
> -  // The hardware watchdog will reset the board
> -  return EFI_INVALID_PARAMETER;
> +  if (mWatchdogNotify == NULL && NotifyFunction == NULL) {
> +return EFI_INVALID_PARAMETER;
> +  }
> +
> +  if (mWatchdogNotify != NULL && NotifyFunction != NULL) {
> +return EFI_ALREADY_STARTED;
> +  }
> +
> +  mWatchdogNotify = NotifyFunction;
> +  return EFI_SUCCESS;
>  }
> 
>  /**
> @@ -202,19 +241,16 @@ SP805SetTimerPeriod (
>  SP805Stop ();
>} else {
>  // Calculate the Watchdog ticks required for a delay of (TimerTicks * 
> 100)
> nanoseconds
> -// The SP805 will count down to ZERO once, generate an interrupt and
> -// then it will again reload the initial value and start again.
> -// On the second time

Re: [edk2] [PATCH 2/4] ArmPlatformPkg/SP805WatchdogDxe: switch to interrupt mode

2018-12-18 Thread Leif Lindholm

On Tue, Dec 18, 2018 at 02:10:12PM +0100, Ard Biesheuvel wrote:
> The SP805 watchdog driver doesn't implement the PI watchdog protocol
> fully, but always simply resets the system if the watchdog time runs
> out.
> 
> However, the hardware does support the intended usage model, as long
> as the SP805 is wired up correctly. So let's implement interrupt based
> mode involving a handler that is registered by the DXE core and invoked
> when the watchdog runs out. In the interrupt handler, we invoke the
> notify function if one was registered, or call the ResetSystem()
> runtime service otherwise (as per the UEFI spec)

The only question mark from my end is - what happens when the
interrupt isn't wired up correctly? Would it be worth to bail out and
refuse to register the driver if PcdSP805WatchdogInterrupt is set to
0?

Thomas?

/
Leif

> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel 
> ---
>  ArmPlatformPkg/ArmPlatformPkg.dec|  1 +
>  ArmPlatformPkg/Drivers/SP805WatchdogDxe/SP805Watchdog.c  | 95 
> ++--
>  ArmPlatformPkg/Drivers/SP805WatchdogDxe/SP805WatchdogDxe.inf |  6 +-
>  3 files changed, 75 insertions(+), 27 deletions(-)
> 
> diff --git a/ArmPlatformPkg/ArmPlatformPkg.dec 
> b/ArmPlatformPkg/ArmPlatformPkg.dec
> index 5f67e7415469..44c00bd0c133 100644
> --- a/ArmPlatformPkg/ArmPlatformPkg.dec
> +++ b/ArmPlatformPkg/ArmPlatformPkg.dec
> @@ -70,6 +70,7 @@
>## SP805 Watchdog
>gArmPlatformTokenSpaceGuid.PcdSP805WatchdogBase|0x0|UINT32|0x0023
>
> gArmPlatformTokenSpaceGuid.PcdSP805WatchdogClockFrequencyInHz|32000|UINT32|0x0021
> +  gArmPlatformTokenSpaceGuid.PcdSP805WatchdogInterrupt|0|UINT32|0x002E
>  
>## PL011 UART
>gArmPlatformTokenSpaceGuid.PL011UartClkInHz|2400|UINT32|0x001F
> diff --git a/ArmPlatformPkg/Drivers/SP805WatchdogDxe/SP805Watchdog.c 
> b/ArmPlatformPkg/Drivers/SP805WatchdogDxe/SP805Watchdog.c
> index 12c2f0a1fe49..4f09acf1fa28 100644
> --- a/ArmPlatformPkg/Drivers/SP805WatchdogDxe/SP805Watchdog.c
> +++ b/ArmPlatformPkg/Drivers/SP805WatchdogDxe/SP805Watchdog.c
> @@ -21,12 +21,17 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
> +#include 
>  #include 
>  
>  #include "SP805Watchdog.h"
>  
> -STATIC EFI_EVENT  mEfiExitBootServicesEvent;
> +STATIC EFI_EVENTmEfiExitBootServicesEvent;
> +STATIC EFI_HARDWARE_INTERRUPT_PROTOCOL  *mInterrupt;
> +STATIC EFI_WATCHDOG_TIMER_NOTIFYmWatchdogNotify;
> +STATIC UINT32   mTimerPeriod;
>  
>  /**
>Make sure the SP805 registers are unlocked for writing.
> @@ -65,6 +70,33 @@ SP805Lock (
>}
>  }
>  
> +STATIC
> +VOID
> +EFIAPI
> +SP805InterruptHandler (
> +  IN  HARDWARE_INTERRUPT_SOURCE   Source,
> +  IN  EFI_SYSTEM_CONTEXT  SystemContext
> +  )
> +{
> +  //
> +  // The notify function should be called with the elapsed number of ticks
> +  // since the watchdog was armed, which should exceed the timer period.
> +  // We don't actually know the elapsed number of ticks, so let's return
> +  // the timer period plus 1.
> +  //
> +  if (mWatchdogNotify != NULL) {
> +mWatchdogNotify (mTimerPeriod + 1);
> +  } else {
> +gRT->ResetSystem (EfiResetCold, EFI_TIMEOUT, 0, NULL);
> +  }
> +
> +  SP805Unlock ();
> +  MmioWrite32 (SP805_WDOG_INT_CLR_REG, 0); // write of any value clears the 
> irq
> +  SP805Lock ();
> +
> +  mInterrupt->EndOfInterrupt (mInterrupt, Source);
> +}
> +
>  /**
>Stop the SP805 watchdog timer from counting down by disabling interrupts.
>  **/
> @@ -149,9 +181,16 @@ SP805RegisterHandler (
>IN EFI_WATCHDOG_TIMER_NOTIFYNotifyFunction
>)
>  {
> -  // ERROR: This function is not supported.
> -  // The hardware watchdog will reset the board
> -  return EFI_INVALID_PARAMETER;
> +  if (mWatchdogNotify == NULL && NotifyFunction == NULL) {
> +return EFI_INVALID_PARAMETER;
> +  }
> +
> +  if (mWatchdogNotify != NULL && NotifyFunction != NULL) {
> +return EFI_ALREADY_STARTED;
> +  }
> +
> +  mWatchdogNotify = NotifyFunction;
> +  return EFI_SUCCESS;
>  }
>  
>  /**
> @@ -202,19 +241,16 @@ SP805SetTimerPeriod (
>  SP805Stop ();
>} else {
>  // Calculate the Watchdog ticks required for a delay of (TimerTicks * 
> 100) nanoseconds
> -// The SP805 will count down to ZERO once, generate an interrupt and
> -// then it will again reload the initial value and start again.
> -// On the second time when it reaches ZERO, it will actually reset the 
> board.
> -// Therefore, we need to load half the required delay.
> +// The SP805 will count down to zero and generate an interrupt.
>  //
> -// WatchdogTicks = ((TimerPeriod * 100 * SP805_CLOCK_FREQUENCY) / 1GHz) 
> / 2 ;
> +// WatchdogTicks = ((TimerPeriod * 100 * SP805_CLOCK_FREQUENCY) / 1GHz);
>  //
>  // i.e.:
>  //
> -// WatchdogTicks = (TimerPeriod * SP805_CLOCK_FREQUENCY) / 20 MHz ;
> +//

[edk2] [PATCH 2/4] ArmPlatformPkg/SP805WatchdogDxe: switch to interrupt mode

2018-12-18 Thread Ard Biesheuvel

The SP805 watchdog driver doesn't implement the PI watchdog protocol
fully, but always simply resets the system if the watchdog time runs
out.

However, the hardware does support the intended usage model, as long
as the SP805 is wired up correctly. So let's implement interrupt based
mode involving a handler that is registered by the DXE core and invoked
when the watchdog runs out. In the interrupt handler, we invoke the
notify function if one was registered, or call the ResetSystem()
runtime service otherwise (as per the UEFI spec)

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel 
---
 ArmPlatformPkg/ArmPlatformPkg.dec|  1 +
 ArmPlatformPkg/Drivers/SP805WatchdogDxe/SP805Watchdog.c  | 95 
++--
 ArmPlatformPkg/Drivers/SP805WatchdogDxe/SP805WatchdogDxe.inf |  6 +-
 3 files changed, 75 insertions(+), 27 deletions(-)

diff --git a/ArmPlatformPkg/ArmPlatformPkg.dec 
b/ArmPlatformPkg/ArmPlatformPkg.dec
index 5f67e7415469..44c00bd0c133 100644
--- a/ArmPlatformPkg/ArmPlatformPkg.dec
+++ b/ArmPlatformPkg/ArmPlatformPkg.dec
@@ -70,6 +70,7 @@
   ## SP805 Watchdog
   gArmPlatformTokenSpaceGuid.PcdSP805WatchdogBase|0x0|UINT32|0x0023
   
gArmPlatformTokenSpaceGuid.PcdSP805WatchdogClockFrequencyInHz|32000|UINT32|0x0021
+  gArmPlatformTokenSpaceGuid.PcdSP805WatchdogInterrupt|0|UINT32|0x002E
 
   ## PL011 UART
   gArmPlatformTokenSpaceGuid.PL011UartClkInHz|2400|UINT32|0x001F
diff --git a/ArmPlatformPkg/Drivers/SP805WatchdogDxe/SP805Watchdog.c 
b/ArmPlatformPkg/Drivers/SP805WatchdogDxe/SP805Watchdog.c
index 12c2f0a1fe49..4f09acf1fa28 100644
--- a/ArmPlatformPkg/Drivers/SP805WatchdogDxe/SP805Watchdog.c
+++ b/ArmPlatformPkg/Drivers/SP805WatchdogDxe/SP805Watchdog.c
@@ -21,12 +21,17 @@
 #include 
 #include 
 #include 
+#include 
 
+#include 
 #include 
 
 #include "SP805Watchdog.h"
 
-STATIC EFI_EVENT  mEfiExitBootServicesEvent;
+STATIC EFI_EVENTmEfiExitBootServicesEvent;
+STATIC EFI_HARDWARE_INTERRUPT_PROTOCOL  *mInterrupt;
+STATIC EFI_WATCHDOG_TIMER_NOTIFYmWatchdogNotify;
+STATIC UINT32   mTimerPeriod;
 
 /**
   Make sure the SP805 registers are unlocked for writing.
@@ -65,6 +70,33 @@ SP805Lock (
   }
 }
 
+STATIC
+VOID
+EFIAPI
+SP805InterruptHandler (
+  IN  HARDWARE_INTERRUPT_SOURCE   Source,
+  IN  EFI_SYSTEM_CONTEXT  SystemContext
+  )
+{
+  //
+  // The notify function should be called with the elapsed number of ticks
+  // since the watchdog was armed, which should exceed the timer period.
+  // We don't actually know the elapsed number of ticks, so let's return
+  // the timer period plus 1.
+  //
+  if (mWatchdogNotify != NULL) {
+mWatchdogNotify (mTimerPeriod + 1);
+  } else {
+gRT->ResetSystem (EfiResetCold, EFI_TIMEOUT, 0, NULL);
+  }
+
+  SP805Unlock ();
+  MmioWrite32 (SP805_WDOG_INT_CLR_REG, 0); // write of any value clears the irq
+  SP805Lock ();
+
+  mInterrupt->EndOfInterrupt (mInterrupt, Source);
+}
+
 /**
   Stop the SP805 watchdog timer from counting down by disabling interrupts.
 **/
@@ -149,9 +181,16 @@ SP805RegisterHandler (
   IN EFI_WATCHDOG_TIMER_NOTIFYNotifyFunction
   )
 {
-  // ERROR: This function is not supported.
-  // The hardware watchdog will reset the board
-  return EFI_INVALID_PARAMETER;
+  if (mWatchdogNotify == NULL && NotifyFunction == NULL) {
+return EFI_INVALID_PARAMETER;
+  }
+
+  if (mWatchdogNotify != NULL && NotifyFunction != NULL) {
+return EFI_ALREADY_STARTED;
+  }
+
+  mWatchdogNotify = NotifyFunction;
+  return EFI_SUCCESS;
 }
 
 /**
@@ -202,19 +241,16 @@ SP805SetTimerPeriod (
 SP805Stop ();
   } else {
 // Calculate the Watchdog ticks required for a delay of (TimerTicks * 100) 
nanoseconds
-// The SP805 will count down to ZERO once, generate an interrupt and
-// then it will again reload the initial value and start again.
-// On the second time when it reaches ZERO, it will actually reset the 
board.
-// Therefore, we need to load half the required delay.
+// The SP805 will count down to zero and generate an interrupt.
 //
-// WatchdogTicks = ((TimerPeriod * 100 * SP805_CLOCK_FREQUENCY) / 1GHz) / 
2 ;
+// WatchdogTicks = ((TimerPeriod * 100 * SP805_CLOCK_FREQUENCY) / 1GHz);
 //
 // i.e.:
 //
-// WatchdogTicks = (TimerPeriod * SP805_CLOCK_FREQUENCY) / 20 MHz ;
+// WatchdogTicks = (TimerPeriod * SP805_CLOCK_FREQUENCY) / 10 MHz ;
 
 Ticks64bit = MultU64x32 (TimerPeriod, PcdGet32 
(PcdSP805WatchdogClockFrequencyInHz));
-Ticks64bit = DivU64x32 (Ticks64bit, 2000);
+Ticks64bit = DivU64x32 (Ticks64bit, 10 * 1000 * 1000);
 
 // The registers in the SP805 are only 32 bits
 if (Ticks64bit > MAX_UINT32) {
@@ -233,9 +269,12 @@ SP805SetTimerPeriod (
 SP805Start ();
   }
 
+  mTimerPeriod = TimerPeriod;
+
 EXIT:
   // Ensure the watchdog is locked before exiting.
   SP805Lock ();
+

Re: [edk2] [PATCH 2/4] ArmPlatformPkg/SP805WatchdogDxe: switch to interrupt mode

Re: [edk2] [PATCH 2/4] ArmPlatformPkg/SP805WatchdogDxe: switch to interrupt mode

Re: [edk2] [PATCH 2/4] ArmPlatformPkg/SP805WatchdogDxe: switch to interrupt mode

Re: [edk2] [PATCH 2/4] ArmPlatformPkg/SP805WatchdogDxe: switch to interrupt mode

Re: [edk2] [PATCH 2/4] ArmPlatformPkg/SP805WatchdogDxe: switch to interrupt mode

[edk2] [PATCH 2/4] ArmPlatformPkg/SP805WatchdogDxe: switch to interrupt mode

6 matches

Site Navigation

Mail list logo

Footer information