Attention is currently required from: Nico Huber, Tim Wawrzynczak, Subrata Banik, Angel Pons, Michael Niewöhner, EricR Lai.
Patch set 60:Code-Review +1
3 comments:
Patchset:
Thanks for your patience Eric. Just one comment about the usage of PCIE_CLK_FREE. I think it is defined as enum and macro. But there are really two different values in the two cases. I think we can just drop pch.h completely and move the definitions to romstage/fsp_params.c since they are now used only in that file. Also, better to add the prefix FSP_ to indicate that they are the values expected by FSP.
File src/soc/intel/alderlake/romstage/fsp_params.c:
Patch Set #60, Line 142: PCIE_CLK_FREE
Looks like PCIE_CLK_FREE got reused for two different purposes.
1. Mainboard setting in pcie_clk_config_flag --> I think this should be named PCIE_CLK_FREE_RUNNING
2. FSP value for free running clock --> I think it should be named FSP_CLK_FREE_RUNNING just to make it clear that it is something that FSP expects. It will be the one that is assigned to PcieClkSrcUsage.
File src/soc/intel/common/block/include/intelblocks/pcie_rp.h:
Patch Set #60, Line 33: PCIE_CLK_FREE
Woops. This name clashes with the definition her required by FSP: https://review.coreboot.org/cgit/coreboot.git/tree/src/soc/intel/alderlake/include/soc/pch.h?id=refs/heads/master#n8
Probably name this PCIE_CLK_FREE_RUNNING?
To view, visit change 48340. To unsubscribe, or for help writing mail filters, visit settings.