coreboot-gerrit June 2020

coreboot-gerrit@coreboot.org

1 participants
2826 discussions

Change in coreboot[master]: mb/google/zork: Drop RAM_ID configuration from romstage gpio table
by Furquan Shaikh (Code Review) 25 Jun '20

25 Jun '20

Hello Aaron Durbin, I'd like you to do a code review. Please visit https://review.coreboot.org/c/coreboot/+/42720 to review the following change. Change subject: mb/google/zork: Drop RAM_ID configuration from romstage gpio table ...................................................................... mb/google/zork: Drop RAM_ID configuration from romstage gpio table RAM_ID GPIOs are configured by ABL based on the information added to APCB. Coreboot does not need to configure these pads. This change drops the RAM_ID configuration from trembyle baseboard. Dalboz never really configured RAM_IDs in coreboot. BUG=b:154351731 Signed-off-by: Furquan Shaikh <furquan(a)google.com> Change-Id: Ie1dfcc3c185304d917ab4386920445ba0119ac69 Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/third_party/coreboot/… Commit-Queue: Furquan Shaikh <furquan(a)chromium.org> Tested-by: Furquan Shaikh <furquan(a)chromium.org> Reviewed-by: Aaron Durbin <adurbin(a)google.com> --- M src/mainboard/google/zork/variants/baseboard/gpio_baseboard_trembyle.c 1 file changed, 0 insertions(+), 10 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/20/42720/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 c3e1e42..b32312f 100644 --- a/src/mainboard/google/zork/variants/baseboard/gpio_baseboard_trembyle.c +++ b/src/mainboard/google/zork/variants/baseboard/gpio_baseboard_trembyle.c @@ -28,20 +28,10 @@ PAD_GPO(GPIO_68, HIGH), /* EN_PWR_CAMERA - reset */ PAD_GPO(GPIO_76, LOW), - /* RAM_ID_4 */ - PAD_GPI(GPIO_84, PULL_NONE), /* 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), - /* RAM_ID_3 */ - PAD_GPI(GPIO_116, PULL_NONE), - /* RAM_ID_1 */ - PAD_GPI(GPIO_120, PULL_NONE), - /* RAM_ID_0 */ - PAD_GPI(GPIO_121, PULL_NONE), - /* RAM_ID_2 */ - PAD_GPI(GPIO_131, PULL_NONE), /* CLK_REQ4_L - SSD */ PAD_NF(GPIO_132, CLK_REQ4_L, PULL_UP), /* BIOS_FLASH_WP_ODL */ -- To view, visit https://review.coreboot.org/c/coreboot/+/42720 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ie1dfcc3c185304d917ab4386920445ba0119ac69 Gerrit-Change-Number: 42720 Gerrit-PatchSet: 1 Gerrit-Owner: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-MessageType: newchange

2 5

Change in coreboot[master]: mb/google/zork: Move variant_early_gpio_table to gpio_baseboard_common.c
by Furquan Shaikh (Code Review) 25 Jun '20

25 Jun '20

Hello Aaron Durbin, I'd like you to do a code review. Please visit https://review.coreboot.org/c/coreboot/+/42719 to review the following change. Change subject: mb/google/zork: Move variant_early_gpio_table to gpio_baseboard_common.c ...................................................................... mb/google/zork: Move variant_early_gpio_table to gpio_baseboard_common.c This change moves the GPIOs that need to be configured for early access in coreboot to early_gpio_table[] in gpio_baseboard_common.c. These GPIOs include: * Pads to talk to EC * Pads to talk to TPM * Pads to talk to serial console These should be configured in the first stage that runs coreboot i.e. in case of VBOOT_STARTS_BEFORE_BOOTBLOCK, it should be done as part of verstage (which starts on PSP), else it should be done as part of bootblock (which is the first stage that runs on x86). This change drops GPIO_137 from early_gpio_table since that is not really required in early stages. BUG=b:154351731 TEST=Verified that trembyle still boots. Signed-off-by: Furquan Shaikh <furquan(a)google.com> Change-Id: Ifbdbb02cbfc65ddb68f0ae75cf4b1f2ea1656b91 Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/third_party/coreboot/… Commit-Queue: Furquan Shaikh <furquan(a)chromium.org> Tested-by: Furquan Shaikh <furquan(a)chromium.org> Reviewed-by: Aaron Durbin <adurbin(a)google.com> --- M src/mainboard/google/zork/bootblock.c M src/mainboard/google/zork/variants/baseboard/Makefile.inc A 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 M src/mainboard/google/zork/verstage.c 6 files changed, 39 insertions(+), 89 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/19/42719/1 diff --git a/src/mainboard/google/zork/bootblock.c b/src/mainboard/google/zork/bootblock.c index 0e7af52..11391ef 100644 --- a/src/mainboard/google/zork/bootblock.c +++ b/src/mainboard/google/zork/bootblock.c @@ -8,6 +8,8 @@ size_t num_gpios; const struct soc_amd_gpio *gpios; - gpios = variant_early_gpio_table(&num_gpios); - program_gpios(gpios, num_gpios); + if (!CONFIG(VBOOT_STARTS_BEFORE_BOOTBLOCK)) { + gpios = variant_early_gpio_table(&num_gpios); + program_gpios(gpios, num_gpios); + } } diff --git a/src/mainboard/google/zork/variants/baseboard/Makefile.inc b/src/mainboard/google/zork/variants/baseboard/Makefile.inc index 4bcaf68..7e4ccae 100644 --- a/src/mainboard/google/zork/variants/baseboard/Makefile.inc +++ b/src/mainboard/google/zork/variants/baseboard/Makefile.inc @@ -1,18 +1,22 @@ # SPDX-License-Identifier: GPL-2.0-or-later +bootblock-y += gpio_baseboard_common.c bootblock-$(CONFIG_BOARD_GOOGLE_BASEBOARD_TREMBYLE) += gpio_baseboard_trembyle.c bootblock-$(CONFIG_BOARD_GOOGLE_BASEBOARD_DALBOZ) += gpio_baseboard_dalboz.c +verstage-y += gpio_baseboard_common.c ifeq ($(CONFIG_VBOOT_STARTS_BEFORE_BOOTBLOCK),y) verstage-$(CONFIG_BOARD_GOOGLE_BASEBOARD_TREMBYLE) += gpio_baseboard_trembyle.c verstage-$(CONFIG_BOARD_GOOGLE_BASEBOARD_DALBOZ) += gpio_baseboard_dalboz.c endif verstage-y += tpm_tis.c +romstage-y += gpio_baseboard_common.c romstage-$(CONFIG_BOARD_GOOGLE_BASEBOARD_TREMBYLE) += gpio_baseboard_trembyle.c romstage-$(CONFIG_BOARD_GOOGLE_BASEBOARD_DALBOZ) += gpio_baseboard_dalboz.c romstage-y += tpm_tis.c +ramstage-y += gpio_baseboard_common.c ramstage-$(CONFIG_BOARD_GOOGLE_BASEBOARD_TREMBYLE) += gpio_baseboard_trembyle.c ramstage-$(CONFIG_BOARD_GOOGLE_BASEBOARD_TREMBYLE) += fsps_baseboard_trembyle.c ramstage-$(CONFIG_BOARD_GOOGLE_BASEBOARD_DALBOZ) += gpio_baseboard_dalboz.c diff --git a/src/mainboard/google/zork/variants/baseboard/gpio_baseboard_common.c b/src/mainboard/google/zork/variants/baseboard/gpio_baseboard_common.c new file mode 100644 index 0000000..e903700 --- /dev/null +++ b/src/mainboard/google/zork/variants/baseboard/gpio_baseboard_common.c @@ -0,0 +1,30 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later */ + +#include <baseboard/variants.h> +#include <soc/gpio.h> +#include <stdlib.h> +#include <boardid.h> +#include <variant/gpio.h> + +static const struct soc_amd_gpio early_gpio_table[] = { + /* H1_FCH_INT_ODL */ + PAD_INT(GPIO_3, PULL_UP, EDGE_LOW, STATUS), + /* I2C3_SCL - H1 */ + PAD_NF(GPIO_19, I2C3_SCL, PULL_UP), + /* I2C3_SDA - H1 */ + PAD_NF(GPIO_20, I2C3_SDA, PULL_UP), + /* FCH_ESPI_EC_CS_L */ + PAD_NF(GPIO_30, ESPI_CS_L, PULL_NONE), + /* ESPI_ALERT_L (may be unused) */ + PAD_NF(GPIO_108, ESPI_ALERT_L, PULL_UP), + /* UART0_RXD - DEBUG */ + PAD_NF(GPIO_136, UART0_RXD, PULL_NONE), + /* UART0_TXD - DEBUG */ + PAD_NF(GPIO_138, UART0_TXD, PULL_NONE), +}; + +const __weak struct soc_amd_gpio *variant_early_gpio_table(size_t *size) +{ + *size = ARRAY_SIZE(early_gpio_table); + return early_gpio_table; +} 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 b5b2847..46c2321 100644 --- a/src/mainboard/google/zork/variants/baseboard/gpio_baseboard_dalboz.c +++ b/src/mainboard/google/zork/variants/baseboard/gpio_baseboard_dalboz.c @@ -7,37 +7,9 @@ #include <boardid.h> #include <variant/gpio.h> -static const struct soc_amd_gpio gpio_set_stage_reset[] = { - /* H1_FCH_INT_ODL */ - PAD_INT(GPIO_3, PULL_UP, EDGE_LOW, STATUS), - /* I2C3_SCL - H1 */ - PAD_NF(GPIO_19, I2C3_SCL, PULL_UP), - /* I2C3_SDA - H1 */ - PAD_NF(GPIO_20, I2C3_SDA, PULL_UP), - - /* FCH_ESPI_EC_CS_L */ - PAD_NF(GPIO_30, ESPI_CS_L, PULL_NONE), - - /* ESPI_ALERT_L (may be unused) */ - PAD_NF(GPIO_108, ESPI_ALERT_L, PULL_UP), - - /* UART0_RXD - DEBUG */ - PAD_NF(GPIO_136, UART0_RXD, PULL_NONE), - /* BIOS_FLASH_WP_ODL */ - PAD_GPI(GPIO_137, PULL_NONE), - /* UART0_TXD - DEBUG */ - PAD_NF(GPIO_138, UART0_TXD, PULL_NONE), -}; - static const struct soc_amd_gpio gpio_set_stage_rom[] = { - /* H1_FCH_INT_ODL */ - PAD_INT(GPIO_3, PULL_UP, EDGE_LOW, STATUS), /* PEN_POWER_EN - reset */ PAD_GPO(GPIO_5, LOW), - /* I2C3_SCL - H1 */ - PAD_NF(GPIO_19, I2C3_SCL, PULL_UP), - /* I2C3_SDA - H1 */ - PAD_NF(GPIO_20, I2C3_SDA, PULL_UP), /* EC_FCH_WAKE_L */ PAD_GPI(GPIO_24, PULL_UP), PAD_WAKE(GPIO_24, PULL_UP, EDGE_LOW, S3_S4_S5), @@ -46,8 +18,6 @@ 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), - /* FCH_ESPI_EC_CS_L */ - PAD_NF(GPIO_30, ESPI_CS_L, PULL_NONE), /* NVME_AUX_RESET_L */ PAD_GPO(GPIO_40, HIGH), /* WIFI_AUX_RESET_L */ @@ -60,18 +30,12 @@ PAD_GPO(GPIO_76, LOW), /* CLK_REQ0_L - WIFI */ PAD_NF(GPIO_92, CLK_REQ0_L, PULL_UP), - /* ESPI_ALERT_L (may be unused) */ - PAD_NF(GPIO_108, ESPI_ALERT_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), - /* UART0_RXD - DEBUG */ - PAD_NF(GPIO_136, UART0_RXD, PULL_NONE), /* BIOS_FLASH_WP_ODL */ PAD_GPI(GPIO_137, PULL_NONE), - /* UART0_TXD - DEBUG */ - PAD_NF(GPIO_138, UART0_TXD, PULL_NONE), /* USI_RESET - reset */ PAD_GPO(GPIO_140, HIGH), /* USB_HUB_RST_L - reset*/ @@ -198,13 +162,6 @@ }; const __weak -struct soc_amd_gpio *variant_early_gpio_table(size_t *size) -{ - *size = ARRAY_SIZE(gpio_set_stage_reset); - return gpio_set_stage_reset; -} - -const __weak struct soc_amd_gpio *variant_romstage_gpio_table(size_t *size) { *size = ARRAY_SIZE(gpio_set_stage_rom); 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 6101330..c3e1e42 100644 --- a/src/mainboard/google/zork/variants/baseboard/gpio_baseboard_trembyle.c +++ b/src/mainboard/google/zork/variants/baseboard/gpio_baseboard_trembyle.c @@ -7,37 +7,9 @@ #include <boardid.h> #include <variant/gpio.h> -static const struct soc_amd_gpio gpio_set_stage_reset[] = { - /* H1_FCH_INT_ODL */ - PAD_INT(GPIO_3, PULL_UP, EDGE_LOW, STATUS), - /* I2C3_SCL - H1 */ - PAD_NF(GPIO_19, I2C3_SCL, PULL_UP), - /* I2C3_SDA - H1 */ - PAD_NF(GPIO_20, I2C3_SDA, PULL_UP), - - /* FCH_ESPI_EC_CS_L */ - PAD_NF(GPIO_30, ESPI_CS_L, PULL_NONE), - - /* ESPI_ALERT_L (may be unused) */ - PAD_NF(GPIO_108, ESPI_ALERT_L, PULL_UP), - - /* UART0_RXD - DEBUG */ - PAD_NF(GPIO_136, UART0_RXD, PULL_NONE), - /* BIOS_FLASH_WP_ODL */ - PAD_GPI(GPIO_137, PULL_NONE), - /* UART0_TXD - DEBUG */ - PAD_NF(GPIO_138, UART0_TXD, PULL_NONE), -}; - static const struct soc_amd_gpio gpio_set_stage_rom[] = { - /* H1_FCH_INT_ODL */ - PAD_INT(GPIO_3, PULL_UP, EDGE_LOW, STATUS), /* PEN_POWER_EN - reset */ PAD_GPO(GPIO_5, LOW), - /* I2C3_SCL - H1 */ - PAD_NF(GPIO_19, I2C3_SCL, PULL_UP), - /* I2C3_SDA - H1 */ - PAD_NF(GPIO_20, I2C3_SDA, PULL_UP), /* EC_FCH_WAKE_L */ PAD_GPI(GPIO_24, PULL_UP), PAD_WAKE(GPIO_24, PULL_UP, EDGE_LOW, S3_S4_S5), @@ -46,8 +18,6 @@ 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), - /* FCH_ESPI_EC_CS_L */ - PAD_NF(GPIO_30, ESPI_CS_L, PULL_NONE), /* NVME_AUX_RESET_L */ PAD_GPO(GPIO_40, HIGH), /* WIFI_AUX_RESET_L */ @@ -62,8 +32,6 @@ PAD_GPI(GPIO_84, PULL_NONE), /* CLK_REQ0_L - WIFI */ PAD_NF(GPIO_92, CLK_REQ0_L, PULL_UP), - /* ESPI_ALERT_L (may be unused) */ - PAD_NF(GPIO_108, ESPI_ALERT_L, PULL_UP), /* CLK_REQ1_L - SD Card */ PAD_NF(GPIO_115, CLK_REQ1_L, PULL_UP), /* RAM_ID_3 */ @@ -76,12 +44,8 @@ PAD_GPI(GPIO_131, PULL_NONE), /* CLK_REQ4_L - SSD */ PAD_NF(GPIO_132, CLK_REQ4_L, PULL_UP), - /* UART0_RXD - DEBUG */ - PAD_NF(GPIO_136, UART0_RXD, PULL_NONE), /* BIOS_FLASH_WP_ODL */ PAD_GPI(GPIO_137, PULL_NONE), - /* UART0_TXD - DEBUG */ - PAD_NF(GPIO_138, UART0_TXD, PULL_NONE), /* USI_RESET - reset */ PAD_GPO(GPIO_140, HIGH), /* SD_AUX_RESET_L */ @@ -207,13 +171,6 @@ }; const __weak -struct soc_amd_gpio *variant_early_gpio_table(size_t *size) -{ - *size = ARRAY_SIZE(gpio_set_stage_reset); - return gpio_set_stage_reset; -} - -const __weak struct soc_amd_gpio *variant_romstage_gpio_table(size_t *size) { *size = ARRAY_SIZE(gpio_set_stage_rom); diff --git a/src/mainboard/google/zork/verstage.c b/src/mainboard/google/zork/verstage.c index 7bfc107..8401196 100644 --- a/src/mainboard/google/zork/verstage.c +++ b/src/mainboard/google/zork/verstage.c @@ -12,7 +12,7 @@ size_t num_gpios; printk(BIOS_DEBUG, "Setting GPIOs\n"); - gpios = variant_romstage_gpio_table(&num_gpios); + gpios = variant_early_gpio_table(&num_gpios); program_gpios(gpios, num_gpios); printk(BIOS_DEBUG, "GPIOs setup\n"); } -- To view, visit https://review.coreboot.org/c/coreboot/+/42719 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ifbdbb02cbfc65ddb68f0ae75cf4b1f2ea1656b91 Gerrit-Change-Number: 42719 Gerrit-PatchSet: 1 Gerrit-Owner: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-MessageType: newchange

2 2

Change in coreboot[master]: mb/supermicro/x11-lga1151v2-series: Add support for X11SCH-F
by Christian Walter (Code Review) 25 Jun '20

25 Jun '20

Christian Walter has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37441 ) Change subject: mb/supermicro/x11-lga1151v2-series: Add support for X11SCH-F ...................................................................... Patch Set 57: (1 comment) https://review.coreboot.org/c/coreboot/+/37441/51//COMMIT_MSG Commit Message: https://review.coreboot.org/c/coreboot/+/37441/51//COMMIT_MSG@23 PS51, Line 23: 84 > decimal No it's hex. -- To view, visit https://review.coreboot.org/c/coreboot/+/37441 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I0ab1cb9462607b9af068bc2374508d99c60d0a30 Gerrit-Change-Number: 37441 Gerrit-PatchSet: 57 Gerrit-Owner: Christian Walter <christian.walter(a)9elements.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com> Gerrit-Reviewer: Guido Beyer @ Prodrive Technologies <guido.beyer(a)prodrive-technologies.com> Gerrit-Reviewer: Justin van Son <justin.van.son(a)prodrive-technologies.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Philipp Deppenwiese <zaolin.daisuki(a)gmail.com> Gerrit-Reviewer: Stef van Os <stef.van.os(a)prodrive-technologies.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Reviewer: wouter.eckhardt(a)prodrive-technologies.com Gerrit-CC: Jonas Löffelholz Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Comment-Date: Thu, 25 Jun 2020 07:16:51 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Jonas Löffelholz Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: lib: Add a function to print hex dump with prefix
by Paul Menzel (Code Review) 25 Jun '20

25 Jun '20

Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42793 ) Change subject: lib: Add a function to print hex dump with prefix ...................................................................... Patch Set 2: (1 comment) https://review.coreboot.org/c/coreboot/+/42793/2//COMMIT_MSG Commit Message: https://review.coreboot.org/c/coreboot/+/42793/2//COMMIT_MSG@12 PS2, Line 12: around the location where a memory bug is found. So the prefix before was the pointer address(?), and now instead of it, a custom prefix can be added? -- To view, visit https://review.coreboot.org/c/coreboot/+/42793 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I94869b10cd2d388a39cd5d86b1db936eff5473fa Gerrit-Change-Number: 42793 Gerrit-PatchSet: 2 Gerrit-Owner: Harshit Sharma <harshitsharmajs(a)gmail.com> Gerrit-Reviewer: Werner Zeh <werner.zeh(a)siemens.com> 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 06:19:34 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: lib: Add a function to print hex dump with prefix
by Harshit Sharma (Code Review) 25 Jun '20

25 Jun '20

Harshit Sharma has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42793 ) Change subject: lib: Add a function to print hex dump with prefix ...................................................................... Patch Set 2: This change is ready for review. -- To view, visit https://review.coreboot.org/c/coreboot/+/42793 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I94869b10cd2d388a39cd5d86b1db936eff5473fa Gerrit-Change-Number: 42793 Gerrit-PatchSet: 2 Gerrit-Owner: Harshit Sharma <harshitsharmajs(a)gmail.com> Gerrit-Reviewer: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Thu, 25 Jun 2020 06:13:16 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/amd/common: Avoid aliasing on ACPIMMIO writes
by Aaron Durbin (Code Review) 25 Jun '20

25 Jun '20

Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42692 ) Change subject: soc/amd/common: Avoid aliasing on ACPIMMIO writes ...................................................................... Patch Set 4: (1 comment) https://review.coreboot.org/c/coreboot/+/42692/4/src/soc/amd/common/block/i… File src/soc/amd/common/block/include/amdblocks/acpimmio.h: https://review.coreboot.org/c/coreboot/+/42692/4/src/soc/amd/common/block/i… PS4, Line 242: return read8((void *)acpimmio_iomux + reg); please do the cast after arithmetic -- To view, visit https://review.coreboot.org/c/coreboot/+/42692 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I7369b8b07a23e9b52c2961c527dc8d29ad4b505b Gerrit-Change-Number: 42692 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 06:01:25 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/amd/common: Refactor single GPIO programming
by Aaron Durbin (Code Review) 25 Jun '20

25 Jun '20

Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42691 ) Change subject: soc/amd/common: Refactor single GPIO programming ...................................................................... Patch Set 4: (1 comment) https://review.coreboot.org/c/coreboot/+/42691/4/src/soc/amd/common/block/g… File src/soc/amd/common/block/gpio_banks/gpio.c: https://review.coreboot.org/c/coreboot/+/42691/4/src/soc/amd/common/block/g… PS4, Line 216: if (gev_tbl == NULL) I'm sorry I'm currently being lazy. Could we lift the logic exactly in one patch and then modify accordingly to see the diff? I understand if you don't want to. I'll just need to come back w/ more scrutiny later. general approach looks good to me: let the compiler optimize whatever it wants and make things more readable. -- To view, visit https://review.coreboot.org/c/coreboot/+/42691 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I7aa6bd6996fd14dde4b1abcccbd2ae6ef933c87b Gerrit-Change-Number: 42691 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 06:00:11 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/amd/common: Complete ACPIMMIO GPIO bank separation
by Aaron Durbin (Code Review) 25 Jun '20

25 Jun '20

Aaron Durbin 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. I thought we already established this function? Why is it reintroduced? Or am I just confused now? -- 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 05:57:25 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/amd/common: Use gpio_setbits32()
by Aaron Durbin (Code Review) 25 Jun '20

25 Jun '20

Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42688 ) Change subject: soc/amd/common: Use gpio_setbits32() ...................................................................... Patch Set 4: (2 comments) https://review.coreboot.org/c/coreboot/+/42688/4//COMMIT_MSG Commit Message: https://review.coreboot.org/c/coreboot/+/42688/4//COMMIT_MSG@8 PS4, Line 8: commit description could use some elaboration w.r.t. motivation, etc. https://review.coreboot.org/c/coreboot/+/42688/4/src/soc/amd/common/block/g… File src/soc/amd/common/block/gpio_banks/gpio.c: https://review.coreboot.org/c/coreboot/+/42688/4/src/soc/amd/common/block/g… PS4, Line 103: reg32 |= (ctrl & mask); Why wouldn't this just call __gpio_update32() by passing in ~mask and ctrl (or just ctrl & mask) ? It seem odd to repeat the exact same sequence. tail call optimization should kick in. -- To view, visit https://review.coreboot.org/c/coreboot/+/42688 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I4413d113dff550900348a44f71b949b7547a9cfc Gerrit-Change-Number: 42688 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 05:50:05 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/amd/common: Refactor GPIO SCI/SMI interrupts
by Aaron Durbin (Code Review) 25 Jun '20

25 Jun '20

Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42687 ) Change subject: soc/amd/common: Refactor GPIO SCI/SMI interrupts ...................................................................... Patch Set 4: Code-Review+1 (1 comment) https://review.coreboot.org/c/coreboot/+/42687/4/src/soc/amd/common/block/g… File src/soc/amd/common/block/gpio_banks/gpio.c: https://review.coreboot.org/c/coreboot/+/42687/4/src/soc/amd/common/block/g… PS4, Line 73: regs->polarity &= ~mask; It'd be helpful to add comments to handling of LEVEL vs EDGE stanzas here. -- To view, visit https://review.coreboot.org/c/coreboot/+/42687 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ib2c7cd70ab38d0d8e745b0a611b780d2b0b8dc5b Gerrit-Change-Number: 42687 Gerrit-PatchSet: 4 Gerrit-Owner: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> 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 05:46:55 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit June 2020