Attention is currently required from: Furquan Shaikh, Angel Pons, EricR Lai. Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48340 )
Change subject: soc/intel/alderlake: Revise PCIE port config ......................................................................
Patch Set 34:
(5 comments)
File src/soc/intel/alderlake/romstage/fsp_params.c:
https://review.coreboot.org/c/coreboot/+/48340/comment/2cafc494_6651a771 PS34, Line 25: convert_ nit: convert_ seems unncessary, clk_src_to_fsp sounds good enough
https://review.coreboot.org/c/coreboot/+/48340/comment/11f6c6c2_359f69c4 PS34, Line 29: 0x80 PCIE_CLK_FREE
https://review.coreboot.org/c/coreboot/+/48340/comment/7544bd14_dae79f56 PS34, Line 44: CpuPcieRpEnableMask This still needs to be mapped to the 3 RPs
https://review.coreboot.org/c/coreboot/+/48340/comment/c22343f7_57535e41 PS34, Line 42: return m_cfg->PcieRpEnableMask & BIT(rp_number); : if (type == CPU_PCIE_RP) : return m_cfg->CpuPcieRpEnableMask; Instead of requiring the user to have written into `m_cfg` before calling this function, why not have this function take a `uint32_t` that is the mask instead?; the caller already knows whether it's CPU or PCH.
File src/soc/intel/common/block/include/intelblocks/pcie_rp.h:
https://review.coreboot.org/c/coreboot/+/48340/comment/5cd26afb_b00a7430 PS34, Line 17: 0x80
okay, make sense.
Sorry what I mean is using PCI_CLK_FREE in a devicetree (or code) makes the code easier to read; agree with Angel, leave this comment as `/* Clock source is free running */`.