[U-Boot] [PATCH] zlib: Add further watchdog reset calls
Patch 253cb831 [zlib: add watchdog reset call] added already a few watchdog reset calls to the new zlib U-Boot port. But on some boards this is not enough. Additional calls are needed on boards with short watchdog timeouts.
This was detected and tested on the lwmon5 board with a very short watchdog timeout. Without this patch, the board resets during Linux kernel decompression. With it, the decompression succeeds.
Signed-off-by: Stefan Roese sr@denx.de --- lib/zlib.c | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/lib/zlib.c b/lib/zlib.c index 26e5af1..39d5dab 100644 --- a/lib/zlib.c +++ b/lib/zlib.c @@ -1599,6 +1599,8 @@ int flush; strm->adler = state->check = adler32(0L, Z_NULL, 0); state->mode = TYPE; case TYPE: + if (strm->outcb != Z_NULL) + (*strm->outcb)(Z_NULL, 0); /* call WATCHDOG_RESET */ if (flush == Z_BLOCK) goto inf_leave; case TYPEDO: if (state->last) {
Hi Stefan,
Patch 253cb831 [zlib: add watchdog reset call] added already a few watchdog reset calls to the new zlib U-Boot port. But on some boards this is not enough. Additional calls are needed on boards with short watchdog timeouts.
This was detected and tested on the lwmon5 board with a very short watchdog timeout. Without this patch, the board resets during Linux kernel decompression. With it, the decompression succeeds.
Signed-off-by: Stefan Roese sr@denx.de
lib/zlib.c | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/lib/zlib.c b/lib/zlib.c index 26e5af1..39d5dab 100644 --- a/lib/zlib.c +++ b/lib/zlib.c @@ -1599,6 +1599,8 @@ int flush; strm->adler = state->check = adler32(0L, Z_NULL, 0); state->mode = TYPE; case TYPE:
if (strm->outcb != Z_NULL)(*strm->outcb)(Z_NULL, 0); /* call WATCHDOG_RESET */ if (flush == Z_BLOCK) goto inf_leave; case TYPEDO: if (state->last) {
Can you please tell me (and include in the comment) why this calls a watchdog reset and why you did not use a "regular plain WATCHDOG_RESET"?
Thanks! Detlev
Hi Detlev,
On Thursday 02 September 2010 18:40:38 Detlev Zundel wrote:
case TYPE:
if (strm->outcb != Z_NULL)(*strm->outcb)(Z_NULL, 0); /* call WATCHDOG_RESET */ if (flush == Z_BLOCK) goto inf_leave; case TYPEDO: if (state->last) {Can you please tell me (and include in the comment) why this calls a watchdog reset and why you did not use a "regular plain WATCHDOG_RESET"?
I did it this way, because that's the way these watchdog reset calls have been implemented in the U-Boot zlib version till now. Frankly I'm not sure why it was done this way instead of using "regular plain WATCHDOG_RESET" calls. Perhaps Wolfgang remembers the reasoning behind it.
I could change this function pointer approach to "regular plain WATCHDOG_RESET" call though if preferred. Just let me know and I'll try to come up with a new patch version.
Cheers, Stefan
-- DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-0 Fax: (+49)-8142-66989-80 Email: office@denx.de
Dear Stefan Roese,
In message 201009030910.08643.sr@denx.de you wrote:
I did it this way, because that's the way these watchdog reset calls have been implemented in the U-Boot zlib version till now. Frankly I'm not sure why it was done this way instead of using "regular plain WATCHDOG_RESET" calls. Perhaps Wolfgang remembers the reasoning behind it.
It allows to easily adjust the granularity of trigger points depending on data block size.
Best regards,
Wolfgang Denk
Hi Wolfgang,
On Friday 03 September 2010 10:30:24 Wolfgang Denk wrote:
I did it this way, because that's the way these watchdog reset calls have been implemented in the U-Boot zlib version till now. Frankly I'm not sure why it was done this way instead of using "regular plain WATCHDOG_RESET" calls. Perhaps Wolfgang remembers the reasoning behind it.
It allows to easily adjust the granularity of trigger points depending on data block size.
Hmmm, I fail to see how the current implementation would differ from the one Detlev suggested:
"outcb" is initialised with either WATCHDOG_RESET or NULL in gunzip.c. Later on in zlib.c, the function referenced by outcb is called if not NULL. So those statements:
if (strm->outcb != Z_NULL) (*strm->outcb)(Z_NULL, 0);
could be replaced by:
WATCHDOG_RESET;
Perhaps I'm missing something?
Cheers, Stefan
-- DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-0 Fax: (+49)-8142-66989-80 Email: office@denx.de
Dear Stefan Roese,
In message 1283274708-32533-1-git-send-email-sr@denx.de you wrote:
Patch 253cb831 [zlib: add watchdog reset call] added already a few watchdog reset calls to the new zlib U-Boot port. But on some boards this is not enough. Additional calls are needed on boards with short watchdog timeouts.
This was detected and tested on the lwmon5 board with a very short watchdog timeout. Without this patch, the board resets during Linux kernel decompression. With it, the decompression succeeds.
Signed-off-by: Stefan Roese sr@denx.de
lib/zlib.c | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/lib/zlib.c b/lib/zlib.c index 26e5af1..39d5dab 100644 --- a/lib/zlib.c +++ b/lib/zlib.c @@ -1599,6 +1599,8 @@ int flush; strm->adler = state->check = adler32(0L, Z_NULL, 0); state->mode = TYPE; case TYPE:
if (strm->outcb != Z_NULL)(*strm->outcb)(Z_NULL, 0); /* call WATCHDOG_RESET */ if (flush == Z_BLOCK) goto inf_leave;
Indentation wrong?
Best regards,
Wolfgang Denk
Hi Wolfgang,
On Thursday 09 September 2010 20:27:08 Wolfgang Denk wrote:
+++ b/lib/zlib.c @@ -1599,6 +1599,8 @@ int flush;
strm->adler = state->check = adler32(0L, Z_NULL, 0); state->mode = TYPE; case TYPE:
if (strm->outcb != Z_NULL)(*strm->outcb)(Z_NULL, 0); /* call WATCHDOG_RESET */ if (flush == Z_BLOCK) goto inf_leave;Indentation wrong?
Might be. But please see the following patchset, which supersedes this patch:
[PATCH 1/2] zlib/gunzip: Use WATCHDOG_RESET macro http://www.mail-archive.com/u-boot@lists.denx.de/msg37605.html
[PATCH 2/2 v2] zlib: Add further watchdog reset calls http://www.mail-archive.com/u-boot@lists.denx.de/msg37606.html
Thanks.
Cheers, Stefan
-- DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-0 Fax: (+49)-8142-66989-80 Email: office@denx.de
participants (3)
-
Detlev Zundel -
Stefan Roese -
Wolfgang Denk