Attention is currently required from: Hung-Te Lin, Xi Chen, Xixi Chen. Yu-Ping Wu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/61293 )
Change subject: TEST-ONLY: Refactor dramc_param to common header ......................................................................
Patch Set 4:
(6 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/61293/comment/42652f31_83e56673 PS4, Line 7: Refactor dramc_param Move ddr_base_info
(I assume this is for ddr_base_info, right?)
https://review.coreboot.org/c/coreboot/+/61293/comment/5c99897e_216040f1 PS4, Line 9: The file dramc_param.h on different platform(8192/8195/8186) : share many the same structures. Also, adding a new member on : one platform may cause the other build error and runtime : error because they share the common memory.c. To avoiding : this bad condition, reuse dramc_param as much as possible. The ddr_base_info struct, which stores basic DDR information, should be platform independent. Currently the struct is defined in each SoC's dramc_parah.h. To decrease duplicate code, move it as well as other related structs and enums to a common header.
Patchset:
PS4: I think we can merge this even without the CB:61334. So I'll start reviewing, and please rebase! Thanks.
File src/soc/mediatek/common/include/soc/dramc_param_common.h:
https://review.coreboot.org/c/coreboot/+/61293/comment/4e3ef93e_d3854684 PS4, Line 1: /* SPDX-License-Identifier: GPL-2.0-only */
DOS line endings
Please fix.
File src/soc/mediatek/mt8186/include/soc/dramc_param.h:
https://review.coreboot.org/c/coreboot/+/61293/comment/e51b6d08_c940c812 PS4, Line 14: #include <soc/dramc_param_common.h> This doesn't need to be changed. The original order is correct.
File src/soc/mediatek/mt8192/include/soc/dramc_param.h:
https://review.coreboot.org/c/coreboot/+/61293/comment/fa0d8af7_3d51e45a PS4, Line 125: struct dramc_data { : struct ddr_base_info ddr_info; : struct sdram_params freq_params[DRAM_DFS_SHU_MAX]; : }; : : struct dramc_param { : struct dramc_param_header header; : void (*do_putc)(unsigned char c); : struct dramc_data dramc_datas; : }; : Did you remove this intentionally?