This is a driver for the on-chip watchdog device found on some
MIPS RM9000 processors.
Signed-off-by: Thomas Koeller <[email protected]>
---
drivers/char/watchdog/Kconfig | 10 +
drivers/char/watchdog/Makefile | 1
drivers/char/watchdog/rm9k_wdt.c | 435
++++++++++++++++++++++++++++++++++++++
3 files changed, 446 insertions(+), 0 deletions(-)
diff --git a/drivers/char/watchdog/Kconfig b/drivers/char/watchdog/Kconfig
index d53f664..3207299 100644
--- a/drivers/char/watchdog/Kconfig
+++ b/drivers/char/watchdog/Kconfig
@@ -477,6 +477,16 @@ config INDYDOG
timer expired and no process has written to /dev/watchdog during
that time.
+config WDT_RM9K_GPI
+ tristate "RM9000/GPI hardware watchdog"
+ depends on WATCHDOG && CPU_RM9000
+ help
+ Watchdog implementation using the GPI hardware found on
+ PMC-Sierra RM9xxx CPUs.
+
+ To compile this driver as a module, choose M here: the
+ module will be called rm9k_wdt.
+
# S390 Architecture
config ZVM_WATCHDOG
diff --git a/drivers/char/watchdog/Makefile b/drivers/char/watchdog/Makefile
index 6ab77b6..f2a6281 100644
--- a/drivers/char/watchdog/Makefile
+++ b/drivers/char/watchdog/Makefile
@@ -67,6 +67,7 @@ obj-$(CONFIG_WATCHDOG_RTAS) += wdrtas.o
# MIPS Architecture
obj-$(CONFIG_INDYDOG) += indydog.o
+obj-$(CONFIG_WDT_RM9K_GPI) += rm9k_wdt.o
# S390 Architecture
diff --git a/drivers/char/watchdog/rm9k_wdt.c
b/drivers/char/watchdog/rm9k_wdt.c
new file mode 100644
index 0000000..f6a9d17
--- /dev/null
+++ b/drivers/char/watchdog/rm9k_wdt.c
@@ -0,0 +1,435 @@
+/*
+ * Watchdog implementation for GPI h/w found on PMC-Sierra RM9xxx
+ * chips.
+ *
+ * Copyright (C) 2006 by Basler Vision Technologies AG
+ * Author: Thomas Koeller <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+ */
+
+#include <linux/config.h>
+#include <linux/platform_device.h>
+#include <linux/module.h>
+#include <linux/moduleparam.h>
+#include <linux/interrupt.h>
+#include <linux/fs.h>
+#include <linux/reboot.h>
+#include <linux/miscdevice.h>
+#include <linux/watchdog.h>
+#include <asm/io.h>
+#include <asm/atomic.h>
+#include <asm/uaccess.h>
+#include <asm/system.h>
+#include <asm/rm9k-ocd.h>
+
+#include <rm9k_wdt.h>
+
+
+#define CLOCK 125000000
+#define MAX_TIMEOUT_SECONDS 32
+#define CPCCR 0x0080
+#define CPGIG1SR 0x0044
+#define CPGIG1ER 0x0054
+
+
+
+/* Function prototypes */
+static int __init wdt_gpi_probe(struct device *);
+static int __exit wdt_gpi_remove(struct device *);
+static void wdt_gpi_set_timeout(unsigned int);
+static int wdt_gpi_open(struct inode *, struct file *);
+static int wdt_gpi_release(struct inode *, struct file *);
+static ssize_t wdt_gpi_write(struct file *, const char __user *, size_t,
loff_t *);
+static long wdt_gpi_ioctl(struct file *, unsigned int, unsigned long);
+static const struct resource *wdt_gpi_get_resource(struct platform_device *,
const char *, unsigned int);
+static int wdt_gpi_notify(struct notifier_block *, unsigned long, void *);
+static irqreturn_t wdt_gpi_irqhdl(int, void *, struct pt_regs *);
+
+
+
+
+static const char wdt_gpi_name[] = "wdt_gpi";
+static atomic_t opencnt;
+static int expect_close;
+static int locked = 0;
+
+
+
+/* These are set from device resources */
+static void __iomem * wd_regs;
+static unsigned int wd_irq, wd_ctr;
+
+
+
+/* Module arguments */
+static int timeout = MAX_TIMEOUT_SECONDS;
+module_param(timeout, int, 444);
+static unsigned long resetaddr = 0xbffdc200;
+module_param(resetaddr, ulong, 444);
+static unsigned long flagaddr = 0xbffdc104;
+module_param(flagaddr, ulong, 444);
+static int powercycle = 0;
+module_param(powercycle, bool, 444);
+
+static int nowayout =
+#if defined(CONFIG_WATCHDOG_NOWAYOUT)
+ 1;
+#else
+ 0;
+#endif
+module_param(nowayout, bool, 444);
+
+
+
+static struct file_operations fops =
+{
+ .owner = THIS_MODULE,
+ .open = wdt_gpi_open,
+ .release = wdt_gpi_release,
+ .write = wdt_gpi_write,
+ .unlocked_ioctl = wdt_gpi_ioctl
+};
+
+static struct miscdevice miscdev =
+{
+ .minor = WATCHDOG_MINOR,
+ .name = wdt_gpi_name,
+ .fops = &fops
+};
+
+static struct device_driver wdt_gpi_driver =
+{
+ .name = (char *) wdt_gpi_name,
+ .bus = &platform_bus_type,
+ .owner = THIS_MODULE,
+ .probe = wdt_gpi_probe,
+ .remove = __exit_p(wdt_gpi_remove),
+ .shutdown = NULL,
+ .suspend = NULL,
+ .resume = NULL
+};
+
+static struct notifier_block wdt_gpi_shutdown =
+{
+ wdt_gpi_notify
+};
+
+
+
+static const struct resource *
+wdt_gpi_get_resource(struct platform_device *pdv, const char *name,
+ unsigned int type)
+{
+ char buf[80];
+ if (snprintf(buf, sizeof buf, "%s_0", name) >= sizeof buf)
+ return NULL;
+ return platform_get_resource_byname(pdv, type, buf);
+}
+
+
+
+/* No hotplugging on the platform bus - use __init */
+static int __init wdt_gpi_probe(struct device *dev)
+{
+ int res;
+ struct platform_device * const pdv = to_platform_device(dev);
+ const struct resource
+ * const rr = wdt_gpi_get_resource(pdv, WDT_RESOURCE_REGS,
+ IORESOURCE_MEM),
+ * const ri = wdt_gpi_get_resource(pdv, WDT_RESOURCE_IRQ,
+ IORESOURCE_IRQ),
+ * const rc = wdt_gpi_get_resource(pdv, WDT_RESOURCE_COUNTER,
+ 0);
+
+ if (unlikely(!rr || !ri || !rc))
+ return -ENXIO;
+
+ wd_regs = ioremap_nocache(rr->start, rr->end + 1 - rr->start);
+ wd_irq = ri->start;
+ wd_ctr = rc->start;
+ res = misc_register(&miscdev);
+ if (res)
+ iounmap(wd_regs);
+ register_reboot_notifier(&wdt_gpi_shutdown);
+ return res;
+}
+
+
+
+static int __exit wdt_gpi_remove(struct device *dev)
+{
+ int res;
+
+ unregister_reboot_notifier(&wdt_gpi_shutdown);
+ res = misc_deregister(&miscdev);
+ iounmap(wd_regs);
+ wd_regs = NULL;
+ return res;
+}
+
+
+static void wdt_gpi_set_timeout(unsigned int to)
+{
+ u32 reg;
+ const u32 wdval = (to * CLOCK) & ~0x0000000f;
+
+ lock_titan_regs();
+ reg = titan_readl(CPCCR) & ~(0xf << (wd_ctr * 4));
+ titan_writel(reg, CPCCR);
+ wmb();
+ __raw_writel(wdval, wd_regs + 0x0000);
+ wmb();
+ titan_writel(reg | (0x2 << (wd_ctr * 4)), CPCCR);
+ wmb();
+ titan_writel(reg | (0x5 << (wd_ctr * 4)), CPCCR);
+ iob();
+ unlock_titan_regs();
+}
+
+
+
+static int wdt_gpi_open(struct inode *i, struct file *f)
+{
+ int res;
+ u32 reg;
+
+ if (unlikely(0 > atomic_dec_if_positive(&opencnt)))
+ return -EBUSY;
+
+ expect_close = 0;
+ if (locked) {
+ module_put(THIS_MODULE);
+ free_irq(wd_irq, &miscdev);
+ locked = 0;
+ }
+
+ res = request_irq(wd_irq, wdt_gpi_irqhdl, SA_SHIRQ | SA_INTERRUPT,
+ wdt_gpi_name, &miscdev);
+ if (unlikely(res))
+ return res;
+
+ wdt_gpi_set_timeout(timeout);
+
+ lock_titan_regs();
+ reg = titan_readl(CPGIG1ER);
+ titan_writel(reg | (0x100 << wd_ctr), CPGIG1ER);
+ iob();
+ unlock_titan_regs();
+
+ printk(KERN_INFO "%s: watchdog started, timeout = %u seconds\n",
+ wdt_gpi_name, timeout);
+ return 0;
+}
+
+
+
+static int wdt_gpi_release(struct inode *i, struct file *f)
+{
+ if (nowayout) {
+ printk(KERN_NOTICE "%s: no way out - watchdog left running\n",
+ wdt_gpi_name);
+ __module_get(THIS_MODULE);
+ locked = 1;
+ } else {
+ if (expect_close) {
+ u32 reg;
+
+ lock_titan_regs();
+ reg = titan_readl(CPCCR) & ~(0xf << (wd_ctr * 4));
+ titan_writel(reg, CPCCR);
+ reg = titan_readl(CPGIG1ER);
+ titan_writel(reg & ~(0x100 << wd_ctr), CPGIG1ER);
+ iob();
+ unlock_titan_regs();
+ free_irq(wd_irq, &miscdev);
+ printk(KERN_INFO "%s: watchdog stopped\n", wdt_gpi_name);
+ } else {
+ printk(KERN_NOTICE "%s: unexpected close() -"
+ " watchdog left running\n",
+ wdt_gpi_name);
+ wdt_gpi_set_timeout(timeout);
+ __module_get(THIS_MODULE);
+ locked = 1;
+ }
+ }
+
+ atomic_inc(&opencnt);
+ return 0;
+}
+
+
+
+static ssize_t
+wdt_gpi_write(struct file *f, const char __user *d, size_t s, loff_t *o)
+{
+ char val;
+
+ wdt_gpi_set_timeout(timeout);
+ expect_close = (s > 0) && !get_user(val, d) && (val == 'V');
+ return s ? 1 : 0;
+}
+
+
+
+static long
+wdt_gpi_ioctl(struct file *f, unsigned int cmd, unsigned long arg)
+{
+ long res = -ENOTTY;
+ const long size = _IOC_SIZE(cmd);
+ int stat;
+ static struct watchdog_info wdinfo = {
+ .identity = "RM9xxx/GPI watchdog",
+ .firmware_version = 0,
+ .options = WDIOF_SETTIMEOUT | WDIOF_KEEPALIVEPING
+ };
+
+ if (unlikely(_IOC_TYPE(cmd) != WATCHDOG_IOCTL_BASE))
+ return -ENOTTY;
+
+ if ((_IOC_DIR(cmd) & _IOC_READ)
+ && !access_ok(VERIFY_WRITE, arg, size))
+ return -EFAULT;
+
+ if ((_IOC_DIR(cmd) & _IOC_WRITE)
+ && !access_ok(VERIFY_READ, arg, size))
+ return -EFAULT;
+
+ expect_close = 0;
+
+ switch (cmd) {
+ case WDIOC_GETSUPPORT:
+ wdinfo.options = nowayout ?
+ WDIOF_SETTIMEOUT | WDIOF_KEEPALIVEPING :
+ WDIOF_SETTIMEOUT | WDIOF_KEEPALIVEPING | WDIOF_MAGICCLOSE;
+ copy_to_user((void __user *)arg, &wdinfo, size);
+ res = size;
+ break;
+
+ case WDIOC_GETSTATUS:
+ break;
+
+ case WDIOC_GETBOOTSTATUS:
+ stat = (*(volatile char *) flagaddr & 0x01)
+ ? WDIOF_CARDRESET : 0;
+ copy_to_user((void __user *)arg, &stat, size);
+ res = size;
+ break;
+
+ case WDIOC_SETOPTIONS:
+ break;
+
+ case WDIOC_KEEPALIVE:
+ wdt_gpi_set_timeout(timeout);
+ res = size;
+ break;
+
+ case WDIOC_SETTIMEOUT:
+ {
+ int val;
+ __copy_from_user(&val, (const void __user *) arg,
+ size);
+ if (val > 32)
+ val = 32;
+
+ timeout = val;
+ wdt_gpi_set_timeout(val);
+ res = size;
+ printk("%s: timeout set to %u seconds\n",
+ wdt_gpi_name, timeout);
+ }
+ break;
+
+ case WDIOC_GETTIMEOUT:
+ __copy_to_user((void __user *) arg, &timeout, size);
+ res = size;
+ break;
+ }
+
+ return res;
+}
+
+
+
+
+static irqreturn_t wdt_gpi_irqhdl(int irq, void *ctxt, struct pt_regs *regs)
+{
+ if (!unlikely(__raw_readl(wd_regs + 0x0008) & 0x1))
+ return IRQ_NONE;
+ __raw_writel(0x1, wd_regs + 0x0008);
+
+
+ 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;
+
+ return IRQ_HANDLED;
+}
+
+
+
+static int
+wdt_gpi_notify(struct notifier_block *this, unsigned long code, void *unused)
+{
+ if(code == SYS_DOWN || code == SYS_HALT) {
+ u32 reg;
+
+ lock_titan_regs();
+ reg = titan_readl(CPCCR) & ~(0xf << (wd_ctr * 4));
+ titan_writel(reg, CPCCR);
+ reg = titan_readl(CPGIG1ER);
+ titan_writel(reg & ~(0x100 << wd_ctr), CPGIG1ER);
+ iob();
+ unlock_titan_regs();
+ }
+ return NOTIFY_DONE;
+}
+
+
+
+static int __init wdt_gpi_init_module(void)
+{
+ atomic_set(&opencnt, 1);
+ if (timeout > MAX_TIMEOUT_SECONDS)
+ timeout = MAX_TIMEOUT_SECONDS;
+ return driver_register(&wdt_gpi_driver);
+}
+
+
+
+static void __exit wdt_gpi_cleanup_module(void)
+{
+ driver_unregister(&wdt_gpi_driver);
+}
+
+module_init(wdt_gpi_init_module);
+module_exit(wdt_gpi_cleanup_module);
+
+
+
+MODULE_AUTHOR("Thomas Koeller <[email protected]>");
+MODULE_DESCRIPTION("Basler eXcite watchdog driver for gpi devices");
+MODULE_VERSION("0.1");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS_MISCDEV(WATCHDOG_MINOR);
+MODULE_PARM_DESC(timeout, "Watchdog timeout in seconds");
+MODULE_PARM_DESC(resetaddr, "Address to write to to force a reset");
+MODULE_PARM_DESC(flagaddr, "Address to write to boot flags to");
+MODULE_PARM_DESC(nowayout, "Watchdog cannot be disabled once started");
+MODULE_PARM_DESC(powercycle, "Cycle power if watchdog expires");
--
1.4.0
--
Thomas Koeller, Software Development
Basler Vision Technologies
An der Strusbek 60-62
22926 Ahrensburg
Germany
Tel +49 (4102) 463-390
Fax +49 (4102) 463-46390
mailto:[email protected]
http://www.baslerweb.com
Ar Iau, 2006-08-10 am 23:19 +0200, ysgrifennodd
[email protected]:
> + 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.
On Thu, Aug 10, 2006 at 11:19:13PM +0200, [email protected] wrote:
> This is a driver for the on-chip watchdog device found on some
> MIPS RM9000 processors.
>
> Signed-off-by: Thomas Koeller <[email protected]>
Mostly same nit-picking comments as your other driver..
> +++ b/drivers/char/watchdog/rm9k_wdt.c
> ...
> +
> +#include <linux/config.h>
not needed.
> +/* Function prototypes */
> +static int __init wdt_gpi_probe(struct device *);
> +static int __exit wdt_gpi_remove(struct device *);
> +static void wdt_gpi_set_timeout(unsigned int);
> +static int wdt_gpi_open(struct inode *, struct file *);
> +static int wdt_gpi_release(struct inode *, struct file *);
> +static ssize_t wdt_gpi_write(struct file *, const char __user *, size_t,
> loff_t *);
> +static long wdt_gpi_ioctl(struct file *, unsigned int, unsigned long);
> +static const struct resource *wdt_gpi_get_resource(struct platform_device *,
> const char *, unsigned int);
> +static int wdt_gpi_notify(struct notifier_block *, unsigned long, void *);
> +static irqreturn_t wdt_gpi_irqhdl(int, void *, struct pt_regs *);
Can probably (mostly?) go away with some creative reordering.
> +static int locked = 0;
unneeded initialisation.
> +static int nowayout =
> +#if defined(CONFIG_WATCHDOG_NOWAYOUT)
> + 1;
> +#else
> + 0;
> +#endif
static int nowayout = CONFIG_WATCHDOG_NOWAYOUT;
should work.
> +static void wdt_gpi_set_timeout(unsigned int to)
> +{
> + u32 reg;
> + const u32 wdval = (to * CLOCK) & ~0x0000000f;
> +
> + lock_titan_regs();
> + reg = titan_readl(CPCCR) & ~(0xf << (wd_ctr * 4));
> + titan_writel(reg, CPCCR);
> + wmb();
> + __raw_writel(wdval, wd_regs + 0x0000);
> + wmb();
> + titan_writel(reg | (0x2 << (wd_ctr * 4)), CPCCR);
> + wmb();
> + titan_writel(reg | (0x5 << (wd_ctr * 4)), CPCCR);
> + iob();
> + unlock_titan_regs();
> +}
As in the previous driver, are these barriers strong enough?
Or do they need explicit reads of the written addresses to flush the write?
Dave
--
http://www.codemonkey.org.uk
On Friday 11 August 2006 22:56, Dave Jones wrote:
> On Thu, Aug 10, 2006 at 11:19:13PM +0200, [email protected] wrote:
> > This is a driver for the on-chip watchdog device found on some
> > MIPS RM9000 processors.
> >
> > Signed-off-by: Thomas Koeller <[email protected]>
>
> Mostly same nit-picking comments as your other driver..
Which one?
>
> > +++ b/drivers/char/watchdog/rm9k_wdt.c
> > ...
> > +
> > +#include <linux/config.h>
>
> not needed.
It is, otherwise I do not get CONFIG_WATCHDOG_NOWAYOUT.
>
> > +/* Function prototypes */
> > +static int __init wdt_gpi_probe(struct device *);
> > +static int __exit wdt_gpi_remove(struct device *);
> > +static void wdt_gpi_set_timeout(unsigned int);
> > +static int wdt_gpi_open(struct inode *, struct file *);
> > +static int wdt_gpi_release(struct inode *, struct file *);
> > +static ssize_t wdt_gpi_write(struct file *, const char __user *,
> > size_t, loff_t *);
> > +static long wdt_gpi_ioctl(struct file *, unsigned int, unsigned long);
> > +static const struct resource *wdt_gpi_get_resource(struct
> > platform_device *, const char *, unsigned int);
> > +static int wdt_gpi_notify(struct notifier_block *, unsigned long, void
> > *); +static irqreturn_t wdt_gpi_irqhdl(int, void *, struct pt_regs *);
>
> Can probably (mostly?) go away with some creative reordering.
Probably, but should it? I always considered it good style to have
prototypes for all functions.
>
> > +static int locked = 0;
>
> unneeded initialisation.
Not strictly needed, that's true, but does not do any harm either
and expresses the intention clearly.
>
> > +static int nowayout =
> > +#if defined(CONFIG_WATCHDOG_NOWAYOUT)
> > + 1;
> > +#else
> > + 0;
> > +#endif
>
> static int nowayout = CONFIG_WATCHDOG_NOWAYOUT;
>
> should work.
Does not work. If the option is not selected, CONFIG_WATCHDOG_NOWAYOUT
is undefined, not zero.
>
> > +static void wdt_gpi_set_timeout(unsigned int to)
> > +{
> > + u32 reg;
> > + const u32 wdval = (to * CLOCK) & ~0x0000000f;
> > +
> > + lock_titan_regs();
> > + reg = titan_readl(CPCCR) & ~(0xf << (wd_ctr * 4));
> > + titan_writel(reg, CPCCR);
> > + wmb();
> > + __raw_writel(wdval, wd_regs + 0x0000);
> > + wmb();
> > + titan_writel(reg | (0x2 << (wd_ctr * 4)), CPCCR);
> > + wmb();
> > + titan_writel(reg | (0x5 << (wd_ctr * 4)), CPCCR);
> > + iob();
> > + unlock_titan_regs();
> > +}
>
> As in the previous driver, are these barriers strong enough?
> Or do they need explicit reads of the written addresses to flush the write?
I think they are. Remember, the entire device is integrated in the
processor. No external buses involved.
>
> Dave
Thanks for your comments!
Thomas
--
Thomas Koeller
[email protected]
On Sat, Aug 12, 2006 at 01:49:23AM +0200, Thomas Koeller wrote:
> On Friday 11 August 2006 22:56, Dave Jones wrote:
> > On Thu, Aug 10, 2006 at 11:19:13PM +0200, [email protected] wrote:
> > > This is a driver for the on-chip watchdog device found on some
> > > MIPS RM9000 processors.
> > >
> > > Signed-off-by: Thomas Koeller <[email protected]>
> >
> > Mostly same nit-picking comments as your other driver..
>
> Which one?
The image capture driver.
> > > +#include <linux/config.h>
> >
> > not needed.
>
> It is, otherwise I do not get CONFIG_WATCHDOG_NOWAYOUT.
kbuild automatically includes it for you in the last few kernels.
> > As in the previous driver, are these barriers strong enough?
> > Or do they need explicit reads of the written addresses to flush the write?
>
> I think they are. Remember, the entire device is integrated in the
> processor. No external buses involved.
Ok.
Dave
--
http://www.codemonkey.org.uk
Thomas Koeller wrote:
> On Friday 11 August 2006 22:56, Dave Jones wrote:
>
>>On Thu, Aug 10, 2006 at 11:19:13PM +0200, [email protected] wrote:
>> > This is a driver for the on-chip watchdog device found on some
>> > MIPS RM9000 processors.
>> >
>> > Signed-off-by: Thomas Koeller <[email protected]>
>>
>>Mostly same nit-picking comments as your other driver..
>
>
> Which one?
>
<http://permalink.gmane.org/gmane.linux.ports.mips.general/13500>
--
-o--=O`C
#oo'L O
<___=E M
On Fri, Aug 11, 2006 at 08:06:36PM -0400, Dave Jones wrote:
> > I think they are. Remember, the entire device is integrated in the
> > processor. No external buses involved.
>
> Ok.
I think it's ok in this case, we're unlikely to see these peripherals
on other chips than PMC-Sierra's - but that's of course just a guess.
With a broader perspective the embedded world is increasingly converging
to using standardized components (so called "IP") and on-chip
interconnects such as OCP for new designs and portable drivers should
reflect this.
Ralf
On Friday 11 August 2006 22:07, Alan Cox wrote:
> > + 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();
I tried to find out about the purpose of cpu_relax(). On MIPS, at least,
it maps to barrier(). I do not quite understand why I would need a
barrier() in this place. Would you, or someone else, care to
enlighten me?
I am sending a revised patch in a separate mail.
Thomas
--
Thomas Koeller
[email protected]
On Friday 11 August 2006 22:07, Alan Cox wrote:
> Also if this is a software watchdog why is it better than using
> softdog ?
>
This is _not_ a software watchdog. If the timer expires, an interrupt
is generated, and the timer is reset to count through another cycle.
If it expires again, it resets the CPU.
Thomas
--
Thomas Koeller, Software Development
Basler Vision Technologies
An der Strusbek 60-62
22926 Ahrensburg
Germany
Tel +49 (4102) 463-390
Fax +49 (4102) 463-46390
mailto:[email protected]
http://www.baslerweb.com
Ar Sad, 2006-08-12 am 19:45 +0200, ysgrifennodd Thomas Koeller:
> On Friday 11 August 2006 22:07, Alan Cox wrote:
> > Also if this is a software watchdog why is it better than using
> > softdog ?
> >
>
> This is _not_ a software watchdog. If the timer expires, an interrupt
> is generated, and the timer is reset to count through another cycle.
> If it expires again, it resets the CPU.
Ok thanks, then it does make sense for it to be in kernel.
On Sat, Aug 12, 2006 at 06:06:02PM +0200, Thomas Koeller wrote:
> > > + while (1) continue;
> >
> > cpu_relax();
>
> I tried to find out about the purpose of cpu_relax(). On MIPS, at least,
> it maps to barrier(). I do not quite understand why I would need a
> barrier() in this place. Would you, or someone else, care to
> enlighten me?
Busy wait loops are meant to be filled with cpu_relax() in Linux. On
processors like the Pentium 4 this expands into something that keeps
the CPU from consuming excessive amounts of energy for just twiddling
thumbs and probably also CPU dependant. On MIPS cpu_relax() so far is
meaningless and therfore just defined as barrier().
Ralf
On Sat, Aug 12, 2006 at 01:49:23AM +0200, Thomas Koeller wrote:
> On Friday 11 August 2006 22:56, Dave Jones wrote:
> > On Thu, Aug 10, 2006 at 11:19:13PM +0200, [email protected] wrote:
> > > +
> > > +#include <linux/config.h>
> >
> > not needed.
>
> It is, otherwise I do not get CONFIG_WATCHDOG_NOWAYOUT.
It shouldn't be necessary, so it's probably a bug. I could not
begin to tell you where.
I've CC'd the Kbuild maintainer -- apologies if I'm way off base
here.
Still, if this #include is to stay, you'd probably better comment
it, or it's likely someone will rip it out in a cleanup:
http://www.gossamer-threads.com/lists/linux/kernel/663918
>
> >
> > > +static int locked = 0;
> >
> > unneeded initialisation.
>
> Not strictly needed, that's true, but does not do any harm either
> and expresses the intention clearly.
My meager understanding is that it makes the kernel image bigger.
> >
> > > +static int nowayout =
> > > +#if defined(CONFIG_WATCHDOG_NOWAYOUT)
> > > + 1;
> > > +#else
> > > + 0;
> > > +#endif
> >
> > static int nowayout = CONFIG_WATCHDOG_NOWAYOUT;
> >
> > should work.
>
> Does not work. If the option is not selected, CONFIG_WATCHDOG_NOWAYOUT
> is undefined, not zero.
Possibly related?
--
Joseph Fannin
[email protected] || [email protected]
On Mon, Aug 14, 2006 at 10:14:45AM -0400, Joseph Fannin wrote:
> On Sat, Aug 12, 2006 at 01:49:23AM +0200, Thomas Koeller wrote:
> > On Friday 11 August 2006 22:56, Dave Jones wrote:
> > > On Thu, Aug 10, 2006 at 11:19:13PM +0200, [email protected] wrote:
> > > > +
> > > > +#include <linux/config.h>
> > >
> > > not needed.
> >
> > It is, otherwise I do not get CONFIG_WATCHDOG_NOWAYOUT.
Yes you do - try it.
make V=1 tells you that -include include/linux/autoconf.h pulls in the
CONFIG_ definitions.
Sam
Hi All,
> > > +#include <linux/config.h>
> >
> > not needed.
>
> It is, otherwise I do not get CONFIG_WATCHDOG_NOWAYOUT.
We'll fix this in the watchdog.h include file.
> > > +static int nowayout =
> > > +#if defined(CONFIG_WATCHDOG_NOWAYOUT)
> > > + 1;
> > > +#else
> > > + 0;
> > > +#endif
> >
> > static int nowayout = CONFIG_WATCHDOG_NOWAYOUT;
> >
> > should work.
>
> Does not work. If the option is not selected, CONFIG_WATCHDOG_NOWAYOUT
> is undefined, not zero.
This should be:
static int nowayout = WATCHDOG_NOWAYOUT;
Can you resent me your latest diff?
Thanks,
Wim.
On Mon, 14 Aug 2006 17:30:33 +0200 Sam Ravnborg wrote:
> On Mon, Aug 14, 2006 at 10:14:45AM -0400, Joseph Fannin wrote:
> > On Sat, Aug 12, 2006 at 01:49:23AM +0200, Thomas Koeller wrote:
> > > On Friday 11 August 2006 22:56, Dave Jones wrote:
> > > > On Thu, Aug 10, 2006 at 11:19:13PM +0200,
> > > > [email protected] wrote:
> > > > > +
> > > > > +#include <linux/config.h>
> > > >
> > > > not needed.
> > >
> > > It is, otherwise I do not get CONFIG_WATCHDOG_NOWAYOUT.
> Yes you do - try it.
> make V=1 tells you that -include include/linux/autoconf.h pulls in
> the CONFIG_ definitions.
Sure, autoconf.h is included, but I think his point is that
CONFIG_WATCHDOG_NOWAYOUT may not be defined there at all,
as in my 2.6.18-rc4 autoconf.h file, since my .config file says:
# CONFIG_WATCHDOG_NOWAYOUT is not set
---
~Randy
On Mon, Aug 14, 2006 at 09:21:24AM -0700, Randy.Dunlap wrote:
> On Mon, 14 Aug 2006 17:30:33 +0200 Sam Ravnborg wrote:
>
> > On Mon, Aug 14, 2006 at 10:14:45AM -0400, Joseph Fannin wrote:
> > > On Sat, Aug 12, 2006 at 01:49:23AM +0200, Thomas Koeller wrote:
> > > > On Friday 11 August 2006 22:56, Dave Jones wrote:
> > > > > On Thu, Aug 10, 2006 at 11:19:13PM +0200,
> > > > > [email protected] wrote:
> > > > > > +
> > > > > > +#include <linux/config.h>
> > > > >
> > > > > not needed.
> > > >
> > > > It is, otherwise I do not get CONFIG_WATCHDOG_NOWAYOUT.
> > Yes you do - try it.
> > make V=1 tells you that -include include/linux/autoconf.h pulls in
> > the CONFIG_ definitions.
>
> Sure, autoconf.h is included, but I think his point is that
> CONFIG_WATCHDOG_NOWAYOUT may not be defined there at all,
> as in my 2.6.18-rc4 autoconf.h file, since my .config file says:
> # CONFIG_WATCHDOG_NOWAYOUT is not set
As Wim pointed out, linux/watchdog.h has this magick..
#ifdef CONFIG_WATCHDOG_NOWAYOUT
#define WATCHDOG_NOWAYOUT 1
#else
#define WATCHDOG_NOWAYOUT 0
#endif
Which takes some of the ugly out of the drivers :-)
Dave
--
http://www.codemonkey.org.uk
Randy> Sure, autoconf.h is included, but I think his point is that
Randy> CONFIG_WATCHDOG_NOWAYOUT may not be defined there at all,
Randy> as in my 2.6.18-rc4 autoconf.h file, since my .config file
Randy> says: # CONFIG_WATCHDOG_NOWAYOUT is not set
Huh? How would including <linux/config.h> help with that? And why
would you want CONFIG_WATCHDOG_NOWAYOUT to be defined if
WATCHDOG_NOWAYOUT is not set in your configuration? That would
utterly break code that does something like
#ifdef CONFIG_WATCHDOG_NOWAYOUT
- R.
[email protected] ha scritto:
> diff --git a/drivers/char/watchdog/rm9k_wdt.c
> b/drivers/char/watchdog/rm9k_wdt.c
> new file mode 100644
> index 0000000..f6a9d17
> --- /dev/null
> +++ b/drivers/char/watchdog/rm9k_wdt.c
> @@ -0,0 +1,435 @@
[...]
> +/* Module arguments */
> +static int timeout = MAX_TIMEOUT_SECONDS;
> +module_param(timeout, int, 444);
> +static unsigned long resetaddr = 0xbffdc200;
> +module_param(resetaddr, ulong, 444);
> +static unsigned long flagaddr = 0xbffdc104;
> +module_param(flagaddr, ulong, 444);
> +static int powercycle = 0;
> +module_param(powercycle, bool, 444);
File permissions should be in octal ;)
Luca
--
Home: http://kronoz.cjb.net
"New processes are created by other processes, just like new
humans. New humans are created by other humans, of course,
not by processes." -- Unix System Administration Handbook
On Monday 14 August 2006 17:50, Wim Van Sebroeck wrote:
> Hi All,
>
> > > > +#include <linux/config.h>
> > >
> > > not needed.
> >
> > It is, otherwise I do not get CONFIG_WATCHDOG_NOWAYOUT.
>
> We'll fix this in the watchdog.h include file.
>
> > > > +static int nowayout =
> > > > +#if defined(CONFIG_WATCHDOG_NOWAYOUT)
> > > > + 1;
> > > > +#else
> > > > + 0;
> > > > +#endif
> > >
> > > static int nowayout = CONFIG_WATCHDOG_NOWAYOUT;
> > >
> > > should work.
> >
> > Does not work. If the option is not selected, CONFIG_WATCHDOG_NOWAYOUT
> > is undefined, not zero.
>
> This should be:
> static int nowayout = WATCHDOG_NOWAYOUT;
Yes, now I see.
>
> Can you resent me your latest diff?
Here it is. Several people have been pointing out that the filr permission
masks for the sysfs files should be in octal, I fixed that as well.
---------------------------------------------------------------------------
This is a driver for the on-chip watchdog device found on some
MIPS RM9000 processors.
Signed-off-by: Thomas Koeller <[email protected]>
---
drivers/char/watchdog/Kconfig | 10 +
drivers/char/watchdog/Makefile | 1
drivers/char/watchdog/rm9k_wdt.c | 431
++++++++++++++++++++++++++++++++++++++
3 files changed, 442 insertions(+), 0 deletions(-)
diff --git a/drivers/char/watchdog/Kconfig b/drivers/char/watchdog/Kconfig
index d53f664..3207299 100644
--- a/drivers/char/watchdog/Kconfig
+++ b/drivers/char/watchdog/Kconfig
@@ -477,6 +477,16 @@ config INDYDOG
timer expired and no process has written to /dev/watchdog during
that time.
+config WDT_RM9K_GPI
+ tristate "RM9000/GPI hardware watchdog"
+ depends on WATCHDOG && CPU_RM9000
+ help
+ Watchdog implementation using the GPI hardware found on
+ PMC-Sierra RM9xxx CPUs.
+
+ To compile this driver as a module, choose M here: the
+ module will be called rm9k_wdt.
+
# S390 Architecture
config ZVM_WATCHDOG
diff --git a/drivers/char/watchdog/Makefile b/drivers/char/watchdog/Makefile
index 6ab77b6..f2a6281 100644
--- a/drivers/char/watchdog/Makefile
+++ b/drivers/char/watchdog/Makefile
@@ -67,6 +67,7 @@ obj-$(CONFIG_WATCHDOG_RTAS) += wdrtas.o
# MIPS Architecture
obj-$(CONFIG_INDYDOG) += indydog.o
+obj-$(CONFIG_WDT_RM9K_GPI) += rm9k_wdt.o
# S390 Architecture
diff --git a/drivers/char/watchdog/rm9k_wdt.c
b/drivers/char/watchdog/rm9k_wdt.c
new file mode 100644
index 0000000..4f2c7a4
--- /dev/null
+++ b/drivers/char/watchdog/rm9k_wdt.c
@@ -0,0 +1,431 @@
+/*
+ * Watchdog implementation for GPI h/w found on PMC-Sierra RM9xxx
+ * chips.
+ *
+ * Copyright (C) 2004 by Basler Vision Technologies AG
+ * Author: Thomas Koeller <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+ */
+
+#include <linux/platform_device.h>
+#include <linux/module.h>
+#include <linux/moduleparam.h>
+#include <linux/interrupt.h>
+#include <linux/fs.h>
+#include <linux/reboot.h>
+#include <linux/miscdevice.h>
+#include <linux/watchdog.h>
+#include <asm/io.h>
+#include <asm/atomic.h>
+#include <asm/processor.h>
+#include <asm/uaccess.h>
+#include <asm/system.h>
+#include <asm/rm9k-ocd.h>
+
+#include <rm9k_wdt.h>
+
+
+#define CLOCK 125000000
+#define MAX_TIMEOUT_SECONDS 32
+#define CPCCR 0x0080
+#define CPGIG1SR 0x0044
+#define CPGIG1ER 0x0054
+
+
+
+/* Function prototypes */
+static int __init wdt_gpi_probe(struct device *);
+static int __exit wdt_gpi_remove(struct device *);
+static void wdt_gpi_set_timeout(unsigned int);
+static int wdt_gpi_open(struct inode *, struct file *);
+static int wdt_gpi_release(struct inode *, struct file *);
+static ssize_t wdt_gpi_write(struct file *, const char __user *, size_t,
loff_t *);
+static long wdt_gpi_ioctl(struct file *, unsigned int, unsigned long);
+static const struct resource *wdt_gpi_get_resource(struct platform_device *,
const char *, unsigned int);
+static int wdt_gpi_notify(struct notifier_block *, unsigned long, void *);
+static irqreturn_t wdt_gpi_irqhdl(int, void *, struct pt_regs *);
+
+
+
+
+static const char wdt_gpi_name[] = "wdt_gpi";
+static atomic_t opencnt;
+static int expect_close;
+static int locked = 0;
+
+
+
+/* These are set from device resources */
+static void __iomem * wd_regs;
+static unsigned int wd_irq, wd_ctr;
+
+
+
+/* Module arguments */
+static int timeout = MAX_TIMEOUT_SECONDS;
+module_param(timeout, int, 0444);
+static unsigned long resetaddr = 0xbffdc200;
+module_param(resetaddr, ulong, 0444);
+static unsigned long flagaddr = 0xbffdc104;
+module_param(flagaddr, ulong, 0444);
+static int powercycle = 0;
+module_param(powercycle, bool, 0444);
+
+static int nowayout = WATCHDOG_NOWAYOUT;
+module_param(nowayout, bool, 0444);
+
+
+
+static struct file_operations fops = {
+ .owner = THIS_MODULE,
+ .open = wdt_gpi_open,
+ .release = wdt_gpi_release,
+ .write = wdt_gpi_write,
+ .unlocked_ioctl = wdt_gpi_ioctl
+};
+
+static struct miscdevice miscdev = {
+ .minor = WATCHDOG_MINOR,
+ .name = wdt_gpi_name,
+ .fops = &fops
+};
+
+static struct device_driver wdt_gpi_driver = {
+ .name = (char *) wdt_gpi_name,
+ .bus = &platform_bus_type,
+ .owner = THIS_MODULE,
+ .probe = wdt_gpi_probe,
+ .remove = __exit_p(wdt_gpi_remove),
+ .shutdown = NULL,
+ .suspend = NULL,
+ .resume = NULL
+};
+
+static struct notifier_block wdt_gpi_shutdown = {
+ .notifier_call = wdt_gpi_notify
+};
+
+
+
+static const struct resource *
+wdt_gpi_get_resource(struct platform_device *pdv, const char *name,
+ unsigned int type)
+{
+ char buf[80];
+ if (snprintf(buf, sizeof buf, "%s_0", name) >= sizeof buf)
+ return NULL;
+ return platform_get_resource_byname(pdv, type, buf);
+}
+
+
+
+/* No hotplugging on the platform bus - use __init */
+static int __init wdt_gpi_probe(struct device *dev)
+{
+ int res;
+ struct platform_device * const pdv = to_platform_device(dev);
+ const struct resource
+ * const rr = wdt_gpi_get_resource(pdv, WDT_RESOURCE_REGS,
+ IORESOURCE_MEM),
+ * const ri = wdt_gpi_get_resource(pdv, WDT_RESOURCE_IRQ,
+ IORESOURCE_IRQ),
+ * const rc = wdt_gpi_get_resource(pdv, WDT_RESOURCE_COUNTER,
+ 0);
+
+ if (unlikely(!rr || !ri || !rc))
+ return -ENXIO;
+
+ wd_regs = ioremap_nocache(rr->start, rr->end + 1 - rr->start);
+ if (unlikely(!wd_regs))
+ return -ENOMEM;
+ wd_irq = ri->start;
+ wd_ctr = rc->start;
+ res = misc_register(&miscdev);
+ if (res)
+ iounmap(wd_regs);
+ else
+ register_reboot_notifier(&wdt_gpi_shutdown);
+ return res;
+}
+
+
+
+static int __exit wdt_gpi_remove(struct device *dev)
+{
+ int res;
+
+ unregister_reboot_notifier(&wdt_gpi_shutdown);
+ res = misc_deregister(&miscdev);
+ iounmap(wd_regs);
+ wd_regs = NULL;
+ return res;
+}
+
+
+static void wdt_gpi_set_timeout(unsigned int to)
+{
+ u32 reg;
+ const u32 wdval = (to * CLOCK) & ~0x0000000f;
+
+ lock_titan_regs();
+ reg = titan_readl(CPCCR) & ~(0xf << (wd_ctr * 4));
+ titan_writel(reg, CPCCR);
+ wmb();
+ __raw_writel(wdval, wd_regs + 0x0000);
+ wmb();
+ titan_writel(reg | (0x2 << (wd_ctr * 4)), CPCCR);
+ wmb();
+ titan_writel(reg | (0x5 << (wd_ctr * 4)), CPCCR);
+ iob();
+ unlock_titan_regs();
+}
+
+
+
+static int wdt_gpi_open(struct inode *i, struct file *f)
+{
+ int res;
+ u32 reg;
+
+ if (unlikely(0 > atomic_dec_if_positive(&opencnt)))
+ return -EBUSY;
+
+ expect_close = 0;
+ if (locked) {
+ module_put(THIS_MODULE);
+ free_irq(wd_irq, &miscdev);
+ locked = 0;
+ }
+
+ res = request_irq(wd_irq, wdt_gpi_irqhdl, SA_SHIRQ | SA_INTERRUPT,
+ wdt_gpi_name, &miscdev);
+ if (unlikely(res))
+ return res;
+
+ wdt_gpi_set_timeout(timeout);
+
+ lock_titan_regs();
+ reg = titan_readl(CPGIG1ER);
+ titan_writel(reg | (0x100 << wd_ctr), CPGIG1ER);
+ iob();
+ unlock_titan_regs();
+
+ printk(KERN_INFO "%s: watchdog started, timeout = %u seconds\n",
+ wdt_gpi_name, timeout);
+ return 0;
+}
+
+
+
+static int wdt_gpi_release(struct inode *i, struct file *f)
+{
+ if (nowayout) {
+ printk(KERN_NOTICE "%s: no way out - watchdog left running\n",
+ wdt_gpi_name);
+ __module_get(THIS_MODULE);
+ locked = 1;
+ } else {
+ if (expect_close) {
+ u32 reg;
+
+ lock_titan_regs();
+ reg = titan_readl(CPCCR) & ~(0xf << (wd_ctr * 4));
+ titan_writel(reg, CPCCR);
+ reg = titan_readl(CPGIG1ER);
+ titan_writel(reg & ~(0x100 << wd_ctr), CPGIG1ER);
+ iob();
+ unlock_titan_regs();
+ free_irq(wd_irq, &miscdev);
+ printk(KERN_INFO "%s: watchdog stopped\n",
wdt_gpi_name);
+ } else {
+ printk(KERN_NOTICE "%s: unexpected close() -"
+ " watchdog left running\n",
+ wdt_gpi_name);
+ wdt_gpi_set_timeout(timeout);
+ __module_get(THIS_MODULE);
+ locked = 1;
+ }
+ }
+
+ atomic_inc(&opencnt);
+ return 0;
+}
+
+
+
+static ssize_t
+wdt_gpi_write(struct file *f, const char __user *d, size_t s, loff_t *o)
+{
+ char val;
+
+ wdt_gpi_set_timeout(timeout);
+ expect_close = (s > 0) && !get_user(val, d) && (val == 'V');
+ return s ? 1 : 0;
+}
+
+
+
+static long
+wdt_gpi_ioctl(struct file *f, unsigned int cmd, unsigned long arg)
+{
+ long res = -ENOTTY;
+ const long size = _IOC_SIZE(cmd);
+ int stat;
+ static struct watchdog_info wdinfo = {
+ .identity = "RM9xxx/GPI watchdog",
+ .firmware_version = 0,
+ .options = WDIOF_SETTIMEOUT |
WDIOF_KEEPALIVEPING
+ };
+
+ if (unlikely(_IOC_TYPE(cmd) != WATCHDOG_IOCTL_BASE))
+ return -ENOTTY;
+
+ if ((_IOC_DIR(cmd) & _IOC_READ)
+ && !access_ok(VERIFY_WRITE, arg, size))
+ return -EFAULT;
+
+ if ((_IOC_DIR(cmd) & _IOC_WRITE)
+ && !access_ok(VERIFY_READ, arg, size))
+ return -EFAULT;
+
+ expect_close = 0;
+
+ switch (cmd) {
+ case WDIOC_GETSUPPORT:
+ wdinfo.options = nowayout ?
+ WDIOF_SETTIMEOUT | WDIOF_KEEPALIVEPING :
+ WDIOF_SETTIMEOUT | WDIOF_KEEPALIVEPING |
WDIOF_MAGICCLOSE;
+ res = __copy_to_user((void __user *)arg, &wdinfo, size) ?
+ -EFAULT : size;
+ break;
+
+ case WDIOC_GETSTATUS:
+ break;
+
+ case WDIOC_GETBOOTSTATUS:
+ stat = (*(volatile char *) flagaddr & 0x01)
+ ? WDIOF_CARDRESET : 0;
+ res = __copy_to_user((void __user *)arg, &stat, size) ?
+ -EFAULT : size;
+ break;
+
+ case WDIOC_SETOPTIONS:
+ break;
+
+ case WDIOC_KEEPALIVE:
+ wdt_gpi_set_timeout(timeout);
+ res = size;
+ break;
+
+ case WDIOC_SETTIMEOUT:
+ {
+ int val;
+ if (unlikely(__copy_from_user(&val, (const void __user
*) arg,
+ size))) {
+ res = -EFAULT;
+ break;
+ }
+
+ if (val > 32)
+ val = 32;
+ timeout = val;
+ wdt_gpi_set_timeout(val);
+ res = size;
+ printk("%s: timeout set to %u seconds\n",
+ wdt_gpi_name, timeout);
+ }
+ break;
+
+ case WDIOC_GETTIMEOUT:
+ res = __copy_to_user((void __user *) arg, &timeout, size) ?
+ -EFAULT : size;
+ break;
+ }
+
+ return res;
+}
+
+
+
+
+static irqreturn_t wdt_gpi_irqhdl(int irq, void *ctxt, struct pt_regs *regs)
+{
+ if (!unlikely(__raw_readl(wd_regs + 0x0008) & 0x1))
+ return IRQ_NONE;
+ __raw_writel(0x1, wd_regs + 0x0008);
+
+
+ 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)
+ cpu_relax();
+}
+
+
+
+static int
+wdt_gpi_notify(struct notifier_block *this, unsigned long code, void *unused)
+{
+ if(code == SYS_DOWN || code == SYS_HALT) {
+ u32 reg;
+
+ lock_titan_regs();
+ reg = titan_readl(CPCCR) & ~(0xf << (wd_ctr * 4));
+ titan_writel(reg, CPCCR);
+ reg = titan_readl(CPGIG1ER);
+ titan_writel(reg & ~(0x100 << wd_ctr), CPGIG1ER);
+ iob();
+ unlock_titan_regs();
+ }
+ return NOTIFY_DONE;
+}
+
+
+
+static int __init wdt_gpi_init_module(void)
+{
+ atomic_set(&opencnt, 1);
+ if (timeout > MAX_TIMEOUT_SECONDS)
+ timeout = MAX_TIMEOUT_SECONDS;
+ return driver_register(&wdt_gpi_driver);
+}
+
+
+
+static void __exit wdt_gpi_cleanup_module(void)
+{
+ driver_unregister(&wdt_gpi_driver);
+}
+
+module_init(wdt_gpi_init_module);
+module_exit(wdt_gpi_cleanup_module);
+
+
+
+MODULE_AUTHOR("Thomas Koeller <[email protected]>");
+MODULE_DESCRIPTION("Basler eXcite watchdog driver for gpi devices");
+MODULE_VERSION("0.1");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS_MISCDEV(WATCHDOG_MINOR);
+MODULE_PARM_DESC(timeout, "Watchdog timeout in seconds");
+MODULE_PARM_DESC(resetaddr, "Address to write to to force a reset");
+MODULE_PARM_DESC(flagaddr, "Address to write to boot flags to");
+MODULE_PARM_DESC(nowayout, "Watchdog cannot be disabled once started");
+MODULE_PARM_DESC(powercycle, "Cycle power if watchdog expires");
--
1.4.0
--
Thomas Koeller, Software Development
Basler Vision Technologies
An der Strusbek 60-62
22926 Ahrensburg
Germany
Tel +49 (4102) 463-390
Fax +49 (4102) 463-46390
mailto:[email protected]
http://www.baslerweb.com
Hi All,
Can you all review the following watchdog driver? I want to include
it in my linux-2.6-watchdog-mm tree, but would appreciate it if you
could have all a look at it again.
Thomas: I moved start and stop code into seperate functions. I also
deleted the #include <rm9k_wdt.h> because the file doesn't exist.
Note that the shutdown function of the platform_device_driver is
similar to a reboot notifier.
Thanks in advance,
Wim.
--------------------------------------------------------------------------------
/*
* Watchdog implementation for GPI h/w found on PMC-Sierra RM9xxx
* chips.
*
* Copyright (C) 2004 by Basler Vision Technologies AG
* Author: Thomas Koeller <[email protected]>
*
* This program is free software; you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
* the Free Software Foundation; either version 2 of the License, or
* (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
*
* You should have received a copy of the GNU General Public License
* along with this program; if not, write to the Free Software
* Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
*/
#include <linux/platform_device.h>
#include <linux/module.h>
#include <linux/moduleparam.h>
#include <linux/interrupt.h>
#include <linux/fs.h>
#include <linux/reboot.h>
#include <linux/notifier.h>
#include <linux/miscdevice.h>
#include <linux/watchdog.h>
#include <asm/io.h>
#include <asm/atomic.h>
#include <asm/processor.h>
#include <asm/uaccess.h>
#include <asm/system.h>
#include <asm/rm9k-ocd.h>
#define CLOCK 125000000
#define MAX_TIMEOUT_SECONDS 32
#define CPCCR 0x0080
#define CPGIG1SR 0x0044
#define CPGIG1ER 0x0054
/* Function prototypes */
static int __init wdt_gpi_probe(struct device *);
static int __exit wdt_gpi_remove(struct device *);
static void wdt_gpi_start(void);
static void wdt_gpi_stop(void);
static void wdt_gpi_set_timeout(unsigned int);
static int wdt_gpi_open(struct inode *, struct file *);
static int wdt_gpi_release(struct inode *, struct file *);
static ssize_t wdt_gpi_write(struct file *, const char __user *, size_t, loff_t *);
static long wdt_gpi_ioctl(struct file *, unsigned int, unsigned long);
static const struct resource *wdt_gpi_get_resource(struct platform_device *, const char *, unsigned int);
static int wdt_gpi_notify(struct notifier_block *, unsigned long, void *);
static irqreturn_t wdt_gpi_irqhdl(int, void *, struct pt_regs *);
static const char wdt_gpi_name[] = "wdt_gpi";
static atomic_t opencnt;
static int expect_close;
static int locked = 0;
/* These are set from device resources */
static void __iomem * wd_regs;
static unsigned int wd_irq, wd_ctr;
/* Module arguments */
static int timeout = MAX_TIMEOUT_SECONDS;
module_param(timeout, int, 0444);
static unsigned long resetaddr = 0xbffdc200;
module_param(resetaddr, ulong, 0444);
static unsigned long flagaddr = 0xbffdc104;
module_param(flagaddr, ulong, 0444);
static int powercycle = 0;
module_param(powercycle, bool, 0444);
static int nowayout = WATCHDOG_NOWAYOUT;
module_param(nowayout, bool, 0444);
static struct file_operations fops = {
.owner = THIS_MODULE,
.open = wdt_gpi_open,
.release = wdt_gpi_release,
.write = wdt_gpi_write,
.unlocked_ioctl = wdt_gpi_ioctl,
};
static struct miscdevice miscdev = {
.minor = WATCHDOG_MINOR,
.name = wdt_gpi_name,
.fops = &fops,
};
static struct device_driver wdt_gpi_driver = {
.name = (char *) wdt_gpi_name,
.bus = &platform_bus_type,
.owner = THIS_MODULE,
.probe = wdt_gpi_probe,
.remove = __exit_p(wdt_gpi_remove),
.shutdown = NULL,
.suspend = NULL,
.resume = NULL,
};
static struct notifier_block wdt_gpi_shutdown = {
.notifier_call = wdt_gpi_notify,
};
static const struct resource *
wdt_gpi_get_resource(struct platform_device *pdv, const char *name,
unsigned int type)
{
char buf[80];
if (snprintf(buf, sizeof buf, "%s_0", name) >= sizeof buf)
return NULL;
return platform_get_resource_byname(pdv, type, buf);
}
/* No hotplugging on the platform bus - use __init */
static int __init wdt_gpi_probe(struct device *dev)
{
int res;
struct platform_device * const pdv = to_platform_device(dev);
const struct resource
* const rr = wdt_gpi_get_resource(pdv, WDT_RESOURCE_REGS,
IORESOURCE_MEM),
* const ri = wdt_gpi_get_resource(pdv, WDT_RESOURCE_IRQ,
IORESOURCE_IRQ),
* const rc = wdt_gpi_get_resource(pdv, WDT_RESOURCE_COUNTER,
0);
if (unlikely(!rr || !ri || !rc))
return -ENXIO;
wd_regs = ioremap_nocache(rr->start, rr->end + 1 - rr->start);
if (unlikely(!wd_regs))
return -ENOMEM;
wd_irq = ri->start;
wd_ctr = rc->start;
res = misc_register(&miscdev);
if (res)
iounmap(wd_regs);
else
register_reboot_notifier(&wdt_gpi_shutdown);
return res;
}
static int __exit wdt_gpi_remove(struct device *dev)
{
int res;
unregister_reboot_notifier(&wdt_gpi_shutdown);
res = misc_deregister(&miscdev);
iounmap(wd_regs);
wd_regs = NULL;
return res;
}
static void wdt_gpi_start(void)
{
u32 reg;
lock_titan_regs();
reg = titan_readl(CPGIG1ER);
titan_writel(reg | (0x100 << wd_ctr), CPGIG1ER);
iob();
unlock_titan_regs();
}
static void wdt_gpi_stop(void)
{
u32 reg;
lock_titan_regs();
reg = titan_readl(CPCCR) & ~(0xf << (wd_ctr * 4));
titan_writel(reg, CPCCR);
reg = titan_readl(CPGIG1ER);
titan_writel(reg & ~(0x100 << wd_ctr), CPGIG1ER);
iob();
unlock_titan_regs();
}
static void wdt_gpi_set_timeout(unsigned int to)
{
u32 reg;
const u32 wdval = (to * CLOCK) & ~0x0000000f;
lock_titan_regs();
reg = titan_readl(CPCCR) & ~(0xf << (wd_ctr * 4));
titan_writel(reg, CPCCR);
wmb();
__raw_writel(wdval, wd_regs + 0x0000);
wmb();
titan_writel(reg | (0x2 << (wd_ctr * 4)), CPCCR);
wmb();
titan_writel(reg | (0x5 << (wd_ctr * 4)), CPCCR);
iob();
unlock_titan_regs();
}
static int wdt_gpi_open(struct inode *inode, struct file *file)
{
int res;
if (unlikely(0 > atomic_dec_if_positive(&opencnt)))
return -EBUSY;
expect_close = 0;
if (locked) {
module_put(THIS_MODULE);
free_irq(wd_irq, &miscdev);
locked = 0;
}
res = request_irq(wd_irq, wdt_gpi_irqhdl, SA_SHIRQ | SA_INTERRUPT,
wdt_gpi_name, &miscdev);
if (unlikely(res))
return res;
wdt_gpi_set_timeout(timeout);
wdt_gpi_start();
printk(KERN_INFO "%s: watchdog started, timeout = %u seconds\n",
wdt_gpi_name, timeout);
return nonseekable_open(inode, file);
}
static int wdt_gpi_release(struct inode *i, struct file *f)
{
if (nowayout) {
printk(KERN_NOTICE "%s: no way out - watchdog left running\n",
wdt_gpi_name);
__module_get(THIS_MODULE);
locked = 1;
} else {
if (expect_close) {
wdt_gpi_stop();
free_irq(wd_irq, &miscdev);
printk(KERN_INFO "%s: watchdog stopped\n", wdt_gpi_name);
} else {
printk(KERN_NOTICE "%s: unexpected close() -"
" watchdog left running\n",
wdt_gpi_name);
wdt_gpi_set_timeout(timeout);
__module_get(THIS_MODULE);
locked = 1;
}
}
atomic_inc(&opencnt);
return 0;
}
static ssize_t
wdt_gpi_write(struct file *f, const char __user *d, size_t s, loff_t *o)
{
char val;
wdt_gpi_set_timeout(timeout);
expect_close = (s > 0) && !get_user(val, d) && (val == 'V');
return s ? 1 : 0;
}
static long
wdt_gpi_ioctl(struct file *f, unsigned int cmd, unsigned long arg)
{
long res = -ENOTTY;
const long size = _IOC_SIZE(cmd);
int stat;
static struct watchdog_info wdinfo = {
.identity = "RM9xxx/GPI watchdog",
.firmware_version = 0,
.options = WDIOF_SETTIMEOUT | WDIOF_KEEPALIVEPING
};
if (unlikely(_IOC_TYPE(cmd) != WATCHDOG_IOCTL_BASE))
return -ENOTTY;
if ((_IOC_DIR(cmd) & _IOC_READ)
&& !access_ok(VERIFY_WRITE, arg, size))
return -EFAULT;
if ((_IOC_DIR(cmd) & _IOC_WRITE)
&& !access_ok(VERIFY_READ, arg, size))
return -EFAULT;
expect_close = 0;
switch (cmd) {
case WDIOC_GETSUPPORT:
wdinfo.options = nowayout ?
WDIOF_SETTIMEOUT | WDIOF_KEEPALIVEPING :
WDIOF_SETTIMEOUT | WDIOF_KEEPALIVEPING | WDIOF_MAGICCLOSE;
res = __copy_to_user((void __user *)arg, &wdinfo, size) ?
-EFAULT : size;
break;
case WDIOC_GETSTATUS:
break;
case WDIOC_GETBOOTSTATUS:
stat = (*(volatile char *) flagaddr & 0x01)
? WDIOF_CARDRESET : 0;
res = __copy_to_user((void __user *)arg, &stat, size) ?
-EFAULT : size;
break;
case WDIOC_SETOPTIONS:
break;
case WDIOC_KEEPALIVE:
wdt_gpi_set_timeout(timeout);
res = size;
break;
case WDIOC_SETTIMEOUT:
{
int val;
if (unlikely(__copy_from_user(&val, (const void __user *) arg,
size))) {
res = -EFAULT;
break;
}
if (val > 32)
val = 32;
timeout = val;
wdt_gpi_set_timeout(val);
res = size;
printk("%s: timeout set to %u seconds\n",
wdt_gpi_name, timeout);
}
break;
case WDIOC_GETTIMEOUT:
res = __copy_to_user((void __user *) arg, &timeout, size) ?
-EFAULT : size;
break;
}
return res;
}
static irqreturn_t wdt_gpi_irqhdl(int irq, void *ctxt, struct pt_regs *regs)
{
if (!unlikely(__raw_readl(wd_regs + 0x0008) & 0x1))
return IRQ_NONE;
__raw_writel(0x1, wd_regs + 0x0008);
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)
cpu_relax();
}
static int
wdt_gpi_notify(struct notifier_block *this, unsigned long code, void *unused)
{
if(code == SYS_DOWN || code == SYS_HALT) {
wdt_gpi_stop();
}
return NOTIFY_DONE;
}
static int __init wdt_gpi_init_module(void)
{
atomic_set(&opencnt, 1);
if (timeout > MAX_TIMEOUT_SECONDS)
timeout = MAX_TIMEOUT_SECONDS;
return driver_register(&wdt_gpi_driver);
}
static void __exit wdt_gpi_cleanup_module(void)
{
driver_unregister(&wdt_gpi_driver);
}
module_init(wdt_gpi_init_module);
module_exit(wdt_gpi_cleanup_module);
MODULE_AUTHOR("Thomas Koeller <[email protected]>");
MODULE_DESCRIPTION("Basler eXcite watchdog driver for gpi devices");
MODULE_VERSION("0.1");
MODULE_LICENSE("GPL");
MODULE_ALIAS_MISCDEV(WATCHDOG_MINOR);
MODULE_PARM_DESC(timeout, "Watchdog timeout in seconds");
MODULE_PARM_DESC(resetaddr, "Address to write to to force a reset");
MODULE_PARM_DESC(flagaddr, "Address to write to boot flags to");
MODULE_PARM_DESC(nowayout, "Watchdog cannot be disabled once started");
MODULE_PARM_DESC(powercycle, "Cycle power if watchdog expires");
On Wed, Nov 01, 2006 at 07:46:33PM +0100, Wim Van Sebroeck wrote:
> Thomas: I moved start and stop code into seperate functions. I also
> deleted the #include <rm9k_wdt.h> because the file doesn't exist.
You just didn't see it, include/asm-mips/mach-excite/rm9k_wdt.h.
Ralf
Hi Ralf,
> > Thomas: I moved start and stop code into seperate functions. I also
> > deleted the #include <rm9k_wdt.h> because the file doesn't exist.
>
> You just didn't see it, include/asm-mips/mach-excite/rm9k_wdt.h.
Thanks, didn't see it indeed :-). I'll include that again.
Any other remarks still?
Greetings,
Wim.
Hi Wim.
Not much of my area so only some nitpicking stuff.
> /*
> * Watchdog implementation for GPI h/w found on PMC-Sierra RM9xxx
> * chips.
> *
> * Copyright (C) 2004 by Basler Vision Technologies AG
> * Author: Thomas Koeller <[email protected]>
> *
> * This program is free software; you can redistribute it and/or modify
We have COPYING in top level directory. No need to include GPL here.
> #define CPGIG1ER 0x0054
>
>
>
> /* Function prototypes */
Too many empty lines. Get it down to 2. Seen on many places.
> static int __init wdt_gpi_probe(struct device *);
> static int __exit wdt_gpi_remove(struct device *);
> static void wdt_gpi_start(void);
> static void wdt_gpi_stop(void);
> static void wdt_gpi_set_timeout(unsigned int);
3 prototpyes not needed
> static int wdt_gpi_open(struct inode *, struct file *);
> static int wdt_gpi_release(struct inode *, struct file *);
> static ssize_t wdt_gpi_write(struct file *, const char __user *, size_t, loff_t *);
> static long wdt_gpi_ioctl(struct file *, unsigned int, unsigned long);
> static const struct resource *wdt_gpi_get_resource(struct platform_device *, const char *, unsigned int);
80 char coloum
Prototype not needed.
> /* These are set from device resources */
> static void __iomem * wd_regs;
Good to see code annotated.
I assume sparse did not give any warnings on this code?
> /* Module arguments */
> static int timeout = MAX_TIMEOUT_SECONDS;
> module_param(timeout, int, 0444);
> static unsigned long resetaddr = 0xbffdc200;
> module_param(resetaddr, ulong, 0444);
> static unsigned long flagaddr = 0xbffdc104;
> module_param(flagaddr, ulong, 0444);
> static int powercycle = 0;
> module_param(powercycle, bool, 0444);
>
> static int nowayout = WATCHDOG_NOWAYOUT;
> module_param(nowayout, bool, 0444);
Locate parameter descriptions clode to parameter definition - not in
bottom of file.
> static struct device_driver wdt_gpi_driver = {
> .name = (char *) wdt_gpi_name,
If this cast is really needed then struct device_driver ought to be updated?
> static int __init wdt_gpi_probe(struct device *dev)
> {
> int res;
> struct platform_device * const pdv = to_platform_device(dev);
> const struct resource
> * const rr = wdt_gpi_get_resource(pdv, WDT_RESOURCE_REGS,
> IORESOURCE_MEM),
> * const ri = wdt_gpi_get_resource(pdv, WDT_RESOURCE_IRQ,
> IORESOURCE_IRQ),
> * const rc = wdt_gpi_get_resource(pdv, WDT_RESOURCE_COUNTER,
Separate variable definition and assignment => nicer code.
> static long
> wdt_gpi_ioctl(struct file *f, unsigned int cmd, unsigned long arg)
> {
arg is a __user pointer - why hide this fact?
> res = __copy_to_user((void __user *)arg, &wdinfo, size) ?
> -EFAULT : size;
then you get rid of this cast.
And code this as if () else
Using ?: is just ugly here.
> case WDIOC_GETBOOTSTATUS:
> stat = (*(volatile char *) flagaddr & 0x01)
> ? WDIOF_CARDRESET : 0;
Use of volatile almost always indicate a bug.
Please explain why volatile is needed and consider the other options.
> printk("%s: timeout set to %u seconds\n",
> wdt_gpi_name, timeout);
I just noticed this pringk miss a <KERNEL_DEBUG> or similar specifier.
I guess this is all of them.
> if (!unlikely(__raw_readl(wd_regs + 0x0008) & 0x1))
> return IRQ_NONE;
> __raw_writel(0x1, wd_regs + 0x0008);
Magics suchs as '0x0008' deserve a comment.
> *(volatile char *) flagaddr |= 0x01;
> *(volatile char *) resetaddr = powercycle ? 0x01 : 0x2;
volatile again...
> MODULE_AUTHOR("Thomas Koeller <[email protected]>");
> MODULE_DESCRIPTION("Basler eXcite watchdog driver for gpi devices");
> MODULE_VERSION("0.1");
> MODULE_LICENSE("GPL");
> MODULE_ALIAS_MISCDEV(WATCHDOG_MINOR);
These five tags belongs in the start of the file.
> MODULE_PARM_DESC(timeout, "Watchdog timeout in seconds");
> MODULE_PARM_DESC(resetaddr, "Address to write to to force a reset");
> MODULE_PARM_DESC(flagaddr, "Address to write to boot flags to");
> MODULE_PARM_DESC(nowayout, "Watchdog cannot be disabled once started");
> MODULE_PARM_DESC(powercycle, "Cycle power if watchdog expires");
Same here - but already noted.
Sam
On Wed, 1 Nov 2006 19:46:33 +0100 Wim Van Sebroeck wrote:
> /* Function prototypes */
> static int __init wdt_gpi_probe(struct device *);
> static int __exit wdt_gpi_remove(struct device *);
> static void wdt_gpi_start(void);
> static void wdt_gpi_stop(void);
> static void wdt_gpi_set_timeout(unsigned int);
> static int wdt_gpi_open(struct inode *, struct file *);
> static int wdt_gpi_release(struct inode *, struct file *);
> static ssize_t wdt_gpi_write(struct file *, const char __user *, size_t, loff_t *);
> static long wdt_gpi_ioctl(struct file *, unsigned int, unsigned long);
> static const struct resource *wdt_gpi_get_resource(struct platform_device *, const char *, unsigned int);
Some lines too long (stay <= 80 columns).
> static int wdt_gpi_notify(struct notifier_block *, unsigned long, void *);
> static irqreturn_t wdt_gpi_irqhdl(int, void *, struct pt_regs *);
>
>
>
>
> static const char wdt_gpi_name[] = "wdt_gpi";
> static atomic_t opencnt;
> static int expect_close;
> static int locked = 0;
Don't need to init to 0.
> /* Module arguments */
> static int timeout = MAX_TIMEOUT_SECONDS;
> module_param(timeout, int, 0444);
> static unsigned long resetaddr = 0xbffdc200;
> module_param(resetaddr, ulong, 0444);
> static unsigned long flagaddr = 0xbffdc104;
> module_param(flagaddr, ulong, 0444);
> static int powercycle = 0;
no need to init to 0.
> module_param(powercycle, bool, 0444);
>
> static int nowayout = WATCHDOG_NOWAYOUT;
> module_param(nowayout, bool, 0444);
>
>
>
> /* No hotplugging on the platform bus - use __init */
> static int __init wdt_gpi_probe(struct device *dev)
> {
> int res;
> struct platform_device * const pdv = to_platform_device(dev);
> const struct resource
> * const rr = wdt_gpi_get_resource(pdv, WDT_RESOURCE_REGS,
> IORESOURCE_MEM),
> * const ri = wdt_gpi_get_resource(pdv, WDT_RESOURCE_IRQ,
> IORESOURCE_IRQ),
> * const rc = wdt_gpi_get_resource(pdv, WDT_RESOURCE_COUNTER,
> 0);
>
> if (unlikely(!rr || !ri || !rc))
> return -ENXIO;
>
> wd_regs = ioremap_nocache(rr->start, rr->end + 1 - rr->start);
> if (unlikely(!wd_regs))
> return -ENOMEM;
There's no way to return the resources on failure?
> wd_irq = ri->start;
> wd_ctr = rc->start;
> res = misc_register(&miscdev);
> if (res)
> iounmap(wd_regs);
> else
> register_reboot_notifier(&wdt_gpi_shutdown);
> return res;
> }
>
>
>
> static int wdt_gpi_open(struct inode *inode, struct file *file)
> {
> int res;
>
> if (unlikely(0 > atomic_dec_if_positive(&opencnt)))
Style: Instead of
if (constant op function_or_variable)
we prefer
if (function_or_variable op constant)
> return -EBUSY;
>
> expect_close = 0;
> if (locked) {
> module_put(THIS_MODULE);
> free_irq(wd_irq, &miscdev);
> locked = 0;
> }
>
> res = request_irq(wd_irq, wdt_gpi_irqhdl, SA_SHIRQ | SA_INTERRUPT,
> wdt_gpi_name, &miscdev);
> if (unlikely(res))
> return res;
>
> wdt_gpi_set_timeout(timeout);
> wdt_gpi_start();
>
> printk(KERN_INFO "%s: watchdog started, timeout = %u seconds\n",
> wdt_gpi_name, timeout);
> return nonseekable_open(inode, file);
> }
>
>
>
>
> static long
> wdt_gpi_ioctl(struct file *f, unsigned int cmd, unsigned long arg)
> {
> long res = -ENOTTY;
> const long size = _IOC_SIZE(cmd);
> int stat;
> static struct watchdog_info wdinfo = {
> .identity = "RM9xxx/GPI watchdog",
> .firmware_version = 0,
> .options = WDIOF_SETTIMEOUT | WDIOF_KEEPALIVEPING
> };
>
> if (unlikely(_IOC_TYPE(cmd) != WATCHDOG_IOCTL_BASE))
> return -ENOTTY;
>
> if ((_IOC_DIR(cmd) & _IOC_READ)
> && !access_ok(VERIFY_WRITE, arg, size))
> return -EFAULT;
>
> if ((_IOC_DIR(cmd) & _IOC_WRITE)
> && !access_ok(VERIFY_READ, arg, size))
> return -EFAULT;
>
> expect_close = 0;
>
> switch (cmd) {
> case WDIOC_GETSUPPORT:
> wdinfo.options = nowayout ?
> WDIOF_SETTIMEOUT | WDIOF_KEEPALIVEPING :
> WDIOF_SETTIMEOUT | WDIOF_KEEPALIVEPING | WDIOF_MAGICCLOSE;
> res = __copy_to_user((void __user *)arg, &wdinfo, size) ?
> -EFAULT : size;
> break;
>
> case WDIOC_GETSTATUS:
> break;
>
> case WDIOC_GETBOOTSTATUS:
> stat = (*(volatile char *) flagaddr & 0x01)
> ? WDIOF_CARDRESET : 0;
> res = __copy_to_user((void __user *)arg, &stat, size) ?
> -EFAULT : size;
> break;
>
> case WDIOC_SETOPTIONS:
> break;
>
> case WDIOC_KEEPALIVE:
> wdt_gpi_set_timeout(timeout);
> res = size;
> break;
>
> case WDIOC_SETTIMEOUT:
> {
> int val;
> if (unlikely(__copy_from_user(&val, (const void __user *) arg,
> size))) {
> res = -EFAULT;
> break;
> }
>
> if (val > 32)
> val = 32;
There's a #defined constant for that "32".
Please use it.
> timeout = val;
> wdt_gpi_set_timeout(val);
> res = size;
> printk("%s: timeout set to %u seconds\n",
> wdt_gpi_name, timeout);
> }
> break;
>
> case WDIOC_GETTIMEOUT:
> res = __copy_to_user((void __user *) arg, &timeout, size) ?
> -EFAULT : size;
> break;
> }
>
> return res;
> }
>
>
>
>
> static irqreturn_t wdt_gpi_irqhdl(int irq, void *ctxt, struct pt_regs *regs)
> {
> if (!unlikely(__raw_readl(wd_regs + 0x0008) & 0x1))
> return IRQ_NONE;
> __raw_writel(0x1, wd_regs + 0x0008);
>
>
> printk(KERN_WARNING "%s: watchdog expired - resetting system\n",
> wdt_gpi_name);
I would expect a KERN_ALERT or KERN_EMERG or KERN_CRIT there...
>
> *(volatile char *) flagaddr |= 0x01;
> *(volatile char *) resetaddr = powercycle ? 0x01 : 0x2;
> iob();
> while (1)
> cpu_relax();
> }
>
>
>
> static int
> wdt_gpi_notify(struct notifier_block *this, unsigned long code, void *unused)
> {
> if(code == SYS_DOWN || code == SYS_HALT) {
Space between if and (.
> wdt_gpi_stop();
> }
No braces around one-statement blocks.
> return NOTIFY_DONE;
> }
---
~Randy
On Wed, Nov 01, 2006 at 10:11:25PM -0800, Randy Dunlap wrote:
> > wd_regs = ioremap_nocache(rr->start, rr->end + 1 - rr->start);
> > if (unlikely(!wd_regs))
> > return -ENOMEM;
>
> There's no way to return the resources on failure?
MIPS drivers (and this one is specific to a particular MIPS SOC) are
generally a bit sloopy about checking of return values of ioremap because
ioremap is only doing some address arithmetic but no allocations that
actually could fail. So for 64-bit kernels or addresses below 0x20000000
on a 32-bit system ioremap cannot fail. In the same cases ioremap happens
to be a no-op because where nothing was allocated nothing needs to be
freed.
> > if (unlikely(__copy_from_user(&val, (const void __user *) arg,
Note to self, __copy_from_user and gang are generally assume to not
return an error so it might be a good idea to move that unlikely() into
the macro definitions.
Ralf