On 02/18/2015 06:17 AM, Boris Brezillon wrote:
Hi Guenter,
On Wed, 18 Feb 2015 05:59:01 -0800
Guenter Roeck <linux@xxxxxxxxxxxx> wrote:
On 02/18/2015 04:57 AM, Timo Kokkonen 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
"early-timeout-sec" which will let the kernel timer to ping the
watchdog HW only as long as the specified timeout permits. 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/watchdog.txt | 7 +++++++
drivers/watchdog/at91sam9_wdt.c | 9 ++++++++-
2 files changed, 15 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/watchdog/watchdog.txt b/Documentation/devicetree/bindings/watchdog/watchdog.txt
index 7e3686c..32647cf 100644
--- a/Documentation/devicetree/bindings/watchdog/watchdog.txt
+++ b/Documentation/devicetree/bindings/watchdog/watchdog.txt
@@ -4,9 +4,16 @@ using these definitions.
Optional properties:
- timeout-sec: Contains the watchdog timeout in seconds.
+- early-timeout-sec: If present, specifies a timeout value in seconds
+ that the driver keeps on ticking the watchdog HW on behalf of user
+ space. Once this timeout expires watchdog is left to expire in
+ timeout-sec seconds. If this propery is set to zero, watchdog is
+ started (or left running) so that a reset occurs in timeout-sec
+ since the watchdog was started.
Example:
watchdog {
timeout-sec = <60>;
+ early-timeout-sec = <120>;
That is not a generic property as you defined it; if so,
it would have to be implemented in the watchdog core code,
not in the at91 code. You'll have to document it in the bindings
description for at91sam9_wdt.
Then, if this is a controller specific property, it should be defined
with the 'atmel,' prefix.
We're kind of looping here: the initial discussion was "is there a need
for this property to be a generic one ?", and now you're saying no,
while you previously left the door opened.
Tomi is proposing a generic approach, as you asked him to. I agree that
parsing the property in core code and making its value part of the
generic watchdog struct makes sense (that's what I proposed to Tomi a
few weeks ago).
Hmm ... the problem here is that the property description creates the
assumption or expectation that the property is used if defined,
which is not the case.
I am not sure how to best resolve this. Maybe a comment in the property
description stating that implementation of is device (driver) dependent ?
After all, that is true for the timeout-sec property as well.
Thanks,
Guenter
--
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
