Patchwork [2/2] powerpc/watchdog: allow the e500 watchdog driver to be compiled as a module

login
register
mail settings
Submitter Timur Tabi
Date Sept. 17, 2010, 10:53 p.m.
Message ID <1284764008-19469-2-git-send-email-timur@freescale.com>
Download mbox | patch
Permalink /patch/65113/
State Superseded
Headers show

Comments

Timur Tabi - Sept. 17, 2010, 10:53 p.m.
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(-)
Josh Boyer - Sept. 18, 2010, 12:37 a.m.
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
Timur Tabi - Sept. 20, 2010, 3:51 p.m.
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 */
Josh Boyer - Sept. 20, 2010, 7:30 p.m.
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
Timur Tabi - Sept. 20, 2010, 7:53 p.m.
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.

Patch

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");