Julius Werner 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 2:
(4 comments)
This implements nothing of the special recovery-mode behavior we discussed (both not using the cache at all in recovery mode, and erasing the cache when the retrain switch is hit). Can you please add those as well?
https://review.coreboot.org/c/coreboot/+/46111/1/src/soc/qualcomm/common/inc... File src/soc/qualcomm/common/include/soc/qclib_common.h:
https://review.coreboot.org/c/coreboot/+/46111/1/src/soc/qualcomm/common/inc... PS1, Line 14: #define QCLIB_FR_DDR_TRAINING_DATA "RW_MRC_CACHE" Shouldn't be needed at all anymore?
https://review.coreboot.org/c/coreboot/+/46111/1/src/soc/qualcomm/common/qcl... File src/soc/qualcomm/common/qclib.c:
https://review.coreboot.org/c/coreboot/+/46111/1/src/soc/qualcomm/common/qcl... PS1, Line 14: #include <mrc_cache.h> nit: please alphabetize above "soc"
https://review.coreboot.org/c/coreboot/+/46111/1/src/soc/qualcomm/common/qcl... PS1, Line 75: 0 Probably a good idea to define a macro for the version, even if we're probably not going to ever change it.
https://review.coreboot.org/c/coreboot/+/46111/1/src/soc/qualcomm/common/qcl... PS1, Line 132: mrc_cache_load_current(MRC_TRAINING_DATA, 0, _ddr_training, Still need to check return value, right?