Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48549 )
Change subject: mb/purism/librem_cnl: Use FMAP-based SPD cache ......................................................................
Patch Set 2:
(3 comments)
https://review.coreboot.org/c/coreboot/+/48549/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/48549/2//COMMIT_MSG@13 PS2, Line 13: implemention implement*at*ion
https://review.coreboot.org/c/coreboot/+/48549/2/src/mainboard/purism/librem... File src/mainboard/purism/librem_cnl/romstage.c:
https://review.coreboot.org/c/coreboot/+/48549/2/src/mainboard/purism/librem... PS2, Line 68: true nit: for clarity, maybe set this to `dimm_changed`. The value is only used when `dimm_changed` is true.
Unifying both variables can't be done, since it would break the case where `load_spd_cache` fails
https://review.coreboot.org/c/coreboot/+/48549/2/src/mainboard/purism/librem... PS2, Line 104: cannonlake_memcfg_init(mem_cfg, &memcfg); Wouldn't it make more sense to move the spd cache code into `cannonlake_memcfg_init`?