[U-Boot] [PATCH] zlib: Add further watchdog reset calls

[U-Boot] [PATCH] zlib: Add further watchdog reset calls

[Stefan Roese]

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) {
-- 
1.7.2.2

[U-Boot] [PATCH] zlib: Add further watchdog reset calls

[Detlev Zundel]

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

-- 
(3)   With sufficient thrust,  pigs fly just fine.  However, this is not
necessarily a good idea.  It is hard to be sure  where they are going to
land, and it could be dangerous sitting under them as they fly overhead.
                              -- The Twelve Networking Truths (RFC 1925)
--
DENX Software Engineering GmbH,      MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich,  Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: dzu at denx.de

[U-Boot] [PATCH] zlib: Add further watchdog reset calls

[Stefan Roese]

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

[U-Boot] [PATCH] zlib: Add further watchdog reset calls

[Wolfgang Denk]

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

-- 
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
You know that feeling when you're leaning back  on  a  stool  and  it
starts to tip over? Well, that's how I feel all the time.
- Steven Wright

[U-Boot] [PATCH] zlib: Add further watchdog reset calls

[Stefan Roese]

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

[U-Boot] [PATCH] zlib: Add further watchdog reset calls

[Wolfgang Denk]

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

-- 
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
There is a theory which states that if ever anyone discovers  exactly
what  the  Universe is for and why it is here, it will instantly dis-
appear and be replaced by something even more  bizarre  and  inexpli-
cable.  There  is  another  theory which states that this has already
happened.    -- Douglas Adams, "The Hitchhiker's Guide to the Galaxy"

[U-Boot] [PATCH] zlib: Add further watchdog reset calls

[Stefan Roese]

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 at lists.denx.de/msg37605.html

[PATCH 2/2 v2] zlib: Add further watchdog reset calls
http://www.mail-archive.com/u-boot at 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 at denx.de