Attention is currently required from: Hung-Te Lin, Jarried Lin, Yidi Lin, Yu-Ping Wu.
Hope Wang has posted comments on this change by Jarried Lin. ( https://review.coreboot.org/c/coreboot/+/85127?usp=email )
Change subject: soc/mediatek/mt8196: Add PMIC MT6363 driver ......................................................................
Patch Set 13:
(13 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/85127/comment/9d9ad395_d0f1908a?usp... : PS7, Line 9: SOC
SoC
Done
https://review.coreboot.org/c/coreboot/+/85127/comment/4a82dcf1_462761da?usp... : PS7, Line 12: MT6363_sdmadc
This isn't added in this patch.
Done
File src/soc/mediatek/common/include/soc/mt6363.h:
https://review.coreboot.org/c/coreboot/+/85127/comment/c6b748ec_06b72ee0?usp... : PS7, Line 11: a
Let's use all upper cases for consistency.
Done
https://review.coreboot.org/c/coreboot/+/85127/comment/c78e403c_4eca0165?usp... : PS7, Line 28: 0x24e
Should this be sorted?
Done
https://review.coreboot.org/c/coreboot/+/85127/comment/0824904e_a843d2ef?usp... : PS7, Line 115: #endif /* __SOC_MEDIATEK_MT6363_H__ */
One blank line above.
Done
File src/soc/mediatek/common/mt6363.c:
https://review.coreboot.org/c/coreboot/+/85127/comment/6c65a4f8_f6245599?usp... : PS7, Line 11: MT6363_UDELAY_200US
MT6363_SET_DELAY_US […]
Done
https://review.coreboot.org/c/coreboot/+/85127/comment/a702d331_3a23e2e9?usp... : PS7, Line 141: )
remove
Done
https://review.coreboot.org/c/coreboot/+/85127/comment/e177dac0_16a8afc8?usp... : PS7, Line 260: mt6363_sdmadc_init
This doesn't exist (yet). Remove it.
Done
File src/soc/mediatek/mt8196/mt6363.c:
https://review.coreboot.org/c/coreboot/+/85127/comment/18291d43_750c2dbc?usp... : PS7, Line 13: 0
If `shift` is always 0, we can remove the field from the `pmic_setting` struct.
Done
https://review.coreboot.org/c/coreboot/+/85127/comment/b5ce8284_1137bce8?usp... : PS7, Line 380: /* Suspend */
Is this a TODO comment? Can we remove this in this patch, if not needed?
Done
https://review.coreboot.org/c/coreboot/+/85127/comment/ce1c580c_2d4d2399?usp... : PS7, Line 390: /* We should disable unused modem power in a separate function and
Is this a TODO comment? If so, change it to `TODO: Disable unused ... […]
Done
https://review.coreboot.org/c/coreboot/+/85127/comment/1d5e2287_9dc9e060?usp... : PS7, Line 390: /*
Wrong format.
Done
https://review.coreboot.org/c/coreboot/+/85127/comment/3fa71362_7bdb12db?usp... : PS7, Line 396: pmic_lp_setting
Where is this called?
Done