SC1200 support?

SC1200 support?

[Roy Sigurd Karlsbakk]

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.

Re: SC1200 support?

[Alan Cox]

> 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

Re: SC1200 support?

[Christer Weinigel]

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

Re: SC1200 support?

[Keith Owens]

On Thu, 21 Feb 2002 01:02:02 +0100 (CET), 
Christer Weinigel <wingel@nano-system.com> 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.

[DRIVER][RFC] SC1200 Watchdog driver

[Zwane Mwaikambo]

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 <zwane@commfireservices.com>,
 *			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 <zwane@commfireservices.com>");
MODULE_DESCRIPTION("Driver for National Semiconductor PC87307/PC97307 watchdog component");
MODULE_LICENSE("GPL");
EXPORT_NO_SYMBOLS;

Re: SC1200 support?

[Zwane Mwaikambo]

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

Re: SC1200 support?

[Zwane Mwaikambo]

On Thu, 21 Feb 2002 nick@snowman.net 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

Re: SC1200 support?

[nick]

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 majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
> 

Re: [DRIVER][RFC] SC1200 Watchdog driver

[Jeff Garzik]

> /*
>  *      National Semiconductor PC87307/PC97307 (ala SC1200) WDT driver
>  *      (c) Copyright 2002 Zwane Mwaikambo <zwane@commfireservices.com>,
>  *                      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"

Re: [DRIVER][RFC] SC1200 Watchdog driver

[Zwane Mwaikambo]

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

Re: [DRIVER][RFC] SC1200 Watchdog driver

[Zwane Mwaikambo]

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

Re: [DRIVER][RFC] SC1200 Watchdog driver

[Jeff Garzik]

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"

Re: SC1200 support?

[Christer Weinigel]

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

Re: SC1200 support?

[Keith Owens]

On Thu, 21 Feb 2002 11:56:12 +0100 (CET), 
Christer Weinigel <wingel@acolyte.hack.org> 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.

Re: [DRIVER][RFC] SC1200 Watchdog driver

[Christer Weinigel]

Jeff Garzik <jgarzik@mandrakesoft.com> 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

Re: SC1200 support?

[David Woodhouse]

wingel@acolyte.hack.org 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

Re: [DRIVER][RFC] SC1200 Watchdog driver

[Christer Weinigel]

[-- Attachment #1: Type: text/plain, Size: 1022 bytes --]

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.

[-- Attachment #2: scx200_watchdog.c --]
[-- Type: text/x-csrc, Size: 5885 bytes --]

/* linux/drivers/char/scx200_watchdog.c 

   National Semiconductor SCx200 Watchdog support

   Copyright (c) 2001,2002 Christer Weinigel <wingel@nano-system.com>

   Som code taken from:
   National Semiconductor PC87307/PC97307 (ala SC1200) WDT driver
   (c) Copyright 2002 Zwane Mwaikambo <zwane@commfireservices.com>

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

#include <linux/config.h>
#include <linux/module.h>
#include <linux/errno.h>
#include <linux/kernel.h>
#include <linux/init.h>
#include <linux/miscdevice.h>
#include <linux/watchdog.h>
#include <linux/notifier.h>
#include <linux/reboot.h>
#include <asm/uaccess.h>

#include <linux/scx200.h>

MODULE_AUTHOR("Christer Weinigel <wingel@nano-system.com>");
MODULE_DESCRIPTION("NatSemi SCx200 Watchdog");
MODULE_LICENSE("GPL");

#ifndef CONFIG_WATCHDOG_NOWAYOUT
#define CONFIG_WATCHDOG_NOWAYOUT 0
#endif

static char name[] = "scx200_watchdog";

static int margin = 60;		/* in seconds */
MODULE_PARM(margin, "i");

static int nowayout = CONFIG_WATCHDOG_NOWAYOUT;
MODULE_PARM(nowayout, "i");

static u16 wdto_restart;
static struct semaphore open_sem;
static unsigned expect_close;

#define WDTO 0x00		/* Time-Out Register */
#define WDCNFG 0x02		/* Configuration Register */
#define    W_ENABLE 0x00fa	/* Enable watchdog */
#define    W_DISABLE 0x0000	/* Disable watchdog */
#define WDSTS 0x04		/* Status Register */
#define    WDOVF (1<<0)		/* Overflow */

static void scx200_watchdog_ping(void)
{
	outw(wdto_restart, scx200_config_block + WDTO);
}

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();	
}

static void scx200_watchdog_enable(void)
{
	printk(KERN_DEBUG "%s: enable watchdog timer, wdto_restart = %d\n", 
	       name, wdto_restart);

	outw(0, scx200_config_block + WDTO);
	outb(WDOVF, scx200_config_block + WDSTS);
	outw(W_ENABLE, scx200_config_block + WDCNFG);

	scx200_watchdog_ping();
}

static void scx200_watchdog_disable(void)
{
	printk(KERN_DEBUG "%s: disabling watchdog timer\n", name);
		
	outw(0, scx200_config_block + WDTO);
	outb(WDOVF, scx200_config_block + WDSTS);
	outw(W_DISABLE, scx200_config_block + WDCNFG);
}

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

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);
	} else if (!nowayout) {
		scx200_watchdog_disable();
	}
        up(&open_sem);

	return 0;
}

static int scx200_watchdog_notify_sys(struct notifier_block *this, 
				      unsigned long code, void *unused)
{
        if (code == SYS_DOWN || code == SYS_HALT)
		scx200_watchdog_disable();

        return NOTIFY_DONE;
}

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

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

	/* check for a magic close character */
	if (len) 
	{
		size_t i;

		scx200_watchdog_ping();

		expect_close = 0;
		for (i = 0; i < len; ++i) {
			if (data[i] == 'V')
				expect_close = 1;
		}

		return len;
	}

	return 0;
}

static int scx200_watchdog_ioctl(struct inode *inode, struct file *file,
	unsigned int cmd, unsigned long arg)
{
	static struct watchdog_info ident = {
		options : 0,
		firmware_version : 1, 
		identity : "SCx200 Watchdog",
	};
	int new_margin;
	
	switch (cmd) {
	default:
		return -ENOTTY;
	case WDIOC_GETSUPPORT:
		if(copy_to_user((struct watchdog_info *)arg, &ident, 
				sizeof(ident)))
			return -EFAULT;
		return 0;
	case WDIOC_GETSTATUS:
	case WDIOC_GETBOOTSTATUS:
		return put_user(0, (int *)arg);
	case WDIOC_KEEPALIVE:
		scx200_watchdog_ping();
		return 0;
	case WDIOC_SETTIMEOUT:
		if (get_user(new_margin, (int *)arg))
			return -EFAULT;
		margin = new_margin;
		scx200_watchdog_update_margin();
		return 0;
#ifdef WDIOC_GETTIMEOUT		
	case WDIOC_GETTIMEOUT:
		return put_user(margin, (int *)arg);
#endif
	}
}

static struct file_operations scx200_watchdog_fops = {
	owner: THIS_MODULE,
	write: scx200_watchdog_write,
	ioctl: scx200_watchdog_ioctl,
	open: scx200_watchdog_open,
	release: scx200_watchdog_release,
};

static struct miscdevice scx200_watchdog_miscdev = {
	minor: WATCHDOG_MINOR,
	name: name,
	fops: &scx200_watchdog_fops,
};

static int __init scx200_watchdog_init(void)
{
	int r;

	scx200_watchdog_update_margin();
        sema_init(&open_sem, 1);

	r = misc_register(&scx200_watchdog_miscdev);
	if (r)
		return r;

	r = register_reboot_notifier(&scx200_watchdog_notifier);
        if (r) {
                printk(KERN_ERR "%s: unable to register reboot notifier", 
		       name);
		misc_deregister(&scx200_watchdog_miscdev);
                return r;
        }

	return 0;
}

static void __exit scx200_watchdog_cleanup(void)
{
        unregister_reboot_notifier(&scx200_watchdog_notifier);
	misc_deregister(&scx200_watchdog_miscdev);
}

module_init(scx200_watchdog_init);
module_exit(scx200_watchdog_cleanup);

/*
    Local variables:
        compile-command: "cd ../../.. && ./build.sh fast"
        c-basic-offset: 8
    End:
*/

Re: SC1200 support?

[Christer Weinigel]

Zwane Mwaikambo <zwane@linux.realnet.co.sz> 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.

Re: [DRIVER][RFC] SC1200 Watchdog driver

[Zwane Mwaikambo]

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 <zwane@commfireservices.com>,
 *			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 <zwane@commfireservices.com>");
MODULE_DESCRIPTION("Driver for National Semiconductor PC87307/PC97307 watchdog component");
MODULE_LICENSE("GPL");
EXPORT_NO_SYMBOLS;

Re: [DRIVER][RFC] SC1200 Watchdog driver

[Jeff Garzik]

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"

Re: [DRIVER][RFC] SC1200 Watchdog driver

[Zwane Mwaikambo]

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

Re: [DRIVER][RFC] SC1200 Watchdog driver

[Jeff Garzik]

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"

Re: [DRIVER][RFC] SC1200 Watchdog driver

[Christer Weinigel]

[-- Attachment #1: Type: text/plain, Size: 2673 bytes --]

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.

[-- Attachment #2: d.diff --]
[-- Type: application/octet-stream, Size: 2483 bytes --]

diff -urw nano/drivers/char/scx200_watchdog.c.orig nano/drivers/char/scx200_watchdog.c
--- nano/drivers/char/scx200_watchdog.c.orig	Thu Feb 21 13:49:53 2002
+++ nano/drivers/char/scx200_watchdog.c	Thu Feb 21 13:55:30 2002
@@ -38,13 +38,15 @@
 #define CONFIG_WATCHDOG_NOWAYOUT 0
 #endif
 
-static char name[] = "scx200_watchdog";
+static const char name[] = "scx200_watchdog";
 
 static int margin = 60;		/* in seconds */
 MODULE_PARM(margin, "i");
+MODULE_PARM_DESC(margin, "Watchdog margin in seconds");
 
 static int nowayout = CONFIG_WATCHDOG_NOWAYOUT;
 MODULE_PARM(nowayout, "i");
+MODULE_PARM_DESC(nowayout, "If true the watchdog can't be disabled\n");
 
 static u16 wdto_restart;
 static struct semaphore open_sem;
@@ -57,6 +59,8 @@
 #define WDSTS 0x04		/* Status Register */
 #define    WDOVF (1<<0)		/* Overflow */
 
+#define W_SCALE (32768/1024)	/* This depends on the value of W_ENABLE */
+
 static void scx200_watchdog_ping(void)
 {
 	outw(wdto_restart, scx200_config_block + WDTO);
@@ -65,7 +69,7 @@
 static void scx200_watchdog_update_margin(void)
 {
 	printk(KERN_INFO "%s: timer margin %d seconds\n", name, margin);
-	wdto_restart = 32768 / 1024 * margin;
+	wdto_restart = margin * W_SCALE;
 	scx200_watchdog_ping();	
 }
 
@@ -117,7 +121,7 @@
 static int scx200_watchdog_notify_sys(struct notifier_block *this, 
 				      unsigned long code, void *unused)
 {
-        if (code == SYS_DOWN || code == SYS_HALT)
+        if (code == SYS_HALT || code == SYS_POWER_OFF)
 		scx200_watchdog_disable();
 
         return NOTIFY_DONE;
@@ -125,9 +129,7 @@
 
 static struct notifier_block scx200_watchdog_notifier =
 {
-        scx200_watchdog_notify_sys,
-        NULL,
-        0
+        notifier_call: scx200_watchdog_notify_sys
 };
 
 static ssize_t scx200_watchdog_write(struct file *file, const char *data, 
@@ -182,13 +184,12 @@
 	case WDIOC_SETTIMEOUT:
 		if (get_user(new_margin, (int *)arg))
 			return -EFAULT;
-		margin = new_margin;
+		margin = new_margin * 60; /* convert minutes to seconds */
 		scx200_watchdog_update_margin();
 		return 0;
-
 #ifdef WDIOC_GETTIMEOUT		
 	case WDIOC_GETTIMEOUT:
-		return put_user(margin, (int *)arg);
+		return put_user((margin + 59) / 60, (int *)arg);
 #endif
 	}
 }
@@ -210,6 +211,11 @@
 static int __init scx200_watchdog_init(void)
 {
 	int r;
+
+	if (scx200_f0_pdev == NULL) {
+		printk(KERN_ERR "%s: Not a SCx200 CPU\n", name);
+		return -ENODEV;
+	}
 
 	scx200_watchdog_update_margin();
         sema_init(&open_sem, 1);

Re: [DRIVER][RFC] SC1200 Watchdog driver

[Jeff Garzik]

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"

Re: [DRIVER][RFC] SC1200 Watchdog driver

[Alan Cox]

> 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

Re: [DRIVER][RFC] SC1200 Watchdog driver

[Zwane Mwaikambo]

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

Re: [DRIVER][RFC] SC1200 Watchdog driver

[Dave Jones]

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

Re: [DRIVER][RFC] SC1200 Watchdog driver

[Alan Cox]

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

Re: [DRIVER][RFC] SC1200 Watchdog driver

[Joel Becker]

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/
			jlbec@evilplan.org

Re: [DRIVER][RFC] SC1200 Watchdog driver

[Zwane Mwaikambo]

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 <zwane@commfireservices.com>,
 *			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 <zwane@commfireservices.com>");
MODULE_DESCRIPTION("Driver for National Semiconductor PC87307/PC97307 watchdog component");
MODULE_LICENSE("GPL");
EXPORT_NO_SYMBOLS;

Re: [DRIVER][RFC] SC1200 Watchdog driver

[Roy Sigurd Karlsbakk]

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.

Re: [DRIVER][RFC] SC1200 Watchdog driver

[Christer Weinigel]

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

Re: [DRIVER][RFC] SC1200 Watchdog driver

[Christer Weinigel]

[-- Attachment #1: Type: text/plain, Size: 422 bytes --]

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

I'm blaming the hangover.  *sigh*  I do this all the time...

I really have to turn on my brain sometime today :-)

  /Christer

-- 
"Nothing is as easy as it seems"

[-- Attachment #2: watchdog-api.txt --]
[-- Type: text/plain, Size: 11395 bytes --]

The Linux Watchdog driver API.

Copyright 2002 Christer Weingel <wingel@nano-system.com>

Some parts of this document are copied verbatim from the sbc60xxwdt
driver which is (c) Copyright 2000 Jakob Oestergaard <jakob@ostenfeld.dk>

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.

Re: [DRIVER][RFC] SC1200 Watchdog driver

[Joel Becker]

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/
			jlbec@evilplan.org

Re: [DRIVER][RFC] SC1200 Watchdog driver

[Roy Sigurd Karlsbakk]

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.

Re: [DRIVER][RFC] SC1200 Watchdog driver

[Zwane Mwaikambo]

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

Re: [DRIVER][RFC] SC1200 Watchdog driver

[Roy Sigurd Karlsbakk]

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

Re: [DRIVER][RFC] SC1200 Watchdog driver

[Randy.Dunlap]

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

Re: [DRIVER][RFC] SC1200 Watchdog driver

[Christer Weinigel]

Randy.Dunlap <rddunlap@osdl.org> 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?"

Re: [DRIVER][RFC] SC1200 Watchdog driver

[Christer Weinigel]

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 <wingel@nano-system.com>
+
+Some parts of this document are copied verbatim from the sbc60xxwdt
+driver which is (c) Copyright 2000 Jakob Oestergaard <jakob@ostenfeld.dk>
+
+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?"

Re: [DRIVER][RFC] SC1200 Watchdog driver

[Jakob Østergaard]

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 <jakob@ostenfeld.dk>

My e-mail address now is:  jakob@unthought.net

....
> +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   :)

-- 
................................................................
:   jakob@unthought.net   : 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}...............:

Re: [DRIVER][RFC] SC1200 Watchdog driver

[Alan Cox]

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