Hung-Te Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36921 )
Change subject: soc/mediatek/mt8183: skip fast calibration for high frequency of TX RX window ......................................................................
Patch Set 6:
(11 comments)
the fixes addressing comments were moved to next patchset.
https://review.coreboot.org/c/coreboot/+/36921/3//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/36921/3//COMMIT_MSG@7 PS3, Line 7: soc/mediatek/mt8183: TX RX window should not do fast K for High frequency
Please make it a statement of the change (imperative mood). Maybe: […]
Ack
https://review.coreboot.org/c/coreboot/+/36921/3//COMMIT_MSG@9 PS3, Line 9: frquency
frequencies
Ack
https://review.coreboot.org/c/coreboot/+/36921/3//COMMIT_MSG@10 PS3, Line 10: High
high
Ack
https://review.coreboot.org/c/coreboot/+/36921/3//COMMIT_MSG@10 PS3, Line 10: High frequency
Like what values? […]
Ack
https://review.coreboot.org/c/coreboot/+/36921/3//COMMIT_MSG@11 PS3, Line 11:
Please quickly explain the implementation. Why do you lower some values for example.
Ack
https://review.coreboot.org/c/coreboot/+/36921/3//COMMIT_MSG@14 PS3, Line 14: TEST=Boots correctly on Kukui
Before it just hung or what?
Ack
https://review.coreboot.org/c/coreboot/+/36921/3//COMMIT_MSG@17 PS3, Line 17:
Your Signed-off-by line is missing.
Ack
https://review.coreboot.org/c/coreboot/+/36921/3/src/soc/mediatek/mt8183/dra... File src/soc/mediatek/mt8183/dramc_pi_calibration_api.c:
https://review.coreboot.org/c/coreboot/+/36921/3/src/soc/mediatek/mt8183/dra... PS3, Line 1591: freq_group
Please add a comment here: […]
Ack
https://review.coreboot.org/c/coreboot/+/36921/3/src/soc/mediatek/mt8183/dra... PS3, Line 1593: 796
the real CK DRAMC output is 796,1196,1596,1972, NOT 800,1200,1400,1866 […]
Ack
https://review.coreboot.org/c/coreboot/+/36921/3/src/soc/mediatek/mt8183/dra... PS3, Line 1648: clock_rate
this place need using clock rate do computer, so NOT need /2
Ack
https://review.coreboot.org/c/coreboot/+/36921/3/src/soc/mediatek/mt8183/dra... PS3, Line 1650:
trailing whitespace
Ack