Yu-Ping Wu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37996 )
Change subject: soc/mediatek/mt8183: do TX tracking for DRAM DVFS feature ......................................................................
Patch Set 9:
(12 comments)
https://review.coreboot.org/c/coreboot/+/37996/5//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/37996/5//COMMIT_MSG@10 PS5, Line 10:
updated at commit msg
Ack
https://review.coreboot.org/c/coreboot/+/37996/8/src/soc/mediatek/mt8183/dra... File src/soc/mediatek/mt8183/dramc_pi_calibration_api.c:
https://review.coreboot.org/c/coreboot/+/37996/8/src/soc/mediatek/mt8183/dra... PS8, Line 2380: u4value = READ32_BITFIELD(&ch[chn].ao.shu[0].rk[rnk].selph_dqsg0, SHURK0_SELPH_DQSG0_TX_DLY_DQS0_GATED);
I would try to split these lines
Done
https://review.coreboot.org/c/coreboot/+/37996/9/src/soc/mediatek/mt8183/dra... File src/soc/mediatek/mt8183/dramc_pi_calibration_api.c:
https://review.coreboot.org/c/coreboot/+/37996/9/src/soc/mediatek/mt8183/dra... PS9, Line 2142: return; Should we return -1 on error?
https://review.coreboot.org/c/coreboot/+/37996/9/src/soc/mediatek/mt8183/dra... PS9, Line 2150: Remove extra space
https://review.coreboot.org/c/coreboot/+/37996/9/src/soc/mediatek/mt8183/dra... PS9, Line 2189: tck = 1000000 / clock_rate; Are you sure we want to round down here, rather than round up or round to closest?
https://review.coreboot.org/c/coreboot/+/37996/9/src/soc/mediatek/mt8183/dra... PS9, Line 2194: (mr23 * tck * tck) Remove unnecessary parentheses. Same below.
https://review.coreboot.org/c/coreboot/+/37996/9/src/soc/mediatek/mt8183/dra... PS9, Line 2216: 0 Please use DRAM_DFS_SHUFFLE_1.
https://review.coreboot.org/c/coreboot/+/37996/9/src/soc/mediatek/mt8183/dra... PS9, Line 2221: * Add spaces around binary operators.
https://review.coreboot.org/c/coreboot/+/37996/9/src/soc/mediatek/mt8183/dra... PS9, Line 2314: static int div_round_close(const int n, const int d) There's already DIV_ROUND_CLOSEST in commonlib/include/commonlib/helpers.h, but you may want to check if it's exactly the same.
https://review.coreboot.org/c/coreboot/+/37996/9/src/soc/mediatek/mt8183/dra... PS9, Line 2330: shu_dst >= DRAM_DFS_SHUFFLE_MAX) { Please align with previous line.
https://review.coreboot.org/c/coreboot/+/37996/9/src/soc/mediatek/mt8183/dra... PS9, Line 2340: + Add spaces around binary operators.
https://review.coreboot.org/c/coreboot/+/37996/9/src/soc/mediatek/mt8183/dra... PS9, Line 2386: SHURK0_SELPH_DQSG0_TX_DLY_DQS0_GATED); Need one more tab. Same below.