Shelley Chen has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46111 )
Change subject: trogdor: Modify DDR training to use mrc_cache ......................................................................
Patch Set 8:
(6 comments)
This change is ready for review.
https://review.coreboot.org/c/coreboot/+/46111/6/src/mainboard/google/trogdo... File src/mainboard/google/trogdor/chromeos.fmd:
https://review.coreboot.org/c/coreboot/+/46111/6/src/mainboard/google/trogdo... PS6, Line 12: RO_VPD(PRESERVE) 228K
Sorry, I just submitted CB:45705 so you'll have to rebase onto that. […]
Done
https://review.coreboot.org/c/coreboot/+/46111/6/src/soc/qualcomm/common/qcl... File src/soc/qualcomm/common/qclib.c:
https://review.coreboot.org/c/coreboot/+/46111/6/src/soc/qualcomm/common/qcl... PS6, Line 78: * because we always want to retrain.
Well, the reason we're not writing it back is more that recovery uses RO firmware, and RW firmware m […]
Done
https://review.coreboot.org/c/coreboot/+/46111/6/src/soc/qualcomm/common/qcl... PS6, Line 142: (CONFIG(VBOOT) && get_recovery_mode_retrain_switch())) {
The retrain isn't really important here (retrain always implies recovery anyway), the important part […]
Ack. I've added the config and I'll ping Tom about how to force retraining from the EC.
https://review.coreboot.org/c/coreboot/+/46111/6/src/soc/qualcomm/common/qcl... PS6, Line 143: data_size = 0;
We should probably still pass the full size of the region to QcLib even if we're not intending to us […]
Done
https://review.coreboot.org/c/coreboot/+/46111/6/src/soc/qualcomm/common/qcl... PS6, Line 149: printk(BIOS_ERR, "Unable to load previous training data.\n");
I think this is exactly the case we would hit if we would run the CONFIG_MRC_CLEAR_NORMAL_CACHE_ON_R […]
Done
https://review.coreboot.org/c/coreboot/+/46111/6/src/soc/qualcomm/common/qcl... PS6, Line 152: data_size
As mentioned on the other patch, this would probably better be REGION_SIZE(ddr_training) after all.
Done