Change in ...coreboot[master]: mb/google/hatch: Add correct GPIO programming for GPP_C0 to GPP_C7
Hello Maulik V Vaghela,
I'd like you to do a code review. Please visit
https://review.coreboot.org/c/coreboot/+/31640
to review the following change.
Change subject: mb/google/hatch: Add correct GPIO programming for GPP_C0 to GPP_C7 ......................................................................
mb/google/hatch: Add correct GPIO programming for GPP_C0 to GPP_C7
Coreboot did not program all GPIOs from C0 to C7 correctly which are SMBUS GPIO for CSME. Some of the GPIOs are left in default mode which is native function but we need to configure as GPIO mode and provide proper configuration as per schematic. After fixing GPIO, CSME power gating issue also gets fixed since all SMBUS GPIOs are configured properly
BUG=b:123702553 BRANCH=none TEST=Check on hatch board. CSME was not getting power gated for s0ix. After applying this patch CSME is power gated now
Change-Id: I5c6b9310dcc7bade0023abd5524781ce71df28be Signed-off-by: Maulik V Vaghela maulik.v.vaghela@intel.corp-partner.google.com --- M src/mainboard/google/hatch/variants/baseboard/gpio.c 1 file changed, 14 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/40/31640/1
diff --git a/src/mainboard/google/hatch/variants/baseboard/gpio.c b/src/mainboard/google/hatch/variants/baseboard/gpio.c index 769d2ab..7387f92 100644 --- a/src/mainboard/google/hatch/variants/baseboard/gpio.c +++ b/src/mainboard/google/hatch/variants/baseboard/gpio.c @@ -45,10 +45,22 @@ PAD_CFG_NF(GPP_B17, NONE, DEEP, NF1), /* H1_SLAVE_SPI_MOSI_R */ PAD_CFG_NF(GPP_B18, NONE, DEEP, NF1), - /* TOUCHSCREEN_DIS_L */ - PAD_CFG_GPO(GPP_C4, 0, DEEP), + /* GPP_C0 => NC */ + PAD_NC(GPP_C0, NONE), /* PCIE_14_WLAN_WAKE_ODL */ PAD_CFG_GPI_SCI_LOW(GPP_C1, NONE, DEEP, EDGE_SINGLE), + /* GPP_C2 => NC */ + PAD_NC(GPP_C2, NONE), + /* WLAN_OFF_L */ + PAD_CFG_GPO(GPP_C3, 1, DEEP), + /* TOUCHSCREEN_DIS_L */ + PAD_CFG_GPO(GPP_C4, 1, DEEP), + /* GPP_C5 => NC */ + PAD_NC(GPP_C5, NONE), + /* PEN_PDCT_OD_L */ + PAD_CFG_GPI(GPP_C6, NONE, DEEP), + /* PEN_IRQ_OD_L */ + PAD_CFG_GPI(GPP_C7, NONE, DEEP), /* GPP_C10_TP */ PAD_NC(GPP_C10, DN_20K), /* GPP_C11_TP */
Hello Subrata Banik, Maulik V Vaghela, Rizwan Qureshi, build bot (Jenkins), Furquan Shaikh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/31640
to look at the new patch set (#2).
Change subject: mb/google/hatch: Add GPIO programming for GPP_C0 to GPP_C7 ......................................................................
mb/google/hatch: Add GPIO programming for GPP_C0 to GPP_C7
coreboot did not program all GPIOs from C0 to C7 correctly which are SMBUS GPIO. Some of the GPIOs are left in default mode which is native function but we need to configure as GPIO mode and provide proper configuration as per schematic.
After fixing GPIO, CSME power gating issue also gets fixed since SMBUS was not getting idle due to GPIO configuration and CSME was not getting power gated due to SMBUS.
BUG=b:123702553 BRANCH=none TEST=Check on hatch board. CSME was not getting power gated for s0ix. After applying this patch CSME is power gated now
Change-Id: I5c6b9310dcc7bade0023abd5524781ce71df28be Signed-off-by: Maulik V Vaghela maulik.v.vaghela@intel.corp-partner.google.com --- M src/mainboard/google/hatch/variants/baseboard/gpio.c 1 file changed, 14 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/40/31640/2
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31640 )
Change subject: mb/google/hatch: Add GPIO programming for GPP_C0 to GPP_C7 ......................................................................
Patch Set 2: Code-Review+2
(2 comments)
https://review.coreboot.org/#/c/31640/2/src/mainboard/google/hatch/variants/... File src/mainboard/google/hatch/variants/baseboard/gpio.c:
https://review.coreboot.org/#/c/31640/2/src/mainboard/google/hatch/variants/... PS2, Line 61: PAD_CFG_GPI(GPP_C6, NONE, DEEP), This might have to be configured differently depending upon usage. But we can revisit that later.
https://review.coreboot.org/#/c/31640/2/src/mainboard/google/hatch/variants/... PS2, Line 63: NONE This will have to be fixed once FSP behavior is fixed.
Patrick Georgi has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/31640 )
Change subject: mb/google/hatch: Add GPIO programming for GPP_C0 to GPP_C7 ......................................................................
mb/google/hatch: Add GPIO programming for GPP_C0 to GPP_C7
coreboot did not program all GPIOs from C0 to C7 correctly which are SMBUS GPIO. Some of the GPIOs are left in default mode which is native function but we need to configure as GPIO mode and provide proper configuration as per schematic.
After fixing GPIO, CSME power gating issue also gets fixed since SMBUS was not getting idle due to GPIO configuration and CSME was not getting power gated due to SMBUS.
BUG=b:123702553 BRANCH=none TEST=Check on hatch board. CSME was not getting power gated for s0ix. After applying this patch CSME is power gated now
Change-Id: I5c6b9310dcc7bade0023abd5524781ce71df28be Signed-off-by: Maulik V Vaghela maulik.v.vaghela@intel.corp-partner.google.com Reviewed-on: https://review.coreboot.org/c/31640 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Furquan Shaikh furquan@google.com --- M src/mainboard/google/hatch/variants/baseboard/gpio.c 1 file changed, 14 insertions(+), 2 deletions(-)
Approvals: build bot (Jenkins): Verified Furquan Shaikh: 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 b88f459..857204e 100644 --- a/src/mainboard/google/hatch/variants/baseboard/gpio.c +++ b/src/mainboard/google/hatch/variants/baseboard/gpio.c @@ -45,10 +45,22 @@ PAD_CFG_NF(GPP_B17, NONE, DEEP, NF1), /* H1_SLAVE_SPI_MOSI_R */ PAD_CFG_NF(GPP_B18, NONE, DEEP, NF1), - /* TOUCHSCREEN_DIS_L */ - PAD_CFG_GPO(GPP_C4, 0, DEEP), + /* GPP_C0 => NC */ + PAD_NC(GPP_C0, NONE), /* PCIE_14_WLAN_WAKE_ODL */ PAD_CFG_GPI_SCI_LOW(GPP_C1, NONE, DEEP, EDGE_SINGLE), + /* GPP_C2 => NC */ + PAD_NC(GPP_C2, NONE), + /* WLAN_OFF_L */ + PAD_CFG_GPO(GPP_C3, 1, DEEP), + /* TOUCHSCREEN_DIS_L */ + PAD_CFG_GPO(GPP_C4, 1, DEEP), + /* GPP_C5 => NC */ + PAD_NC(GPP_C5, NONE), + /* PEN_PDCT_OD_L */ + PAD_CFG_GPI(GPP_C6, NONE, DEEP), + /* PEN_IRQ_OD_L */ + PAD_CFG_GPI_APIC(GPP_C7, NONE, DEEP, LEVEL, NONE), /* GPP_C10_TP */ PAD_NC(GPP_C10, DN_20K), /* GPP_C11_TP */
-
Furquan Shaikh (Code Review) -
Maulik V Vaghela (Code Review)
-
Patrick Georgi (Code Review)