On Wed, Mar 07, 2012 at 03:57:55PM +0530, Viresh Kumar wrote: > @@ -156,10 +159,22 @@ static int mpcore_wdt_open(struct inode *inode, struct file *file) > > file->private_data = wdt; > > + if (wdt->clk) { if (!IS_ERR(wdt->clk)) { > + ret = clk_enable(wdt->clk); What about preparing the clock? > + if (ret) { > + dev_err(wdt->dev, "clock enable fail"); > + goto err; > + } > + } > + > /* Activate timer */ > mpcore_wdt_start(wdt); > > return nonseekable_open(inode, file); > + > +err: > + clear_bit(0, &wdt->timer_alive); > + return ret; > } > > static int mpcore_wdt_release(struct inode *inode, struct file *file) > @@ -169,9 +184,12 @@ static int mpcore_wdt_release(struct inode *inode, struct file *file) > /* > * Shut off the timer. Lock it in if it's a module and we set nowayout > */ > - if (wdt->expect_close == 42) > + if (wdt->expect_close == 42) { > mpcore_wdt_stop(wdt); > - else { > + > + if (wdt->clk) if (!IS_ERR(wdt->clk)) > + clk_disable(wdt->clk); What about unpreparing the clock? > + } else { > dev_printk(KERN_CRIT, wdt->dev, > "unexpected close, not stopping watchdog!\n"); > mpcore_wdt_keepalive(wdt); > @@ -350,6 +368,28 @@ static int __devinit mpcore_wdt_probe(struct platform_device *pdev) > } > } > > + wdt->clk = clk_get(&pdev->dev, NULL); > + if (IS_ERR(wdt->clk)) { > + dev_warn(&pdev->dev, "Clock not found\n"); > + wdt->clk = NULL; Remove this line. > + } > + > + if (wdt->clk) { if (!IS_ERR(wdt->clk)) { > + ret = clk_enable(wdt->clk); What about preparing the clock? > + if (ret) { > + dev_err(&pdev->dev, "Clock enable failed\n"); > + goto err_put_clk; > + } > + } > + > + wdt->boot_status = (readl(wdt->base + TWD_WDOG_RESETSTAT) & > + TWD_WDOG_RESETSTAT_MASK) ? WDIOF_CARDRESET : 0; > + > + mpcore_wdt_stop(wdt); > + > + if (wdt->clk) > + clk_disable(wdt->clk); Same two comments... > + > /* > * Check that the margin value is within it's range; > * if not reset to the default > @@ -368,25 +408,32 @@ static int __devinit mpcore_wdt_probe(struct platform_device *pdev) > dev_printk(KERN_ERR, wdt->dev, > "cannot register miscdev on minor=%d (err=%d)\n", > WATCHDOG_MINOR, ret); > - return ret; > + goto err_put_clk; > } > > - wdt->boot_status = (readl(wdt->base + TWD_WDOG_RESETSTAT) & > - TWD_WDOG_RESETSTAT_MASK) ? WDIOF_CARDRESET : 0; > - > - mpcore_wdt_stop(wdt); > platform_set_drvdata(pdev, wdt); > mpcore_wdt_pdev = pdev; > > return 0; > + > +err_put_clk: > + if (wdt->clk) > + clk_put(wdt->clk); > + > + return ret; > } > > static int __devexit mpcore_wdt_remove(struct platform_device *pdev) > { > + struct mpcore_wdt *wdt = platform_get_drvdata(pdev); > + > platform_set_drvdata(pdev, NULL); > > misc_deregister(&mpcore_wdt_miscdev); > > + if (wdt->clk) > + clk_put(wdt->clk); Same comments. > + > mpcore_wdt_pdev = NULL; > > return 0; > @@ -399,6 +446,9 @@ static int mpcore_wdt_suspend(struct device *dev) > > if (test_bit(0, &wdt->timer_alive)) > mpcore_wdt_stop(wdt); /* Turn the WDT off */ > + > + if (wdt->clk) > + clk_disable(wdt->clk); Same comments... And what if the watchdog is not open? Doesn't this disable an already disabled clock? > return 0; > } > > @@ -407,8 +457,13 @@ static int mpcore_wdt_resume(struct device *dev) > struct mpcore_wdt *wdt = dev_get_drvdata(dev); > > /* re-activate timer */ > - if (test_bit(0, &wdt->timer_alive)) > + if (test_bit(0, &wdt->timer_alive)) { > + if (wdt->clk) > + clk_enable(wdt->clk); Same two comments. > + > mpcore_wdt_start(wdt); > + } > + > return 0; > } > #endif > -- > 1.7.8.110.g4cb5d > > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@xxxxxxxxxxxxxxxxxxx > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel -- To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html