[PATCH] at91sam9_wdt: Allow watchdog to reset device at early boot

[timo.kokkonen@offcode.fi (Timo Kokkonen)]

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@offcode.fi> 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@offcode.fi>
>> ---
>>   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.

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?) 
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?

Thanks,
-Timo

> Apart from that, I'm not opposed to the approach, and I'm fine with the
> current implementation.
>
> Regards,
>
> Boris
>