Matt DeVillier has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47899 )
Change subject: mb/google/hatch: Drop use of SPD cache for puff-based variants ......................................................................
Patch Set 1:
(2 comments)
https://review.coreboot.org/c/coreboot/+/47899/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/47899/1//COMMIT_MSG@11 PS1, Line 11: it's actually slower than simply letting FSP (vs coreboot) : read the SPD data via smbus, so drop it.
SPD cache is used not just to speed up boot, but also to decide when memory should be retrained if a […]
but it doesn't - retraining works just fine either way
https://review.coreboot.org/c/coreboot/+/47899/1//COMMIT_MSG@19 PS1, Line 19: Test: build/boot WYVERN variant, check boot times via cbmem: : w/SPD caching: ~722 ms : w/FSP reading: ~627 ms
I don't understand why there is a difference from original implementation.
I spent some time adding RW_SPD_CACHE support to the default FMAP for use on another CML board, and saw worse boot times. Spun my tires for a bit, then decided to validate on puff by having FSP read the SPDs directly via smbus. ~100ms improvement. So I have no idea how this ever showed an improvement, unless early CML FSP had some horrible read times. But even reverting commit 74109923 and having coreboot read the SPDs via smbus shaved 30ms or so off the boot time. So the SPD cache was the worst performing of the 3 methods.