[U-Boot] [PATCH] Add support for LZ4 decompression algorithm

[Julius Werner]

>
> I get this build error with sandbox.
>
> test/built-in.o: In function `uncompress_using_lz4':
> /home/sjg/c/src/third_party/u-boot/files/test/compression.c:278:
> undefined reference to `ulz4fn'
>

Yeah... that's because you didn't configure it in. I'm not really sure how
this is supposed to work... there should really be some sort of dependency
between selecting the compression tests and selecting the algorithm, or the
algorithms should be #ifdefed out like in bootm.c. But neither of those
seems to be in place right now... include/configs/sandbox.h just enables
all the other compression algorithms, that's why it works for them.

I could add a 'select LZ4' to config UNIT_TEST in the Kconfig to create
such a dependency, I think that would be the best option?

You should be able to run the tests using:
>
> make O=sandbox defconfig all
>  ./sandbox/u-boot -c "ut_compression"


I tried, but I can't find myself through the SDL dependency hell... sorry.
I installed Ubuntu's libsdl2-dev but apparently that wasn't enough... I
still get "make[2]: sdl-config: Command not found" and
"../arch/sandbox/cpu/sdl.c:9:21: fatal error: SDL/SDL.h: No such file or
directory" when I'm trying to build. (I ran it with make -k now so I
confirmed that it builds both lib/lz4_wrapper.o and test/compression.o
without errors. I just can't test the linking step.)


> Can you instead add this option to lib/Kconfig and put your help
> there? We are moving away from the old CONFIGS.


Done in next version.

You should be able to replace this license with
>
> SPDX-License-Identifier: BSD-2-Clause
> [...]
> Should be able to use:
>
>  SPDX-License-Identifier: GPL-2.0+ BSD-2-Clause


Done in next version.


> I can see why you want to keep lz4.c as is. But this file is written
> by you, isn't it? If so, can you fix the checkpatch errors that are
> fixable (e.g. run 'patman').
>
> warning: lib/lz4_wrapper.c,41: do not add new typedefs
> warning: lib/lz4_wrapper.c,42: do not add new typedefs
> warning: lib/lz4_wrapper.c,43: do not add new typedefs
> warning: lib/lz4_wrapper.c,44: do not add new typedefs
> warning: lib/lz4_wrapper.c,45: do not add new typedefs
> warning: lib/lz4_wrapper.c,47: storage class should be at the
> beginning of the declaration
> error: lib/lz4_wrapper.c,59: spaces prohibited around that ':' (ctx:WxW)
> error: lib/lz4_wrapper.c,60: spaces prohibited around that ':' (ctx:WxW)
> error: lib/lz4_wrapper.c,61: spaces prohibited around that ':' (ctx:WxW)
> error: lib/lz4_wrapper.c,62: spaces prohibited around that ':' (ctx:WxW)
> error: lib/lz4_wrapper.c,63: spaces prohibited around that ':' (ctx:WxW)
> error: lib/lz4_wrapper.c,64: spaces prohibited around that ':' (ctx:WxW)
> error: lib/lz4_wrapper.c,70: spaces prohibited around that ':' (ctx:WxW)
> error: lib/lz4_wrapper.c,71: spaces prohibited around that ':' (ctx:WxW)
> error: lib/lz4_wrapper.c,72: spaces prohibited around that ':' (ctx:WxW)
> warning: lib/lz4_wrapper.c,77: __packed is preferred over
> __attribute__((packed))
> warning: lib/lz4_wrapper.c,77: Adding new packed members is to be done
> with care
> error: lib/lz4_wrapper.c,83: spaces prohibited around that ':' (ctx:WxW)
> error: lib/lz4_wrapper.c,84: spaces prohibited around that ':' (ctx:WxW)
> warning: lib/lz4_wrapper.c,89: __packed is preferred over
> __attribute__((packed))
> warning: lib/lz4_wrapper.c,89: Adding new packed members is to be done
> with care
> error: test/compression.c,282: return is not a function, parentheses
> are not required


Okay, I replaced __attribute__((packed)) with __packed and fixed the
whitespace for bit fields. I think those are the only actionable ones...
the camel case comes from names inside lz4.c, and I need the typedefs to
map U-Boot's types to the ones lz4.c expects.


> Could you return int here, and use proper error numbers below? Like
> -EINVAL, -EPROTONOSUPPORT, -ENOSPC, etc.


Okay, I switched it to the model where it returns an int and the size is an
output pointer, like the other algorithms.