Krishna P Bhat D has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/32176
Change subject: mb/google/hatch: Update GPIO settings ......................................................................
mb/google/hatch: Update GPIO settings
This patch updates the following GPIO settings. 1. Set Native termination for GPP_G0 - G4 SD card pins. 2. Disable I2C #2 in devicetree. 3. Set GPP_B19, GPP_G7 to NF1.
BUG=b:123907904
Change-Id: I4549ac7377d7f58f51cda0eb96a62604fd31d2f2 Signed-off-by: Krishna Prasad Bhat krishna.p.bhat.d@intel.com --- M src/mainboard/google/hatch/variants/baseboard/gpio.c M src/mainboard/google/hatch/variants/hatch/overridetree.cb M src/mainboard/google/hatch/variants/hatch_whl/overridetree.cb 3 files changed, 11 insertions(+), 11 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/76/32176/1
diff --git a/src/mainboard/google/hatch/variants/baseboard/gpio.c b/src/mainboard/google/hatch/variants/baseboard/gpio.c index 4d1c87e..10f851b 100644 --- a/src/mainboard/google/hatch/variants/baseboard/gpio.c +++ b/src/mainboard/google/hatch/variants/baseboard/gpio.c @@ -106,8 +106,8 @@ PAD_CFG_NF(GPP_B17, NONE, DEEP, NF1), /* B18 : H1_SLAVE_SPI_MOSI_R */ PAD_CFG_NF(GPP_B18, NONE, DEEP, NF1), - /* B19 : GPP_B19 ==> NC */ - PAD_NC(GPP_B19, NONE), + /* B19 : Set to NF1 */ + PAD_CFG_NF(GPP_B19, NONE, DEEP, NF1), /* B20 : PCH_SPI_FPMCU_CLK_R */ PAD_CFG_NF(GPP_B20, NONE, DEEP, NF1), /* B21 : PCH_SPI_FPMCU_MISO */ @@ -329,21 +329,21 @@ PAD_NC(GPP_F23, NONE),
/* G0 : SD_CMD */ - PAD_CFG_NF(GPP_G0, NONE, DEEP, NF1), + PAD_CFG_NF(GPP_G0, NATIVE, DEEP, NF1), /* G1 : SD_DATA0 */ - PAD_CFG_NF(GPP_G1, NONE, DEEP, NF1), + PAD_CFG_NF(GPP_G1, NATIVE, DEEP, NF1), /* G2 : SD_DATA1 */ - PAD_CFG_NF(GPP_G2, NONE, DEEP, NF1), + PAD_CFG_NF(GPP_G2, NATIVE, DEEP, NF1), /* G3 : SD_DATA2 */ - PAD_CFG_NF(GPP_G3, NONE, DEEP, NF1), + PAD_CFG_NF(GPP_G3, NATIVE, DEEP, NF1), /* G4 : SD_DATA3 */ - PAD_CFG_NF(GPP_G4, NONE, DEEP, NF1), + PAD_CFG_NF(GPP_G4, NATIVE, DEEP, NF1), /* G5 : SD_CD# */ PAD_CFG_NF(GPP_G5, NONE, DEEP, NF1), /* G6 : SD_CLK */ PAD_CFG_NF(GPP_G6, NONE, DEEP, NF1), - /* G7 : SD_WP => NC */ - PAD_NC(GPP_G7, DN_20K), + /* G7 : Set to NF1 */ + PAD_CFG_NF(GPP_G7, NONE, DEEP, NF1),
/* * H0 : HP_INT_L diff --git a/src/mainboard/google/hatch/variants/hatch/overridetree.cb b/src/mainboard/google/hatch/variants/hatch/overridetree.cb index 28644d1..57daba2 100644 --- a/src/mainboard/google/hatch/variants/hatch/overridetree.cb +++ b/src/mainboard/google/hatch/variants/hatch/overridetree.cb @@ -69,7 +69,7 @@ device i2c 49 on end end end # I2C #1 - device pci 15.2 on + device pci 15.2 off chip drivers/generic/gpio_keys register "name" = ""PENH"" register "gpio" = "ACPI_GPIO_INPUT_ACTIVE_HIGH(GPP_A8)" diff --git a/src/mainboard/google/hatch/variants/hatch_whl/overridetree.cb b/src/mainboard/google/hatch/variants/hatch_whl/overridetree.cb index 9b772c2..098bca6 100644 --- a/src/mainboard/google/hatch/variants/hatch_whl/overridetree.cb +++ b/src/mainboard/google/hatch/variants/hatch_whl/overridetree.cb @@ -54,7 +54,7 @@ device i2c 49 on end end end # I2C #1 - device pci 15.2 on + device pci 15.2 off chip drivers/generic/gpio_keys register "name" = ""PENH"" register "gpio" = "ACPI_GPIO_INPUT_ACTIVE_HIGH(GPP_A8)"
Hello Rizwan Qureshi, build bot (Jenkins), Furquan Shaikh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/32176
to look at the new patch set (#2).
Change subject: mb/google/hatch: Update GPIO settings ......................................................................
mb/google/hatch: Update GPIO settings
This patch updates the following GPIO settings. 1. Set Native termination for GPP_G0 - G4 SD card pins. 2. Disable I2C #2 in devicetree. 3. Set GPP_B19, GPP_G7 to NF1.
BUG=b:123907904
Change-Id: I4549ac7377d7f58f51cda0eb96a62604fd31d2f2 Signed-off-by: Krishna Prasad Bhat krishna.p.bhat.d@intel.com --- M src/mainboard/google/hatch/variants/baseboard/gpio.c M src/mainboard/google/hatch/variants/hatch/overridetree.cb M src/mainboard/google/hatch/variants/hatch_whl/overridetree.cb 3 files changed, 11 insertions(+), 11 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/76/32176/2
Hello Rizwan Qureshi, build bot (Jenkins), Furquan Shaikh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/32176
to look at the new patch set (#3).
Change subject: mb/google/hatch: Update GPIO settings ......................................................................
mb/google/hatch: Update GPIO settings
This patch updates the following GPIO settings. 1. Set Native termination for GPP_G0 - G4 SD card pins, pull-down termination for GPP_G7 SD card Write Protect pin. 2. Disable I2C #2 in variant devicetree. 3. Set GPP_B19 to NF1.
BUG=b:123907904
Change-Id: I4549ac7377d7f58f51cda0eb96a62604fd31d2f2 Signed-off-by: Krishna Prasad Bhat krishna.p.bhat.d@intel.com --- M src/mainboard/google/hatch/variants/baseboard/gpio.c M src/mainboard/google/hatch/variants/hatch/overridetree.cb M src/mainboard/google/hatch/variants/hatch_whl/overridetree.cb 3 files changed, 11 insertions(+), 11 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/76/32176/3
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32176 )
Change subject: mb/google/hatch: Update GPIO settings ......................................................................
Patch Set 3: Code-Review+1
Rizwan Qureshi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32176 )
Change subject: mb/google/hatch: Update GPIO settings ......................................................................
Patch Set 3:
please rebase the patch and also add what tests you have performed after changing the GPIO configuration.
Hello Rizwan Qureshi, Tim Wawrzynczak, Shelley Chen, build bot (Jenkins), Furquan Shaikh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/32176
to look at the new patch set (#4).
Change subject: mb/google/hatch: Update GPIO settings ......................................................................
mb/google/hatch: Update GPIO settings
This patch updates the following GPIO settings. 1. Set Native termination for GPP_G0 - G4 SD card pins, pull-down termination for GPP_G7 SD card Write Protect pin. 2. Set GPP_B19 to NF1.
BUG=b:123907904 TEST=Verified SD card functionality on hatch. Checked for SD detection, transferred files to and from SD card.
Change-Id: I4549ac7377d7f58f51cda0eb96a62604fd31d2f2 Signed-off-by: Krishna Prasad Bhat krishna.p.bhat.d@intel.com --- M src/mainboard/google/hatch/variants/baseboard/gpio.c 1 file changed, 9 insertions(+), 9 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/76/32176/4
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32176 )
Change subject: mb/google/hatch: Update GPIO settings ......................................................................
Patch Set 4:
(2 comments)
https://review.coreboot.org/#/c/32176/4/src/mainboard/google/hatch/variants/... File src/mainboard/google/hatch/variants/baseboard/gpio.c:
https://review.coreboot.org/#/c/32176/4/src/mainboard/google/hatch/variants/... PS4, Line 32: PAD_NC(GPP_A7, NONE), What about this? Don't you need NF here?
https://review.coreboot.org/#/c/32176/4/src/mainboard/google/hatch/variants/... PS4, Line 332: NATIVE Is this the recommendation for SD controller or for all controllers?
Rizwan Qureshi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32176 )
Change subject: mb/google/hatch: Update GPIO settings ......................................................................
Patch Set 4:
(2 comments)
https://review.coreboot.org/#/c/32176/4/src/mainboard/google/hatch/variants/... File src/mainboard/google/hatch/variants/baseboard/gpio.c:
https://review.coreboot.org/#/c/32176/4/src/mainboard/google/hatch/variants/... PS4, Line 32: PAD_NC(GPP_A7, NONE),
What about this? Don't you need NF here?
CS1 is not being configured in FSP.
https://review.coreboot.org/#/c/32176/4/src/mainboard/google/hatch/variants/... PS4, Line 332: NATIVE
Is this the recommendation for SD controller or for all controllers?
This is only for SD Controller.
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32176 )
Change subject: mb/google/hatch: Update GPIO settings ......................................................................
Patch Set 4:
(3 comments)
https://review.coreboot.org/#/c/32176/4/src/mainboard/google/hatch/variants/... File src/mainboard/google/hatch/variants/baseboard/gpio.c:
https://review.coreboot.org/#/c/32176/4/src/mainboard/google/hatch/variants/... PS4, Line 32: PAD_NC(GPP_A7, NONE),
CS1 is not being configured in FSP.
This is really weird. It seems completely broken to me. How does FSP decide which CS has to be configured?
https://review.coreboot.org/#/c/32176/4/src/mainboard/google/hatch/variants/... PS4, Line 345: Set to NF1 Keep as SD_WP to match the other comments.
https://review.coreboot.org/#/c/32176/4/src/mainboard/google/hatch/variants/... PS4, Line 346: DN_20K Does FSP configure this too?
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32176 )
Change subject: mb/google/hatch: Update GPIO settings ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/#/c/32176/4/src/mainboard/google/hatch/variants/... File src/mainboard/google/hatch/variants/baseboard/gpio.c:
https://review.coreboot.org/#/c/32176/4/src/mainboard/google/hatch/variants/... PS4, Line 109: /* B19 : Set to NF1 */ : PAD_CFG_NF(GPP_B19, NONE, DEEP, NF1), This is still a NC in the Hatch 2.1 reference schematic.
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32176 )
Change subject: mb/google/hatch: Update GPIO settings ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/#/c/32176/4/src/mainboard/google/hatch/variants/... File src/mainboard/google/hatch/variants/baseboard/gpio.c:
https://review.coreboot.org/#/c/32176/4/src/mainboard/google/hatch/variants/... PS4, Line 109: /* B19 : Set to NF1 */ : PAD_CFG_NF(GPP_B19, NONE, DEEP, NF1),
This is still a NC in the Hatch 2.1 reference schematic.
Yeah, it is being set here just to match what FSP is doing.
Rizwan Qureshi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32176 )
Change subject: mb/google/hatch: Update GPIO settings ......................................................................
Patch Set 4:
(2 comments)
https://review.coreboot.org/#/c/32176/4/src/mainboard/google/hatch/variants/... File src/mainboard/google/hatch/variants/baseboard/gpio.c:
https://review.coreboot.org/#/c/32176/4/src/mainboard/google/hatch/variants/... PS4, Line 32: PAD_NC(GPP_A7, NONE),
This is really weird. It seems completely broken to me. […]
Actually it seems like a miss in FSP, i am following it up internally to get it fixed.
https://review.coreboot.org/#/c/32176/4/src/mainboard/google/hatch/variants/... PS4, Line 346: DN_20K
Does FSP configure this too?
No, FSP is just setting it a native mode along with other SD controller pins. Checking to make this configuration optional.
Hello Rizwan Qureshi, Tim Wawrzynczak, Shelley Chen, build bot (Jenkins), Furquan Shaikh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/32176
to look at the new patch set (#5).
Change subject: mb/google/hatch: Update GPIO settings ......................................................................
mb/google/hatch: Update GPIO settings
This patch updates the following GPIO settings. 1. Set Native termination for GPP_G0 - G4 SD card pins, pull-down termination for GPP_G7 SD card Write Protect pin. 2. Set GPP_B19 to NF1.
BUG=b:123907904 TEST=Verified SD card functionality on hatch. Checked for SD detection, transferred files to and from SD card.
Change-Id: I4549ac7377d7f58f51cda0eb96a62604fd31d2f2 Signed-off-by: Krishna Prasad Bhat krishna.p.bhat.d@intel.com --- M src/mainboard/google/hatch/variants/baseboard/gpio.c 1 file changed, 10 insertions(+), 9 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/76/32176/5
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32176 )
Change subject: mb/google/hatch: Update GPIO settings ......................................................................
Patch Set 5:
(1 comment)
https://review.coreboot.org/#/c/32176/5/src/mainboard/google/hatch/variants/... File src/mainboard/google/hatch/variants/baseboard/gpio.c:
https://review.coreboot.org/#/c/32176/5/src/mainboard/google/hatch/variants/... PS5, Line 109: Set to NF1 Can you add a comment indicating why this is set to NF1 even though it is unused.
Hello Rizwan Qureshi, Tim Wawrzynczak, Shelley Chen, build bot (Jenkins), Furquan Shaikh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/32176
to look at the new patch set (#6).
Change subject: mb/google/hatch: Update GPIO settings ......................................................................
mb/google/hatch: Update GPIO settings
This patch updates the following GPIO settings. 1. Set Native termination for GPP_G0 - G4 SD card pins, pull-down termination for GPP_G7 SD card Write Protect pin. 2. Set GPP_B19 to NF1.
BUG=b:123907904 TEST=Verified SD card functionality on hatch. Checked for SD detection, transferred files to and from SD card.
Change-Id: I4549ac7377d7f58f51cda0eb96a62604fd31d2f2 Signed-off-by: Krishna Prasad Bhat krishna.p.bhat.d@intel.com --- M src/mainboard/google/hatch/variants/baseboard/gpio.c 1 file changed, 10 insertions(+), 9 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/76/32176/6
Krishna P Bhat D has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32176 )
Change subject: mb/google/hatch: Update GPIO settings ......................................................................
Patch Set 6:
(1 comment)
https://review.coreboot.org/#/c/32176/5/src/mainboard/google/hatch/variants/... File src/mainboard/google/hatch/variants/baseboard/gpio.c:
https://review.coreboot.org/#/c/32176/5/src/mainboard/google/hatch/variants/... PS5, Line 109: Set to NF1
Can you add a comment indicating why this is set to NF1 even though it is unused.
Done
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32176 )
Change subject: mb/google/hatch: Update GPIO settings ......................................................................
Patch Set 6:
(1 comment)
https://review.coreboot.org/#/c/32176/6/src/mainboard/google/hatch/variants/... File src/mainboard/google/hatch/variants/baseboard/gpio.c:
https://review.coreboot.org/#/c/32176/6/src/mainboard/google/hatch/variants/... PS6, Line 109: /* B19 : Set to NF1 to match FSP setting it to NF1, i.e., GSPI1_CS0# */ : PAD_CFG_NF(GPP_B19, NONE, DEEP, NF1), I am wondering if this can be fixed by setting SerialIoSpi1CsEnable: https://review.coreboot.org/cgit/coreboot.git/tree/src/vendorcode/intel/fsp/...
Did you try playing with it?
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32176 )
Change subject: mb/google/hatch: Update GPIO settings ......................................................................
Patch Set 6:
(1 comment)
https://review.coreboot.org/#/c/32176/6/src/mainboard/google/hatch/variants/... File src/mainboard/google/hatch/variants/baseboard/gpio.c:
https://review.coreboot.org/#/c/32176/6/src/mainboard/google/hatch/variants/... PS6, Line 109: /* B19 : Set to NF1 to match FSP setting it to NF1, i.e., GSPI1_CS0# */ : PAD_CFG_NF(GPP_B19, NONE, DEEP, NF1),
I am wondering if this can be fixed by setting SerialIoSpi1CsEnable: https://review.coreboot. […]
Were you able to try this out?
Krishna P Bhat D has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32176 )
Change subject: mb/google/hatch: Update GPIO settings ......................................................................
Patch Set 6:
(1 comment)
https://review.coreboot.org/#/c/32176/6/src/mainboard/google/hatch/variants/... File src/mainboard/google/hatch/variants/baseboard/gpio.c:
https://review.coreboot.org/#/c/32176/6/src/mainboard/google/hatch/variants/... PS6, Line 109: /* B19 : Set to NF1 to match FSP setting it to NF1, i.e., GSPI1_CS0# */ : PAD_CFG_NF(GPP_B19, NONE, DEEP, NF1),
Were you able to try this out?
Sorry Furquan for late response. SerialIoSpi1CsEnable is not checked while setting GPIO native function in FSP. Setting it to 0 or 1 does not affect SPI1 CS0 being set to NF1. However, this UPD is checked to set the SPI CS polarity. I would recommend setting this GPIO pin in coreboot to align with the FSP settings.
Rizwan Qureshi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32176 )
Change subject: mb/google/hatch: Update GPIO settings ......................................................................
Patch Set 6:
(1 comment)
https://review.coreboot.org/#/c/32176/6/src/mainboard/google/hatch/variants/... File src/mainboard/google/hatch/variants/baseboard/gpio.c:
https://review.coreboot.org/#/c/32176/6/src/mainboard/google/hatch/variants/... PS6, Line 341: /* G5 : SD_CD# */ : PAD_CFG_NF(GPP_G5, NONE, DEEP, NF1), This patch will need a rebase, this is set PLTRST in https://review.coreboot.org/c/coreboot/+/32782
Krishna P Bhat D has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32176 )
Change subject: mb/google/hatch: Update GPIO settings ......................................................................
Patch Set 7:
(1 comment)
https://review.coreboot.org/#/c/32176/6/src/mainboard/google/hatch/variants/... File src/mainboard/google/hatch/variants/baseboard/gpio.c:
https://review.coreboot.org/#/c/32176/6/src/mainboard/google/hatch/variants/... PS6, Line 341: /* G5 : SD_CD# */ : PAD_CFG_NF(GPP_G5, NONE, DEEP, NF1),
This patch will need a rebase, this is set PLTRST in https://review.coreboot. […]
Done
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32176 )
Change subject: mb/google/hatch: Update GPIO settings ......................................................................
Patch Set 7:
(1 comment)
https://review.coreboot.org/#/c/32176/7/src/mainboard/google/hatch/variants/... File src/mainboard/google/hatch/variants/baseboard/gpio.c:
https://review.coreboot.org/#/c/32176/7/src/mainboard/google/hatch/variants/... PS7, Line 342: PLTRST Is there a reason SD_CD# is PLTRST and the other signals are DEEP?
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32176 )
Change subject: mb/google/hatch: Update GPIO settings ......................................................................
Patch Set 7:
(1 comment)
https://review.coreboot.org/#/c/32176/7/src/mainboard/google/hatch/variants/... File src/mainboard/google/hatch/variants/baseboard/gpio.c:
https://review.coreboot.org/#/c/32176/7/src/mainboard/google/hatch/variants/... PS7, Line 342: PLTRST
Is there a reason SD_CD# is PLTRST and the other signals are DEEP?
Probably because it was triggering interrupt storm when booting from S5?
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32176 )
Change subject: mb/google/hatch: Update GPIO settings ......................................................................
Patch Set 7: Code-Review+1
Krishna P Bhat D has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32176 )
Change subject: mb/google/hatch: Update GPIO settings ......................................................................
Patch Set 7:
(1 comment)
https://review.coreboot.org/#/c/32176/7/src/mainboard/google/hatch/variants/... File src/mainboard/google/hatch/variants/baseboard/gpio.c:
https://review.coreboot.org/#/c/32176/7/src/mainboard/google/hatch/variants/... PS7, Line 342: PLTRST
Probably because it was triggering interrupt storm when booting from S5?
I haven't checked the interrupt storm. I have verified with pad reset config set to DEEP for SD_CD# on hatch proto, we observe that the system waits for few seconds at "Starting kernel" and then resets, when we press the power button before the system enters G3. However, this issue is not observed on hatch EVT. This appears like a proto specific board issue.
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32176 )
Change subject: mb/google/hatch: Update GPIO settings ......................................................................
Patch Set 7:
(1 comment)
https://review.coreboot.org/#/c/32176/7/src/mainboard/google/hatch/variants/... File src/mainboard/google/hatch/variants/baseboard/gpio.c:
https://review.coreboot.org/#/c/32176/7/src/mainboard/google/hatch/variants/... PS7, Line 342: PLTRST
I haven't checked the interrupt storm. […]
As i understand you might need set PLTRST for proto alone and DEEP is final solution for EVT onwards ?
Krishna P Bhat D has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32176 )
Change subject: mb/google/hatch: Update GPIO settings ......................................................................
Patch Set 7:
(1 comment)
https://review.coreboot.org/#/c/32176/7/src/mainboard/google/hatch/variants/... File src/mainboard/google/hatch/variants/baseboard/gpio.c:
https://review.coreboot.org/#/c/32176/7/src/mainboard/google/hatch/variants/... PS7, Line 342: PLTRST
As i understand you might need set PLTRST for proto alone and DEEP is final solution for EVT onwards […]
I have checked on 1 hatch EVT board. More EVT boards have arrived, will check on few more and confirm the same.
Krishna P Bhat D has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32176 )
Change subject: mb/google/hatch: Update GPIO settings ......................................................................
Patch Set 7:
(1 comment)
https://review.coreboot.org/#/c/32176/7/src/mainboard/google/hatch/variants/... File src/mainboard/google/hatch/variants/baseboard/gpio.c:
https://review.coreboot.org/#/c/32176/7/src/mainboard/google/hatch/variants/... PS7, Line 342: PLTRST
I have checked on 1 hatch EVT board. […]
I further checked on EVT boards with pad reset config set to DEEP, I can see the issue reproducing. Hence we need to set the pad reset config as PLTRST for both proto and EVT systems.
Aamir Bohra has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32176 )
Change subject: mb/google/hatch: Update GPIO settings ......................................................................
Patch Set 7:
(1 comment)
https://review.coreboot.org/#/c/32176/7/src/mainboard/google/hatch/variants/... File src/mainboard/google/hatch/variants/baseboard/gpio.c:
https://review.coreboot.org/#/c/32176/7/src/mainboard/google/hatch/variants/... PS7, Line 346: PAD_CFG_NF(GPP_G7, DN_20K, DEEP, NF1), we did not want to set the SD WP from FSP either right? and had it changed in FSP https://chrome-internal-review.googlesource.com/c/chromeos/third_party/intel....
So we should not be setting it to native pad mode here as this would force SD WP.
Hello Subrata Banik, Aamir Bohra, Tim Wawrzynczak, Rizwan Qureshi, Shelley Chen, build bot (Jenkins), Furquan Shaikh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/32176
to look at the new patch set (#8).
Change subject: mb/google/hatch: Update GPIO settings ......................................................................
mb/google/hatch: Update GPIO settings
This patch updates the following GPIO settings. 1. Set Native termination for GPP_G0 - G4 SD card pins. 2. Set GPP_B19 to NF1.
BUG=b:123907904 TEST=Verified SD card functionality on hatch. Checked for SD detection, transferred files to and from SD card.
Change-Id: I4549ac7377d7f58f51cda0eb96a62604fd31d2f2 Signed-off-by: Krishna Prasad Bhat krishna.p.bhat.d@intel.com --- M src/mainboard/google/hatch/variants/baseboard/gpio.c 1 file changed, 7 insertions(+), 7 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/76/32176/8
Krishna P Bhat D has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32176 )
Change subject: mb/google/hatch: Update GPIO settings ......................................................................
Patch Set 8:
(1 comment)
https://review.coreboot.org/#/c/32176/7/src/mainboard/google/hatch/variants/... File src/mainboard/google/hatch/variants/baseboard/gpio.c:
https://review.coreboot.org/#/c/32176/7/src/mainboard/google/hatch/variants/... PS7, Line 346: PAD_CFG_NF(GPP_G7, DN_20K, DEEP, NF1),
we did not want to set the SD WP from FSP either right? and had it changed in FSP https://chrome-int […]
Yes Aamir. With SD WP Enable UPD disabled, we can leave the GPP_G7 pad as NC, FSP then does not configure it to native function.
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32176 )
Change subject: mb/google/hatch: Update GPIO settings ......................................................................
Patch Set 8: Code-Review+2
Aamir Bohra has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32176 )
Change subject: mb/google/hatch: Update GPIO settings ......................................................................
Patch Set 8: Code-Review+2
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32176 )
Change subject: mb/google/hatch: Update GPIO settings ......................................................................
Patch Set 8:
(2 comments)
https://review.coreboot.org/#/c/32176/8//COMMIT_MSG Commit Message:
https://review.coreboot.org/#/c/32176/8//COMMIT_MSG@7 PS8, Line 7: mb/google/hatch: Update GPIO settings Maybe more specific:
Update GPIO settings for SD card
https://review.coreboot.org/#/c/32176/8//COMMIT_MSG@12 PS8, Line 12: Where did you get this information from? Schematics? Data sheet?
Krishna P Bhat D has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32176 )
Change subject: mb/google/hatch: Update GPIO settings ......................................................................
Patch Set 8:
(2 comments)
https://review.coreboot.org/#/c/32176/8//COMMIT_MSG Commit Message:
https://review.coreboot.org/#/c/32176/8//COMMIT_MSG@7 PS8, Line 7: mb/google/hatch: Update GPIO settings
Maybe more specific: […]
Done
https://review.coreboot.org/#/c/32176/8//COMMIT_MSG@12 PS8, Line 12:
Where did you get this information from? Schematics? Data sheet?
Termination is recommendation for SD controller. GPP_B19 is being aligned here with FSP setting it to NF1.
Hello Subrata Banik, Aamir Bohra, Tim Wawrzynczak, Rizwan Qureshi, Shelley Chen, build bot (Jenkins), Furquan Shaikh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/32176
to look at the new patch set (#9).
Change subject: mb/google/hatch: Update GPIO settings for SD card and SPI1 Chip select ......................................................................
mb/google/hatch: Update GPIO settings for SD card and SPI1 Chip select
This patch updates the following GPIO settings. 1. Set Native termination for GPP_G0 - G4 SD card pins. 2. Set GPP_B19 to NF1.
BUG=b:123907904 TEST=Verified SD card functionality on hatch. Checked for SD detection, transferred files to and from SD card.
Change-Id: I4549ac7377d7f58f51cda0eb96a62604fd31d2f2 Signed-off-by: Krishna Prasad Bhat krishna.p.bhat.d@intel.com --- M src/mainboard/google/hatch/variants/baseboard/gpio.c 1 file changed, 7 insertions(+), 7 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/76/32176/9
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32176 )
Change subject: mb/google/hatch: Update GPIO settings for SD card and SPI1 Chip select ......................................................................
Patch Set 9: Code-Review+2
Martin Roth has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/32176 )
Change subject: mb/google/hatch: Update GPIO settings for SD card and SPI1 Chip select ......................................................................
mb/google/hatch: Update GPIO settings for SD card and SPI1 Chip select
This patch updates the following GPIO settings. 1. Set Native termination for GPP_G0 - G4 SD card pins. 2. Set GPP_B19 to NF1.
BUG=b:123907904 TEST=Verified SD card functionality on hatch. Checked for SD detection, transferred files to and from SD card.
Change-Id: I4549ac7377d7f58f51cda0eb96a62604fd31d2f2 Signed-off-by: Krishna Prasad Bhat krishna.p.bhat.d@intel.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/32176 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Tim Wawrzynczak twawrzynczak@chromium.org --- M src/mainboard/google/hatch/variants/baseboard/gpio.c 1 file changed, 7 insertions(+), 7 deletions(-)
Approvals: build bot (Jenkins): Verified Tim Wawrzynczak: Looks good to me, approved
diff --git a/src/mainboard/google/hatch/variants/baseboard/gpio.c b/src/mainboard/google/hatch/variants/baseboard/gpio.c index 56ae601..528ab02 100644 --- a/src/mainboard/google/hatch/variants/baseboard/gpio.c +++ b/src/mainboard/google/hatch/variants/baseboard/gpio.c @@ -99,8 +99,8 @@ PAD_CFG_NF(GPP_B17, NONE, DEEP, NF1), /* B18 : H1_SLAVE_SPI_MOSI_R */ PAD_CFG_NF(GPP_B18, NONE, DEEP, NF1), - /* B19 : GPP_B19 ==> NC */ - PAD_NC(GPP_B19, NONE), + /* B19 : Set to NF1 to match FSP setting it to NF1, i.e., GSPI1_CS0# */ + PAD_CFG_NF(GPP_B19, NONE, DEEP, NF1), /* B20 : PCH_SPI_FPMCU_CLK_R */ PAD_CFG_NF(GPP_B20, NONE, DEEP, NF1), /* B21 : PCH_SPI_FPMCU_MISO */ @@ -311,15 +311,15 @@ PAD_NC(GPP_F23, NONE),
/* G0 : SD_CMD */ - PAD_CFG_NF(GPP_G0, NONE, DEEP, NF1), + PAD_CFG_NF(GPP_G0, NATIVE, DEEP, NF1), /* G1 : SD_DATA0 */ - PAD_CFG_NF(GPP_G1, NONE, DEEP, NF1), + PAD_CFG_NF(GPP_G1, NATIVE, DEEP, NF1), /* G2 : SD_DATA1 */ - PAD_CFG_NF(GPP_G2, NONE, DEEP, NF1), + PAD_CFG_NF(GPP_G2, NATIVE, DEEP, NF1), /* G3 : SD_DATA2 */ - PAD_CFG_NF(GPP_G3, NONE, DEEP, NF1), + PAD_CFG_NF(GPP_G3, NATIVE, DEEP, NF1), /* G4 : SD_DATA3 */ - PAD_CFG_NF(GPP_G4, NONE, DEEP, NF1), + PAD_CFG_NF(GPP_G4, NATIVE, DEEP, NF1), /* G5 : SD_CD# */ PAD_CFG_NF(GPP_G5, NONE, PLTRST, NF1), /* G6 : SD_CLK */