Dear Eric,
please don't top-post / full quote!
In message CAOzkcmGNEmMGUvp7ETPTZnWj7ApiMRRD0=7JJG8vXmcNex2gnw@mail.gmail.com you wrote:
Sorry about not being clear in my earlier message. Let me attempt to point out why I think the current code is incorrect.
From a systems point of view watchdog is used to protect against a system becoming hung by bad code or corrupted memory or files. By automatically feeding them the code has circumvented this important feature.
As I explained before, U-Boot does not make any attempts to implement watchdog monitoring of the U-Boot code itself.
So some examples of where I would expect the watchdog to recover from but with the current code it will not. If u-boot loads a corrupted OS image, when the code starts but before it gets to the point that it has replaced u-boots exception handler it falls into an infinite loop it will hang and the uboot exception handler will continue to keep the system "running".
No, this is not correct. U-Boot will disable all interrupts before booting Linux, so the watdoch would reset the board in such situations.
Note that when we load the Linux kernel, we will overwrite all U-Boot exception handlers, so we could not execute these even if we tried.
Another example and how we actually discovered this bug due to a leak in a network driver that after many loops would run u-boot out of memory. Once this occurs the hush parser drops into a "for ;;" loop again this is not recovered by the watchdog.
Indeed - this may happen, and U-Boot currently does not attempt to protect against this.
So while I understand that some users may want to have the cpu automatically reset the watchdog, We don't. I believe that setting the default value of CONFIG_SYS_WATCHDOG_FREQ if it was not defined is a serious bug. The feature should not be enabled if I did not define this variable.
You misunderstand. There is not a feature we can enable: such a feature has not been implemented yet.
If you need it, you must implement it, before you can enable and use it.
As mentioned: patches welcome.
I have attached what we believe is a patch to address our concerns and still allow this to be set by a target that wishes to do so.
Ptaches must be inline (no MIME attachments, and folow certain rules. Please see http://www.denx.de/wiki/U-Boot/Patches for details.
Make automatic watchdog reset optional. Based upon defining CONFIG_SYS_WATCHDOG_FREQ
Signed-off-by: Eric Olsen eolsen@google.com
Sorry, but this will most likely break a numbr of boards.
On which exact hardware configurations did you test your patch?
Best regards,
Wolfgang Denk