From patchwork Wed Jun 10 09:13:37 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Rasmus Villemoes X-Patchwork-Id: 242037 List-Id: U-Boot discussion From: rasmus.villemoes at prevas.dk (Rasmus Villemoes) Date: Wed, 10 Jun 2020 11:13:37 +0200 Subject: [PATCH] image: Add support for ZSTD decompression In-Reply-To: References: <20200425173721.2759955-1-robert.marko@sartura.hr> <20200501145605.GG4468@bill-the-cat> <20200501164246.GA12564@bill-the-cat> <20200504130358.GG12564@bill-the-cat> <20200520123528.GA26741@bill-the-cat> Message-ID: On 08/06/2020 21.01, Robert Marko wrote: > On Wed, May 20, 2020 at 2:35 PM Tom Rini wrote: >> >> On Wed, May 20, 2020 at 01:38:01PM +0200, Robert Marko wrote: >> >>> Tom, >>> I have tried various things but CONFIG_IS_ENABLED won't work inside of >>> switch case. >>> It works fine outside of if though. >> >> OK, thanks, I'll poke things more to make sure what I'm worried about >> doesn't happen. > > Hi, > were you able to test the commit? So I took a look at this since I'm also interested in getting zstd support in. And the problem is that when building host tools, the IS_ENABLED / CONFIG_IS_ENABLED logic doesn't make much sense anyway - one can of course include the kconfig.h header that defines those macros to avoid the cpp error (one has to be careful, because the headers included vary wildly depending on USE_HOSTCC), but I don't think it's meaningful - the host tools should not depend on configuration for any specific board. Nor do they currently; the use of ifdef CONFIG_GZIP for example just means that that part is ignored for the host tools - doing uint size = unc_len; should be a no-op when CONFIG_GZIP=y, but it breaks the build of the host tools. That, in turn, means that the fit_check_sign tool (which AFAICT is the only host tool that actually care about this - we really should build host tools with the -ffunction-sections,-fdata-sections,-Wl,--gc-sections flags) currently "works" in the sense that it does check the signature, but it doesn't really check if the image(s) can be decompressed using the vendored copy of the decompression code; hidden in the output is Unimplemented compression type 5 but for some reason that ends up being ok. So, for now, I suggest simply surrounding all the CASE_IH_COMP_* (except NONE) by a #ifndef USE_HOSTCC - I don't think that will change anything at all for the existing host tools. Then you should be able to use the proper CONFIG_IS_ENABLED(ZSTD) to guard the IH_COMP_ZSTD case. Also, it would allow us to change the existing bare ifdefs to be SPL-aware, i.e. change #ifdef CONFIG_GZIP to #if CONFIG_IS_ENABLED(GZIP) [assuming the SPL_GZIP symbol actually exists]. Rasmus --- a/common/image.c +++ b/common/image.c @@ -430,12 +430,12 @@ int image_decomp(int comp, ulong load, ulong image_start, int type, else ret = -ENOSPC; break; -#ifdef CONFIG_GZIP +//#ifdef CONFIG_GZIP case IH_COMP_GZIP: { ret = gunzip(load_buf, unc_len, image_buf, &image_len); break; } -#endif /* CONFIG_GZIP */ +//#endif /* CONFIG_GZIP */ #ifdef CONFIG_BZIP2 case IH_COMP_BZIP2: {