2002-02-20 13:33:03

by Roy Sigurd Karlsbakk

[permalink] [raw]
Subject: SC1200 support?

hi

I'm sorry if this is OT in LKML, but I've tried searching around, and
couldn't find anything.

I have this set-top box with a National Semiconductor Geode SC1200 chip
with a built-in watch-dog plus a lot more.

Does anyone know if there is any support for the sc1200-specific features
in the current kernels, or if there are patches available?

thanks

roy

--
Roy Sigurd Karlsbakk, MCSE, MCNE, CLS, LCA

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


2002-02-20 15:00:24

by Alan

[permalink] [raw]
Subject: Re: SC1200 support?

> I have this set-top box with a National Semiconductor Geode SC1200 chip
> with a built-in watch-dog plus a lot more.

It depends what BIOS firmware you have

> Does anyone know if there is any support for the sc1200-specific features
> in the current kernels, or if there are patches available?

Most Cyrix MediaGX / NatSemi Geode stuff seems to work. Its all a bit
complicated because most of the hardware is a BIOS manufactured illusion
using SMM mode. On the CS5530/5530x at least we support VSA1 video, audio
(including the needed bug workarounds) etc. Not afaik the watchdog.
Watchdog drivers are easy to right fortunately.

If you have VSA2 based firmware then I've no idea what you'll get

Alan

2002-02-21 00:02:33

by Christer Weinigel

[permalink] [raw]
Subject: Re: SC1200 support?

Roy Sigurd Karlsbakk wrote:
>I have this set-top box with a National Semiconductor Geode SC1200 chip
>with a built-in watch-dog plus a lot more.
>
>Does anyone know if there is any support for the sc1200-specific features
>in the current kernels, or if there are patches available?

Darn, I've been meaning to clean these patches up for a month or so,
but I haven't had the time yet. I've made a snapshot of my CVS tree
that you can find at:

http://www.nano-system.com/scx200/

These patches are for a box called the Nano Computer, a SC2200 based
system that I've been involved in the design of. I've written drivers
for the Watchdog, an I2C bus, a MTD map driver and a few small fixes
and workarounds. These drivers ought to work across the whole SCx200
line of processors.

First of all, the current snapshot is based upon Linux-2.4.17 + Keith
Owens kbuild-2.5 system. To get something that you can use, download,
unpack and patch all the neccesary stuff:

tar xvfz scx200-20020219.tar.gz
cd nano/kernel
tar xvfz linux-2.4.17.tar.gz
cd linux
bzcat kbuild-2.5-2.4.16-3.bz2 | patch -p1
bzcat kbuild-2.5-2.4.17-1.bz2 | patch -p1

Then, to build a kernel use nano/kernel/build.sh which is just a shell
script that wraps the kbuild stuff:

cd nano/kernel
./build.sh config
./build.sh

And, now for the drivers.

nano/kernel/arch/i386/kernel/scx200.c -- probes for a SCx200 CPU
and allocates some resources, and code to control the GPIO pins

nano/kernel/arch/i386/kernel/scx200_nano.c -- board specific setup
for the Nano Computer, it mostly sets up a few things that the
BIOS hasn't set up the way I want them to be

nano/kernel/nano/init/main.c -- has been modified to call
scx200_init and scx200_nano_init

nano/kernel/nano/drivers/char/scx200_watchdog.c -- a driver for
the built in watchdog of a SCx200 CPU

nano/kernel/nano/drivers/i2c/scx200_i2c.c -- a driver for an I2C
bus using two GPIO pins

nano/kernel/nano/drivers/mtd/maps/scx200_docflash.c -- a driver
for using Intel Strataflash mapped via the DOCCS pin

nano/kernel/nano/drivers/ide/* -- I've modified the CS5530 IDE
driver to recognize the SCx200 IDE controller. WARNING! I'm not
sure if this is a safe thing to do, the specifications for the
CS5530 and the SCx200 look quite similar, but I might have missed
something. Remove this directory to be on the safe side.

nano/kernel/nano/drivers/net/natsemi.c -- contains some debugging
code and a fix to turn off wake-on-lan since it caused an
interrupt storm if I did ifconfig eth0 down. I belive this has
been fixed in newer versions of the natsemi driver.

nano/kernel/nano/include/linux/pci_ids.h -- updated with PCI IDs
for the SCx200 CPUs.

nano/kernel/nano/include/linux/scx200.h -- header file for the
scx200.c functions.

nano/kernel/sensors/* -- is just a copy of the lm75 and eeprom
drivers from lm_sensors (the board I'm using have a serial eeprom
and two temperature sensors)

It should be trivial to move these drivers to a newer Linux kernel and
to work without the kbuild stuff.

Any questions, suggestions or flames regarding the drivers are
welcome. I'd be especially happy if someone can take a look at the
GPIO functions in scx200.h and tell me if they look ok. I think I've
managed to build an interrupt safe way of touching the I/O ports, but
I might have missed something.

Hm, I'd better put this mail up as a README.txt at the same place.

/Christer

2002-02-21 00:27:47

by Keith Owens

[permalink] [raw]
Subject: Re: SC1200 support?

On Thu, 21 Feb 2002 01:02:02 +0100 (CET),
Christer Weinigel <[email protected]> wrote:
>Darn, I've been meaning to clean these patches up for a month or so,
>but I haven't had the time yet. I've made a snapshot of my CVS tree
>that you can find at:
>
> http://www.nano-system.com/scx200/
>
>First of all, the current snapshot is based upon Linux-2.4.17 + Keith
>Owens kbuild-2.5 system...
>It should be trivial to move these drivers to a newer Linux kernel and
>to work without the kbuild stuff.

I assume that the nano files are in a shadow tree. You can convert
base plus shadow trees to a single view by

make -f $KBUILD_SRCTREE_000/Makefile-2.5 $KBUILD_OBJTREE/.tmp_src

$KBUILD_OBJTREE/.tmp_src will be built as a directory containing 10,000
symlinks pointing at the relevant source files.

diff -urN $KBUILD_SRCTREE_000 $KBUILD_OBJTREE/.tmp_src

to generate a patch from base to base+shadow. Use that diff against a
separate copy of the base tree as a starting point for kbuild 2.4.


2002-02-21 06:06:07

by Zwane Mwaikambo

[permalink] [raw]
Subject: [DRIVER][RFC] SC1200 Watchdog driver

Roy, careful what you wish for ;) Mind you i don't have this device, but the basic
framework is there that if i missed something, it should be pretty trivial to fix.
One thing to note is that the SC1200 is based on the PC87307/PC97307 which is a PnP
device, however this driver requires an io parameter, but i'll add the PnP support
if the driver has a chance of living.

Cheers,
Zwane Mwaikambo

/*
* National Semiconductor PC87307/PC97307 (ala SC1200) WDT driver
* (c) Copyright 2002 Zwane Mwaikambo <[email protected]>,
* All Rights Reserved.
* Based on wdt.c and wdt977.c by Alan Cox and Woody Suwalski respectively.
*
* This program is free software; you can redistribute it and/or
* modify it under the terms of the GNU General Public License
* as published by the Free Software Foundation; either version
* 2 of the License, or (at your option) any later version.
*
* The author(s) of this software shall not be held liable for damages
* of any nature resulting due to the use of this software. This
* software is provided AS-IS with no warranties.
*
* Changelog:
* 20020220 Zwane Mwaikambo Code based on datasheet, no hardware.
*/

#include <linux/config.h>
#include <linux/module.h>
#include <linux/version.h>
#include <linux/types.h>
#include <linux/errno.h>
#include <linux/kernel.h>
#include <linux/sched.h>
#include <linux/smp_lock.h>
#include <linux/miscdevice.h>
#include <linux/watchdog.h>
#include <linux/slab.h>
#include <linux/ioport.h>
#include <linux/fcntl.h>
#include <asm/semaphore.h>
#include <asm/io.h>
#include <asm/uaccess.h>
#include <asm/system.h>
#include <linux/notifier.h>
#include <linux/reboot.h>
#include <linux/init.h>

#define SC1200_MODULE_VER "build 20020220"
#define SC1200_MODULE_NAME "sc1200wdt"
#define PFX SC1200_MODULE_NAME ": "

#define PMIR (io) /* Power Management Index Register */
#define PMDR (io+1) /* Power Management Data Register */

/* Data Register indexes */
#define FER1 0x00 /* Function enable register 1 */
#define FER2 0x01 /* Function enable register 2 */
#define PMC1 0x02 /* Power Management Ctrl 1 */
#define PMC2 0x03 /* Power Management Ctrl 2 */
#define PMC3 0x04 /* Power Management Ctrl 3 */
#define WDTO 0x05 /* Watchdog timeout register */
#define WDCF 0x06 /* Watchdog config register */
#define WDST 0x07 /* Watchdog status register */

/* WDO Status */
#define WDO_ENABLED 0x00
#define WDO_DISABLED 0x01

/* WDCF bitfields - which devices assert WDO */
#define KBC_IRQ 0x01 /* Keyboard Controller */
#define MSE_IRQ 0x02 /* Mouse */
#define UART1_IRQ 0x03 /* Serial0 */
#define UART2_IRQ 0x04 /* Serial1 */
/* 5 -7 are reserved */

static char banner[] __initdata = KERN_INFO PFX SC1200_MODULE_VER " timeout = %d min(s)\n";
static int timeout = 1; /* default to 1 minute (0 - 255), 0 disables it */
static int io;
struct semaphore open_sem;

MODULE_PARM(io, "i");
MODULE_PARM_DESC(io, "io port");
MODULE_PARM(timeout, "i");
MODULE_PARM_DESC(timeout, "range is (0-255), default is 1");

/* Read from Data Register */
static inline void sc1200wdt_read_data(unsigned char index, unsigned char *data)
{
outb_p(index, PMIR);
*data = inb(PMDR);
}

/* Write to Data Register */
static inline void sc1200wdt_write_data(unsigned char index, unsigned char data)
{
outb_p(index, PMIR);
outb(data, PMDR);
}

/* this returns the status of the WDO signal, inactive high
* WDO_ENABLED or WDO_DISABLED
*/
static int sc1200wdt_status(void)
{
unsigned char ret;

sc1200wdt_read_data(WDST, &ret);
return (ret & 0x01); /* bits 1 - 7 are undefined */
}

static int sc1200wdt_open(struct inode *inode, struct file *file)
{
unsigned char reg;

/* allow one at a time */
if (down_trylock(&open_sem))
return -EBUSY;

MOD_INC_USE_COUNT;

if (timeout > 255)
timeout = 255;

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);
printk(KERN_INFO PFX "Watchdog enabled");

return 0;
}

static int sc1200wdt_ioctl(struct inode *inode, struct file *file, unsigned int cmd, unsigned long arg)
{
int new_timeout;
static struct watchdog_info ident = {
WDIOF_SETTIMEOUT,
0,
"PC87307/PC97307"
};

switch (cmd) {
default:
return -ENOTTY; /* Keep Pavel Machek amused ;) */

case WDIOC_GETSUPPORT:
if (copy_to_user((struct watchdog_info *)arg, &ident, sizeof(ident)))
return -EFAULT;
return 0;

case WDIOC_GETSTATUS:
return put_user(sc1200wdt_status(), (int *)arg);

case WDIOC_KEEPALIVE:
sc1200wdt_write_data(WDTO, timeout);
return 0;

case WDIOC_SETTIMEOUT:
if (get_user(new_timeout, (int *)arg))
return -EFAULT;
if (new_timeout < 0 || new_timeout > 255)
return -EINVAL;
timeout = new_timeout;
sc1200wdt_write_data(WDTO, timeout);
/* fall through */

case WDIOC_GETTIMEOUT:
return put_user(timeout, (int *)arg);
}
}

static int sc1200wdt_release(struct inode *inode, struct file *file)
{
lock_kernel();

/* Disable it on the way out */
sc1200wdt_write_data(WDTO, 0);
up(&open_sem);

unlock_kernel();

printk(KERN_INFO PFX "Watchdog disabled\n");
MOD_DEC_USE_COUNT;

return 0;
}

static ssize_t sc1200wdt_write(struct file *file, const char *data, size_t len, loff_t *ppos)
{
if (ppos != &file->f_pos)
return -ESPIPE;

if (len) {
sc1200wdt_write_data(WDTO, timeout);
return 1;
}

return 0;
}

static int sc1200wdt_notify_sys(struct notifier_block *this, unsigned long code, void *unused)
{
if (code == SYS_DOWN || code == SYS_HALT)
sc1200wdt_write_data(WDTO, 0);

return NOTIFY_DONE;
}

static struct notifier_block sc1200wdt_notifier =
{
sc1200wdt_notify_sys,
NULL,
0
};

#ifndef MODULE
static int __init sc1200wdt_setup(char *str)
{
int ints[4];

str = get_options (str, ARRAY_SIZE(ints), ints);

if (ints[0] > 0)
{
io = ints[1];
if (ints[0] > 1)
timeout = ints[2];
}

return 1;
}

__setup("sc1200wdt=", sc1200wdt_setup);
#endif /* MODULE */

static struct file_operations sc1200wdt_fops =
{
owner: THIS_MODULE,
write: sc1200wdt_write,
ioctl: sc1200wdt_ioctl,
open: sc1200wdt_open,
release: sc1200wdt_release,
};

static struct miscdevice sc1200wdt_miscdev =
{
WATCHDOG_MINOR,
"watchdog",
&sc1200wdt_fops
};

static int __init sc1200wdt_probe(int base_io)
{
/* How can we probe for this thing? */
return 0;
}

static int __init sc1200wdt_init(void)
{
int ret;

printk(banner, timeout);

if (!request_region(io, 2, SC1200_MODULE_NAME)) {
printk(KERN_ERR PFX "IO port %#x not free.\n", io);
ret = -EBUSY;
goto out_clean;
}

ret = sc1200wdt_probe(io);
if (ret == -ENODEV)
goto out_io;

ret = register_reboot_notifier(&sc1200wdt_notifier);
if (ret) {
printk(KERN_ERR PFX "Unable to register reboot notifier err = %d\n", ret);
goto out_io;
}

sema_init(&open_sem, 1);
ret = misc_register(&sc1200wdt_miscdev);
if (ret) {
printk(KERN_ERR PFX "Unable to register miscdev on minor %d\n", WATCHDOG_MINOR);
goto out_rbt;
}

out_clean:
return ret;

out_rbt:
unregister_reboot_notifier(&sc1200wdt_notifier);
out_io:
release_region(io, 2);
goto out_clean;
}

static void __exit sc1200wdt_exit(void)
{
misc_deregister(&sc1200wdt_miscdev);
unregister_reboot_notifier(&sc1200wdt_notifier);
release_region(io, 2);
}

module_init(sc1200wdt_init);
module_exit(sc1200wdt_exit);

MODULE_AUTHOR("Zwane Mwaikambo <[email protected]>");
MODULE_DESCRIPTION("Driver for National Semiconductor PC87307/PC97307 watchdog component");
MODULE_LICENSE("GPL");
EXPORT_NO_SYMBOLS;


2002-02-21 06:30:53

by Zwane Mwaikambo

[permalink] [raw]
Subject: Re: SC1200 support?

On Thu, 21 Feb 2002, Christer Weinigel wrote:

> Roy Sigurd Karlsbakk wrote:
> >I have this set-top box with a National Semiconductor Geode SC1200 chip
> >with a built-in watch-dog plus a lot more.
> >
> >Does anyone know if there is any support for the sc1200-specific features
> >in the current kernels, or if there are patches available?
>
> Darn, I've been meaning to clean these patches up for a month or so,
> but I haven't had the time yet. I've made a snapshot of my CVS tree
> that you can find at:
>
> http://www.nano-system.com/scx200/

Wow i'm a complete moron, just spent two hours last night cooking up a
watchdog driver, and all along you had it squirreled away somewhere =)
Next time i'll go to the pub.

Cheers,
Zwane Mwaikambo


2002-02-21 06:35:54

by Nick

[permalink] [raw]
Subject: Re: SC1200 support?

Heh, if you're looking for stuff to write or fix drivers for let me know
<Grin>
Nick

On Thu, 21 Feb 2002, Zwane Mwaikambo wrote:

> On Thu, 21 Feb 2002, Christer Weinigel wrote:
>
> > Roy Sigurd Karlsbakk wrote:
> > >I have this set-top box with a National Semiconductor Geode SC1200 chip
> > >with a built-in watch-dog plus a lot more.
> > >
> > >Does anyone know if there is any support for the sc1200-specific features
> > >in the current kernels, or if there are patches available?
> >
> > Darn, I've been meaning to clean these patches up for a month or so,
> > but I haven't had the time yet. I've made a snapshot of my CVS tree
> > that you can find at:
> >
> > http://www.nano-system.com/scx200/
>
> Wow i'm a complete moron, just spent two hours last night cooking up a
> watchdog driver, and all along you had it squirreled away somewhere =)
> Next time i'll go to the pub.
>
> Cheers,
> Zwane Mwaikambo
>
>
> -
> 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/
>

2002-02-21 06:42:15

by Zwane Mwaikambo

[permalink] [raw]
Subject: Re: SC1200 support?

On Thu, 21 Feb 2002 [email protected] wrote:

> Heh, if you're looking for stuff to write or fix drivers for let me know
> <Grin>
> Nick

What do you have in mind? I'm usually game.

Regards,
Zwane Mwaikambo

2002-02-21 09:40:04

by Jeff Garzik

[permalink] [raw]
Subject: Re: [DRIVER][RFC] SC1200 Watchdog driver

> /*
> * National Semiconductor PC87307/PC97307 (ala SC1200) WDT driver
> * (c) Copyright 2002 Zwane Mwaikambo <[email protected]>,
> * All Rights Reserved.
> * Based on wdt.c and wdt977.c by Alan Cox and Woody Suwalski respectively.


whee, nice and clean driver....


> #include <linux/config.h>
> #include <linux/module.h>
> #include <linux/version.h>
> #include <linux/types.h>
> #include <linux/errno.h>
> #include <linux/kernel.h>
> #include <linux/sched.h>
> #include <linux/smp_lock.h>
> #include <linux/miscdevice.h>
> #include <linux/watchdog.h>
> #include <linux/slab.h>
> #include <linux/ioport.h>
> #include <linux/fcntl.h>
> #include <asm/semaphore.h>
> #include <asm/io.h>
> #include <asm/uaccess.h>
> #include <asm/system.h>
> #include <linux/notifier.h>
> #include <linux/reboot.h>
> #include <linux/init.h>

try deleting all includes and rebuild this list from scratch... I'll bet
it can be made smaller.



> static int sc1200wdt_status(void)

why not make this 'static inline' too, it's pretty small, and
sc1200wdt_read_data is likewise static inline.

> static int sc1200wdt_open(struct inode *inode, struct file *file)
> {
> unsigned char reg;
>
> /* allow one at a time */
> if (down_trylock(&open_sem))
> return -EBUSY;
>
> MOD_INC_USE_COUNT;

No need for MOD_INC_USE_COUNT when you initialize "owner: THIS_MODULE".

Maybe this is a cleanup you copied from another driver, and this change
needs to be propagated to other drivers too?

> if (timeout > 255)
> timeout = 255;

IMHO use a constant here instead of magic number 255. maybe use max()
or max_t(), too.


> case WDIOC_SETTIMEOUT:
> if (get_user(new_timeout, (int *)arg))
> return -EFAULT;
> if (new_timeout < 0 || new_timeout > 255)
> return -EINVAL;

likewise


> static int sc1200wdt_release(struct inode *inode, struct file *file)
> {
> lock_kernel();
>
> /* Disable it on the way out */
> sc1200wdt_write_data(WDTO, 0);
> up(&open_sem);
>
> unlock_kernel();
>
> printk(KERN_INFO PFX "Watchdog disabled\n");
> MOD_DEC_USE_COUNT;
>
> return 0;
> }

are you certain we need lock_kernel(), unlock_kernel() here?
especially with a semaphore...


> static struct file_operations sc1200wdt_fops =
> {
> owner: THIS_MODULE,
> write: sc1200wdt_write,
> ioctl: sc1200wdt_ioctl,
> open: sc1200wdt_open,
> release: sc1200wdt_release,
> };

I noticed wdt_pci.c implements ->read, too, why not here as well?


> static int __init sc1200wdt_probe(int base_io)
> {
> /* How can we probe for this thing? */
> return 0;
> }
>
> static int __init sc1200wdt_init(void)

Look at how i810_rng does its PCI probe. [I'm guessing] Surely this
SC1200 hardware has _some_ sort of identifier, like a list of commonly
found PCI host bridges, that is better than the simple request_region()
provided.

Overall, looks good... nice, clean driver.

Jeff



--
Jeff Garzik | "Why is it that attractive girls like you
Building 1024 | always seem to have a boyfriend?"
MandrakeSoft | "Because I'm a nympho that owns a brewery?"
| - BBC TV show "Coupling"

2002-02-21 09:59:47

by Zwane Mwaikambo

[permalink] [raw]
Subject: Re: [DRIVER][RFC] SC1200 Watchdog driver

On Thu, 21 Feb 2002, Jeff Garzik wrote:
> > #include <asm/system.h>
> > #include <linux/notifier.h>
> > #include <linux/reboot.h>
> > #include <linux/init.h>
>
> try deleting all includes and rebuild this list from scratch... I'll bet
> it can be made smaller.

Will do.

> > static int sc1200wdt_release(struct inode *inode, struct file *file)
> > {
> > lock_kernel();
> >
> > /* Disable it on the way out */
> > sc1200wdt_write_data(WDTO, 0);
> > up(&open_sem);
> >
> > unlock_kernel();
> >
> > printk(KERN_INFO PFX "Watchdog disabled\n");
> > MOD_DEC_USE_COUNT;
> >
> > return 0;
> > }
>
> are you certain we need lock_kernel(), unlock_kernel() here?
> especially with a semaphore...

I'll remove the lock/unlock_kernel from there and shuffle the semaphore
around.

> > static struct file_operations sc1200wdt_fops =
> > {
> > owner: THIS_MODULE,
> > write: sc1200wdt_write,
> > ioctl: sc1200wdt_ioctl,
> > open: sc1200wdt_open,
> > release: sc1200wdt_release,
> > };
>
> I noticed wdt_pci.c implements ->read, too, why not here as well?

Hmm i see wdt_pci uses its read call for getting temperature status, the
only thing i can report back is the status of the watchdog, and that i
currently send back via an ioctl call (WDIOC_GETSTATUS). The chip i have a
datasheet for doesn't have temperature reporting via watchdog, but there
are bits (supported by lmsensors) which can do that.

> Look at how i810_rng does its PCI probe. [I'm guessing] Surely this
> SC1200 hardware has _some_ sort of identifier, like a list of commonly
> found PCI host bridges, that is better than the simple request_region()
> provided.

Its an ISAPNP device so we can probe like that (logical device 8), this
particular module doesn't have PnP support (i was gonna add it later), i
was wondering wether there was a possible non PnP probe we could do.

> Overall, looks good... nice, clean driver.

Thanks, but i think i wasted my time on this one, there is a driver for
most of the SC1200 bits (including watchdog) at http://www.nano-system.com/scx200

Cheers,
Zwane Mwaikambo



2002-02-21 10:16:01

by Jeff Garzik

[permalink] [raw]
Subject: Re: [DRIVER][RFC] SC1200 Watchdog driver

Zwane Mwaikambo wrote:
> Thanks, but i think i wasted my time on this one, there is a driver for
> most of the SC1200 bits (including watchdog) at http://www.nano-system.com/scx200

I just looked at their watchdog driver -- yours might be better... They
don't use semaphores in _open, they don't use request_region, etc.

Of course, OTOH their include list is smaller and they don't use
MOD_{INC,DEC}_USE_COUNT ;-)

Jeff

--
Jeff Garzik | "Why is it that attractive girls like you
Building 1024 | always seem to have a boyfriend?"
MandrakeSoft | "Because I'm a nympho that owns a brewery?"
| - BBC TV show "Coupling"

2002-02-21 10:21:30

by Zwane Mwaikambo

[permalink] [raw]
Subject: Re: [DRIVER][RFC] SC1200 Watchdog driver

On Thu, 21 Feb 2002, Jeff Garzik wrote:

> Zwane Mwaikambo wrote:
> > Thanks, but i think i wasted my time on this one, there is a driver for
> > most of the SC1200 bits (including watchdog) at http://www.nano-system.com/scx200
>
> I just looked at their watchdog driver -- yours might be better... They
> don't use semaphores in _open, they don't use request_region, etc.
>
> Of course, OTOH their include list is smaller and they don't use
> MOD_{INC,DEC}_USE_COUNT ;-)

Well those other minors are now taken care of, but they also have the
added advantage of having real hardware to test it out on ;)

Cheers,
Zwane Mwaikambo


2002-02-21 10:56:40

by Christer Weinigel

[permalink] [raw]
Subject: Re: SC1200 support?

Keith Owens wrote:
>I assume that the nano files are in a shadow tree. You can convert
>base plus shadow trees to a single view by
>
> make -f $KBUILD_SRCTREE_000/Makefile-2.5 $KBUILD_OBJTREE/.tmp_src
>
>$KBUILD_OBJTREE/.tmp_src will be built as a directory containing 10,000
>symlinks pointing at the relevant source files.
>
> diff -urN $KBUILD_SRCTREE_000 $KBUILD_OBJTREE/.tmp_src
>
>to generate a patch from base to base+shadow. Use that diff against a
>separate copy of the base tree as a starting point for kbuild 2.4.

Ah, nice. I'll probably do that the next time then.

I decided to try out kbuild properly the last time it was discussed
here, if I'm going to complain I'd better know what I'm talking about :-)

It is working really well, it's very convenient to just have to check
my rather small changes in CVS rather than the whole Linux kernel
tree. It makes it rather easy to do automatic builds too, since to be
certain that everything really is recompiled properly with the old
kbuild I had to do "make distclean; cat saved_config >.config; make
oldconfig; make dep; make bzImage modules".

I have a few small nits though:

I usually add "NO_MAKEFILE_GEN=anything" when compiling things so to
avoid the dependency checking when just doing small changes, but I'm
not allowed to do "make NO_MAKEFILE_GEN=anything install" which is a
bit of a pain. I usually run everything on my test system over NFS so
I install the modules straight into the NFS tree. So I'd really like
to be able to tell the kbuild system "I know what I'm doing, don't
babysit me".

Is it still possible to build modules outside of the kernel tree? I
really like the MTD model of building some modules where the Makefile
looks like this:

ifndef TOPDIR
TOPDIR:=$(shell cd ../linux && pwd)
endif

all:
make -C $(TOPDIR) SUBDIRS=`pwd` modules

M_OBJS := my_module.o

# Real actions are started from Rules.make
include $(TOPDIR)/Rules.make

Because this way I can build third party modules afterwards without
having to recompile everything else.

/Christer






2002-02-21 11:15:17

by Keith Owens

[permalink] [raw]
Subject: Re: SC1200 support?

On Thu, 21 Feb 2002 11:56:12 +0100 (CET),
Christer Weinigel <[email protected]> wrote:
>I usually add "NO_MAKEFILE_GEN=anything" when compiling things so to
>avoid the dependency checking when just doing small changes, but I'm
>not allowed to do "make NO_MAKEFILE_GEN=anything install" which is a
>bit of a pain. I usually run everything on my test system over NFS so
>I install the modules straight into the NFS tree. So I'd really like
>to be able to tell the kbuild system "I know what I'm doing, don't
>babysit me".

A build with NO_MAKEFILE_GEN=1 is _not_ necessarily complete or
correct. NO_MAKEFILE_GEN is for quick compiles to catch typing errors
or to add the odd printk, I cannot guarantee that the result has
recompiled everything. I suppose I could add an option like
I_KNOW_THAT_THIS_BUILD_MAY_BE_INCORRECT_BUT_I_WANT_TO_INSTALL_IT_ANYWAY ;)

>Is it still possible to build modules outside of the kernel tree? I
>really like the MTD model of building some modules where the Makefile
>looks like this:
>
>ifndef TOPDIR
>TOPDIR:=$(shell cd ../linux && pwd)
>endif
>
>all:
> make -C $(TOPDIR) SUBDIRS=`pwd` modules
>
>M_OBJS := my_module.o
>
># Real actions are started from Rules.make
>include $(TOPDIR)/Rules.make
>
>Because this way I can build third party modules afterwards without
>having to recompile everything else.

Shadow trees already handle separate compilation. Unlike kbuild 2.4,
nothing else is recompiled in kbuild 2.5, unless you have changed it.

2002-02-21 11:19:37

by Christer Weinigel

[permalink] [raw]
Subject: Re: [DRIVER][RFC] SC1200 Watchdog driver

Jeff Garzik <[email protected]> wrote:

> I just looked at their watchdog driver -- yours might be better... They
> don't use semaphores in _open, they don't use request_region, etc.

I do use request region, it's all done in another source file though.

What use would the semaphore be? To disallow multiple opens?

I'm not too sure about the locking requirements on release, do I need
to add lock_kernel to the release function, or is that automatically
handled by "owner: THIS_MODULE"?

static int scx200_watchdog_release(struct inode *inode, struct file *file)
{
lock_kernel();
if (!expect_close) {
printk(KERN_WARNING "%s: watchdog device closed unexpectedly, will not disable the watchdog timer\n", name);
} else if (!nowayout) {
scx200_watchdog_disable();
}
clear_bit(0, &in_use);
unlock_kernel();

return 0;
}

Is there anything else that I ought to change in the driver? (Except
to get rid of all the magic constants, I'm planning to do this, I
promise).

/Christer

2002-02-21 11:25:29

by David Woodhouse

[permalink] [raw]
Subject: Re: SC1200 support?


[email protected] said:
> Is it still possible to build modules outside of the kernel tree? I
> really like the MTD model of building some modules where the Makefile
> looks like this:

> ifndef TOPDIR
> TOPDIR:=$(shell cd ../linux && pwd)
> endif

Er, that's not mine, is it?

My CVS tree has no hacks in the Makefiles, but has GNUmakefiles which look
something like the one below.

The hacking of TOPDIR is for compiling in old kernels without having
compatibility hacks in the Makefile - in the GNUmakefile I set TOPDIR to a
directory in which I have a hacked Rules.make which sets M_OBJS from obj-m,
sets TOPDIR back to OLDTOPDIR and includes the proper $(TOPDIR)/Rules.make.

Making this work with kbuild-2.5 so that I can just check stuff out from my
CVS tree and type 'make' as I do at the moment is something I haven't tried
yet.

# $Id: GNUmakefile,v 1.10 2002/01/03 15:00:54 dwmw2 Exp $

LINUXDIR=/lib/modules/$(shell uname -r)/build

ifndef VERSION

# Someone just typed 'make'

modules:
make -C $(LINUXDIR) SUBDIRS=`pwd` modules

dep:
make -C $(LINUXDIR) SUBDIRS=`pwd` dep

clean:
rm -f *.o */*.o

else



ifndef CONFIG_MTD

# We're being invoked outside a normal kernel build. Fake it

# We add to $CC rather than setting EXTRA_CFLAGS because the local
# header files _must_ appear before the in-kernel ones.
CC += -I$(shell pwd)/../../include

CONFIG_MTD := m
CONFIG_MTD_PARTITIONS := m
CONFIG_MTD_REDBOOT_PARTS := m
#CONFIG_MTD_BOOTLDR_PARTS := m
CONFIG_MTD_AFS_PARTS := m
CONFIG_MTD_CHAR := m
CONFIG_MTD_BLOCK := m
CONFIG_FTL := m
CONFIG_NFTL := m

CFLAGS_nftl.o := -DCONFIG_NFTL_RW

endif

# Normal case - build in-kernel

ifeq ($(VERSION),2)
ifneq ($(PATCHLEVEL),4)
ifneq ($(PATCHLEVEL),5)
OLDTOPDIR := $(TOPDIR)
TOPDIR := $(shell pwd)
endif
endif
endif

ifeq ($(VERSION),2)
ifeq ($(PATCHLEVEL),0)
obj-y += initcalls.o
endif
endif

include Makefile

endif

--
dwmw2


2002-02-21 11:59:42

by Christer Weinigel

[permalink] [raw]
Subject: Re: [DRIVER][RFC] SC1200 Watchdog driver

Following up to myself:
> Is there anything else that I ought to change in the driver? (Except
> to get rid of all the magic constants, I'm planning to do this, I
> promise).

In a private mail Jeff Garzik wrote:
>You should use a semaphore in ->open for example, not an atomic test
>op. (note - atomic bit ops must ONLY be performed on unsigned long)

Ok, I've switched to a semaphore instead and gotten rid of the magic
numbers. I've also added the ability to set and get the timeout
(where is WDIOC_GETTIMEOUT supposed to be defined? it's not in my
2.4.17 kernel), and a reboot notifier so that the system can be halted
without the watchdog kicking in.

I have compiled this, but I haven't tested it for real yet (I have no
hardware close right now), so I might have done some stupid mistake.
I'll test this and put up a new snapshot when done.

http://www.nano-system.com/scx200/

Is there anything else I ought to change?

/Christer

--
Blatant plug: I'm a freelance consultant looking for interesting work.

2002-02-21 12:05:42

by Christer Weinigel

[permalink] [raw]
Subject: Re: SC1200 support?

Zwane Mwaikambo <[email protected]> wrote:
> Wow i'm a complete moron, just spent two hours last night cooking up a
> watchdog driver, and all along you had it squirreled away somewhere =)
> Next time i'll go to the pub.

"Never do today what you can leave until tomorrow".

Laziness is a virtue :-)

I've shamelessly stolen some code from you and incorporated it into my
driver. Thanks.

/Christer

--
Blatant plug: I'm a freelance consultant looking for interesting work.

2002-02-21 12:19:01

by Zwane Mwaikambo

[permalink] [raw]
Subject: Re: [DRIVER][RFC] SC1200 Watchdog driver

On Thu, 21 Feb 2002, Christer Weinigel wrote:
> Ok, I've switched to a semaphore instead and gotten rid of the magic
> numbers. I've also added the ability to set and get the timeout
> (where is WDIOC_GETTIMEOUT supposed to be defined? it's not in my
> 2.4.17 kernel), and a reboot notifier so that the system can be halted
> without the watchdog kicking in.

Put the GETTIMEOUT stuff in an ioctl, heres my current version, it
implements a number of the ioctls as well as the reboot notifier and
semaphores for locking.

/*
* National Semiconductor PC87307/PC97307 (ala SC1200) WDT driver
* (c) Copyright 2002 Zwane Mwaikambo <[email protected]>,
* All Rights Reserved.
* Based on wdt.c and wdt977.c by Alan Cox and Woody Suwalski respectively.
*
* This program is free software; you can redistribute it and/or
* modify it under the terms of the GNU General Public License
* as published by the Free Software Foundation; either version
* 2 of the License, or (at your option) any later version.
*
* The author(s) of this software shall not be held liable for damages
* of any nature resulting due to the use of this software. This
* software is provided AS-IS with no warranties.
*
* Changelog:
* 20020220 Zwane Mwaikambo Code based on datasheet, no hardware.
* 20020221 Zwane Mwaikambo Code cleanups (thanks to Jeff Garzik).
*/

#include <linux/config.h>
#include <linux/module.h>
#include <linux/version.h>
#include <linux/kernel.h>
#include <linux/miscdevice.h>
#include <linux/watchdog.h>
#include <linux/ioport.h>
#include <asm/semaphore.h>
#include <asm/io.h>
#include <asm/uaccess.h>
#include <linux/notifier.h>
#include <linux/reboot.h>
#include <linux/init.h>

#define SC1200_MODULE_VER "build 20020220"
#define SC1200_MODULE_NAME "sc1200wdt"
#define PFX SC1200_MODULE_NAME ": "

#define MAX_TIMEOUT 255 /* 255 minutes */
#define PMIR (io) /* Power Management Index Register */
#define PMDR (io+1) /* Power Management Data Register */

/* Data Register indexes */
#define FER1 0x00 /* Function enable register 1 */
#define FER2 0x01 /* Function enable register 2 */
#define PMC1 0x02 /* Power Management Ctrl 1 */
#define PMC2 0x03 /* Power Management Ctrl 2 */
#define PMC3 0x04 /* Power Management Ctrl 3 */
#define WDTO 0x05 /* Watchdog timeout register */
#define WDCF 0x06 /* Watchdog config register */
#define WDST 0x07 /* Watchdog status register */

/* WDO Status */
#define WDO_ENABLED 0x00
#define WDO_DISABLED 0x01

/* WDCF bitfields - which devices assert WDO */
#define KBC_IRQ 0x01 /* Keyboard Controller */
#define MSE_IRQ 0x02 /* Mouse */
#define UART1_IRQ 0x03 /* Serial0 */
#define UART2_IRQ 0x04 /* Serial1 */
/* 5 -7 are reserved */

static char banner[] __initdata = KERN_INFO PFX SC1200_MODULE_VER " timeout = %d min(s)\n";
static int timeout = 1; /* default to 1 minute (0 - 255), 0 disables it */
static int io;
struct semaphore open_sem;

MODULE_PARM(io, "i");
MODULE_PARM_DESC(io, "io port");
MODULE_PARM(timeout, "i");
MODULE_PARM_DESC(timeout, "range is (0-255), default is 1");

/* Read from Data Register */
static inline void sc1200wdt_read_data(unsigned char index, unsigned char *data)
{
outb_p(index, PMIR);
*data = inb(PMDR);
}

/* Write to Data Register */
static inline void sc1200wdt_write_data(unsigned char index, unsigned char data)
{
outb_p(index, PMIR);
outb(data, PMDR);
}

/* this returns the status of the WDO signal, inactive high
* WDO_ENABLED or WDO_DISABLED
*/
static int sc1200wdt_status(void)
{
unsigned char ret;

sc1200wdt_read_data(WDST, &ret);
return (ret & 0x01); /* bits 1 - 7 are undefined */
}

static int sc1200wdt_open(struct inode *inode, struct file *file)
{
unsigned char reg;

/* allow one at a time */
if (down_trylock(&open_sem))
return -EBUSY;

if (timeout > MAX_TIMEOUT)
timeout = MAX_TIMEOUT;

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);
printk(KERN_INFO PFX "Watchdog enabled");

return 0;
}

static int sc1200wdt_ioctl(struct inode *inode, struct file *file, unsigned int cmd, unsigned long arg)
{
int new_timeout;
static struct watchdog_info ident = {
WDIOF_SETTIMEOUT,
0,
"PC87307/PC97307"
};

switch (cmd) {
default:
return -ENOTTY; /* Keep Pavel Machek amused ;) */

case WDIOC_GETSUPPORT:
if (copy_to_user((struct watchdog_info *)arg, &ident, sizeof(ident)))
return -EFAULT;
return 0;

case WDIOC_GETSTATUS:
return put_user(sc1200wdt_status(), (int *)arg);

case WDIOC_KEEPALIVE:
sc1200wdt_write_data(WDTO, timeout);
return 0;

case WDIOC_SETTIMEOUT:
if (get_user(new_timeout, (int *)arg))
return -EFAULT;
if (new_timeout < 0 || new_timeout > MAX_TIMEOUT)
return -EINVAL;
timeout = new_timeout;
sc1200wdt_write_data(WDTO, timeout);
/* fall through */

case WDIOC_GETTIMEOUT:
return put_user(timeout, (int *)arg);
}
}

static int sc1200wdt_release(struct inode *inode, struct file *file)
{
/* Disable it on the way out */
sc1200wdt_write_data(WDTO, 0);
up(&open_sem);

printk(KERN_INFO PFX "Watchdog disabled\n");

return 0;
}

static ssize_t sc1200wdt_write(struct file *file, const char *data, size_t len, loff_t *ppos)
{
if (ppos != &file->f_pos)
return -ESPIPE;

if (len) {
sc1200wdt_write_data(WDTO, timeout);
return 1;
}

return 0;
}

static int sc1200wdt_notify_sys(struct notifier_block *this, unsigned long code, void *unused)
{
if (code == SYS_DOWN || code == SYS_HALT)
sc1200wdt_write_data(WDTO, 0);

return NOTIFY_DONE;
}

static struct notifier_block sc1200wdt_notifier =
{
sc1200wdt_notify_sys,
NULL,
0
};

#ifndef MODULE
static int __init sc1200wdt_setup(char *str)
{
int ints[4];

str = get_options (str, ARRAY_SIZE(ints), ints);

if (ints[0] > 0)
{
io = ints[1];
if (ints[0] > 1)
timeout = ints[2];
}

return 1;
}

__setup("sc1200wdt=", sc1200wdt_setup);
#endif /* MODULE */

static struct file_operations sc1200wdt_fops =
{
owner: THIS_MODULE,
write: sc1200wdt_write,
ioctl: sc1200wdt_ioctl,
open: sc1200wdt_open,
release: sc1200wdt_release,
};

static struct miscdevice sc1200wdt_miscdev =
{
WATCHDOG_MINOR,
"watchdog",
&sc1200wdt_fops
};

static int __init sc1200wdt_probe(int base_io)
{
/* How can we probe for this thing? */
return 0;
}

static int __init sc1200wdt_init(void)
{
int ret;

printk(banner, timeout);

if (!request_region(io, 2, SC1200_MODULE_NAME)) {
printk(KERN_ERR PFX "IO port %#x not free.\n", io);
ret = -EBUSY;
goto out_clean;
}

ret = sc1200wdt_probe(io);
if (ret == -ENODEV)
goto out_io;

ret = register_reboot_notifier(&sc1200wdt_notifier);
if (ret) {
printk(KERN_ERR PFX "Unable to register reboot notifier err = %d\n", ret);
goto out_io;
}

sema_init(&open_sem, 1);
ret = misc_register(&sc1200wdt_miscdev);
if (ret) {
printk(KERN_ERR PFX "Unable to register miscdev on minor %d\n", WATCHDOG_MINOR);
goto out_rbt;
}

out_clean:
return ret;

out_rbt:
unregister_reboot_notifier(&sc1200wdt_notifier);
out_io:
release_region(io, 2);
goto out_clean;
}

static void __exit sc1200wdt_exit(void)
{
misc_deregister(&sc1200wdt_miscdev);
unregister_reboot_notifier(&sc1200wdt_notifier);
release_region(io, 2);
}

module_init(sc1200wdt_init);
module_exit(sc1200wdt_exit);

MODULE_AUTHOR("Zwane Mwaikambo <[email protected]>");
MODULE_DESCRIPTION("Driver for National Semiconductor PC87307/PC97307 watchdog component");
MODULE_LICENSE("GPL");
EXPORT_NO_SYMBOLS;


2002-02-21 12:23:12

by Jeff Garzik

[permalink] [raw]
Subject: Re: [DRIVER][RFC] SC1200 Watchdog driver

Christer Weinigel wrote:
> static int margin = 60; /* in seconds */
> MODULE_PARM(margin, "i");
>
> static int nowayout = CONFIG_WATCHDOG_NOWAYOUT;
> MODULE_PARM(nowayout, "i");

MODULE_PARM_DESC would be nice

> static void scx200_watchdog_update_margin(void)
> {
> printk(KERN_INFO "%s: timer margin %d seconds\n", name, margin);
> wdto_restart = 32768 / 1024 * margin;
> scx200_watchdog_ping();
> }

if you can turn multiplication and division of powers-of-2 into left and
right shifts, other simplications sometimes follow. Certainly you want
to avoid division especially and multiplication also if possible.

> static int scx200_watchdog_open(struct inode *inode, struct file *file)
> {
> /* only allow one at a time */
> if (down_trylock(&open_sem))
> return -EBUSY;
> scx200_watchdog_enable();
> expect_close = 0;
>
> return 0;
> }

now, a policy question -- do you want to fail or simply put to sleep
multiple openers? if you want to fail, this should be ok I think. if
you want to sleep, you can look at sound/oss/* in 2.5.x or
drivers/sound/* in 2.4.x for some examples of semaphore use on open(2).

> static int scx200_watchdog_release(struct inode *inode, struct file *file)
> {
> if (!expect_close) {
> printk(KERN_WARNING "%s: watchdog device closed unexpectedly, "
> "will not disable the watchdog timer\n", name);

I wonder why 'name' is not simply a macro defining a string constant?
Oh yeah, it matters very little. You might want to make 'name' const,
though.

> static struct notifier_block scx200_watchdog_notifier =
> {
> scx200_watchdog_notify_sys,
> NULL,
> 0
> };

use name:value style of struct initialization, and omit any struct
members which are 0/NULL (that's implicit).

> static int __init scx200_watchdog_init(void)
> {
> int r;

Here's a big one, I still don't like this lack of probing in the
driver. Sure we have "probed elsewhere", but IMO each driver like this
one needs to check -something- to ensure that SC1200 hardware is
present. Otherwise, a random user from a distro-that-builds-all-drivers
might "modprobe sc1200_watchdog" and things go boom.

If the upper levels have set up resources correctly, for example, then
there should be a resource for the watchdog that is -not- marked busy,
which the watchdog subsequently calls request_region on. Or, the
resource is already marked busy (this is present state, as you
indicated) and you peek at the resource strings for the one you want.

Tiny drivers for hardware embedded on some larger device tends to need
little hacks like that to check for the presence of hardware... but you
need that sanity check somewhere in each driver.

Jeff



--
Jeff Garzik | "Why is it that attractive girls like you
Building 1024 | always seem to have a boyfriend?"
MandrakeSoft | "Because I'm a nympho that owns a brewery?"
| - BBC TV show "Coupling"

2002-02-21 12:43:57

by Zwane Mwaikambo

[permalink] [raw]
Subject: Re: [DRIVER][RFC] SC1200 Watchdog driver

On Thu, 21 Feb 2002, Jeff Garzik wrote:

> now, a policy question -- do you want to fail or simply put to sleep
> multiple openers? if you want to fail, this should be ok I think. if
> you want to sleep, you can look at sound/oss/* in 2.5.x or
> drivers/sound/* in 2.4.x for some examples of semaphore use on open(2).

i think the following would be ok here.

[...]
if (down_interruptible(&open_sem))
return -ERESTARTSYS;
[...]

> Here's a big one, I still don't like this lack of probing in the
> driver. Sure we have "probed elsewhere", but IMO each driver like this
> one needs to check -something- to ensure that SC1200 hardware is
> present. Otherwise, a random user from a distro-that-builds-all-drivers
> might "modprobe sc1200_watchdog" and things go boom.

The actual SuperIO chip the SC1200 is based on is fully PnP so we can
easily do a search without frobbing hardware. To support non PnP mode, we
could find a register with a default value which isn't 0xFF or 0x00, i
reckon that would work quite well.

Regards,
Zwane Mwaikambo


2002-02-21 12:47:07

by Jeff Garzik

[permalink] [raw]
Subject: Re: [DRIVER][RFC] SC1200 Watchdog driver

Zwane Mwaikambo wrote:
>
> On Thu, 21 Feb 2002, Jeff Garzik wrote:
>
> > now, a policy question -- do you want to fail or simply put to sleep
> > multiple openers? if you want to fail, this should be ok I think. if
> > you want to sleep, you can look at sound/oss/* in 2.5.x or
> > drivers/sound/* in 2.4.x for some examples of semaphore use on open(2).
>
> i think the following would be ok here.
>
> [...]
> if (down_interruptible(&open_sem))
> return -ERESTARTSYS;
> [...]

Read the OSS sound drivers, people have put a lot of time into coding
and debugging those open(2) routines :)


> > Here's a big one, I still don't like this lack of probing in the
> > driver. Sure we have "probed elsewhere", but IMO each driver like this
> > one needs to check -something- to ensure that SC1200 hardware is
> > present. Otherwise, a random user from a distro-that-builds-all-drivers
> > might "modprobe sc1200_watchdog" and things go boom.
>
> The actual SuperIO chip the SC1200 is based on is fully PnP so we can
> easily do a search without frobbing hardware. To support non PnP mode, we
> could find a register with a default value which isn't 0xFF or 0x00, i
> reckon that would work quite well.

Well, if you already probe elsewhere, one could easily EXPORT_SYMBOL
such information from the main driver, so you would need to only check a
variable.

But I agree in principle with you about probing and PNP, just saying we
need to look at the big picture too, what other SC1200 modules are in
the system and what info/capabilities they provide.

Jeff


--
Jeff Garzik | "Why is it that attractive girls like you
Building 1024 | always seem to have a boyfriend?"
MandrakeSoft | "Because I'm a nympho that owns a brewery?"
| - BBC TV show "Coupling"

2002-02-21 12:58:08

by Christer Weinigel

[permalink] [raw]
Subject: Re: [DRIVER][RFC] SC1200 Watchdog driver

Jeff Garzik wrote:
> MODULE_PARM_DESC would be nice

Done.

> > static void scx200_watchdog_update_margin(void)
> > {
> > printk(KERN_INFO "%s: timer margin %d seconds\n", name, margin);
> > wdto_restart = 32768 / 1024 * margin;
> > scx200_watchdog_ping();
> > }
>
> if you can turn multiplication and division of powers-of-2 into left and
> right shifts, other simplications sometimes follow. Certainly you want
> to avoid division especially and multiplication also if possible.

Since this is only called on initialization I'm not overly concerned
with performance here, I prefer code clarity. This ought to be
optimized by gcc anyways.

> now, a policy question -- do you want to fail or simply put to sleep
> multiple openers? if you want to fail, this should be ok I think. if
> you want to sleep, you can look at sound/oss/* in 2.5.x or
> drivers/sound/* in 2.4.x for some examples of semaphore use on
> open(2).

I'm not even sure if single-open sematics are neccesary at all, but I
copied most of the interface from wdt285.c so I copied this too. The
watchdog API seems to be a rather ad hoc thing. For example I just
noticed that the WDIOC_SETTIMEOUT call probably takes a parameter
which seems to be minutes, not seconds. "Someone (tm)" ought to write
a more formal API specification.

> I wonder why 'name' is not simply a macro defining a string constant?
> Oh yeah, it matters very little. You might want to make 'name' const,
> though.

Because "%s: " is less text than "scx200_watchdog" and I'm not sure if
gcc is able to merge duplicate strings. Not much of a difference.

> > static struct notifier_block scx200_watchdog_notifier =
> > {
> > scx200_watchdog_notify_sys,
> > NULL,
> > 0
> > };
>
> use name:value style of struct initialization, and omit any struct
> members which are 0/NULL (that's implicit).

Done. I also changed the notifier codes that cause the watchdog to
shut down to something that seems more useful.

> > static int __init scx200_watchdog_init(void)
> > {
> > int r;
>
> Here's a big one, I still don't like this lack of probing in the
> driver. Sure we have "probed elsewhere", but IMO each driver like this
> one needs to check -something- to ensure that SC1200 hardware is
> present. Otherwise, a random user from a distro-that-builds-all-drivers
> might "modprobe sc1200_watchdog" and things go boom.

You're right, I just assumed that nobody would load this driver unless
they are on a SCx200 system. Done. I'll update all the other drivers
too.

/Christer (off to lunch)

--
Blatant plug: I'm a freelance consultant looking for interesting work.


Attachments:
d.diff (2.42 kB)

2002-02-21 13:20:44

by Jeff Garzik

[permalink] [raw]
Subject: Re: [DRIVER][RFC] SC1200 Watchdog driver

Christer Weinigel wrote:
> Jeff Garzik wrote:
> > > static void scx200_watchdog_update_margin(void)
> > > {
> > > printk(KERN_INFO "%s: timer margin %d seconds\n", name, margin);
> > > wdto_restart = 32768 / 1024 * margin;
> > > scx200_watchdog_ping();
> > > }
> >
> > if you can turn multiplication and division of powers-of-2 into left and
> > right shifts, other simplications sometimes follow. Certainly you want
> > to avoid division especially and multiplication also if possible.
>
> Since this is only called on initialization I'm not overly concerned
> with performance here, I prefer code clarity. This ought to be
> optimized by gcc anyways.

I mention it because we ran into a case with the ppc md where gcc did
not... I doubt this code would be used on PPC :) but I mention it mainly
as a matter of principle


> > now, a policy question -- do you want to fail or simply put to sleep
> > multiple openers? if you want to fail, this should be ok I think. if
> > you want to sleep, you can look at sound/oss/* in 2.5.x or
> > drivers/sound/* in 2.4.x for some examples of semaphore use on
> > open(2).
>
> I'm not even sure if single-open sematics are neccesary at all, but I
> copied most of the interface from wdt285.c so I copied this too. The
> watchdog API seems to be a rather ad hoc thing. For example I just
> noticed that the WDIOC_SETTIMEOUT call probably takes a parameter
> which seems to be minutes, not seconds. "Someone (tm)" ought to write
> a more formal API specification.

;-) hey, if you took 30 minutes to jot down into a text file your
observations on the implementation of the API, I'm sure we could get
that into 2.4 and 2.5 ...


> > I wonder why 'name' is not simply a macro defining a string constant?
> > Oh yeah, it matters very little. You might want to make 'name' const,
> > though.
>
> Because "%s: " is less text than "scx200_watchdog" and I'm not sure if
> gcc is able to merge duplicate strings. Not much of a difference.

Note that every place where you aren't sure, you are using string
catenation anyway with the KERN_xxx symbols:

printk (KERN_ERR "...


> You're right, I just assumed that nobody would load this driver unless
> they are on a SCx200 system. Done. I'll update all the other drivers
> too.

Thanks!

Jeff



--
Jeff Garzik | "Why is it that attractive girls like you
Building 1024 | always seem to have a boyfriend?"
MandrakeSoft | "Because I'm a nympho that owns a brewery?"
| - BBC TV show "Coupling"

2002-02-21 13:24:54

by Alan

[permalink] [raw]
Subject: Re: [DRIVER][RFC] SC1200 Watchdog driver

> Put the GETTIMEOUT stuff in an ioctl, heres my current version, it
> implements a number of the ioctls as well as the reboot notifier and
> semaphores for locking.

Aside from the other comments Jeff made its got one bug that I only noticed
because I fixed it in a pile of other stuff 8)

> /* Write to Data Register */
> static inline void sc1200wdt_write_data(unsigned char index, unsigned char data)
> {
> outb_p(index, PMIR);
> outb(data, PMDR);
> }

two instruction sequence

> case WDIOC_KEEPALIVE:
> sc1200wdt_write_data(WDTO, timeout);
> return 0;

open
fork
ioctl from two processes one per cpu at the same time

> static int sc1200wdt_release(struct inode *inode, struct file *file)
> {
> /* Disable it on the way out */
> sc1200wdt_write_data(WDTO, 0);
> up(&open_sem);

NOWAYOUT support is missing - trivial to fix. Just remember to MOD_INC_USE..
on the nowayout path

> static int sc1200wdt_notify_sys(struct notifier_block *this, unsigned long code, void *unused)
> {
> if (code == SYS_DOWN || code == SYS_HALT)
> sc1200wdt_write_data(WDTO, 0);

Ditto on the nowayout

Alan in pedantic mode

2002-02-21 14:39:00

by Zwane Mwaikambo

[permalink] [raw]
Subject: Re: [DRIVER][RFC] SC1200 Watchdog driver

On Thu, 21 Feb 2002, Alan Cox wrote:

> Aside from the other comments Jeff made its got one bug that I only noticed
> because I fixed it in a pile of other stuff 8)
> open
> fork
> ioctl from two processes one per cpu at the same time

Thats a nice one =)

> NOWAYOUT support is missing - trivial to fix. Just remember to MOD_INC_USE..
> on the nowayout path

*Added to the TODO*

> Alan in pedantic mode

Thanks Alan and Jeff for the input, i'll cleanup this stuff. Out of
interest, do we normally take in patches for specialised embedded boxes? I
see the AMD Elan stuff got in but that only touched one area and was easy
to integrate. I presume they'd get accepted if the code was broken up into
seperate modules instead of being overly specialised. For example, the
CRIS stuff in the Etrax tree (developer.axis.com).

Regards,
Zwane Mwaikambo



2002-02-21 14:55:12

by Dave Jones

[permalink] [raw]
Subject: Re: [DRIVER][RFC] SC1200 Watchdog driver

On Thu, Feb 21, 2002 at 04:27:34PM +0200, Zwane Mwaikambo wrote:
> Thanks Alan and Jeff for the input, i'll cleanup this stuff. Out of
> interest, do we normally take in patches for specialised embedded boxes? I
> see the AMD Elan stuff got in but that only touched one area and was easy
> to integrate. I presume they'd get accepted if the code was broken up into
> seperate modules instead of being overly specialised. For example, the
> CRIS stuff in the Etrax tree (developer.axis.com).

It seems lately there has been a surge of interest in getting
niche x86 clones included in mainline (Voyager, numaq, Elan, etc).
I forget who it was who suggested it, but the idea came up of
using a similar approach to arm's subarchitecture support for x86.

The downsides would probably be a lot of code duplication,
The upside would be hiding away specialised code from the 99% of
people who don't need to see it. The Voyager patch for example
was ~150kb iirc, and was imo still quite intrusive even after
a first round of suggestions. Putting it into arch/x86-32/voyager/
would allow those that do care about it to do whatever they deem
necessary without inflicting dozens of #ifdefs and the likes
on the majority.

--
| Dave Jones. http://www.codemonkey.org.uk
| SuSE Labs

2002-02-21 15:03:42

by Alan

[permalink] [raw]
Subject: Re: [DRIVER][RFC] SC1200 Watchdog driver

> Thanks Alan and Jeff for the input, i'll cleanup this stuff. Out of
> interest, do we normally take in patches for specialised embedded boxes? I

A lot of the watchdogs are for specific embedded boards. It is improving as
vendors (notably Intel) figure that watchdogs on the chipset are not much
silicon and very useful

> to integrate. I presume they'd get accepted if the code was broken up into
> seperate modules instead of being overly specialised. For example, the
> CRIS stuff in the Etrax tree (developer.axis.com).

Etrax is an entire architecture - its a bit differently weird 8)

2002-02-21 15:54:14

by Joel Becker

[permalink] [raw]
Subject: Re: [DRIVER][RFC] SC1200 Watchdog driver

On Thu, Feb 21, 2002 at 01:57:43PM +0100, Christer Weinigel wrote:
> I'm not even sure if single-open sematics are neccesary at all, but I
> copied most of the interface from wdt285.c so I copied this too. The
> watchdog API seems to be a rather ad hoc thing. For example I just
> noticed that the WDIOC_SETTIMEOUT call probably takes a parameter
> which seems to be minutes, not seconds. "Someone (tm)" ought to write
> a more formal API specification.

WDIOC_SETTIMEOUT is defined as being in seconds. This is
commented in linux/watchdog.h against the WDIOF_SETTIMOUT flag. Where
you see funky math against the passed parameter is where a watchdog card
takes strange values on its hardware and the driver is computing the
proper strange value for the given amount of seconds.
Also note that WDIOC_SETTIMEOUT rounds up to the nearest second
interval the watchdog can handle if the watchdog cannot support the
passed interval. For this reason, it always returns the actual timeout
set in the passed int*. This is usually done with the fall-through to
the WDIOC_GETTIMEOUT code.
You don't see WDIOC_SETTIMEOUT in 2.4.17 because it was added in
2.4.18pre.

Joel

--

"To fall in love is to create a religion that has a fallible god."
-Jorge Luis Borges

http://www.jlbec.org/
[email protected]

2002-02-22 10:26:55

by Zwane Mwaikambo

[permalink] [raw]
Subject: Re: [DRIVER][RFC] SC1200 Watchdog driver

Here's a finalised version, with the recommended changes (including
probe). ISAPNP to follow shortly. Alan, regarding that race in ioctl,
read/write. Wouldn't the open semaphore protect against that in this case.
Otherwise irq safe spinlocks could be added to read/write.

/*
* National Semiconductor PC87307/PC97307 (ala SC1200) WDT driver
* (c) Copyright 2002 Zwane Mwaikambo <[email protected]>,
* All Rights Reserved.
* Based on wdt.c and wdt977.c by Alan Cox and Woody Suwalski respectively.
*
* This program is free software; you can redistribute it and/or
* modify it under the terms of the GNU General Public License
* as published by the Free Software Foundation; either version
* 2 of the License, or (at your option) any later version.
*
* The author(s) of this software shall not be held liable for damages
* of any nature resulting due to the use of this software. This
* software is provided AS-IS with no warranties.
*
* Changelog:
* 20020220 Zwane Mwaikambo Code based on datasheet, no hardware.
* 20020221 Zwane Mwaikambo Cleanups as suggested by Jeff Garzik and Alan Cox.
* 20020222 Zwane Mwaikambo Added probing.
*/

#include <linux/config.h>
#include <linux/module.h>
#include <linux/version.h>
#include <linux/kernel.h>
#include <linux/miscdevice.h>
#include <linux/watchdog.h>
#include <linux/ioport.h>
#include <asm/semaphore.h>
#include <asm/io.h>
#include <asm/uaccess.h>
#include <linux/notifier.h>
#include <linux/reboot.h>
#include <linux/init.h>

#define SC1200_MODULE_VER "build 20020222"
#define SC1200_MODULE_NAME "sc1200wdt"
#define PFX SC1200q_MODULE_NAME ": "

#define MAX_TIMEOUT 255 /* 255 minutes */
#define PMIR (io) /* Power Management Index Register */
#define PMDR (io+1) /* Power Management Data Register */

/* Data Register indexes */
#define FER1 0x00 /* Function enable register 1 */
#define FER2 0x01 /* Function enable register 2 */
#define PMC1 0x02 /* Power Management Ctrl 1 */
#define PMC2 0x03 /* Power Management Ctrl 2 */
#define PMC3 0x04 /* Power Management Ctrl 3 */
#define WDTO 0x05 /* Watchdog timeout register */
#define WDCF 0x06 /* Watchdog config register */
#define WDST 0x07 /* Watchdog status register */

/* WDO Status */
#define WDO_ENABLED 0x00
#define WDO_DISABLED 0x01

/* WDCF bitfields - which devices assert WDO */
#define KBC_IRQ 0x01 /* Keyboard Controller */
#define MSE_IRQ 0x02 /* Mouse */
#define UART1_IRQ 0x03 /* Serial0 */
#define UART2_IRQ 0x04 /* Serial1 */
/* 5 -7 are reserved */

static char banner[] __initdata = KERN_INFO PFX SC1200_MODULE_VER " timeout = %d min(s)\n";
static int timeout = 1; /* default to 1 minute (0 - 255), 0 disables it */
static int io;
struct semaphore open_sem;

MODULE_PARM(io, "i");
MODULE_PARM_DESC(io, "io port");
MODULE_PARM(timeout, "i");
MODULE_PARM_DESC(timeout, "range is 0-255 minutes, default is 1");

/* Read from Data Register */
static inline void sc1200wdt_read_data(unsigned char index, unsigned char *data)
{
outb_p(index, PMIR);
*data = inb(PMDR);
}

/* Write to Data Register */
static inline void sc1200wdt_write_data(unsigned char index, unsigned char data)
{
outb_p(index, PMIR);
outb(data, PMDR);
}

/* this returns the status of the WDO signal, inactive high
* returns WDO_ENABLED or WDO_DISABLED
*/
static inline int sc1200wdt_status(void)
{
unsigned char ret;

sc1200wdt_read_data(WDST, &ret);
return (ret & 0x01); /* bits 1 - 7 are undefined */
}

static int sc1200wdt_open(struct inode *inode, struct file *file)
{
unsigned char reg;

/* allow one at a time */
if (down_trylock(&open_sem))
return -EBUSY;

#ifdef CONFIG_WATCHDOG_NOWAYOUT
MOD_INC_USE_COUNT;
#endif

if (timeout > MAX_TIMEOUT)
timeout = MAX_TIMEOUT;

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);
printk(KERN_INFO PFX "Watchdog enabled");

return 0;
}

static int sc1200wdt_ioctl(struct inode *inode, struct file *file, unsigned int cmd, unsigned long arg)
{
int new_timeout;
static struct watchdog_info ident = {
options: WDIOF_SETTIMEOUT,
firmware_version: 0,
identity: "PC87307/PC97307"
};

switch (cmd) {
default:
return -ENOTTY; /* Keep Pavel Machek amused ;) */

case WDIOC_GETSUPPORT:
if (copy_to_user((struct watchdog_info *)arg, &ident, sizeof(ident)))
return -EFAULT;
return 0;

case WDIOC_GETSTATUS:
return put_user(sc1200wdt_status(), (int *)arg);

case WDIOC_KEEPALIVE:
sc1200wdt_write_data(WDTO, timeout);
return 0;

case WDIOC_SETTIMEOUT:
if (get_user(new_timeout, (int *)arg))
return -EFAULT;

/* the API states this is given in secs */
new_timeout /= 60;
if (new_timeout < 0 || new_timeout > MAX_TIMEOUT)
return -EINVAL;

timeout = new_timeout;
sc1200wdt_write_data(WDTO, timeout);
/* fall through and return the new timeout */

case WDIOC_GETTIMEOUT:
return put_user(timeout * 60, (int *)arg);
}
}

static int sc1200wdt_release(struct inode *inode, struct file *file)
{
#ifndef CONFIG_WATCHDOG_NOWAYOUT
/* Disable it on the way out */
sc1200wdt_write_data(WDTO, 0);
printk(KERN_INFO PFX "Watchdog disabled\n");
#endif
up(&open_sem);

return 0;
}

static ssize_t sc1200wdt_write(struct file *file, const char *data, size_t len, loff_t *ppos)
{
if (ppos != &file->f_pos)
return -ESPIPE;

if (len) {
sc1200wdt_write_data(WDTO, timeout);
return 1;
}

return 0;
}

static int sc1200wdt_notify_sys(struct notifier_block *this, unsigned long code, void *unused)
{
if (code == SYS_DOWN || code == SYS_HALT)
sc1200wdt_write_data(WDTO, 0);

return NOTIFY_DONE;
}

static struct notifier_block sc1200wdt_notifier =
{
notifier_call: sc1200wdt_notify_sys,
notifier_block: NULL,
priority: 0
};

#ifndef MODULE
static int __init sc1200wdt_setup(char *str)
{
int ints[4];

str = get_options (str, ARRAY_SIZE(ints), ints);

if (ints[0] > 0)
{
io = ints[1];
if (ints[0] > 1)
timeout = ints[2];
}

return 1;
}

__setup("sc1200wdt=", sc1200wdt_setup);
#endif /* MODULE */

static struct file_operations sc1200wdt_fops =
{
owner: THIS_MODULE,
write: sc1200wdt_write,
ioctl: sc1200wdt_ioctl,
open: sc1200wdt_open,
release: sc1200wdt_release,
};

static struct miscdevice sc1200wdt_miscdev =
{
minor: WATCHDOG_MINOR,
name: "watchdog",
fops: &sc1200wdt_fops,
};

static int __init sc1200wdt_probe(void)
{
/* The probe works by reading the PMC3 register's default value of 0x0e
* there is one caveat, if the device disables the parallel port or any
* of the UARTs we won't be able to detect it.
* Nb. This could be done with accuracy by reading the SID registers, but
* we don't have access to those io regions.
*/

unsigned char reg;

sc1200wdt_read_data(PMC3, &reg);
reg &= 0x0f; /* we don't want the UART busy bits */
return (reg == 0x0e) ? 0 : -ENODEV;
}

static int __init sc1200wdt_init(void)
{
int ret;

printk(banner, timeout);

if (!request_region(io, 2, SC1200_MODULE_NAME)) {
printk(KERN_ERR PFX "IO port %#x not free.\n", io);
ret = -EBUSY;
goto out_clean;
}

ret = sc1200wdt_probe();
if (ret == -ENODEV)
goto out_io;

ret = register_reboot_notifier(&sc1200wdt_notifier);
if (ret) {
printk(KERN_ERR PFX "Unable to register reboot notifier err = %d\n", ret);
goto out_io;
}

sema_init(&open_sem, 1);
ret = misc_register(&sc1200wdt_miscdev);
if (ret) {
printk(KERN_ERR PFX "Unable to register miscdev on minor %d\n", WATCHDOG_MINOR);
goto out_rbt;
}

out_clean:
return ret;

out_rbt:
unregister_reboot_notifier(&sc1200wdt_notifier);
out_io:
release_region(io, 2);
goto out_clean;
}

static void __exit sc1200wdt_exit(void)
{
misc_deregister(&sc1200wdt_miscdev);
unregister_reboot_notifier(&sc1200wdt_notifier);
release_region(io, 2);
}

module_init(sc1200wdt_init);
module_exit(sc1200wdt_exit);

MODULE_AUTHOR("Zwane Mwaikambo <[email protected]>");
MODULE_DESCRIPTION("Driver for National Semiconductor PC87307/PC97307 watchdog component");
MODULE_LICENSE("GPL");
EXPORT_NO_SYMBOLS;


2002-02-22 19:32:53

by Roy Sigurd Karlsbakk

[permalink] [raw]
Subject: Re: [DRIVER][RFC] SC1200 Watchdog driver

On Fri, 22 Feb 2002, Zwane Mwaikambo wrote:
> Here's a finalised version, with the recommended changes (including
> probe). ISAPNP to follow shortly. Alan, regarding that race in ioctl,
> read/write. Wouldn't the open semaphore protect against that in this case.
> Otherwise irq safe spinlocks could be added to read/write.

Can anyone make a patch out of this?

thanks

roy

--
Roy Sigurd Karlsbakk, Datavaktmester

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

2002-02-22 19:57:43

by Christer Weinigel

[permalink] [raw]
Subject: Re: [DRIVER][RFC] SC1200 Watchdog driver

I wrote:
> > "Someone (tm)" ought to write a more formal API specification.

Jeff Garzik wrote:

> ;-) hey, if you took 30 minutes to jot down into a text file your
> observations on the implementation of the API, I'm sure we could get
> that into 2.4 and 2.5 ...

It took quite a bit more than 30 minutes, but being at home with a
hangover, what else is there to do? :-)

I've gone through the drivers and tried to write down "established
practice". I guess I'm too wordy as usual, but it should be a
starting point. Please take a look at the attached file and if you or
anyone has any comments or can fill in information, please mail me.

/Christer

--
"Just how much can I get away with and still go to heaven?"

2002-02-22 20:49:08

by Christer Weinigel

[permalink] [raw]
Subject: Re: [DRIVER][RFC] SC1200 Watchdog driver

The Linux Watchdog driver API.

Copyright 2002 Christer Weingel <[email protected]>

Some parts of this document are copied verbatim from the sbc60xxwdt
driver which is (c) Copyright 2000 Jakob Oestergaard <[email protected]>

This document describes the state of the Linux 2.4.17 kernel.

Introduction:

A Watchdog Timer (WDT) is a hardware circuit that can reset the
computer system in case of a software fault. You probably knew that
already.

Usually a userspace daemon will notify the kernel watchdog driver via the
/proc/watchdog special device file that userspace is still alive, at
regular intervals. When such a notification occurs, the driver will
usually tell the hardware watchdog that everything is in order, and
that the watchdog should wait for yet another little while to reset
the system. If userspace fails (RAM error, kernel bug, whatever), the
notifications cease to occur, and the hardware watchdog will reset the
system (causing a reboot) after the timeout occurs.

The Linux watchdog API is a rather AD hoc construction and different
drivers implent different, and sometimes incompatible parts of it.
This file is an attempt to document the existing usage and allow
future driver writers to use it as a reference.

The simplest API:

All drivers support the basic mode of operation, where the watchdog
activates as soon as /dev/watchdog is opened and will reboot unless
the watchdog is pinged within a certain time, this time is called the
timeout or margin. The simplest way to ping the watchdog is to write
some data to the device. So a very simple watchdog daemon would look
like this:

int main(int argc, const char *argv[]) {
int fd=open("/dev/watchdog",O_WRONLY);
if (fd==-1) {
perror("watchdog");
exit(1);
}
while(1) {
write(fd, "\0", 1);
sleep(10);
}
}

A more advanced driver could for example check that a HTTP server is
still responding before doing the write call to ping the watchdog.

When the device is closed, the watchdog is disabled. This is not
always such a good idea, since if there is a bug in the watchdog
daemon and it crashes the system will not reboot. Because of this,
some of the drivers support the configuration option "Disable watchdog
shutdown on close", CONFIG_WATCHDOG_NOWAYOUT. If it is set to Y when
compiling the kernel, there is no way of disabling the watchdog once
it has been started. So, if the watchdog dameon crashes, the system
will reboot after the timeout has passed.

Some other drivers will not disable the watchdog, unless a specific
magic character 'V' has been sent /dev/watchdog just before closing
the file. If the userspace daemon closes the file without sending
this special character, the driver will assume that the daemon (and
userspace in general) died, and will stop pinging the watchdog without
disabling it first. This will then cause a reboot.

The ioctl API:

Some drivers also support an ioctl API. I belive this API was first
used in the Berkshire PC Watchdog driver and has been adopted by the
other drivers.

Pinging the watchdog using an ioctl:

All drivers that have an ioctl interface support at least one ioctl,
KEEPALIVE. This ioctl does exactly the same thing as a write to the
watchdog device, so the main loop in the above program could be
replaced with:

while (1) {
ioctl(fd, WDIOC_KEEPALIVE, 0);
sleep(10);
}

the argument to the ioctl is ignored.

Setting the timeout:

For some drivers it is possible to modify the watchdog timeout on the
fly with the SETTIMEOUT ioctl. The argument is an integer
representing the timeout in seconds. The driver returns the real
timeout used in the same variable, and this timeout might differ from
the requested one due to limitation of the hardware.

int timeout = 45;
ioctl(fd, WDIOC_SETTIMEOUT, &timeout);
printf("The timeout was set to %d seconds\n", timeout);

This example might actually print "The timeout was set to 60 seconds"
if the device has a granularity of minutes for its timeout.

Querying the timeout:

Starting with the Linux 2.4.18 kernel (2.4.18-pre actually), it is
possible to query the current timeout using the GETTIMEOUT ioctl.

ioctl(fd, WDIOC_GETTIMEOUT, &timeout);
printf("The timeout was is %d seconds\n", timeout);

Envinronmental monitoring:

Some watchdog drivers can return more information about the system,
some do temperature, fan and power level monitoring, some can tell you
the reason for the last reebot of the system. The GETSUPPORT ioctl is
available to ask what the device can do:

struct watchdog_info ident;
ioctl(fd, WDIOC_GETSUPPORT, &ident);

the fields returned in the ident struct are:

identity a string identifying the watchdog driver
firmware_version the firmware version of the card if available
options a flags describing what the device supports

the options field can have the following bits set, and describes what
kind of information that the GET_STATUS and GET_BOOT_STATUS ioctls can
return. [FIXME -- Is this correct?]

WDIOF_OVERHEAT Reset due to CPU overheat
WDIOF_FANFAULT Fan failed
WDIOF_EXTERN1 External relay 1
WDIOF_EXTERN2 External relay 2
WDIOF_POWERUNDER Power bad/power fault
WDIOF_CARDRESET Card previously reset the CPU
WDIOF_POWEROVER Power over voltage
WDIOF_KEEPALIVEPING Keep alive ping reply

[FIXME -- Can somebody explain what the flags mean in more detail,
especially KEEPALIVEPING?]

For those drivers that return any bits set in the option field, the
GETSTATUS and GETBOOTSTATUS ioctls can be used to ask for the current
status, and the status at the last reboot, respectively.

int flags;
ioctl(fd, WDIOC_GETSTATUS, &flags);

or

ioctl(fd, WDIOC_GETBOOTSTATUS, &flags);

Note that not all devices support these two calls, and some only
support the GETBOOTSTATUS call.

Some drivers can measure the temperature using the GETTEMP ioctl.
[FIXME -- what is the value returned in temperature, degrees celsius?]

int temperature;
ioctl(fd, WDIOC_GETTEMP, &temperature);

Finally the SETOPTIONS ioctl can be used to control some aspects of
the cards operation; right now the pcwd driver is the only one
supporting thiss ioctl.

int options = 0;
ioctl(fd, WDIOC_SETOPTIONS, options);

The following options are available:

WDIOS_DISABLECARD Turn off the watchdog timer
WDIOS_ENABLECARD Turn on the watchdog timer
WDIOS_TEMPPANIC Kernel panic on temperature trip

[FIXME -- better explanations]

Implementations in the current drivers in the kernel tree:

Here I have tried to summarize what the different drivers support and
where they do strange things compared to the other drivers.

acquirewdt.c -- Acquire Single Board Computer

This driver has a hardcoded timeout of 1 minute

Supports CONFIG_WATCHDOG_NOWAYOUT

GETSUPPORT returns KEEPALIVEPING and GETSTATUS will return 1
if the device is open, 0 if not. [FIXME -- isn't this rather
silly? To be able to use the ioctl, the device must be open
and so GETSTATUS will always return 1].

advantechwdt.c -- Advantech Single Board Computer

Hardcoded timeout of 60 seconds

Supports CONFIG_WATCHDOG_NOWAYOUT

GETSUPPORT returns WDIOF_KEEPALIVEPING, and the GETSTATUS call
returns if the device is open or not. [FIXME -- silliness
again?]

eurotechwdt.c -- Eurotech CPU-1220/1410

The timeout can be set using the SETTIMEOUT ioctl and defaults
to 60 seconds.

Also has a module parameter "ev", event type which controls
what should happen on a timeout, the string "int" or anything
else that causes a reboot. [FIXME -- better description]

Supports CONFIG_WATCHDOG_NOWAYOUT

GETSUPPORT returns CARDRESET but GETSTATUS is not supported
and GETBOOTSTATUS just returns 0.

i810-tco.c -- Intel 810 chipset

Also has support for a lot of other i810 stuff, but the
watchdog is one of the things.

The timeout is set using the module parameter "i810_margin",
which is in steps of 0.6 seconds where 2<i810_margin<64

Supports CONFIG_WATCHDOG_NOWAYOUT.

No bits set in GETSUPPORT, but the GETSTATUS call returns some
kind of timer value. GETBOOT status returns some kind of
hardware specific boot status. [FIXME -- describe this]

ib700wdt.c -- IB700 Single Board Computer

Hardcoded timeout of 30 seconds

Supports CONFIG_WATCHDOG_NOWAYOUT

GETSUPPORT returns WDIOF_KEEPALIVEPING, and the GETSTATUS call
returns if the device is open or not. [FIXME -- silliness
again?]

machzwd.c -- MachZ ZF-Logic

Hardcoded timeout of 10 seconds

Has a module parameter "action" that controls what happens
when the timeout runs out which can be 0 = RESET (default),
1 = SMI, 2 = NMI, 3 = SCI.

Supports CONFIG_WATCHDOG_NOWAYOUT and the the magic character
'V' close handling.

GETSUPPORT returns WDIOF_KEEPALIVEPING, and the GETSTATUS call
returns if the device is open or not. [FIXME -- silliness
again?]

mixcomwd.c -- MixCom Watchdog

[FIXME -- I'm unable to tell what the timeout is]

Supports CONFIG_WATCHDOG_NOWAYOUT

GETSUPPORT returns WDIOF_KEEPALIVEPING, GETSTATUS returns if
the device is opened or not [FIXME -- I'm not really sure how
this works, there seems to be some magic connected to
CONFIG_WATCHDOG_NOWAYOUT]

pcwd.c -- Berkshire PC Watchdog

Hardcoded timeout of 1.5 seconds

Supports CONFIG_WATCHDOG_NOWAYOUT

GETSUPPORT returns WDIOF_OVERHEAT|WDIOF_CARDRESET and both
GETSTATUS and GETBOOTSTATUS return something useful.

The SETOPTIONS call can be used to enable and disable the card
and to ask the driver to call panic if the system overheats.

sbc60xxwdt.c -- 60xx Single Board Computer

Hardcoded timeout of 10 seconds

Does not support CONFIG_WATCHDOG_NOWAYOUT, but has the magic
character 'V' close handling.

No bits set in GETSUPPORT

shwdt.c -- SuperH 3/4 processors

[FIXME -- I'm unable to tell what the timeout is]

Supports CONFIG_WATCHDOG_NOWAYOUT

GETSUPPORT returns WDIOF_KEEPALIVEPING, and the GETSTATUS call
returns if the device is open or not. [FIXME -- silliness
again?]

softdog.c -- Software watchdog

The timeout is set with the module parameter "soft_margin"
which defaults to 60 seconds

Supports CONFIG_WATCHDOG_NOWAYOUT

No bits set in GETSUPPORT

w83877f_wdt.c -- W83877F Computer

Hardcoded timeout of 30 seconds

Does not support CONFIG_WATCHDOG_NOWAYOUT, but has the magic
character 'V' close handling.

No bits set in GETSUPPORT

wdt.c -- ICS WDT500/501 ISA and wdt_pci.c -- ICS WDT500/501 PCI

Hardcoded timeout of 120 seconds.

Supports CONFIG_WATCHDOG_NOWAYOUT

GETSUPPORT returns with a lot of bits set, it seems that the
driver can check for overheating, power failure, fan failure
and a two external relay inputs. GETSTATUS can be used to
read these flags.

wdt285.c -- Footbridge watchdog

The timeout is set with the module parameter "soft_margin"
which defaults to 60 seconds

Does not support CONFIG_WATCHDOG_NOWAYOUT

No bits set in GETSUPPORT

wdt977.c -- Netwinder W83977AF chip

Hardcoded timeout of 3 minutes

Supports CONFIG_WATCHDOG_NOWAYOUT

Does not support any ioctls at all.

scx200.c -- National SCx200 CPUs

Not in the kernel yet.

The timeout is set using a module parameter "margin" which
defaults to 60 seconds. The timeout can also be set using
SETTIMEOUT and read using GETTIMEOUT.

Supports a module parameter "nowayout" that is initialized
with the value of CONFIG_WATCHDOG_NOWAYOUT. Also supports the
magic character 'V' handling.

2002-02-22 22:57:06

by Joel Becker

[permalink] [raw]
Subject: Re: [DRIVER][RFC] SC1200 Watchdog driver

On Fri, Feb 22, 2002 at 09:48:23PM +0100, Christer Weinigel wrote:
> Francois Romieu wrote:
> > > I've gone through the drivers and tried to write down "established
> > > practice". I guess I'm too wordy as usual, but it should be a
> > > starting point. Please take a look at the attached file and if you or
...
> Implementations in the current drivers in the kernel tree:
>
> Here I have tried to summarize what the different drivers support and
> where they do strange things compared to the other drivers.

Christer,
Looks good. Have you checked out the 2.4.18pre trees? The
WDIOF_SETTIMEOUT flag is set for GETSTATUS in all of the drivers that
support WDIOC_GETTIMEOUT/WDIOC_SETTIMEOUT. There are 8 or 9 of them,
but this did not seem to reflected in your document.

Joel

--

"The one important thing i have learned over the years is the difference
between taking one's work seriously and taking one's self seriously. The
first is imperative and the second is disastrous."
-Margot Fonteyn

http://www.jlbec.org/
[email protected]

2002-02-24 17:31:05

by Roy Sigurd Karlsbakk

[permalink] [raw]
Subject: Re: [DRIVER][RFC] SC1200 Watchdog driver

Will this driver shut off the watchdog when /dev/watchdog is closed, or
does it require an explicit shutdown message like Jakob Oestergaard's
sbc60xxwdt driver?

On Thu, 21 Feb 2002, Zwane Mwaikambo wrote:

> On Thu, 21 Feb 2002, Jeff Garzik wrote:
> > > #include <asm/system.h>
> > > #include <linux/notifier.h>
> > > #include <linux/reboot.h>
> > > #include <linux/init.h>
> >
> > try deleting all includes and rebuild this list from scratch... I'll bet
> > it can be made smaller.
>
> Will do.
>
> > > static int sc1200wdt_release(struct inode *inode, struct file *file)
> > > {
> > > lock_kernel();
> > >
> > > /* Disable it on the way out */
> > > sc1200wdt_write_data(WDTO, 0);
> > > up(&open_sem);
> > >
> > > unlock_kernel();
> > >
> > > printk(KERN_INFO PFX "Watchdog disabled\n");
> > > MOD_DEC_USE_COUNT;
> > >
> > > return 0;
> > > }
> >
> > are you certain we need lock_kernel(), unlock_kernel() here?
> > especially with a semaphore...
>
> I'll remove the lock/unlock_kernel from there and shuffle the semaphore
> around.
>
> > > static struct file_operations sc1200wdt_fops =
> > > {
> > > owner: THIS_MODULE,
> > > write: sc1200wdt_write,
> > > ioctl: sc1200wdt_ioctl,
> > > open: sc1200wdt_open,
> > > release: sc1200wdt_release,
> > > };
> >
> > I noticed wdt_pci.c implements ->read, too, why not here as well?
>
> Hmm i see wdt_pci uses its read call for getting temperature status, the
> only thing i can report back is the status of the watchdog, and that i
> currently send back via an ioctl call (WDIOC_GETSTATUS). The chip i have a
> datasheet for doesn't have temperature reporting via watchdog, but there
> are bits (supported by lmsensors) which can do that.
>
> > Look at how i810_rng does its PCI probe. [I'm guessing] Surely this
> > SC1200 hardware has _some_ sort of identifier, like a list of commonly
> > found PCI host bridges, that is better than the simple request_region()
> > provided.
>
> Its an ISAPNP device so we can probe like that (logical device 8), this
> particular module doesn't have PnP support (i was gonna add it later), i
> was wondering wether there was a possible non PnP probe we could do.
>
> > Overall, looks good... nice, clean driver.
>
> Thanks, but i think i wasted my time on this one, there is a driver for
> most of the SC1200 bits (including watchdog) at http://www.nano-system.com/scx200
>
> Cheers,
> Zwane Mwaikambo
>
>
>

--
Roy Sigurd Karlsbakk, Datavaktmester

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


2002-02-25 08:35:00

by Zwane Mwaikambo

[permalink] [raw]
Subject: Re: [DRIVER][RFC] SC1200 Watchdog driver

On Sun, 24 Feb 2002, Roy Sigurd Karlsbakk wrote:

> Will this driver shut off the watchdog when /dev/watchdog is closed, or
> does it require an explicit shutdown message like Jakob Oestergaard's
> sbc60xxwdt driver?

Depends on wether CONFIG_WATCHDOG_NOWAYOUT is specified.

Zwane


2002-02-25 09:27:51

by Zwane Mwaikambo

[permalink] [raw]
Subject: [PATCH] SC1200 Watchdog driver

On Fri, 22 Feb 2002, Roy Sigurd Karlsbakk wrote:

> On Fri, 22 Feb 2002, Zwane Mwaikambo wrote:
> > Here's a finalised version, with the recommended changes (including
> > probe). ISAPNP to follow shortly. Alan, regarding that race in ioctl,
> > read/write. Wouldn't the open semaphore protect against that in this case.
> > Otherwise irq safe spinlocks could be added to read/write.
>
> Can anyone make a patch out of this?

Heres what i currently have, i'm providing this so that Christer can use
it if he'd like something which is more generalised. ISAPNP support
framework is in, but i haven't a clue what their vendor id is. And i think
i may be using the ISAPNP_FUNCTION macro incorrectly.

Regards,
Zwane

Patch is really against 2.4.18-rc4

diff -urN linux-2.4.18-rc1/Documentation/Configure.help linux-2.4.18-rc1-zm1/Documentation/Configure.help
--- linux-2.4.18-rc1/Documentation/Configure.help Mon Feb 25 10:50:28 2002
+++ linux-2.4.18-rc1-zm1/Documentation/Configure.help Mon Feb 25 11:05:05 2002
@@ -17439,6 +17439,18 @@
The module is called machzwd.o. If you want to compile it as a
module, say M here and read <file:Documentation/modules.txt>.

+CONFIG_SC1200_WDT
+ This is a driver for National Semiconductor PC87307/PC97307 hardware
+ watchdog cards as found on the SC1200. This watchdog is mainly used
+ for power management purposes and can be used to power down the device
+ during inactivity periods (includes interrupt activity monitoring).
+
+ This driver is also available as a module ( = code which can be
+ inserted in and removed from the running kernel whenever you want).
+ The module is called sc1200wdt.o. If you want to compile it as a
+ module, say M here and read <file:Documentation/modules.txt>. Most
+ people will say N.
+
SuperH 3/4 Watchdog
CONFIG_SH_WDT
This driver adds watchdog support for the integrated watchdog in the
diff -urN linux-2.4.18-rc1/MAINTAINERS linux-2.4.18-rc1-zm1/MAINTAINERS
--- linux-2.4.18-rc1/MAINTAINERS Mon Feb 25 10:50:28 2002
+++ linux-2.4.18-rc1-zm1/MAINTAINERS Mon Feb 25 11:05:05 2002
@@ -1319,6 +1319,11 @@
L: [email protected]
S: Maintained

+SC1200 WDT DRIVER
+P: Zwane Mwaikambo
+M: [email protected]
+S: Maintained
+
SCSI CDROM DRIVER
P: Jens Axboe
M: [email protected]
diff -urN linux-2.4.18-rc1/drivers/char/Config.in linux-2.4.18-rc1-zm1/drivers/char/Config.in
--- linux-2.4.18-rc1/drivers/char/Config.in Mon Feb 25 10:50:39 2002
+++ linux-2.4.18-rc1-zm1/drivers/char/Config.in Mon Feb 25 11:05:05 2002
@@ -169,6 +169,8 @@
tristate ' Intel i810 TCO timer / Watchdog' CONFIG_I810_TCO
tristate ' Mixcom Watchdog' CONFIG_MIXCOMWD
tristate ' SBC-60XX Watchdog Timer' CONFIG_60XX_WDT
+ dep_tristate ' SC1200 Watchdog Timer (EXPERIMENTAL)' CONFIG_SC1200_WDT $CONFIG_EXPERIMENTAL
+
tristate ' W83877F (EMACS) Watchdog Timer' CONFIG_W83877F_WDT
tristate ' ZF MachZ Watchdog' CONFIG_MACHZ_WDT
fi
diff -urN linux-2.4.18-rc1/drivers/char/Makefile linux-2.4.18-rc1-zm1/drivers/char/Makefile
--- linux-2.4.18-rc1/drivers/char/Makefile Mon Feb 25 10:50:39 2002
+++ linux-2.4.18-rc1-zm1/drivers/char/Makefile Mon Feb 25 11:05:05 2002
@@ -234,6 +234,7 @@
obj-$(CONFIG_SH_WDT) += shwdt.o
obj-$(CONFIG_EUROTECH_WDT) += eurotechwdt.o
obj-$(CONFIG_SOFT_WATCHDOG) += softdog.o
+obj-$(CONFIG_SC1200_WDT) += sc1200wdt.o

subdir-$(CONFIG_MWAVE) += mwave
ifeq ($(CONFIG_MWAVE),y)
diff -urN linux-2.4.18-rc1/drivers/char/sc1200wdt.c linux-2.4.18-rc1-zm1/drivers/char/sc1200wdt.c
--- linux-2.4.18-rc1/drivers/char/sc1200wdt.c Thu Jan 1 02:00:00 1970
+++ linux-2.4.18-rc1-zm1/drivers/char/sc1200wdt.c Mon Feb 25 11:05:05 2002
@@ -0,0 +1,416 @@
+/*
+ * National Semiconductor PC87307/PC97307 (ala SC1200) WDT driver
+ * (c) Copyright 2002 Zwane Mwaikambo <[email protected]>,
+ * All Rights Reserved.
+ * Based on wdt.c and wdt977.c by Alan Cox and Woody Suwalski respectively.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version
+ * 2 of the License, or (at your option) any later version.
+ *
+ * The author(s) of this software shall not be held liable for damages
+ * of any nature resulting due to the use of this software. This
+ * software is provided AS-IS with no warranties.
+ *
+ * Changelog:
+ * 20020220 Zwane Mwaikambo Code based on datasheet, no hardware.
+ * 20020221 Zwane Mwaikambo Cleanups as suggested by Jeff Garzik and Alan Cox.
+ * 20020222 Zwane Mwaikambo Added probing.
+ * 20020225 Zwane Mwaikambo Added ISAPNP support.
+ */
+
+#include <linux/config.h>
+#include <linux/module.h>
+#include <linux/version.h>
+#include <linux/kernel.h>
+#include <linux/miscdevice.h>
+#include <linux/watchdog.h>
+#include <linux/ioport.h>
+#include <linux/spinlock.h>
+#include <linux/ioport.h>
+#include <asm/semaphore.h>
+#include <asm/io.h>
+#include <asm/uaccess.h>
+#include <linux/notifier.h>
+#include <linux/reboot.h>
+#include <linux/init.h>
+#include <linux/isapnp.h>
+
+#define SC1200_MODULE_VER "build 20020225"
+#define SC1200_MODULE_NAME "sc1200wdt"
+#define PFX SC1200_MODULE_NAME ": "
+
+#define MAX_TIMEOUT 255 /* 255 minutes */
+#define PMIR (io) /* Power Management Index Register */
+#define PMDR (io+1) /* Power Management Data Register */
+
+/* Data Register indexes */
+#define FER1 0x00 /* Function enable register 1 */
+#define FER2 0x01 /* Function enable register 2 */
+#define PMC1 0x02 /* Power Management Ctrl 1 */
+#define PMC2 0x03 /* Power Management Ctrl 2 */
+#define PMC3 0x04 /* Power Management Ctrl 3 */
+#define WDTO 0x05 /* Watchdog timeout register */
+#define WDCF 0x06 /* Watchdog config register */
+#define WDST 0x07 /* Watchdog status register */
+
+/* WDO Status */
+#define WDO_ENABLED 0x00
+#define WDO_DISABLED 0x01
+
+/* WDCF bitfields - which devices assert WDO */
+#define KBC_IRQ 0x01 /* Keyboard Controller */
+#define MSE_IRQ 0x02 /* Mouse */
+#define UART1_IRQ 0x03 /* Serial0 */
+#define UART2_IRQ 0x04 /* Serial1 */
+/* 5 -7 are reserved */
+
+static char banner[] __initdata = KERN_INFO PFX SC1200_MODULE_VER;
+static int timeout = 1;
+static int io = -1;
+static int io_len = 2; /* for non plug and play */
+struct semaphore open_sem;
+spinlock_t sc1200wdt_lock; /* io port access serialisation */
+
+#if defined CONFIG_ISAPNP || defined CONFIG_ISAPNP_MODULE
+static int isapnp = 1;
+static struct pci_dev *wdt_dev;
+#endif
+
+MODULE_PARM(io, "i");
+MODULE_PARM_DESC(io, "io port");
+MODULE_PARM(timeout, "i");
+MODULE_PARM_DESC(timeout, "range is 0-255 minutes, default is 1");
+
+#if defined CONFIG_ISAPNP || defined CONFIG_ISAPNP_MODULE
+MODULE_PARM(isapnp, "i");
+MODULE_PARM_DESC(isapnp, "When set to 0 driver ISA PnP support will be disabled");
+#endif
+
+
+/* Read from Data Register */
+static inline void sc1200wdt_read_data(unsigned char index, unsigned char *data)
+{
+ spin_lock(&sc1200wdt_lock);
+ outb_p(index, PMIR);
+ *data = inb(PMDR);
+ spin_unlock(&sc1200wdt_lock);
+}
+
+
+/* Write to Data Register */
+static inline void sc1200wdt_write_data(unsigned char index, unsigned char data)
+{
+ spin_lock(&sc1200wdt_lock);
+ outb_p(index, PMIR);
+ outb(data, PMDR);
+ spin_unlock(&sc1200wdt_lock);
+}
+
+
+/* This returns the status of the WDO signal, inactive high.
+ * returns WDO_ENABLED or WDO_DISABLED
+ */
+static inline int sc1200wdt_status(void)
+{
+ unsigned char ret;
+
+ sc1200wdt_read_data(WDST, &ret);
+ return (ret & 0x01); /* bits 1 - 7 are undefined */
+}
+
+
+static int sc1200wdt_open(struct inode *inode, struct file *file)
+{
+ unsigned char reg;
+
+ /* allow one at a time */
+ if (down_trylock(&open_sem))
+ return -EBUSY;
+
+#ifdef CONFIG_WATCHDOG_NOWAYOUT
+ MOD_INC_USE_COUNT;
+#endif
+
+ if (timeout > MAX_TIMEOUT)
+ timeout = MAX_TIMEOUT;
+
+ 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);
+ printk(KERN_INFO PFX "Watchdog enabled, timeout = %d min(s)", timeout);
+
+ return 0;
+}
+
+
+static int sc1200wdt_ioctl(struct inode *inode, struct file *file, unsigned int cmd, unsigned long arg)
+{
+ int new_timeout;
+ static struct watchdog_info ident = {
+ options: WDIOF_SETTIMEOUT,
+ identity: "PC87307/PC97307"
+ };
+
+ switch (cmd) {
+ default:
+ return -ENOTTY; /* Keep Pavel Machek amused ;) */
+
+ case WDIOC_GETSUPPORT:
+ if (copy_to_user((struct watchdog_info *)arg, &ident, sizeof ident))
+ return -EFAULT;
+ return 0;
+
+ case WDIOC_GETSTATUS:
+ return put_user(sc1200wdt_status(), (int *)arg);
+
+ case WDIOC_KEEPALIVE:
+ sc1200wdt_write_data(WDTO, timeout);
+ return 0;
+
+ case WDIOC_SETTIMEOUT:
+ if (get_user(new_timeout, (int *)arg))
+ return -EFAULT;
+
+ /* the API states this is given in secs */
+ new_timeout /= 60;
+ if (new_timeout < 0 || new_timeout > MAX_TIMEOUT)
+ return -EINVAL;
+
+ timeout = new_timeout;
+ sc1200wdt_write_data(WDTO, timeout);
+ /* fall through and return the new timeout */
+
+ case WDIOC_GETTIMEOUT:
+ return put_user(timeout * 60, (int *)arg);
+ }
+}
+
+
+static int sc1200wdt_release(struct inode *inode, struct file *file)
+{
+#ifndef CONFIG_WATCHDOG_NOWAYOUT
+ /* Disable it on the way out */
+ sc1200wdt_write_data(WDTO, 0);
+ printk(KERN_INFO PFX "Watchdog disabled\n");
+#endif
+ up(&open_sem);
+
+ return 0;
+}
+
+
+static ssize_t sc1200wdt_write(struct file *file, const char *data, size_t len, loff_t *ppos)
+{
+ if (ppos != &file->f_pos)
+ return -ESPIPE;
+
+ if (len) {
+ sc1200wdt_write_data(WDTO, timeout);
+ return 1;
+ }
+
+ return 0;
+}
+
+
+static int sc1200wdt_notify_sys(struct notifier_block *this, unsigned long code, void *unused)
+{
+ if (code == SYS_DOWN || code == SYS_HALT)
+ sc1200wdt_write_data(WDTO, 0);
+
+ return NOTIFY_DONE;
+}
+
+
+static struct notifier_block sc1200wdt_notifier =
+{
+ notifier_call: sc1200wdt_notify_sys
+};
+
+static struct file_operations sc1200wdt_fops =
+{
+ owner: THIS_MODULE,
+ write: sc1200wdt_write,
+ ioctl: sc1200wdt_ioctl,
+ open: sc1200wdt_open,
+ release: sc1200wdt_release
+};
+
+static struct miscdevice sc1200wdt_miscdev =
+{
+ minor: WATCHDOG_MINOR,
+ name: "watchdog",
+ fops: &sc1200wdt_fops
+};
+
+
+static int __init sc1200wdt_probe(void)
+{
+ /* The probe works by reading the PMC3 register's default value of 0x0e
+ * there is one caveat, if the device disables the parallel port or any
+ * of the UARTs we won't be able to detect it.
+ * Nb. This could be done with accuracy by reading the SID registers, but
+ * we don't have access to those io regions.
+ */
+
+ unsigned char reg;
+
+ sc1200wdt_read_data(PMC3, &reg);
+ reg &= 0x0f; /* we don't want the UART busy bits */
+ return (reg == 0x0e) ? 0 : -ENODEV;
+}
+
+
+#if defined CONFIG_ISAPNP || defined CONFIG_ISAPNP_MODULE
+
+static int __init sc1200wdt_isapnp_probe(void)
+{
+ int ret;
+
+ /* The WDT is logical device 8 on the main device */
+ wdt_dev = isapnp_find_dev(NULL, ISAPNP_VENDOR('N','S','C'), ISAPNP_FUNCTION(0x08), NULL);
+ if (!wdt_dev)
+ return -ENODEV;
+
+ if (wdt_dev->prepare(wdt_dev) < 0) {
+ printk(KERN_ERR PFX "ISA PnP found device that could not be autoconfigured\n");
+ return -EAGAIN;
+ }
+
+ if (!(pci_resource_flags(wdt_dev, 0) & IORESOURCE_IO)) {
+ printk(KERN_ERR PFX "ISA PnP could not find io ports\n");
+ return -ENODEV;
+ }
+
+ ret = wdt_dev->activate(wdt_dev);
+ if (ret && (ret != -EBUSY))
+ return -ENOMEM;
+
+ /* io port resource overriding support? */
+ io = pci_resource_start(wdt_dev, 0);
+ io_len = pci_resource_len(wdt_dev, 0);
+
+ printk(KERN_DEBUG PFX "ISA PnP found device at io port %#x/%d\n", io, io_len);
+ return 0;
+}
+
+#endif /* CONFIG_ISAPNP */
+
+
+static int __init sc1200wdt_init(void)
+{
+ int ret;
+
+ printk(banner);
+
+ spin_lock_init(&sc1200wdt_lock);
+ sema_init(&open_sem, 1);
+
+#if defined CONFIG_ISAPNP || defined CONFIG_ISAPNP_MODULE
+ if (isapnp) {
+ ret = sc1200wdt_isapnp_probe();
+ if (ret)
+ goto out_clean;
+ }
+#endif
+
+ if (io == -1) {
+ printk(KERN_ERR PFX "io parameter must be specified\n");
+ ret = -EINVAL;
+ goto out_clean;
+ }
+
+ if (!request_region(io, io_len, SC1200_MODULE_NAME)) {
+ printk(KERN_ERR PFX "Unable to register IO port %#x\n", io);
+ ret = -EBUSY;
+ goto out_pnp;
+ }
+
+ ret = sc1200wdt_probe();
+ if (ret)
+ goto out_io;
+
+ ret = register_reboot_notifier(&sc1200wdt_notifier);
+ if (ret) {
+ printk(KERN_ERR PFX "Unable to register reboot notifier err = %d\n", ret);
+ goto out_io;
+ }
+
+ ret = misc_register(&sc1200wdt_miscdev);
+ if (ret) {
+ printk(KERN_ERR PFX "Unable to register miscdev on minor %d\n", WATCHDOG_MINOR);
+ goto out_rbt;
+ }
+
+ /* ret = 0 */
+
+out_clean:
+ return ret;
+
+out_rbt:
+ unregister_reboot_notifier(&sc1200wdt_notifier);
+
+out_io:
+ release_region(io, io_len);
+
+out_pnp:
+#if defined CONFIG_ISAPNP || defined CONFIG_ISAPNP_MODULE
+ if (isapnp && wdt_dev)
+ wdt_dev->deactivate(wdt_dev);
+#endif
+ goto out_clean;
+}
+
+
+static void __exit sc1200wdt_exit(void)
+{
+ misc_deregister(&sc1200wdt_miscdev);
+ unregister_reboot_notifier(&sc1200wdt_notifier);
+
+#if defined CONFIG_ISAPNP || defined CONFIG_ISAPNP_MODULE
+ if(isapnp && wdt_dev)
+ wdt_dev->deactivate(wdt_dev);
+#endif
+
+ release_region(io, io_len);
+}
+
+
+#ifndef MODULE
+static int __init sc1200wdt_setup(char *str)
+{
+ int ints[4];
+
+ str = get_options (str, ARRAY_SIZE(ints), ints);
+
+ if (ints[0] > 0) {
+ io = ints[1];
+ if (ints[0] > 1)
+ timeout = ints[2];
+
+#if defined CONFIG_ISAPNP || defined CONFIG_ISAPNP_MODULE
+ if (ints[0] > 2)
+ isapnp = ints[3];
+#endif
+ }
+
+ return 1;
+}
+
+__setup("sc1200wdt=", sc1200wdt_setup);
+#endif /* MODULE */
+
+
+module_init(sc1200wdt_init);
+module_exit(sc1200wdt_exit);
+
+MODULE_AUTHOR("Zwane Mwaikambo <[email protected]>");
+MODULE_DESCRIPTION("Driver for National Semiconductor PC87307/PC97307 watchdog component");
+MODULE_LICENSE("GPL");
+EXPORT_NO_SYMBOLS;
+

2002-02-25 21:02:57

by Roy Sigurd Karlsbakk

[permalink] [raw]
Subject: Re: [DRIVER][RFC] SC1200 Watchdog driver

> > Will this driver shut off the watchdog when /dev/watchdog is closed, or
> > does it require an explicit shutdown message like Jakob Oestergaard's
> > sbc60xxwdt driver?
>
> Depends on wether CONFIG_WATCHDOG_NOWAYOUT is specified.

problem is - I want to be able to explicitly close it, like Jakob's
variant is. Isn't this a functionality everyone would like?

Think about it - like - if a server of some reason uses long time to
umount its filesystems. Would it be a good thing if the server just
reboots in the middle of a umount? I think not. I beleive Jakob's solution
is quite a bit better, and it shouldn't be _too_ hard to add those extra
lines of code.

Btw: I'm implementing Jakob's way on the softdog ... patch coming later

roy

--
Roy Sigurd Karlsbakk, Datavaktmester

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

2002-02-25 22:28:36

by Randy.Dunlap

[permalink] [raw]
Subject: Re: [DRIVER][RFC] SC1200 Watchdog driver


Hi Christer,

Thanks for doing this "watchdog-api.txt".
Will you add it to linux/Documentation/ also?
Even with the FIXME's, it's valuable to have this information.

Typo: in the 3rd paragraph following "Introduction:",
change "implent" to "implement".

Questions: At various places it refers to
/proc/watchdog and/or /dev/watchdog special device files.
Is /proc/watchdog actually used, or just /dev/watchdog?
And is /proc/watchdog considered a special device file?

--
~Randy

2002-02-26 01:23:13

by Christer Weinigel

[permalink] [raw]
Subject: Re: [DRIVER][RFC] SC1200 Watchdog driver

Randy.Dunlap <[email protected]> wrote:
> Questions: At various places it refers to
> /proc/watchdog and/or /dev/watchdog special device files.
> Is /proc/watchdog actually used, or just /dev/watchdog?
> And is /proc/watchdog considered a special device file?

That's just a typo. It should be /dev/watchdog.

/Christer

--
"Just how much can I get away with and still go to heaven?"

2002-02-26 01:38:33

by Christer Weinigel

[permalink] [raw]
Subject: Re: [DRIVER][RFC] SC1200 Watchdog driver

Hi,

this is a patch trying to document the Watchdog API in the kernel.
I'd suggest that it gets into the mainline kernel so that people read
it and (hopefully) also update it.

/Christer

diff -urN linux.orig/Documentation/watchdog-api.txt linux/Documentation/watchdog-api.txt
--- /dev/null Wed Dec 31 16:00:00 1969
+++ linux/Documentation/watchdog-api.txt Tue Feb 26 01:16:58 2002
@@ -0,0 +1,365 @@
+The Linux Watchdog driver API.
+
+Copyright 2002 Christer Weingel <[email protected]>
+
+Some parts of this document are copied verbatim from the sbc60xxwdt
+driver which is (c) Copyright 2000 Jakob Oestergaard <[email protected]>
+
+This document describes the state of the Linux 2.4.18 kernel.
+
+Introduction:
+
+A Watchdog Timer (WDT) is a hardware circuit that can reset the
+computer system in case of a software fault. You probably knew that
+already.
+
+Usually a userspace daemon will notify the kernel watchdog driver via the
+/dev/watchdog special device file that userspace is still alive, at
+regular intervals. When such a notification occurs, the driver will
+usually tell the hardware watchdog that everything is in order, and
+that the watchdog should wait for yet another little while to reset
+the system. If userspace fails (RAM error, kernel bug, whatever), the
+notifications cease to occur, and the hardware watchdog will reset the
+system (causing a reboot) after the timeout occurs.
+
+The Linux watchdog API is a rather AD hoc construction and different
+drivers implement different, and sometimes incompatible, parts of it.
+This file is an attempt to document the existing usage and allow
+future driver writers to use it as a reference.
+
+The simplest API:
+
+All drivers support the basic mode of operation, where the watchdog
+activates as soon as /dev/watchdog is opened and will reboot unless
+the watchdog is pinged within a certain time, this time is called the
+timeout or margin. The simplest way to ping the watchdog is to write
+some data to the device. So a very simple watchdog daemon would look
+like this:
+
+int main(int argc, const char *argv[]) {
+ int fd=open("/dev/watchdog",O_WRONLY);
+ if (fd==-1) {
+ perror("watchdog");
+ exit(1);
+ }
+ while(1) {
+ write(fd, "\0", 1);
+ sleep(10);
+ }
+}
+
+A more advanced driver could for example check that a HTTP server is
+still responding before doing the write call to ping the watchdog.
+
+When the device is closed, the watchdog is disabled. This is not
+always such a good idea, since if there is a bug in the watchdog
+daemon and it crashes the system will not reboot. Because of this,
+some of the drivers support the configuration option "Disable watchdog
+shutdown on close", CONFIG_WATCHDOG_NOWAYOUT. If it is set to Y when
+compiling the kernel, there is no way of disabling the watchdog once
+it has been started. So, if the watchdog dameon crashes, the system
+will reboot after the timeout has passed.
+
+Some other drivers will not disable the watchdog, unless a specific
+magic character 'V' has been sent /dev/watchdog just before closing
+the file. If the userspace daemon closes the file without sending
+this special character, the driver will assume that the daemon (and
+userspace in general) died, and will stop pinging the watchdog without
+disabling it first. This will then cause a reboot.
+
+The ioctl API:
+
+Some drivers also support an ioctl API. I belive this API was first
+used in the Berkshire PC Watchdog driver and has been adopted by the
+other drivers.
+
+Pinging the watchdog using an ioctl:
+
+All drivers that have an ioctl interface support at least one ioctl,
+KEEPALIVE. This ioctl does exactly the same thing as a write to the
+watchdog device, so the main loop in the above program could be
+replaced with:
+
+ while (1) {
+ ioctl(fd, WDIOC_KEEPALIVE, 0);
+ sleep(10);
+ }
+
+the argument to the ioctl is ignored.
+
+Setting and getting the timeout:
+
+For some drivers it is possible to modify the watchdog timeout on the
+fly with the SETTIMEOUT ioctl, those drivers have the WDIOF_SETTIMEOUT
+flag set in their option field. The argument is an integer
+representing the timeout in seconds. The driver returns the real
+timeout used in the same variable, and this timeout might differ from
+the requested one due to limitation of the hardware.
+
+ int timeout = 45;
+ ioctl(fd, WDIOC_SETTIMEOUT, &timeout);
+ printf("The timeout was set to %d seconds\n", timeout);
+
+This example might actually print "The timeout was set to 60 seconds"
+if the device has a granularity of minutes for its timeout.
+
+Starting with the Linux 2.4.18 kernel, it is possible to query the
+current timeout using the GETTIMEOUT ioctl.
+
+ ioctl(fd, WDIOC_GETTIMEOUT, &timeout);
+ printf("The timeout was is %d seconds\n", timeout);
+
+Envinronmental monitoring:
+
+Some watchdog drivers can return more information about the system,
+some do temperature, fan and power level monitoring, some can tell you
+the reason for the last reebot of the system. The GETSUPPORT ioctl is
+available to ask what the device can do:
+
+ struct watchdog_info ident;
+ ioctl(fd, WDIOC_GETSUPPORT, &ident);
+
+the fields returned in the ident struct are:
+
+ identity a string identifying the watchdog driver
+ firmware_version the firmware version of the card if available
+ options a flags describing what the device supports
+
+the options field can have the following bits set, and describes what
+kind of information that the GET_STATUS and GET_BOOT_STATUS ioctls can
+return. [FIXME -- Is this correct?]
+
+ WDIOF_OVERHEAT Reset due to CPU overheat
+ WDIOF_FANFAULT Fan failed
+ WDIOF_EXTERN1 External relay 1
+ WDIOF_EXTERN2 External relay 2
+ WDIOF_POWERUNDER Power bad/power fault
+ WDIOF_CARDRESET Card previously reset the CPU
+ WDIOF_POWEROVER Power over voltage
+ WDIOF_KEEPALIVEPING Keep alive ping reply
+ WDIOF_SETTIMEOUT Can set/get the timeout
+
+[FIXME -- Can somebody explain what the flags mean in more detail,
+especially KEEPALIVEPING?]
+
+For those drivers that return any bits set in the option field, the
+GETSTATUS and GETBOOTSTATUS ioctls can be used to ask for the current
+status, and the status at the last reboot, respectively.
+
+ int flags;
+ ioctl(fd, WDIOC_GETSTATUS, &flags);
+
+ or
+
+ ioctl(fd, WDIOC_GETBOOTSTATUS, &flags);
+
+Note that not all devices support these two calls, and some only
+support the GETBOOTSTATUS call.
+
+Some drivers can measure the temperature using the GETTEMP ioctl. The
+returned value is the temperature in degrees farenheit.
+
+ int temperature;
+ ioctl(fd, WDIOC_GETTEMP, &temperature);
+
+Finally the SETOPTIONS ioctl can be used to control some aspects of
+the cards operation; right now the pcwd driver is the only one
+supporting thiss ioctl.
+
+ int options = 0;
+ ioctl(fd, WDIOC_SETOPTIONS, options);
+
+The following options are available:
+
+ WDIOS_DISABLECARD Turn off the watchdog timer
+ WDIOS_ENABLECARD Turn on the watchdog timer
+ WDIOS_TEMPPANIC Kernel panic on temperature trip
+
+[FIXME -- better explanations]
+
+Implementations in the current drivers in the kernel tree:
+
+Here I have tried to summarize what the different drivers support and
+where they do strange things compared to the other drivers.
+
+acquirewdt.c -- Acquire Single Board Computer
+
+ This driver has a hardcoded timeout of 1 minute
+
+ Supports CONFIG_WATCHDOG_NOWAYOUT
+
+ GETSUPPORT returns KEEPALIVEPING. GETSTATUS will return 1 if
+ the device is open, 0 if not. [FIXME -- isn't this rather
+ silly? To be able to use the ioctl, the device must be open
+ and so GETSTATUS will always return 1].
+
+advantechwdt.c -- Advantech Single Board Computer
+
+ Timeout that defaults to 60 seconds, supports SETTIMEOUT.
+
+ Supports CONFIG_WATCHDOG_NOWAYOUT
+
+ GETSUPPORT returns WDIOF_KEEPALIVEPING and WDIOF_SETTIMEOUT.
+ The GETSTATUS call returns if the device is open or not.
+ [FIXME -- silliness again?]
+
+eurotechwdt.c -- Eurotech CPU-1220/1410
+
+ The timeout can be set using the SETTIMEOUT ioctl and defaults
+ to 60 seconds.
+
+ Also has a module parameter "ev", event type which controls
+ what should happen on a timeout, the string "int" or anything
+ else that causes a reboot. [FIXME -- better description]
+
+ Supports CONFIG_WATCHDOG_NOWAYOUT
+
+ GETSUPPORT returns CARDRESET and WDIOF_SETTIMEOUT but
+ GETSTATUS is not supported and GETBOOTSTATUS just returns 0.
+
+i810-tco.c -- Intel 810 chipset
+
+ Also has support for a lot of other i810 stuff, but the
+ watchdog is one of the things.
+
+ The timeout is set using the module parameter "i810_margin",
+ which is in steps of 0.6 seconds where 2<i810_margin<64. The
+ driver supports the SETTIMEOUT ioctl.
+
+ Supports CONFIG_WATCHDOG_NOWAYOUT.
+
+ GETSUPPORT returns WDIOF_SETTIMEOUT. The GETSTATUS call
+ returns some kind of timer value which ist not compatible with
+ the other drivers. GETBOOT status returns some kind of
+ hardware specific boot status. [FIXME -- describe this]
+
+ib700wdt.c -- IB700 Single Board Computer
+
+ Default timeout of 30 seconds and the timeout is settable
+ using the SETTIMEOUT ioctl. Note that only a few timeout
+ values are supported.
+
+ Supports CONFIG_WATCHDOG_NOWAYOUT
+
+ GETSUPPORT returns WDIOF_KEEPALIVEPING and WDIOF_SETTIMEOUT.
+ The GETSTATUS call returns if the device is open or not.
+ [FIXME -- silliness again?]
+
+machzwd.c -- MachZ ZF-Logic
+
+ Hardcoded timeout of 10 seconds
+
+ Has a module parameter "action" that controls what happens
+ when the timeout runs out which can be 0 = RESET (default),
+ 1 = SMI, 2 = NMI, 3 = SCI.
+
+ Supports CONFIG_WATCHDOG_NOWAYOUT and the the magic character
+ 'V' close handling.
+
+ GETSUPPORT returns WDIOF_KEEPALIVEPING, and the GETSTATUS call
+ returns if the device is open or not. [FIXME -- silliness
+ again?]
+
+mixcomwd.c -- MixCom Watchdog
+
+ [FIXME -- I'm unable to tell what the timeout is]
+
+ Supports CONFIG_WATCHDOG_NOWAYOUT
+
+ GETSUPPORT returns WDIOF_KEEPALIVEPING, GETSTATUS returns if
+ the device is opened or not [FIXME -- I'm not really sure how
+ this works, there seems to be some magic connected to
+ CONFIG_WATCHDOG_NOWAYOUT]
+
+pcwd.c -- Berkshire PC Watchdog
+
+ Hardcoded timeout of 1.5 seconds
+
+ Supports CONFIG_WATCHDOG_NOWAYOUT
+
+ GETSUPPORT returns WDIOF_OVERHEAT|WDIOF_CARDRESET and both
+ GETSTATUS and GETBOOTSTATUS return something useful.
+
+ The SETOPTIONS call can be used to enable and disable the card
+ and to ask the driver to call panic if the system overheats.
+
+sbc60xxwdt.c -- 60xx Single Board Computer
+
+ Hardcoded timeout of 10 seconds
+
+ Does not support CONFIG_WATCHDOG_NOWAYOUT, but has the magic
+ character 'V' close handling.
+
+ No bits set in GETSUPPORT
+
+shwdt.c -- SuperH 3/4 processors
+
+ [FIXME -- I'm unable to tell what the timeout is]
+
+ Supports CONFIG_WATCHDOG_NOWAYOUT
+
+ GETSUPPORT returns WDIOF_KEEPALIVEPING, and the GETSTATUS call
+ returns if the device is open or not. [FIXME -- silliness
+ again?]
+
+softdog.c -- Software watchdog
+
+ The timeout is set with the module parameter "soft_margin"
+ which defaults to 60 seconds, the timeout is also settable
+ using the SETTIMEOUT ioctl.
+
+ Supports CONFIG_WATCHDOG_NOWAYOUT
+
+ WDIOF_SETTIMEOUT bit set in GETSUPPORT
+
+w83877f_wdt.c -- W83877F Computer
+
+ Hardcoded timeout of 30 seconds
+
+ Does not support CONFIG_WATCHDOG_NOWAYOUT, but has the magic
+ character 'V' close handling.
+
+ No bits set in GETSUPPORT
+
+wdt.c -- ICS WDT500/501 ISA and
+wdt_pci.c -- ICS WDT500/501 PCI
+
+ Default timeout of 60 seconds. The timeout is also settable
+ using the SETTIMEOUT ioctl.
+
+ Supports CONFIG_WATCHDOG_NOWAYOUT
+
+ GETSUPPORT returns with a lot of bits set, it seems that the
+ driver can check for overheating, power failure, fan failure
+ and a two external relay inputs. GETSTATUS can be used to
+ read these flags.
+
+wdt285.c -- Footbridge watchdog
+
+ The timeout is set with the module parameter "soft_margin"
+ which defaults to 60 seconds. The timeout is also settable
+ using the SETTIMEOUT ioctl.
+
+ Does not support CONFIG_WATCHDOG_NOWAYOUT
+
+ WDIOF_SETTIMEOUT bit set in GETSUPPORT
+
+wdt977.c -- Netwinder W83977AF chip
+
+ Hardcoded timeout of 3 minutes
+
+ Supports CONFIG_WATCHDOG_NOWAYOUT
+
+ Does not support any ioctls at all.
+
+scx200.c -- National SCx200 CPUs
+
+ Not in the kernel yet.
+
+ The timeout is set using a module parameter "margin" which
+ defaults to 60 seconds. The timeout can also be set using
+ SETTIMEOUT and read using GETTIMEOUT.
+
+ Supports a module parameter "nowayout" that is initialized
+ with the value of CONFIG_WATCHDOG_NOWAYOUT. Also supports the
+ magic character 'V' handling.


--
"Just how much can I get away with and still go to heaven?"

2002-02-26 01:59:53

by Jakob Oestergaard

[permalink] [raw]
Subject: Re: [DRIVER][RFC] SC1200 Watchdog driver

On Tue, Feb 26, 2002 at 02:37:35AM +0100, Christer Weinigel wrote:
> Hi,
>
> this is a patch trying to document the Watchdog API in the kernel.
> I'd suggest that it gets into the mainline kernel so that people read
> it and (hopefully) also update it.

Great !

...
> +
> +Some parts of this document are copied verbatim from the sbc60xxwdt
> +driver which is (c) Copyright 2000 Jakob Oestergaard <[email protected]>

My e-mail address now is: [email protected]

....
> +A Watchdog Timer (WDT) is a hardware circuit that can reset the
> +computer system in case of a software fault. You probably knew that
> +already.

Hardware faults such as memory corruption (leading to software malfunction)
are included as well :)

> +Usually a userspace daemon will notify the kernel watchdog driver via the
> +/dev/watchdog special device file that userspace is still alive, at
> +regular intervals. When such a notification occurs, the driver will
> +usually tell the hardware watchdog that everything is in order, and
> +that the watchdog should wait for yet another little while to reset
> +the system. If userspace fails (RAM error, kernel bug, whatever), the
> +notifications cease to occur, and the hardware watchdog will reset the
> +system (causing a reboot) after the timeout occurs.

Exactly.

...
> +A more advanced driver could for example check that a HTTP server is
> +still responding before doing the write call to ping the watchdog.

I think that's a bad example - you would start httpd from init if it was that
critical, or use a monitoring system, or something... Spontaneously booting
the machine because the admin made an error in httpd.conf seems a little
impractical :) Especially because it will keep on re-booting, until someone
starts it in single-user mode and fixes the httpd config...

...

A very nice document ! Some day, someone ought to standardize the way
that /dev/watchdog is used... Some other day I presume :)

--
................................................................
: [email protected] : And I see the elder races, :
:.........................: putrid forms of man :
: Jakob ?stergaard : See him rise and claim the earth, :
: OZ9ABN : his downfall is at hand. :
:.........................:............{Konkhra}...............:

2002-02-26 02:28:03

by Alan

[permalink] [raw]
Subject: Re: [DRIVER][RFC] SC1200 Watchdog driver

> +The ioctl API:
> +
> +Some drivers also support an ioctl API. I belive this API was first
> +used in the Berkshire PC Watchdog driver and has been adopted by the
> +other drivers.

The original API is from the WDT500/501, whic first added watchdogs, and
then expanded on in the existing watchdog documentation including
the Docbook 'how to write one' manual