Yu-Ping Wu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41949 )
Change subject: soc/mediatek/mt8183: Add ddr geometry to support 6GB, 8GB DDR bootup ......................................................................
Patch Set 14:
(3 comments)
https://review.coreboot.org/c/coreboot/+/41949/14/src/soc/mediatek/mt8183/in... File src/soc/mediatek/mt8183/include/soc/dramc_param.h:
https://review.coreboot.org/c/coreboot/+/41949/14/src/soc/mediatek/mt8183/in... PS14, Line 6: #include <stdint.h> : #include <sys/types.h> : #include <soc/emi.h> : #include <soc/dramc_common_mt8183.h> Could we sort these lexicographically?
https://review.coreboot.org/c/coreboot/+/41949/14/src/soc/mediatek/mt8183/in... PS14, Line 11: enum DRAMC_PARAM_SOURCE { : DRAMC_PARAM_SOURCE_SDRAM_INVALID = 0, : DRAMC_PARAM_SOURCE_SDRAM_CONFIG, : DRAMC_PARAM_SOURCE_FLASH, : }; Why move this here? Since this file already includes emi.h, could we keep this in emi.h?
https://review.coreboot.org/c/coreboot/+/41949/12/src/soc/mediatek/mt8183/in... File src/soc/mediatek/mt8183/include/soc/emi.h:
https://review.coreboot.org/c/coreboot/+/41949/12/src/soc/mediatek/mt8183/in... PS12, Line 9: struct sdram_params {
to less changes for MT8183 soc.
Ack