Attention is currently required from: Subrata Banik, Matt DeVillier, Tim Wawrzynczak, Sridhar Siricilla, Lean Sheng Tan, Patrick Rudolph, EricR Lai. Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/61650 )
Change subject: soc/intel/skylake: Add function to clear PMCON status bits ......................................................................
Patch Set 3: -Code-Review
(2 comments)
File src/soc/intel/skylake/pmc.c:
https://review.coreboot.org/c/coreboot/+/61650/comment/8f4ab33d_aefb0784 PS3, Line 99: pci_or_config32(dev, GEN_PMCON_A, 0);
well... […]
Sorry, not sure what you mean. pmc_soc_init runs way before finalize, so this one here clears bit 18 and it will be clear already in finalize. Correct me if I'm wrong, please
File src/soc/intel/skylake/pmutil.c:
https://review.coreboot.org/c/coreboot/+/61650/comment/78c770ff_6e676718 PS3, Line 269: void pmc_clear_pmcon_sts(void)
this function is all the same for all platforms using common code, so why exactly do we have to copy-pasta it once again instead of finally moving it to soc/intel/common?
There are few SOCs which eventually uses the IA PMC common code but the SOC EDS does have exact register details. I've tried creating the common code which eventually need to resolve the register definitions from SoC layer as GEN_PMCON_A offset is not same across all SoC. But due to documentation limitation issue, I just avoided pursuing that path.
Well, when the GEN_PMCON_A offset differs, it can be defined in the SoC headers, but the code can still be common code. Same applies to the bit MS4V, which is the only bit offset needed to be known, right?
btw. this probably should be done in soc_pmc_init instead of finalize.
The reason this is being done as part of finalize is to ensure that prior booting to OS, those bits are cleared.
Note: FSP can even request a global reset during FSP notify (which is post soc_pmc_init) hence, after global reset, this corresponding bit would have set. And, we might need to clear it prior to boot.
Got it, thanks! Add a short comment for this, please.
There is no harm if you would like to call this more than once.
Please don't 😄