Attention is currently required from: Shelley Chen, Ravi kumar, Paul Menzel, mturney mturney, Julius Werner. Ravi Kumar Bokka has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/59195 )
Change subject: soc/qualcomm/common: Add dram information to CBMEM table ......................................................................
Patch Set 18:
(11 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/59195/comment/c3bb36e2_8fa9658a PS1, Line 7: Added
Add
Done
Commit Message:
https://review.coreboot.org/c/coreboot/+/59195/comment/e2ddcb70_92ef9ddb PS6, Line 7: soc
soc/qualcomm/common
Done
Commit Message:
https://review.coreboot.org/c/coreboot/+/59195/comment/270ce996_a3edc0a0 PS8, Line 7: Added
Please use imperative mood (present): Add.
Done
https://review.coreboot.org/c/coreboot/+/59195/comment/4ba87986_f748f961 PS8, Line 7: cbmem
CBMEM table(?)
Done
https://review.coreboot.org/c/coreboot/+/59195/comment/c29832b6_c5036877 PS8, Line 7: soc
Which one? […]
Done
https://review.coreboot.org/c/coreboot/+/59195/comment/33d4f817_ac07e9ad PS8, Line 10: TEST=Validated on qualcomm sc7280 developement board
How exactly?
structure entries tested through proc interface(HLOS). Please find below logs.
localhost ~ # ls -lrt /proc/device-tree/firmware/memchipinfo/ total 0 -r--r--r--. 1 root root 4 Oct 20 16:55 revision_id -r--r--r--. 1 root root 12 Oct 20 16:55 name -r--r--r--. 1 root root 4 Oct 20 16:55 io_width -r--r--r--. 1 root root 4 Oct 20 16:55 density -r--r--r--. 1 root root 12 Oct 20 16:55 compatible -r--r--r--. 1 root root 16 Oct 20 16:55 reg -r--r--r--. 1 root root 4 Oct 20 16:55 manufacturer_id localhost ~ #
Commit Message:
https://review.coreboot.org/c/coreboot/+/59195/comment/7ffea020_853f600a PS16, Line 9: BUG=b:182963902
Please add Bug id 177917361, to BUG=
Done
File src/soc/qualcomm/common/include/soc/mmu_common.h:
https://review.coreboot.org/c/coreboot/+/59195/comment/1ea3ffe6_5bf45951 PS3, Line 13: static struct region * const ddr_region = (struct region *)_ddr_information;
We will removew once mem_chip_info is squared away, this is temporarily on hold.
Done
File src/soc/qualcomm/common/qclib.c:
https://review.coreboot.org/c/coreboot/+/59195/comment/f5ac0849_147af8fc PS5, Line 26: static void write_mem_chip_information(struct qclib_cb_if_table_entry *te);
static functions don't need a prototype.
Done
https://review.coreboot.org/c/coreboot/+/59195/comment/7ca18747_3d5a0f4e PS5, Line 37:
There should be some kind of check here to make sure the data was actually filled out (e.g. […]
Done
https://review.coreboot.org/c/coreboot/+/59195/comment/0f289d4a_513f7821 PS5, Line 40: ASSERT(mem_region_base != NULL);
nit: can just write […]
Done
-
Ravi Kumar Bokka (Code Review)