Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44205 )
Change subject: soc/intel/{cnl,icl,jsl,tgl}: Use Bus Master for setting up PWRMBASE ......................................................................
Patch Set 2:
(3 comments)
https://review.coreboot.org/c/coreboot/+/44205/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/44205/2//COMMIT_MSG@11 PS2, Line 11: along with PCI_COMMAND_MEMORY (BIT 1).
Is that required by some specification?
in general PCI resource enabling program says to disable bus master cycle before assigning a new BAR to avoid cycle is getting decoded if already BAR been implemented. Hence its safe to make MMIO and BUS master disable prior to BAR assignment and enable it back after the programming done
https://review.coreboot.org/c/coreboot/+/44205/2/src/soc/intel/cannonlake/bo... File src/soc/intel/cannonlake/bootblock/pch.c:
https://review.coreboot.org/c/coreboot/+/44205/2/src/soc/intel/cannonlake/bo... PS2, Line 75: (char *)
void *?
Ack
https://review.coreboot.org/c/coreboot/+/44205/2/src/soc/intel/tigerlake/boo... File src/soc/intel/tigerlake/bootblock/pch.c:
https://review.coreboot.org/c/coreboot/+/44205/2/src/soc/intel/tigerlake/boo... PS2, Line 50: soc_config_pwrmbase
Looks like this is a good candidate to move to common/block/pmc?
yes, we might need to handle using PMC_V1 and V2 as soc_config_pwrmbase programming logic is different between SKL-KBL/APL-GLK/CNL-ICL-TGL-ADL