EricR Lai has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/40487 )
Change subject: mb/google/deltaur: Update H1 I2C gpio pin setting ......................................................................
mb/google/deltaur: Update H1 I2C gpio pin setting
H1 using I2C3 in the HW schematics and connect to GPP_H6 and GPP_H7.
BUG=b:150165131
Signed-off-by: Eric Lai ericr_lai@compal.corp-partner.google.com Change-Id: I43c18baea66b927d51689579a40a53f72b94ef36 --- M src/mainboard/google/deltaur/variants/baseboard/gpio.c 1 file changed, 12 insertions(+), 12 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/87/40487/1
diff --git a/src/mainboard/google/deltaur/variants/baseboard/gpio.c b/src/mainboard/google/deltaur/variants/baseboard/gpio.c index 46a5cdd..71a07b6 100644 --- a/src/mainboard/google/deltaur/variants/baseboard/gpio.c +++ b/src/mainboard/google/deltaur/variants/baseboard/gpio.c @@ -303,14 +303,14 @@ PAD_CFG_GPI(GPP_H4, NONE, DEEP), /* H5 : GPP_H5 ==> DDR_CHB_EN_1P8 */ PAD_CFG_GPI(GPP_H5, NONE, DEEP), - /* H6 : GPP_H6 ==> SPK_DET1 */ - PAD_CFG_GPI(GPP_H6, NONE, PLTRST), - /* H7 : GPP_H7 ==> NC */ - PAD_NC(GPP_H7, NONE), - /* H8 : GPP_H8 ==> I2C_SDA_PCH_H1 */ - PAD_CFG_NF(GPP_H8, NONE, DEEP, NF1), - /* H9 : GPP_H9 ==> I2C_SCL_PCH_H1 */ - PAD_CFG_NF(GPP_H9, NONE, DEEP, NF1), + /* H6 : GPP_H6 ==> I2C_SDA_PCH_H1 */ + PAD_CFG_NF(GPP_H6, NONE, DEEP, NF1), + /* H7 : GPP_H7 ==> I2C_SCL_PCH_H1 */ + PAD_CFG_NF(GPP_H7, NONE, DEEP, NF1), + /* H8 : GPP_H8 ==> NC */ + PAD_NC(GPP_H8, NONE), + /* H9 : GPP_H9 ==> NC */ + PAD_NC(GPP_H9, NONE), /* H10 : GPP_H10 ==> CLKREQ_PCIE#4 */ PAD_CFG_NF(GPP_H10, NONE, DEEP, NF1), /* H11 : GPP_H11 ==> CLKREQ_PCIE#5 */ @@ -436,10 +436,10 @@ PAD_CFG_GPI(GPP_H4, NONE, DEEP), /* H5 : GPP_H5 ==> DDR_CHB_EN_1P8 */ PAD_CFG_GPI(GPP_H5, NONE, DEEP), - /* H8 : GPP_H8 ==> I2C_SDA_PCH_H1 */ - PAD_CFG_NF(GPP_H8, NONE, DEEP, NF1), - /* H9 : GPP_H9 ==> I2C_SCL_PCH_H1 */ - PAD_CFG_NF(GPP_H9, NONE, DEEP, NF1), + /* H6 : GPP_H6 ==> I2C_SDA_PCH_H1 */ + PAD_CFG_NF(GPP_H6, NONE, DEEP, NF1), + /* H7 : GPP_H7 ==> I2C_SCL_PCH_H1 */ + PAD_CFG_NF(GPP_H7, NONE, DEEP, NF1), /* GPD3: GPD3 ==> SIO_PWRBTN# */ PAD_CFG_NF(GPD3, UP_20K, DEEP, NF1), };
EricR Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40487 )
Change subject: mb/google/deltaur: Update H1 I2C gpio pin setting ......................................................................
Patch Set 1:
But I still see the I2C error after patched it. Maybe still missing something.
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40487 )
Change subject: mb/google/deltaur: Update H1 I2C gpio pin setting ......................................................................
Patch Set 1: Code-Review+2
Oops, good catch Eric!
EricR Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40487 )
Change subject: mb/google/deltaur: Update H1 I2C gpio pin setting ......................................................................
Patch Set 1:
Patch Set 1: Code-Review+2
Oops, good catch Eric!
I blamed to our HW. But we need double check next time. The GPIO table they provide is wrong 😞
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40487 )
Change subject: mb/google/deltaur: Update H1 I2C gpio pin setting ......................................................................
Patch Set 1:
Patch Set 1:
Patch Set 1: Code-Review+2
Oops, good catch Eric!
I blamed to our HW. But we need double check next time. The GPIO table they provide is wrong 😞
Yeah I'm going to stop looking at that table 😊
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40487 )
Change subject: mb/google/deltaur: Update H1 I2C gpio pin setting ......................................................................
Patch Set 1:
(4 comments)
https://review.coreboot.org/c/coreboot/+/40487/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/40487/1//COMMIT_MSG@9 PS1, Line 9: connect connects?
https://review.coreboot.org/c/coreboot/+/40487/1//COMMIT_MSG@9 PS1, Line 9: using uses
https://review.coreboot.org/c/coreboot/+/40487/1//COMMIT_MSG@9 PS1, Line 9: Only one space.
https://review.coreboot.org/c/coreboot/+/40487/1//COMMIT_MSG@11 PS1, Line 11: BUG=b:150165131 Tested how?
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40487 )
Change subject: mb/google/deltaur: Update H1 I2C gpio pin setting ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/40487/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/40487/1//COMMIT_MSG@11 PS1, Line 11: BUG=b:150165131
Tested how?
Sorry Paul, sometimes I forget that the context in my head is already there 😊 The board isn't booting yet, this was just an oversight from the schematic review. Eric, would you mind adding something to the commit msg saying we're correcting a mistake from the original GPIO CL?
Hello Bora Guvendik, build bot (Jenkins), Tim Wawrzynczak, Selma Bensaid,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/40487
to look at the new patch set (#2).
Change subject: mb/google/deltaur: Correct H1 I2C gpio pin setting ......................................................................
mb/google/deltaur: Correct H1 I2C gpio pin setting
H1 uses I2C3 in the HW schematics and connects to GPP_H6 and GPP_H7. Previous setting was wrong so correct it.
BUG=b:150165131
Signed-off-by: Eric Lai ericr_lai@compal.corp-partner.google.com Change-Id: I43c18baea66b927d51689579a40a53f72b94ef36 --- M src/mainboard/google/deltaur/variants/baseboard/gpio.c 1 file changed, 12 insertions(+), 12 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/87/40487/2
EricR Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40487 )
Change subject: mb/google/deltaur: Correct H1 I2C gpio pin setting ......................................................................
Patch Set 2:
(4 comments)
https://review.coreboot.org/c/coreboot/+/40487/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/40487/1//COMMIT_MSG@9 PS1, Line 9:
Only one space.
Done
https://review.coreboot.org/c/coreboot/+/40487/1//COMMIT_MSG@9 PS1, Line 9: connect
connects?
Done
https://review.coreboot.org/c/coreboot/+/40487/1//COMMIT_MSG@9 PS1, Line 9: using
uses
Done
https://review.coreboot.org/c/coreboot/+/40487/1//COMMIT_MSG@11 PS1, Line 11: BUG=b:150165131
Sorry Paul, sometimes I forget that the context in my head is already there 😊 […]
Done
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40487 )
Change subject: mb/google/deltaur: Correct H1 I2C gpio pin setting ......................................................................
Patch Set 2: Code-Review+1
Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/40487 )
Change subject: mb/google/deltaur: Correct H1 I2C gpio pin setting ......................................................................
mb/google/deltaur: Correct H1 I2C gpio pin setting
H1 uses I2C3 in the HW schematics and connects to GPP_H6 and GPP_H7. Previous setting was wrong so correct it.
BUG=b:150165131
Signed-off-by: Eric Lai ericr_lai@compal.corp-partner.google.com Change-Id: I43c18baea66b927d51689579a40a53f72b94ef36 Reviewed-on: https://review.coreboot.org/c/coreboot/+/40487 Reviewed-by: Paul Menzel paulepanter@users.sourceforge.net Reviewed-by: Tim Wawrzynczak twawrzynczak@chromium.org Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/mainboard/google/deltaur/variants/baseboard/gpio.c 1 file changed, 12 insertions(+), 12 deletions(-)
Approvals: build bot (Jenkins): Verified Paul Menzel: Looks good to me, but someone else must approve Tim Wawrzynczak: Looks good to me, approved
diff --git a/src/mainboard/google/deltaur/variants/baseboard/gpio.c b/src/mainboard/google/deltaur/variants/baseboard/gpio.c index 46a5cdd..71a07b6 100644 --- a/src/mainboard/google/deltaur/variants/baseboard/gpio.c +++ b/src/mainboard/google/deltaur/variants/baseboard/gpio.c @@ -303,14 +303,14 @@ PAD_CFG_GPI(GPP_H4, NONE, DEEP), /* H5 : GPP_H5 ==> DDR_CHB_EN_1P8 */ PAD_CFG_GPI(GPP_H5, NONE, DEEP), - /* H6 : GPP_H6 ==> SPK_DET1 */ - PAD_CFG_GPI(GPP_H6, NONE, PLTRST), - /* H7 : GPP_H7 ==> NC */ - PAD_NC(GPP_H7, NONE), - /* H8 : GPP_H8 ==> I2C_SDA_PCH_H1 */ - PAD_CFG_NF(GPP_H8, NONE, DEEP, NF1), - /* H9 : GPP_H9 ==> I2C_SCL_PCH_H1 */ - PAD_CFG_NF(GPP_H9, NONE, DEEP, NF1), + /* H6 : GPP_H6 ==> I2C_SDA_PCH_H1 */ + PAD_CFG_NF(GPP_H6, NONE, DEEP, NF1), + /* H7 : GPP_H7 ==> I2C_SCL_PCH_H1 */ + PAD_CFG_NF(GPP_H7, NONE, DEEP, NF1), + /* H8 : GPP_H8 ==> NC */ + PAD_NC(GPP_H8, NONE), + /* H9 : GPP_H9 ==> NC */ + PAD_NC(GPP_H9, NONE), /* H10 : GPP_H10 ==> CLKREQ_PCIE#4 */ PAD_CFG_NF(GPP_H10, NONE, DEEP, NF1), /* H11 : GPP_H11 ==> CLKREQ_PCIE#5 */ @@ -436,10 +436,10 @@ PAD_CFG_GPI(GPP_H4, NONE, DEEP), /* H5 : GPP_H5 ==> DDR_CHB_EN_1P8 */ PAD_CFG_GPI(GPP_H5, NONE, DEEP), - /* H8 : GPP_H8 ==> I2C_SDA_PCH_H1 */ - PAD_CFG_NF(GPP_H8, NONE, DEEP, NF1), - /* H9 : GPP_H9 ==> I2C_SCL_PCH_H1 */ - PAD_CFG_NF(GPP_H9, NONE, DEEP, NF1), + /* H6 : GPP_H6 ==> I2C_SDA_PCH_H1 */ + PAD_CFG_NF(GPP_H6, NONE, DEEP, NF1), + /* H7 : GPP_H7 ==> I2C_SCL_PCH_H1 */ + PAD_CFG_NF(GPP_H7, NONE, DEEP, NF1), /* GPD3: GPD3 ==> SIO_PWRBTN# */ PAD_CFG_NF(GPD3, UP_20K, DEEP, NF1), };
-
EricR Lai (Code Review) -
Patrick Georgi (Code Review)
-
Paul Menzel (Code Review)
-
Tim Wawrzynczak (Code Review)