Hi Gilles,
> +static ssize_t fop_write(struct file *file, const char __user *buf,
> + size_t count, loff_t *ppos)
> +{
> + size_t i;
> + char c;
> + int got_magic_char = 0;
> +
> + /* is there a magic char ? */
> + for (i = 0; i != count; i++) {
> + if (get_user(c, buf + i))
> + return -EFAULT;
> + if (c == SBC7240_MAGIC_CHAR) {
> + got_magic_char = 1;
> + break;
> + }
> + }
> +
> + if (got_magic_char)
> + wdt_disable();
> + else
> + wdt_keepalive();
> +
> + return count;
> +}
> +
> +static int fop_open(struct inode *inode, struct file *file)
> +{
> + if (test_and_set_bit(OPEN_STATUS_BIT, &wdt_status))
> + return -EBUSY;
> +
> + return nonseekable_open(inode, file);
> +}
> +
> +static int fop_close(struct inode *inode, struct file *file)
> +{
> + if (!nowayout)
> + wdt_disable();
> +
> + clear_bit(OPEN_STATUS_BIT, &wdt_status);
> + return 0;
> +}
Normal watchdog behaviour: The magic char should be written and then
/dev/watchdog can be closed and then the watchdog will stop.
What you did here was: if you write the magic char then stop the watchdog
even when the /dev/watchdog device is still open.
This is not the way that it should work.
> +static int __init sbc7240_wdt_init(void)
> +{
> + int rc = -EBUSY;
> +
> + if (!request_region(SBC7240_ENABLE_PORT, 1, "SBC7240 WDT")) {
> + printk(KERN_ERR SBC7240_PREFIX "I/O address 0x%04x already in
> use\n",
> + SBC7240_ENABLE_PORT);
> + rc = -EIO;
> + goto err_out;
> + }
> +
> + /* The IO port 0x043 used to disable the watchdog
> + * is already claimed by the system timer, so we
> + * cant request_region() it ...*/
> +
> + rc = misc_register(&wdt_miscdev);
> + if (rc) {
> + printk(KERN_ERR SBC7240_PREFIX
> + "cannot register miscdev on minor=%d (err=%d)\n",
> + wdt_miscdev.minor, rc);
> + goto err_out_region1;
> + }
> +
> + rc = register_reboot_notifier(&wdt_notifier);
> + if (rc) {
> + printk(KERN_ERR SBC7240_PREFIX
> + "cannot register reboot notifier (err=%d)\n", rc);
> + goto err_out_miscdev;
> + }
> +
> + if (timeout < 1 || timeout > SBC7240_MAX_TIMEOUT) {
> + timeout = SBC7240_TIMEOUT;
> + printk(KERN_INFO SBC7240_PREFIX
> + "timeout value must be 1<=x<=%d, using %d\n",
> + SBC7240_MAX_TIMEOUT, timeout);
> + }
> + wdt_set_timeout(timeout);
> +
> + printk(KERN_INFO SBC7240_PREFIX
> + "Watchdog driver for SBC7240 initialised (nowayout=%d)\n",
> + nowayout);
It's better to do the misc_register as last action. Reason herefor is that
you want to only enable the user-space interface when all the other settings
and interfacing was succesfull. Else you risk that userspace get's enabled and
that you then exit the watchdog driver.
Second problem that I see here is that you should make sure that the watchdog
device driver is stopped/disabled before that you give userspace access via
/dev/watchdog to the watchdog.
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/
