Attention is currently required from: Ravi kumar, Shelley Chen, mturney mturney, Arthur Heymans. Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/59194 )
Change subject: src/lib: Add CBMEM tag id to parse ddr information ......................................................................
Patch Set 5:
(3 comments)
File src/commonlib/include/commonlib/cbmem_id.h:
https://review.coreboot.org/c/coreboot/+/59194/comment/29c86f47_c6a0023b PS5, Line 82: 0x10
maybe write it down in 0x%08x format like the rest? Also a more 'random looking' number might be bet […]
They're not actually random, they're meant to be readable in a hexdump. So this one could be something like 0x5048434D ('MCHP').
https://review.coreboot.org/c/coreboot/+/59194/comment/6b047f72_6d7bde73 PS5, Line 152: no extra space
File src/commonlib/include/commonlib/coreboot_tables.h:
https://review.coreboot.org/c/coreboot/+/59194/comment/10ddee1e_13bd202f PS5, Line 88: /* The following options are CMOS-related */ Your entry is not CMOS-related so it should be added above here (as 0x0043).
*However*, I think I'd actually prefer you don't add this to the coreboot table at all. This practice of having a separate coreboot table entry for each CBMEM area is sort of an anti-pattern that we should try to get away from if possible. If you can wait submitting this until CB:59491 lands, you can use the function introduced there to look it up through the lb_cbmem_entry tag.