+All that noise.
+ if (!timeout)
+ dev_info(wdog->dev, "Watchdog timer stopped");
+
Would it be acceptable to turn these calls into dev_dbg() calls, here
and elsewhere?
+
+ wdt->resolution = SECWDOG_DEFAULT_RESOLUTION;
+ ret = bcm_kona_wdt_set_resolution_reg(wdt);
+ if (ret) {
+ dev_err(dev, "Failed to set resolution (error: %d)", ret);
ret can be -EAGAIN or -EINVAL. -EINVAL suggests a bad internale error (hopefully
SECWDOG_DEFAULT_RESOLUTION is defined to be smaller than SECWDOG_MAX_RES),
and if it is -EAGAIN there should be no error message.
Actually, bcm_kona_wdt_set_resolution_reg is only called from here, meaning the
error check in the function is really unnecessary.
This again goes back to making resolution available to userland. Then
bcm_kona_wdt_set_resolution_reg() would be called from elsewhere. Why
is it bad to print an error message on timeout? Would this still apply
if I switch the code to -ETIMEDOUT?That is one option, or -EIO if the condition indicates a chip error.
Oh, I agree it should return an error, and an error message is ok as well.+ return ret;
+ }
+
+ spin_lock_init(&wdt->lock);
+ platform_set_drvdata(pdev, wdt);
+ watchdog_set_drvdata(&bcm_kona_wdt_wdd, wdt);
+
+ ret = bcm_kona_wdt_set_timeout_reg(&bcm_kona_wdt_wdd);
+ if (ret) {
+ dev_err(dev, "Failed set watchdog timeout");
The only error case is -EAGAIN. I don't think there should be an error mesasge
in this case (though I am not sure what the reaction should be).
I am thinking that probe() needs to return an error if setting the
timeout fails, as it can't really rely on the watchdog timer or let
the system use it. Shouldn't that be accompanied by an error message
letting the user know what happened?
+ return ret;Such messages are in general considered nuisance nowadays. I would suggest to
+ }
+
+ ret = watchdog_register_device(&bcm_kona_wdt_wdd);
+ if (ret) {
+ dev_err(dev, "Failed to register watchdog device");
+ return ret;
+ }
+
+#ifdef CONFIG_BCM_KONA_WDT_DEBUG
+ wdt->debugfs = bcm_kona_wdt_debugfs_init(wdt, &bcm_kona_wdt_wdd);
+#endif
+ dev_info(dev, "Broadcom Kona Watchdog Timer");
+
remove it (or ask Greg KH for advice).