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

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

On 05.12.2014 16:12, Boris Brezillon wrote:
> Hi Timo,
>
> On Fri, 05 Dec 2014 14:57:05 +0200
> Timo Kokkonen <timo.kokkonen@offcode.fi> 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@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.
>
> 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
>