Hi Stefan,
+/* The hardware supports a maximum timeout value of 0xfffff ticks
- (just below 16 seconds).
- U-boot users specify the timeout as a number of milliseconds
- by using variable CONFIG_WATCHDOG_TIMEOUT_MSECS.
- The maximum value should be 15999 ms in our case.
- However, u-boot internally converts this config value to seconds,
- thus specifying 15999 actually means 15000 ms (0xf0000 ticks).
means 15999 ms?
No, 15000 ms. Actually I observed that whatever value 15xxx I set in .config, the driver code is called with value 15000. I suppose this is due to the code in wdt-uclass.c which converts the config value to an integer number of seconds. So the limit I wrote in the code (15000 ms, 0xf0000 ticks) is actually "the max value the user can specify in configuration", whereas the hardware limit is a little higher (0xfffff ticks).
However, thinking twice it is probably not a good idea to deal with the behavior of higher layers here. So I will change back the driver limit to 0xfffff ticks and replace this long explanation with a minimal comment.
+ writel(PM_PASSWORD | timeout_ticks, base + PM_WDOG); + cur = readl(base + PM_RSTC); + writel(PM_PASSWORD | (cur & PM_RSTC_WRCFG_CLR) | + PM_RSTC_WRCFG_FULL_RESET, base + PM_RSTC);
This does not seem to be aligned with the "(" from the line above.
BTW: Please use scripts/checkpatch.pl to see, if the patch has no more issues.
I did run it, but apparently it did not catch this one. I will fix it.
+ if (priv->timeout_ticks > PM_WDOG_MAX_TICKS) { + printf("WARNING: bcm2835_wdt cannot handle large timeout values.\n"); + printf(" Setting the max value of 15000 ms instead.\n"); + printf(" Set CONFIG_WATCHDOG_TIMEOUT_MSECS=15000 at most " + "to avoid this warning.\n");
This seems way too long for a warning from this driver. Please shorten it to one line. The comment above already covers this limitation AFAICT.
OK.
Thanks, Etienne