Po Xu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46390 )
Change subject: soc/mediatek/mt8192: Add auxadc driver ......................................................................
Patch Set 14:
(9 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.
Done
https://review.coreboot.org/c/coreboot/+/46390/4//COMMIT_MSG@10 PS4, Line 10:
Please merge your answer into the commit description.
Done
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)));
Is the 300ms documented somewhere in data sheet? If yes you can simply add a comment for it.
The auxadc datasheet will be updated in the next version. thanks.
https://review.coreboot.org/c/coreboot/+/46390/12/src/soc/mediatek/mt8192/au... File src/soc/mediatek/mt8192/auxadc.c:
PS12:
Remove execute bit
Done
https://review.coreboot.org/c/coreboot/+/46390/12/src/soc/mediatek/mt8192/in... File src/soc/mediatek/mt8192/include/soc/addressmap.h:
PS12:
Remove execute bit
Done
https://review.coreboot.org/c/coreboot/+/46390/12/src/soc/mediatek/mt8192/in... File src/soc/mediatek/mt8192/include/soc/auxadc.h:
PS12:
Remove execute bit
Done
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: int
Voltage is non-negative: `unsigned int`?
fixed it.
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()`.
fixed it.
https://review.coreboot.org/c/coreboot/+/46390/12/src/soc/mediatek/mt8192/in... File src/soc/mediatek/mt8192/include/soc/efuse.h:
PS12:
Remove execute bit
Done