All of lore.kernel.org
 help / color / mirror / Atom feed
From: Guenter Roeck <linux@roeck-us.net>
To: Chris Packham <Chris.Packham@alliedtelesis.co.nz>
Cc: "wim@iguana.be" <wim@iguana.be>,
	"gregory.clement@free-electrons.com" 
	<gregory.clement@free-electrons.com>,
	"devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
	"linux-watchdog@vger.kernel.org" <linux-watchdog@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 2/3] watchdog: orion: don't enable rstout if an interrupt is configured
Date: Wed, 27 Feb 2019 15:07:07 -0800	[thread overview]
Message-ID: <20190227230707.GA28635@roeck-us.net> (raw)
In-Reply-To: <636717314b294a5f9a4ed1ffa44625c4@svr-chch-ex1.atlnz.lc>

On Wed, Feb 27, 2019 at 08:59:07PM +0000, Chris Packham wrote:
> Hijacking an old thread,
> 
> On 11/10/17 5:30 PM, Chris Packham wrote:
> > On 11/10/17 16:42, Guenter Roeck wrote:
> >> On 10/10/2017 07:29 PM, Chris Packham wrote:
> >>> The orion_wdt_irq invokes panic() so we are going to reset the CPU
> >>> regardless.  By not setting this bit we get a chance to gather debug
> >>> from the panic output before the system is reset.
> >>>
> >>> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
> >>
> >> Unless I am missing something, this assumes that the interrupt is
> >> handled, ie that the system is not stuck with interrupts disabled.
> >> This makes the watchdog less reliable. This added verbosity comes
> >> at a significant cost. I'd like to get input from others if this
> >> is acceptable.
> >>
> >> That would be different if there was a means to configure a pretimeout,
> >> ie a means to tell the system to generate an irq first, followed by a
> >> hard reset if the interrupt is not served. that does not seem to be
> >> the case here, though.
> >>
> > 
> > As far as I can tell there is no pretimeout capability in the orion
> > /armada WDT. I have got a work-in-progress patch that enables the RSTOUT
> > in the interrupt handler which some-what mitigates your concern but
> > still requires the interrupt to be handled at least once.
> > 
> > Another option would be to use one of the spare global timers to provide
> > the interrupt-driven aspect.
> 
> So as Guenter rightly pointed out my original patch meant that we'd hang 
> if we got stuck in a loop with interrupts disabled. Shortly after that 
> that's exactly what happened on my test setup.
> 
> I've now been able to follow-up on the idea of using one of the spare 
> global timers as a pretimeout indication and it's looking pretty 
> promising. The patch is below (beware MUA wrapping). I'll submit it 
> properly but I'm wondering if I should add the timer1 based watchdog as 
> a separate watchdog device or like I have below roll it into the 
> existing watchdog device.
> 

Why not use a pretimeout ? Isn't this exactly what you are doing,
with a fixed pretimeout of timeout / 2 ?

Guenter

  reply	other threads:[~2019-02-27 23:07 UTC|newest]

Thread overview: 35+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-10-11  2:29 [PATCH 0/3] getting more output from orion_wdt Chris Packham
2017-10-11  2:29 ` Chris Packham
2017-10-11  2:29 ` [PATCH 1/3] watchdog: orion: fix typo Chris Packham
2017-10-11  3:35   ` Guenter Roeck
2017-10-11  2:29 ` [PATCH 2/3] watchdog: orion: don't enable rstout if an interrupt is configured Chris Packham
2017-10-11  3:41   ` Guenter Roeck
2017-10-11  3:41     ` Guenter Roeck
2017-10-11  4:30     ` Chris Packham
2017-10-11  4:30       ` Chris Packham
2019-02-27 20:59       ` Chris Packham
2019-02-27 23:07         ` Guenter Roeck [this message]
2019-03-04 22:51           ` [PATCH 0/2] watchdog: orion_wdt: add pretimeout support Chris Packham
2019-03-04 22:51             ` Chris Packham
2019-03-04 22:51             ` [PATCH 1/2] watchdog: orion_wdt: remove orion_wdt_set_timeout Chris Packham
2019-03-04 22:51               ` Chris Packham
2019-03-05 17:17               ` Guenter Roeck
2019-03-05 17:17                 ` Guenter Roeck
2019-03-04 22:51             ` [PATCH 2/2] watchdog: orion_wdt: use timer1 as a pretimeout Chris Packham
2019-03-04 22:51               ` Chris Packham
2019-03-05  0:57               ` Andrew Lunn
2019-03-05  0:57                 ` Andrew Lunn
2019-03-05  1:26                 ` Chris Packham
2019-03-05  1:26                   ` Chris Packham
2019-03-05  2:09                   ` Andrew Lunn
2019-03-05  2:09                     ` Andrew Lunn
2019-03-05  2:09                     ` Andrew Lunn
2019-03-05 17:27                   ` Guenter Roeck
2019-03-05 17:27                     ` Guenter Roeck
2017-10-11  2:29 ` [PATCH 3/3] ARM: mvebu: dts: connect interrupt for WD on armada-38x Chris Packham
2017-10-11  2:29   ` Chris Packham
2017-10-12 14:16   ` Gregory CLEMENT
2017-10-12 14:16     ` Gregory CLEMENT
2017-10-12 14:16     ` Gregory CLEMENT
2017-10-12 20:12     ` Chris Packham
2017-10-12 20:12       ` Chris Packham

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=20190227230707.GA28635@roeck-us.net \
    --to=linux@roeck-us.net \
    --cc=Chris.Packham@alliedtelesis.co.nz \
    --cc=devicetree@vger.kernel.org \
    --cc=gregory.clement@free-electrons.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-watchdog@vger.kernel.org \
    --cc=wim@iguana.be \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.