linux-mips
[Top] [All Lists]

Re: [PATCH][MIPS][5/7] AR7: watchdog timer

To: Matteo Croce <technoboy85@gmail.com>
Subject: Re: [PATCH][MIPS][5/7] AR7: watchdog timer
From: Wim Van Sebroeck <wim@iguana.be>
Date: Wed, 3 Oct 2007 21:24:14 +0200
Cc: linux-mips@linux-mips.org, Nicolas Thill <nico@openwrt.org>, Enrik Berkhan <Enrik.Berkhan@akk.org>, Christer Weinigel <wingel@nano-system.com>, Andrew Morton <akpm@linux-foundation.org>
In-reply-to: <200709201806.41942.technoboy85@gmail.com>
Original-recipient: rfc822;linux-mips@linux-mips.org
References: <200709201728.10866.technoboy85@gmail.com> <200709201806.41942.technoboy85@gmail.com>
Sender: linux-mips-bounce@linux-mips.org
User-agent: Mutt/1.4.2.1i
Hi Matteo,

Sorry for the late response. Some personal/work issues
prevented me in reacting faster.

> +static int ar7_wdt_open(struct inode *inode, struct file *file)
> +{
> +     /* only allow one at a time */
> +     if (down_trylock(&open_semaphore))
> +             return -EBUSY;
> +     ar7_wdt_enable_wdt();
> +     expect_close = 0;
> +
> +     return 0;
> +}

The /dev/watchdog device is a VFS (Virtual File System). We thus
use a: return nonseekable_open(inode, file);

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

Since we use the nonseekable_open we don't need the
 if (*ppos != file->f_pos) return -ESPIPE;

> +static int __init ar7_wdt_init(void)
> +{
...
> +     rc = misc_register(&ar7_wdt_miscdev);
> +     if (rc) {
> +             printk(KERN_ERR DRVNAME ": unable to register misc device\n");
> +             goto out_alloc;
> +     }
> +
> +     rc = register_reboot_notifier(&ar7_wdt_notifier);
> +     if (rc) {
> +             printk(KERN_ERR DRVNAME
> +                     ": unable to register reboot notifier\n");
> +             goto out_register;
> +     }
> +     goto out;
> +
> +out_register:
> +     misc_deregister(&ar7_wdt_miscdev);
> +out_alloc:
> +     release_mem_region(ar7_regs_wdt, sizeof(struct ar7_wdt));
> +out:
> +     return rc;
> +}

It's better to first register the reboot-notifier instead of
registering the misc-device. The misc-device gives userspace
allready access to the device and that's something that you
want to do as the last action to prevent problems.

For the rest: all OK.

If you want I'll add it to the linux-2.6-watchdog-mm tree with
the above mentioned changes.

Greetings,
Wim.


<Prev in Thread] Current Thread [Next in Thread>