Xi Chen has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44710 )
Change subject: soc/mediatek/mt8192: Do dramc duty calibration ......................................................................
Patch Set 48:
(11 comments)
https://review.coreboot.org/c/coreboot/+/44710/44/src/soc/mediatek/mt8192/dr... File src/soc/mediatek/mt8192/dramc_pi_calibration_api.c:
https://review.coreboot.org/c/coreboot/+/44710/44/src/soc/mediatek/mt8192/dr... PS44, Line 573: delay_tmp
We could directly assign values to *delay.
Ack
https://review.coreboot.org/c/coreboot/+/44710/44/src/soc/mediatek/mt8192/dr... PS44, Line 587: delay_tmp
Just "delay"
Ack
https://review.coreboot.org/c/coreboot/+/44710/44/src/soc/mediatek/mt8192/dr... PS44, Line 589: duty_delay_reg_convert
Don't we need to do the same for duty_delay[1] (because RANK_MAX is 2)?
RANK0 and RANK1 share the same duty delay, use RANK0 is ok.
https://review.coreboot.org/c/coreboot/+/44710/44/src/soc/mediatek/mt8192/dr... PS44, Line 597: delay_tmp
No need to use an array. […]
Ack
https://review.coreboot.org/c/coreboot/+/44710/44/src/soc/mediatek/mt8192/dr... PS44, Line 599: DQS_NUMBER
Should we use DQS_NUMBER_LP4 to match the declaration in dramc_param.h? I know they're the same.
DQS_NUMBER is for common, dram_param DQS_NUMBER_LP4 is for LP4, keep DQS_NUMBER here.
https://review.coreboot.org/c/coreboot/+/44710/44/src/soc/mediatek/mt8192/dr... PS44, Line 600: (
No need for parentheses.
Ack
https://review.coreboot.org/c/coreboot/+/44710/44/src/soc/mediatek/mt8192/dr... PS44, Line 609: delay_tmp
Same
Ack
https://review.coreboot.org/c/coreboot/+/44710/44/src/soc/mediatek/mt8192/dr... PS44, Line 612: (
Same
Ack
https://review.coreboot.org/c/coreboot/+/44710/44/src/soc/mediatek/mt8192/dr... PS44, Line 618: static void dramc_duty_set_dqdqm_delay_cell(u8 chn, : const s8 *duty_delay, u8 k_type)
Write […]
Ack
https://review.coreboot.org/c/coreboot/+/44710/44/src/soc/mediatek/mt8192/dr... PS44, Line 622: delay_tmp
Same
Ack
https://review.coreboot.org/c/coreboot/+/44710/44/src/soc/mediatek/mt8192/dr... PS44, Line 625: (
Same
Ack