On 08/06/2020 21.01, Robert Marko wrote:
> On Wed, May 20, 2020 at 2:35 PM Tom Rini <tr...@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].

Rasmus

Reply via email to