Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42690 )
Change subject: soc/amd/common: Complete ACPIMMIO GPIO bank separation
......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/42690/4/src/soc/amd/common/block/g…
File src/soc/amd/common/block/gpio_banks/gpio.c:
https://review.coreboot.org/c/coreboot/+/42690/4/src/soc/amd/common/block/g…
PS4, Line 83: return acpimmio_gpio0 + gpio_num * sizeof(uint32_t);
> I skipped ahead a few patches to this one. […]
The inlined varinat moves away from <acpimmio.h> here. Intent is for this gpio.c to have full ownership of the GPIO bank. Sure, one could still dereference acpimmio_gpio0 elsewhere, but hopefully such attempt would get caught in the review and use of <gpio.h> APIs would be encorced instead.
--
To view, visit https://review.coreboot.org/c/coreboot/+/42690
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I4c8c3f2028ca89dca5c7f0548fcd18e1045999d6
Gerrit-Change-Number: 42690
Gerrit-PatchSet: 4
Gerrit-Owner: Kyösti Mälkki <kyosti.malkki(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Aaron Durbin <adurbin(a)chromium.org>
Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Comment-Date: Thu, 25 Jun 2020 11:14:03 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Aaron Durbin <adurbin(a)chromium.org>
Gerrit-MessageType: comment
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42486 )
Change subject: Revert "soc/amd/common/block/acpimmio: Update acpimmio for psp_verstage"
......................................................................
Patch Set 5:
> Patch Set 5:
>
> Shouldn't we combine this w/ a resulting full solution? I suspect we should let Martin's series land then modify files as needed.
Once ARCH_VERSTAGE_ARM lands we are forced to squash several commits together to satisfy jenkins every-commit-builds requirement. From what it appears to me, all the forking and splitting of source files in Martin's series is undesireable and unnecessary, if we take a closer look at it.
In some recent intel-car change-series, I was requested to hide cache-as-ram related symbols from romstage because they only had proper references in bootblock. Following this logic, all x86-centric #defines of memory base addresses should be guarded with ENV_X86. I believe many of those continue to leak into psp-verstage currently, although most of them will be handled by --gc-sections.
--
To view, visit https://review.coreboot.org/c/coreboot/+/42486
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I1d632f52745bc6cd3c3dbddb1ea5ff9ba962c2e8
Gerrit-Change-Number: 42486
Gerrit-PatchSet: 5
Gerrit-Owner: Kyösti Mälkki <kyosti.malkki(a)gmail.com>
Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Krystian Hebel <krystian.hebel(a)3mdeb.com>
Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com>
Gerrit-Reviewer: Martin Roth <martinroth(a)google.com>
Gerrit-Reviewer: Michał Żygowski <michal.zygowski(a)3mdeb.com>
Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com>
Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Comment-Date: Thu, 25 Jun 2020 10:42:48 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Furquan Shaikh has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/42739 )
Change subject: mb/google/zork: Move PCIE_RST0_L configuration to early GPIO table
......................................................................
mb/google/zork: Move PCIE_RST0_L configuration to early GPIO table
This change moves the configuration of PCIE_RST0_L as native
function to happen in early GPIO table. This ensures that the PERST#
signal is deasserted as soon as possible when the system comes out
of sleep state in case the sleep path asserted/deasserted the PERST#
as GPIO out.
A big difference in functionality with this change is that PCIE_RST0_L
signal is now configured as part of RO, which should be fine since
all PCIe devices have a second AUX_RESET_L signal or use PCIE_RST1_L
to control the actual reset to the device.
Signed-off-by: Furquan Shaikh <furquan(a)google.com>
Change-Id: I21a9c25b5a8a6d502cdb79cbe0dbad6ef98d6d63
---
M src/mainboard/google/zork/variants/baseboard/gpio_baseboard_common.c
M src/mainboard/google/zork/variants/baseboard/gpio_baseboard_dalboz.c
M src/mainboard/google/zork/variants/baseboard/gpio_baseboard_trembyle.c
3 files changed, 2 insertions(+), 12 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/39/42739/1
diff --git a/src/mainboard/google/zork/variants/baseboard/gpio_baseboard_common.c b/src/mainboard/google/zork/variants/baseboard/gpio_baseboard_common.c
index e903700..a4e8648 100644
--- a/src/mainboard/google/zork/variants/baseboard/gpio_baseboard_common.c
+++ b/src/mainboard/google/zork/variants/baseboard/gpio_baseboard_common.c
@@ -13,6 +13,8 @@
PAD_NF(GPIO_19, I2C3_SCL, PULL_UP),
/* I2C3_SDA - H1 */
PAD_NF(GPIO_20, I2C3_SDA, PULL_UP),
+ /* PCIE_RST0_L - Fixed timings */
+ PAD_NF(GPIO_26, PCIE_RST_L, PULL_NONE),
/* FCH_ESPI_EC_CS_L */
PAD_NF(GPIO_30, ESPI_CS_L, PULL_NONE),
/* ESPI_ALERT_L (may be unused) */
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 d4ed86e..d6343a6 100644
--- a/src/mainboard/google/zork/variants/baseboard/gpio_baseboard_dalboz.c
+++ b/src/mainboard/google/zork/variants/baseboard/gpio_baseboard_dalboz.c
@@ -15,9 +15,6 @@
/* EC_FCH_WAKE_L */
PAD_GPI(GPIO_24, PULL_UP),
PAD_WAKE(GPIO_24, PULL_UP, EDGE_LOW, S3_S4_S5),
- /* PCIE_RST0_L - Fixed timings */
- /* TODO: Make sure this gets locked at end of post */
- PAD_NF(GPIO_26, PCIE_RST_L, PULL_NONE),
/* PCIE_RST1_L - Variable timings (May remove) */
PAD_NF(GPIO_27, PCIE_RST1_L, PULL_NONE),
/* NVME_AUX_RESET_L */
@@ -52,9 +49,6 @@
/* EC_FCH_WAKE_L */
PAD_GPI(GPIO_24, PULL_UP),
PAD_WAKE(GPIO_24, PULL_UP, EDGE_LOW, S3_S4_S5),
- /* PCIE_RST0_L - Fixed timings */
- /* TODO: Make sure this gets locked at end of post */
- PAD_NF(GPIO_26, PCIE_RST_L, PULL_NONE),
/* PCIE_RST1_L - Variable timings (May remove) */
PAD_NF(GPIO_27, PCIE_RST1_L, PULL_NONE),
/* NVME_AUX_RESET_L */
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 abee520..f93309d 100644
--- a/src/mainboard/google/zork/variants/baseboard/gpio_baseboard_trembyle.c
+++ b/src/mainboard/google/zork/variants/baseboard/gpio_baseboard_trembyle.c
@@ -15,9 +15,6 @@
/* EC_FCH_WAKE_L */
PAD_GPI(GPIO_24, PULL_UP),
PAD_WAKE(GPIO_24, PULL_UP, EDGE_LOW, S3_S4_S5),
- /* PCIE_RST0_L - Fixed timings */
- /* TODO: Make sure this gets locked at end of post */
- PAD_NF(GPIO_26, PCIE_RST_L, PULL_NONE),
/* PCIE_RST1_L - Variable timings (May remove) */
PAD_NF(GPIO_27, PCIE_RST1_L, PULL_NONE),
/* NVME_AUX_RESET_L */
@@ -50,9 +47,6 @@
/* EC_FCH_WAKE_L */
PAD_GPI(GPIO_24, PULL_UP),
PAD_WAKE(GPIO_24, PULL_UP, EDGE_LOW, S3_S4_S5),
- /* PCIE_RST0_L - Fixed timings */
- /* TODO: Make sure this gets locked at end of post */
- PAD_NF(GPIO_26, PCIE_RST_L, PULL_NONE),
/* PCIE_RST1_L - Variable timings (May remove) */
PAD_NF(GPIO_27, PCIE_RST1_L, PULL_NONE),
/* NVME_AUX_RESET_L */
--
To view, visit https://review.coreboot.org/c/coreboot/+/42739
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I21a9c25b5a8a6d502cdb79cbe0dbad6ef98d6d63
Gerrit-Change-Number: 42739
Gerrit-PatchSet: 1
Gerrit-Owner: Furquan Shaikh <furquan(a)google.com>
Gerrit-MessageType: newchange