Re: SP805 driver

[Ming Huang <ming.huang@linaro.org>]

On 12/17/2018 8:55 PM, Leif Lindholm wrote:
> 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.)

OK.

Thanks.

> 
> /
>     Leif