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 13:
(10 comments)
https://review.coreboot.org/c/coreboot/+/48340/9/src/soc/intel/alderlake/chi... File src/soc/intel/alderlake/chip.h:
https://review.coreboot.org/c/coreboot/+/48340/9/src/soc/intel/alderlake/chi... PS9, Line 122: struct {
SG! Let me try this.
Done
https://review.coreboot.org/c/coreboot/+/48340/1/src/soc/intel/alderlake/chi... File src/soc/intel/alderlake/chip.h:
https://review.coreboot.org/c/coreboot/+/48340/1/src/soc/intel/alderlake/chi... PS1, Line 126: PcieRp
make it lowercase ?
Done
https://review.coreboot.org/c/coreboot/+/48340/1/src/soc/intel/alderlake/rom... File src/soc/intel/alderlake/romstage/fsp_params.c:
https://review.coreboot.org/c/coreboot/+/48340/1/src/soc/intel/alderlake/rom... PS1, Line 48: PcieRp[i].clksrc
I hope it can use like this in devicetree. Need to try though... […]
Done
https://review.coreboot.org/c/coreboot/+/48340/2/src/soc/intel/alderlake/rom... File src/soc/intel/alderlake/romstage/fsp_params.c:
https://review.coreboot.org/c/coreboot/+/48340/2/src/soc/intel/alderlake/rom... PS2, Line 48: config->PcieClkSrcUsage[PcieRp[i].clksrc] = i;
This may not use for it root port. Need to redefine it.. hmmm..
Ack
https://review.coreboot.org/c/coreboot/+/48340/9/src/soc/intel/alderlake/rom... File src/soc/intel/alderlake/romstage/fsp_params.c:
https://review.coreboot.org/c/coreboot/+/48340/9/src/soc/intel/alderlake/rom... PS9, Line 25: 0xFF
`PCIE_CLK_NOTUSED`
Done
https://review.coreboot.org/c/coreboot/+/48340/9/src/soc/intel/alderlake/rom... PS9, Line 31: /* Back up special usage */
This is not required.
Done
https://review.coreboot.org/c/coreboot/+/48340/9/src/soc/intel/alderlake/rom... PS9, Line 38: 0xff
PCIE_CLK_NOTUSED
Done
https://review.coreboot.org/c/coreboot/+/48340/9/src/soc/intel/alderlake/rom... PS9, Line 40: config->PcieRp[i].clkreq
Let us use enums :p I like this way.
Done
https://review.coreboot.org/c/coreboot/+/48340/9/src/soc/intel/alderlake/rom... PS9, Line 40: PcieClkSrcClkReq
You don't need to set this in a local array. It can directly be set in m_cfg->PcieClkSrcClkReq. […]
Done
https://review.coreboot.org/c/coreboot/+/48340/9/src/soc/intel/alderlake/rom... PS9, Line 43: : i
Let's not do this. Instead let's ensure that mainboard sets it correctly.
Done