hsin-hsiung wang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45398 )
Change subject: soc/mediatek/mt8192: add pmif driver ......................................................................
Patch Set 23:
(10 comments)
https://review.coreboot.org/c/coreboot/+/45398/7/src/soc/mediatek/mt8192/pmi... File src/soc/mediatek/mt8192/pmif_spi.c:
https://review.coreboot.org/c/coreboot/+/45398/7/src/soc/mediatek/mt8192/pmi... PS7, Line 234: /* Wait for completion of sending the commands */ : do { : rdata = read32(&arb->mtk_pmif->inf_busy_sta); : } while ((rdata & PMIF_SPI_AP) != 0x0); : : do { : rdata = read32(&arb->mtk_pmif->other_busy_sta_0); : } while (GET_CMDISSUE_BUSY(rdata) != 0x0); : : do { : rdata = read32(&mtk_pmicspi_mst->other_busy_sta_0); : } while (GET_PMICSPI_BUSY(rdata) != 0x0);
Could these be endless loops?
Done
https://review.coreboot.org/c/coreboot/+/45398/7/src/soc/mediatek/mt8192/pmi... PS7, Line 254: Setup
Set up
Done
https://review.coreboot.org/c/coreboot/+/45398/7/src/soc/mediatek/mt8192/pmi... PS7, Line 257: Setup
Set up
Done
https://review.coreboot.org/c/coreboot/+/45398/7/src/soc/mediatek/mt8192/pmi... PS7, Line 268: int ret = 0;
Use CB_SUCCESS and friends?
Done
https://review.coreboot.org/c/coreboot/+/45398/7/src/soc/mediatek/mt8192/pmi... PS7, Line 288: printk(BIOS_ERR, "[%s] data calibration fail,ret=%d\n",
Please elaborate and add a space after the comma.
Done
https://review.coreboot.org/c/coreboot/+/45398/7/src/soc/mediatek/mt8192/pmi... PS7, Line 298: Checking
checking
Done
https://review.coreboot.org/c/coreboot/+/45398/7/src/soc/mediatek/mt8192/pmi... PS7, Line 298: 1. Signature Checking using CRC (CRC 0 only) : * 2. EINT update : * 3. Read back Auxadc thermal data for GPS
- Check signature using CRC (CRC 0 only) […]
Done
https://review.coreboot.org/c/coreboot/+/45398/7/src/soc/mediatek/mt8192/pmi... PS7, Line 300: Auxadc
AUXADC
Done
https://review.coreboot.org/c/coreboot/+/45398/7/src/soc/mediatek/mt8192/pmi... PS7, Line 302: initstaupd
`init_staupd()` for consistency with `init_sistrobe()`.
Done
https://review.coreboot.org/c/coreboot/+/45398/7/src/soc/mediatek/mt8192/pmi... PS7, Line 321: udelay(100);
How is that done by a delay?
After checking, we don't use md, so there is no need for this.