All of lore.kernel.org
 help / color / mirror / Atom feed
From: Guenter Roeck <linux@roeck-us.net>
To: "Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>,
	"Wim Van Sebroeck" <wim@linux-watchdog.org>,
	"Rob Herring" <robh+dt@kernel.org>,
	"Frank Rowand" <frowand.list@gmail.com>
Cc: linux-watchdog@vger.kernel.org, devicetree@vger.kernel.org,
	kernel@pengutronix.de
Subject: Re: [RFC] Using a watchdog as system reset
Date: Tue, 6 Oct 2020 04:56:15 -0700	[thread overview]
Message-ID: <460aa962-9da5-6e1e-b5db-3f9f1d78110a@roeck-us.net> (raw)
In-Reply-To: <20201006102949.dbw6b2mrgt2ltgpw@pengutronix.de>


[-- Attachment #1.1: Type: text/plain, Size: 2903 bytes --]

On 10/6/20 3:29 AM, Uwe Kleine-König wrote:
> Hello,
> 
> I have an i.MX25 system here with an external watchdog (using the
> gpio_wdt driver). So the internal watchdog (imx2_wdt) is unused.
> 
> The problem with the unused imx2_wdt is that this usually provides the
> restart handler and now a reboot ends with
> 
> 	reboot: Restarting system
> 	Reboot failed -- System halted
> 
> until eventually the watchdog bites and resets the machine.
> 
> I imagine that this is a common enough issue to warrant a generic
> solution. My suggestion is to formalize and implement something like:
> 
> 	watchdog {
> 		compatible = "linux,wdt-gpio";
> 		...
> 		provide-system-reset;
> 	}
> 
> with the sematic of: "This is the dedicated mechanism to reset this
> machine."
> 

Some systems have more than one means to reset it, which is why
restart handlers have a priority. This in turn suggests that we should
maybe have a means to set that priority dynamically for the imx2_wdt driver
(or for watchdog drivers in general) instead of having it fixed at 128.
That would also solve your problem, assuming there is a different
(currently lower priority) means to reset the hardware in your system.

Alternatively, can't you just blacklist the imx2-wdt driver ?

> (OK, I could enable the imx2_wdt driver and make sure with some udev
> magic that the gpio watchdog is the one being fed by userspace. But
> having two watchdogs fills me with some trepidation.)
> 

Hmm - that suggests that the reset may fail  because the reset code
in imx2_wdt doesn't work, not because it isn't wired.
If so, the driver code handling the reset may be buggy or incomplete.
Have you tried setting (or not setting) the fsl,ext-reset-output
property ?

> Having said that, I wonder what the typical restart callback does
> different from setting the timeout to a minimal value, start it and then
> maybe call delay() to not return until the watchdog triggers. At least
> that's what I would do for a watchdog that doesn't provide an explicit
> .restart handler but has the provide-system-reset property.

The intent of the callback was to handle situations where the watchdog
hardware also generates the system reset. The secondary use was for systems
which don't have a means to reset the system other than what you describe
above.

Guenter

> 
> In my eyes this is somewhat of a hardware property, but I can imagine
> that others don't agree and argue this shouldn't go into the device
> tree. @Rob: What is your position here?
> 
> Does this sound sane? Do we also need a property like
> "no-provide-system-reset" to better maintain backward compatibility?
> (which then would result in not registering the watchdog as reset
> trigger even if the driver provides a .restart.)
> Does someone know a better name for the property?
> 
> Best regards
> Uwe
> 



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

  reply	other threads:[~2020-10-06 11:56 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-10-06 10:29 [RFC] Using a watchdog as system reset Uwe Kleine-König
2020-10-06 11:56 ` Guenter Roeck [this message]
2020-10-06 14:29   ` Guenter Roeck
2020-10-06 18:41     ` Uwe Kleine-König
2020-10-06 21:04       ` Guenter Roeck
2020-10-07  7:12         ` Uwe Kleine-König
2020-10-07  7:25           ` Ahmad Fatoum
2020-10-07  7:32             ` Ahmad Fatoum
2020-10-07 10:18             ` dt-binding to define default watchdog and machine reset (Was: Re: [RFC] Using a watchdog as system reset) Uwe Kleine-König
2020-10-07 11:04               ` Guenter Roeck
2020-10-07 11:35                 ` Ahmad Fatoum

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=460aa962-9da5-6e1e-b5db-3f9f1d78110a@roeck-us.net \
    --to=linux@roeck-us.net \
    --cc=devicetree@vger.kernel.org \
    --cc=frowand.list@gmail.com \
    --cc=kernel@pengutronix.de \
    --cc=linux-watchdog@vger.kernel.org \
    --cc=robh+dt@kernel.org \
    --cc=u.kleine-koenig@pengutronix.de \
    --cc=wim@linux-watchdog.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 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.