Further WatchDog Updates

Further WatchDog Updates

[Rob Radez]

Ok, new version of watchdog updates is up at
http://osinvestor.com/bigwatchdog-4.diff

This also includes replacing the three watchdog docs in Documentation/
and slight modification of the watchdog API after discussion with Alan Cox.

Diffstat:
 Documentation/pcwd-watchdog.txt       |  132 -----------
 Documentation/watchdog-api.txt        |  390 ----------------------------------
 Documentation/watchdog.txt            |  113 ---------
 Documentation/watchdog/api.txt        |  139 ++++++++++++
 Documentation/watchdog/howtowrite.txt |   55 ++++
 Documentation/watchdog/status.txt     |  137 +++++++++++
 drivers/char/acquirewdt.c             |  100 +++++---
 drivers/char/advantechwdt.c           |   88 ++++---
 drivers/char/alim7101_wdt.c           |   83 ++++---
 drivers/char/eurotechwdt.c            |   65 +++--
 drivers/char/i810-tco.c               |   68 +++--
 drivers/char/ib700wdt.c               |   87 ++++---
 drivers/char/machzwd.c                |   96 ++++----
 drivers/char/mixcomwd.c               |   24 --
 drivers/char/pcwd.c                   |   26 +-
 drivers/char/sbc60xxwdt.c             |   75 ++++--
 drivers/char/sc1200wdt.c              |   30 --
 drivers/char/sc520_wdt.c              |   55 ++--
 drivers/char/shwdt.c                  |   67 ++---
 drivers/char/softdog.c                |   30 +-
 drivers/char/w83877f_wdt.c            |   52 ++--
 drivers/char/wafer5823wdt.c           |   45 +++
 drivers/char/wdt.c                    |   33 ++
 drivers/char/wdt285.c                 |   26 --
 drivers/char/wdt977.c                 |  137 ++++++++---
 drivers/char/wdt_pci.c                |   23 --
 26 files changed, 1063 insertions(+), 1113 deletions(-)

Please take a look at it, especially the documentation.

Regards,
Rob Radez

Re: Further WatchDog Updates

[Justin Cormack]

> 
> Ok, new version of watchdog updates is up at
> http://osinvestor.com/bigwatchdog-4.diff

> 
> Please take a look at it, especially the documentation.
> 
> Regards,
> Rob Radez

ok for wafer5823wdt, which I wrote:

Timeout is a single byte, so should be in range 1-255 (and should probably be
a byte not an int...), not 1-3600.

All uses of waf_expect_close should be wrapped in
#ifndef CONFIG_WATCHDOG_NOWAYOUT, rather than just haveing it set to zero
if nowayout is not set. Saves a byte and makes it clearer. Though personally
I think the 'V' to close interface shouldnt be there at all. If you dont
want your watchdog to shut down, use no way out. If you do, then dont.

These may apply to other drivers too.

Justin

Re: Further WatchDog Updates

[Roy Sigurd Karlsbakk]

hi

will this be merged into any of the standard pathces or the main tree?

On Sun, 7 Apr 2002, Rob Radez wrote:

> Ok, new version of watchdog updates is up at
> http://osinvestor.com/bigwatchdog-4.diff
> 
> This also includes replacing the three watchdog docs in Documentation/
> and slight modification of the watchdog API after discussion with Alan Cox.
> 
> Diffstat:
>  Documentation/pcwd-watchdog.txt       |  132 -----------
>  Documentation/watchdog-api.txt        |  390 ----------------------------------
>  Documentation/watchdog.txt            |  113 ---------
>  Documentation/watchdog/api.txt        |  139 ++++++++++++
>  Documentation/watchdog/howtowrite.txt |   55 ++++
>  Documentation/watchdog/status.txt     |  137 +++++++++++
>  drivers/char/acquirewdt.c             |  100 +++++---
>  drivers/char/advantechwdt.c           |   88 ++++---
>  drivers/char/alim7101_wdt.c           |   83 ++++---
>  drivers/char/eurotechwdt.c            |   65 +++--
>  drivers/char/i810-tco.c               |   68 +++--
>  drivers/char/ib700wdt.c               |   87 ++++---
>  drivers/char/machzwd.c                |   96 ++++----
>  drivers/char/mixcomwd.c               |   24 --
>  drivers/char/pcwd.c                   |   26 +-
>  drivers/char/sbc60xxwdt.c             |   75 ++++--
>  drivers/char/sc1200wdt.c              |   30 --
>  drivers/char/sc520_wdt.c              |   55 ++--
>  drivers/char/shwdt.c                  |   67 ++---
>  drivers/char/softdog.c                |   30 +-
>  drivers/char/w83877f_wdt.c            |   52 ++--
>  drivers/char/wafer5823wdt.c           |   45 +++
>  drivers/char/wdt.c                    |   33 ++
>  drivers/char/wdt285.c                 |   26 --
>  drivers/char/wdt977.c                 |  137 ++++++++---
>  drivers/char/wdt_pci.c                |   23 --
>  26 files changed, 1063 insertions(+), 1113 deletions(-)
> 
> Please take a look at it, especially the documentation.
> 
> Regards,
> Rob Radez
> 
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
> 

-- 
Roy Sigurd Karlsbakk, Datavaktmester

Computers are like air conditioners.
They stop working when you open Windows.

Re: Further WatchDog Updates

[Rob Radez]

On Mon, 8 Apr 2002, Roy Sigurd Karlsbakk wrote:

> will this be merged into any of the standard pathces or the main tree?

I hope so, if people find it useful and don't have any objections to it.

Regards,
Rob Radez

Re: Further WatchDog Updates

[Corey Minyard]

Rob Radez wrote:

>Ok, new version of watchdog updates is up at
>http://osinvestor.com/bigwatchdog-4.diff
>
Could the timeout be in milliseconds?  A lot of watchdogs have lower 
resolution, and I have written applications that require a lower 
resolution than a second.  Milliseconds is small enough to not cause 
problems, but big enough to give a good range of time.

-Corey

Re: Further WatchDog Updates

[Rob Radez]

On Mon, 8 Apr 2002, Corey Minyard wrote:

> Rob Radez wrote:
>
> >Ok, new version of watchdog updates is up at
> >http://osinvestor.com/bigwatchdog-4.diff
> >
> Could the timeout be in milliseconds?  A lot of watchdogs have lower
> resolution, and I have written applications that require a lower
> resolution than a second.  Milliseconds is small enough to not cause
> problems, but big enough to give a good range of time.

Not in 2.4, and I wonder if that might be too fine-grained for some
drivers which have an upper limit of 255 seconds.  I also wonder if it
would be considered ugly to extend WDIOC_SETOPTIONS to have a
WDIOS_TIMEINMILLI bit.

Regards,
Rob Radez

Re: Further WatchDog Updates

[Zwane Mwaikambo]

-/* This returns the status of the WDO signal, inactive high.
- * returns WDO_ENABLED or WDO_DISABLED
- */
-static inline int sc1200wdt_status(void)
+static int sc1200wdt_start(void);
 {
-	unsigned char ret;
+	sc1200wdt_read_data(WDCF, &reg);
+	/* assert WDO when any of the following interrupts are triggered 
too */
+	reg |= (KBC_IRQ | MSE_IRQ | UART1_IRQ | UART2_IRQ);
+	sc1200wdt_write_data(WDCF, reg);
+	/* set the timeout and get the ball rolling */
+	sc1200wdt_write_data(WDTO, timeout);
+}
 
-	sc1200wdt_read_data(WDST, &ret);
-	return (ret & 0x01);		/* bits 1 - 7 are undefined */
+
+static int sc1200wdt_stop(void)
+{
+	sc1200wdt_write_data(WDTO, 0);
 }

Did you forget return values? Or perhaps just redeclare those...
Also i don't quite understand the new status reporting you're doing, mind 
just explaining it to me a bit? The previous code would tell you wether 
the watchdog is enabled/disabled so you can tell wether the timeout period 
has passed.

Regards,
	Zwane

-- 
http://function.linuxpower.ca
		

Re: Further WatchDog Updates

[Corey Minyard]

Rob Radez wrote:

>On Mon, 8 Apr 2002, Corey Minyard wrote:
>
>>Rob Radez wrote:
>>
>>>Ok, new version of watchdog updates is up at
>>>http://osinvestor.com/bigwatchdog-4.diff
>>>
>>Could the timeout be in milliseconds?  A lot of watchdogs have lower
>>resolution, and I have written applications that require a lower
>>resolution than a second.  Milliseconds is small enough to not cause
>>problems, but big enough to give a good range of time.
>>
>
>Not in 2.4, and I wonder if that might be too fine-grained for some
>drivers which have an upper limit of 255 seconds.  I also wonder if it
>would be considered ugly to extend WDIOC_SETOPTIONS to have a
>WDIOS_TIMEINMILLI bit.
>
>Regards,
>Rob Radez
>
Why is that too fine grained?  You would just set the values from 1000 
to 255000 instead of 1 to 255, and round up.

I have a board that sets the time value in wierd times (like 225ms, 
450ms, 900ms, 1800ms, 3600ms, etc.).  I wouldn't be against the 
WDIOS_TIMEINMILLI option, but milliseconds should be good enough for anyone.

-Corey

Re: Further WatchDog Updates

[Rob Radez]

On Tue, 9 Apr 2002, Zwane Mwaikambo wrote:

> Did you forget return values? Or perhaps just redeclare those...
> Also i don't quite understand the new status reporting you're doing, mind
> just explaining it to me a bit? The previous code would tell you wether
> the watchdog is enabled/disabled so you can tell wether the timeout period
> has passed.

Oops, yea, I forgot return values.  I'll fix that up.  I got rid of
sc1200wdt_status because it returns bit 1, which is defined as WDIOF_OVERHEAT
I suppose it would be possible to return WDIOF_KEEPALIVEPING instead.
So something like if(ret & 0x01) return WDIOF_KEEPALIVEPING;?

Regards,
Rob Radez

Re: Further WatchDog Updates

[Rob Radez]

On Tue, 9 Apr 2002, Corey Minyard wrote:

> Why is that too fine grained?  You would just set the values from 1000
> to 255000 instead of 1 to 255, and round up.
>
> I have a board that sets the time value in wierd times (like 225ms,
> 450ms, 900ms, 1800ms, 3600ms, etc.).  I wouldn't be against the
> WDIOS_TIMEINMILLI option, but milliseconds should be good enough for anyone.

Yet Another Brainfart.  I've been having a lot of them recently.

I don't feel comfortable changing the API that much in a stable kernel
series.  Also, some other boards that have very small timeout windows
emulate a larger userspace timeout since it's quite possible that a
process won't get scheduled every 250ms.  I guess the only reason I can see
for such a small timeout window is if one needs 99.9999% uptime and the 29
extra seconds that the watchdog waits before kicking off is important.

Regards,
Rob Radez

Re: Further WatchDog Updates

[Zwane Mwaikambo]

On Tue, 9 Apr 2002, Rob Radez wrote:

> Oops, yea, I forgot return values.  I'll fix that up.  I got rid of
> sc1200wdt_status because it returns bit 1, which is defined as WDIOF_OVERHEAT
> I suppose it would be possible to return WDIOF_KEEPALIVEPING instead.
> So something like if(ret & 0x01) return WDIOF_KEEPALIVEPING;?

Yes, that should be fine. But don't forget its inactive high ;)

so its...

return !(ret & 0x01) ? WDIOF_KEEPALIVEPING : 0;

Thanks,
	Zwane

-- 
http://function.linuxpower.ca
		

Re: Further WatchDog Updates

[Corey Minyard]

Rob Radez wrote:

>On Tue, 9 Apr 2002, Corey Minyard wrote:
>
>>Why is that too fine grained?  You would just set the values from 1000
>>to 255000 instead of 1 to 255, and round up.
>>
>>I have a board that sets the time value in wierd times (like 225ms,
>>450ms, 900ms, 1800ms, 3600ms, etc.).  I wouldn't be against the
>>WDIOS_TIMEINMILLI option, but milliseconds should be good enough for anyone.
>>
>
>Yet Another Brainfart.  I've been having a lot of them recently.
>
>I don't feel comfortable changing the API that much in a stable kernel
>series.  Also, some other boards that have very small timeout windows
>emulate a larger userspace timeout since it's quite possible that a
>process won't get scheduled every 250ms.  I guess the only reason I can see
>for such a small timeout window is if one needs 99.9999% uptime and the 29
>extra seconds that the watchdog waits before kicking off is important.
>
The actual reason for a small timeout is if the system is in a 
high-throughput highly available application, to get the board out of 
commission as soon as possible and avoid a big delay in message handling 
and/or lose as little traffic as possible.  Or if the system can fail 
and start misbehaving, to kill it as soon as possible to minimize the 
damage.  With the preemptable kernel and real-time processes, it's not 
unreasonable to schedule something every 250ms.

-Corey

Re: Further WatchDog Updates

[Rob Radez]

On Tue, 9 Apr 2002, Corey Minyard wrote:

> The actual reason for a small timeout is if the system is in a
> high-throughput highly available application, to get the board out of
> commission as soon as possible and avoid a big delay in message handling
> and/or lose as little traffic as possible.  Or if the system can fail
> and start misbehaving, to kill it as soon as possible to minimize the
> damage.  With the preemptable kernel and real-time processes, it's not
> unreasonable to schedule something every 250ms.

Oops, I forgot.  Since you can set the timeout to 1 second, you would
really only be losing 750ms.  Right now the API says timeout in seconds,
maybe that will change in 2.5, maybe not, and I'm not sure what the
reception would be to an option for granularity.

Regards,
Rob Radez