linux-kernel - [PATCH] add watchdog driver IT8716 IT8726 IT8712J/K

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [day] [month] [year] [list]

Message-ID: <47CEC0DD.1090507@aol.com>
Date:	Wed, 05 Mar 2008 16:48:45 +0100
From:	Oliver Schuster <olivers137@....com>
To:	linux-kernel@...r.kernel.org, wim@...ana.be
Subject: [PATCH] add watchdog driver IT8716 IT8726 IT8712J/K

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

 >Could we put the "e" back...

exclusive 8-)

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

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

test_bit() is now used.

 >Is this comment about `buf' true?

Comment about wdt_write is changed.

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

 >> +/*
 >> + *      wdt_ioctl:
 >> + *      @inode: inode ...

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

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

View attachment "2.6.24.3-it87_wdt-1.12.patch" of type "text/x-patch" (20226 bytes)