[PATCH] watchdog: booke_wdt: clean up status messages

Improve the status messages that are displayed during some operations of the
PowerPC watchdog timer driver.  When the watchdog is enabled, the timeout is
displayed as a number of seconds, instead of an obscure "period".  The "period"
is the position of a bit in a 64-bit timer register.  The higher the value,
the quicker the watchdog timeout occurs.  Some people chose a high "period"
value for the timer and get confused as to why the board resets within a
few seconds.

Messages displayed during open and close are now debug messages, so that they
don't clutter the console by default.  Finally, printk() is replaced with the
pr_xxx() equivalent.

Signed-off-by: Timur Tabi <redacted>
---
 drivers/watchdog/booke_wdt.c |   17 ++++++++---------
 1 files changed, 8 insertions(+), 9 deletions(-)

diff --git a/drivers/watchdog/booke_wdt.c b/drivers/watchdog/booke_wdt.c
index 7e7ec9c..935d0dd 100644
--- a/drivers/watchdog/booke_wdt.c
+++ b/drivers/watchdog/booke_wdt.c

@@ -221,9 +221,8 @@ static int booke_wdt_open(struct inode *inode, struct file *file)
 	if (booke_wdt_enabled == 0) {
 		booke_wdt_enabled = 1;
 		on_each_cpu(__booke_wdt_enable, NULL, 0);
-		printk(KERN_INFO
-		      "PowerPC Book-E Watchdog Timer Enabled (wdt_period=%d)\n",
-				booke_wdt_period);
+		pr_debug("booke_wdt: watchdog enabled (timeout = %llu sec)\n",
+			period_to_sec(booke_wdt_period));
 	}
 	spin_unlock(&booke_wdt_lock);
 

@@ -240,6 +239,7 @@ static int booke_wdt_release(struct inode *inode, struct file *file)
 	 */
 	on_each_cpu(__booke_wdt_disable, NULL, 0);
 	booke_wdt_enabled = 0;
+	pr_debug("booke_wdt: watchdog disabled\n",
 #endif
 
 	clear_bit(0, &wdt_is_active);

@@ -271,21 +271,20 @@ static int __init booke_wdt_init(void)
 {
 	int ret = 0;
 
-	printk(KERN_INFO "PowerPC Book-E Watchdog Timer Loaded\n");
+	pr_info("PowerPC Book-E Watchdog Timer Loaded\n");
 	ident.firmware_version = cur_cpu_spec->pvr_value;
 
 	ret = misc_register(&booke_wdt_miscdev);
 	if (ret) {
-		printk(KERN_CRIT "Cannot register miscdev on minor=%d: %d\n",
-				WATCHDOG_MINOR, ret);
+		pr_err("booke_wdt: cannot register device (minor=%u, ret=%i)\n",
+		       WATCHDOG_MINOR, ret);
 		return ret;
 	}
 
 	spin_lock(&booke_wdt_lock);
 	if (booke_wdt_enabled == 1) {
-		printk(KERN_INFO
-		      "PowerPC Book-E Watchdog Timer Enabled (wdt_period=%d)\n",
-				booke_wdt_period);
+		pr_info("booke_wdt: watchdog enabled (timeout = %llu sec)\n",
+			period_to_sec(booke_wdt_period));
 		on_each_cpu(__booke_wdt_enable, NULL, 0);
 	}
 	spin_unlock(&booke_wdt_lock);

-- 
1.7.3.4

On Mon, 2011-01-17 at 15:29 -0600, Timur Tabi wrote:

Improve the status messages that are displayed during some operations of the
PowerPC watchdog timer driver.  When the watchdog is enabled, the timeout is
displayed as a number of seconds, instead of an obscure "period".  The "period"
is the position of a bit in a 64-bit timer register.  The higher the value,
the quicker the watchdog timeout occurs.  Some people chose a high "period"
value for the timer and get confused as to why the board resets within a
few seconds.

Messages displayed during open and close are now debug messages, so that they
don't clutter the console by default.  Finally, printk() is replaced with the
pr_xxx() equivalent.

Minor nit bu

 
-	printk(KERN_INFO "PowerPC Book-E Watchdog Timer Loaded\n");
+	pr_info("PowerPC Book-E Watchdog Timer Loaded\n");
 	ident.firmware_version = cur_cpu_spec->pvr_value;
 
 	ret = misc_register(&booke_wdt_miscdev);
 	if (ret) {
-		printk(KERN_CRIT "Cannot register miscdev on minor=%d: %d\n",
-				WATCHDOG_MINOR, ret);
+		pr_err("booke_wdt: cannot register device (minor=%u, ret=%i)\n",
+		       WATCHDOG_MINOR, ret);
 		return ret;
 	}
 
 	spin_lock(&booke_wdt_lock);
 	if (booke_wdt_enabled == 1) {
-		printk(KERN_INFO
-		      "PowerPC Book-E Watchdog Timer Enabled (wdt_period=%d)\n",
-				booke_wdt_period);
+		pr_info("booke_wdt: watchdog enabled (timeout = %llu sec)\n",
+			period_to_sec(booke_wdt_period));
 		on_each_cpu(__booke_wdt_enable, NULL, 0);
 	}

If you're going to cleanup the messages, then I think displaying:

PowerPC Book-E Watchdog Timer Loaded
booke_wdt: watchdog enabled (timeout = xxx sec)

Isn't very nice.

Lacks consistency ... you can prefix both lines the same way, or
maybe better print only one line with all the necessary info.

Ben.

Benjamin Herrenschmidt wrote:

If you're going to cleanup the messages, then I think displaying:

PowerPC Book-E Watchdog Timer Loaded
booke_wdt: watchdog enabled (timeout = xxx sec)

Isn't very nice.

Lacks consistency ... you can prefix both lines the same way, or
maybe better print only one line with all the necessary info.

Yeah, I see your point.  I'll post a v2.

-- 
Timur Tabi
Linux kernel developer at Freescale