Wolfgang, 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. 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". 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. 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. 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. Comments are appreciated. --eric --------------------------------- Eric Olsen Staff Software Engineer Google, Inc. 650-253-2767 On Mon, Feb 13, 2012 at 12:36 PM, Wolfgang Denk <w...@denx.de> wrote: > Dear Eric, > > In message <CAOzkcmEydTOcGQ7= > ll9nhkyuutjjbe73dyceosffphtrgu7...@mail.gmail.com> you wrote: > > > > Today we discovered an issue with the implementation of watchdog on the > > PowerPc. In the file arch/powerpc/lib/interrupt.c, the code currently > > automatically feeds the watchdog via the timer interrupt handler if > either > > CONFIG_WATCHDOG or CONFIG_HW_WATCHDOG are defined. I am not exactly sure > > when this got added or why, but I'd guess that for most systems this is > not > > a good feature. For us it is exactly the opposite of what we want. > Would > > either of you object to a patch that uses a different config flag to > enable > > this. > > You don't mention it, but I assume you are referring to U-Boot code > here. > > Please always poost U-Boot related questions to the U-Boot mailing > list only (unless you are asking for commercial support from DENX). > > > I do not exactly understand why you think the current implementation > is not OK as it - my guess is that your expectations are higher than > what U-Boot offers. U-Boot is a boot loader, not an OS, and we use a > lot of simplistic concepts - trading complexity and features which > you can expect from a full-flavored OS for small code size and > simple, easily maintainable code. U-Boot is capable to triggering a > hardware watchdog (which is nevessary, as there are systems where > this cannot be switched off or suspended), but we make no attempts to > implement watchdog monitoring of the U-Boot code itself. If U-Boot > should hang or crash, this is not expected to be detected or > recovered by the watchdog. > > As such, the existing code does what it is supposed to do. > > > Patches to add additional, new features to U-Boot (like full watchdog > monitoring) are of course welcome. Please post these on the mailing > list, then. > > Thanks. > > 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: w...@denx.de > Is a computer language with goto's totally Wirth-less? >
patch
Description: Binary data
_______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
