Hung-Te Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41949 )
Change subject: soc/mediatek/mt8183: Support 6GB, 8GB DDR bootup ......................................................................
Patch Set 7:
(4 comments)
https://review.coreboot.org/c/coreboot/+/41949/6/src/soc/mediatek/mt8183/dra... File src/soc/mediatek/mt8183/dramc_pi_calibration_api.c:
https://review.coreboot.org/c/coreboot/+/41949/6/src/soc/mediatek/mt8183/dra... PS6, Line 2694: get_dram_infor_after_cal ok... you changed that to a pointer. not the type I preferred but fine if you still want to do that.
Anyway, can you rename this to
void dramc_get_calibrated_info(u8 *density_result)
https://review.coreboot.org/c/coreboot/+/41949/6/src/soc/mediatek/mt8183/emi... File src/soc/mediatek/mt8183/emi.c:
https://review.coreboot.org/c/coreboot/+/41949/6/src/soc/mediatek/mt8183/emi... PS6, Line 591: K finished (current clock: %u K finished (current clock: %u, density: %u)\n"
https://review.coreboot.org/c/coreboot/+/41949/4/src/soc/mediatek/mt8183/inc... File src/soc/mediatek/mt8183/include/soc/dramc_param.h:
https://review.coreboot.org/c/coreboot/+/41949/4/src/soc/mediatek/mt8183/inc... PS4, Line 67: ddr_geometry oops, I just realized we're duplicating ddr geometry here.
@huayang, it's really sorry but I wonder if we can revert the changes to a easier way, e.g.:
1. Don't put ddr_geometry in header. Keep dramc_param.h unchanged. 2. Add ddr_geometry in sdram_param (and leave it in emi.h)
So we have minimal changes in Coreboot side.
On the contrary, you can have some logic in dramk-k blob to read and trust the ddr geometry first from sdram_config.
https://review.coreboot.org/c/coreboot/+/41949/6/src/soc/mediatek/mt8183/inc... File src/soc/mediatek/mt8183/include/soc/emi.h:
https://review.coreboot.org/c/coreboot/+/41949/6/src/soc/mediatek/mt8183/inc... PS6, Line 11: struct dramc_param; : struct dramc_param_ops; No longer needed given you've included dramc_param.h