Dear Eric, please don't top-post / full quote!
In message <CAOzkcmGNEmMGUvp7ETPTZnWj7ApiMRRD0=7jjg8vxmcnex2...@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 <[email protected]> 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 -- DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: [email protected] All he had was nothing, but that was something, and now it had been taken away. - Terry Pratchett, _Sourcery_ _______________________________________________ U-Boot mailing list [email protected] http://lists.denx.de/mailman/listinfo/u-boot
