Change in coreboot[master]: mb/google/volteer/variants/eldrid: Configure DP_HPD as PAD_NC and dis...
Nick Chen has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/45246 )
Change subject: mb/google/volteer/variants/eldrid: Configure DP_HPD as PAD_NC and disable DdiPortHpd ......................................................................
mb/google/volteer/variants/eldrid: Configure DP_HPD as PAD_NC and disable DdiPortHpd
GPP_A19(DP_HPD1) and GPP_A20(DP_HPD2) were configured native function (NF1) without internal pull-down which wrongly presents HPD interrupts. This change configures GPP_A19 and GPP_A20 to be no connection and disables DdiPort1Hpd and DdiPort2Hpd.
BUG=b:165893624, b:168090618
Signed-off-by: nick_xr_chen nick_xr_chen@wistron.corp-partner.google.com Change-Id: I31b25be1c9248debf855435c7b688b358e2cd57e --- M src/mainboard/google/volteer/variants/eldrid/gpio.c M src/mainboard/google/volteer/variants/eldrid/overridetree.cb 2 files changed, 4 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/46/45246/1
diff --git a/src/mainboard/google/volteer/variants/eldrid/gpio.c b/src/mainboard/google/volteer/variants/eldrid/gpio.c index aeccfab..855e96d 100644 --- a/src/mainboard/google/volteer/variants/eldrid/gpio.c +++ b/src/mainboard/google/volteer/variants/eldrid/gpio.c @@ -19,9 +19,9 @@ /* A16 : USB_OC3# ==> USB_C0_OC_ODL */ PAD_CFG_NF(GPP_A16, NONE, DEEP, NF1), /* A19 : DDSP_HPD1 ==> USB_C0_DP_HPD */ - PAD_CFG_NF(GPP_A19, NONE, DEEP, NF1), + PAD_NC(GPP_A19, NONE), /* A20 : DDSP_HPD2 ==> USB_C1_DP_HPD */ - PAD_CFG_NF(GPP_A20, NONE, DEEP, NF1), + PAD_NC(GPP_A20, NONE), /* A21 : DDPC_CTRCLK ==> EN_FP_PWR */ PAD_CFG_GPO(GPP_A21, 1, DEEP), /* A23 : I2S1_SCLK ==> I2S1_SPKR_SCLK */ diff --git a/src/mainboard/google/volteer/variants/eldrid/overridetree.cb b/src/mainboard/google/volteer/variants/eldrid/overridetree.cb index 171e397..b04b1e7 100644 --- a/src/mainboard/google/volteer/variants/eldrid/overridetree.cb +++ b/src/mainboard/google/volteer/variants/eldrid/overridetree.cb @@ -1,6 +1,8 @@ chip soc/intel/tigerlake
register "TcssAuxOri" = "1" + register "DdiPort1Hpd" = "0" + register "DdiPort2Hpd" = "0" register "IomTypeCPortPadCfg[0]" = "0x090E000A" register "IomTypeCPortPadCfg[1]" = "0x090E000D" #+-------------------+---------------------------+
Hello build bot (Jenkins), Patrick Georgi, Caveh Jalali, Derek Huang,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/45246
to look at the new patch set (#2).
Change subject: mb/google/volteer/variants/eldrid: Configure DP_HPD as PAD_NC and disable DdiPortHpd ......................................................................
mb/google/volteer/variants/eldrid: Configure DP_HPD as PAD_NC and disable DdiPortHpd
GPP_A19(DP_HPD1) and GPP_A20(DP_HPD2) were configured native function (NF1) without internal pull-down which wrongly presents HPD interrupts. This change configures GPP_A19 and GPP_A20 to be no connection and disables DdiPort1Hpd and DdiPort2Hpd.
BUG=b:165893624, b:168090618
Signed-off-by: nick_xr_chen nick_xr_chen@wistron.corp-partner.google.com Change-Id: I31b25be1c9248debf855435c7b688b358e2cd57e --- M src/mainboard/google/volteer/variants/eldrid/gpio.c M src/mainboard/google/volteer/variants/eldrid/overridetree.cb 2 files changed, 4 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/46/45246/2
Scott Chao has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45246 )
Change subject: mb/google/volteer/variants/eldrid: Configure DP_HPD as PAD_NC and disable DdiPortHpd ......................................................................
Patch Set 2: Code-Review+1
Hi Googlers, can you kindly help to review this CL?
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45246 )
Change subject: mb/google/volteer/variants/eldrid: Configure DP_HPD as PAD_NC and disable DdiPortHpd ......................................................................
Patch Set 2: Code-Review+2
(1 comment)
https://review.coreboot.org/c/coreboot/+/45246/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/45246/2//COMMIT_MSG@7 PS2, Line 7: mb/google/volteer/variants/eldrid: Configure DP_HPD as PAD_NC and disable DdiPortHpd Try to keep first line to 72 chars, I suggest: mb/google/volteer: Eldrid: Disable DP_HPD1/DP_HPD2
Hello build bot (Jenkins), Patrick Georgi, Caveh Jalali, Tim Wawrzynczak, Duncan Laurie, Nick Vaccaro, Scott Chao, Derek Huang,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/45246
to look at the new patch set (#3).
Change subject: mb/google/volteer/variants/eldrid: Configure DP_HPD as PAD_NC and disable DdiPortHpd ......................................................................
mb/google/volteer/variants/eldrid: Configure DP_HPD as PAD_NC and disable DdiPortHpd
GPP_A19(DP_HPD1) and GPP_A20(DP_HPD2) were configured native function (NF1) without internal pull-down which wrongly presents HPD interrupts. This change configures GPP_A19 and GPP_A20 to be no connection and disables DdiPort1Hpd and DdiPort2Hpd.
BUG=b:165893624, b:168090618
Signed-off-by: nick_xr_chen nick_xr_chen@wistron.corp-partner.google.com Change-Id: I31b25be1c9248debf855435c7b688b358e2cd57e --- M src/mainboard/google/volteer/variants/eldrid/gpio.c M src/mainboard/google/volteer/variants/eldrid/overridetree.cb 2 files changed, 4 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/46/45246/3
Nick Chen has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45246 )
Change subject: mb/google/volteer/variants/eldrid: Configure DP_HPD as PAD_NC and disable DdiPortHpd ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45246/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/45246/2//COMMIT_MSG@7 PS2, Line 7: mb/google/volteer/variants/eldrid: Configure DP_HPD as PAD_NC and disable DdiPortHpd
Try to keep first line to 72 chars, I suggest: […]
Done
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45246 )
Change subject: mb/google/volteer/variants/eldrid: Configure DP_HPD as PAD_NC and disable DdiPortHpd ......................................................................
Patch Set 3:
(2 comments)
https://review.coreboot.org/c/coreboot/+/45246/3//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/45246/3//COMMIT_MSG@7 PS3, Line 7: extra space
https://review.coreboot.org/c/coreboot/+/45246/3//COMMIT_MSG@8 PS3, Line 8: and disable DdiPortHpd You mention this below, the line above is a good summary, I'd remove this.
Hello build bot (Jenkins), Patrick Georgi, Caveh Jalali, Tim Wawrzynczak, Duncan Laurie, Nick Vaccaro, Scott Chao, Derek Huang,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/45246
to look at the new patch set (#4).
Change subject: mb/google/volteer/variants/eldrid: Configure DP_HPD as PAD_NC ......................................................................
mb/google/volteer/variants/eldrid: Configure DP_HPD as PAD_NC
GPP_A19(DP_HPD1) and GPP_A20(DP_HPD2) were configured native function (NF1) without internal pull-down which wrongly presents HPD interrupts. This change configures GPP_A19 and GPP_A20 to be no connection and disables DdiPort1Hpd and DdiPort2Hpd.
BUG=b:165893624, b:168090618
Signed-off-by: nick_xr_chen nick_xr_chen@wistron.corp-partner.google.com Change-Id: I31b25be1c9248debf855435c7b688b358e2cd57e --- M src/mainboard/google/volteer/variants/eldrid/gpio.c M src/mainboard/google/volteer/variants/eldrid/overridetree.cb 2 files changed, 4 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/46/45246/4
陳祥睿 has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45246 )
Change subject: mb/google/volteer/variants/eldrid: Configure DP_HPD as PAD_NC ......................................................................
Patch Set 4:
(2 comments)
https://review.coreboot.org/c/coreboot/+/45246/3//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/45246/3//COMMIT_MSG@7 PS3, Line 7:
extra space
done
https://review.coreboot.org/c/coreboot/+/45246/3//COMMIT_MSG@8 PS3, Line 8: and disable DdiPortHpd
You mention this below, the line above is a good summary, I'd remove this.
done
Nick Chen has removed 陳祥睿 from this change. ( https://review.coreboot.org/c/coreboot/+/45246 )
Change subject: mb/google/volteer/variants/eldrid: Configure DP_HPD as PAD_NC ......................................................................
Removed reviewer 陳祥睿.
Nick Chen has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45246 )
Change subject: mb/google/volteer/variants/eldrid: Configure DP_HPD as PAD_NC ......................................................................
Patch Set 4:
(2 comments)
https://review.coreboot.org/c/coreboot/+/45246/3//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/45246/3//COMMIT_MSG@7 PS3, Line 7:
extra space
Done
https://review.coreboot.org/c/coreboot/+/45246/3//COMMIT_MSG@8 PS3, Line 8: and disable DdiPortHpd
You mention this below, the line above is a good summary, I'd remove this.
Done
Tim Wawrzynczak has submitted this change. ( https://review.coreboot.org/c/coreboot/+/45246 )
Change subject: mb/google/volteer/variants/eldrid: Configure DP_HPD as PAD_NC ......................................................................
mb/google/volteer/variants/eldrid: Configure DP_HPD as PAD_NC
GPP_A19(DP_HPD1) and GPP_A20(DP_HPD2) were configured native function (NF1) without internal pull-down which wrongly presents HPD interrupts. This change configures GPP_A19 and GPP_A20 to be no connection and disables DdiPort1Hpd and DdiPort2Hpd.
BUG=b:165893624, b:168090618
Signed-off-by: nick_xr_chen nick_xr_chen@wistron.corp-partner.google.com Change-Id: I31b25be1c9248debf855435c7b688b358e2cd57e Reviewed-on: https://review.coreboot.org/c/coreboot/+/45246 Reviewed-by: Scott Chao scott_chao@wistron.corp-partner.google.com Reviewed-by: Tim Wawrzynczak twawrzynczak@chromium.org Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/mainboard/google/volteer/variants/eldrid/gpio.c M src/mainboard/google/volteer/variants/eldrid/overridetree.cb 2 files changed, 4 insertions(+), 2 deletions(-)
Approvals: build bot (Jenkins): Verified Tim Wawrzynczak: Looks good to me, approved Scott Chao: Looks good to me, but someone else must approve
diff --git a/src/mainboard/google/volteer/variants/eldrid/gpio.c b/src/mainboard/google/volteer/variants/eldrid/gpio.c index aeccfab..855e96d 100644 --- a/src/mainboard/google/volteer/variants/eldrid/gpio.c +++ b/src/mainboard/google/volteer/variants/eldrid/gpio.c @@ -19,9 +19,9 @@ /* A16 : USB_OC3# ==> USB_C0_OC_ODL */ PAD_CFG_NF(GPP_A16, NONE, DEEP, NF1), /* A19 : DDSP_HPD1 ==> USB_C0_DP_HPD */ - PAD_CFG_NF(GPP_A19, NONE, DEEP, NF1), + PAD_NC(GPP_A19, NONE), /* A20 : DDSP_HPD2 ==> USB_C1_DP_HPD */ - PAD_CFG_NF(GPP_A20, NONE, DEEP, NF1), + PAD_NC(GPP_A20, NONE), /* A21 : DDPC_CTRCLK ==> EN_FP_PWR */ PAD_CFG_GPO(GPP_A21, 1, DEEP), /* A23 : I2S1_SCLK ==> I2S1_SPKR_SCLK */ diff --git a/src/mainboard/google/volteer/variants/eldrid/overridetree.cb b/src/mainboard/google/volteer/variants/eldrid/overridetree.cb index 171e397..b04b1e7 100644 --- a/src/mainboard/google/volteer/variants/eldrid/overridetree.cb +++ b/src/mainboard/google/volteer/variants/eldrid/overridetree.cb @@ -1,6 +1,8 @@ chip soc/intel/tigerlake
register "TcssAuxOri" = "1" + register "DdiPort1Hpd" = "0" + register "DdiPort2Hpd" = "0" register "IomTypeCPortPadCfg[0]" = "0x090E000A" register "IomTypeCPortPadCfg[1]" = "0x090E000D" #+-------------------+---------------------------+
Nick Vaccaro has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45246 )
Change subject: mb/google/volteer/variants/eldrid: Configure DP_HPD as PAD_NC ......................................................................
Patch Set 5:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45246/5/src/mainboard/google/voltee... File src/mainboard/google/volteer/variants/eldrid/gpio.c:
https://review.coreboot.org/c/coreboot/+/45246/5/src/mainboard/google/voltee... PS5, Line 22: PAD_NC(GPP_A19, NONE), Sorry I didn't see this CL sooner. You don't need to add the PAD_NC for GPP_A19 or GPP_A20 because they are already set that way in baseboard's gpio.c.
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45246 )
Change subject: mb/google/volteer/variants/eldrid: Configure DP_HPD as PAD_NC ......................................................................
Patch Set 5:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45246/5/src/mainboard/google/voltee... File src/mainboard/google/volteer/variants/eldrid/gpio.c:
https://review.coreboot.org/c/coreboot/+/45246/5/src/mainboard/google/voltee... PS5, Line 22: PAD_NC(GPP_A19, NONE),
Sorry I didn't see this CL sooner. […]
Good catch Nick, my bad, I'll fix it up.
Nick Vaccaro has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45246 )
Change subject: mb/google/volteer/variants/eldrid: Configure DP_HPD as PAD_NC ......................................................................
Patch Set 5:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45246/5/src/mainboard/google/voltee... File src/mainboard/google/volteer/variants/eldrid/gpio.c:
https://review.coreboot.org/c/coreboot/+/45246/5/src/mainboard/google/voltee... PS5, Line 22: PAD_NC(GPP_A19, NONE),
Good catch Nick, my bad, I'll fix it up.
I should have added "nit:" as it wouldn't hurt anything, just not needed.
9elements QA has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45246 )
Change subject: mb/google/volteer/variants/eldrid: Configure DP_HPD as PAD_NC ......................................................................
Patch Set 5:
Automatic boot test returned (PASS/FAIL/TOTAL): 8/1/9 "QEMU x86 q35/ich9" (x86_32) using payload TianoCore : SUCCESS : https://lava.9esec.io/r/19734 "QEMU x86 q35/ich9" (x86_32) using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/19733 "QEMU x86 i440fx/piix4" (x86_64) using payload SeaBIOS : FAIL : https://lava.9esec.io/r/19732 "QEMU x86 i440fx/piix4" (x86_32) using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/19731 "QEMU AArch64" using payload LinuxBoot_u-root_kexec : SUCCESS : https://lava.9esec.io/r/19730 "HP Z220 SFF Workstation" (x86_32) using payload LinuxBoot_BusyBox_kexec : SUCCESS : https://lava.9esec.io/r/19738 "HP Z220 SFF Workstation" (x86_32) using payload LinuxBoot_BusyBox_kexec : SUCCESS : https://lava.9esec.io/r/19737 "HP Compaq 8200 Elite SFF PC" (x86_32) using payload TianoCore : SUCCESS : https://lava.9esec.io/r/19736 "HP Compaq 8200 Elite SFF PC" (x86_32) using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/19735
Please note: This test is under development and might not be accurate at all!
Nick Chen has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45246 )
Change subject: mb/google/volteer/variants/eldrid: Configure DP_HPD as PAD_NC ......................................................................
Patch Set 5:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45246/5/src/mainboard/google/voltee... File src/mainboard/google/volteer/variants/eldrid/gpio.c:
https://review.coreboot.org/c/coreboot/+/45246/5/src/mainboard/google/voltee... PS5, Line 22: PAD_NC(GPP_A19, NONE),
I should have added "nit:" as it wouldn't hurt anything, just not needed.
I need remove GPP_A19 or GPP_A20 on the CL?
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45246 )
Change subject: mb/google/volteer/variants/eldrid: Configure DP_HPD as PAD_NC ......................................................................
Patch Set 5:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45246/5/src/mainboard/google/voltee... File src/mainboard/google/volteer/variants/eldrid/gpio.c:
https://review.coreboot.org/c/coreboot/+/45246/5/src/mainboard/google/voltee... PS5, Line 22: PAD_NC(GPP_A19, NONE),
I need remove GPP_A19 or GPP_A20 on the CL?
I fixed it here: https://review.coreboot.org/c/coreboot/+/45485
-
9elements QA (Code Review) -
Nick Chen (Code Review)
-
Nick Vaccaro (Code Review)
-
Scott Chao (Code Review)
-
Tim Wawrzynczak (Code Review)
-
陳祥睿 (Code Review)