EricR Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48340 )
Change subject: soc/intel/alderlake: Revise PCIE port config ......................................................................
Patch Set 28:
(6 comments)
https://review.coreboot.org/c/coreboot/+/48340/28/src/soc/intel/alderlake/ch... File src/soc/intel/alderlake/chip.h:
https://review.coreboot.org/c/coreboot/+/48340/28/src/soc/intel/alderlake/ch... PS28, Line 22: #define PCIE_RP(x) ((x) - 1) : #define PCH_RP(x) PCIE_RP(x) : #define CPU_RP(x) PCIE_RP(x) :
not soc-specific
Done
https://review.coreboot.org/c/coreboot/+/48340/28/src/soc/intel/alderlake/ch... PS28, Line 27: PCIE_RP_HOTPLUG_ENABLED = (1 << 0), : PCIE_RP_LTR_ENABLED = (1 << 1), : /* PCIE RP Advanced Error Report */ : PCIE_RP_AER_ENABLED = (1 << 2),
nit: maybe drop _ENABLED? I'd say it's pretty clear what a flag PCIE_RP_HOTPLUG would do
https://review.coreboot.org/c/coreboot/+/48340/28/src/soc/intel/alderlake/ch... File src/soc/intel/alderlake/chip.c:
https://review.coreboot.org/c/coreboot/+/48340/28/src/soc/intel/alderlake/ch... PS28, Line 19: t struct pcie_rp_group pch_lp_rp_groups[] = { : { .slot = PCH_DEV_SLOT_PCIE, .count = 8 }, : { .slot = PCH_DEV_SLOT_PCIE_1, .count = 4 }, : { 0 } : };
moving this and the related parts could be done in a separate change to reduce patch size and make r […]
Done
https://review.coreboot.org/c/coreboot/+/48340/28/src/soc/intel/alderlake/ro... File src/soc/intel/alderlake/romstage/fsp_params.c:
https://review.coreboot.org/c/coreboot/+/48340/28/src/soc/intel/alderlake/ro... PS28, Line 41: return m_cfg->PcieRpEnableMask
This already done by lookup devicetree. […]
Done
https://review.coreboot.org/c/coreboot/+/48340/28/src/soc/intel/alderlake/ro... PS28, Line 57: pcie_is_flag_enabled(cfg[i], PCIE_RP_CLK_REQ_UNUSED)) : m_cfg->PcieClkSrcClkReq[cfg[i].clk_src] = cfg[i].clk_req; : m_cfg->PcieClkSrcUsage[cfg[i].clk_src] = : convert_clk_src_to_fsp(type, cfg[i], i);
oh... I forgot the ! in line53.. thanks.
Done
https://review.coreboot.org/c/coreboot/+/48340/28/src/soc/intel/alderlake/ro... PS28, Line 157: CONFIG_MAX_ROOT_PORTS
just use CONFIG_MAX_ROOT_PORTS above in pcie_rp_init: […]
We plan to add new Kconfig for CPU_RP ports. It would be different size though. Thanks :)