On 05.12.2014 16:12, Boris Brezillon wrote:
Hi Timo,
On Fri, 05 Dec 2014 14:57:05 +0200
Timo Kokkonen <timo.kokkonen@xxxxxxxxxx> wrote:
On 27.11.2014 21:00, Boris Brezillon wrote:
Hi Timo,
Sorry for the late reply.
On Fri, 21 Nov 2014 14:23:30 +0200
Timo Kokkonen <timo.kokkonen@xxxxxxxxxx> wrote:
By default the driver will start a kernel timer which keeps on kicking
the watchdog HW until user space has opened the watchdog
device. Usually this is desirable as the watchdog HW is running by
default and the user space may not have any watchdog daemon running at
all.
However, on production systems it may be mandatory that also early
crashes and lockups will lead to a watchdog reset, even if they happen
before the user space has opened the watchdog device.
To resolve the issue, add a new device tree property
"enable-early-reset" which will prevent the kernel timer from pinging
the watchdog HW on behalf of user space. The default is still to use
kernel timer, but more strict behavior can be enabled via the device
tree property.
Signed-off-by: Timo Kokkonen <timo.kokkonen@xxxxxxxxxx>
---
Documentation/devicetree/bindings/watchdog/atmel-wdt.txt | 4 ++++
drivers/watchdog/at91sam9_wdt.c | 6 +++++-
2 files changed, 9 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/watchdog/atmel-wdt.txt b/Documentation/devicetree/bindings/watchdog/atmel-wdt.txt
index f90e294..a0b7b75 100644
--- a/Documentation/devicetree/bindings/watchdog/atmel-wdt.txt
+++ b/Documentation/devicetree/bindings/watchdog/atmel-wdt.txt
@@ -28,6 +28,10 @@ Optional properties:
entering idle state.
- atmel,dbg-halt : Should be present if you want to stop the watchdog when
entering debug state.
+- enable-early-reset : Should be present if you want to let the
+ watchdog timer to expire even before user space has opened the
+ device. If not set, a kernel timer will keep on pinging the
+ watchdog until it is opened.
If you want to make this property generic, maybe you should document it
in a generic binding doc
(Documentation/devicetree/bindings/watchdog/watchdog.txt ?).
Once you're at it, maybe you could document the generic timeout-sec
property in this file.
Moreover, you might want to parse this property in watchdog_core.c and
store the information in the watchdog_device struct.
I gave a little thought about this today and we could maybe have it like
watchdog_init_timeout() is today. But I can't really think of any
generic handling for this property, anything else except storing the
parsed value to a variable in watchdog_device struct. Everything else is
HW specific, except how we read the variable.. If we had some logic or
checking for this variable (other than to check it is not negative)
maybe then it would make sense.
Okay, I'm fine with keeping this DT property parsing out of the core.
Ok, good.
So I could write a patch to document generic watchdog properties in
Documentation/devicetree/bindings/watchdog/watchdog.txt (anything else
generic that would go there except timeout-sec and enable-early-reset?)
I'm not sure you and Guenter agreed on the 'early-keepalive-sec'
property (and this one won't be used in atmel driver since you can't
change the timeout once set), so I'd say the timeout-sec and
enable-early-reset are the only common properties for now.
Actually it can be done in Atmel driver as well. It already has a timer
that keeps on pinging the watchdog until user space opens it. We just
modify the code so that it keeps on pinging the watchdog until
early-keepalive-sec has expired. No need to change the HW timeouts at
all. And I rather have it implemented that way anyway, the 16 second
timeout is a little tight if there happens to be a lot of stuff already
in the early user space.
Yeah, I didn't get anything back from Guenter about my
early-keepalive-sec property proposal. Frankly, I already forgot about
it my self already. I guess I'll get back to you with v3 set once I get
time for it. Probably easiest to continue from there.
Thanks for the feedback!
-Timo
and change this patch so that it doesn't document anything in
atmel-wdt.txt as we already have it documented in generic watchdog.txt.
Sounds good?
Yes, sounds good to me.
Regards,
Boris
--
To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
