Attention is currently required from: Hung-Te Lin, Xi Chen, Paul Menzel, Julius Werner, Xixi Chen. Yu-Ping Wu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/61334 )
Change subject: soc/mediatek: Pass dram info to cbmem ......................................................................
Patch Set 23:
(14 comments)
File src/soc/mediatek/common/memory.c:
https://review.coreboot.org/c/coreboot/+/61334/comment/7bef2169_f9291a24 PS21, Line 29: curr_dram_info
Please remove this.
Done
https://review.coreboot.org/c/coreboot/+/61334/comment/9ae4a525_4ae8aedf PS21, Line 108: p
channel
Done
https://review.coreboot.org/c/coreboot/+/61334/comment/6764cbff_b6bddea7 PS21, Line 117: total_density / mc->num_channels
mem_chip_info -> channel -> density is per channel. […]
Ack
https://review.coreboot.org/c/coreboot/+/61334/comment/eb3642b6_a163bd54 PS21, Line 129: void *mem_region_base
struct mem_chip_info *mc;
Done
https://review.coreboot.org/c/coreboot/+/61334/comment/ceb3b642_33c92b76 PS21, Line 131:
Check if curr_ddr_info is NULL.
Ack
https://review.coreboot.org/c/coreboot/+/61334/comment/dc923fe0_db894d3a PS21, Line 132: curr_dram_info
*mc
Done
https://review.coreboot.org/c/coreboot/+/61334/comment/e2fc918b_73d76060 PS21, Line 134: ASSERT
assert(mc);
Done
https://review.coreboot.org/c/coreboot/+/61334/comment/ad6a976b_9917a98a PS21, Line 217: ddr_info
The reason to rename to ddr_info is that when the first patch, the name of mem_chip_info is "dram_in […]
Ack
File src/soc/mediatek/common/memory.c:
https://review.coreboot.org/c/coreboot/+/61334/comment/ac1fafc1_4af60604 PS22, Line 104: size_t mtk_dram_size(void)
Need to declare this in emi.h.
Done
https://review.coreboot.org/c/coreboot/+/61334/comment/39dcbc00_60c73625 PS22, Line 107:
return 0 if !curr_ddr_info
Done
https://review.coreboot.org/c/coreboot/+/61334/comment/7ecf4a89_cb705198 PS22, Line 123: p = (void *)((uintptr_t)mc + sizeof(*mc));
If &mc->channel[0] does not work, what about "p = mc->channel" ?
No it doesn't work either.
File src/soc/mediatek/common/memory.c:
https://review.coreboot.org/c/coreboot/+/61334/comment/c8f2a4be_5974ece2 PS23, Line 28: const struct ddr_base_info *curr_ddr_info; static
https://review.coreboot.org/c/coreboot/+/61334/comment/94894ff0_eadf38b5 PS23, Line 117: uint64_t size_t
https://review.coreboot.org/c/coreboot/+/61334/comment/3168237f_52240ebc PS23, Line 123: assert(size);