Furquan Shaikh has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/43475 )
Change subject: mb/google/zork: Drop variant_romstage_gpio_table() ......................................................................
mb/google/zork: Drop variant_romstage_gpio_table()
gpio_set_stage_rom table is now configuring only PCIe related GPIOs in romstage. This change moves the configuration of PCIe related GPIOs to variant_pcie_gpio_configure() to keep all the configuration for WiFi and non-WiFi PCIe pads in one place. It also drops the functin variant_romstage_gpio_table() as it is unused.
BUG=b:154351731
Signed-off-by: Furquan Shaikh furquan@google.com Change-Id: Ib1c41ba141dce6b52b6e0a250a3aa07c296068aa --- M src/mainboard/google/zork/romstage.c M src/mainboard/google/zork/variants/baseboard/gpio_baseboard_dalboz.c M src/mainboard/google/zork/variants/baseboard/gpio_baseboard_trembyle.c M src/mainboard/google/zork/variants/baseboard/include/baseboard/variants.h 4 files changed, 37 insertions(+), 53 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/75/43475/1
diff --git a/src/mainboard/google/zork/romstage.c b/src/mainboard/google/zork/romstage.c index bd4c96e..39f23cf 100644 --- a/src/mainboard/google/zork/romstage.c +++ b/src/mainboard/google/zork/romstage.c @@ -7,10 +7,5 @@
void mainboard_romstage_entry_s3(int s3_resume) { - size_t num_gpios; - const struct soc_amd_gpio *gpios; - - gpios = variant_romstage_gpio_table(&num_gpios); - program_gpios(gpios, num_gpios); - variant_pcie_power_reset_configure(); + variant_pcie_gpio_configure(); } 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 a3684d0..3f1da03 100644 --- a/src/mainboard/google/zork/variants/baseboard/gpio_baseboard_dalboz.c +++ b/src/mainboard/google/zork/variants/baseboard/gpio_baseboard_dalboz.c @@ -9,21 +9,6 @@ #include <boardid.h> #include <variant/gpio.h>
-static const struct soc_amd_gpio gpio_set_stage_rom[] = { - /* PCIE_RST1_L - Variable timings (May remove) */ - PAD_NF(GPIO_27, PCIE_RST1_L, PULL_NONE), - /* NVME_AUX_RESET_L */ - PAD_GPO(GPIO_40, HIGH), - /* CLK_REQ0_L - WIFI */ - PAD_NF(GPIO_92, CLK_REQ0_L, PULL_UP), - /* CLK_REQ1_L - SD Card */ - PAD_NF(GPIO_115, CLK_REQ1_L, PULL_UP), - /* CLK_REQ2_L - NVMe */ - PAD_NF(GPIO_116, CLK_REQ2_L, PULL_UP), - /* SD_AUX_RESET_L */ - PAD_GPO(GPIO_142, HIGH), -}; - static const struct soc_amd_gpio gpio_set_stage_ram[] = {
/* PWR_BTN_L */ @@ -140,13 +125,6 @@ };
const __weak -struct soc_amd_gpio *variant_romstage_gpio_table(size_t *size) -{ - *size = ARRAY_SIZE(gpio_set_stage_rom); - return gpio_set_stage_rom: -} - -const __weak struct soc_amd_gpio *variant_base_gpio_table(size_t *size) { *size = ARRAY_SIZE(gpio_set_stage_ram); @@ -247,8 +225,25 @@ gpio_set(GPIO_42, 1); }
-__weak void variant_pcie_power_reset_configure(void) +__weak void variant_pcie_gpio_configure(void) { + static const struct soc_amd_gpio pcie_gpio_table[] = { + /* PCIE_RST1_L - Variable timings (May remove) */ + PAD_NF(GPIO_27, PCIE_RST1_L, PULL_NONE), + /* NVME_AUX_RESET_L */ + PAD_GPO(GPIO_40, HIGH), + /* CLK_REQ0_L - WIFI */ + PAD_NF(GPIO_92, CLK_REQ0_L, PULL_UP), + /* CLK_REQ1_L - SD Card */ + PAD_NF(GPIO_115, CLK_REQ1_L, PULL_UP), + /* CLK_REQ2_L - NVMe */ + PAD_NF(GPIO_116, CLK_REQ2_L, PULL_UP), + /* SD_AUX_RESET_L */ + PAD_GPO(GPIO_142, HIGH), + }; + + program_gpios(pcie_gpio_table, ARRAY_SIZE(pcie_gpio_table)); + /* Deassert PCIE_RST1_L */ gpio_set(GPIO_27, 1);
diff --git a/src/mainboard/google/zork/variants/baseboard/gpio_baseboard_trembyle.c b/src/mainboard/google/zork/variants/baseboard/gpio_baseboard_trembyle.c index b3cbf67..a998cd7 100644 --- a/src/mainboard/google/zork/variants/baseboard/gpio_baseboard_trembyle.c +++ b/src/mainboard/google/zork/variants/baseboard/gpio_baseboard_trembyle.c @@ -9,19 +9,6 @@ #include <boardid.h> #include <variant/gpio.h>
-static const struct soc_amd_gpio gpio_set_stage_rom[] = { - /* NVME_AUX_RESET_L */ - PAD_GPO(GPIO_40, HIGH), - /* CLK_REQ0_L - WIFI */ - PAD_NF(GPIO_92, CLK_REQ0_L, PULL_UP), - /* CLK_REQ1_L - SD Card */ - PAD_NF(GPIO_115, CLK_REQ1_L, PULL_UP), - /* CLK_REQ4_L - SSD */ - PAD_NF(GPIO_132, CLK_REQ4_L, PULL_UP), - /* SD_AUX_RESET_L */ - PAD_GPO(GPIO_142, HIGH), -}; - static const struct soc_amd_gpio gpio_set_stage_ram[] = {
/* PWR_BTN_L */ @@ -136,13 +123,6 @@ };
const __weak -struct soc_amd_gpio *variant_romstage_gpio_table(size_t *size) -{ - *size = ARRAY_SIZE(gpio_set_stage_rom); - return gpio_set_stage_rom; -} - -const __weak struct soc_amd_gpio *variant_base_gpio_table(size_t *size) { *size = ARRAY_SIZE(gpio_set_stage_ram); @@ -243,8 +223,23 @@ gpio_set(GPIO_42, 1); }
-__weak void variant_pcie_power_reset_configure(void) +__weak void variant_pcie_gpio_configure(void) { + static const struct soc_amd_gpio pcie_gpio_table[] = { + /* NVME_AUX_RESET_L */ + PAD_GPO(GPIO_40, HIGH), + /* CLK_REQ0_L - WIFI */ + PAD_NF(GPIO_92, CLK_REQ0_L, PULL_UP), + /* CLK_REQ1_L - SD Card */ + PAD_NF(GPIO_115, CLK_REQ1_L, PULL_UP), + /* CLK_REQ4_L - SSD */ + PAD_NF(GPIO_132, CLK_REQ4_L, PULL_UP), + /* SD_AUX_RESET_L */ + PAD_GPO(GPIO_142, HIGH), + }; + + program_gpios(pcie_gpio_table, ARRAY_SIZE(pcie_gpio_table)); + if (variant_uses_v3_schematics()) wifi_power_reset_configure_v3(); else diff --git a/src/mainboard/google/zork/variants/baseboard/include/baseboard/variants.h b/src/mainboard/google/zork/variants/baseboard/include/baseboard/variants.h index dab7332..90e8b04 100644 --- a/src/mainboard/google/zork/variants/baseboard/include/baseboard/variants.h +++ b/src/mainboard/google/zork/variants/baseboard/include/baseboard/variants.h @@ -12,7 +12,6 @@
const struct sci_source *variant_gpe_table(size_t *num); const struct soc_amd_gpio *variant_early_gpio_table(size_t *size); -const struct soc_amd_gpio *variant_romstage_gpio_table(size_t *size); /* * This function provides base GPIO configuration table. It is typically provided by * baseboard using a weak implementation. If GPIO configuration for a variant differs @@ -36,8 +35,8 @@ void variant_devtree_update(void); /* Update audio configuration in devicetree during ramstage. */ void variant_audio_update(void); -/* Configure PCIe power and reset lines as per variant sequencing requirements. */ -void variant_pcie_power_reset_configure(void); +/* Configure PCIe GPIOs as per variant sequencing requirements. */ +void variant_pcie_gpio_configure(void);
/* Per variant FSP-S initialization, default implementation in baseboard and * overrideable by the variant. */
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43475 )
Change subject: mb/google/zork: Drop variant_romstage_gpio_table() ......................................................................
Patch Set 1:
(2 comments)
https://review.coreboot.org/c/coreboot/+/43475/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/43475/1//COMMIT_MSG@9 PS1, Line 9: gpio_set_stage_rom table is now configuring only PCIe related GPIOs in : romstage. This change moves the configuration of PCIe related : GPIOs to variant_pcie_gpio_configure() to keep all the configuration : for WiFi and non-WiFi PCIe pads in one place. It also drops the : functin variant_romstage_gpio_table() as it is unused. Please re-flow for 75 characters per line.
https://review.coreboot.org/c/coreboot/+/43475/1//COMMIT_MSG@13 PS1, Line 13: functin function
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43475 )
Change subject: mb/google/zork: Drop variant_romstage_gpio_table() ......................................................................
Patch Set 1: Code-Review+2
Hello build bot (Jenkins), Aaron Durbin,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/43475
to look at the new patch set (#2).
Change subject: mb/google/zork: Drop variant_romstage_gpio_table() ......................................................................
mb/google/zork: Drop variant_romstage_gpio_table()
gpio_set_stage_rom table is now configuring only PCIe related GPIOs in romstage. This change moves the configuration of PCIe related GPIOs to variant_pcie_gpio_configure() to keep all the configuration for WiFi and non-WiFi PCIe pads in one place. It also drops the functin variant_romstage_gpio_table() as it is unused.
BUG=b:154351731
Signed-off-by: Furquan Shaikh furquan@google.com Change-Id: Ib1c41ba141dce6b52b6e0a250a3aa07c296068aa --- M src/mainboard/google/zork/romstage.c M src/mainboard/google/zork/variants/baseboard/gpio_baseboard_dalboz.c M src/mainboard/google/zork/variants/baseboard/gpio_baseboard_trembyle.c M src/mainboard/google/zork/variants/baseboard/include/baseboard/variants.h 4 files changed, 37 insertions(+), 53 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/75/43475/2
Hello build bot (Jenkins), Aaron Durbin,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/43475
to look at the new patch set (#3).
Change subject: git rebasmb/google/zork: Drop variant_romstage_gpio_table() ......................................................................
git rebasmb/google/zork: Drop variant_romstage_gpio_table()
gpio_set_stage_rom table is now configuring only PCIe related GPIOs in romstage. This change moves the configuration of PCIe related GPIOs to variant_pcie_gpio_configure() to keep all the configuration for WiFi and non-WiFi PCIe pads in one place. It also drops the function variant_romstage_gpio_table() as it is unused.
BUG=b:154351731
Signed-off-by: Furquan Shaikh furquan@google.com Change-Id: Ib1c41ba141dce6b52b6e0a250a3aa07c296068aa --- M src/mainboard/google/zork/romstage.c M src/mainboard/google/zork/variants/baseboard/gpio_baseboard_dalboz.c M src/mainboard/google/zork/variants/baseboard/gpio_baseboard_trembyle.c M src/mainboard/google/zork/variants/baseboard/include/baseboard/variants.h 4 files changed, 37 insertions(+), 53 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/75/43475/3
Hello build bot (Jenkins), Aaron Durbin,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/43475
to look at the new patch set (#4).
Change subject: mb/google/zork: Drop variant_romstage_gpio_table() ......................................................................
mb/google/zork: Drop variant_romstage_gpio_table()
gpio_set_stage_rom table is now configuring only PCIe related GPIOs in romstage. This change moves the configuration of PCIe related GPIOs to variant_pcie_gpio_configure() to keep all the configuration for WiFi and non-WiFi PCIe pads in one place. It also drops the function variant_romstage_gpio_table() as it is unused.
BUG=b:154351731
Signed-off-by: Furquan Shaikh furquan@google.com Change-Id: Ib1c41ba141dce6b52b6e0a250a3aa07c296068aa --- M src/mainboard/google/zork/romstage.c M src/mainboard/google/zork/variants/baseboard/gpio_baseboard_dalboz.c M src/mainboard/google/zork/variants/baseboard/gpio_baseboard_trembyle.c M src/mainboard/google/zork/variants/baseboard/include/baseboard/variants.h 4 files changed, 37 insertions(+), 53 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/75/43475/4
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43475 )
Change subject: mb/google/zork: Drop variant_romstage_gpio_table() ......................................................................
Patch Set 4:
(2 comments)
https://review.coreboot.org/c/coreboot/+/43475/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/43475/1//COMMIT_MSG@13 PS1, Line 13: functin
function
Done
https://review.coreboot.org/c/coreboot/+/43475/1//COMMIT_MSG@9 PS1, Line 9: gpio_set_stage_rom table is now configuring only PCIe related GPIOs in : romstage. This change moves the configuration of PCIe related : GPIOs to variant_pcie_gpio_configure() to keep all the configuration : for WiFi and non-WiFi PCIe pads in one place. It also drops the : functin variant_romstage_gpio_table() as it is unused.
Please re-flow for 75 characters per line.
Done
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43475 )
Change subject: mb/google/zork: Drop variant_romstage_gpio_table() ......................................................................
Patch Set 4: Code-Review+2
Aaron Durbin has submitted this change. ( https://review.coreboot.org/c/coreboot/+/43475 )
Change subject: mb/google/zork: Drop variant_romstage_gpio_table() ......................................................................
mb/google/zork: Drop variant_romstage_gpio_table()
gpio_set_stage_rom table is now configuring only PCIe related GPIOs in romstage. This change moves the configuration of PCIe related GPIOs to variant_pcie_gpio_configure() to keep all the configuration for WiFi and non-WiFi PCIe pads in one place. It also drops the function variant_romstage_gpio_table() as it is unused.
BUG=b:154351731
Signed-off-by: Furquan Shaikh furquan@google.com Change-Id: Ib1c41ba141dce6b52b6e0a250a3aa07c296068aa Reviewed-on: https://review.coreboot.org/c/coreboot/+/43475 Reviewed-by: Aaron Durbin adurbin@chromium.org Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/mainboard/google/zork/romstage.c M src/mainboard/google/zork/variants/baseboard/gpio_baseboard_dalboz.c M src/mainboard/google/zork/variants/baseboard/gpio_baseboard_trembyle.c M src/mainboard/google/zork/variants/baseboard/include/baseboard/variants.h 4 files changed, 37 insertions(+), 53 deletions(-)
Approvals: build bot (Jenkins): Verified Aaron Durbin: Looks good to me, approved
diff --git a/src/mainboard/google/zork/romstage.c b/src/mainboard/google/zork/romstage.c index bd4c96e..39f23cf 100644 --- a/src/mainboard/google/zork/romstage.c +++ b/src/mainboard/google/zork/romstage.c @@ -7,10 +7,5 @@
void mainboard_romstage_entry_s3(int s3_resume) { - size_t num_gpios; - const struct soc_amd_gpio *gpios; - - gpios = variant_romstage_gpio_table(&num_gpios); - program_gpios(gpios, num_gpios); - variant_pcie_power_reset_configure(); + variant_pcie_gpio_configure(); } 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 135ecca..3f1da03 100644 --- a/src/mainboard/google/zork/variants/baseboard/gpio_baseboard_dalboz.c +++ b/src/mainboard/google/zork/variants/baseboard/gpio_baseboard_dalboz.c @@ -9,21 +9,6 @@ #include <boardid.h> #include <variant/gpio.h>
-static const struct soc_amd_gpio gpio_set_stage_rom[] = { - /* PCIE_RST1_L - Variable timings (May remove) */ - PAD_NF(GPIO_27, PCIE_RST1_L, PULL_NONE), - /* NVME_AUX_RESET_L */ - PAD_GPO(GPIO_40, HIGH), - /* CLK_REQ0_L - WIFI */ - PAD_NF(GPIO_92, CLK_REQ0_L, PULL_UP), - /* CLK_REQ1_L - SD Card */ - PAD_NF(GPIO_115, CLK_REQ1_L, PULL_UP), - /* CLK_REQ2_L - NVMe */ - PAD_NF(GPIO_116, CLK_REQ2_L, PULL_UP), - /* SD_AUX_RESET_L */ - PAD_GPO(GPIO_142, HIGH), -}; - static const struct soc_amd_gpio gpio_set_stage_ram[] = {
/* PWR_BTN_L */ @@ -140,13 +125,6 @@ };
const __weak -struct soc_amd_gpio *variant_romstage_gpio_table(size_t *size) -{ - *size = ARRAY_SIZE(gpio_set_stage_rom); - return gpio_set_stage_rom; -} - -const __weak struct soc_amd_gpio *variant_base_gpio_table(size_t *size) { *size = ARRAY_SIZE(gpio_set_stage_ram); @@ -247,8 +225,25 @@ gpio_set(GPIO_42, 1); }
-__weak void variant_pcie_power_reset_configure(void) +__weak void variant_pcie_gpio_configure(void) { + static const struct soc_amd_gpio pcie_gpio_table[] = { + /* PCIE_RST1_L - Variable timings (May remove) */ + PAD_NF(GPIO_27, PCIE_RST1_L, PULL_NONE), + /* NVME_AUX_RESET_L */ + PAD_GPO(GPIO_40, HIGH), + /* CLK_REQ0_L - WIFI */ + PAD_NF(GPIO_92, CLK_REQ0_L, PULL_UP), + /* CLK_REQ1_L - SD Card */ + PAD_NF(GPIO_115, CLK_REQ1_L, PULL_UP), + /* CLK_REQ2_L - NVMe */ + PAD_NF(GPIO_116, CLK_REQ2_L, PULL_UP), + /* SD_AUX_RESET_L */ + PAD_GPO(GPIO_142, HIGH), + }; + + program_gpios(pcie_gpio_table, ARRAY_SIZE(pcie_gpio_table)); + /* Deassert PCIE_RST1_L */ gpio_set(GPIO_27, 1);
diff --git a/src/mainboard/google/zork/variants/baseboard/gpio_baseboard_trembyle.c b/src/mainboard/google/zork/variants/baseboard/gpio_baseboard_trembyle.c index b3cbf67..a998cd7 100644 --- a/src/mainboard/google/zork/variants/baseboard/gpio_baseboard_trembyle.c +++ b/src/mainboard/google/zork/variants/baseboard/gpio_baseboard_trembyle.c @@ -9,19 +9,6 @@ #include <boardid.h> #include <variant/gpio.h>
-static const struct soc_amd_gpio gpio_set_stage_rom[] = { - /* NVME_AUX_RESET_L */ - PAD_GPO(GPIO_40, HIGH), - /* CLK_REQ0_L - WIFI */ - PAD_NF(GPIO_92, CLK_REQ0_L, PULL_UP), - /* CLK_REQ1_L - SD Card */ - PAD_NF(GPIO_115, CLK_REQ1_L, PULL_UP), - /* CLK_REQ4_L - SSD */ - PAD_NF(GPIO_132, CLK_REQ4_L, PULL_UP), - /* SD_AUX_RESET_L */ - PAD_GPO(GPIO_142, HIGH), -}; - static const struct soc_amd_gpio gpio_set_stage_ram[] = {
/* PWR_BTN_L */ @@ -136,13 +123,6 @@ };
const __weak -struct soc_amd_gpio *variant_romstage_gpio_table(size_t *size) -{ - *size = ARRAY_SIZE(gpio_set_stage_rom); - return gpio_set_stage_rom; -} - -const __weak struct soc_amd_gpio *variant_base_gpio_table(size_t *size) { *size = ARRAY_SIZE(gpio_set_stage_ram); @@ -243,8 +223,23 @@ gpio_set(GPIO_42, 1); }
-__weak void variant_pcie_power_reset_configure(void) +__weak void variant_pcie_gpio_configure(void) { + static const struct soc_amd_gpio pcie_gpio_table[] = { + /* NVME_AUX_RESET_L */ + PAD_GPO(GPIO_40, HIGH), + /* CLK_REQ0_L - WIFI */ + PAD_NF(GPIO_92, CLK_REQ0_L, PULL_UP), + /* CLK_REQ1_L - SD Card */ + PAD_NF(GPIO_115, CLK_REQ1_L, PULL_UP), + /* CLK_REQ4_L - SSD */ + PAD_NF(GPIO_132, CLK_REQ4_L, PULL_UP), + /* SD_AUX_RESET_L */ + PAD_GPO(GPIO_142, HIGH), + }; + + program_gpios(pcie_gpio_table, ARRAY_SIZE(pcie_gpio_table)); + if (variant_uses_v3_schematics()) wifi_power_reset_configure_v3(); else diff --git a/src/mainboard/google/zork/variants/baseboard/include/baseboard/variants.h b/src/mainboard/google/zork/variants/baseboard/include/baseboard/variants.h index dab7332..90e8b04 100644 --- a/src/mainboard/google/zork/variants/baseboard/include/baseboard/variants.h +++ b/src/mainboard/google/zork/variants/baseboard/include/baseboard/variants.h @@ -12,7 +12,6 @@
const struct sci_source *variant_gpe_table(size_t *num); const struct soc_amd_gpio *variant_early_gpio_table(size_t *size); -const struct soc_amd_gpio *variant_romstage_gpio_table(size_t *size); /* * This function provides base GPIO configuration table. It is typically provided by * baseboard using a weak implementation. If GPIO configuration for a variant differs @@ -36,8 +35,8 @@ void variant_devtree_update(void); /* Update audio configuration in devicetree during ramstage. */ void variant_audio_update(void); -/* Configure PCIe power and reset lines as per variant sequencing requirements. */ -void variant_pcie_power_reset_configure(void); +/* Configure PCIe GPIOs as per variant sequencing requirements. */ +void variant_pcie_gpio_configure(void);
/* Per variant FSP-S initialization, default implementation in baseboard and * overrideable by the variant. */
-
Aaron Durbin (Code Review) -
Furquan Shaikh (Code Review)
-
Paul Menzel (Code Review)