Change in coreboot[master]: mb/google/zork: Move PCIE_RST1_L deassertion to happen early for dalboz
Hello Aaron Durbin,
I'd like you to do a code review. Please visit
https://review.coreboot.org/c/coreboot/+/42936
to review the following change.
Change subject: mb/google/zork: Move PCIE_RST1_L deassertion to happen early for dalboz ......................................................................
mb/google/zork: Move PCIE_RST1_L deassertion to happen early for dalboz
This change moves PCIE_RST1_L deassertion to happen as part of variant_pcie_power_reset_configure() instead of variant_romstage_entry() since romstage is guaranteed to run 100ms+ after PP3300_NVME is enabled. This is one of the first things that coreboot on x86 does as part of early mainboard configuration.
Additionally, this change also drops deassertion of PCIE_RST0_L on bid 1 for dalboz since PCIE_RST0_L is already deasserted much earlier in the boot flow.
BUG=b:152582706
Signed-off-by: Furquan Shaikh furquan@google.com Change-Id: Ib734aa6ff664268e68388b1997ddce676504f8d2 Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/third_party/coreboot/+... Reviewed-by: Aaron Durbin adurbin@google.com Commit-Queue: Aaron Durbin adurbin@google.com Tested-by: Aaron Durbin adurbin@google.com --- M src/mainboard/google/zork/variants/baseboard/gpio_baseboard_dalboz.c M src/mainboard/google/zork/variants/dalboz/Makefile.inc D src/mainboard/google/zork/variants/dalboz/romstage.c 3 files changed, 3 insertions(+), 27 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/36/42936/1
diff --git a/src/mainboard/google/zork/variants/baseboard/gpio_baseboard_dalboz.c b/src/mainboard/google/zork/variants/baseboard/gpio_baseboard_dalboz.c index ba50362..2b6f516 100644 --- a/src/mainboard/google/zork/variants/baseboard/gpio_baseboard_dalboz.c +++ b/src/mainboard/google/zork/variants/baseboard/gpio_baseboard_dalboz.c @@ -270,6 +270,9 @@ { uint32_t board_version;
+ /* Deassert PCIE_RST1_L */ + gpio_set(GPIO_27, 1); + if (!google_chromeec_cbi_get_board_version(&board_version) && (board_version >= CONFIG_VARIANT_MIN_BOARD_ID_V3_SCHEMATICS)) wifi_power_reset_configure_v3(); diff --git a/src/mainboard/google/zork/variants/dalboz/Makefile.inc b/src/mainboard/google/zork/variants/dalboz/Makefile.inc index a616e2f..51d19fe 100644 --- a/src/mainboard/google/zork/variants/dalboz/Makefile.inc +++ b/src/mainboard/google/zork/variants/dalboz/Makefile.inc @@ -2,7 +2,5 @@
subdirs-y += ./spd
-romstage-y += romstage.c - ramstage-y += gpio.c ramstage-y += variant.c diff --git a/src/mainboard/google/zork/variants/dalboz/romstage.c b/src/mainboard/google/zork/variants/dalboz/romstage.c deleted file mode 100644 index 42e36c4..0000000 --- a/src/mainboard/google/zork/variants/dalboz/romstage.c +++ /dev/null @@ -1,25 +0,0 @@ -/* SPDX-License-Identifier: GPL-2.0-or-later */ - -#include <stddef.h> -#include <soc/romstage.h> -#include <baseboard/variants.h> -#include <ec/google/chromeec/ec.h> -#include <gpio.h> -#include <soc/gpio.h> -#include <variant/gpio.h> - -void variant_romstage_entry(void) -{ - uint32_t board_version; - - if (google_chromeec_cbi_get_board_version(&board_version)) - board_version = 1; - - if (board_version < 2) { - /* SET PCIE_RST0_L HIGH */ - gpio_set(WIFI_PCIE_RESET_L, 1); - } else { - /* SET PCIE_RST1_L HIGH */ - gpio_set(PCIE_RST1_L, 1); - } -}
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42936 )
Change subject: mb/google/zork: Move PCIE_RST1_L deassertion to happen early for dalboz ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/42936/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/42936/1//COMMIT_MSG@15 PS1, Line 15: bid : 1 What is *bid*? bit?
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42936 )
Change subject: mb/google/zork: Move PCIE_RST1_L deassertion to happen early for dalboz ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/42936/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/42936/1//COMMIT_MSG@15 PS1, Line 15: bid : 1
What is *bid*? bit?
Reading the other change-sets, it’s *board id*.
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42936 )
Change subject: mb/google/zork: Move PCIE_RST1_L deassertion to happen early for dalboz ......................................................................
Patch Set 1: Code-Review+2
Furquan Shaikh has submitted this change. ( https://review.coreboot.org/c/coreboot/+/42936 )
Change subject: mb/google/zork: Move PCIE_RST1_L deassertion to happen early for dalboz ......................................................................
mb/google/zork: Move PCIE_RST1_L deassertion to happen early for dalboz
This change moves PCIE_RST1_L deassertion to happen as part of variant_pcie_power_reset_configure() instead of variant_romstage_entry() since romstage is guaranteed to run 100ms+ after PP3300_NVME is enabled. This is one of the first things that coreboot on x86 does as part of early mainboard configuration.
Additionally, this change also drops deassertion of PCIE_RST0_L on bid 1 for dalboz since PCIE_RST0_L is already deasserted much earlier in the boot flow.
BUG=b:152582706
Signed-off-by: Furquan Shaikh furquan@google.com Change-Id: Ib734aa6ff664268e68388b1997ddce676504f8d2 Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/third_party/coreboot/+... Reviewed-by: Aaron Durbin adurbin@google.com Commit-Queue: Aaron Durbin adurbin@google.com Tested-by: Aaron Durbin adurbin@google.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/42936 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Aaron Durbin adurbin@chromium.org --- M src/mainboard/google/zork/variants/baseboard/gpio_baseboard_dalboz.c M src/mainboard/google/zork/variants/dalboz/Makefile.inc D src/mainboard/google/zork/variants/dalboz/romstage.c 3 files changed, 3 insertions(+), 27 deletions(-)
Approvals: build bot (Jenkins): Verified Aaron Durbin: Looks good to me, approved
diff --git a/src/mainboard/google/zork/variants/baseboard/gpio_baseboard_dalboz.c b/src/mainboard/google/zork/variants/baseboard/gpio_baseboard_dalboz.c index 5874e89..c243876 100644 --- a/src/mainboard/google/zork/variants/baseboard/gpio_baseboard_dalboz.c +++ b/src/mainboard/google/zork/variants/baseboard/gpio_baseboard_dalboz.c @@ -268,6 +268,9 @@ { uint32_t board_version;
+ /* Deassert PCIE_RST1_L */ + gpio_set(GPIO_27, 1); + if (!google_chromeec_cbi_get_board_version(&board_version) && (board_version >= CONFIG_VARIANT_MIN_BOARD_ID_V3_SCHEMATICS)) wifi_power_reset_configure_v3(); diff --git a/src/mainboard/google/zork/variants/dalboz/Makefile.inc b/src/mainboard/google/zork/variants/dalboz/Makefile.inc index a616e2f..51d19fe 100644 --- a/src/mainboard/google/zork/variants/dalboz/Makefile.inc +++ b/src/mainboard/google/zork/variants/dalboz/Makefile.inc @@ -2,7 +2,5 @@
subdirs-y += ./spd
-romstage-y += romstage.c - ramstage-y += gpio.c ramstage-y += variant.c diff --git a/src/mainboard/google/zork/variants/dalboz/romstage.c b/src/mainboard/google/zork/variants/dalboz/romstage.c deleted file mode 100644 index 42e36c4..0000000 --- a/src/mainboard/google/zork/variants/dalboz/romstage.c +++ /dev/null @@ -1,25 +0,0 @@ -/* SPDX-License-Identifier: GPL-2.0-or-later */ - -#include <stddef.h> -#include <soc/romstage.h> -#include <baseboard/variants.h> -#include <ec/google/chromeec/ec.h> -#include <gpio.h> -#include <soc/gpio.h> -#include <variant/gpio.h> - -void variant_romstage_entry(void) -{ - uint32_t board_version; - - if (google_chromeec_cbi_get_board_version(&board_version)) - board_version = 1; - - if (board_version < 2) { - /* SET PCIE_RST0_L HIGH */ - gpio_set(WIFI_PCIE_RESET_L, 1); - } else { - /* SET PCIE_RST1_L HIGH */ - gpio_set(PCIE_RST1_L, 1); - } -}
-
Aaron Durbin (Code Review) -
Furquan Shaikh (Code Review)
-
Paul Menzel (Code Review)