Re: [PATCH 04/10] Platform/NXP : Add support for Watchdog driver

[Meenakshi Aggarwal <meenakshi.aggarwal@nxp.com>]

> -----Original Message-----
> From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
> Sent: Monday, November 13, 2017 5:53 PM
> To: Meenakshi Aggarwal <meenakshi.aggarwal@nxp.com>
> Cc: Leif Lindholm <leif.lindholm@linaro.org>; Kinney, Michael D
> <michael.d.kinney@intel.com>; edk2-devel@lists.01.org; Udit Kumar
> <udit.kumar@nxp.com>; Varun Sethi <V.Sethi@nxp.com>
> Subject: Re: [PATCH 04/10] Platform/NXP : Add support for Watchdog driver
> 
> On 7 November 2017 at 14:42, Meenakshi Aggarwal
> <meenakshi.aggarwal@nxp.com> wrote:
> > Installs watchdog timer arch protocol
> >
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Meenakshi Aggarwal <meenakshi.aggarwal@nxp.com>
> > ---
> >  Platform/NXP/Drivers/WatchDog/WatchDog.c      | 386
> ++++++++++++++++++++++++++
> >  Platform/NXP/Drivers/WatchDog/WatchDog.h      |  37 +++
> >  Platform/NXP/Drivers/WatchDog/WatchDogDxe.inf |  47 ++++
> >  3 files changed, 470 insertions(+)
> >  create mode 100644 Platform/NXP/Drivers/WatchDog/WatchDog.c
> >  create mode 100644 Platform/NXP/Drivers/WatchDog/WatchDog.h
> >  create mode 100644 Platform/NXP/Drivers/WatchDog/WatchDogDxe.inf
> >
> > diff --git a/Platform/NXP/Drivers/WatchDog/WatchDog.c
> > b/Platform/NXP/Drivers/WatchDog/WatchDog.c
> > new file mode 100644
> > index 0000000..f257a1d
> > --- /dev/null
> > +++ b/Platform/NXP/Drivers/WatchDog/WatchDog.c
> > @@ -0,0 +1,386 @@
> > +/** WatchDog.c
> > +*
> > +*  Based on Watchdog driver implemenation available in
> > +*  ArmPlatformPkg/Drivers/SP805WatchdogDxe/SP805Watchdog.c
> > +*
> > +*  Copyright (c) 2011-2013, ARM Limited. All rights reserved.
> > +*  Copyright 2017 NXP
> > +*
> > +*  This program and the accompanying materials
> > +*  are licensed and made available under the terms and conditions of
> > +the BSD License
> > +*  which accompanies this distribution.  The full text of the license
> > +may be found at
> > +*
> >
> +https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fop
> e
> > +nsource.org%2Flicenses%2Fbsd-
> license.php&data=02%7C01%7Cmeenakshi.agg
> >
> +arwal%40nxp.com%7Cfe03b552d02b478a923608d52a9150f8%7C686ea1d3bc
> 2b4c6f
> >
> +a92cd99c5c301635%7C0%7C0%7C636461725998922414&sdata=094m%2FkgX
> NuvwXRS
> > +Ewe%2Bke8do3KvWhW7PBAvMTy2O%2Fc4%3D&reserved=0
> > +*
> > +*  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS"
> > +BASIS,
> > +*  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER
> EXPRESS OR IMPLIED.
> > +*
> > +**/
> > +
> > +#include <PiDxe.h>
> > +#include <Library/BaseLib.h>
> > +#include <Library/BeIoLib.h>
> > +#include <Library/DebugLib.h>
> > +#include <Library/IoLib.h>
> > +#include <Library/PcdLib.h>
> > +#include <Library/UefiBootServicesTableLib.h>
> > +#include <Protocol/WatchdogTimer.h>
> > +
> > +#include "WatchDog.h"
> > +
> 
> STATIC please, and drop the redundant initializer.
OK, will correct this.
> 
> > +EFI_EVENT  EfiExitBootServicesEvent = (EFI_EVENT)NULL;
> > +
> 
> STATIC
> 
> In general, any function that is not exported from the driver should be
> STATIC, even if it is referenced from a protocol struct
OK, will correct in all patches.
> 
> > +UINT16
> > +EFIAPI
> > +WdogRead (
> > +  IN  UINTN     Address
> > +  )
> > +{
> > +  if (FixedPcdGetBool(PcdWdogBigEndian)) {
> 
> In general, but I will mention it here: every function call or macro invocation
> requires a space before (
Will correct in all patches.
> 
> > +    return BeMmioRead16(Address);
> > +  } else {
> > +    return MmioRead16(Address);
> > +  }
> > +}
> > +
> > +UINT16
> > +EFIAPI
> > +WdogWrite (
> > +  IN  UINTN     Address,
> > +  IN  UINT16    Value
> > +  )
> > +{
> > +  if (FixedPcdGetBool(PcdWdogBigEndian)) {
> > +    return BeMmioWrite16(Address, Value);
> > +  } else {
> > +    return MmioWrite16(Address, Value);
> > +  }
> > +}
> > +
> > +STATIC
> > +VOID
> > +WdogPing (
> > +  VOID
> > +  )
> > +{
> > +  WdogWrite(PcdGet64(PcdWdog1BaseAddr) + WDOG_WSR_OFFSET,
> > +                     WDOG_SERVICE_SEQ1);
> > +  WdogWrite(PcdGet64(PcdWdog1BaseAddr) + WDOG_WSR_OFFSET,
> > +                     WDOG_SERVICE_SEQ2); }
> > +
> > +/**
> > +  Stop the Wdog watchdog timer from counting down.
> > +**/
> > +STATIC
> > +VOID
> > +WdogStop (
> > +  VOID
> > +  )
> > +{
> > +  //  Watchdog cannot be disabled by software once started.
> > +  // At best, we can keep pinging the watchdog
> > +  WdogPing();
> 
> How is this supposed to work when entering the OS?

Watchdog controller of LS1043 SoC cannot be stopped once enabled.
When linux boots , then on receiving ExitBootServices event, we are calling
WdogStop and here, we are pinging the watchdog i.e. reloading the watchdog
with the maximum value (128 seconds), this much time is sufficient for OS to boot.

> 
> > +}
> > +
> > +/**
> > +  Starts the Wdog 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
> > +WdogStart (
> > +  VOID
> > +  )
> > +{
> > +  /* Reload the timeout value */
> > +  WdogPing();
> > +}
> > +
> > +/**
> > +    On exiting boot services we must make sure the Wdog Watchdog Timer
> > +    is stopped.
> > +**/
> > +VOID
> > +EFIAPI
> > +ExitBootServicesEvent (
> > +  IN EFI_EVENT  Event,
> > +  IN VOID       *Context
> > +  )
> > +{
> > +  WdogStop();
> 
> The comment above says this is impossible.
> 
Please refer above description for this.

> > +}
> > +
> > +/**
> > +  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.
> > +
> > +**/
> > +EFI_STATUS
> > +EFIAPI
> > +WdogRegisterHandler (
> > +  IN CONST 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;
> 
> Doesn't this violate the protocol? Why does it make sense to implement the
> protocol based on this hardware?
We have referred ArmPlatformPkg/Drivers/SP805WatchdogDxe/SP805Watchdog.c
Please suggest any user of this function.
> 
> > +}
> > +
> > +/**
> > +
> > +  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.
> > +
> > +**/
> > +EFI_STATUS
> > +EFIAPI
> > +WdogSetTimerPeriod (
> > +  IN CONST 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
> > +    WdogStop();
> > +    goto EXIT;
> > +  } else {
> > +    // Convert the TimerPeriod (in 100 ns unit) to an equivalent
> > + second value
> > +
> > +    TimerPeriodInSec = DivU64x32(TimerPeriod, NANO_SECOND_BASE);
> > +
> > +    // The registers in the Wdog are only 32 bits
> > +    if(TimerPeriodInSec > WT_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;
> > +      goto EXIT;
> > +    }
> > +
> > +    // set the new timeout value in the WCR
> > +    Val = WdogRead(PcdGet64(PcdWdog1BaseAddr) +
> WDOG_WCR_OFFSET);
> > +    Val &= ~WDOG_WCR_WT;
> > +    // Convert the timeout value from Seconds to timer count
> > +    Val |= ((WD_COUNT(TimerPeriodInSec) & WD_COUNT_MASK) << 8);
> > +    WdogWrite(PcdGet64(PcdWdog1BaseAddr) + WDOG_WCR_OFFSET,
> Val);
> > +
> > +    // Start the watchdog
> > +    WdogStart();
> > +  }
> > +
> > +  EXIT:
> > +  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.
> > +
> > +**/
> > +EFI_STATUS
> > +EFIAPI
> > +WdogGetTimerPeriod (
> > +  IN CONST 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
> > + ((WdogRead(PcdGet64(PcdWdog1BaseAddr) + WDOG_WCR_OFFSET)
> > +              & WDOG_WCR_WDE) == 0 ) {
> > +    // It is stopped, so return zero.
> > +    ReturnValue = 0;
> > +  } else {
> > +    // Convert the Watchdog ticks into equivalent TimerPeriod second
> value.
> > +    Val = (WdogRead(PcdGet64(PcdWdog1BaseAddr) +
> WDOG_WCR_OFFSET)
> > +            & WDOG_WCR_WT ) >> 8;
> > +    ReturnValue = WD_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.
> > +
> > +**/
> > +EFI_WATCHDOG_TIMER_ARCH_PROTOCOL    gWatchdogTimer = {
> 
> STATIC please
> 
OK
> 
> > +  (EFI_WATCHDOG_TIMER_REGISTER_HANDLER) WdogRegisterHandler,
> > +  (EFI_WATCHDOG_TIMER_SET_TIMER_PERIOD) WdogSetTimerPeriod,
> > +  (EFI_WATCHDOG_TIMER_GET_TIMER_PERIOD) WdogGetTimerPeriod
> 
> The () casts shouldn't be necessary.
> 
OK
> > +};
> > +
> > +/**
> > +  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
> > +WdogInitialize (
> > +  IN EFI_HANDLE         ImageHandle,
> > +  IN EFI_SYSTEM_TABLE   *SystemTable
> > +  )
> > +{
> > +  EFI_STATUS  Status;
> > +  EFI_HANDLE  Handle;
> > +  UINT16      Val;
> > +
> > +
> > +  Val = WdogRead(PcdGet64(PcdWdog1BaseAddr) +
> WDOG_WCR_OFFSET);
> > +
> > +  Val &= ~WDOG_WCR_WT;
> > +
> > +  Val &= ~WDOG_WCR_WDE;
> > +
> > +  Val |= WD_COUNT(WT_MAX_TIME) & WD_COUNT_MASK;
> > +
> > +  WdogWrite(PcdGet64(PcdWdog1BaseAddr) + WDOG_WCR_OFFSET,
> Val);
> > +
> 
> Can you use MmioAndThenOr16 () here?
> 
Yes, will update this.

> > +  Val |= WDOG_WCR_WDE;
> > +
> 
> What is the purpose of this assignment?
> 
Ah.. will correct this.

> > +  //
> > +  // 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,
> > + &EfiExitBootServicesEvent);  if (EFI_ERROR(Status)) {
> > +    Status = EFI_OUT_OF_RESOURCES;
> 
> Why not return Status directly?
> 
Will correct same.

> > +    goto EXIT;
> > +  }
> > +
> > +  // Install the Timer Architectural Protocol onto a new handle
> > + Handle = NULL;  Status = gBS->InstallMultipleProtocolInterfaces(
> > +                  &Handle,
> > +                  &gEfiWatchdogTimerArchProtocolGuid, &gWatchdogTimer,
> > +                  NULL
> > +                  );
> > +  if (EFI_ERROR(Status)) {
> > +    Status = EFI_OUT_OF_RESOURCES;
> 
> Why not return Status directly?
> 
> > +    goto EXIT;
> > +  }
> > +
> > +EXIT:
> > +  if(EFI_ERROR(Status)) {
> > +    // The watchdog failed to initialize
> > +    ASSERT(FALSE);
> > +  }
> > +
> > +  WdogPing();
> > +
> > +  return Status;
> 
> Please close the event and/or uninstall the protocol before returning with an
> error.
> 
OK
> > +}
> > diff --git a/Platform/NXP/Drivers/WatchDog/WatchDog.h
> > b/Platform/NXP/Drivers/WatchDog/WatchDog.h
> > new file mode 100644
> > index 0000000..56ddbde
> > --- /dev/null
> > +++ b/Platform/NXP/Drivers/WatchDog/WatchDog.h
> > @@ -0,0 +1,37 @@
> > +/** WatchDog.h
> > +*
> > +*  Copyright 2017 NXP
> > +*
> > +*  This program and the accompanying materials
> > +*  are licensed and made available under the terms and conditions of
> > +the BSD License
> > +*  which accompanies this distribution.  The full text of the license
> > +may be found at
> > +*
> >
> +https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fop
> e
> > +nsource.org%2Flicenses%2Fbsd-
> license.php&data=02%7C01%7Cmeenakshi.agg
> >
> +arwal%40nxp.com%7Cfe03b552d02b478a923608d52a9150f8%7C686ea1d3bc
> 2b4c6f
> >
> +a92cd99c5c301635%7C0%7C0%7C636461725998922414&sdata=094m%2FkgX
> NuvwXRS
> > +Ewe%2Bke8do3KvWhW7PBAvMTy2O%2Fc4%3D&reserved=0
> > +*
> > +*  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS"
> > +BASIS,
> > +*  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER
> EXPRESS OR IMPLIED.
> > +*
> > +**/
> > +
> > +#ifndef __WATCHDOG_H__
> > +#define __WATCHDOG_H__
> > +
> > +#define WDOG_SIZE           0x1000
> > +#define WDOG_WCR_OFFSET     0
> > +#define WDOG_WSR_OFFSET     2
> > +#define WDOG_WRSR_OFFSET    4
> > +#define WDOG_WICR_OFFSET    6
> > +#define WDOG_WCR_WT         (0xFF << 8)
> > +#define WDOG_WCR_WDE        (1 << 2)
> > +#define WDOG_SERVICE_SEQ1   0x5555
> > +#define WDOG_SERVICE_SEQ2   0xAAAA
> > +#define WDOG_WCR_WDZST      0x1
> > +#define WDOG_WCR_WRE        (1 << 3)  /* -> WDOG Reset Enable */
> > +
> > +#define WT_MAX_TIME         128
> > +#define WD_COUNT(Sec)       (((Sec) * 2 - 1) << 8)
> > +#define WD_COUNT_MASK       0xff00
> > +#define WD_SEC(Cnt)         (((Cnt) + 1) / 2)
> > +
> > +#define NANO_SECOND_BASE    10000000
> > +
> > +#endif //__WATCHDOG_H__
> > diff --git a/Platform/NXP/Drivers/WatchDog/WatchDogDxe.inf
> > b/Platform/NXP/Drivers/WatchDog/WatchDogDxe.inf
> > new file mode 100644
> > index 0000000..4cf7d84
> > --- /dev/null
> > +++ b/Platform/NXP/Drivers/WatchDog/WatchDogDxe.inf
> > @@ -0,0 +1,47 @@
> > +#  WatchDog.inf
> > +#
> > +#  Component description file for  WatchDog module # #  Copyright
> > +2017 NXP # #  This program and the accompanying materials #  are
> > +licensed and made available under the terms and conditions of the BSD
> > +License #  which accompanies this distribution.  The full text of the
> > +license may be found at #
> >
> +https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fop
> e
> > +nsource.org%2Flicenses%2Fbsd-
> license.php&data=02%7C01%7Cmeenakshi.agg
> >
> +arwal%40nxp.com%7Cfe03b552d02b478a923608d52a9150f8%7C686ea1d3bc
> 2b4c6f
> >
> +a92cd99c5c301635%7C0%7C0%7C636461725998922414&sdata=094m%2FkgX
> NuvwXRS
> > +Ewe%2Bke8do3KvWhW7PBAvMTy2O%2Fc4%3D&reserved=0
> > +#
> > +#  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS"
> > +BASIS, #  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND,
> EITHER EXPRESS OR IMPLIED.
> > +#
> > +#
> > +
> > +[Defines]
> > +  INF_VERSION                    = 0x00010005
> 
> 0x0001001A
> 
OK
> > +  BASE_NAME                      = WatchDogDxe
> > +  FILE_GUID                      = ebd705fb-fa92-46a7-b32b-7f566d944614
> 
> Please don't reuse the SP805WatchdogDxe GUID
> 
OK
> > +  MODULE_TYPE                    = DXE_DRIVER
> > +  VERSION_STRING                 = 1.0
> > +  ENTRY_POINT                    = WdogInitialize
> > +
> > +[Sources.common]
> > +  WatchDog.c
> > +
> > +[Packages]
> > +  MdePkg/MdePkg.dec
> > +  edk2-platforms/Platform/NXP/NxpQoriqLs.dec
> > +
> > +[LibraryClasses]
> > +  BaseLib
> > +  BeIoLib
> > +  PcdLib
> > +  UefiBootServicesTableLib
> > +  UefiDriverEntryPoint
> > +
> > +[Pcd]
> > +  gNxpQoriqLsTokenSpaceGuid.PcdWdog1BaseAddr
> > +  gNxpQoriqLsTokenSpaceGuid.PcdWdogBigEndian
> > +
> > +[Protocols]
> > +  gEfiWatchdogTimerArchProtocolGuid
> > +
> > +[Depex]
> > +  TRUE
> > --
> > 1.9.1
> >