linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: jgunthorpe@obsidianresearch.com (Jason Gunthorpe)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 01/15] watchdog: orion: Remove unneeded BRIDGE_CAUSE clear
Date: Tue, 27 Aug 2013 15:02:48 -0600	[thread overview]
Message-ID: <20130827210248.GA19625@obsidianresearch.com> (raw)
In-Reply-To: <521D063F.4040404@gmail.com>

On Tue, Aug 27, 2013 at 10:04:15PM +0200, Sebastian Hesselbarth wrote:

> In the current use of watchdog even for non-DT boards, you do not
> have to clear the interrupt cause. It will ultimately lead to a
> reset in any way. Maybe it is not a big deal to remove it now
> even without non-DT replacement.

But that is the likely reason..

AFAIK, to be immune to bootloader left over you must do these steps in
order:
 - Gain control of the WDT timer, so that it doesn't trigger
 - Clear the cause register
 - Enable the reset out function

Which is what orion_wdt_start does today.

If you fiddle with the order you risk creating an errant WDT trigger,
depending on what the bootloader did.

eg having cause asserted and then setting the reset out bit will
reboot the board.

Hoisting the reset out register write into board code now requires
that the bootloader left the WDT subsystem in some kind of sane state,
probably not great..

The cause register clear should be done by the IRQ driver when the
wdt driver attaches to the interrupt, so the order must be:
 - program WDT timer registers, zero base counter, halt counting
 - attach interrupt (clears cause prior to attach)
 - enable reset out function
 - enable WDT timers

You'll need to hook the WDT interrupt (just call panic) to make this
work properly.

Our systems have a delay after the RSTOUT pin is asserted but before
the Armada is reset. This delay is designed to be long enough for an
oops to make it out of the serial port. Thus, I'm using this as the
WDT interrupt:

+static irqreturn_t orion_wdt_handler(int irq, void *arg)
+{
+       console_verbose();
+       pr_crit("Oops: Watchdog Timeout");
+       show_regs(get_irq_regs());
+       panic("Watchdog Timeout");
+       return IRQ_HANDLED;
+}

Which produces a detailed report of what was holding the CPU and
preventing userspace from running. On other hardware you can get the
oops by not enabling RSTOUTn..

Since this is so complex, maybe it is OK to abandon support for non-DT
boards in the WDT driver? Watchdog is not critical functionality, so
nobody should be critically impacted. Boards would have to migrate
to DT to get WDT back?

Jason

  reply	other threads:[~2013-08-27 21:02 UTC|newest]

Thread overview: 35+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-08-27 14:34 [PATCH 00/15] Armada 370/XP watchdog support Ezequiel Garcia
2013-08-27 14:34 ` [PATCH 01/15] watchdog: orion: Remove unneeded BRIDGE_CAUSE clear Ezequiel Garcia
2013-08-27 14:39   ` Thomas Petazzoni
2013-08-27 15:11     ` Ezequiel Garcia
2013-08-27 15:25       ` Jason Cooper
2013-08-27 19:13         ` Ezequiel Garcia
2013-08-27 20:04           ` Sebastian Hesselbarth
2013-08-27 21:02             ` Jason Gunthorpe [this message]
2013-08-27 21:41               ` Jason Cooper
2013-08-27 21:55                 ` Thomas Petazzoni
2013-08-27 22:04                   ` Jason Cooper
2013-08-27 22:11                     ` Jason Gunthorpe
2013-08-28 12:02                       ` Jason Cooper
2013-09-05 16:32                   ` Gregory CLEMENT
2013-08-27 22:17               ` Ezequiel Garcia
2013-08-27 22:25                 ` Jason Gunthorpe
2013-08-28 12:08                   ` Ezequiel Garcia
2013-08-28 16:33                     ` Jason Gunthorpe
2013-10-01 11:55                       ` Jason Cooper
2014-01-17 15:48                         ` Ezequiel Garcia
2013-08-27 14:34 ` [PATCH 02/15] ARM: orion: Assert watchdog RSTOUT enable bit Ezequiel Garcia
2013-08-27 14:34 ` [PATCH 03/15] ARM: mvebu: Add watchdog RSTOUT enable in system-controller init Ezequiel Garcia
2013-08-27 14:34 ` [PATCH 04/15] watchdog: orion: Remove RSTOUT bit enable/disable Ezequiel Garcia
2013-08-27 14:34 ` [PATCH 05/15] watchdog: orion: Allow to build in any Orion platform Ezequiel Garcia
2013-08-27 14:34 ` [PATCH 06/15] watchdog: orion: Introduce an orion_watchdog device structure Ezequiel Garcia
2013-08-27 14:34 ` [PATCH 07/15] watchdog: orion: Introduce per-compatible of_device_id data Ezequiel Garcia
2013-08-27 14:34 ` [PATCH 08/15] watchdog: orion: Add per-compatible clock initialization Ezequiel Garcia
2013-08-27 14:34 ` [PATCH 09/15] watchdog: orion: Add support for Armada 370 and Armada XP SoC Ezequiel Garcia
2013-08-27 14:34 ` [PATCH 10/15] ARM: mvebu: Add RSTOUT cell to system-controller DT node Ezequiel Garcia
2013-08-27 14:34 ` [PATCH 11/15] ARM: mvebu: Enable Armada 370/XP watchdog in the devicetree Ezequiel Garcia
2013-08-27 14:34 ` [PATCH 12/15] watchdog: orion: Rename device-tree binding documentation Ezequiel Garcia
2013-08-27 14:34 ` [PATCH 13/15] watchdog: orion: Add other compatibles to devicetree binding Ezequiel Garcia
2013-08-27 14:34 ` [PATCH 14/15] ARM: mvebu: system-controller: Add second reg cell devicetree specification Ezequiel Garcia
2013-08-27 14:34 ` [PATCH 15/15] ARM: mvebu: Enable watchdog in defconfig Ezequiel Garcia
2013-08-27 15:06 ` [PATCH 00/15] Armada 370/XP watchdog support Jason Cooper

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-all 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=20130827210248.GA19625@obsidianresearch.com \
    --to=jgunthorpe@obsidianresearch.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    /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;
as well as URLs for NNTP newsgroup(s).