JG Poxu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32800 )
Change subject: mt8183: adding efuse calibration in auxadc ......................................................................
Patch Set 9:
(9 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)
well there's some problem here. ~((uint32_t)1) = -2. […]
Done
https://review.coreboot.org/#/c/32800/5/src/soc/mediatek/mt8183/auxadc.c@35 PS5, Line 35: int
Cali_oe & cali_ge may be negative
Done
https://review.coreboot.org/#/c/32800/5/src/soc/mediatek/mt8183/auxadc.c@36 PS5, Line 36: int
Cali_oe & cali_ge may be negative
Done
https://review.coreboot.org/#/c/32800/5/src/soc/mediatek/mt8183/auxadc.c@40 PS5, Line 40: int
I will fix it
Done
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)
I will fix it
Done
https://review.coreboot.org/#/c/32800/5/src/soc/mediatek/mt8183/auxadc.c@49 PS5, Line 49: cali_ge_a - 512
yes
Done
https://review.coreboot.org/#/c/32800/5/src/soc/mediatek/mt8183/auxadc.c@74 PS5, Line 74: rawvalue
I will fix it
Done
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();
Let's not overcomplicate things. […]
Done
https://review.coreboot.org/#/c/32800/5/src/soc/mediatek/mt8183/auxadc.c@84 PS5, Line 84: int64_t
Hmmm. I think there are few things that can be improved. […]
1. No need,because it read the value in the data register directly, and value=sample data & 0xfff,fully guaranteed value range from 0 to 4096 2. not negative