linux-mips
[Top] [All Lists]

Re: [PATCH] Added MIPS RM9K watchdog driver

To: thomas@koeller.dyndns.org
Subject: Re: [PATCH] Added MIPS RM9K watchdog driver
From: Alan Cox <alan@lxorguk.ukuu.org.uk>
Date: Fri, 11 Aug 2006 21:07:14 +0100
Cc: wim@iguana.be, linux-kernel@vger.kernel.org, Ralf Baechle <ralf@linux-mips.org>, linux-mips@linux-mips.org
In-reply-to: <200608102319.13679.thomas@koeller.dyndns.org>
Original-recipient: rfc822;linux-mips@linux-mips.org
References: <200608102319.13679.thomas@koeller.dyndns.org>
Sender: linux-mips-bounce@linux-mips.org
Ar Iau, 2006-08-10 am 23:19 +0200, ysgrifennodd
thomas@koeller.dyndns.org:
> +     wd_regs = ioremap_nocache(rr->start, rr->end + 1 - rr->start);

If this fails ?

> +     res = misc_register(&miscdev);
> +     if (res)
> +             iounmap(wd_regs);
> +     register_reboot_notifier(&wdt_gpi_shutdown);
> +     return res;

Failure path appears incomplete, surely you don't want to register a
reboot notifier then unload and error ?

> +                     copy_to_user((void __user *)arg, &wdinfo, size);

This function returns an error and should be checked. (The tricks with
IOC bits and verify_area aren't enough to be sure it won't error and
actually probably aren't worth doing)

> +     printk(KERN_WARNING "%s: watchdog expired - resetting system\n",
> +            wdt_gpi_name);
> +
> +     *(volatile char *) flagaddr |= 0x01;
> +     *(volatile char *) resetaddr = powercycle ? 0x01 : 0x2;
> +     iob();
> +     while (1) continue;

cpu_relax();
> +
> +     return IRQ_HANDLED;

Unreachable code.

Also if this is a software watchdog why is it better than using
softdog ?

Otherwise it looks pretty sound.


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