Re: [PATCH edk2-platforms 03/12] Silicon/NXP : Add support for Watchdog driver

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

Hi Meenakshi,

As you will have no doubt noticed, edk2-devel@lists.01.org has been
retired - we are now using devel@edk2.groups.io, please subscribe (and
post patches) to that one instead.

I have bounced my replies to your first two patches there, and will
try to remember to manually change the list address in my replies to
the remaining patches.

On Wed, Oct 09, 2019 at 10:19:09PM +0530, Meenakshi Aggarwal wrote:
> Installs watchdog timer arch protocol
>

I am a little bit surprised by this patch being submitted again - in
our last communication on this set (29 January), you said
"Decided to use watchdog driver from MdeModulePkg."

Did you change your mind? This driver still registers a 
EFI_WATCHDOG_TIMER_ARCH_PROTOCOL, which is a poor match for what this
hardware actually does.

Best Regards,

Leif

> Signed-off-by: Meenakshi Aggarwal <meenakshi.aggarwal@nxp.com>
> ---
>  Silicon/NXP/Drivers/WatchDog/WatchDog.c      | 396 +++++++++++++++++++++++++++
>  Silicon/NXP/Drivers/WatchDog/WatchDog.h      |  32 +++
>  Silicon/NXP/Drivers/WatchDog/WatchDogDxe.inf |  41 +++
>  3 files changed, 469 insertions(+)
>  create mode 100644 Silicon/NXP/Drivers/WatchDog/WatchDog.c
>  create mode 100644 Silicon/NXP/Drivers/WatchDog/WatchDog.h
>  create mode 100644 Silicon/NXP/Drivers/WatchDog/WatchDogDxe.inf
> 
> diff --git a/Silicon/NXP/Drivers/WatchDog/WatchDog.c b/Silicon/NXP/Drivers/WatchDog/WatchDog.c
> new file mode 100644
> index 0000000..c2d104a
> --- /dev/null
> +++ b/Silicon/NXP/Drivers/WatchDog/WatchDog.c
> @@ -0,0 +1,396 @@
> +/** WatchDog.c
> +*
> +*  Based on Watchdog driver implemenation available in
> +*  ArmPlatformPkg/Drivers/SP805WatchdogDxe/SP805Watchdog.c
> +*
> +*  Copyright 2017-2019 NXP
> +*
> +*  SPDX-License-Identifier: BSD-2-Clause-Patent
> +*
> +**/
> +
> +#include <PiDxe.h>
> +#include <Library/BaseLib.h>
> +#include <Library/IoAccessLib.h>
> +#include <Library/DebugLib.h>
> +#include <Library/PcdLib.h>
> +#include <Library/UefiBootServicesTableLib.h>
> +#include <Protocol/WatchdogTimer.h>
> +
> +#include "WatchDog.h"
> +
> +STATIC EFI_EVENT  mEfiExitBootServicesEvent;
> +STATIC EFI_EVENT  mWatchdogFeedEvent;
> +STATIC MMIO_OPERATIONS_16 *mMmioOps;
> +
> +
> +STATIC
> +VOID
> +WatchdogPing (
> +  VOID
> +  )
> +{
> +  //
> +  // To reload a timeout value to the counter the proper service sequence begins by
> +  // writing 0x_5555 followed by 0x_AAAA to the Watchdog Service Register (WATCHDOG_WSR).
> +  // This service sequence will reload the counter with the timeout value WATCHDOG[7:0] of
> +  // Watchdog Control Register (WATCHDOG_WCR).
> +  //
> +
> +  mMmioOps->Write16 (PcdGet64 (PcdWatchdog1BaseAddr) + WATCHDOG_WSR_OFFSET,
> +                     WATCHDOG_SERVICE_SEQ1);
> +  mMmioOps->Write16 (PcdGet64 (PcdWatchdog1BaseAddr) + WATCHDOG_WSR_OFFSET,
> +                     WATCHDOG_SERVICE_SEQ2);
> +}
> +
> +/**
> +  Stop the Watchdog watchdog timer from counting down.
> +**/
> +STATIC
> +VOID
> +WatchdogStop (
> +  VOID
> +  )
> +{
> +  // Watchdog cannot be disabled by software once started.
> +  // At best, we can keep reload counter with maximum value
> +
> +  mMmioOps->AndThenOr16 (PcdGet64 (PcdWatchdog1BaseAddr) + WATCHDOG_WCR_OFFSET,
> +                        (UINT16)(~WATCHDOG_WCR_TIMEOUT),
> +                        (WATCHDOG_COUNT (WATCHDOG_MAX_TIME) & WATCHDOG_COUNT_MASK));
> +  WatchdogPing ();
> +}
> +
> +/**
> +  Starts the Watchdog counting down by feeding Service register with
> +  desired pattern.
> +  The count down will start from the value stored in the Load register,
> +  not from the value where it was previously stopped.
> +**/
> +STATIC
> +VOID
> +WatchdogStart (
> +  VOID
> +  )
> +{
> +  //Reload the timeout value
> +  WatchdogPing ();
> +}
> +
> +/**
> +    On exiting boot services we must make sure the Watchdog Watchdog Timer
> +    is stopped.
> +**/
> +STATIC
> +VOID
> +EFIAPI
> +ExitBootServicesEvent (
> +  IN EFI_EVENT  Event,
> +  IN VOID       *Context
> +  )
> +{
> +  WatchdogStop ();
> +}
> +
> +/**
> +  This function registers the handler NotifyFunction so it is called every time
> +  the watchdog timer expires.  It also passes the amount of time since the last
> +  handler call to the NotifyFunction.
> +  If NotifyFunction is not NULL and a handler is not already registered,
> +  then the new handler is registered and EFI_SUCCESS is returned.
> +  If NotifyFunction is NULL, and a handler is already registered,
> +  then that handler is unregistered.
> +  If an attempt is made to register a handler when a handler is already registered,
> +  then EFI_ALREADY_STARTED is returned.
> +  If an attempt is made to unregister a handler when a handler is not registered,
> +  then EFI_INVALID_PARAMETER is returned.
> +
> +  @param  This             The EFI_TIMER_ARCH_PROTOCOL instance.
> +  @param  NotifyFunction   The function to call when a timer interrupt fires. This
> +                           function executes at TPL_HIGH_LEVEL. The DXE Core will
> +                           register a handler for the timer interrupt, so it can know
> +                           how much time has passed. This information is used to
> +                           signal timer based events. NULL will unregister the handler.
> +
> +  @retval EFI_SUCCESS           The watchdog timer handler was registered.
> +  @retval EFI_ALREADY_STARTED   NotifyFunction is not NULL, and a handler is already
> +                                registered.
> +  @retval EFI_INVALID_PARAMETER NotifyFunction is NULL, and a handler was not
> +                                previously registered.
> +
> +**/
> +STATIC
> +EFI_STATUS
> +EFIAPI
> +WatchdogRegisterHandler (
> +  IN EFI_WATCHDOG_TIMER_ARCH_PROTOCOL   *This,
> +  IN EFI_WATCHDOG_TIMER_NOTIFY          NotifyFunction
> +  )
> +{
> +  // ERROR: This function is not supported.
> +  // The hardware watchdog will reset the board
> +  return EFI_INVALID_PARAMETER;
> +}
> +
> +/**
> +
> +  This function adjusts the period of timer interrupts to the value specified
> +  by TimerPeriod.  If the timer period is updated, then the selected timer
> +  period is stored in EFI_TIMER.TimerPeriod, and EFI_SUCCESS is returned.  If
> +  the timer hardware is not programmable, then EFI_UNSUPPORTED is returned.
> +  If an error occurs while attempting to update the timer period, then the
> +  timer hardware will be put back in its state prior to this call, and
> +  EFI_DEVICE_ERROR is returned.  If TimerPeriod is 0, then the timer interrupt
> +  is disabled.  This is not the same as disabling the CPU's interrupts.
> +  Instead, it must either turn off the timer hardware, or it must adjust the
> +  interrupt controller so that a CPU interrupt is not generated when the timer
> +  interrupt fires.
> +
> +  @param  This             The EFI_TIMER_ARCH_PROTOCOL instance.
> +  @param  TimerPeriod      The rate to program the timer interrupt in 100 nS units. If
> +                           the timer hardware is not programmable, then EFI_UNSUPPORTED is
> +                           returned. If the timer is programmable, then the timer period
> +                           will be rounded up to the nearest timer period that is supported
> +                           by the timer hardware. If TimerPeriod is set to 0, then the
> +                           timer interrupts will be disabled.
> +
> +
> +  @retval EFI_SUCCESS           The timer period was changed.
> +  @retval EFI_UNSUPPORTED       The platform cannot change the period of the timer interrupt.
> +  @retval EFI_DEVICE_ERROR      The timer period could not be changed due to a device error.
> +
> +**/
> +STATIC
> +EFI_STATUS
> +EFIAPI
> +WatchdogSetTimerPeriod (
> +  IN EFI_WATCHDOG_TIMER_ARCH_PROTOCOL   *This,
> +  IN UINT64                             TimerPeriod   // In 100ns units
> +  )
> +{
> +  EFI_STATUS  Status;
> +  UINT64      TimerPeriodInSec;
> +  UINT16      Val;
> +
> +  Status = EFI_SUCCESS;
> +
> +  if (TimerPeriod == 0) {
> +    // This is a watchdog stop request
> +    WatchdogStop ();
> +    return Status;
> +  } else {
> +    // Convert the TimerPeriod (in 100 ns unit) to an equivalent second value
> +
> +    TimerPeriodInSec = DivU64x32 (TimerPeriod, NANO_SECOND_BASE);
> +
> +    // The registers in the Watchdog are only 32 bits
> +    if (TimerPeriodInSec > WATCHDOG_MAX_TIME) {
> +      // We could load the watchdog with the maximum supported value but
> +      // if a smaller value was requested, this could have the watchdog
> +      // triggering before it was intended.
> +      // Better generate an error to let the caller know.
> +      Status = EFI_DEVICE_ERROR;
> +      return Status;
> +    }
> +
> +    // set the new timeout value in the WCR
> +    // Convert the timeout value from Seconds to timer count
> +    Val = ((WATCHDOG_COUNT(TimerPeriodInSec) & WATCHDOG_COUNT_MASK) << 8);
> +
> +    mMmioOps->AndThenOr16 (PcdGet64 (PcdWatchdog1BaseAddr) + WATCHDOG_WCR_OFFSET,
> +                          (UINT16)(~WATCHDOG_WCR_TIMEOUT),
> +                          Val);
> +    // Start the watchdog
> +    WatchdogStart ();
> +  }
> +
> +  return Status;
> +}
> +
> +/**
> +  This function retrieves the period of timer interrupts in 100 ns units,
> +  returns that value in TimerPeriod, and returns EFI_SUCCESS.  If TimerPeriod
> +  is NULL, then EFI_INVALID_PARAMETER is returned.  If a TimerPeriod of 0 is
> +  returned, then the timer is currently disabled.
> +
> +  @param  This             The EFI_TIMER_ARCH_PROTOCOL instance.
> +  @param  TimerPeriod      A pointer to the timer period to retrieve in 100 ns units. If
> +                           0 is returned, then the timer is currently disabled.
> +
> +
> +  @retval EFI_SUCCESS           The timer period was returned in TimerPeriod.
> +  @retval EFI_INVALID_PARAMETER TimerPeriod is NULL.
> +
> +**/
> +STATIC
> +EFI_STATUS
> +EFIAPI
> +WatchdogGetTimerPeriod (
> +  IN  EFI_WATCHDOG_TIMER_ARCH_PROTOCOL   *This,
> +  OUT UINT64                             *TimerPeriod
> +  )
> +{
> +  EFI_STATUS  Status;
> +  UINT64      ReturnValue;
> +  UINT16      Val;
> +
> +  Status = EFI_SUCCESS;
> +
> +  if (TimerPeriod == NULL) {
> +    return EFI_INVALID_PARAMETER;
> +  }
> +
> +  // Check if the watchdog is stopped
> +  if ((mMmioOps->Read16 (PcdGet64 (PcdWatchdog1BaseAddr) + WATCHDOG_WCR_OFFSET)
> +              & WATCHDOG_WCR_ENABLE) == 0 ) {
> +    // It is stopped, so return zero.
> +    ReturnValue = 0;
> +  } else {
> +    // Convert the Watchdog ticks into equivalent TimerPeriod second value.
> +    Val = (mMmioOps->Read16 (PcdGet64 (PcdWatchdog1BaseAddr) + WATCHDOG_WCR_OFFSET)
> +            & WATCHDOG_WCR_TIMEOUT ) >> 8;
> +    ReturnValue = WATCHDOG_SEC(Val);
> +  }
> +
> +  *TimerPeriod = ReturnValue;
> +  return Status;
> +}
> +
> +/**
> +  Interface structure for the Watchdog Architectural Protocol.
> +
> +  @par Protocol Description:
> +  This protocol provides a service to set the amount of time to wait
> +  before firing the watchdog timer, and it also provides a service to
> +  register a handler that is invoked when the watchdog timer fires.
> +
> +  @par When the watchdog timer fires, control will be passed to a handler
> +  if one has been registered.  If no handler has been registered,
> +  or the registered handler returns, then the system will be
> +  reset by calling the Runtime Service ResetSystem().
> +
> +  @param RegisterHandler
> +  Registers a handler that will be called each time the
> +  watchdogtimer interrupt fires.  TimerPeriod defines the minimum
> +  time between timer interrupts, so TimerPeriod will also
> +  be the minimum time between calls to the registered
> +  handler.
> +  NOTE: If the watchdog resets the system in hardware, then
> +        this function will not have any chance of executing.
> +
> +  @param SetTimerPeriod
> +  Sets the period of the timer interrupt in 100 nS units.
> +  This function is optional, and may return EFI_UNSUPPORTED.
> +  If this function is supported, then the timer period will
> +  be rounded up to the nearest supported timer period.
> +
> +  @param GetTimerPeriod
> +  Retrieves the period of the timer interrupt in 100 nS units.
> +
> +**/
> +STATIC
> +EFI_WATCHDOG_TIMER_ARCH_PROTOCOL  mWatchdogTimer = {
> +  WatchdogRegisterHandler,
> +  WatchdogSetTimerPeriod,
> +  WatchdogGetTimerPeriod
> +};
> +
> +/**
> +  Call back function when the timer event is signaled.
> +  This function will feed the watchdog with maximum value
> +  so that system wont reset in idle case e.g. stopped on UEFI shell.
> +
> +  @param[in]  Event     The Event this notify function registered to.
> +  @param[in]  Context   Pointer to the context data registered to the
> +                        Event.
> +
> +**/
> +VOID
> +EFIAPI
> +WatchdogFeed (
> +  IN EFI_EVENT          Event,
> +  IN VOID*              Context
> +  )
> +{
> +  WatchdogPing();
> +}
> +/**
> +  Initialize state information for the Watchdog Timer Architectural Protocol.
> +
> +  @param  ImageHandle   of the loaded driver
> +  @param  SystemTable   Pointer to the System Table
> +
> +  @retval EFI_SUCCESS           Protocol registered
> +  @retval EFI_OUT_OF_RESOURCES  Cannot allocate protocol data structure
> +  @retval EFI_DEVICE_ERROR      Hardware problems
> +
> +**/
> +EFI_STATUS
> +EFIAPI
> +WatchdogInitialize (
> +  IN EFI_HANDLE         ImageHandle,
> +  IN EFI_SYSTEM_TABLE   *SystemTable
> +  )
> +{
> +  EFI_STATUS  Status;
> +  EFI_HANDLE  Handle;
> +
> +  mMmioOps = GetMmioOperations16 (FixedPcdGetBool (PcdWatchdogBigEndian));
> +
> +  mMmioOps->AndThenOr16 (PcdGet64 (PcdWatchdog1BaseAddr) + WATCHDOG_WCR_OFFSET,
> +                        (UINT16)(~WATCHDOG_WCR_TIMEOUT),
> +                        (WATCHDOG_COUNT (WATCHDOG_MAX_TIME) & WATCHDOG_COUNT_MASK));
> +
> +  mMmioOps->Or16 (PcdGet64 (PcdWatchdog1BaseAddr) + WATCHDOG_WCR_OFFSET, WATCHDOG_WCR_ENABLE);
> +
> +  //
> +  // Make sure the Watchdog Timer Architectural Protocol
> +  // has not been installed in the system yet.
> +  // This will avoid conflicts with the universal watchdog
> +  //
> +  ASSERT_PROTOCOL_ALREADY_INSTALLED (NULL, &gEfiWatchdogTimerArchProtocolGuid);
> +
> +  // Register for an ExitBootServicesEvent
> +  Status = gBS->CreateEvent (EVT_SIGNAL_EXIT_BOOT_SERVICES, TPL_NOTIFY,
> +              ExitBootServicesEvent, NULL, &mEfiExitBootServicesEvent);
> +  if (EFI_ERROR (Status)) {
> +    Status = EFI_OUT_OF_RESOURCES;
> +    return Status;
> +  }
> +
> +  //
> +  // Start the timer to feed Watchdog with maximum timeout value.
> +  //
> +  Status = gBS->CreateEvent (
> +                  EVT_TIMER | EVT_NOTIFY_SIGNAL,
> +                  TPL_NOTIFY,
> +                  WatchdogFeed,
> +                  NULL,
> +                  &mWatchdogFeedEvent
> +                  );
> +  if (EFI_ERROR (Status)) {
> +    return Status;
> +  }
> +
> +  Status = gBS->SetTimer (mWatchdogFeedEvent, TimerPeriodic, WATCHDOG_FEED_INTERVAL);
> +  if (EFI_ERROR (Status)) {
> +    return Status;
> +  }
> +
> +  // Install the Timer Architectural Protocol onto a new handle
> +  Handle = NULL;
> +  Status = gBS->InstallMultipleProtocolInterfaces (
> +                  &Handle,
> +                  &gEfiWatchdogTimerArchProtocolGuid, &mWatchdogTimer,
> +                  NULL
> +                  );
> +  if (EFI_ERROR (Status)) {
> +    gBS->CloseEvent (mEfiExitBootServicesEvent);
> +    Status = EFI_OUT_OF_RESOURCES;
> +    return Status;
> +  }
> +
> +  WatchdogPing ();
> +
> +  return Status;
> +}
> diff --git a/Silicon/NXP/Drivers/WatchDog/WatchDog.h b/Silicon/NXP/Drivers/WatchDog/WatchDog.h
> new file mode 100644
> index 0000000..8262a69
> --- /dev/null
> +++ b/Silicon/NXP/Drivers/WatchDog/WatchDog.h
> @@ -0,0 +1,32 @@
> +/** WatchDog.h
> +*
> +*  Copyright 2017-2019 NXP
> +*
> +*  SPDX-License-Identifier: BSD-2-Clause-Patent
> +*
> +**/
> +
> +#ifndef WATCHDOG_H_
> +#define WATCHDOG_H_
> +
> +#define WATCHDOG_SIZE             0x1000
> +#define WATCHDOG_WCR_OFFSET       0
> +#define WATCHDOG_WSR_OFFSET       2
> +#define WATCHDOG_WRSR_OFFSET      4
> +#define WATCHDOG_WICR_OFFSET      6
> +#define WATCHDOG_WCR_TIMEOUT     (0xFF << 8)
> +#define WATCHDOG_WCR_ENABLE    (1 << 2)
> +#define WATCHDOG_SERVICE_SEQ1     0x5555
> +#define WATCHDOG_SERVICE_SEQ2     0xAAAA
> +#define WATCHDOG_WCR_WRE          (1 << 3)  /* -> WATCHDOG Reset Enable */
> +
> +#define WATCHDOG_MAX_TIME         128
> +#define WATCHDOG_COUNT(Sec)       (((Sec) * 2 - 1) << 8)
> +#define WATCHDOG_COUNT_MASK       0xff00
> +#define WATCHDOG_SEC(Cnt)         (((Cnt) + 1) / 2)
> +
> +#define NANO_SECOND_BASE          10000000
> +
> +#define WATCHDOG_FEED_INTERVAL    (WATCHDOG_MAX_TIME * NANO_SECOND_BASE)
> +
> +#endif //WATCHDOG_H_
> diff --git a/Silicon/NXP/Drivers/WatchDog/WatchDogDxe.inf b/Silicon/NXP/Drivers/WatchDog/WatchDogDxe.inf
> new file mode 100644
> index 0000000..2d410a9
> --- /dev/null
> +++ b/Silicon/NXP/Drivers/WatchDog/WatchDogDxe.inf
> @@ -0,0 +1,41 @@
> +#  WatchDog.inf
> +#
> +#  Component description file for  WatchDog module
> +#
> +#  Copyright 2017-2019 NXP
> +#
> +#  SPDX-License-Identifier: BSD-2-Clause-Patent
> +#
> +#
> +
> +[Defines]
> +  INF_VERSION                    = 0x0001001A
> +  BASE_NAME                      = WatchDogDxe
> +  FILE_GUID                      = 0358b544-ec65-4339-89cd-cad60a3dd787
> +  MODULE_TYPE                    = DXE_DRIVER
> +  VERSION_STRING                 = 1.0
> +  ENTRY_POINT                    = WatchdogInitialize
> +
> +[Sources.common]
> +  WatchDog.c
> +
> +[Packages]
> +  MdePkg/MdePkg.dec
> +  Silicon/NXP/NxpQoriqLs.dec
> +
> +[LibraryClasses]
> +  BaseLib
> +  IoAccessLib
> +  PcdLib
> +  UefiBootServicesTableLib
> +  UefiDriverEntryPoint
> +
> +[Pcd]
> +  gNxpQoriqLsTokenSpaceGuid.PcdWatchdog1BaseAddr
> +  gNxpQoriqLsTokenSpaceGuid.PcdWatchdogBigEndian
> +
> +[Protocols]
> +  gEfiWatchdogTimerArchProtocolGuid
> +
> +[Depex]
> +  TRUE
> -- 
> 1.9.1
>