Attention is currently required from: Ravi Kumar Bokka. Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/63026 )
Change subject: soc/qualcomm/common: verify size of memchipinfo structure ......................................................................
Patch Set 1:
(7 comments)
File src/commonlib/bsd/include/commonlib/bsd/mem_chip_info.h:
https://review.coreboot.org/c/coreboot/+/63026/comment/42557f70_dce7e329 PS1, Line 5: This needs to #include <stddef.h> for size_t now.
https://review.coreboot.org/c/coreboot/+/63026/comment/090277f7_bac0856a PS1, Line 28: static inline size_t mem_chip_info_size(struct mem_chip_info *info) {
open brace '{' following function definitions go on the next line
Please fix.
https://review.coreboot.org/c/coreboot/+/63026/comment/41c90498_803fa329 PS1, Line 29: return sizeof(*info) + sizeof(info->channel[0]) * info->num_channels;
please, no spaces at the start of a line
Please fix.
File src/soc/qualcomm/common/qclib.c:
https://review.coreboot.org/c/coreboot/+/63026/comment/dd42c618_11696979 PS1, Line 31: mem_chip_info_size(&memchip) No you're supposed to check the mem_chip_info at te->blob_address, not some random mem_chip_info you just made up. (te->size == mem_chip_info_size((void *)te->blob_address))
https://review.coreboot.org/c/coreboot/+/63026/comment/ec4626d7_3b7f83b1 PS1, Line 51: sizeof(struct mem_chip_info) mem_chip_info_size(mem_chip_addr)
https://review.coreboot.org/c/coreboot/+/63026/comment/fe26d801_c39cb580 PS1, Line 55: memchip Again this is supposed to be mem_chip_addr here.
https://review.coreboot.org/c/coreboot/+/63026/comment/c66a77e7_86cfb6cc PS1, Line 186: mem_chip_addr, sizeof(mem_chip_addr), 0); I left more comments in the other CL, please address all of them (and mark them Done in that CL so we can more easily follow).