Change in ...coreboot[master]: mb/intel/../../cml_u: Override LPSS related FSP UPD for CMLRVP
Hello Subrata Banik,
I'd like you to do a code review. Please visit
https://review.coreboot.org/c/coreboot/+/31285
to review the following change.
Change subject: mb/intel/../../cml_u: Override LPSS related FSP UPD for CMLRVP ......................................................................
mb/intel/../../cml_u: Override LPSS related FSP UPD for CMLRVP
This patch overrides required LPSS FSP UPDs for CMLRVP from devicetree.cb
Change-Id: I82e3323df952762e2d9c14f1e3cfa75872ccc9b4 Signed-off-by: Subrata Banik subrata.banik@intel.com --- M src/mainboard/intel/coffeelake_rvp/variants/cml_u/devicetree.cb 1 file changed, 21 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/85/31285/1
diff --git a/src/mainboard/intel/coffeelake_rvp/variants/cml_u/devicetree.cb b/src/mainboard/intel/coffeelake_rvp/variants/cml_u/devicetree.cb index e30da3a..a810117 100644 --- a/src/mainboard/intel/coffeelake_rvp/variants/cml_u/devicetree.cb +++ b/src/mainboard/intel/coffeelake_rvp/variants/cml_u/devicetree.cb @@ -71,6 +71,27 @@ register "PcieClkSrcClkReq[4]" = "4" register "PcieClkSrcClkReq[5]" = "5"
+ register "SerialIoI2cMode" = "{ + [PchSerialIoIndexI2C0] = PchSerialIoPci, + [PchSerialIoIndexI2C1] = PchSerialIoPci, + [PchSerialIoIndexI2C2] = PchSerialIoPci, + [PchSerialIoIndexI2C3] = PchSerialIoPci, + [PchSerialIoIndexI2C4] = PchSerialIoDisabled, + [PchSerialIoIndexI2C5] = PchSerialIoPci, + }" + + register "SerialIoGSpiMode" = "{ + [PchSerialIoIndexGSPI0] = PchSerialIoPci, + [PchSerialIoIndexGSPI1] = PchSerialIoPci, + [PchSerialIoIndexGSPI2] = PchSerialIoDisabled, + }" + + register "SerialIoUartMode" = "{ + [PchSerialIoIndexUART0] = PchSerialIoSkipInit, + [PchSerialIoIndexUART1] = PchSerialIoDisabled, + [PchSerialIoIndexUART2] = PchSerialIoSkipInit, + }" + # Enable "Intel Speed Shift Technology" register "speed_shift_enable" = "1"
Hello Subrata Banik, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/31285
to look at the new patch set (#2).
Change subject: mb/intel/../../cml_u: Override LPSS related FSP UPD for CMLRVP ......................................................................
mb/intel/../../cml_u: Override LPSS related FSP UPD for CMLRVP
This patch overrides required LPSS FSP UPDs for CMLRVP from devicetree.cb File devicetree-override.cb will override required UPDs and is only applicable to CML soc for now
Change-Id: I82e3323df952762e2d9c14f1e3cfa75872ccc9b4 Signed-off-by: Subrata Banik subrata.banik@intel.com Signed-off-by: Maulik V Vaghela maulik.v.vaghela@intel.com --- M src/mainboard/intel/coffeelake_rvp/Kconfig A src/mainboard/intel/coffeelake_rvp/variants/cml_u/devicetree-override.cb 2 files changed, 27 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/85/31285/2
Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31285 )
Change subject: mb/intel/../../cml_u: Override LPSS related FSP UPD for CMLRVP ......................................................................
Patch Set 2: Code-Review+1
(1 comment)
https://review.coreboot.org/#/c/31285/2/src/mainboard/intel/coffeelake_rvp/v... File src/mainboard/intel/coffeelake_rvp/variants/cml_u/devicetree-override.cb:
https://review.coreboot.org/#/c/31285/2/src/mainboard/intel/coffeelake_rvp/v... PS2, Line 7: [PchSerialIoIndexI2C3] = PchSerialIoPci, please use tabs instead of space
Aamir Bohra has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31285 )
Change subject: mb/intel/../../cml_u: Override LPSS related FSP UPD for CMLRVP ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/#/c/31285/2/src/mainboard/intel/coffeelake_rvp/K... File src/mainboard/intel/coffeelake_rvp/Kconfig:
https://review.coreboot.org/#/c/31285/2/src/mainboard/intel/coffeelake_rvp/K... PS2, Line 25: OVERRIDE_DEVICETREE The devicetree configured currently in Kconfig is variant specific. why are we not making the changes in devicetree.cb?
Rizwan Qureshi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31285 )
Change subject: mb/intel/../../cml_u: Override LPSS related FSP UPD for CMLRVP ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/#/c/31285/2/src/mainboard/intel/coffeelake_rvp/K... File src/mainboard/intel/coffeelake_rvp/Kconfig:
https://review.coreboot.org/#/c/31285/2/src/mainboard/intel/coffeelake_rvp/K... PS2, Line 25: OVERRIDE_DEVICETREE
The devicetree configured currently in Kconfig is variant specific. […]
Ideally this board should be used with CML SoC, since we do not have the SoC yet we will use the WHL SoC. Since there are differences in the FSP UPDs between CML and CNL and that has a bearing on the devicetree.cb depending upon the SoC used this was added to isolation. Once we have the CML SoC available we will select the CML specific config and this devicetree along with related FSP UPD changes should get picked.
Aamir Bohra has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31285 )
Change subject: mb/intel/../../cml_u: Override LPSS related FSP UPD for CMLRVP ......................................................................
Patch Set 2: Code-Review+1
(1 comment)
https://review.coreboot.org/#/c/31285/2/src/mainboard/intel/coffeelake_rvp/K... File src/mainboard/intel/coffeelake_rvp/Kconfig:
https://review.coreboot.org/#/c/31285/2/src/mainboard/intel/coffeelake_rvp/K... PS2, Line 25: OVERRIDE_DEVICETREE
Ideally this board should be used with CML SoC, since we do not have the SoC yet we will use the WHL […]
Ok. Understood
Hello Naresh Solanki, Aaron Durbin, Subrata Banik, Aamir Bohra, Patrick Rudolph, Duncan Laurie, Rizwan Qureshi, Shelley Chen, V Sowmya, build bot (Jenkins), Furquan Shaikh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/31285
to look at the new patch set (#5).
Change subject: mb/intel/../../cml_u: Override LPSS related FSP UPD for CMLRVP ......................................................................
mb/intel/../../cml_u: Override LPSS related FSP UPD for CMLRVP
This patch overrides required LPSS FSP UPDs for CMLRVP from devicetree.cb File devicetree-override.cb will override required UPDs and is only applicable to CML soc for now
Change-Id: I82e3323df952762e2d9c14f1e3cfa75872ccc9b4 Signed-off-by: Subrata Banik subrata.banik@intel.com Signed-off-by: Maulik V Vaghela maulik.v.vaghela@intel.com --- M src/mainboard/intel/coffeelake_rvp/Kconfig A src/mainboard/intel/coffeelake_rvp/variants/cml_u/devicetree-override.cb 2 files changed, 27 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/85/31285/5
Maulik V Vaghela has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31285 )
Change subject: mb/intel/../../cml_u: Override LPSS related FSP UPD for CMLRVP ......................................................................
Patch Set 5:
Patch Set 4:
looks like override mechanism is not working. debug in progress
It was file name issue. I have verified, its working with patch set 5.
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31285 )
Change subject: mb/intel/../../cml_u: Override LPSS related FSP UPD for CMLRVP ......................................................................
Patch Set 5:
(1 comment)
https://review.coreboot.org/#/c/31285/5//COMMIT_MSG Commit Message:
https://review.coreboot.org/#/c/31285/5//COMMIT_MSG@12 PS5, Line 12: What values are overridden, and why?
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31285 )
Change subject: mb/intel/../../cml_u: Override LPSS related FSP UPD for CMLRVP ......................................................................
Patch Set 5:
(1 comment)
https://review.coreboot.org/#/c/31285/5/src/mainboard/intel/coffeelake_rvp/K... File src/mainboard/intel/coffeelake_rvp/Kconfig:
https://review.coreboot.org/#/c/31285/5/src/mainboard/intel/coffeelake_rvp/K... PS5, Line 25: OVERRIDE_DEVICETREE What is the point of OVERRIDE_DEVICETREE if: 1. It is used by CMLRVP only and 2. Devicetree for CMLRVP is independent of baseboard devicetree?
Hello Naresh Solanki, Aaron Durbin, Subrata Banik, Aamir Bohra, Patrick Rudolph, Duncan Laurie, Rizwan Qureshi, Shelley Chen, V Sowmya, build bot (Jenkins), Furquan Shaikh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/31285
to look at the new patch set (#6).
Change subject: mb/intel/../../cml_u: Override LPSS related FSP UPD for CMLRVP ......................................................................
mb/intel/../../cml_u: Override LPSS related FSP UPD for CMLRVP
This patch overrides required LPSS FSP UPDs for CMLRVP from devicetree.cb File devicetree-override.cb will override required UPDs and is only applicable to CML soc for now
Change-Id: I82e3323df952762e2d9c14f1e3cfa75872ccc9b4 Signed-off-by: Subrata Banik subrata.banik@intel.com Signed-off-by: Maulik V Vaghela maulik.v.vaghela@intel.com --- M src/mainboard/intel/coffeelake_rvp/variants/cml_u/devicetree.cb 1 file changed, 21 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/85/31285/6
Maulik V Vaghela has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31285 )
Change subject: mb/intel/../../cml_u: Override LPSS related FSP UPD for CMLRVP ......................................................................
Patch Set 6:
(1 comment)
https://review.coreboot.org/#/c/31285/5/src/mainboard/intel/coffeelake_rvp/K... File src/mainboard/intel/coffeelake_rvp/Kconfig:
https://review.coreboot.org/#/c/31285/5/src/mainboard/intel/coffeelake_rvp/K... PS5, Line 25: OVERRIDE_DEVICETREE
What is the point of OVERRIDE_DEVICETREE if: […]
I had added override file because FSP headers were not available and we needed to skip those configs. I have removed override since FSP headers are available in vendorcode. I can check what are exact differences for CMLRVP and check if we can have override mechanism
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31285 )
Change subject: mb/intel/../../cml_u: Override LPSS related FSP UPD for CMLRVP ......................................................................
Patch Set 5:
(1 comment)
https://review.coreboot.org/#/c/31285/5/src/mainboard/intel/coffeelake_rvp/K... File src/mainboard/intel/coffeelake_rvp/Kconfig:
https://review.coreboot.org/#/c/31285/5/src/mainboard/intel/coffeelake_rvp/K... PS5, Line 25: OVERRIDE_DEVICETREE
What is the point of OVERRIDE_DEVICETREE if: […]
Agree with furquan, override model might not applicable for cml_rvp. hence we can get rid of that
Maulik V Vaghela has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31285 )
Change subject: mb/intel/../../cml_u: Override LPSS related FSP UPD for CMLRVP ......................................................................
Patch Set 8:
(1 comment)
https://review.coreboot.org/#/c/31285/5/src/mainboard/intel/coffeelake_rvp/K... File src/mainboard/intel/coffeelake_rvp/Kconfig:
https://review.coreboot.org/#/c/31285/5/src/mainboard/intel/coffeelake_rvp/K... PS5, Line 25: OVERRIDE_DEVICETREE
Agree with furquan, override model might not applicable for cml_rvp. […]
Yes, I have removed this in latest pathset. We needed it to avoid compilation issues due to FSP headers but since we have headers now, we can put everything under devicetree.cb
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31285 )
Change subject: mb/intel/../../cml_u: Override LPSS related FSP UPD for CMLRVP ......................................................................
Patch Set 8: Code-Review+2
Hello Naresh Solanki, Aaron Durbin, Subrata Banik, Aamir Bohra, Patrick Rudolph, Duncan Laurie, Rizwan Qureshi, Shelley Chen, V Sowmya, build bot (Jenkins), Furquan Shaikh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/31285
to look at the new patch set (#12).
Change subject: mb/intel/../../cml_u: Override LPSS related FSP UPD for CMLRVP ......................................................................
mb/intel/../../cml_u: Override LPSS related FSP UPD for CMLRVP
This patch overrides required LPSS FSP UPDs for CMLRVP from devicetree.cb File devicetree-override.cb will override required UPDs and is only applicable to CML soc for now
Change-Id: I82e3323df952762e2d9c14f1e3cfa75872ccc9b4 Signed-off-by: Subrata Banik subrata.banik@intel.com Signed-off-by: Maulik V Vaghela maulik.v.vaghela@intel.com --- M src/mainboard/intel/coffeelake_rvp/variants/cml_u/devicetree.cb 1 file changed, 16 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/85/31285/12
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31285 )
Change subject: mb/intel/../../cml_u: Override LPSS related FSP UPD for CMLRVP ......................................................................
Patch Set 16: Code-Review+2
Subrata Banik has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/31285 )
Change subject: mb/intel/../../cml_u: Override LPSS related FSP UPD for CMLRVP ......................................................................
mb/intel/../../cml_u: Override LPSS related FSP UPD for CMLRVP
This patch overrides required LPSS FSP UPDs for CMLRVP from devicetree.cb File devicetree-override.cb will override required UPDs and is only applicable to CML soc for now
Change-Id: I82e3323df952762e2d9c14f1e3cfa75872ccc9b4 Signed-off-by: Subrata Banik subrata.banik@intel.com Signed-off-by: Maulik V Vaghela maulik.v.vaghela@intel.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/31285 Reviewed-by: Furquan Shaikh furquan@google.com Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/mainboard/intel/coffeelake_rvp/variants/cml_u/devicetree.cb 1 file changed, 16 insertions(+), 0 deletions(-)
Approvals: build bot (Jenkins): Verified Furquan Shaikh: Looks good to me, approved
diff --git a/src/mainboard/intel/coffeelake_rvp/variants/cml_u/devicetree.cb b/src/mainboard/intel/coffeelake_rvp/variants/cml_u/devicetree.cb index e30da3a..6484330 100644 --- a/src/mainboard/intel/coffeelake_rvp/variants/cml_u/devicetree.cb +++ b/src/mainboard/intel/coffeelake_rvp/variants/cml_u/devicetree.cb @@ -8,6 +8,22 @@ register "SaGv" = "SaGv_Enabled" register "ScsEmmcHs400Enabled" = "1" register "HeciEnabled" = "1" + register "s0ix_enable" = "1" + + register "SerialIoDevMode" = "{ + [PchSerialIoIndexI2C0] = PchSerialIoPci, + [PchSerialIoIndexI2C1] = PchSerialIoPci, + [PchSerialIoIndexI2C2] = PchSerialIoPci, + [PchSerialIoIndexI2C3] = PchSerialIoPci, + [PchSerialIoIndexI2C4] = PchSerialIoDisabled, + [PchSerialIoIndexI2C5] = PchSerialIoPci, + [PchSerialIoIndexSPI0] = PchSerialIoPci, + [PchSerialIoIndexSPI1] = PchSerialIoPci, + [PchSerialIoIndexSPI2] = PchSerialIoDisabled, + [PchSerialIoIndexUART0] = PchSerialIoSkipInit, + [PchSerialIoIndexUART1] = PchSerialIoDisabled, + [PchSerialIoIndexUART2] = PchSerialIoSkipInit, + }"
register "usb2_ports[0]" = "USB2_PORT_TYPE_C(OC0)" register "usb2_ports[1]" = "USB2_PORT_MID(OC0)"
-
Aamir Bohra (Code Review) -
Furquan Shaikh (Code Review)
-
Maulik V Vaghela (Code Review)
-
Patrick Rudolph (Code Review)
-
Paul Menzel (Code Review)
-
Rizwan Qureshi (Code Review)
-
Subrata Banik (Code Review)