[PATCH] watchdog: add CONFIG_WATCHDOG_NOWAYOUT support to PowerPC Book-E watchdog driver

[Josh Boyer]

On Fri, Dec 03, 2010 at 12:22:30PM -0600, Timur Tabi wrote:
>Josh Boyer wrote:
>> I'm confused why you can't use booke_wdt_enabled for the purposes of the
>> device having been opened.  It seems the use of the wdt_is_active
>> basically duplicates this functionalit (and oddly with the bit
>> manipulation instead of just atomic_inc/dec).
>
>Because the watchdog can be enabled even when the driver is not open.
>booke_wdt_enabled is also initialized in setup_32.c.  So booke_wdt_enabled
>represents the watchdog hardwre, whereas wdt_is_active represents the open
>condition of /dev/watchdog.

Doh!  The big fat comment right above the variable would have told me
that had I bothered to read it.

(As an aside, the "For E500 cpus.." part of that comment also applies to
4xx.)

>However, now that I think about it, maybe that just causes confusion.  If the
>watchdog is already running because of a command-line parameter, should we
>prevent /dev/watchdog from ever being opened?  If you're okay with that, then I
>can combine the two variables.

No, you definitely want to allow something to open it.  If it's running
and nothing opens the device and calls write, then you have no way of
preventing the watchdog from just resetting your board every time.

I had a driver for a different mechanism that would do the equivalent of
booke_wdt_ping internally to the kernel until somthing opened the
device.  That way you could have the watchdog active during bootup and
allow userspace to takeover control when it was ready.  Then NOWAYOUT
dictated what happened if the userspace process exited or died (or
otherwise closed the device).  I'm not sure we want to do that for this
hardware, but it does illustrate the main use of a watchdog on a number
of boards.

I guess I don't really care either way if multiple processes have the
device open, as long as none of them can "disable" the watchdog if
NOWAYOUT is set.

>
>> If you were to keep this variable instead of just using
>> booke_wdt_enabled, wouldn't it be more correct to have the clear_bit
>> only done inside the #ifndef?  The timer is very much still active if
>> NOWAYOUT is set...
>
>In this case, yes.

ok.

josh