Subrata Banik has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/48080 )
Change subject: mainboard/intel/adlrvp: Enable PCH PCIE device over x1 slot ......................................................................
mainboard/intel/adlrvp: Enable PCH PCIE device over x1 slot
List of changes: 1. Enable Root Port 8 aka 0:0x1c:7 2. Assign free running clock for RP8 3. Driven OEB 7:GPP_A7 and OEB 6:GPP_E5 low
TEST=Able to detect PCIE SD card over x1 slot localhost ~ # dmesg | grep mmc [ 3.643755] mmc0: SDHCI controller on PCI [0000:02:00.0] using ADMA [ 3.825201] mmc0: new ultra high speed DDR50 SDHC card at address 17f8 [ 3.835452] mmcblk0: mmc0:17f8 SE16G 14.4 GiB [ 3.849158] mmcblk0: p1
Change-Id: Ibea37b8de4dd020ff0108ec90ea6f8bcfaa4fb17 Signed-off-by: Subrata Banik subrata.banik@intel.com --- M src/mainboard/intel/adlrvp/devicetree.cb M src/mainboard/intel/adlrvp/gpio.c 2 files changed, 11 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/80/48080/1
diff --git a/src/mainboard/intel/adlrvp/devicetree.cb b/src/mainboard/intel/adlrvp/devicetree.cb index 80edb92..f2f768a 100644 --- a/src/mainboard/intel/adlrvp/devicetree.cb +++ b/src/mainboard/intel/adlrvp/devicetree.cb @@ -51,6 +51,12 @@ register "PcieClkSrcUsage[5]" = "0x5" register "PcieRpClkReqDetect[5]" = "1"
+ # Enable PCH PCIE RP 8 using free running CLK (0x80) + register "PcieRpEnable[7]" = "1" + register "PcieClkSrcClkReq[7]" = "7" + register "PcieClkSrcUsage[7]" = "0x80" + register "PcieRpClkReqDetect[7]" = "1" + # Enable PCH PCIE RP 9 using CLK 1 register "PcieRpEnable[8]" = "1" register "PcieClkSrcClkReq[1]" = "1" @@ -245,7 +251,7 @@ device pci 1c.4 on end # RP5 device pci 1c.5 on end # RP6 device pci 1c.6 off end # RP7 - device pci 1c.7 off end # RP8 + device pci 1c.7 on end # RP8 device pci 1d.0 on end # RP9 device pci 1d.1 off end # RP10 device pci 1d.2 on end # RP11 diff --git a/src/mainboard/intel/adlrvp/gpio.c b/src/mainboard/intel/adlrvp/gpio.c index 89e6f58..4cb8c3a 100644 --- a/src/mainboard/intel/adlrvp/gpio.c +++ b/src/mainboard/intel/adlrvp/gpio.c @@ -72,6 +72,10 @@ PAD_CFG_GPO(GPP_B4, 1, PLTRST), /* M.2_PCH_SSD_PWREN */ PAD_CFG_GPO(GPP_D16, 1, PLTRST), + /* SRCCLK_OEB7 */ + PAD_CFG_GPO(GPP_A7, 0, PLTRST), + /* SRCCLK_OEB6 */ + PAD_CFG_GPO(GPP_E5, 0, PLTRST),
/* M.2_SSD_PDET_R */ PAD_CFG_NF(GPP_A12, NONE, DEEP, NF1),
Hello build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/48080
to look at the new patch set (#2).
Change subject: mainboard/intel/adlrvp: Enable PCH PCIE device over x1 slot ......................................................................
mainboard/intel/adlrvp: Enable PCH PCIE device over x1 slot
List of changes: 1. Enable Root Port 8 aka 0:0x1c:7 2. Assign free running clock for RP8 3. Driven OEB 7:GPP_A7 and OEB 6:GPP_E5 low
TEST=Able to detect PCIE SD card over x1 slot localhost ~ # dmesg | grep mmc [ 3.643755] mmc0: SDHCI controller on PCI [0000:02:00.0] using ADMA [ 3.825201] mmc0: new ultra high speed DDR50 SDHC card at address 17f8 [ 3.835452] mmcblk0: mmc0:17f8 SE16G 14.4 GiB [ 3.849158] mmcblk0: p1
Change-Id: Ibea37b8de4dd020ff0108ec90ea6f8bcfaa4fb17 Signed-off-by: Subrata Banik subrata.banik@intel.com --- M src/mainboard/intel/adlrvp/devicetree.cb M src/mainboard/intel/adlrvp/gpio.c 2 files changed, 11 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/80/48080/2
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48080 )
Change subject: mainboard/intel/adlrvp: Enable PCH PCIE device over x1 slot ......................................................................
Patch Set 3:
Kindly review this CL as well 😊
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48080 )
Change subject: mainboard/intel/adlrvp: Enable PCH PCIE device over x1 slot ......................................................................
Patch Set 3: Code-Review+2
(3 comments)
https://review.coreboot.org/c/coreboot/+/48080/3//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/48080/3//COMMIT_MSG@7 PS3, Line 7: PCIE PCIe
https://review.coreboot.org/c/coreboot/+/48080/3//COMMIT_MSG@12 PS3, Line 12: Driven Drive
https://review.coreboot.org/c/coreboot/+/48080/3//COMMIT_MSG@14 PS3, Line 14: PCIE PCIe
V Sowmya has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48080 )
Change subject: mainboard/intel/adlrvp: Enable PCH PCIE device over x1 slot ......................................................................
Patch Set 3: Code-Review+2
Hello V Sowmya, build bot (Jenkins), Furquan Shaikh, Tim Wawrzynczak, Angel Pons, Aamir Bohra,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/48080
to look at the new patch set (#4).
Change subject: mainboard/intel/adlrvp: Enable PCH PCIe device over x1 slot ......................................................................
mainboard/intel/adlrvp: Enable PCH PCIe device over x1 slot
List of changes: 1. Enable Root Port 8 aka 0:0x1c:7 2. Assign free running clock for RP8 3. Drive OEB 7:GPP_A7 and OEB 6:GPP_E5 low
TEST=Able to detect PCIe SD card over x1 slot localhost ~ # dmesg | grep mmc [ 3.643755] mmc0: SDHCI controller on PCI [0000:02:00.0] using ADMA [ 3.825201] mmc0: new ultra high speed DDR50 SDHC card at address 17f8 [ 3.835452] mmcblk0: mmc0:17f8 SE16G 14.4 GiB [ 3.849158] mmcblk0: p1
Change-Id: Ibea37b8de4dd020ff0108ec90ea6f8bcfaa4fb17 Signed-off-by: Subrata Banik subrata.banik@intel.com --- M src/mainboard/intel/adlrvp/devicetree.cb M src/mainboard/intel/adlrvp/gpio.c 2 files changed, 11 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/80/48080/4
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48080 )
Change subject: mainboard/intel/adlrvp: Enable PCH PCIe device over x1 slot ......................................................................
Patch Set 4:
(3 comments)
https://review.coreboot.org/c/coreboot/+/48080/3//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/48080/3//COMMIT_MSG@7 PS3, Line 7: PCIE
PCIe
Ack
https://review.coreboot.org/c/coreboot/+/48080/3//COMMIT_MSG@12 PS3, Line 12: Driven
Drive
Ack
https://review.coreboot.org/c/coreboot/+/48080/3//COMMIT_MSG@14 PS3, Line 14: PCIE
PCIe
Ack
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48080 )
Change subject: mainboard/intel/adlrvp: Enable PCH PCIe device over x1 slot ......................................................................
Patch Set 4:
(4 comments)
https://review.coreboot.org/c/coreboot/+/48080/4//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/48080/4//COMMIT_MSG@12 PS4, Line 12: Drive OEB 7:GPP_A7 and OEB 6:GPP_E5 low Why are these being driven low? Are these the CLK pins?
https://review.coreboot.org/c/coreboot/+/48080/4/src/mainboard/intel/adlrvp/... File src/mainboard/intel/adlrvp/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/48080/4/src/mainboard/intel/adlrvp/... PS4, Line 54: free running CLK Just for my education: What is a free running CLK?
https://review.coreboot.org/c/coreboot/+/48080/4/src/mainboard/intel/adlrvp/... File src/mainboard/intel/adlrvp/gpio.c:
https://review.coreboot.org/c/coreboot/+/48080/4/src/mainboard/intel/adlrvp/... PS4, Line 76: PAD_CFG_GPO Shouldn't this be NF?
https://review.coreboot.org/c/coreboot/+/48080/4/src/mainboard/intel/adlrvp/... PS4, Line 78: PAD_CFG_GPO Same here.
Hello V Sowmya, build bot (Jenkins), Furquan Shaikh, Tim Wawrzynczak, Angel Pons, Aamir Bohra,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/48080
to look at the new patch set (#5).
Change subject: mainboard/intel/adlrvp: Enable PCH PCIe device over x1 slot ......................................................................
mainboard/intel/adlrvp: Enable PCH PCIe device over x1 slot
List of changes: 1. Enable Root Port 8 aka 0:0x1c:7 2. Assign free running clock for RP8 3. Apply W/A to get card detected on x1 slot - Drive OEB 7:GPP_A7 and OEB 6:GPP_E5 low
TEST=Able to detect PCIe SD card over x1 slot localhost ~ # dmesg | grep mmc [ 3.643755] mmc0: SDHCI controller on PCI [0000:02:00.0] using ADMA [ 3.825201] mmc0: new ultra high speed DDR50 SDHC card at address 17f8 [ 3.835452] mmcblk0: mmc0:17f8 SE16G 14.4 GiB [ 3.849158] mmcblk0: p1
Change-Id: Ibea37b8de4dd020ff0108ec90ea6f8bcfaa4fb17 Signed-off-by: Subrata Banik subrata.banik@intel.com --- M src/mainboard/intel/adlrvp/devicetree.cb M src/mainboard/intel/adlrvp/gpio.c 2 files changed, 11 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/80/48080/5
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48080 )
Change subject: mainboard/intel/adlrvp: Enable PCH PCIe device over x1 slot ......................................................................
Patch Set 5:
(4 comments)
https://review.coreboot.org/c/coreboot/+/48080/4//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/48080/4//COMMIT_MSG@12 PS4, Line 12: Drive OEB 7:GPP_A7 and OEB 6:GPP_E5 low
Why are these being driven low? Are these the CLK pins?
As per HW team recommendation, this PINs should drive low to get card detected on x1 DT slot
https://review.coreboot.org/c/coreboot/+/48080/4/src/mainboard/intel/adlrvp/... File src/mainboard/intel/adlrvp/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/48080/4/src/mainboard/intel/adlrvp/... PS4, Line 54: free running CLK
Just for my education: What is a free running CLK?
RP8 has CLK6 as source and CLK7 as req. Hence we had asked to configure CLK programming as free running to get the right CLK assigned between CLK6/7.
https://review.coreboot.org/c/coreboot/+/48080/4/src/mainboard/intel/adlrvp/... File src/mainboard/intel/adlrvp/gpio.c:
https://review.coreboot.org/c/coreboot/+/48080/4/src/mainboard/intel/adlrvp/... PS4, Line 76: PAD_CFG_GPO
Shouldn't this be NF?
Yes you are right, ideally this has to be NF1 but this is a recommendation/W/A on RVP where we need to drive those signal low to get the card detected
https://review.coreboot.org/c/coreboot/+/48080/4/src/mainboard/intel/adlrvp/... PS4, Line 78: PAD_CFG_GPO
Same here.
-same-
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48080 )
Change subject: mainboard/intel/adlrvp: Enable PCH PCIe device over x1 slot ......................................................................
Patch Set 5:
(2 comments)
https://review.coreboot.org/c/coreboot/+/48080/4//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/48080/4//COMMIT_MSG@12 PS4, Line 12: Drive OEB 7:GPP_A7 and OEB 6:GPP_E5 low
As per HW team recommendation, this PINs should drive low to get card detected on x1 DT slot
I think it would be good to add a comment to indicate that this is a RVP specific requirement so that it doesn't get copied over to other boards.
https://review.coreboot.org/c/coreboot/+/48080/4/src/mainboard/intel/adlrvp/... File src/mainboard/intel/adlrvp/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/48080/4/src/mainboard/intel/adlrvp/... PS4, Line 54: free running CLK
RP8 has CLK6 as source and CLK7 as req. […]
I still don't understand this. Why not set PcieClkSrcUsage[5]="7" so that CLK SRC 6 is configured for RP8?
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48080 )
Change subject: mainboard/intel/adlrvp: Enable PCH PCIe device over x1 slot ......................................................................
Patch Set 5:
(2 comments)
https://review.coreboot.org/c/coreboot/+/48080/4//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/48080/4//COMMIT_MSG@12 PS4, Line 12: Drive OEB 7:GPP_A7 and OEB 6:GPP_E5 low
I think it would be good to add a comment to indicate that this is a RVP specific requirement so tha […]
Added W/A in commit msg, hope that is okay
https://review.coreboot.org/c/coreboot/+/48080/4/src/mainboard/intel/adlrvp/... File src/mainboard/intel/adlrvp/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/48080/4/src/mainboard/intel/adlrvp/... PS4, Line 54: free running CLK
I still don't understand this. […]
you mean PcieClkSrcUsage[6]="7" ? because CLK6 is for RP6 already.
i have tried the same but unable to see device detection
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48080 )
Change subject: mainboard/intel/adlrvp: Enable PCH PCIe device over x1 slot ......................................................................
Patch Set 5:
(2 comments)
https://review.coreboot.org/c/coreboot/+/48080/4//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/48080/4//COMMIT_MSG@12 PS4, Line 12: Drive OEB 7:GPP_A7 and OEB 6:GPP_E5 low
Added W/A in commit msg, hope that is okay
SG. Thanks!
https://review.coreboot.org/c/coreboot/+/48080/4/src/mainboard/intel/adlrvp/... File src/mainboard/intel/adlrvp/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/48080/4/src/mainboard/intel/adlrvp/... PS4, Line 54: free running CLK
you mean PcieClkSrcUsage[6]="7" ? because CLK6 is for RP6 already. […]
Why is that? Don't mean to block this change, but I am interested in understanding why the normal configuration as stated above does not work. Is there a hardware bug? Or any other limitation you are aware of?
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48080 )
Change subject: mainboard/intel/adlrvp: Enable PCH PCIe device over x1 slot ......................................................................
Patch Set 5:
(1 comment)
https://review.coreboot.org/c/coreboot/+/48080/4/src/mainboard/intel/adlrvp/... File src/mainboard/intel/adlrvp/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/48080/4/src/mainboard/intel/adlrvp/... PS4, Line 54: free running CLK
you mean PcieClkSrcUsage[6]="7" ? because CLK6 is for RP6 already. […]
Actually on RVP, CLK6 is MUXed between Gbe and PCIe x1 hence for x1 port to work, doesn't contain port-clock map for a given board, the clocks will keep on running anyway, allowing PCIe devices to operate.
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48080 )
Change subject: mainboard/intel/adlrvp: Enable PCH PCIe device over x1 slot ......................................................................
Patch Set 5:
(1 comment)
https://review.coreboot.org/c/coreboot/+/48080/4/src/mainboard/intel/adlrvp/... File src/mainboard/intel/adlrvp/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/48080/4/src/mainboard/intel/adlrvp/... PS4, Line 54: free running CLK
Actually on RVP, CLK6 is MUXed between Gbe and PCIe x1 […]
Ah I see. Thanks for the details Subrata.
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48080 )
Change subject: mainboard/intel/adlrvp: Enable PCH PCIe device over x1 slot ......................................................................
Patch Set 5:
(1 comment)
https://review.coreboot.org/c/coreboot/+/48080/4/src/mainboard/intel/adlrvp/... File src/mainboard/intel/adlrvp/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/48080/4/src/mainboard/intel/adlrvp/... PS4, Line 54: free running CLK
Ah I see. Thanks for the details Subrata.
Thanks Furquan 😊
Subrata Banik has submitted this change. ( https://review.coreboot.org/c/coreboot/+/48080 )
Change subject: mainboard/intel/adlrvp: Enable PCH PCIe device over x1 slot ......................................................................
mainboard/intel/adlrvp: Enable PCH PCIe device over x1 slot
List of changes: 1. Enable Root Port 8 aka 0:0x1c:7 2. Assign free running clock for RP8 3. Apply W/A to get card detected on x1 slot - Drive OEB 7:GPP_A7 and OEB 6:GPP_E5 low
TEST=Able to detect PCIe SD card over x1 slot localhost ~ # dmesg | grep mmc [ 3.643755] mmc0: SDHCI controller on PCI [0000:02:00.0] using ADMA [ 3.825201] mmc0: new ultra high speed DDR50 SDHC card at address 17f8 [ 3.835452] mmcblk0: mmc0:17f8 SE16G 14.4 GiB [ 3.849158] mmcblk0: p1
Change-Id: Ibea37b8de4dd020ff0108ec90ea6f8bcfaa4fb17 Signed-off-by: Subrata Banik subrata.banik@intel.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/48080 Reviewed-by: Angel Pons th3fanbus@gmail.com Reviewed-by: V Sowmya v.sowmya@intel.com Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/mainboard/intel/adlrvp/devicetree.cb M src/mainboard/intel/adlrvp/gpio.c 2 files changed, 11 insertions(+), 1 deletion(-)
Approvals: build bot (Jenkins): Verified V Sowmya: Looks good to me, approved Angel Pons: Looks good to me, approved
diff --git a/src/mainboard/intel/adlrvp/devicetree.cb b/src/mainboard/intel/adlrvp/devicetree.cb index 80edb92..f2f768a 100644 --- a/src/mainboard/intel/adlrvp/devicetree.cb +++ b/src/mainboard/intel/adlrvp/devicetree.cb @@ -51,6 +51,12 @@ register "PcieClkSrcUsage[5]" = "0x5" register "PcieRpClkReqDetect[5]" = "1"
+ # Enable PCH PCIE RP 8 using free running CLK (0x80) + register "PcieRpEnable[7]" = "1" + register "PcieClkSrcClkReq[7]" = "7" + register "PcieClkSrcUsage[7]" = "0x80" + register "PcieRpClkReqDetect[7]" = "1" + # Enable PCH PCIE RP 9 using CLK 1 register "PcieRpEnable[8]" = "1" register "PcieClkSrcClkReq[1]" = "1" @@ -245,7 +251,7 @@ device pci 1c.4 on end # RP5 device pci 1c.5 on end # RP6 device pci 1c.6 off end # RP7 - device pci 1c.7 off end # RP8 + device pci 1c.7 on end # RP8 device pci 1d.0 on end # RP9 device pci 1d.1 off end # RP10 device pci 1d.2 on end # RP11 diff --git a/src/mainboard/intel/adlrvp/gpio.c b/src/mainboard/intel/adlrvp/gpio.c index 89e6f58..4cb8c3a 100644 --- a/src/mainboard/intel/adlrvp/gpio.c +++ b/src/mainboard/intel/adlrvp/gpio.c @@ -72,6 +72,10 @@ PAD_CFG_GPO(GPP_B4, 1, PLTRST), /* M.2_PCH_SSD_PWREN */ PAD_CFG_GPO(GPP_D16, 1, PLTRST), + /* SRCCLK_OEB7 */ + PAD_CFG_GPO(GPP_A7, 0, PLTRST), + /* SRCCLK_OEB6 */ + PAD_CFG_GPO(GPP_E5, 0, PLTRST),
/* M.2_SSD_PDET_R */ PAD_CFG_NF(GPP_A12, NONE, DEEP, NF1),
-
Angel Pons (Code Review) -
Furquan Shaikh (Code Review)
-
Subrata Banik (Code Review)
-
V Sowmya (Code Review)