On Wed, Jun 10, 2020 at 11:13:37AM +0200, Rasmus Villemoes wrote:
On 08/06/2020 21.01, Robert Marko wrote:
On Wed, May 20, 2020 at 2:35 PM Tom Rini trini@konsulko.com 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
--- 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: { 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].
Alright, so first thanks for digging here. Looking over the tools, for verifying the signature we don't need to decompress the contents as we are just checking the signature. For dumpimage it's also OK to dump a compressed file and leave it to regular tools to decompress. Do you want to make a patch to guard the existing section and CONFIG_IS_ENABLED it or should I and Suggested-by you? Thanks!