No subject

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 <wd at denx.de> wrote:

> Dear Eric,
>
> In message <CAOzkcmEydTOcGQ7=
> LL9NHkyuUtJJbE73DYceOsffphtrGU7ZYg at 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: wd at denx.de
> Is a computer language with goto's totally Wirth-less?
>

--20cf3071d0cebd882804b8e180df--
--20cf3071d0cebd882b04b8e180e1
Content-Type: application/octet-stream; name=patch
Content-Disposition: attachment; filename=patch
Content-Transfer-Encoding: base64
X-Attachment-Id: f_gym52aj71

TWFrZSBhdXRvbWF0aWMgd2F0Y2hkb2cgcmVzZXQgb3B0aW9uYWwuCiBCYXNlZCB1cG9uIGRlZmlu
aW5nIENPTkZJR19TWVNfV0FUQ0hET0dfRlJFUQoKU2lnbmVkLW9mZi1ieTogRXJpYyBPbHNlbiA8
ZW9sc2VuQGdvb2dsZS5jb20+CgotLS0gdS1ib290L2FyY2gvcG93ZXJwYy9saWIvaW50ZXJydXB0
cy5jLm9yaWcJMjAxMi0wMi0xMiAwMToxMTozMy4wMDAwMDAwMDAgLTA4MDAKKysrIHUtYm9vdC9h
cmNoL3Bvd2VycGMvbGliL2ludGVycnVwdHMuYwkyMDEyLTAyLTEzIDE1OjExOjQwLjg5MDc1ODQy
NiAtMDgwMApAQCAtNDAsMTAgKzQwLDYgQEAgdm9pZCBfX2JvYXJkX3Nob3dfYWN0aXZpdHkgKHVs
b25nIGR1bW15KQogfQogI2VuZGlmIC8qIENPTkZJR19TSE9XX0FDVElWSVRZICovCiAKLSNpZm5k
ZWYgQ09ORklHX1NZU19XQVRDSERPR19GUkVRCi0jZGVmaW5lIENPTkZJR19TWVNfV0FUQ0hET0df
RlJFUSAoQ09ORklHX1NZU19IWiAvIDIpCi0jZW5kaWYKLQogZXh0ZXJuIGludCBpbnRlcnJ1cHRf
aW5pdF9jcHUgKHVuc2lnbmVkICopOwogZXh0ZXJuIHZvaWQgdGltZXJfaW50ZXJydXB0X2NwdSAo
c3RydWN0IHB0X3JlZ3MgKik7CiAKQEAgLTEyMywxMCArMTE5LDEyIEBAIHZvaWQgdGltZXJfaW50
ZXJydXB0IChzdHJ1Y3QgcHRfcmVncyAqcmUKIAogCXRpbWVzdGFtcCsrOwogCisjaWZkZWYgQ09O
RklHX1NZU19XQVRDSERPR19GUkVRCiAjaWYgZGVmaW5lZChDT05GSUdfV0FUQ0hET0cpIHx8IGRl
ZmluZWQgKENPTkZJR19IV19XQVRDSERPRykKIAlpZiAoKHRpbWVzdGFtcCAlIChDT05GSUdfU1lT
X1dBVENIRE9HX0ZSRVEpKSA9PSAwKQogCQlXQVRDSERPR19SRVNFVCAoKTsKICNlbmRpZiAgICAv
KiBDT05GSUdfV0FUQ0hET0cgfHwgQ09ORklHX0hXX1dBVENIRE9HICovCisjZW5kaWYgICAgLyog
Q09ORklHX1NZU19XQVRDSERPR19GUkVRICovCiAKICNpZmRlZiBDT05GSUdfU1RBVFVTX0xFRAog
CXN0YXR1c19sZWRfdGljayAodGltZXN0YW1wKTsK
--20cf3071d0cebd882b04b8e180e1--