Re: SP805 driver

[Udit Kumar <udit.kumar@nxp.com>]

Thanks for note Leif,

I am trying to understand this behavior  of specification.

>"If no handler has been registered, or the registered handler returns, then the system will be reset by calling the Runtime Service ResetSystem()."
I believe,  this handler needs to be called by wdt driver after wdt is expired. Meaning , we want wdt to work without resetting the system.
Therefore a mask is needed with wdt, which will prevent to reset the system.   I see it working, at least for SP805, because of PMU mask bits.

Also ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c has this limitation.
I haven’t read spec of this wdt.  I hope there should a mask around.

Coming back to hardware, which does not have mask around wdt, how to implement this feature.


Regards
Udit


From: Leif Lindholm <leif.lindholm@linaro.org>
Sent: Monday, December 17, 2018 6:25 PM
To: Sami Mujawar <sami.mujawar@arm.com>; Thomas Panakamattam Abraham <thomas.abraham@arm.com>
Cc: Meenakshi Aggarwal <meenakshi.aggarwal@nxp.com>; Udit Kumar <udit.kumar@nxp.com>; Ard Biesheuvel <ard.biesheuvel@linaro.org>; edk2-devel (edk2-devel@lists.01.org) <edk2-devel@lists.01.org>; Ming Huang <ming.huang@linaro.org>; Heyi Guo <heyi.guo@linaro.org>; Matteo Carlini <Matteo.Carlini@arm.com>; Nariman Poushin <nariman.poushin@linaro.org>
Subject: SP805 driver

Hi Sami, Thomas, (and others on cc)

NXP are upstreaming a set containing an implementation of
EFI_WATCHDOG_TIMER_ARCH_PROTOCOL using a hardware watchdog as backing.
This idea comes from the SP805 driver ArmPlatformPkg/Drivers/SP805WatchdogDxe, which does the same.

The problem is that this is a horrible idea. The point of the EFI_WATCHDOG_TIMER_ARCH_PROTOCOL is that it lets you schedule software events when the watchdog timer expires. However, the SP805 driver does not let you do this:

EFI_STATUS
EFIAPI
SP805RegisterHandler (
  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;
}

From section 12.14 of the PI 1.6 spec:
"If no handler has been registered, or the registered handler returns, then the system will be reset by calling the Runtime Service ResetSystem()."
Blatantly, any driver that does the above (and initializes hardware that will reset the system without software control) will violate this.

Meanwhile, the only two ARM platforms that include this driver are TC2 and FVP.

Now, NXP are not at fault for following examples given to them in the reference ARM driver section - but can we please keep further people from making the same mistake?
So my question to {Sami|Thomas} is - can we nuke this driver, and if so, can you provide the patches to remove it from edk2 and the resulting ones needed for edk2-platforms?

I have no issues with reintroducing a fixed SP805 driver in the future that does not claim to conform to the above protocol.

Finally - both the D03 and D05 platform .dsc files, as well as the Hisilicon ArmPlatformLib, contain references to it. (Ming/Heyi - please provide a patch.)

/
    Leif