Hung-Te Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32466 )
Change subject: google/kukui: Raising the CPU frequency ......................................................................
Patch Set 3:
(2 comments)
https://review.coreboot.org/#/c/32466/3/src/mainboard/google/kukui/romstage.... File src/mainboard/google/kukui/romstage.c:
https://review.coreboot.org/#/c/32466/3/src/mainboard/google/kukui/romstage.... PS3, Line 32: mt_pll_raise_ca53_freq I think it makes more send to me to add the comment here.
mt6358_init(); /* * Raising frequency need higher voltage, which is set inside mt6358_init * and need 200us to be stable before setting new frequency. * If we want to move mt_pll_raise_ca53_freq to other places, be sure * to measure again the duration between them. */ mt_pll_rais_ca53_freq(1989 * MHz);
https://review.coreboot.org/#/c/32466/3/src/soc/mediatek/mt8183/mt6358.c File src/soc/mediatek/mt8183/mt6358.c:
https://review.coreboot.org/#/c/32466/3/src/soc/mediatek/mt8183/mt6358.c@783 PS3, Line 783: /* : * Voltage will take 200us to be stable if we change it. : * Do not return from this function befere voltage is stable. : */ I think it's weird to have the comment here since it's not trivial to figure out which function will be changing voltage, and how/when we can ensure 200us is met.