Change in coreboot[master]: mb/purism/librem_skl: Fix CLKREQ for 15v3 NVMe

[Matt DeVillier (Code Review)]

Matt DeVillier has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/40950 )

Change subject: mb/purism/librem_skl: Fix CLKREQ for 15v3 NVMe
......................................................................

mb/purism/librem_skl: Fix CLKREQ for 15v3 NVMe

Per the schematics, SRCCLKREQ2# is used for the NVMe and should be enabled.
Enable CLKREQ for PCIe RP9 to match the comment, and just comment
to indicate correct value per schematic.

Test: build/boot Librem 15v3 with NVMe drive, verify drive identified
properly and no errors in boot log.

Signed-off-by: Matt DeVillier 
Change-Id: I159cb7ce1f5195d95c0229490c3bbde26edbd375
---
M src/mainboard/purism/librem_skl/variants/librem15v3/devicetree.cb
1 file changed, 2 insertions(+), 2 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/50/40950/1

diff --git a/src/mainboard/purism/librem_skl/variants/librem15v3/devicetree.cb b/src/mainboard/purism/librem_skl/variants/librem15v3/devicetree.cb
index 308688a..d9c4f14 100644
--- a/src/mainboard/purism/librem_skl/variants/librem15v3/devicetree.cb
+++ b/src/mainboard/purism/librem_skl/variants/librem15v3/devicetree.cb
@@ -164,8 +164,8 @@
    register "PcieRpEnable[4]" = "1"
    register "PcieRpEnable[8]" = "1"
    # Enable CLKREQ# for RP9
-	register "PcieRpClkReqSupport[8]" = "0"
-	# ClkReq for NVMe - Bruteforced (no other value works)
+	register "PcieRpClkReqSupport[8]" = "1"
+	# SRCCLKREQ2# for NVMe per schematic
    register "PcieRpClkReqNumber[8]" = "2"

register "usb2_ports[0]" = "USB2_PORT_TYPE_C(OC_SKIP)"	# Type-C Port

[Matt DeVillier (Code Review)]

Hello Nico Huber, Paul Menzel, Angel Pons,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/40950

to look at the new patch set (#2).

Change subject: mb/purism/librem_skl: Fix CLKREQ for 15v3 NVMe
......................................................................

mb/purism/librem_skl: Fix CLKREQ for 15v3 NVMe

Per the schematics, SRCCLKREQ2# is used for the NVMe and should be
enabled. Enable CLKREQ for PCIe RP9, and adjust comments to indicate
correct value used per schematic.

Test: build/boot Librem 15v3 with NVMe drive, verify drive identified
properly and no errors in boot log.

Signed-off-by: Matt DeVillier 
Change-Id: I159cb7ce1f5195d95c0229490c3bbde26edbd375
---
M src/mainboard/purism/librem_skl/variants/librem15v3/devicetree.cb
1 file changed, 2 insertions(+), 2 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/50/40950/2

[Michael Niewöhner (Code Review)]

Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40950 )

Change subject: mb/purism/librem_skl: Fix CLKREQ for 15v3 NVMe
......................................................................

Patch Set 2: Code-Review+1

[Nico Huber (Code Review)]

Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40950 )

Change subject: mb/purism/librem_skl: Fix CLKREQ for 15v3 NVMe
......................................................................

Patch Set 2: Code-Review+2

[Angel Pons (Code Review)]

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40950 )

Change subject: mb/purism/librem_skl: Fix CLKREQ for 15v3 NVMe
......................................................................

Patch Set 2: Code-Review+2

[Paul Menzel (Code Review)]

Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40950 )

Change subject: mb/purism/librem_skl: Fix CLKREQ for 15v3 NVMe
......................................................................

Patch Set 2:

(1 comment)

https://review.coreboot.org/c/coreboot/+/40950/2//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/coreboot/+/40950/2//COMMIT_MSG@14 
PS2, Line 14: properly and no errors in boot log.
So before there were problems? Please cite one of these errors, so users with such an error may search with that.

[Matt DeVillier (Code Review)]

Matt DeVillier has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40950 )

Change subject: mb/purism/librem_skl: Fix CLKREQ for 15v3 NVMe
......................................................................

Patch Set 2:

(1 comment)

https://review.coreboot.org/c/coreboot/+/40950/2//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/coreboot/+/40950/2//COMMIT_MSG@14 
PS2, Line 14: properly and no errors in boot log.
> So before there were problems? Please cite one of these errors, so users with such an error may sear […]

any errors that might have occurred were early on when this board was initially ported without schematics, and the proper PCIe SRCCLK was not known, hence the trial and error in the original comment. The test comment here only serves to indicate that there were no issues with the change, enabling SRCCLK usage for the NVMe drive

[Paul Menzel (Code Review)]

Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40950 )

Change subject: mb/purism/librem_skl: Fix CLKREQ for 15v3 NVMe
......................................................................

Patch Set 2:

(1 comment)

https://review.coreboot.org/c/coreboot/+/40950/2//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/coreboot/+/40950/2//COMMIT_MSG@14 
PS2, Line 14: properly and no errors in boot log.
> any errors that might have occurred were early on when this board was initially ported without schem […]

Ack

[Patrick Georgi (Code Review)]

Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/40950 )

Change subject: mb/purism/librem_skl: Fix CLKREQ for 15v3 NVMe
......................................................................

mb/purism/librem_skl: Fix CLKREQ for 15v3 NVMe

Per the schematics, SRCCLKREQ2# is used for the NVMe and should be
enabled. Enable CLKREQ for PCIe RP9, and adjust comments to indicate
correct value used per schematic.

Test: build/boot Librem 15v3 with NVMe drive, verify drive identified
properly and no errors in boot log.

Signed-off-by: Matt DeVillier 
Change-Id: I159cb7ce1f5195d95c0229490c3bbde26edbd375
Reviewed-on: https://review.coreboot.org/c/coreboot/+/40950
Tested-by: build bot (Jenkins) 
Reviewed-by: Michael Niewöhner
Reviewed-by: Nico Huber 
Reviewed-by: Angel Pons 
---
M src/mainboard/purism/librem_skl/variants/librem15v3/devicetree.cb
1 file changed, 2 insertions(+), 2 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Nico Huber: Looks good to me, approved
  Angel Pons: Looks good to me, approved
  Michael Niewöhner: Looks good to me, but someone else must approve

diff --git a/src/mainboard/purism/librem_skl/variants/librem15v3/devicetree.cb b/src/mainboard/purism/librem_skl/variants/librem15v3/devicetree.cb
index d273462..ceeeb43 100644
--- a/src/mainboard/purism/librem_skl/variants/librem15v3/devicetree.cb
+++ b/src/mainboard/purism/librem_skl/variants/librem15v3/devicetree.cb
@@ -167,8 +167,8 @@
    register "PcieRpEnable[4]" = "1"
    register "PcieRpEnable[8]" = "1"
    # Enable CLKREQ# for RP9
-	register "PcieRpClkReqSupport[8]" = "0"
-	# ClkReq for NVMe - Bruteforced (no other value works)
+	register "PcieRpClkReqSupport[8]" = "1"
+	# SRCCLKREQ2# for NVMe per schematic
    register "PcieRpClkReqNumber[8]" = "2"

register "usb2_ports[0]" = "USB2_PORT_TYPE_C(OC_SKIP)"	# Type-C Port

[9elements QA (Code Review)]

9elements QA has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40950 )

Change subject: mb/purism/librem_skl: Fix CLKREQ for 15v3 NVMe
......................................................................

Patch Set 3:

Automatic boot test returned (PASS/FAIL/TOTAL): 4/0/4
Emulation targets:
"QEMU x86 q35/ich9" using payload TianoCore : SUCCESS : https://lava.9esec.io/r/3187
"QEMU x86 q35/ich9" using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/3186
"QEMU x86 i440fx/piix4" using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/3185
"QEMU AArch64" using payload LinuxBoot_u-root_kexec : SUCCESS : https://lava.9esec.io/r/3184

Please note: This test is under development and might not be accurate at all!