Brandon Breitenstein has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37871 )
Change subject: soc/intel/common/block: Enable PMC IPC driver ......................................................................
Patch Set 11:
(7 comments)
https://review.coreboot.org/c/coreboot/+/37871/10//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/37871/10//COMMIT_MSG@10 PS10, Line 10: PMC IPC driver is needed to communicate with EC to get the PD : status
nit: PMC IPC driver should only be a provider of API such that any component that wants to talk to t […]
Done
https://review.coreboot.org/c/coreboot/+/37871/10//COMMIT_MSG@12 PS10, Line 12:
Can you please add appropriate partner BUG=?
Done
https://review.coreboot.org/c/coreboot/+/37871/10/src/soc/intel/common/block... File src/soc/intel/common/block/pmc/pmclib.c:
https://review.coreboot.org/c/coreboot/+/37871/10/src/soc/intel/common/block... PS10, Line 610: if (ipcsts & PMC_IPC_STS_BUSY) : return CB_SUCCESS;
I think somewhere is the moving this patch around a few times this got messed up. […]
Done
https://review.coreboot.org/c/coreboot/+/37871/10/src/soc/intel/common/block... PS10, Line 614:
There is no delay required but I will add a 1us delay here to avoid the tight loop hammering the reg […]
Done
https://review.coreboot.org/c/coreboot/+/37871/10/src/soc/intel/common/block... PS10, Line 618: PMC_IPC_XFER_TIMEOUT_MS
nit: This can go on the previous line.
Done
https://review.coreboot.org/c/coreboot/+/37871/10/src/soc/intel/common/block... PS10, Line 620: I2C
I2C? Why?
Done
https://review.coreboot.org/c/coreboot/+/37871/10/src/soc/intel/common/block... PS10, Line 645: PMC_IPC_CMD_SIZE
If these are offsets, those should be named accordingly.
Done
-
Brandon Breitenstein (Code Review)