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. > > 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. > 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 -- Boris Brezillon, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com -- 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
