Re: [PATCH edk2-platforms] [PATCH v3 2/9] Platform/NXP : Add support for Watchdog driver

["Gao, Liming" <liming.gao@intel.com>]

Leif:
  I suggest return EFI_UNSUPPORTED for this case. The protocol implementation could return its status besides spec defined status.

Thanks
Liming
> -----Original Message-----
> From: Leif Lindholm [mailto:leif.lindholm@linaro.org]
> Sent: Monday, December 4, 2017 10:36 PM
> To: Kinney, Michael D <michael.d.kinney@intel.com>; Gao, Liming <liming.gao@intel.com>
> Cc: Meenakshi Aggarwal <meenakshi.aggarwal@nxp.com>; ard.biesheuvel@linaro.org; edk2-devel@lists.01.org;
> udit.kumar@nxp.com; v.sethi@nxp.com
> Subject: Re: [PATCH edk2-platforms] [PATCH v3 2/9] Platform/NXP : Add support for Watchdog driver
> 
> Mike, Liming, as MdePkg mainteiners - one question below:
> 
> On Mon, Nov 27, 2017 at 04:21:50PM +0530, Meenakshi Aggarwal wrote:
> > diff --git a/Platform/NXP/Drivers/WatchDog/WatchDog.c b/Platform/NXP/Drivers/WatchDog/WatchDog.c
> > new file mode 100644
> > index 0000000..a9c70ef
> > --- /dev/null
> > +++ b/Platform/NXP/Drivers/WatchDog/WatchDog.c
> > @@ -0,0 +1,421 @@
> 
> ...
> 
> > +/**
> > +  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
> > +WdogRegisterHandler (
> > +  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;
> 
> Michael, Liming - what's your take on this?
> 
> Is EFI_WATCHDOG_TIMER_ARCH_PROTOCOL suitable for use with a pure-hw
> watchdog such as this?
> 
> If so, what would be a suitable return code here?
> EFI_INVALID_PARAMETER does not look ideal.
> 
> /
>     Leif