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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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.



[Index of Archives]     [Linux MIPS Home]     [LKML Archive]     [Linux ARM Kernel]     [Linux ARM]     [Linux]     [Git]     [Yosemite News]     [Linux SCSI]     [Linux Hams]

  Powered by Linux