2002-04-08 03:45:50

by Rob Radez

[permalink] [raw]
Subject: Further WatchDog Updates

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


2002-04-08 12:44:09

by Justin Cormack

[permalink] [raw]
Subject: Re: Further WatchDog Updates

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

2002-04-08 13:16:13

by Roy Sigurd Karlsbakk

[permalink] [raw]
Subject: Re: Further WatchDog Updates

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 [email protected]
> 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.

2002-04-08 20:54:28

by Rob Radez

[permalink] [raw]
Subject: Re: Further WatchDog Updates


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

2002-04-09 04:25:10

by Corey Minyard

[permalink] [raw]
Subject: Re: Further WatchDog Updates

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

2002-04-09 10:49:24

by Rob Radez

[permalink] [raw]
Subject: Re: Further WatchDog Updates


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

2002-04-09 12:02:34

by Zwane Mwaikambo

[permalink] [raw]
Subject: Re: Further WatchDog Updates

-/* 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


2002-04-09 13:25:31

by Corey Minyard

[permalink] [raw]
Subject: Re: Further WatchDog Updates

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


2002-04-09 15:02:41

by Rob Radez

[permalink] [raw]
Subject: Re: Further WatchDog Updates


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

2002-04-09 15:17:36

by Rob Radez

[permalink] [raw]
Subject: Re: Further WatchDog Updates


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

2002-04-09 15:46:26

by Zwane Mwaikambo

[permalink] [raw]
Subject: Re: Further WatchDog Updates

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


2002-04-09 15:55:17

by Corey Minyard

[permalink] [raw]
Subject: Re: Further WatchDog Updates

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

2002-04-09 23:02:04

by Rob Radez

[permalink] [raw]
Subject: Re: Further WatchDog Updates


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