Hi Thomas,
I reviewed yor code and have ome remarks:
* All watchdog device drivers should work with seconds. Your driver works
with minutes. Please change to seconds.
* The misc_register function (which opens your driver towards user-space)
should be the last iregister-action you take into the pci probe function.
(So just before you do the printk where you say that the driver has been
initialized).
* The open and release functions should make sure that /dev/watchdog can
only be opened once.
* /dev/watchdog is a VFS (Virtual File System) so your open function should
return with "return nonseekable_open(inode, file);".
* please put the code to start the watchdog in a seperate function (this
will make it easier to migrate your driver to the generic watchdog model
in the future).
* If you watchdog can't be stopped then you should use a timer to make sure
that when /dev/watchdog is closed, that you continue to keepalive/ping the
watchdog device. (please see mixcomwd.c (this device has the same problem)).
* please put the code to change the timeout in a seperate function.
* the default return value for the ioctl calls should be -ENOTTY (instead of
-ENOIOCTLCMD).
* the GETSTATUS call should also have a "put_user(0, (int *)arg);".
* if possible please add sparse testing code into the ioctl handling.
* Personnaly: I would rather see the generic smbios code somewhere in the
arch/x86/ directory. We will probably need that also in other (non-watchdog)
drivers in the future.
Greetings,
Wim.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
