Attention is currently required from: Ravi kumar, Shelley Chen, mturney mturney, Julius Werner. Ravi Kumar Bokka has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/59195 )
Change subject: soc: Added dram information to cbmem ......................................................................
Patch Set 4:
(5 comments)
File src/soc/qualcomm/common/include/soc/mmu_common.h:
https://review.coreboot.org/c/coreboot/+/59195/comment/05fe712a_9b3ad00c PS3, Line 13: static struct region * const ddr_region = (struct region *)_ddr_information;
We don't need ddr_region anymore because mem_chip_info contains the same information (and more). […]
We will removew once mem_chip_info is squared away, this is temporarily on hold.
https://review.coreboot.org/c/coreboot/+/59195/comment/5c5b05cd_dbdaee87 PS3, Line 14: struct region
This isn't a struct region, this is the mem_chip_info struct (which you should define somewhere in s […]
Done
File src/soc/qualcomm/common/include/soc/qclib_common.h:
https://review.coreboot.org/c/coreboot/+/59195/comment/8d33eb8d_f92f13c7 PS3, Line 25: /* memchip info */
This comment adds absolutely nothing useful to this code.
Done
File src/soc/qualcomm/common/qclib.c:
https://review.coreboot.org/c/coreboot/+/59195/comment/d5900e9d_f56fbdf6 PS3, Line 27: #if TEST_CODE /* Added this structure for testing purpose, will be remove once changes are reviewd */
Please clean this stuff up so we can properly read the file.
Done
File src/soc/qualcomm/sc7280/memlayout.ld:
https://review.coreboot.org/c/coreboot/+/59195/comment/b6b2a27c_94017a16 PS3, Line 51: REGION(mem_chip_info, 0x1494E000, 1K, 1K) /* memchip info */
I don't really think we need to put this in a separate SRAM region anymore. […]
Done
-
Ravi Kumar Bokka (Code Review)