From: timo.kokkonen@offcode.fi (Timo Kokkonen)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH] at91sam9_wdt: Allow watchdog to reset device at early boot
Date: Sat, 06 Dec 2014 12:11:33 +0200 [thread overview]
Message-ID: <5482D655.8000700@offcode.fi> (raw)
In-Reply-To: <20141205213946.GA2802@roeck-us.net>
On 05.12.2014 23:39, Guenter Roeck wrote:
> On Fri, Dec 05, 2014 at 10:32:48PM +0200, Timo Kokkonen wrote:
>> On 05.12.2014 21:02, Guenter Roeck wrote:
>>> Not sure about how to name enable-early-reset. I'd prefer to have something
>>> generic, even if only implemented in a single driver for now, but I don't
>>> really know right now what that might/should look like. Maybe just
>>> "enable-early" to indicate that the watchdog should be enabled during init ?
>>
>> Do we need the enable-early or such property at all? Just leave
>> early-timeout-sec to zero and then let it behave just like
>> enable-early would do?
>>
>
> Problem is that the possible conditions are all over the place
> for "early" watchdog handling.
>
> - Disable watchdog
> - Enable watchdog (or keep it enabled), and keep it alive
> until user space kicks in (ie possibly forever)
> - Enable watchdog or keep it enabled, and keep it alive
> for a specified period of time.
> - Keep watchdog enabled if it is already enabled, otherwise
> keep it disabled.
>
> There are probably more conditions which I don't recall right now.
> Which of those conditions would you address with "early-timeout = <0>;" ?
> "enable watchdog early and keep it alive until user space kicks in",
> or "keep watchdog enabled if already running, and set specified early
> timeout" ? One could argue either way which one of the two meanings
> it should be.
Okay, let me elaborate my point of view a bit.
The use case we are concerned about is that we have a device that we
rather not let freeze up at any point. This is what we use the watchdog
for. The only missing corner case right now is the point where kernel
driver initializes the watchdog hardware and pings it on behalf of user
space until a watchdog daemon opens it and starts kicking. This is kind
of bad as kernel might lock up or user space might crash before we get
to the point where the daemon starts taking care of petting the
watchdog. So this is what we are trying to fix.
Right. Some other hardware behave differently to the one in Atmel. They
might have watchdog stopped by bootloader or it might not be running at
all until someone starts it. What do do with such case? If we are still
concerned about the same use case I described above, I would say the
reasonable thing to do is to make sure the watchdog is started as early
as possible and not stopped at any point at all, if possible. If it
needs to be explicitly enabled, bootloader should do it. If it didn't do
it, then kernel should do it.
Now that I think of it, what we really are interested in is to defer
starting of the watchdog to give user space more time to start up. In
Atmel HW it's more tricky as the driver can't be stopped. And in other
hardware we could simply disable it altogether until we enable it after
specific timeout, but we might crash before the timeout expires, in
which case we would not get a chance to enable it. So the right thing to
do is to enable the watchdog as early as possible, kick it on behalf of
the user space until the timeout expires. Special case would be when the
timeout is zero, when we just ensure watchdog is running but we don't do
anything to prolong the first expiration.
I can't think of any other use case someone would be interested in, but
I'm positive that there are plenty of products on the market right now
that have the requirement for race free guarantee that watchdog never stops.
So given the conditions you listed, what I think is really important to
fix is "Enable watchdog or keep it enabled, and keep it alive for a
specified period of time". The only other choice we have right now is
"Disable watchdog and let user space enabled it later, if ever". Yeah,
maybe we could cover those other use cases too. Maybe someone is using
bootloader to decide what to do with watchdog and kernel should somehow
respect that. I don't know if that makes sense or if it would be
reasonable assumption..
Any more thoughts?
Thanks,
-Timo
next prev parent reply other threads:[~2014-12-06 10:11 UTC|newest]
Thread overview: 55+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-10-23 10:40 [PATCH] at91sam9_wdt: Allow watchdog to reset device at early boot Timo Kokkonen
2014-11-12 8:20 ` Timo Kokkonen
2014-11-13 9:12 ` Nicolas Ferre
2014-11-14 8:40 ` Timo Kokkonen
2014-11-21 12:23 ` Timo Kokkonen
2014-11-27 6:53 ` Timo Kokkonen
2014-11-27 9:22 ` Nicolas Ferre
2014-11-27 17:23 ` Guenter Roeck
2014-11-27 19:06 ` Boris Brezillon
2014-11-27 19:31 ` Guenter Roeck
2014-11-28 0:30 ` Alexandre Belloni
2014-11-28 6:40 ` Timo Kokkonen
2014-11-27 19:00 ` Boris Brezillon
2014-11-28 6:42 ` Timo Kokkonen
2014-12-05 12:57 ` Timo Kokkonen
2014-12-05 14:12 ` Boris Brezillon
2014-12-05 18:42 ` Timo Kokkonen
2014-12-05 19:02 ` Guenter Roeck
2014-12-05 20:32 ` Timo Kokkonen
2014-12-05 21:39 ` Guenter Roeck
2014-12-06 10:11 ` Timo Kokkonen [this message]
2015-01-13 14:53 ` Guenter Roeck
2015-01-14 6:09 ` Timo Kokkonen
2015-02-18 12:57 ` [PATCHv3 0/2] watchdog: Introduce "early-timeout-sec" property Timo Kokkonen
2015-02-18 12:57 ` [PATCH 1/2] devicetree: Document generic watchdog properties Timo Kokkonen
2015-02-18 12:57 ` [PATCH 2/2] at91sam9_wdt: Allow watchdog to reset device at early boot Timo Kokkonen
2015-02-18 13:21 ` Boris Brezillon
2015-02-18 13:59 ` Guenter Roeck
2015-02-18 14:17 ` Boris Brezillon
2015-02-18 14:50 ` Guenter Roeck
2015-02-18 16:00 ` Alexandre Belloni
2015-02-18 17:50 ` Guenter Roeck
2015-02-18 20:21 ` Boris Brezillon
2015-02-19 6:02 ` Timo Kokkonen
2015-02-18 21:11 ` Rob Herring
2015-02-19 6:14 ` Timo Kokkonen
2015-02-20 14:06 ` Rob Herring
2015-02-20 16:28 ` Guenter Roeck
2015-02-20 19:43 ` Boris Brezillon
2015-02-20 20:04 ` Guenter Roeck
2015-02-20 7:48 ` Jean-Christophe PLAGNIOL-VILLARD
2015-02-20 7:51 ` Boris Brezillon
2015-02-20 16:33 ` Jean-Christophe PLAGNIOL-VILLARD
2015-02-20 17:16 ` Boris Brezillon
2015-02-20 18:06 ` Guenter Roeck
2015-02-23 7:29 ` Timo Kokkonen
2015-02-23 8:51 ` Boris Brezillon
2015-02-23 9:11 ` Timo Kokkonen
2015-02-23 16:19 ` Guenter Roeck
2015-02-23 17:10 ` Rob Herring
2015-02-23 17:43 ` Guenter Roeck
2015-02-20 8:00 ` Timo Kokkonen
2015-02-20 16:09 ` Guenter Roeck
2015-02-18 13:16 ` [PATCHv3 0/2] watchdog: Introduce "early-timeout-sec" property Boris Brezillon
2015-02-18 13:51 ` Timo Kokkonen
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=5482D655.8000700@offcode.fi \
--to=timo.kokkonen@offcode.fi \
--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).