Yu-Ping Wu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44710 )
Change subject: soc/mediatek/mt8192: Do dramc duty calibration ......................................................................
Patch Set 44:
(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.
https://review.coreboot.org/c/coreboot/+/44710/44/src/soc/mediatek/mt8192/dr... PS44, Line 587: delay_tmp Just "delay"
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)?
https://review.coreboot.org/c/coreboot/+/44710/44/src/soc/mediatek/mt8192/dr... PS44, Line 597: delay_tmp No need to use an array.
u8 delay.
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.
https://review.coreboot.org/c/coreboot/+/44710/44/src/soc/mediatek/mt8192/dr... PS44, Line 600: ( No need for parentheses.
https://review.coreboot.org/c/coreboot/+/44710/44/src/soc/mediatek/mt8192/dr... PS44, Line 609: delay_tmp Same
https://review.coreboot.org/c/coreboot/+/44710/44/src/soc/mediatek/mt8192/dr... PS44, Line 612: ( Same
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
static void dramc_duty_set_dqdqm_delay_cell(u8 chn, const s8 *duty_delay, u8 k_type)
or
static void dramc_duty_set_dqdqm_delay_cell( u8 chn, const s8 *duty_delay, u8 k_type)
https://review.coreboot.org/c/coreboot/+/44710/44/src/soc/mediatek/mt8192/dr... PS44, Line 622: delay_tmp Same
https://review.coreboot.org/c/coreboot/+/44710/44/src/soc/mediatek/mt8192/dr... PS44, Line 625: ( Same