Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46390 )
Change subject: soc/mediatek/mt8192: Add auxadc driver ......................................................................
Patch Set 4:
(4 comments)
https://review.coreboot.org/c/coreboot/+/46390/4//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/46390/4//COMMIT_MSG@9 PS4, Line 9: Add MT8192 auxadc driver. Please describe the implementation. A possible delay of 3 * 300 ms should be explained.
https://review.coreboot.org/c/coreboot/+/46390/4/src/soc/mediatek/mt8192/aux... File src/soc/mediatek/mt8192/auxadc.c:
https://review.coreboot.org/c/coreboot/+/46390/4/src/soc/mediatek/mt8192/aux... PS4, Line 41: setbits32(&mt8192_infracfg->module_sw_cg_1_clr, 1 << 10); : assert(wait_ms(300, !(read32(&mtk_auxadc->con2) & 0x1))); : : clrbits32(&mtk_auxadc->con1, 1 << channel); : assert(wait_ms(300, !(read32(&mtk_auxadc->data[channel]) & (1 << 12)))); : : setbits32(&mtk_auxadc->con1, 1 << channel); : udelay(25); : assert(wait_ms(300, read32(&mtk_auxadc->data[channel]) & (1 << 12))); 300 ms time-out sound too much for coreboot. Please print warning messages if it took longer than 10 ms.
https://review.coreboot.org/c/coreboot/+/46390/4/src/soc/mediatek/mt8192/inc... File src/soc/mediatek/mt8192/include/soc/auxadc.h:
https://review.coreboot.org/c/coreboot/+/46390/4/src/soc/mediatek/mt8192/inc... PS4, Line 20: auxadc_get_voltage Please add the unit to the function name: `auxadc_get_voltage_uv()`.
https://review.coreboot.org/c/coreboot/+/46390/4/src/soc/mediatek/mt8192/inc... PS4, Line 20: int Voltage is non-negative: `unsigned int`?