| Message ID | 1284764008-19469-2-git-send-email-timur@freescale.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
On Fri, Sep 17, 2010 at 6:53 PM, Timur Tabi <timur@freescale.com> wrote: > Register the __init and __exit functions in the PowerPC e500 watchdog driver > as module entry/exit functions, and modify the Kconfig entry. > > Add a .release method for the PowerPC e500 watchdog driver, so that the > watchdog is disabled when the driver is closed. This is used for more than just e500. > > Loosely based on original code from Jiang Yutang <b14898@freescale.com>. > > Signed-off-by: Timur Tabi <timur@freescale.com> > --- > > This patch requires: > > powerpc: export ppc_tb_freq so that modules can reference it > > drivers/watchdog/Kconfig | 5 ++++- > drivers/watchdog/booke_wdt.c | 39 +++++++++++++++++++++++++++++++++++++-- > 2 files changed, 41 insertions(+), 3 deletions(-) > > diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig > index 24efd8e..d9cf5a9 100644 > --- a/drivers/watchdog/Kconfig > +++ b/drivers/watchdog/Kconfig > @@ -957,9 +957,12 @@ config PIKA_WDT > the Warp platform. > > config BOOKE_WDT > - bool "PowerPC Book-E Watchdog Timer" > + tristate "PowerPC Book-E Watchdog Timer" > depends on BOOKE || 4xx > ---help--- > + Watchdog driver for PowerPC e500 chips, such as the Freescale > + MPC85xx SOCs. > + Again, used for more than e500. That || 4xx in the depends statement right above your addition isn't there for fun :). josh
On Fri, Sep 17, 2010 at 7:37 PM, Josh Boyer <jwboyer@gmail.com> wrote: >> config BOOKE_WDT >> - bool "PowerPC Book-E Watchdog Timer" >> + tristate "PowerPC Book-E Watchdog Timer" >> depends on BOOKE || 4xx >> ---help--- >> + Watchdog driver for PowerPC e500 chips, such as the Freescale >> + MPC85xx SOCs. >> + > > Again, used for more than e500. That || 4xx in the depends statement > right above your addition isn't there for fun :). Does this mean that this comment which is already in the driver is wrong? It alludes that "Book-E" is synonymous with "e500". /* If the kernel parameter wdt=1, the watchdog will be enabled at boot. * Also, the wdt_period sets the watchdog timer period timeout. * For E500 cpus the wdt_period sets which bit changing from 0->1 will * trigger a watchog timeout. This watchdog timeout will occur 3 times, the * first time nothing will happen, the second time a watchdog exception will * occur, and the final time the board will reset. */ #ifdef CONFIG_FSL_BOOKE #define WDT_PERIOD_DEFAULT 38 /* Ex. wdt_period=28 bus=333Mhz,reset=~40sec */ #else #define WDT_PERIOD_DEFAULT 3 /* Refer to the PPC40x and PPC4xx manuals */ #endif /* for timing information */
On Mon, Sep 20, 2010 at 10:51:37AM -0500, Timur Tabi wrote: >On Fri, Sep 17, 2010 at 7:37 PM, Josh Boyer <jwboyer@gmail.com> wrote: >>> config BOOKE_WDT >>> - bool "PowerPC Book-E Watchdog Timer" >>> + tristate "PowerPC Book-E Watchdog Timer" >>> depends on BOOKE || 4xx >>> ---help--- >>> + Watchdog driver for PowerPC e500 chips, such as the Freescale >>> + MPC85xx SOCs. >>> + >> >> Again, used for more than e500. That || 4xx in the depends statement >> right above your addition isn't there for fun :). > >Does this mean that this comment which is already in the driver is >wrong? It alludes that "Book-E" is synonymous with "e500". > >/* If the kernel parameter wdt=1, the watchdog will be enabled at boot. > * Also, the wdt_period sets the watchdog timer period timeout. > * For E500 cpus the wdt_period sets which bit changing from 0->1 will > * trigger a watchog timeout. This watchdog timeout will occur 3 times, the > * first time nothing will happen, the second time a watchdog exception will > * occur, and the final time the board will reset. > */ I guess I don't see what you mean. It talks about e500 but it doesn't say "Book-E" anywhere in that comment. (Though I'm pretty sure that comment does apply to 4xx.) > >#ifdef CONFIG_FSL_BOOKE >#define WDT_PERIOD_DEFAULT 38 /* Ex. wdt_period=28 bus=333Mhz,reset=~40sec */ >#else >#define WDT_PERIOD_DEFAULT 3 /* Refer to the PPC40x and PPC4xx manuals */ >#endif /* for timing information */ And this bit is marked as "FSL_BOOKE" vs. 40x and 44x. josh
Josh Boyer wrote: > I guess I don't see what you mean. It talks about e500 but it doesn't > say "Book-E" anywhere in that comment. (Though I'm pretty sure that > comment does apply to 4xx.) That was my point. The comment says e500, but the code is also intended for 4xx.
diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig index 24efd8e..d9cf5a9 100644 --- a/drivers/watchdog/Kconfig +++ b/drivers/watchdog/Kconfig @@ -957,9 +957,12 @@ config PIKA_WDT the Warp platform. config BOOKE_WDT - bool "PowerPC Book-E Watchdog Timer" + tristate "PowerPC Book-E Watchdog Timer" depends on BOOKE || 4xx ---help--- + Watchdog driver for PowerPC e500 chips, such as the Freescale + MPC85xx SOCs. + Please see Documentation/watchdog/watchdog-api.txt for more information. diff --git a/drivers/watchdog/booke_wdt.c b/drivers/watchdog/booke_wdt.c index 3d49671..a989998 100644 --- a/drivers/watchdog/booke_wdt.c +++ b/drivers/watchdog/booke_wdt.c @@ -4,7 +4,7 @@ * Author: Matthew McClintock * Maintainer: Kumar Gala <galak@kernel.crashing.org> * - * Copyright 2005, 2008 Freescale Semiconductor Inc. + * Copyright 2005, 2008, 2010 Freescale Semiconductor Inc. * * 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 @@ -114,6 +114,27 @@ static void __booke_wdt_enable(void *data) mtspr(SPRN_TCR, val); } +/** + * booke_wdt_disable - disable the watchdog on the given CPU + * + * This function is called on each CPU. It disables the watchdog on that CPU. + * + * TCR[WRC] cannot be changed once it has been set to non-zero, but we can + * effectively disable the watchdog by setting its period to the maximum value. + */ +static void __booke_wdt_disable(void *data) +{ + u32 val; + + val = mfspr(SPRN_TCR); + val &= ~(TCR_WIE | WDTP_MASK); + mtspr(SPRN_TCR, val); + + /* clear status to make sure nothing is pending */ + __booke_wdt_ping(NULL); + +} + static ssize_t booke_wdt_write(struct file *file, const char __user *buf, size_t count, loff_t *ppos) { @@ -193,12 +214,21 @@ static int booke_wdt_open(struct inode *inode, struct file *file) return nonseekable_open(inode, file); } +static int booke_wdt_release(struct inode *inode, struct file *file) +{ + on_each_cpu(__booke_wdt_disable, NULL, 0); + booke_wdt_enabled = 0; + + return 0; +} + static const struct file_operations booke_wdt_fops = { .owner = THIS_MODULE, .llseek = no_llseek, .write = booke_wdt_write, .unlocked_ioctl = booke_wdt_ioctl, .open = booke_wdt_open, + .release = booke_wdt_release, }; static struct miscdevice booke_wdt_miscdev = { @@ -237,4 +267,9 @@ static int __init booke_wdt_init(void) return ret; } -device_initcall(booke_wdt_init); + +module_init(booke_wdt_init); +module_exit(booke_wdt_exit); + +MODULE_DESCRIPTION("PowerPC Book-E watchdog driver"); +MODULE_LICENSE("GPL");
Register the __init and __exit functions in the PowerPC e500 watchdog driver as module entry/exit functions, and modify the Kconfig entry. Add a .release method for the PowerPC e500 watchdog driver, so that the watchdog is disabled when the driver is closed. Loosely based on original code from Jiang Yutang <b14898@freescale.com>. Signed-off-by: Timur Tabi <timur@freescale.com> --- This patch requires: powerpc: export ppc_tb_freq so that modules can reference it drivers/watchdog/Kconfig | 5 ++++- drivers/watchdog/booke_wdt.c | 39 +++++++++++++++++++++++++++++++++++++-- 2 files changed, 41 insertions(+), 3 deletions(-)