Xi Chen has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44712 )
Change subject: soc/mediatek/mt8192: Get DDR base information after calibration ......................................................................
Patch Set 45:
(8 comments)
https://review.coreboot.org/c/coreboot/+/44712/41/src/soc/mediatek/mt8192/dr... File src/soc/mediatek/mt8192/dramc_pi_basic_api.c:
https://review.coreboot.org/c/coreboot/+/44712/41/src/soc/mediatek/mt8192/dr... PS41, Line 3844: while
Can we use wait_us?
i use wait_ms, timeout: 10 seconds.
https://review.coreboot.org/c/coreboot/+/44712/41/src/soc/mediatek/mt8192/dr... PS41, Line 3849: =
One space after "="
Ack
https://review.coreboot.org/c/coreboot/+/44712/41/src/soc/mediatek/mt8192/dr... PS41, Line 3856: = 0
No need for initialization.
Ack
https://review.coreboot.org/c/coreboot/+/44712/41/src/soc/mediatek/mt8192/dr... File src/soc/mediatek/mt8192/dramc_pi_main.c:
https://review.coreboot.org/c/coreboot/+/44712/41/src/soc/mediatek/mt8192/dr... PS41, Line 17: static void get_dram_info_after_cal(struct ddr_cali *cali)
If cali->density is the only thing modified in this function, why not return max_density? […]
Actually, get dram info contains vendor id, density and so on. As you noticed, here, we only save the density for use, maybe other information will be saved for future use. so we want to keep "get_dram_info*" function name, is it ok?
https://review.coreboot.org/c/coreboot/+/44712/41/src/soc/mediatek/mt8192/dr... PS41, Line 20: G
g
Ack
https://review.coreboot.org/c/coreboot/+/44712/41/src/soc/mediatek/mt8192/dr... PS41, Line 52: default
If this indicates an error, please print an error message.
Ack
https://review.coreboot.org/c/coreboot/+/44712/41/src/soc/mediatek/mt8192/dr... PS41, Line 60: d
u
Ack
https://review.coreboot.org/c/coreboot/+/44712/41/src/soc/mediatek/mt8192/dr... PS41, Line 134: only need do once for
Only need to do once to
Ack