Hi Andrew,

thank you for your recommendations and corrections.
I've considered all and changed the code.

But first: Add support for it8718, which made the driver slightly more
complex.
Now the driver supports it8716, it8718, it8726, it8712-j and it8712-k,
which are all 16-bit watchdog timer on ITE lpc-super-io-chips.

Andrew Morton wrote:
quoted text
> On Wed, 27 Feb 2008 15:55:54 +0100
> Oliver Schuster <olivers137@aol.com> wrote:
> 
>> Hi,
>> this patch add a hardware watchdog device driver. It supports the
>> 16 bit watchdog timer on superio chip it8712f-j, it 8712f-k, it8716f
>> and it8726f. Today it was tested under vanilla 2.6.24.3, vanilla
>> 2.6.24.2 and Mandriva with 2.6.24.2 and 2.6.24 kernel.
>>
>> ...
>>
> 
>> static  int exclusiv = DEFAULT_EXCLUSIV;
> 
> Could we put the "e" back into "exclusive" please?

exclusive -- sorry 8)

quoted text
>> +	timeout = t;
>> +
>> +	if (wdt_status & BIT_MASK(WDTS_TIMER_RUN)) {
>> +		spin_lock_irqsave(&spinlock, flags);
> 
> Are you sure we are OK testing WDTS_TIMER_RUN prior to taking the lock? 
> Should that test happen inside the lock?

You are right, changed.
About the spinlock, i use it primary to ensure that at the same time 
only one task can have access to the superio chip configuration register.

quoted text
> The driver ..., but it uses this
> open-coded test when reading bits. ...

test_bit() is now used.


quoted text
> Is this comment about `buf' true?

Comment about wdt_write is changed.

quoted text
>> +				if (c == 'V')
>> +					expect_close = WD_MAGIC;
> 
> So we support a write of "VVVVVVVVVVV"?  Seems odd.

I prefer "echo V>/dev/watchdog" (in the case that nowayout is NOT set)
to stop the watchdog. But any write which contain the magic can stop the 
watchdog on next close of the device.
This code originates from softdog.c, now slightly changed.

quoted text
>> +/*
>> + *      wdt_ioctl:
>> + *      @inode: inode ...
 >> ...
quoted text
> 
> The comments in this file are _almost_ in kerneldoc format, but they
> actually aren't.  Please convert them fully (and test it!)

Most of comments are from the code, that i used as template. These
commentaries seems to be kerneldoc, but aren't. I changed this and add 
all needful.

To extract the kerneldoc, i like this colorful way:
scripts/kernel-doc -html ... | lynx -stdin

quoted text
>> +config IT87_WDT
>> +	tristate "IT87 Watchdog Timer"
>> +	depends on EXPERIMENTAL
>> +	---help---
>> +	  This is the ...
> 
> Surely this needs more dependencies.  The driver is x86(?)-specific and
> blindly pokes at IO ports prior to determining whether the device is
> actually present. ...

The driver hwmon/it87.c use the same way to detect the ITE superio chip 
by poking at IO ports. It dependence on HAS_IOMEM, but i decided to be 
more strictly and use X86, because the low pin count interface is 
created for this architecture and the IO base addresses used by these 
chips are X86 specific.

Regards,
Oliver