Re: [U-Boot] [PATCH v3] zlib: updated to v.1.2.3
Please, is there somebody that can test this patch on a differen arch than PPC?
Arm: success.
tornado% git log --pretty=oneline | head -1 b201171f2b4d509f3ad510b214bee70ff902e3d6 zlib: updated to v.1.2.3
U-Boot 2009.06-00494-gb201171 (Jul 27 2009 - 17:05:15)
[...] ## Booting kernel from Legacy Image at 01308000 ... Image Name: Linux-2.6.31-rc3 Image Type: ARM Linux Kernel Image (gzip compressed) Data Size: 1706717 Bytes = 1.6 MB Load Address: 00008000 Entry Point: 00008000 Uncompressing Kernel Image ... OK
/alessandro
On Monday 27 July 2009 17:09:28 Alessandro Rubini wrote:
Please, is there somebody that can test this patch on a differen arch than PPC?
I just noticed a common difference in the failing uncompression's. Load address is 0x0 in this case. Perhaps you could give this a try on your system as well (if possible).
I'll try to uncompress to something != 0 tomorrow on PPC.
Best regards, 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 =====================================================================
The entry point of an image can be 0 (like on PowerPC), so allow next_out to be NULL in inflate()
Signed-off-by: Alessandro Rubini rubini@unipv.it ---
Stefan Roese:
I'll try to uncompress to something != 0 tomorrow on PPC.
You are right. I naively thought my arm has RAM at 0 like PPC, but entry point of image is not really 0.
lib_generic/zlib.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/lib_generic/zlib.c b/lib_generic/zlib.c index f415f6b..49fb145 100644 --- a/lib_generic/zlib.c +++ b/lib_generic/zlib.c @@ -1365,7 +1365,7 @@ int flush; static const unsigned short order[19] = /* permutation of code lengths */ {16, 17, 18, 0, 8, 7, 9, 6, 10, 5, 11, 4, 12, 3, 13, 2, 14, 1, 15};
- if (strm == Z_NULL || strm->state == Z_NULL || strm->next_out == Z_NULL || + if (strm == Z_NULL || strm->state == Z_NULL || (strm->next_in == Z_NULL && strm->avail_in != 0)) return Z_STREAM_ERROR;
Alessandro Rubini wrote:
The entry point of an image can be 0 (like on PowerPC), so allow next_out to be NULL in inflate()
Hi,
this patch solves the inflate() error but now a watchdog reset is triggered during decompression:
## Booting kernel from Legacy Image at 00200000 ... Image Name: Linux-2.4.37.2-dbox2 Image Type: PowerPC Linux Kernel Image (gzip compressed) Data Size: 772042 Bytes = 753.9 kB Load Address: 00000000 Entry Point: 00000000 Verifying Checksum ... OK Uncompressing Kernel Image ... debug: DDF: Calibrating delay loop... debug: DDF: 66.76 BogoMIPS debug: WATCHDOG RESET [...]
The neither the old nor the new code incorporate WATCHDOG_RESET() so the new code seems to be slower than the old one which works. I have yet to dig through the zlib code to insert some WATCHDOG_RESET() here and there like I did with the LZMA code.
Cheers, rhabarber1848
Dear rhabarber1848,
In message h4kq2v$eoj$1@ger.gmane.org you wrote:
this patch solves the inflate() error but now a watchdog reset is triggered during decompression:
...
The neither the old nor the new code incorporate WATCHDOG_RESET() so the new
You are wrong. The urrent code installs WATCHDOG_RESET as s.outcb() callback; see function zunzip() in "lib_generic/gunzip.c".
code seems to be slower than the old one which works. I have yet to dig through the zlib code to insert some WATCHDOG_RESET() here and there like I did with the LZMA code.
I think this is the wrong thing to do. Use the provided callback instead.
I wonder why you do not create a patch between the old original version and what wd have in U-Boot now (which should clearly show what has been changed to adapt this code for U-Boot), and then apply (if necessary, manually) that patch to the current version?
Best regards,
Wolfgang Denk
Wolfgang Denk wrote:
I wonder why you do not create a patch between the old original version and what wd have in U-Boot now (which should clearly show what has been changed to adapt this code for U-Boot), and then apply (if necessary, manually) that patch to the current version?
Hi Wolfgang,
I did exactly that, I took rc1, downloaded the zlib-1.2.3 patch from the newsgroup[1] and added the patch by Alessandro[2]. The WATCHDOG_RESET code in gunzip.c is not touched by either patches, but I still get a WATCHDOG_RESET when the kernel image is decompressed with the new code.
Though is seems that
if (z->outcb != Z_NULL) (*z->outcb)(Z_NULL, 0);
is not used anymore in zlib.c so the WATCHDOG_RESET calls are silently ignored. I will try to re-add them tomorrow.
Besides this, rc1 is working fine here.
Cheers, rhabarber1848
[1] http://article.gmane.org/gmane.comp.boot-loaders.u-boot/64727 [2] http://article.gmane.org/gmane.comp.boot-loaders.u-boot/64969
Dear rhabarber1848,
In message h4l1p8$d52$1@ger.gmane.org you wrote:
Though is seems that
if (z->outcb != Z_NULL) (*z->outcb)(Z_NULL, 0);
is not used anymore in zlib.c so the WATCHDOG_RESET calls are silently ignored. I will try to re-add them tomorrow.
I see.
Besides this, rc1 is working fine here.
Thanks for the confirmation.
Best regards,
Wolfgang Denk
On Monday 27 July 2009 18:40:31 Alessandro Rubini wrote:
The entry point of an image can be 0 (like on PowerPC), so allow next_out to be NULL in inflate()
Signed-off-by: Alessandro Rubini rubini@unipv.it
Stefan Roese:
I'll try to uncompress to something != 0 tomorrow on PPC.
You are right. I naively thought my arm has RAM at 0 like PPC, but entry point of image is not really 0.
Yes, this works:
Tested-by: Stefan Roese sr@denx.de
Thanks.
Best regards, 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 =====================================================================
Very good!!! So is it time to re-apply zlib patch to u-boot main tree, together Alessandro's patch?
Wolfgang, what's your position?
Best Regards, Giuseppe
-----Original Message----- From: u-boot-bounces@lists.denx.de [mailto:u-boot-bounces@lists.denx.de] On Behalf Of Stefan Roese Sent: Tuesday, July 28, 2009 8:31 AM To: Alessandro Rubini Cc: u-boot@lists.denx.de; rhabarber1848@web.de Subject: Re: [U-Boot] [PATCH] zlib: allow 0 as destination pointer
On Monday 27 July 2009 18:40:31 Alessandro Rubini wrote:
The entry point of an image can be 0 (like on PowerPC), so allow next_out to be NULL in inflate()
Signed-off-by: Alessandro Rubini rubini@unipv.it
Stefan Roese:
I'll try to uncompress to something != 0 tomorrow on PPC.
You are right. I naively thought my arm has RAM at 0 like PPC, but entry point of image is not really 0.
Yes, this works:
Tested-by: Stefan Roese sr@denx.de
Thanks.
Best regards, 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 ===================================================================== _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Dear Giuseppe CONDORELLI,
In message 001601ca1012$e0f72730$c08182a4@st.com you wrote:
Very good!!! So is it time to re-apply zlib patch to u-boot main tree, together Alessandro's patch?
Wolfgang, what's your position?
Actually I'm waiting for a new patch that integrates that fix.
Best regards,
Wolfgang Denk
Dear Wolfgang,
Actually I'm waiting for a new patch that integrates that fix.
Maybe I've not understood. Do you mean you are waiting a new zlib patch [v4] that includes Alessandro's patch, to apply only one patch containing all?
Best Regards, Giuseppe -----Original Message----- From: Wolfgang Denk [mailto:wd@denx.de] Sent: Wednesday, July 29, 2009 8:24 AM To: Giuseppe CONDORELLI Cc: u-boot@lists.denx.de Subject: Re: [U-Boot] [PATCH] zlib: allow 0 as destination pointer
Dear Giuseppe CONDORELLI,
In message 001601ca1012$e0f72730$c08182a4@st.com you wrote:
Very good!!! So is it time to re-apply zlib patch to u-boot main tree, together Alessandro's patch?
Wolfgang, what's your position?
Actually I'm waiting for a new patch that integrates that fix.
Best regards,
Wolfgang Denk
Dear Giuseppe CONDORELLI,
In message 001701ca1015$dbf7fae0$c08182a4@st.com you wrote:
Actually I'm waiting for a new patch that integrates that fix.
Maybe I've not understood. Do you mean you are waiting a new zlib patch [v4] that includes Alessandro's patch, to apply only one patch containing all?
Yes. It makes no sense to add a known to be broken patch and fix it in a second commit, as this leaves only a state in the tree where attempts to bisect any issues run into problems. So please submit a new, fixed patch, that really works.
Best regards,
Wolfgang Denk
Wolfgang Denk wrote:
Dear Giuseppe CONDORELLI,
In message 001601ca1012$e0f72730$c08182a4@st.com you wrote:
Very good!!! So is it time to re-apply zlib patch to u-boot main tree, together Alessandro's patch?
Wolfgang, what's your position?
Actually I'm waiting for a new patch that integrates that fix.
Hi,
please do not commit the zlib-1.2.3 patch in its current state, although Alessandro's patch fixes on bug there is still the problem that the WATCHDOG_RESET() calls from gunzip.c
#if defined(CONFIG_HW_WATCHDOG) || defined(CONFIG_WATCHDOG) s.outcb = (cb_func)WATCHDOG_RESET; #else s.outcb = Z_NULL; #endif<>/* CONFIG_HW_WATCHDOG */
are silently ignored in the new zlib code. My C knowledge is insufficient to solve this problem. In its current state it will break zlib decompression on the machines I am taking care of.
Cheers, rhabarber1848
Dear rhabarber1848,
In message h4orm0$l1d$1@ger.gmane.org you wrote:
Actually I'm waiting for a new patch that integrates that fix.
please do not commit the zlib-1.2.3 patch in its current state, although Alessandro's patch fixes on bug there is still the problem that the WATCHDOG_RESET() calls from gunzip.c
#if defined(CONFIG_HW_WATCHDOG) || defined(CONFIG_WATCHDOG) s.outcb = (cb_func)WATCHDOG_RESET; #else s.outcb = Z_NULL; #endif<>/* CONFIG_HW_WATCHDOG */
are silently ignored in the new zlib code. My C knowledge is insufficient to solve this problem. In its current state it will break zlib decompression on the machines I am taking care of.
Agreed - that should be working, too, before we pull that patch into mainline.
Best regards,
Wolfgang Denk
Hi rhabarber1848,
please could you test latest two zlib patches I have sent few minutes ago?
NOTE: the first one is waiting for approval due to its big size.
Thanks, I hope these patches will solve your issue.
G.
-----Original Message----- From: u-boot-bounces@lists.denx.de [mailto:u-boot-bounces@lists.denx.de] On Behalf Of Wolfgang Denk Sent: Wednesday, July 29, 2009 9:12 AM To: rhabarber1848@web.de Cc: u-boot@lists.denx.de Subject: Re: [U-Boot] [PATCH] zlib: allow 0 as destination pointer
Dear rhabarber1848,
In message h4orm0$l1d$1@ger.gmane.org you wrote:
Actually I'm waiting for a new patch that integrates that fix.
please do not commit the zlib-1.2.3 patch in its current state, although Alessandro's patch fixes on bug there is still the problem that the WATCHDOG_RESET() calls from gunzip.c
#if defined(CONFIG_HW_WATCHDOG) || defined(CONFIG_WATCHDOG) s.outcb = (cb_func)WATCHDOG_RESET; #else s.outcb = Z_NULL; #endif<>/* CONFIG_HW_WATCHDOG */
are silently ignored in the new zlib code. My C knowledge is insufficient
to
solve this problem. In its current state it will break zlib decompression on the machines I am taking care of.
Agreed - that should be working, too, before we pull that patch into mainline.
Best regards,
Wolfgang Denk
participants (5)
-
Alessandro Rubini -
Giuseppe CONDORELLI -
rhabarber1848 -
Stefan Roese -
Wolfgang Denk