| Message ID | 20181218131015.20062-5-ard.biesheuvel@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | ArmPkg, ArmPlatformPkg: watchdog driver cleanup | expand |
On Tue, Dec 18, 2018 at 02:10:14PM +0100, Ard Biesheuvel wrote: > Even though UEFI does not appear to use it, let's implement the > complete PI watchdog protocol, including handler registration, > which will be invoked instead of the ResetSystem() runtime service > when the watchdog timer expires. > > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org> Thanks! > --- > ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c | 34 ++++++++++++++------ > 1 file changed, 25 insertions(+), 9 deletions(-) > > diff --git a/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c b/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c > index 717a180a64ec..21118a3c88d1 100644 > --- a/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c > +++ b/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c > @@ -42,6 +42,7 @@ STATIC UINTN mTimerFrequencyHz = 0; > STATIC UINT64 mNumTimerTicks = 0; > > STATIC EFI_HARDWARE_INTERRUPT2_PROTOCOL *mInterruptProtocol; > +STATIC EFI_WATCHDOG_TIMER_NOTIFY mWatchdogNotify; > > STATIC > VOID > @@ -107,17 +108,25 @@ WatchdogInterruptHandler ( > ) > { > STATIC CONST CHAR16 ResetString[]= L"The generic watchdog timer ran out."; > + UINT64 TimerPeriod; > > WatchdogDisable (); > > mInterruptProtocol->EndOfInterrupt (mInterruptProtocol, Source); > > - gRT->ResetSystem ( > - EfiResetCold, > - EFI_TIMEOUT, > - StrSize (ResetString), > - (VOID *) &ResetString > - ); > + // > + // 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) { > + TimerPeriod = ((TIME_UNITS_PER_SECOND / mTimerFrequencyHz) * mNumTimerTicks); > + mWatchdogNotify (TimerPeriod + 1); > + } else { > + gRT->ResetSystem (EfiResetCold, EFI_TIMEOUT, StrSize (ResetString), > + (CHAR16 *)ResetString); > + } > > // If we got here then the reset didn't work > ASSERT (FALSE); > @@ -155,9 +164,16 @@ WatchdogRegisterHandler ( > IN EFI_WATCHDOG_TIMER_NOTIFY NotifyFunction > ) > { > - // ERROR: This function is not supported. > - // The watchdog will reset the board > - return EFI_UNSUPPORTED; > + if (mWatchdogNotify == NULL && NotifyFunction == NULL) { > + return EFI_INVALID_PARAMETER; > + } > + > + if (mWatchdogNotify != NULL && NotifyFunction != NULL) { > + return EFI_ALREADY_STARTED; > + } > + > + mWatchdogNotify = NotifyFunction; > + return EFI_SUCCESS; > } > > /** > -- > 2.17.1 > _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
> -----Original Message----- > From: Ard Biesheuvel <ard.biesheuvel@linaro.org> > Sent: Tuesday, December 18, 2018 6:40 PM > To: edk2-devel@lists.01.org > Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>; Leif Lindholm > <leif.lindholm@linaro.org>; Sami Mujawar <sami.mujawar@arm.com>; Thomas > Panakamattam Abraham <thomas.abraham@arm.com>; Meenakshi Aggarwal > <meenakshi.aggarwal@nxp.com>; Udit Kumar <udit.kumar@nxp.com>; Matteo > Carlini <Matteo.Carlini@arm.com>; Nariman Poushin > <nariman.poushin@linaro.org> > Subject: [PATCH 4/4] ArmPkg/GenericWatchdogDxe: implement > RegisterHandler() method > > Even though UEFI does not appear to use it, let's implement the complete PI > watchdog protocol, including handler registration, which will be invoked instead > of the ResetSystem() runtime service when the watchdog timer expires. > > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> > --- > ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c | 34 > ++++++++++++++------ > 1 file changed, 25 insertions(+), 9 deletions(-) > > diff --git a/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c > b/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c > index 717a180a64ec..21118a3c88d1 100644 > --- a/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c > +++ b/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c > @@ -42,6 +42,7 @@ STATIC UINTN mTimerFrequencyHz = 0; STATIC UINT64 > mNumTimerTicks = 0; > > STATIC EFI_HARDWARE_INTERRUPT2_PROTOCOL *mInterruptProtocol; > +STATIC EFI_WATCHDOG_TIMER_NOTIFY mWatchdogNotify; > > STATIC > VOID > @@ -107,17 +108,25 @@ WatchdogInterruptHandler ( > ) > { > STATIC CONST CHAR16 ResetString[]= L"The generic watchdog timer ran out."; > + UINT64 TimerPeriod; > > WatchdogDisable (); > > mInterruptProtocol->EndOfInterrupt (mInterruptProtocol, Source); > > - gRT->ResetSystem ( > - EfiResetCold, > - EFI_TIMEOUT, > - StrSize (ResetString), > - (VOID *) &ResetString > - ); > + // > + // 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) { > + TimerPeriod = ((TIME_UNITS_PER_SECOND / mTimerFrequencyHz) * > mNumTimerTicks); > + mWatchdogNotify (TimerPeriod + 1); > + } else { > + gRT->ResetSystem (EfiResetCold, EFI_TIMEOUT, StrSize (ResetString), > + (CHAR16 *)ResetString); > + } Here too, please handle reset in all cases > // If we got here then the reset didn't work > ASSERT (FALSE); > @@ -155,9 +164,16 @@ WatchdogRegisterHandler ( > IN EFI_WATCHDOG_TIMER_NOTIFY NotifyFunction > ) > { > - // ERROR: This function is not supported. > - // The watchdog will reset the board > - return EFI_UNSUPPORTED; > + if (mWatchdogNotify == NULL && NotifyFunction == NULL) { > + return EFI_INVALID_PARAMETER; > + } > + > + if (mWatchdogNotify != NULL && NotifyFunction != NULL) { > + return EFI_ALREADY_STARTED; > + } > + > + mWatchdogNotify = NotifyFunction; > + return EFI_SUCCESS; > } > > /** > -- > 2.17.1 _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
diff --git a/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c b/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c index 717a180a64ec..21118a3c88d1 100644 --- a/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c +++ b/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c @@ -42,6 +42,7 @@ STATIC UINTN mTimerFrequencyHz = 0; STATIC UINT64 mNumTimerTicks = 0; STATIC EFI_HARDWARE_INTERRUPT2_PROTOCOL *mInterruptProtocol; +STATIC EFI_WATCHDOG_TIMER_NOTIFY mWatchdogNotify; STATIC VOID @@ -107,17 +108,25 @@ WatchdogInterruptHandler ( ) { STATIC CONST CHAR16 ResetString[]= L"The generic watchdog timer ran out."; + UINT64 TimerPeriod; WatchdogDisable (); mInterruptProtocol->EndOfInterrupt (mInterruptProtocol, Source); - gRT->ResetSystem ( - EfiResetCold, - EFI_TIMEOUT, - StrSize (ResetString), - (VOID *) &ResetString - ); + // + // 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) { + TimerPeriod = ((TIME_UNITS_PER_SECOND / mTimerFrequencyHz) * mNumTimerTicks); + mWatchdogNotify (TimerPeriod + 1); + } else { + gRT->ResetSystem (EfiResetCold, EFI_TIMEOUT, StrSize (ResetString), + (CHAR16 *)ResetString); + } // If we got here then the reset didn't work ASSERT (FALSE); @@ -155,9 +164,16 @@ WatchdogRegisterHandler ( IN EFI_WATCHDOG_TIMER_NOTIFY NotifyFunction ) { - // ERROR: This function is not supported. - // The watchdog will reset the board - return EFI_UNSUPPORTED; + if (mWatchdogNotify == NULL && NotifyFunction == NULL) { + return EFI_INVALID_PARAMETER; + } + + if (mWatchdogNotify != NULL && NotifyFunction != NULL) { + return EFI_ALREADY_STARTED; + } + + mWatchdogNotify = NotifyFunction; + return EFI_SUCCESS; } /**
Even though UEFI does not appear to use it, let's implement the complete PI watchdog protocol, including handler registration, which will be invoked instead of the ResetSystem() runtime service when the watchdog timer expires. Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> --- ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c | 34 ++++++++++++++------ 1 file changed, 25 insertions(+), 9 deletions(-) -- 2.17.1 _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel