Attention is currently required from: Nico Huber, Subrata Banik, Angel Pons, Michael Niewöhner, EricR Lai. Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48340 )
Change subject: soc/intel/alderlake: Refactor PCIE port config ......................................................................
Patch Set 56:
(1 comment)
File src/soc/intel/common/block/include/intelblocks/pcie_rp.h:
https://review.coreboot.org/c/coreboot/+/48340/comment/13516ea9_2e4ebaec PS47, Line 22: PCIE_RP_CLK_FREE = (1 << 3),
As CLKREQ 6 is used for both PCH LAN and RP8 hence we need to make it as free running clock rather assign to particular RP as other PCIe CLK does
Wait.. did you mean CLKSRC7 is used for both PCH LAN and RP8? I don't understand the intent behind sharing of CLKREQ for PCH LAN and RP8.
Also, what is the advantage of setting CLKREQ for the free running clock source? I think that is incorrect.
Eric - I think what we need to do here is: a. Remove the enum PCIE_RP_CLK_FREE since it is not really associated with the root port. b. Add another config field for pcie_clk_config[CONFIG_MAX_PCIE_CLOCKS]; and define a separate enum: ``` enum pcie_clk_src_flags { PCIE_CLOCK_FREE_RUNNING, } ```
With this, mainboard can provide all RP related configuration by using `pcie_rp_config` and any special clock related configuration using `pcie_clk_config`.
With this, `soc_memory_init_params()` will have to do something like:
``` for (i = 0; i < CONFIG_MAX_PCIE_CLOCKS; i++) { if (pcie_clk_config[i].flags & PCIE_CLOCK_FREE_RUNNING) m_cfg->PcieClkSrcUsage[i] = PCIE_CLK_FREE_RUNNING; else m_cfg->PcieClkSrcUsage[i] = PCIE_CLK_NOTUSED; m_cfg->PcieClkSrcClkReq[i] = PCIE_CLK_NOTUSED; } ```
and in adlrvp, it can then use: ``` # Enable PCH PCIE RP 8 using free running CLK (0x80) register "pch_pcie_rp[PCH_RP(8)]" = "{ /* Clock source is shared with LAN and hence marked as free running. */ .flags = PCIE_RP_CLK_SRC_UNUSED, }" register "pcie_clk_config[7].flags" = "PCIE_CLOCK_FREE_RUNNING" ```
Does that make sense?