public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: Leif Lindholm <leif.lindholm@linaro.org>
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
Date: Mon, 17 Dec 2018 12:55:19 +0000	[thread overview]
Message-ID: <CAF7UmSz7EU+82nB-yY8BdoZbCXLZE33AY=Jpjg6gEm67kDjWVw@mail.gmail.com> (raw)

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


             reply	other threads:[~2018-12-17 12:55 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-12-17 12:55 Leif Lindholm [this message]
2018-12-18  5:20 ` SP805 driver Udit Kumar
2018-12-18  9:26   ` Leif Lindholm
2018-12-18 12:30     ` Udit Kumar
2018-12-18 12:49       ` Leif Lindholm
2018-12-18 12:57         ` Udit Kumar
2018-12-18 13:16           ` Leif Lindholm
2018-12-18  7:13 ` Thomas Abraham
2018-12-20 15:00 ` Ming Huang

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAF7UmSz7EU+82nB-yY8BdoZbCXLZE33AY=Jpjg6gEm67kDjWVw@mail.gmail.com' \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox