23 comments:
Where is the interface documented? Please add a datasheet name and revision.
File src/soc/mediatek/mt8192/pmif_spi.c:
Print out as a debug message, how long it took?
In that pattern documented somewhere?
Patch Set #7, Line 217: __func__, rdata, DEFAULT_VALUE_READ_TEST);
Please use more elaborate error messages.
/* 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?
Patch Set #7, Line 254: Setup
Set up
Patch Set #7, Line 257: Setup
Set up
Patch Set #7, Line 268: int ret = 0;
Use CB_SUCCESS and friends?
Patch Set #7, Line 287: if (ret != 0) {
Use CB_SUCCESS and friends.
Patch Set #7, Line 288: printk(BIOS_ERR, "[%s] data calibration fail,ret=%d\n",
Please elaborate and add a space after the comma.
Patch Set #7, Line 298: Checking
checking
1. Signature Checking using CRC (CRC 0 only)
* 2. EINT update
* 3. Read back Auxadc thermal data for GPS
1. Check signature using CRC (CRC 0 only)
2. Update EINT
3. …
Patch Set #7, Line 300: Auxadc
AUXADC
Patch Set #7, Line 302: initstaupd
`init_staupd()` for consistency with `init_sistrobe()`.
Patch Set #7, Line 321: udelay(100);
How is that done by a delay?
File src/soc/mediatek/mt8192/pmif_spmi.c:
Should { be on the next line?
Patch Set #7, Line 102: u32 i = 0;
The native type should be enough for a counting variable.
Patch Set #7, Line 131: printk(BIOS_ERR, "%s: Null argument", __func__);
Error messages should be more elaborate and helpful.
Patch Set #7, Line 162: u32 swinf_no)
Fits into one line.
Patch Set #7, Line 200: write32(&arb->mtk_pmif->inf_cmd_per_1, cmd_per);
Why does a different value have to be programmed here?
Please use one of the recommended coding styles [1].
[1]: https://doc.coreboot.org/coding_style.html#commenting
Patch Set #7, Line 224: 0x2F7
Add enums or macros?
Patch Set #7, Line 251: return -E_NODEV;
Add a debug message?
To view, visit change 45398. To unsubscribe, or for help writing mail filters, visit settings.