| Message ID | 1295299780-27338-1-git-send-email-timur@freescale.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Benjamin Herrenschmidt |
| Headers | show |
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.
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);
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 <timur@freescale.com> --- drivers/watchdog/booke_wdt.c | 17 ++++++++--------- 1 files changed, 8 insertions(+), 9 deletions(-)