Hi Julius, On 2 October 2015 at 23:18, Julius Werner <jwer...@chromium.org> wrote: >> >> 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?
Sounds good. I think it makes sense to enable all possible options in sandbox - since it helps with build testing too. > >> 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.) You can build U-Boot with NO_SDL=1 I have this on my goobuntu laptop: ~> dpkg -l |grep -i sdl ii libsdl1.2-dev 1.2.15-8ubuntu1.1 amd64 Simple DirectMedia Layer development files ii libsdl1.2debian:amd64 1.2.15-8ubuntu1.1 amd64 Simple DirectMedia Layer > >> >> 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. Right. > >> >> 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. Sounds good. Regards, Simon _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
