Attention is currently required from: Xi Chen, Paul Menzel, Xixi Chen, Yu-Ping Wu. Hung-Te Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/61334 )
Change subject: soc/mediatek: Pass dram info to cbmem ......................................................................
Patch Set 18:
(6 comments)
File src/soc/mediatek/common/memory.c:
https://review.coreboot.org/c/coreboot/+/61334/comment/c3c2e039_8c0149f1 PS18, Line 107: blob probably not 'blob'. what about 'ddr_info' or 'ddr'?
https://review.coreboot.org/c/coreboot/+/61334/comment/5249c7b1_f299c0bb PS18, Line 106: struct mem_chip *p = &curr_dram_info; : const struct ddr_base_info *blob = curr_ddr_info; move these to the parameters and pass them from the caller?
maybe rename like fill_dram_info()
https://review.coreboot.org/c/coreboot/+/61334/comment/70f89f3d_78943cc7 PS18, Line 107: only one space
https://review.coreboot.org/c/coreboot/+/61334/comment/fe2223dc_dcdedd50 PS18, Line 112: unsigned int i = 0 move to beginning of the function and declare only one time
https://review.coreboot.org/c/coreboot/+/61334/comment/175cdb22_416f49ec PS18, Line 128: /* Add cbmem */ the cbmem_add is clear enough, no need to have this comment
https://review.coreboot.org/c/coreboot/+/61334/comment/bdf480a0_04248a08 PS18, Line 130: + sizeof(curr_dram_info.channel) * CHANNEL_MAX) so we allocated more but won't copy the rest of them? (the memcpy only copies sizeof(curr_dram_info)