Attention is currently required from: Hung-Te Lin, Shelley Chen, Paul Menzel, Jianjun Wang. Yu-Ping Wu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/59738 )
Change subject: helpers: Add fls support ......................................................................
Patch Set 1:
(6 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/59738/comment/a5f12654_c430502d PS1, Line 12: only used define fls only
https://review.coreboot.org/c/coreboot/+/59738/comment/caf3f59a_515945dd PS1, Line 15: This code is copied from Linux kernel in: include/asm-generic/bitops/fls.h The license here (BSD) is different (from kernel's GPL-2.0), so we usually don't copy things from kernel. No need to mention that in the commit message.
File src/commonlib/bsd/include/commonlib/bsd/helpers.h:
https://review.coreboot.org/c/coreboot/+/59738/comment/488099de_1d1313c8 PS1, Line 129: ARCH_X86 We should use
#if CONFIG(ARCH_X86)
There's build error for x86 platforms:
In file included from src/commonlib/include/commonlib/helpers.h:10, from src/include/stddef.h:4, from src/include/stdarg.h:11, from src/include/console/vtxprintf.h:6, from src/include/console/console.h:7, from src/cpu/x86/mtrr/debug.c:3: src/commonlib/bsd/include/commonlib/bsd/helpers.h:137:28: note: previous definition of 'fls' with type 'int(unsigned int)' 137 | static __always_inline int fls(unsigned int x) | ^~~ make[1]: *** [Makefile:381: /cb-build/coreboot-gerrit.0/chromeos/AMD_MAJOLICA/bootblock/cpu/x86/mtrr/debug.o] Error 1 make[1]: Leaving directory '/home/coreboot/node-root/workspace/coreboot-gerrit' EMULATION_QEMU_X86_Q35_SMM_TSEG build FAILED after 2s!
https://review.coreboot.org/c/coreboot/+/59738/comment/d134ec8b_8f17fa5f PS1, Line 132: @x: the word to search @param x Input number.
https://review.coreboot.org/c/coreboot/+/59738/comment/8974d8bc_6aa4d9f0 PS1, Line 134: ffs There's no such function here. Please remove the comment.
https://review.coreboot.org/c/coreboot/+/59738/comment/d61c5775_8ca4e091 PS1, Line 137: fls Please add unit tests. See CB:56543 as an example.