Attention is currently required from: Ravi kumar, Shelley Chen, mturney mturney. Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/59195 )
Change subject: soc: Add dram information to cbmem ......................................................................
Patch Set 6:
(1 comment)
File src/soc/qualcomm/common/qclib.c:
https://review.coreboot.org/c/coreboot/+/59195/comment/1dd8f696_7d1a3438 PS5, Line 174: mem_chip_info
Since CB isn't passing data into QcLib, would it be better to pass NULL (address) and 0 (size) as th […]
Okay, so you're ignoring the pointer coreboot passes in the table, and having QcLib overwrite it with a pointer to somewhere within its own .data section. That also works, although it wasn't what I was thinking. In that case, yes, please just pass NULL, 0 to clarify that coreboot isn't passing anything in here. There's no need for a global variable then.
It would be good to have some clear documentation on how all of this works, because there are a lot of implicit agreements attached to each interface table entry name now. Please add a comment here to explain that QcLib will write back a pointer to an internal buffer with the data, and also update that qclib-interface.md document with clear explanations on how each interface table entry is supposed to work.