Hung-Te Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32800 )
Change subject: mt8183: add efuse calibration in auxadc ......................................................................
Patch Set 5:
(8 comments)
https://review.coreboot.org/#/c/32800/5/src/soc/mediatek/mt8183/auxadc.c File src/soc/mediatek/mt8183/auxadc.c:
https://review.coreboot.org/#/c/32800/5/src/soc/mediatek/mt8183/auxadc.c@33 PS5, Line 33: ~((uint32_t)1) (~(uint32_t)1)
I personally feel 0xffffffff also works
https://review.coreboot.org/#/c/32800/5/src/soc/mediatek/mt8183/auxadc.c@35 PS5, Line 35: int uint32_t, if the init itself was uint32_t type .
https://review.coreboot.org/#/c/32800/5/src/soc/mediatek/mt8183/auxadc.c@36 PS5, Line 36: int uint32_t
https://review.coreboot.org/#/c/32800/5/src/soc/mediatek/mt8183/auxadc.c@40 PS5, Line 40: int uint32_t (and the rest)
https://review.coreboot.org/#/c/32800/5/src/soc/mediatek/mt8183/auxadc.c@48 PS5, Line 48: ((cali_reg & ADC_GE_A_MASK) >> ADC_GE_A_SHIFT) remove one level of quote just like above:
cali_ge_a = (cali_reg & ADC_GE_A_MASK) >> ADC_GE_A_SHIFT;
https://review.coreboot.org/#/c/32800/5/src/soc/mediatek/mt8183/auxadc.c@74 PS5, Line 74: rawvalue I'd prefer calling this raw_value
https://review.coreboot.org/#/c/32800/5/src/soc/mediatek/mt8183/auxadc.c@77 PS5, Line 77: if (cali_oe == AUXADC_CALI_INIT || cali_ge == AUXADC_CALI_INIT) : mt_auxadc_update_cali(); will it make more sense if we just put this if-check inside mt_auxadc_update_cali?
So we simply call mt_auxadc_update_cali() here.
https://review.coreboot.org/#/c/32800/5/src/soc/mediatek/mt8183/auxadc.c@84 PS5, Line 84: int64_t uint64_t ?