Attention is currently required from: Nico Huber, Subrata Banik, Michael Niewöhner, EricR Lai.
Patch set 47:-Code-Review
4 comments:
File src/mainboard/intel/adlrvp/devicetree.cb:
NB. This can also be set inside the respective `device pci` node, now.
+1. I really like that idea. And hoping that is how we organize things on Google reference board. Makes it easier to read and follow.
File src/soc/intel/alderlake/romstage/fsp_params.c:
Patch Set #47, Line 42: for (i = 0; i < cfg_count; i++) {
nit: This can also be organized to only loop through RPs that are initialized:
while (en_mask) {
i = __ffs(en_mask);
en_mask &= ~i;
...
}
Then you don't need to pass in cfg_count at all. Anyways, not a big deal if we simply loop over the root ports.
Patch Set #47, Line 48: m_cfg->PcieClkSrcClkReq[cfg[i].clk_src] = cfg[i].clk_req;
Why skip the assignment? What is the default value and does it have […]
I think it will have to be initialized to 0xFF (PCIE_CLK_NOT_USED) just like PcieClkSrcUsage is on line 142 below.
File src/soc/intel/common/block/include/intelblocks/pcie_rp.h:
Patch Set #47, Line 22: PCIE_RP_CLK_FREE = (1 << 3),
I'm a bit confused here. Doesn't this usually mean the clock is used for […]
That is what I am confused about too. I assumed "free running clock" meant that the clock is not associated with any RP, but has some unspecified usage. Hence, it is an attribute of the clock and not of the root port. But, looking at the way it is set up for ADLRVP, it looks like a clock source which has a CLKREQ# associated with it is marked as a free running clock. It seems like a bad workaround for some different issue.
To view, visit change 48340. To unsubscribe, or for help writing mail filters, visit settings.