Morgan Jang has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/41690 )
Change subject: mb/ocp/deltalake: Config PCH PCIe ports ......................................................................
mb/ocp/deltalake: Config PCH PCIe ports
Tested on OCP Delta Lake with lspci checking if PCIe speed is changed are expected.
Change-Id: I189027c403814d68db2b7c5f41fc254a293fe3a1 Signed-off-by: Morgan Jang Morgan_Jang@wiwynn.com --- M src/mainboard/ocp/deltalake/romstage.c 1 file changed, 12 insertions(+), 8 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/90/41690/1
diff --git a/src/mainboard/ocp/deltalake/romstage.c b/src/mainboard/ocp/deltalake/romstage.c index c456913..f6b828f 100644 --- a/src/mainboard/ocp/deltalake/romstage.c +++ b/src/mainboard/ocp/deltalake/romstage.c @@ -22,24 +22,28 @@
static void mainboard_config_iio(FSPM_UPD *mupd) { + int index;
mupd->FspmConfig.IIoBifurcationTablePtr = (uint32_t) dl_iio_bifur_table; mupd->FspmConfig.NumOfIIoBifurcationTable = ARRAY_SIZE(dl_iio_bifur_table); + #if 0 mupd->FspmConfig.IioPciConfig.ConfigurationTable = (UPD_PCI_PORT_CONFIG *) dl_iio_pci_port; mupd->FspmConfig.IioPciConfig.NumberOfEntries = ARRAY_SIZE(dl_iio_pci_port); - - mupd->FspmConfig.PchPciConfig.PciPortConfig = - (UPD_PCH_PCIE_PORT *) dl_pch_pci_port; - mupd->FspmConfig.PchPciConfig.NumberOfEntries = - ARRAY_SIZE(dl_pch_pci_port); - - mupd->FspmConfig.PchPciConfig.RootPortFunctionSwapping = 0x00; - mupd->FspmConfig.PchPciConfig.PciePllSsc = 0x00; #endif + + for (index = 0; index < ARRAY_SIZE(dl_pch_pci_port); index++) { + mupd->FspmConfig.PchPcieForceEnable[dl_pch_pci_port[index].PortIndex] = + dl_pch_pci_port[index].ForceEnable; + mupd->FspmConfig.PchPciePortLinkSpeed[dl_pch_pci_port[index].PortIndex] = + dl_pch_pci_port[index].PortLinkSpeed; + } + + mupd->FspmConfig.PchPcieRootPortFunctionSwap = 0x00; + mupd->FspmConfig.PchPciePllSsc = 0xFE; }
void mainboard_memory_init_params(FSPM_UPD *mupd)
Jonathan Zhang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41690 )
Change subject: mb/ocp/deltalake: Config PCH PCIe ports ......................................................................
Patch Set 1:
(2 comments)
https://review.coreboot.org/c/coreboot/+/41690/1/src/mainboard/ocp/deltalake... File src/mainboard/ocp/deltalake/romstage.c:
https://review.coreboot.org/c/coreboot/+/41690/1/src/mainboard/ocp/deltalake... PS1, Line 25: int index; uint_8
https://review.coreboot.org/c/coreboot/+/41690/1/src/mainboard/ocp/deltalake... PS1, Line 46: mupd->FspmConfig.PchPciePllSsc = 0xFE; Let's add a comment to explain the default is different.
Hello build bot (Jenkins), Jonathan Zhang,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/41690
to look at the new patch set (#2).
Change subject: mb/ocp/deltalake: Config PCH PCIe ports ......................................................................
mb/ocp/deltalake: Config PCH PCIe ports
Tested on OCP Delta Lake with lspci checking if PCIe speed is changed are expected.
Change-Id: I189027c403814d68db2b7c5f41fc254a293fe3a1 Signed-off-by: Morgan Jang Morgan_Jang@wiwynn.com --- M src/mainboard/ocp/deltalake/romstage.c 1 file changed, 14 insertions(+), 8 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/90/41690/2
Morgan Jang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41690 )
Change subject: mb/ocp/deltalake: Config PCH PCIe ports ......................................................................
Patch Set 2:
(2 comments)
Patch Set 1:
(2 comments)
https://review.coreboot.org/c/coreboot/+/41690/1/src/mainboard/ocp/deltalake... File src/mainboard/ocp/deltalake/romstage.c:
https://review.coreboot.org/c/coreboot/+/41690/1/src/mainboard/ocp/deltalake... PS1, Line 25: int index;
uint_8
Done.
https://review.coreboot.org/c/coreboot/+/41690/1/src/mainboard/ocp/deltalake... PS1, Line 46: mupd->FspmConfig.PchPciePllSsc = 0xFE;
Let's add a comment to explain the default is different.
Done.
Hello build bot (Jenkins), Jonathan Zhang,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/41690
to look at the new patch set (#3).
Change subject: mb/ocp/deltalake: Config PCH PCIe ports ......................................................................
mb/ocp/deltalake: Config PCH PCIe ports
Tested on OCP Delta Lake with lspci checking if PCIe speed is changed are expected.
Change-Id: I189027c403814d68db2b7c5f41fc254a293fe3a1 Signed-off-by: Morgan Jang Morgan_Jang@wiwynn.com --- M src/mainboard/ocp/deltalake/cpxsp_dl_iio.h M src/mainboard/ocp/deltalake/romstage.c 2 files changed, 16 insertions(+), 9 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/90/41690/3
Hello build bot (Jenkins), Jonathan Zhang,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/41690
to look at the new patch set (#4).
Change subject: mb/ocp/deltalake: Config PCH PCIe ports ......................................................................
mb/ocp/deltalake: Config PCH PCIe ports
Tested on OCP Delta Lake with lspci checking if PCIe speed is changed are expected.
Change-Id: I189027c403814d68db2b7c5f41fc254a293fe3a1 Signed-off-by: Morgan Jang Morgan_Jang@wiwynn.com --- M src/mainboard/ocp/deltalake/cpxsp_dl_iio.h M src/mainboard/ocp/deltalake/romstage.c M src/vendorcode/intel/fsp/fsp2_1/cooperlake_sp/FspmUpd.h 3 files changed, 39 insertions(+), 9 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/90/41690/4
Johnny Lin has uploaded a new patch set (#5) to the change originally created by Morgan Jang. ( https://review.coreboot.org/c/coreboot/+/41690 )
Change subject: mb/ocp/deltalake: Config PCH PCIe ports ......................................................................
mb/ocp/deltalake: Config PCH PCIe ports
Tested on OCP Delta Lake with lspci checking if PCIe speed is changed are expected.
Change-Id: I189027c403814d68db2b7c5f41fc254a293fe3a1 Signed-off-by: Morgan Jang Morgan_Jang@wiwynn.com --- M src/mainboard/ocp/deltalake/cpxsp_dl_iio.h M src/mainboard/ocp/deltalake/romstage.c M src/vendorcode/intel/fsp/fsp2_1/cooperlake_sp/FspmUpd.h 3 files changed, 39 insertions(+), 9 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/90/41690/5
Johnny Lin has uploaded a new patch set (#6) to the change originally created by Morgan Jang. ( https://review.coreboot.org/c/coreboot/+/41690 )
Change subject: mb/ocp/deltalake: Config PCH PCIe ports ......................................................................
mb/ocp/deltalake: Config PCH PCIe ports
Tested on OCP Delta Lake with lspci checking if PCIe speed is changed are expected.
Change-Id: I189027c403814d68db2b7c5f41fc254a293fe3a1 Signed-off-by: Morgan Jang Morgan_Jang@wiwynn.com --- M src/mainboard/ocp/deltalake/cpxsp_dl_iio.h M src/mainboard/ocp/deltalake/romstage.c M src/vendorcode/intel/fsp/fsp2_1/cooperlake_sp/FspmUpd.h 3 files changed, 39 insertions(+), 9 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/90/41690/6
Hello build bot (Jenkins), Jonathan Zhang, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/41690
to look at the new patch set (#8).
Change subject: mb/ocp/deltalake: Config PCH PCIe ports ......................................................................
mb/ocp/deltalake: Config PCH PCIe ports
Tested on OCP Delta Lake with lspci checking if PCIe speed is changed are expected.
Change-Id: I189027c403814d68db2b7c5f41fc254a293fe3a1 Signed-off-by: Morgan Jang Morgan_Jang@wiwynn.com --- M src/mainboard/ocp/deltalake/cpxsp_dl_iio.h M src/mainboard/ocp/deltalake/romstage.c M src/vendorcode/intel/fsp/fsp2_0/cooperlake_sp/FspmUpd.h 3 files changed, 39 insertions(+), 9 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/90/41690/8
Jonathan Zhang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41690 )
Change subject: mb/ocp/deltalake: Config PCH PCIe ports ......................................................................
Patch Set 8: Code-Review+1
LGTM
Christian Walter has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41690 )
Change subject: mb/ocp/deltalake: Config PCH PCIe ports ......................................................................
Patch Set 8:
(1 comment)
https://review.coreboot.org/c/coreboot/+/41690/8/src/mainboard/ocp/deltalake... File src/mainboard/ocp/deltalake/romstage.c:
https://review.coreboot.org/c/coreboot/+/41690/8/src/mainboard/ocp/deltalake... PS8, Line 72: for (index = 0; index < ARRAY_SIZE(dl_pch_pci_port); index++) { : mupd->FspmConfig.PchPcieForceEnable[dl_pch_pci_port[index].PortIndex] = : dl_pch_pci_port[index].ForceEnable; : mupd->FspmConfig.PchPciePortLinkSpeed[dl_pch_pci_port[index].PortIndex] = : dl_pch_pci_port[index].PortLinkSpeed; : } Can't this be done in the SoC code and pass this through the devicetree and via a config?
Jonathan Zhang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41690 )
Change subject: mb/ocp/deltalake: Config PCH PCIe ports ......................................................................
Patch Set 8:
(1 comment)
https://review.coreboot.org/c/coreboot/+/41690/8/src/mainboard/ocp/deltalake... File src/mainboard/ocp/deltalake/romstage.c:
https://review.coreboot.org/c/coreboot/+/41690/8/src/mainboard/ocp/deltalake... PS8, Line 72: for (index = 0; index < ARRAY_SIZE(dl_pch_pci_port); index++) { : mupd->FspmConfig.PchPcieForceEnable[dl_pch_pci_port[index].PortIndex] = : dl_pch_pci_port[index].ForceEnable; : mupd->FspmConfig.PchPciePortLinkSpeed[dl_pch_pci_port[index].PortIndex] = : dl_pch_pci_port[index].PortLinkSpeed; : }
Can't this be done in the SoC code and pass this through the devicetree and via a config?
Hi Christian, for DeltaLake, the PCH PCIe configuration is same for all SKUs. So we choose the header file approach which is striaght forward. I suppose your suggestion is to define such in devicetree.cb instead. Is there a precedence of doing such? I think it is doable, but may be over-kill.
Christian Walter has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41690 )
Change subject: mb/ocp/deltalake: Config PCH PCIe ports ......................................................................
Patch Set 8:
(1 comment)
https://review.coreboot.org/c/coreboot/+/41690/8/src/mainboard/ocp/deltalake... File src/mainboard/ocp/deltalake/romstage.c:
https://review.coreboot.org/c/coreboot/+/41690/8/src/mainboard/ocp/deltalake... PS8, Line 72: for (index = 0; index < ARRAY_SIZE(dl_pch_pci_port); index++) { : mupd->FspmConfig.PchPcieForceEnable[dl_pch_pci_port[index].PortIndex] = : dl_pch_pci_port[index].ForceEnable; : mupd->FspmConfig.PchPciePortLinkSpeed[dl_pch_pci_port[index].PortIndex] = : dl_pch_pci_port[index].PortLinkSpeed; : }
Hi Christian, for DeltaLake, the PCH PCIe configuration is same for all SKUs. […]
What you are doing here is setting the FspmConfig options in the mainboard folder. The "normal" way to do so is bringing this code to soc/intel/xeon_sp/{skx,cpx}/romstage.c where you parse the soc config and write the FSPM UPDs. Therefore you would need to add chip config options and the hand over the values via devicetree. Something like CB:42440 . I don't understand why this should belong into the mb code.
Jonathan Zhang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41690 )
Change subject: mb/ocp/deltalake: Config PCH PCIe ports ......................................................................
Patch Set 8:
(1 comment)
https://review.coreboot.org/c/coreboot/+/41690/8/src/mainboard/ocp/deltalake... File src/mainboard/ocp/deltalake/romstage.c:
https://review.coreboot.org/c/coreboot/+/41690/8/src/mainboard/ocp/deltalake... PS8, Line 72: for (index = 0; index < ARRAY_SIZE(dl_pch_pci_port); index++) { : mupd->FspmConfig.PchPcieForceEnable[dl_pch_pci_port[index].PortIndex] = : dl_pch_pci_port[index].ForceEnable; : mupd->FspmConfig.PchPciePortLinkSpeed[dl_pch_pci_port[index].PortIndex] = : dl_pch_pci_port[index].PortLinkSpeed; : }
What you are doing here is setting the FspmConfig options in the mainboard folder. […]
Thanks Christian, this makes sense. Morgan, please do so accordingly.
Hello Philipp Deppenwiese, build bot (Jenkins), Jonathan Zhang, Christian Walter, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/41690
to look at the new patch set (#9).
Change subject: mb/ocp/deltalake: Config PCH PCIe ports ......................................................................
mb/ocp/deltalake: Config PCH PCIe ports
Tested on OCP Delta Lake with lspci checking if PCIe speed is changed are expected.
Change-Id: I189027c403814d68db2b7c5f41fc254a293fe3a1 Signed-off-by: Morgan Jang Morgan_Jang@wiwynn.com --- M src/mainboard/ocp/deltalake/devicetree.cb M src/mainboard/ocp/deltalake/romstage.c M src/soc/intel/xeon_sp/cpx/chip.h M src/vendorcode/intel/fsp/fsp2_0/cooperlake_sp/FspmUpd.h 4 files changed, 52 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/90/41690/9
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41690 )
Change subject: mb/ocp/deltalake: Config PCH PCIe ports ......................................................................
Patch Set 9:
(4 comments)
https://review.coreboot.org/c/coreboot/+/41690/9/src/mainboard/ocp/deltalake... File src/mainboard/ocp/deltalake/romstage.c:
https://review.coreboot.org/c/coreboot/+/41690/9/src/mainboard/ocp/deltalake... PS9, Line 67: mupd->FspmConfig.PchPcieForceEnable[config->dl_pch_pci_port[index].PortIndex] = line over 96 characters
https://review.coreboot.org/c/coreboot/+/41690/9/src/mainboard/ocp/deltalake... PS9, Line 69: mupd->FspmConfig.PchPciePortLinkSpeed[config->dl_pch_pci_port[index].PortIndex] = line over 96 characters
https://review.coreboot.org/c/coreboot/+/41690/9/src/soc/intel/xeon_sp/cpx/c... File src/soc/intel/xeon_sp/cpx/chip.h:
https://review.coreboot.org/c/coreboot/+/41690/9/src/soc/intel/xeon_sp/cpx/c... PS9, Line 14: trailing whitespace
https://review.coreboot.org/c/coreboot/+/41690/9/src/soc/intel/xeon_sp/cpx/c... PS9, Line 17: trailing whitespace
Hello Philipp Deppenwiese, build bot (Jenkins), Jonathan Zhang, Christian Walter, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/41690
to look at the new patch set (#10).
Change subject: mb/ocp/deltalake: Config PCH PCIe ports ......................................................................
mb/ocp/deltalake: Config PCH PCIe ports
Tested on OCP Delta Lake with lspci checking if PCIe speed is changed are expected.
Change-Id: I189027c403814d68db2b7c5f41fc254a293fe3a1 Signed-off-by: Morgan Jang Morgan_Jang@wiwynn.com --- M src/mainboard/ocp/deltalake/devicetree.cb M src/mainboard/ocp/deltalake/romstage.c M src/soc/intel/xeon_sp/cpx/chip.h M src/vendorcode/intel/fsp/fsp2_0/cooperlake_sp/FspmUpd.h 4 files changed, 51 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/90/41690/10
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41690 )
Change subject: mb/ocp/deltalake: Config PCH PCIe ports ......................................................................
Patch Set 10:
(2 comments)
https://review.coreboot.org/c/coreboot/+/41690/10/src/mainboard/ocp/deltalak... File src/mainboard/ocp/deltalake/romstage.c:
https://review.coreboot.org/c/coreboot/+/41690/10/src/mainboard/ocp/deltalak... PS10, Line 67: mupd->FspmConfig.PchPcieForceEnable[config->dl_pch_pci_port[index].PortIndex] = line over 96 characters
https://review.coreboot.org/c/coreboot/+/41690/10/src/mainboard/ocp/deltalak... PS10, Line 69: mupd->FspmConfig.PchPciePortLinkSpeed[config->dl_pch_pci_port[index].PortIndex] = line over 96 characters
Morgan Jang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41690 )
Change subject: mb/ocp/deltalake: Config PCH PCIe ports ......................................................................
Patch Set 10:
(1 comment)
https://review.coreboot.org/c/coreboot/+/41690/8/src/mainboard/ocp/deltalake... File src/mainboard/ocp/deltalake/romstage.c:
https://review.coreboot.org/c/coreboot/+/41690/8/src/mainboard/ocp/deltalake... PS8, Line 72: for (index = 0; index < ARRAY_SIZE(dl_pch_pci_port); index++) { : mupd->FspmConfig.PchPcieForceEnable[dl_pch_pci_port[index].PortIndex] = : dl_pch_pci_port[index].ForceEnable; : mupd->FspmConfig.PchPciePortLinkSpeed[dl_pch_pci_port[index].PortIndex] = : dl_pch_pci_port[index].PortLinkSpeed; : }
Thanks Christian, this makes sense. […]
Done
Jonathan Zhang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41690 )
Change subject: mb/ocp/deltalake: Config PCH PCIe ports ......................................................................
Patch Set 10:
(1 comment)
https://review.coreboot.org/c/coreboot/+/41690/10/src/soc/intel/xeon_sp/cpx/... File src/soc/intel/xeon_sp/cpx/chip.h:
https://review.coreboot.org/c/coreboot/+/41690/10/src/soc/intel/xeon_sp/cpx/... PS10, Line 16: struct UPD_PCH_PCIE_PORT dl_pch_pci_port[20]; 20 is a magic number here, and is also hardcoded in romstag.c, let's define it. The name of "dl_pch_pci_port" can be renamed to "pch_pci_port". Does it make sense for us to set default to disabled, and only ports to be ForceEnabled should be defined in devicetree.cb?
Hello Philipp Deppenwiese, build bot (Jenkins), Jonathan Zhang, Christian Walter, Angel Pons, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/41690
to look at the new patch set (#11).
Change subject: mb/ocp/deltalake: Config PCH PCIe ports ......................................................................
mb/ocp/deltalake: Config PCH PCIe ports
Tested on OCP Delta Lake with lspci checking if PCIe speed is changed are expected.
Change-Id: I189027c403814d68db2b7c5f41fc254a293fe3a1 Signed-off-by: Morgan Jang Morgan_Jang@wiwynn.com --- M src/mainboard/ocp/deltalake/devicetree.cb M src/mainboard/ocp/deltalake/romstage.c M src/soc/intel/xeon_sp/cpx/chip.h M src/vendorcode/intel/fsp/fsp2_0/cooperlake_sp/FspmUpd.h 4 files changed, 55 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/90/41690/11
Morgan Jang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41690 )
Change subject: mb/ocp/deltalake: Config PCH PCIe ports ......................................................................
Patch Set 11:
(1 comment)
https://review.coreboot.org/c/coreboot/+/41690/10/src/soc/intel/xeon_sp/cpx/... File src/soc/intel/xeon_sp/cpx/chip.h:
https://review.coreboot.org/c/coreboot/+/41690/10/src/soc/intel/xeon_sp/cpx/... PS10, Line 16: struct UPD_PCH_PCIE_PORT dl_pch_pci_port[20];
20 is a magic number here, and is also hardcoded in romstag.c, let's define it. […]
Done
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41690 )
Change subject: mb/ocp/deltalake: Config PCH PCIe ports ......................................................................
Patch Set 11:
(2 comments)
https://review.coreboot.org/c/coreboot/+/41690/11/src/mainboard/ocp/deltalak... File src/mainboard/ocp/deltalake/romstage.c:
https://review.coreboot.org/c/coreboot/+/41690/11/src/mainboard/ocp/deltalak... PS11, Line 70: mupd->FspmConfig.PchPcieForceEnable[config->pch_pci_port[index].PortIndex] = line over 96 characters
https://review.coreboot.org/c/coreboot/+/41690/11/src/mainboard/ocp/deltalak... PS11, Line 72: mupd->FspmConfig.PchPciePortLinkSpeed[config->pch_pci_port[index].PortIndex] = line over 96 characters
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41690 )
Change subject: mb/ocp/deltalake: Config PCH PCIe ports ......................................................................
Patch Set 11: Code-Review+1
(2 comments)
https://review.coreboot.org/c/coreboot/+/41690/11/src/soc/intel/xeon_sp/cpx/... File src/soc/intel/xeon_sp/cpx/chip.h:
https://review.coreboot.org/c/coreboot/+/41690/11/src/soc/intel/xeon_sp/cpx/... PS11, Line 15: /*Struct for configuring PCH PCIe port*/ nit: spaces around the comment start and end symbols:
/* Struct for configuring PCH PCIe port */
https://review.coreboot.org/c/coreboot/+/41690/11/src/vendorcode/intel/fsp/f... File src/vendorcode/intel/fsp/fsp2_0/cooperlake_sp/FspmUpd.h:
https://review.coreboot.org/c/coreboot/+/41690/11/src/vendorcode/intel/fsp/f... PS11, Line 144: { add a space
Hello Philipp Deppenwiese, build bot (Jenkins), Jonathan Zhang, Christian Walter, Angel Pons, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/41690
to look at the new patch set (#12).
Change subject: mb/ocp/deltalake: Config PCH PCIe ports ......................................................................
mb/ocp/deltalake: Config PCH PCIe ports
Tested on OCP Delta Lake with lspci checking if PCIe speed is changed are expected.
Change-Id: I189027c403814d68db2b7c5f41fc254a293fe3a1 Signed-off-by: Morgan Jang Morgan_Jang@wiwynn.com --- M src/mainboard/ocp/deltalake/devicetree.cb M src/mainboard/ocp/deltalake/romstage.c M src/soc/intel/xeon_sp/cpx/chip.h M src/vendorcode/intel/fsp/fsp2_0/cooperlake_sp/FspmUpd.h 4 files changed, 55 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/90/41690/12
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41690 )
Change subject: mb/ocp/deltalake: Config PCH PCIe ports ......................................................................
Patch Set 12:
(2 comments)
https://review.coreboot.org/c/coreboot/+/41690/12/src/mainboard/ocp/deltalak... File src/mainboard/ocp/deltalake/romstage.c:
https://review.coreboot.org/c/coreboot/+/41690/12/src/mainboard/ocp/deltalak... PS12, Line 70: mupd->FspmConfig.PchPcieForceEnable[config->pch_pci_port[index].PortIndex] = line over 96 characters
https://review.coreboot.org/c/coreboot/+/41690/12/src/mainboard/ocp/deltalak... PS12, Line 72: mupd->FspmConfig.PchPciePortLinkSpeed[config->pch_pci_port[index].PortIndex] = line over 96 characters
Morgan Jang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41690 )
Change subject: mb/ocp/deltalake: Config PCH PCIe ports ......................................................................
Patch Set 12:
(2 comments)
https://review.coreboot.org/c/coreboot/+/41690/11/src/soc/intel/xeon_sp/cpx/... File src/soc/intel/xeon_sp/cpx/chip.h:
https://review.coreboot.org/c/coreboot/+/41690/11/src/soc/intel/xeon_sp/cpx/... PS11, Line 15: /*Struct for configuring PCH PCIe port*/
nit: spaces around the comment start and end symbols: […]
Done
https://review.coreboot.org/c/coreboot/+/41690/11/src/vendorcode/intel/fsp/f... File src/vendorcode/intel/fsp/fsp2_0/cooperlake_sp/FspmUpd.h:
https://review.coreboot.org/c/coreboot/+/41690/11/src/vendorcode/intel/fsp/f... PS11, Line 144: {
add a space
Done
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41690 )
Change subject: mb/ocp/deltalake: Config PCH PCIe ports ......................................................................
Patch Set 12: Code-Review+1
Hello Philipp Deppenwiese, build bot (Jenkins), Jonathan Zhang, Christian Walter, Angel Pons, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/41690
to look at the new patch set (#13).
Change subject: mb/ocp/deltalake: Config PCH PCIe ports ......................................................................
mb/ocp/deltalake: Config PCH PCIe ports
Tested on OCP Delta Lake with lspci checking if PCIe speed is changed are expected.
Change-Id: I189027c403814d68db2b7c5f41fc254a293fe3a1 Signed-off-by: Morgan Jang Morgan_Jang@wiwynn.com --- M src/mainboard/ocp/deltalake/devicetree.cb M src/mainboard/ocp/deltalake/romstage.c M src/soc/intel/xeon_sp/cpx/chip.h M src/vendorcode/intel/fsp/fsp2_0/cooperlake_sp/FspmUpd.h 4 files changed, 55 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/90/41690/13
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41690 )
Change subject: mb/ocp/deltalake: Config PCH PCIe ports ......................................................................
Patch Set 13:
(2 comments)
https://review.coreboot.org/c/coreboot/+/41690/13/src/mainboard/ocp/deltalak... File src/mainboard/ocp/deltalake/romstage.c:
https://review.coreboot.org/c/coreboot/+/41690/13/src/mainboard/ocp/deltalak... PS13, Line 88: mupd->FspmConfig.PchPcieForceEnable[config->pch_pci_port[index].PortIndex] = line over 96 characters
https://review.coreboot.org/c/coreboot/+/41690/13/src/mainboard/ocp/deltalak... PS13, Line 90: mupd->FspmConfig.PchPciePortLinkSpeed[config->pch_pci_port[index].PortIndex] = line over 96 characters
Jonathan Zhang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41690 )
Change subject: mb/ocp/deltalake: Config PCH PCIe ports ......................................................................
Patch Set 13: Code-Review+1
Hello Philipp Deppenwiese, build bot (Jenkins), Jonathan Zhang, Christian Walter, Angel Pons, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/41690
to look at the new patch set (#14).
Change subject: mb/ocp/deltalake: Config PCH PCIe ports ......................................................................
mb/ocp/deltalake: Config PCH PCIe ports
Tested on OCP Delta Lake with lspci checking if PCIe speed is changed are expected.
Change-Id: I189027c403814d68db2b7c5f41fc254a293fe3a1 Signed-off-by: Morgan Jang Morgan_Jang@wiwynn.com --- M src/mainboard/ocp/deltalake/devicetree.cb M src/mainboard/ocp/deltalake/romstage.c M src/soc/intel/xeon_sp/cpx/chip.h M src/vendorcode/intel/fsp/fsp2_0/cooperlake_sp/FspmUpd.h 4 files changed, 55 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/90/41690/14
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41690 )
Change subject: mb/ocp/deltalake: Config PCH PCIe ports ......................................................................
Patch Set 14:
(2 comments)
https://review.coreboot.org/c/coreboot/+/41690/14/src/mainboard/ocp/deltalak... File src/mainboard/ocp/deltalake/romstage.c:
https://review.coreboot.org/c/coreboot/+/41690/14/src/mainboard/ocp/deltalak... PS14, Line 88: mupd->FspmConfig.PchPcieForceEnable[config->pch_pci_port[index].PortIndex] = line over 96 characters
https://review.coreboot.org/c/coreboot/+/41690/14/src/mainboard/ocp/deltalak... PS14, Line 90: mupd->FspmConfig.PchPciePortLinkSpeed[config->pch_pci_port[index].PortIndex] = line over 96 characters
Maxim Polyakov has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41690 )
Change subject: mb/ocp/deltalake: Config PCH PCIe ports ......................................................................
Patch Set 14: Code-Review+1
(7 comments)
What about the code for skx and tiogapass, do you make the same changes for this?
https://review.coreboot.org/c/coreboot/+/41690/14//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/41690/14//COMMIT_MSG@7 PS14, Line 7: mb/ocp/deltalake: Config PCH PCIe ports Maybe "mb/ocp/deltalake: Config PCH PCIe ports in devicetree" ?
https://review.coreboot.org/c/coreboot/+/41690/14/src/mainboard/ocp/deltalak... File src/mainboard/ocp/deltalake/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/41690/14/src/mainboard/ocp/deltalak... PS14, Line 43: 0 8
https://review.coreboot.org/c/coreboot/+/41690/14/src/mainboard/ocp/deltalak... PS14, Line 44: .PortIndex = 0x8 remove
https://review.coreboot.org/c/coreboot/+/41690/14/src/mainboard/ocp/deltalak... File src/mainboard/ocp/deltalake/romstage.c:
https://review.coreboot.org/c/coreboot/+/41690/14/src/mainboard/ocp/deltalak... PS14, Line 85: mupd->FspmConfig.PchPcieForceEnable[index] = 0; remove see comment below
https://review.coreboot.org/c/coreboot/+/41690/14/src/mainboard/ocp/deltalak... PS14, Line 88: mupd->FspmConfig.PchPcieForceEnable[config->pch_pci_port[index].PortIndex] = : config->pch_pci_port[index].ForceEnable; I think there is no need for port remapping. I suggest using here "mupd->FspmConfig.PchPcieForceEnable[index] = config->pch_pci_port[index].ForceEnable;" only and removing "PortIndex" option from "struct UPD_PCH_PCIE_PORT"
https://review.coreboot.org/c/coreboot/+/41690/14/src/mainboard/ocp/deltalak... PS14, Line 90: mupd->FspmConfig.PchPciePortLinkSpeed[config->pch_pci_port[index].PortIndex] = : config->pch_pci_port[index].PortLinkSpeed; same
https://review.coreboot.org/c/coreboot/+/41690/14/src/vendorcode/intel/fsp/f... File src/vendorcode/intel/fsp/fsp2_0/cooperlake_sp/FspmUpd.h:
https://review.coreboot.org/c/coreboot/+/41690/14/src/vendorcode/intel/fsp/f... PS14, Line 153: UINT8 PortIndex remove
Hello Philipp Deppenwiese, build bot (Jenkins), Maxim Polyakov, Jonathan Zhang, Christian Walter, Angel Pons, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/41690
to look at the new patch set (#15).
Change subject: mb/ocp/deltalake: Config PCH PCIe ports in devicetree ......................................................................
mb/ocp/deltalake: Config PCH PCIe ports in devicetree
Tested on OCP Delta Lake with lspci checking if PCIe speed is changed are expected.
Change-Id: I189027c403814d68db2b7c5f41fc254a293fe3a1 Signed-off-by: Morgan Jang Morgan_Jang@wiwynn.com --- M src/mainboard/ocp/deltalake/devicetree.cb M src/mainboard/ocp/deltalake/romstage.c M src/soc/intel/xeon_sp/cpx/chip.h M src/vendorcode/intel/fsp/fsp2_0/cooperlake_sp/FspmUpd.h 4 files changed, 53 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/90/41690/15
Morgan Jang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41690 )
Change subject: mb/ocp/deltalake: Config PCH PCIe ports in devicetree ......................................................................
Patch Set 15:
(7 comments)
https://review.coreboot.org/c/coreboot/+/41690/14//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/41690/14//COMMIT_MSG@7 PS14, Line 7: mb/ocp/deltalake: Config PCH PCIe ports
Maybe "mb/ocp/deltalake: Config PCH PCIe ports in devicetree" ?
Done
https://review.coreboot.org/c/coreboot/+/41690/14/src/mainboard/ocp/deltalak... File src/mainboard/ocp/deltalake/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/41690/14/src/mainboard/ocp/deltalak... PS14, Line 43: 0
8
Done
https://review.coreboot.org/c/coreboot/+/41690/14/src/mainboard/ocp/deltalak... PS14, Line 44: .PortIndex = 0x8
remove
Done
https://review.coreboot.org/c/coreboot/+/41690/14/src/mainboard/ocp/deltalak... File src/mainboard/ocp/deltalake/romstage.c:
https://review.coreboot.org/c/coreboot/+/41690/14/src/mainboard/ocp/deltalak... PS14, Line 85: mupd->FspmConfig.PchPcieForceEnable[index] = 0;
remove […]
Done
https://review.coreboot.org/c/coreboot/+/41690/14/src/mainboard/ocp/deltalak... PS14, Line 88: mupd->FspmConfig.PchPcieForceEnable[config->pch_pci_port[index].PortIndex] = : config->pch_pci_port[index].ForceEnable;
I think there is no need for port remapping. I suggest using here […]
Done
https://review.coreboot.org/c/coreboot/+/41690/14/src/mainboard/ocp/deltalak... PS14, Line 90: mupd->FspmConfig.PchPciePortLinkSpeed[config->pch_pci_port[index].PortIndex] = : config->pch_pci_port[index].PortLinkSpeed;
same
Done
https://review.coreboot.org/c/coreboot/+/41690/14/src/vendorcode/intel/fsp/f... File src/vendorcode/intel/fsp/fsp2_0/cooperlake_sp/FspmUpd.h:
https://review.coreboot.org/c/coreboot/+/41690/14/src/vendorcode/intel/fsp/f... PS14, Line 153: UINT8 PortIndex
remove
Done
Maxim Polyakov has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41690 )
Change subject: mb/ocp/deltalake: Config PCH PCIe ports in devicetree ......................................................................
Patch Set 15: Code-Review+2
(2 comments)
https://review.coreboot.org/c/coreboot/+/41690/14/src/mainboard/ocp/deltalak... File src/mainboard/ocp/deltalake/romstage.c:
https://review.coreboot.org/c/coreboot/+/41690/14/src/mainboard/ocp/deltalak... PS14, Line 85: mupd->FspmConfig.PchPcieForceEnable[index] = 0;
Done
Relevant to the last comment from Patchset 15 https://review.coreboot.org/c/coreboot/+/41690/15/src/mainboard/ocp/deltalak...
https://review.coreboot.org/c/coreboot/+/41690/15/src/mainboard/ocp/deltalak... File src/mainboard/ocp/deltalake/romstage.c:
https://review.coreboot.org/c/coreboot/+/41690/15/src/mainboard/ocp/deltalak... PS15, Line 83: : /* Set PchPcieForceEnable to disable by default */ : mupd->FspmConfig.PchPcieForceEnable[index] = 0; : : if (config->pch_pci_port[index].ForceEnable) { : mupd->FspmConfig.PchPcieForceEnable[index] = : config->pch_pci_port[index].ForceEnable; : mupd->FspmConfig.PchPciePortLinkSpeed[index] = : config->pch_pci_port[index].PortLinkSpeed; : } Use
mupd->FspmConfig.PchPcieForceEnable[index] = config->pch_pci_port[index].ForceEnable; mupd->FspmConfig.PchPciePortLinkSpeed[index] = config->pch_pci_port[index].PortLinkSpeed;
instead of this code.
ForceEnable is set to 0 if we don't set it in the devicetree. No need to take care of it here.
Maxim Polyakov has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41690 )
Change subject: mb/ocp/deltalake: Config PCH PCIe ports in devicetree ......................................................................
Patch Set 15:
(1 comment)
https://review.coreboot.org/c/coreboot/+/41690/15/src/vendorcode/intel/fsp/f... File src/vendorcode/intel/fsp/fsp2_0/cooperlake_sp/FspmUpd.h:
https://review.coreboot.org/c/coreboot/+/41690/15/src/vendorcode/intel/fsp/f... PS15, Line 147: PortIndex - PCH PCIe Port Index. : Valid Port Numbers are: 0 to 19. this comment should be removed
Maxim Polyakov has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41690 )
Change subject: mb/ocp/deltalake: Config PCH PCIe ports in devicetree ......................................................................
Patch Set 15:
(1 comment)
https://review.coreboot.org/c/coreboot/+/41690/15/src/vendorcode/intel/fsp/f... File src/vendorcode/intel/fsp/fsp2_0/cooperlake_sp/FspmUpd.h:
https://review.coreboot.org/c/coreboot/+/41690/15/src/vendorcode/intel/fsp/f... PS15, Line 149: Enable ForceEnable
Hello Philipp Deppenwiese, build bot (Jenkins), Maxim Polyakov, Jonathan Zhang, Christian Walter, Angel Pons, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/41690
to look at the new patch set (#16).
Change subject: mb/ocp/deltalake: Config PCH PCIe ports in devicetree ......................................................................
mb/ocp/deltalake: Config PCH PCIe ports in devicetree
Tested on OCP Delta Lake with lspci checking if PCIe speed is changed are expected.
Change-Id: I189027c403814d68db2b7c5f41fc254a293fe3a1 Signed-off-by: Morgan Jang Morgan_Jang@wiwynn.com --- M src/mainboard/ocp/deltalake/devicetree.cb M src/mainboard/ocp/deltalake/romstage.c M src/soc/intel/xeon_sp/cpx/chip.h M src/vendorcode/intel/fsp/fsp2_0/cooperlake_sp/FspmUpd.h 4 files changed, 46 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/90/41690/16
Maxim Polyakov has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41690 )
Change subject: mb/ocp/deltalake: Config PCH PCIe ports in devicetree ......................................................................
Patch Set 16:
(2 comments)
https://review.coreboot.org/c/coreboot/+/41690/16/src/mainboard/ocp/deltalak... File src/mainboard/ocp/deltalake/romstage.c:
https://review.coreboot.org/c/coreboot/+/41690/16/src/mainboard/ocp/deltalak... PS16, Line 84: remove tab
https://review.coreboot.org/c/coreboot/+/41690/16/src/mainboard/ocp/deltalak... PS16, Line 86: remove tab
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41690 )
Change subject: mb/ocp/deltalake: Config PCH PCIe ports in devicetree ......................................................................
Patch Set 16:
(1 comment)
https://review.coreboot.org/c/coreboot/+/41690/16/src/mainboard/ocp/deltalak... File src/mainboard/ocp/deltalake/romstage.c:
https://review.coreboot.org/c/coreboot/+/41690/16/src/mainboard/ocp/deltalak... PS16, Line 83: for (index = 0; index < MAX_PCH_PCIE_PORT; index++) { suspect code indent for conditional statements (8, 24)
Maxim Polyakov has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41690 )
Change subject: mb/ocp/deltalake: Config PCH PCIe ports in devicetree ......................................................................
Patch Set 16:
(2 comments)
https://review.coreboot.org/c/coreboot/+/41690/15/src/vendorcode/intel/fsp/f... File src/vendorcode/intel/fsp/fsp2_0/cooperlake_sp/FspmUpd.h:
https://review.coreboot.org/c/coreboot/+/41690/15/src/vendorcode/intel/fsp/f... PS15, Line 147: PortIndex - PCH PCIe Port Index. : Valid Port Numbers are: 0 to 19.
this comment should be removed
Done
https://review.coreboot.org/c/coreboot/+/41690/15/src/vendorcode/intel/fsp/f... PS15, Line 149: Enable
ForceEnable
Done
Maxim Polyakov has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41690 )
Change subject: mb/ocp/deltalake: Config PCH PCIe ports in devicetree ......................................................................
Patch Set 16:
(1 comment)
https://review.coreboot.org/c/coreboot/+/41690/15/src/mainboard/ocp/deltalak... File src/mainboard/ocp/deltalake/romstage.c:
https://review.coreboot.org/c/coreboot/+/41690/15/src/mainboard/ocp/deltalak... PS15, Line 83: : /* Set PchPcieForceEnable to disable by default */ : mupd->FspmConfig.PchPcieForceEnable[index] = 0; : : if (config->pch_pci_port[index].ForceEnable) { : mupd->FspmConfig.PchPcieForceEnable[index] = : config->pch_pci_port[index].ForceEnable; : mupd->FspmConfig.PchPciePortLinkSpeed[index] = : config->pch_pci_port[index].PortLinkSpeed; : }
Use […]
Done
Hello Philipp Deppenwiese, build bot (Jenkins), Maxim Polyakov, Jonathan Zhang, Christian Walter, Angel Pons, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/41690
to look at the new patch set (#17).
Change subject: mb/ocp/deltalake: Config PCH PCIe ports in devicetree ......................................................................
mb/ocp/deltalake: Config PCH PCIe ports in devicetree
Tested on OCP Delta Lake with lspci checking if PCIe speed is changed are expected.
Change-Id: I189027c403814d68db2b7c5f41fc254a293fe3a1 Signed-off-by: Morgan Jang Morgan_Jang@wiwynn.com --- M src/mainboard/ocp/deltalake/devicetree.cb M src/mainboard/ocp/deltalake/romstage.c M src/soc/intel/xeon_sp/cpx/chip.h M src/vendorcode/intel/fsp/fsp2_0/cooperlake_sp/FspmUpd.h 4 files changed, 46 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/90/41690/17
Maxim Polyakov has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41690 )
Change subject: mb/ocp/deltalake: Config PCH PCIe ports in devicetree ......................................................................
Patch Set 17:
(4 comments)
https://review.coreboot.org/c/coreboot/+/41690/16/src/mainboard/ocp/deltalak... File src/mainboard/ocp/deltalake/romstage.c:
https://review.coreboot.org/c/coreboot/+/41690/16/src/mainboard/ocp/deltalak... PS16, Line 84:
remove tab
Done
https://review.coreboot.org/c/coreboot/+/41690/16/src/mainboard/ocp/deltalak... PS16, Line 86:
remove tab
Done
https://review.coreboot.org/c/coreboot/+/41690/17/src/vendorcode/intel/fsp/f... File src/vendorcode/intel/fsp/fsp2_0/cooperlake_sp/FspmUpd.h:
https://review.coreboot.org/c/coreboot/+/41690/17/src/vendorcode/intel/fsp/f... PS17, Line 49: #define MAX_PCH_PCIE_PORT 20 see comment below
https://review.coreboot.org/c/coreboot/+/41690/17/src/vendorcode/intel/fsp/f... PS17, Line 145: /** : UPD_PCH_PCIE_PORT: : ForceEnable - Enable/Disable PCH PCIe port : PortLinkSpeed - Port Link Speed. Use PCIE_LINK_SPEED to set : **/ : struct UPD_PCH_PCIE_PORT { : UINT8 ForceEnable; : UINT8 PortLinkSpeed; : }; : : /** : PCIe Link Speed Selection : **/ : typedef enum { : PcieAuto = 0, : PcieGen1, : PcieGen2, : PcieGen3 : } PCIE_LINK_SPEED; Any reason to add this structure and enum to FspmUpd.h? if are not, this should be moved to src/soc/intel/xeon_sp/cpx/chip.h
Hello Philipp Deppenwiese, build bot (Jenkins), Maxim Polyakov, Jonathan Zhang, Christian Walter, Angel Pons, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/41690
to look at the new patch set (#18).
Change subject: mb/ocp/deltalake: Config PCH PCIe ports in devicetree ......................................................................
mb/ocp/deltalake: Config PCH PCIe ports in devicetree
Tested on OCP Delta Lake with lspci checking if PCIe speed is changed are expected.
Change-Id: I189027c403814d68db2b7c5f41fc254a293fe3a1 Signed-off-by: Morgan Jang Morgan_Jang@wiwynn.com --- M src/mainboard/ocp/deltalake/devicetree.cb M src/mainboard/ocp/deltalake/romstage.c M src/soc/intel/xeon_sp/cpx/chip.h 3 files changed, 46 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/90/41690/18
Maxim Polyakov has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41690 )
Change subject: mb/ocp/deltalake: Config PCH PCIe ports in devicetree ......................................................................
Patch Set 18: Code-Review+2
(4 comments)
Thanks. This is my last comment
https://review.coreboot.org/c/coreboot/+/41690/18/src/soc/intel/xeon_sp/cpx/... File src/soc/intel/xeon_sp/cpx/chip.h:
https://review.coreboot.org/c/coreboot/+/41690/18/src/soc/intel/xeon_sp/cpx/... PS18, Line 17: UPD_PCH_PCIE_PORT pch_pcie_port
since it is outside of the src/vendorcode/intel/... directory now, there is no need to use the edk2-style of the struct names.
https://review.coreboot.org/c/coreboot/+/41690/18/src/soc/intel/xeon_sp/cpx/... PS18, Line 30: PCIE_LINK_SPEED pcie_link_speed
https://review.coreboot.org/c/coreboot/+/41690/17/src/vendorcode/intel/fsp/f... File src/vendorcode/intel/fsp/fsp2_0/cooperlake_sp/FspmUpd.h:
https://review.coreboot.org/c/coreboot/+/41690/17/src/vendorcode/intel/fsp/f... PS17, Line 49: #define MAX_PCH_PCIE_PORT 20
see comment below
Done
https://review.coreboot.org/c/coreboot/+/41690/17/src/vendorcode/intel/fsp/f... PS17, Line 145: /** : UPD_PCH_PCIE_PORT: : ForceEnable - Enable/Disable PCH PCIe port : PortLinkSpeed - Port Link Speed. Use PCIE_LINK_SPEED to set : **/ : struct UPD_PCH_PCIE_PORT { : UINT8 ForceEnable; : UINT8 PortLinkSpeed; : }; : : /** : PCIe Link Speed Selection : **/ : typedef enum { : PcieAuto = 0, : PcieGen1, : PcieGen2, : PcieGen3 : } PCIE_LINK_SPEED;
Any reason to add this structure and enum to FspmUpd.h? […]
Ack
Hello Philipp Deppenwiese, build bot (Jenkins), Maxim Polyakov, Jonathan Zhang, Christian Walter, Angel Pons, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/41690
to look at the new patch set (#19).
Change subject: mb/ocp/deltalake: Config PCH PCIe ports in devicetree ......................................................................
mb/ocp/deltalake: Config PCH PCIe ports in devicetree
Tested on OCP Delta Lake with lspci checking if PCIe speed is changed are expected.
Change-Id: I189027c403814d68db2b7c5f41fc254a293fe3a1 Signed-off-by: Morgan Jang Morgan_Jang@wiwynn.com --- M src/mainboard/ocp/deltalake/devicetree.cb M src/mainboard/ocp/deltalake/romstage.c M src/soc/intel/xeon_sp/cpx/chip.h 3 files changed, 46 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/90/41690/19
Morgan Jang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41690 )
Change subject: mb/ocp/deltalake: Config PCH PCIe ports in devicetree ......................................................................
Patch Set 18:
Patch Set 18: Code-Review+2
(4 comments)
Thanks. This is my last comment
Thanks for reviewing.
Maxim Polyakov has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41690 )
Change subject: mb/ocp/deltalake: Config PCH PCIe ports in devicetree ......................................................................
Patch Set 19: Code-Review+2
Maxim Polyakov has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41690 )
Change subject: mb/ocp/deltalake: Config PCH PCIe ports in devicetree ......................................................................
Patch Set 19:
(2 comments)
https://review.coreboot.org/c/coreboot/+/41690/18/src/soc/intel/xeon_sp/cpx/... File src/soc/intel/xeon_sp/cpx/chip.h:
https://review.coreboot.org/c/coreboot/+/41690/18/src/soc/intel/xeon_sp/cpx/... PS18, Line 17: UPD_PCH_PCIE_PORT
pch_pcie_port […]
Done
https://review.coreboot.org/c/coreboot/+/41690/18/src/soc/intel/xeon_sp/cpx/... PS18, Line 30: PCIE_LINK_SPEED
pcie_link_speed
Done
Hello Philipp Deppenwiese, build bot (Jenkins), Maxim Polyakov, Jonathan Zhang, Christian Walter, Angel Pons, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/41690
to look at the new patch set (#20).
Change subject: mb/ocp/deltalake: Config PCH PCIe ports in devicetree ......................................................................
mb/ocp/deltalake: Config PCH PCIe ports in devicetree
Tested on OCP Delta Lake with lspci checking if PCIe speed is changed are expected.
Change-Id: I189027c403814d68db2b7c5f41fc254a293fe3a1 Signed-off-by: Morgan Jang Morgan_Jang@wiwynn.com --- M src/mainboard/ocp/deltalake/devicetree.cb M src/mainboard/ocp/deltalake/romstage.c M src/soc/intel/xeon_sp/cpx/chip.h 3 files changed, 46 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/90/41690/20
Maxim Polyakov has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41690 )
Change subject: mb/ocp/deltalake: Config PCH PCIe ports in devicetree ......................................................................
Patch Set 20: Code-Review+2
Philipp Deppenwiese has submitted this change. ( https://review.coreboot.org/c/coreboot/+/41690 )
Change subject: mb/ocp/deltalake: Config PCH PCIe ports in devicetree ......................................................................
mb/ocp/deltalake: Config PCH PCIe ports in devicetree
Tested on OCP Delta Lake with lspci checking if PCIe speed is changed are expected.
Change-Id: I189027c403814d68db2b7c5f41fc254a293fe3a1 Signed-off-by: Morgan Jang Morgan_Jang@wiwynn.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/41690 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Maxim Polyakov max.senia.poliak@gmail.com --- M src/mainboard/ocp/deltalake/devicetree.cb M src/mainboard/ocp/deltalake/romstage.c M src/soc/intel/xeon_sp/cpx/chip.h 3 files changed, 46 insertions(+), 0 deletions(-)
Approvals: build bot (Jenkins): Verified Maxim Polyakov: Looks good to me, approved
diff --git a/src/mainboard/ocp/deltalake/devicetree.cb b/src/mainboard/ocp/deltalake/devicetree.cb index f1b201d..cc17e06 100644 --- a/src/mainboard/ocp/deltalake/devicetree.cb +++ b/src/mainboard/ocp/deltalake/devicetree.cb @@ -39,6 +39,12 @@ register "gen1_dec" = "0x00fc0601" # BIC in-band update support register "gen2_dec" = "0x000c0ca1" # IPMI KCS
+ # configure PCH PCIe port + register "pch_pci_port[8]" = "{ + .ForceEnable = 0x1, + .PortLinkSpeed = PcieAuto, + }" + device cpu_cluster 0 on device lapic 0 on end end diff --git a/src/mainboard/ocp/deltalake/romstage.c b/src/mainboard/ocp/deltalake/romstage.c index 9ce06ad..2c8dc5e 100644 --- a/src/mainboard/ocp/deltalake/romstage.c +++ b/src/mainboard/ocp/deltalake/romstage.c @@ -7,6 +7,7 @@ #include <FspmUpd.h> #include <soc/romstage.h>
+#include "chip.h" #include "ipmi.h" #include "vpd.h"
@@ -74,7 +75,21 @@
static void mainboard_config_iio(FSPM_UPD *mupd) { + uint8_t index; + const config_t *config = config_of_soc(); + oem_update_iio(mupd); + + for (index = 0; index < MAX_PCH_PCIE_PORT; index++) { + mupd->FspmConfig.PchPcieForceEnable[index] = + config->pch_pci_port[index].ForceEnable; + mupd->FspmConfig.PchPciePortLinkSpeed[index] = + config->pch_pci_port[index].PortLinkSpeed; + } + + mupd->FspmConfig.PchPcieRootPortFunctionSwap = 0x00; + /* The default value is 0XFF in FSP, set it to 0xFE by platform */ + mupd->FspmConfig.PchPciePllSsc = 0xFE; }
void mainboard_memory_init_params(FSPM_UPD *mupd) diff --git a/src/soc/intel/xeon_sp/cpx/chip.h b/src/soc/intel/xeon_sp/cpx/chip.h index 61e806e..e46f34f 100644 --- a/src/soc/intel/xeon_sp/cpx/chip.h +++ b/src/soc/intel/xeon_sp/cpx/chip.h @@ -7,10 +7,35 @@ #include <soc/irq.h> #include <stdint.h>
+#define MAX_PCH_PCIE_PORT 20 + +/** + UPD_PCH_PCIE_PORT: + ForceEnable - Enable/Disable PCH PCIe port + PortLinkSpeed - Port Link Speed. Use PCIE_LINK_SPEED to set +**/ +struct pch_pcie_port { + uint8_t ForceEnable; + uint8_t PortLinkSpeed; +}; + +/** + PCIe Link Speed Selection + **/ +typedef enum { + PcieAuto = 0, + PcieGen1, + PcieGen2, + PcieGen3 +} pcie_link_speed; + struct soc_intel_xeon_sp_cpx_config { /* Common struct containing soc config data required by common code */ struct soc_intel_common_config common_soc_config;
+ /* Struct for configuring PCH PCIe port */ + struct pch_pcie_port pch_pci_port[MAX_PCH_PCIE_PORT]; + /** * Interrupt Routing configuration * If bit7 is 1, the interrupt is disabled.
-
Angel Pons (Code Review) -
build bot (Jenkins) (Code Review)
-
Christian Walter (Code Review)
-
Johnny Lin (Code Review)
-
Jonathan Zhang (Code Review)
-
Maxim Polyakov (Code Review)
-
Morgan Jang (Code Review)
-
Philipp Deppenwiese (Code Review)