Hung-Te Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44568 )
Change subject: mb/google/asurada: Init dram in romstage ......................................................................
Patch Set 11:
(1 comment)
https://review.coreboot.org/c/coreboot/+/44568/11/src/mainboard/google/asura... File src/mainboard/google/asurada/romstage.c:
https://review.coreboot.org/c/coreboot/+/44568/11/src/mainboard/google/asura... PS11, Line 17: static bool read_calibration_data_from_flash(struct dramc_param *dparam) : { : const size_t length = sizeof(*dparam); : size_t ret = fmap_read_area(CALIBRATION_REGION, dparam, length); : printk(BIOS_DEBUG, "read data from flash, ret=%#zx, length=%#zx\n", ret, length); : : return ret == length; : } : : static bool write_calibration_data_to_flash(const struct dramc_param *dparam) : { : const size_t length = sizeof(*dparam); : size_t ret = fmap_overwrite_area(CALIBRATION_REGION, dparam, length); : printk(BIOS_DEBUG, "write data from flash, ret=%#zx, length=%#zx\n", ret, length); : : return ret == length; : }
Why can’t these be SoC functions?
The SoC may have different approaches for storing calibration data, especially there is no guarantee the region name will be the same across different boards (in fact, we do plan to change that soon).
I think it should be fine to keep current version until we have finished the migration to unified mrc_cache and then it can live in common driver, or as SoC functions.