Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46458 )
Change subject: tests: Add lib/imd_cbmem-test test case ......................................................................
Patch Set 3: Code-Review+1
(11 comments)
https://review.coreboot.org/c/coreboot/+/46458/3/tests/lib/Makefile.inc File tests/lib/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/46458/3/tests/lib/Makefile.inc@24 PS3, Line 24: # huh?
https://review.coreboot.org/c/coreboot/+/46458/3/tests/lib/imd_cbmem-test.c File tests/lib/imd_cbmem-test.c:
https://review.coreboot.org/c/coreboot/+/46458/3/tests/lib/imd_cbmem-test.c@... PS3, Line 3: UUT Unit Under Test?
https://review.coreboot.org/c/coreboot/+/46458/3/tests/lib/imd_cbmem-test.c@... PS3, Line 5: * another header file nit: missing trailing period
https://review.coreboot.org/c/coreboot/+/46458/3/tests/lib/imd_cbmem-test.c@... PS3, Line 26: please, no space after casts
https://review.coreboot.org/c/coreboot/+/46458/3/tests/lib/imd_cbmem-test.c@... PS3, Line 28: please, no space after casts
https://review.coreboot.org/c/coreboot/+/46458/3/tests/lib/imd_cbmem-test.c@... PS3, Line 32: void cbmem_run_init_hooks(int is_recovery) Maybe add a comment explaining the purpose of this no-op function?
https://review.coreboot.org/c/coreboot/+/46458/3/tests/lib/imd_cbmem-test.c@... PS3, Line 34: please, no space after casts
https://review.coreboot.org/c/coreboot/+/46458/3/tests/lib/imd_cbmem-test.c@... PS3, Line 191: 0x10 I wonder... Do the IDs need to be sequential for any particular reason? And why write them in hex? They could also be const
https://review.coreboot.org/c/coreboot/+/46458/3/tests/lib/imd_cbmem-test.c@... PS3, Line 267: status state?
https://review.coreboot.org/c/coreboot/+/46458/3/tests/lib/imd_cbmem-test.c@... PS3, Line 269: (uintptr_t) Is this cast required?
https://review.coreboot.org/c/coreboot/+/46458/3/tests/lib/imd_cbmem-test.c@... PS3, Line 273: status state?