Michał Żygowski has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35889 )
Change subject: mainboard/pcengines/apu2/mainboard.c: fill SMBIOS type 16 and 17 ......................................................................
Patch Set 2:
(3 comments)
https://review.coreboot.org/c/coreboot/+/35889/2/src/mainboard/pcengines/apu... File src/mainboard/pcengines/apu2/mainboard.c:
https://review.coreboot.org/c/coreboot/+/35889/2/src/mainboard/pcengines/apu... PS2, Line 160: static void set_dimm_info(uint8_t *spd, struct dimm_info *dimm)
that's not board specific
Any ready functions that do the same?
https://review.coreboot.org/c/coreboot/+/35889/2/src/mainboard/pcengines/apu... PS2, Line 222: mem_info = cbmem_add(CBMEM_ID_MEMINFO, sizeof(*mem_info));
usually added in romstage by DRAM init code
Usually, yes. We do not do it, since hooking to AGESA memory data after RAM init is more complex and not worth it (AGESA destroys the structures right after the executed init stage). Here we avoid using CBMEM init romstage hooks and stashing the memory information in CAR to get the ID_MEMINFO populated. Since apu2 has soldered memory, parsing SPD is sufficient.
https://review.coreboot.org/c/coreboot/+/35889/2/src/mainboard/pcengines/apu... PS2, Line 345: t->maximum_capacity = 4 * 1024 * 1024; // 4GB (in kB) due to board design This should be based on SPD I think. Since the 2GB mainboard cannot have 4GB of memory.