Attention is currently required from: Furquan Shaikh, Tim Wawrzynczak, Subrata Banik, Angel Pons, Michael Niewöhner. EricR Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48340 )
Change subject: soc/intel/alderlake: Refactor PCIE port config ......................................................................
Patch Set 36:
(10 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/48340/comment/28657fbe_9b146f3f PS34, Line 9: PCIE ClkSrcUsage and ClkSrcClkReq are always confusing in devicetree. : Make it easier to just fill the number from schematics. :
Will change in the final commit. We still have problem with CPU RP ports need to work out.
Done
File src/soc/intel/alderlake/chip.h:
https://review.coreboot.org/c/coreboot/+/48340/comment/8f0faf3e_85319170 PS34, Line 343: tic inline int pcie_is_flag_enabled(const struct pcie_rp_config pcie_rp, : enum pcie_rp_flags flag_mask) : { : return pcie_rp.flags & flag_mask ? 1 : 0; : } :
Yes, will clean up with move pcie_rp_config to pcie_rp.
Done
File src/soc/intel/alderlake/fsp_params.c:
https://review.coreboot.org/c/coreboot/+/48340/comment/f735409a_aba08824 PS34, Line 264: config->pch_pcie_rp[i]
To save some redundancy, I would use a pointer as follows: […]
let me think. looks fine.
File src/soc/intel/alderlake/romstage/fsp_params.c:
https://review.coreboot.org/c/coreboot/+/48340/comment/b551ee09_491b60cd PS23, Line 154: is_dev_enabled(dev)
okay, I think Subrata not answer my question,yet. But I can follow it for review.
Done
File src/soc/intel/alderlake/romstage/fsp_params.c:
https://review.coreboot.org/c/coreboot/+/48340/comment/1d37fcc6_aafeb0d8 PS28, Line 43: CpuPcieRpEnableMask
only one rp?
Done
File src/soc/intel/alderlake/romstage/fsp_params.c:
https://review.coreboot.org/c/coreboot/+/48340/comment/9ac0a2d7_fdcff608 PS32, Line 20: enum pcie_rp_type {
TODO: move to pcie_rp. […]
only this file use this. no need.
File src/soc/intel/alderlake/romstage/fsp_params.c:
https://review.coreboot.org/c/coreboot/+/48340/comment/9ea8c5ce_55035b2a PS34, Line 25: cfg
nit: pass pointers instead? It should be more efficient than passing by copy
Done
https://review.coreboot.org/c/coreboot/+/48340/comment/e2bde202_1c6215db PS34, Line 29: 0x80
PCIE_CLK_FREE
Done
https://review.coreboot.org/c/coreboot/+/48340/comment/c4afa3c6_d5c5c1a4 PS34, Line 42: return m_cfg->PcieRpEnableMask & BIT(rp_number); : if (type == CPU_PCIE_RP) : return m_cfg->CpuPcieRpEnableMask;
I think this is work use PcieRpEnableMask/CpuPcieRpEnableMask as parameter and remove the type :) Wi […]
Done
https://review.coreboot.org/c/coreboot/+/48340/comment/59ad8ec9_44a35c61 PS34, Line 44: CpuPcieRpEnableMask
Still wait Furquan patch the helper... And I think I need to add new KConfig for CPU_RP count first.
Done