Change in coreboot[master]: [TESTME/RFC] mb/google/kahlee: move SMI/SCI GPIO setup to ramstage
Felix Held has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/48608 )
Change subject: [TESTME/RFC] mb/google/kahlee: move SMI/SCI GPIO setup to ramstage ......................................................................
[TESTME/RFC] mb/google/kahlee: move SMI/SCI GPIO setup to ramstage
SMIs and SCIs aren't used before ramstage or the OS, so there should be no need to already set them up in romstage. Not using this GPIO configuration functionality allows untangling the GPIO and smi_util code and only linking smi_util in ramstage in follow-up patches. In romstage the pins get initialized as inputs with pull-up, so that at least that part still matches the configuration before this patch.
BUG=b:175386410 TEST=UNTESTED
Change-Id: I733bb91ef60dc66093781a376a2e9837f5209671 Signed-off-by: Felix Held felix-coreboot@felixheld.de --- M src/mainboard/google/kahlee/variants/baseboard/gpio.c 1 file changed, 15 insertions(+), 6 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/08/48608/1
diff --git a/src/mainboard/google/kahlee/variants/baseboard/gpio.c b/src/mainboard/google/kahlee/variants/baseboard/gpio.c index 59d7631..8628074 100644 --- a/src/mainboard/google/kahlee/variants/baseboard/gpio.c +++ b/src/mainboard/google/kahlee/variants/baseboard/gpio.c @@ -15,8 +15,8 @@ /* GPIO_4 - EN_PP3300_WLAN */ PAD_GPO(GPIO_4, HIGH),
- /* GPIO_6 - APU_RST_L / EC_SMI_ODL, SMI */ - PAD_SMI(GPIO_6, PULL_UP, LEVEL_LOW), + /* GPIO_6 - APU_RST_L / EC_SMI_ODL, SMI gets configured in ramstage */ + PAD_GPI(GPIO_6, PULL_UP),
/* GPIO_9 - H1_PCH_INT_ODL */ PAD_INT(GPIO_9, PULL_UP, EDGE_LOW, STATUS), @@ -24,11 +24,11 @@ /* GPIO_15 - EC_IN_RW_OD */ PAD_GPI(GPIO_15, PULL_UP),
- /* GPIO_22 - EC_SCI_ODL, SCI */ - PAD_SCI(GPIO_22, PULL_UP, EDGE_LOW), + /* GPIO_22 - EC_SCI_ODL, SCI gets configured in ramstage */ + PAD_GPI(GPIO_22, PULL_UP),
- /* GPIO_24 - EC_PCH_WAKE_L, SCI */ - PAD_SCI(GPIO_24, PULL_UP, EDGE_LOW), + /* GPIO_24 - EC_PCH_WAKE_L, SCI gets configured in ramstage */ + PAD_GPI(GPIO_24, PULL_UP),
/* GPIO_26 - APU_PCIE_RST_L */ PAD_NF(GPIO_26, PCIE_RST_L, PULL_NONE), @@ -90,6 +90,9 @@ /* GPIO_5 - PCH_TRACKPAD_INT_3V3_ODL, SCI */ PAD_SCI(GPIO_5, PULL_UP, EDGE_LOW),
+ /* GPIO_6 - APU_RST_L / EC_SMI_ODL, SMI */ + PAD_SMI(GPIO_6, PULL_UP, LEVEL_LOW), + /* GPIO_7 - APU_PWROK_OD (currently not used) */ PAD_GPI(GPIO_7, PULL_UP),
@@ -129,6 +132,12 @@ /* GPIO_21 - APU_PEN_INT_ODL, SCI */ PAD_SCI(GPIO_21, PULL_UP, EDGE_LOW),
+ /* GPIO_22 - EC_SCI_ODL, SCI */ + PAD_SCI(GPIO_22, PULL_UP, EDGE_LOW), + + /* GPIO_24 - EC_PCH_WAKE_L, SCI */ + PAD_SCI(GPIO_24, PULL_UP, EDGE_LOW), + /* GPIO_25 - SD_CD */ PAD_NF(GPIO_25, SD0_CD, PULL_UP),
Martin Roth has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48608 )
Change subject: [TESTME/RFC] mb/google/kahlee: move SMI/SCI GPIO setup to ramstage ......................................................................
Patch Set 1: Code-Review+2
Hello build bot (Jenkins), Martin Roth, Marshall Dawson,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/48608
to look at the new patch set (#2).
Change subject: mb/google/kahlee: move SMI/SCI GPIO setup to ramstage ......................................................................
mb/google/kahlee: move SMI/SCI GPIO setup to ramstage
SMIs and SCIs aren't used before ramstage or the OS, so there should be no need to already set them up in romstage. Not using this GPIO configuration functionality allows untangling the GPIO and smi_util code and only linking smi_util in ramstage in follow-up patches. In romstage the pins get initialized as inputs with pull-up, so that at least that part still matches the configuration before this patch.
BUG=b:175386410
Change-Id: I733bb91ef60dc66093781a376a2e9837f5209671 Signed-off-by: Felix Held felix-coreboot@felixheld.de --- M src/mainboard/google/kahlee/variants/baseboard/gpio.c 1 file changed, 15 insertions(+), 6 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/08/48608/2
Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/48608 )
Change subject: mb/google/kahlee: move SMI/SCI GPIO setup to ramstage ......................................................................
mb/google/kahlee: move SMI/SCI GPIO setup to ramstage
SMIs and SCIs aren't used before ramstage or the OS, so there should be no need to already set them up in romstage. Not using this GPIO configuration functionality allows untangling the GPIO and smi_util code and only linking smi_util in ramstage in follow-up patches. In romstage the pins get initialized as inputs with pull-up, so that at least that part still matches the configuration before this patch.
BUG=b:175386410
Change-Id: I733bb91ef60dc66093781a376a2e9837f5209671 Signed-off-by: Felix Held felix-coreboot@felixheld.de Reviewed-on: https://review.coreboot.org/c/coreboot/+/48608 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Martin Roth martinroth@google.com --- M src/mainboard/google/kahlee/variants/baseboard/gpio.c 1 file changed, 15 insertions(+), 6 deletions(-)
Approvals: build bot (Jenkins): Verified Martin Roth: Looks good to me, approved
diff --git a/src/mainboard/google/kahlee/variants/baseboard/gpio.c b/src/mainboard/google/kahlee/variants/baseboard/gpio.c index 59d7631..8628074 100644 --- a/src/mainboard/google/kahlee/variants/baseboard/gpio.c +++ b/src/mainboard/google/kahlee/variants/baseboard/gpio.c @@ -15,8 +15,8 @@ /* GPIO_4 - EN_PP3300_WLAN */ PAD_GPO(GPIO_4, HIGH),
- /* GPIO_6 - APU_RST_L / EC_SMI_ODL, SMI */ - PAD_SMI(GPIO_6, PULL_UP, LEVEL_LOW), + /* GPIO_6 - APU_RST_L / EC_SMI_ODL, SMI gets configured in ramstage */ + PAD_GPI(GPIO_6, PULL_UP),
/* GPIO_9 - H1_PCH_INT_ODL */ PAD_INT(GPIO_9, PULL_UP, EDGE_LOW, STATUS), @@ -24,11 +24,11 @@ /* GPIO_15 - EC_IN_RW_OD */ PAD_GPI(GPIO_15, PULL_UP),
- /* GPIO_22 - EC_SCI_ODL, SCI */ - PAD_SCI(GPIO_22, PULL_UP, EDGE_LOW), + /* GPIO_22 - EC_SCI_ODL, SCI gets configured in ramstage */ + PAD_GPI(GPIO_22, PULL_UP),
- /* GPIO_24 - EC_PCH_WAKE_L, SCI */ - PAD_SCI(GPIO_24, PULL_UP, EDGE_LOW), + /* GPIO_24 - EC_PCH_WAKE_L, SCI gets configured in ramstage */ + PAD_GPI(GPIO_24, PULL_UP),
/* GPIO_26 - APU_PCIE_RST_L */ PAD_NF(GPIO_26, PCIE_RST_L, PULL_NONE), @@ -90,6 +90,9 @@ /* GPIO_5 - PCH_TRACKPAD_INT_3V3_ODL, SCI */ PAD_SCI(GPIO_5, PULL_UP, EDGE_LOW),
+ /* GPIO_6 - APU_RST_L / EC_SMI_ODL, SMI */ + PAD_SMI(GPIO_6, PULL_UP, LEVEL_LOW), + /* GPIO_7 - APU_PWROK_OD (currently not used) */ PAD_GPI(GPIO_7, PULL_UP),
@@ -129,6 +132,12 @@ /* GPIO_21 - APU_PEN_INT_ODL, SCI */ PAD_SCI(GPIO_21, PULL_UP, EDGE_LOW),
+ /* GPIO_22 - EC_SCI_ODL, SCI */ + PAD_SCI(GPIO_22, PULL_UP, EDGE_LOW), + + /* GPIO_24 - EC_PCH_WAKE_L, SCI */ + PAD_SCI(GPIO_24, PULL_UP, EDGE_LOW), + /* GPIO_25 - SD_CD */ PAD_NF(GPIO_25, SD0_CD, PULL_UP),
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48608 )
Change subject: mb/google/kahlee: move SMI/SCI GPIO setup to ramstage ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/48608/3/src/mainboard/google/kahlee... File src/mainboard/google/kahlee/variants/baseboard/gpio.c:
https://review.coreboot.org/c/coreboot/+/48608/3/src/mainboard/google/kahlee... PS3, Line 19: PAD_GPI I know this change is already submitted. But, why configure the pads in early stage at all? Bootblock is meant to configure the bare minimum pads required to get upto ramstage. None of the EC_SMI_ODL/EC_SCI_ODL/EC_PCH_WAKE_L need to be configured in early stages. We should just drop their configuration in bootblock and rely on ramstage to do the right thing.
Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48608 )
Change subject: mb/google/kahlee: move SMI/SCI GPIO setup to ramstage ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/48608/3/src/mainboard/google/kahlee... File src/mainboard/google/kahlee/variants/baseboard/gpio.c:
https://review.coreboot.org/c/coreboot/+/48608/3/src/mainboard/google/kahlee... PS3, Line 19: PAD_GPI
I know this change is already submitted. […]
since I don't have a google/kahlee variant to test this on, i just went the safe route here. if you can verify that removing those 3 GPIs in here doesn't have any negative impact, I'd like you to push a patch that removes them
-
Felix Held (Code Review) -
Furquan Shaikh (Code Review)
-
Martin Roth (Code Review)