coreboot-gerrit December 2021

coreboot-gerrit@coreboot.org

1 participants
5525 discussions

Change in coreboot[master]: soc/amd/cezanne/acpi: Add support for RTC workaround
by Felix Held (Code Review) 18 Dec '21

18 Dec '21

Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/60172 ) Change subject: soc/amd/cezanne/acpi: Add support for RTC workaround ...................................................................... soc/amd/cezanne/acpi: Add support for RTC workaround The RTC on Cezanne is an unstable wake source when the system is in S0i3. We instead need to use an internal timer that triggers a GPIO that acts as a wake source. This change provides the ACPI necessary to allow the OS to manage the wake source. BUG=b:209705576 TEST=Boot guybrush with this patch and several OS patches. Verified the OS sets the correct wake bit, the system correctly suspends and resumes, and the wake source is correctly accounted for. Signed-off-by: Raul E Rangel <rrangel(a)chromium.org> Change-Id: I1f14d14df5d30d48d244416f2ec8c10ac5c8040e Reviewed-on: https://review.coreboot.org/c/coreboot/+/60172 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Mario Limonciello <mario.limonciello(a)amd.corp-partner.google.com> Reviewed-by: Jason Glenesk <jason.glenesk(a)gmail.com> Reviewed-by: Paul Menzel <paulepanter(a)mailbox.org> Reviewed-by: Felix Held <felix-coreboot(a)felixheld.de> --- A src/soc/amd/cezanne/acpi/rtc_workaround.asl M src/soc/amd/cezanne/acpi/soc.asl 2 files changed, 28 insertions(+), 0 deletions(-) Approvals: build bot (Jenkins): Verified Paul Menzel: Looks good to me, but someone else must approve Felix Held: Looks good to me, approved Jason Glenesk: Looks good to me, but someone else must approve Mario Limonciello: Looks good to me, but someone else must approve diff --git a/src/soc/amd/cezanne/acpi/rtc_workaround.asl b/src/soc/amd/cezanne/acpi/rtc_workaround.asl new file mode 100644 index 0000000..e69e974 --- /dev/null +++ b/src/soc/amd/cezanne/acpi/rtc_workaround.asl @@ -0,0 +1,26 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +/* + * Workaround for RTC on Cezanne. + * See https://source.chromium.org/chromiumos/chromiumos/codesearch/+/main:src/thi… + */ + +Scope (\_SB.PEP) { + Name (_PRW, Package () { + Package() {\_SB.GPIO, 0}, + 0x03 + }) +} + +Scope (\_SB.GPIO) { + Name (_AEI, ResourceTemplate () { + GpioInt(Edge, ActiveHigh, ExclusiveAndWake, PullNone, 0x0000, "\\_SB.GPIO",,,,) + { + 44 /* int_shdwsysalarmfire */ + } + }) + + Method (_E2C, 0, Serialized) { + Notify (\_SB_.PEP, 0x02) + } +} diff --git a/src/soc/amd/cezanne/acpi/soc.asl b/src/soc/amd/cezanne/acpi/soc.asl index e603307..bf2838a 100644 --- a/src/soc/amd/cezanne/acpi/soc.asl +++ b/src/soc/amd/cezanne/acpi/soc.asl @@ -27,6 +27,8 @@ #include <soc/amd/common/acpi/upep.asl> +#include "rtc_workaround.asl" + /* * Platform Wake Notify * -- To view, visit https://review.coreboot.org/c/coreboot/+/60172 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I1f14d14df5d30d48d244416f2ec8c10ac5c8040e Gerrit-Change-Number: 60172 Gerrit-PatchSet: 2 Gerrit-Owner: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Mario Limonciello <mario.limonciello(a)amd.corp-partner.google.com> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

Change in coreboot[master]: soc/amd/cezanne/acpi: Add support for RTC workaround
by Felix Held (Code Review) 18 Dec '21

18 Dec '21

Attention is currently required from: Raul Rangel, Marshall Dawson, Karthik Ramasubramanian. Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/60172 ) Change subject: soc/amd/cezanne/acpi: Add support for RTC workaround ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/60172 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I1f14d14df5d30d48d244416f2ec8c10ac5c8040e Gerrit-Change-Number: 60172 Gerrit-PatchSet: 1 Gerrit-Owner: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Mario Limonciello <mario.limonciello(a)amd.corp-partner.google.com> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org> Gerrit-Attention: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Comment-Date: Sat, 18 Dec 2021 12:23:09 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in coreboot[master]: mainboard: Fix comment about early GPIOs
by Angel Pons (Code Review) 18 Dec '21

18 Dec '21

Angel Pons has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/60210 ) Change subject: mainboard: Fix comment about early GPIOs ...................................................................... mainboard: Fix comment about early GPIOs These boards program the early GPIO table in bootblock, not romstage. Change-Id: Iae9353d106483f30cefa2d035d96e63e4c127261 Signed-off-by: Angel Pons <th3fanbus(a)gmail.com> --- M src/mainboard/asrock/h110m/include/gpio.h M src/mainboard/google/glados/variants/asuka/include/variant/gpio.h M src/mainboard/google/glados/variants/caroline/include/variant/gpio.h M src/mainboard/google/glados/variants/cave/include/variant/gpio.h M src/mainboard/google/glados/variants/chell/include/variant/gpio.h M src/mainboard/google/glados/variants/glados/include/variant/gpio.h M src/mainboard/google/glados/variants/lars/include/variant/gpio.h M src/mainboard/google/glados/variants/sentry/include/variant/gpio.h M src/mainboard/intel/kblrvp/variants/rvp11/include/variant/gpio.h M src/mainboard/intel/kblrvp/variants/rvp3/include/variant/gpio.h M src/mainboard/intel/kblrvp/variants/rvp7/include/variant/gpio.h M src/mainboard/intel/kblrvp/variants/rvp8/include/variant/gpio.h M src/mainboard/intel/kunimitsu/gpio.h M src/mainboard/intel/saddlebrook/gpio.h M src/mainboard/starlabs/labtop/variants/tgl/gpio.c M src/mainboard/system76/lemp9/gpio.h 16 files changed, 16 insertions(+), 16 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/10/60210/1 diff --git a/src/mainboard/asrock/h110m/include/gpio.h b/src/mainboard/asrock/h110m/include/gpio.h index 9b2193b..18a1eb8 100644 --- a/src/mainboard/asrock/h110m/include/gpio.h +++ b/src/mainboard/asrock/h110m/include/gpio.h @@ -455,7 +455,7 @@ PAD_CFG_NF(GPP_I10, DN_20K, PLTRST, NF1), }; -/* Early pad configuration in romstage */ +/* Early pad configuration in bootblock */ static const struct pad_config early_gpio_table[] = { /* ------- GPIO Group GPP_A ------- */ /* GPP_A0 - RCIN# */ diff --git a/src/mainboard/google/glados/variants/asuka/include/variant/gpio.h b/src/mainboard/google/glados/variants/asuka/include/variant/gpio.h index d21d9e7..3ee5532 100644 --- a/src/mainboard/google/glados/variants/asuka/include/variant/gpio.h +++ b/src/mainboard/google/glados/variants/asuka/include/variant/gpio.h @@ -214,7 +214,7 @@ /* LANPHYC */ PAD_NC(GPD11, NONE), }; -/* Early pad configuration in romstage. */ +/* Early pad configuration in bootblock. */ static const struct pad_config early_gpio_table[] = { /* SPI_WP_STATUS */ PAD_CFG_GPI_GPIO_DRIVER(GPP_C23, UP_20K, DEEP), /* GD_UART2_RXD */ PAD_CFG_NF(GPP_C20, NONE, DEEP, NF1), diff --git a/src/mainboard/google/glados/variants/caroline/include/variant/gpio.h b/src/mainboard/google/glados/variants/caroline/include/variant/gpio.h index 95d2d04..7f8787c 100644 --- a/src/mainboard/google/glados/variants/caroline/include/variant/gpio.h +++ b/src/mainboard/google/glados/variants/caroline/include/variant/gpio.h @@ -231,7 +231,7 @@ /* LANPHYC */ PAD_NC(GPD11, NONE), }; -/* Early pad configuration in romstage. */ +/* Early pad configuration in bootblock. */ static const struct pad_config early_gpio_table[] = { /* UART2_CTS# */ PAD_CFG_GPI_GPIO_DRIVER(GPP_C23, UP_20K, DEEP), /* PCH_WP */ /* GD_UART2_RXD */ PAD_CFG_NF(GPP_C20, NONE, DEEP, NF1), diff --git a/src/mainboard/google/glados/variants/cave/include/variant/gpio.h b/src/mainboard/google/glados/variants/cave/include/variant/gpio.h index cd71eba..0eb7de7 100644 --- a/src/mainboard/google/glados/variants/cave/include/variant/gpio.h +++ b/src/mainboard/google/glados/variants/cave/include/variant/gpio.h @@ -223,7 +223,7 @@ /* LANPHYC */ PAD_NC(GPD11, NONE), }; -/* Early pad configuration in romstage. */ +/* Early pad configuration in bootblock. */ static const struct pad_config early_gpio_table[] = { /* UART2_CTS# */ PAD_CFG_GPI_GPIO_DRIVER(GPP_C23, UP_20K, DEEP), /* PCH_WP */ /* GD_UART2_RXD */ PAD_CFG_NF(GPP_C20, NONE, DEEP, NF1), diff --git a/src/mainboard/google/glados/variants/chell/include/variant/gpio.h b/src/mainboard/google/glados/variants/chell/include/variant/gpio.h index ce78ed3..e152862 100644 --- a/src/mainboard/google/glados/variants/chell/include/variant/gpio.h +++ b/src/mainboard/google/glados/variants/chell/include/variant/gpio.h @@ -217,7 +217,7 @@ /* LANPHYC */ PAD_NC(GPD11, NONE), }; -/* Early pad configuration in romstage. */ +/* Early pad configuration in bootblock. */ static const struct pad_config early_gpio_table[] = { /* SRCCLKREQ2# */ PAD_CFG_NF(GPP_B7, NONE, DEEP, NF1), /* KEPLER */ /* UART0_CTS# */ PAD_CFG_GPO(GPP_C11, 0, DEEP), /* EN_PP3300_KEPLER */ diff --git a/src/mainboard/google/glados/variants/glados/include/variant/gpio.h b/src/mainboard/google/glados/variants/glados/include/variant/gpio.h index fc4eebde..248e7dd 100644 --- a/src/mainboard/google/glados/variants/glados/include/variant/gpio.h +++ b/src/mainboard/google/glados/variants/glados/include/variant/gpio.h @@ -220,7 +220,7 @@ /* LANPHYC */ /* GPD11 */ }; -/* Early pad configuration in romstage. */ +/* Early pad configuration in bootblock. */ static const struct pad_config early_gpio_table[] = { /* SRCCLKREQ2# */ PAD_CFG_NF(GPP_B7, NONE, DEEP, NF1), /* KEPLER */ /* UART0_CTS# */ PAD_CFG_GPO(GPP_C11, 0, DEEP), /* EN_PP3300_KEPLER */ diff --git a/src/mainboard/google/glados/variants/lars/include/variant/gpio.h b/src/mainboard/google/glados/variants/lars/include/variant/gpio.h index 51514b2..ee55512 100644 --- a/src/mainboard/google/glados/variants/lars/include/variant/gpio.h +++ b/src/mainboard/google/glados/variants/lars/include/variant/gpio.h @@ -208,7 +208,7 @@ /* LANPHYC */ PAD_NC(GPD11, NONE), }; -/* Early pad configuration in romstage. */ +/* Early pad configuration in bootblock. */ static const struct pad_config early_gpio_table[] = { /* SRCCLKREQ2# */ PAD_CFG_NF(GPP_B7, NONE, DEEP, NF1), /* KEPLER */ /* UART0_CTS# */ PAD_CFG_GPO(GPP_C11, 0, DEEP), /* EN_PP3300_KEPLER */ diff --git a/src/mainboard/google/glados/variants/sentry/include/variant/gpio.h b/src/mainboard/google/glados/variants/sentry/include/variant/gpio.h index b4011c7..06f6cd8 100644 --- a/src/mainboard/google/glados/variants/sentry/include/variant/gpio.h +++ b/src/mainboard/google/glados/variants/sentry/include/variant/gpio.h @@ -216,7 +216,7 @@ /* LANPHYC */ PAD_NC(GPD11, NONE), }; -/* Early pad configuration in romstage. */ +/* Early pad configuration in bootblock. */ static const struct pad_config early_gpio_table[] = { /* SRCCLKREQ2# */ PAD_CFG_NF(GPP_B7, NONE, DEEP, NF1), /* KEPLER */ /* SPI_WP_STATUS */ PAD_CFG_GPI_GPIO_DRIVER(GPP_C23, UP_20K, DEEP), diff --git a/src/mainboard/intel/kblrvp/variants/rvp11/include/variant/gpio.h b/src/mainboard/intel/kblrvp/variants/rvp11/include/variant/gpio.h index fe8c2e2..aaec3eb 100644 --- a/src/mainboard/intel/kblrvp/variants/rvp11/include/variant/gpio.h +++ b/src/mainboard/intel/kblrvp/variants/rvp11/include/variant/gpio.h @@ -257,7 +257,7 @@ /* LANPHYC */ PAD_CFG_NF(GPD11, NONE, DEEP, NF1), }; -/* Early pad configuration in romstage. */ +/* Early pad configuration in bootblock. */ static const struct pad_config early_gpio_table[] = { /* UART0_RXD */ PAD_CFG_NF(GPP_C8, NONE, DEEP, NF1), /* UART0_TXD */ PAD_CFG_NF(GPP_C9, NONE, DEEP, NF1), diff --git a/src/mainboard/intel/kblrvp/variants/rvp3/include/variant/gpio.h b/src/mainboard/intel/kblrvp/variants/rvp3/include/variant/gpio.h index e0292cf..26eb1d9 100644 --- a/src/mainboard/intel/kblrvp/variants/rvp3/include/variant/gpio.h +++ b/src/mainboard/intel/kblrvp/variants/rvp3/include/variant/gpio.h @@ -193,7 +193,7 @@ /* LANPHYC */ PAD_CFG_NF(GPD11, NONE, DEEP, NF1), }; -/* Early pad configuration in romstage. */ +/* Early pad configuration in bootblock. */ static const struct pad_config early_gpio_table[] = { /* UART2_RXD */ PAD_CFG_NF(GPP_C20, NONE, DEEP, NF1), /* UART2_TXD */ PAD_CFG_NF(GPP_C21, NONE, DEEP, NF1), diff --git a/src/mainboard/intel/kblrvp/variants/rvp7/include/variant/gpio.h b/src/mainboard/intel/kblrvp/variants/rvp7/include/variant/gpio.h index a608505..d2e76d9 100644 --- a/src/mainboard/intel/kblrvp/variants/rvp7/include/variant/gpio.h +++ b/src/mainboard/intel/kblrvp/variants/rvp7/include/variant/gpio.h @@ -193,7 +193,7 @@ /* LANPHYC */ PAD_CFG_NF(GPD11, NONE, DEEP, NF1), }; -/* Early pad configuration in romstage. */ +/* Early pad configuration in bootblock. */ static const struct pad_config early_gpio_table[] = { /* UART0_RXD */ PAD_CFG_NF(GPP_C8, NONE, DEEP, NF1), /* UART0_TXD */ PAD_CFG_NF(GPP_C9, NONE, DEEP, NF1), diff --git a/src/mainboard/intel/kblrvp/variants/rvp8/include/variant/gpio.h b/src/mainboard/intel/kblrvp/variants/rvp8/include/variant/gpio.h index 5b588a1..44997c7 100644 --- a/src/mainboard/intel/kblrvp/variants/rvp8/include/variant/gpio.h +++ b/src/mainboard/intel/kblrvp/variants/rvp8/include/variant/gpio.h @@ -159,7 +159,7 @@ }; -/* Early pad configuration in romstage. */ +/* Early pad configuration in bootblock. */ static const struct pad_config early_gpio_table[] = { /* UART0_RXD */ PAD_CFG_NF(GPP_C8, NONE, DEEP, NF1), /* UART0_TXD */ PAD_CFG_NF(GPP_C9, NONE, DEEP, NF1), diff --git a/src/mainboard/intel/kunimitsu/gpio.h b/src/mainboard/intel/kunimitsu/gpio.h index 439bd89..a436d8f 100644 --- a/src/mainboard/intel/kunimitsu/gpio.h +++ b/src/mainboard/intel/kunimitsu/gpio.h @@ -217,7 +217,7 @@ /* LANPHYC */ PAD_NC(GPD11, NONE), }; -/* Early pad configuration in romstage. */ +/* Early pad configuration in bootblock. */ static const struct pad_config early_gpio_table[] = { /* SRCCLKREQ2# */ PAD_CFG_NF(GPP_B7, NONE, DEEP, NF1), /* KEPLER */ /* SPI_WP_STATUS */ PAD_CFG_GPI_GPIO_DRIVER(GPP_C23, UP_20K, DEEP), diff --git a/src/mainboard/intel/saddlebrook/gpio.h b/src/mainboard/intel/saddlebrook/gpio.h index f60515b..b012462 100644 --- a/src/mainboard/intel/saddlebrook/gpio.h +++ b/src/mainboard/intel/saddlebrook/gpio.h @@ -245,7 +245,7 @@ /* LANPHYC */ PAD_CFG_NF(GPD11, NONE, DEEP, NF1), }; -/* Early pad configuration in romstage. */ +/* Early pad configuration in bootblock. */ static const struct pad_config early_gpio_table[] = { /* UART0_CTS# */ PAD_CFG_GPO(GPP_C11, 1, DEEP), /* EN_PP3300_KEPLER */ }; diff --git a/src/mainboard/starlabs/labtop/variants/tgl/gpio.c b/src/mainboard/starlabs/labtop/variants/tgl/gpio.c index a541c07..81f6cb7 100644 --- a/src/mainboard/starlabs/labtop/variants/tgl/gpio.c +++ b/src/mainboard/starlabs/labtop/variants/tgl/gpio.c @@ -7,7 +7,7 @@ * using the stock BIOS and with coreboot. */ -/* Early pad configuration in romstage. */ +/* Early pad configuration in bootblock. */ const struct pad_config early_gpio_table[] = { PAD_CFG_NF(GPP_C20, NONE, DEEP, NF1), PAD_CFG_NF(GPP_C21, NONE, DEEP, NF1), diff --git a/src/mainboard/system76/lemp9/gpio.h b/src/mainboard/system76/lemp9/gpio.h index 6e9f481..60cc7b6 100644 --- a/src/mainboard/system76/lemp9/gpio.h +++ b/src/mainboard/system76/lemp9/gpio.h @@ -10,7 +10,7 @@ /* Name format: <pad name> / <net/pin name in schematics> */ -/* Early pad configuration in romstage. */ +/* Early pad configuration in bootblock. */ static const struct pad_config early_gpio_table[] = { PAD_CFG_NF(GPP_C20, NONE, DEEP, NF1), /* UART2_RXD */ PAD_CFG_NF(GPP_C21, NONE, DEEP, NF1), /* UART2_TXD */ -- To view, visit https://review.coreboot.org/c/coreboot/+/60210 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Iae9353d106483f30cefa2d035d96e63e4c127261 Gerrit-Change-Number: 60210 Gerrit-PatchSet: 1 Gerrit-Owner: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: newchange

1 0

Change in coreboot[master]: mainboard/starlabs: Add LabTop Mk IV
by Angel Pons (Code Review) 18 Dec '21

18 Dec '21

Attention is currently required from: Sean Rhodes, Andy Pont. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/58428 ) Change subject: mainboard/starlabs: Add LabTop Mk IV ...................................................................... Patch Set 66: Code-Review+1 (15 comments) File src/mainboard/starlabs/labtop/Kconfig.name: https://review.coreboot.org/c/coreboot/+/58428/comment/45f64760_68bfe2d0 PS66, Line 4: i3-10110u and i7-10710u nit: `u` in the model names should be uppercase File src/mainboard/starlabs/labtop/include/memory.h: https://review.coreboot.org/c/coreboot/+/58428/comment/0e1acdc2_eb99c56a PS66, Line 6: u8 get_memory_config_straps(void); : const struct cnl_mb_cfg *get_memory_cfg(struct cnl_mb_cfg *mem_cfg); You don't need this file at all if you make these functions static (in romstage.c). File src/mainboard/starlabs/labtop/variants/cml/devicetree.cb: https://review.coreboot.org/c/coreboot/+/58428/comment/8f6ca2ce_76398478 PS66, Line 125: device pci 19.0 off end # I2C4 Even if it's unused, you should enable this PCI device so that UART #2 can be enumerated properly. I recall that the PCI spec requires that multifunction devices implement function 0. You should also add `[PchSerialIoIndexI2C4] = PchSerialIoSkipInit,` to `SerialIoDevMode` so that FSP leaves the device enabled but skips configuring it. https://review.coreboot.org/c/coreboot/+/58428/comment/4569f439_25fda37f PS66, Line 182: off Not sure if this should be set to `hidden` File src/mainboard/starlabs/labtop/variants/cml/gpio.c: https://review.coreboot.org/c/coreboot/+/58428/comment/3c79cf18_99f50989 PS66, Line 10: romstage bootblock https://review.coreboot.org/c/coreboot/+/58428/comment/bdd63d42_d7bf1633 PS66, Line 12: PAD_CFG_GPO(GPP_E22, 1, PLTRST), : PAD_CFG_GPO(GPP_E23, 1, PLTRST), The native functions for these GPIOs are DPPD_CTRLCLK and DPPD_CTRLDATA, respectively. Do you know why they're used as GPOs? https://review.coreboot.org/c/coreboot/+/58428/comment/3e136dce_c3810d57 PS66, Line 14: PAD_CFG_GPI(GPP_H6, NONE, PLTRST), : PAD_CFG_GPI(GPP_H7, NONE, PLTRST), Am curious about these as well. File src/mainboard/starlabs/labtop/variants/cml/romstage.c: https://review.coreboot.org/c/coreboot/+/58428/comment/86463f2a_b544053a PS66, Line 12: u8 get_memory_config_straps(void) This function should be declared as static. https://review.coreboot.org/c/coreboot/+/58428/comment/6e272a00_cbefd682 PS66, Line 19: Missing opening parenthesis `(` here https://review.coreboot.org/c/coreboot/+/58428/comment/ec26876f_bcf341e6 PS66, Line 38: gpio_t nit: `const gpio_t` https://review.coreboot.org/c/coreboot/+/58428/comment/ae08805b_86783fa9 PS66, Line 39: (u8) Is it necessary to use `u8`? I'd simply use `u32` or `unsigned int` to avoid casts. https://review.coreboot.org/c/coreboot/+/58428/comment/65d91544_c4ec6913 PS66, Line 42: const struct cnl_mb_cfg *get_memory_cfg(struct cnl_mb_cfg *mem_cfg) This function should be declared as static. https://review.coreboot.org/c/coreboot/+/58428/comment/a85d6f07_7c510337 PS66, Line 46: struct cnl_mb_cfg std_memcfg = { : .rcomp_resistor = {121, 81, 100}, : .rcomp_targets = {100, 40, 20, 20, 26}, : .dq_pins_interleaved = 0, : .vref_ca_config = 2, : .ect = 0, : }; I'd put this initialisation in `mainboard_memory_init_params()` so that this function only fills in the SPD data. static bool is_dual_channel(const unsigned int memid) { return memid != 3 && memid != 5 && memid != 7; } static void fill_spd_data(struct cnl_mb_cfg *mem_cfg) { const unsigned int memid = get_memory_config_straps(); printk(BIOS_DEBUG, "Memory config straps: 0x%.2x\n", memid); /* * If we are using single channel ID = 3, 5 or 7 then we only * populate .spd[0].If we are dual channel then we also populate * .spd[2] as well. */ mem_cfg->spd[0].read_type = READ_SPD_CBFS; mem_cfg->spd[0].spd_spec.spd_index = memid; if (is_dual_channel(memid)) { mem_cfg->spd[2].read_type = READ_SPD_CBFS; mem_cfg->spd[2].spd_spec.spd_index = memid; } } void mainboard_memory_init_params(FSPM_UPD *memupd) { struct cnl_mb_cfg memcfg = { .rcomp_resistor = {121, 81, 100}, .rcomp_targets = {100, 40, 20, 20, 26}, .dq_pins_interleaved = 0, .vref_ca_config = 2, .ect = 0, }; const uint8_t vtd = get_uint_option("vtd", 1); memupd->FspmTestConfig.VtdDisable = !vtd; const uint8_t ht = get_uint_option("hyper_threading", memupd->FspmConfig.HyperThreading); memupd->FspmConfig.HyperThreading = ht; fill_spd_data(&memcfg); cannonlake_memcfg_init(&memupd->FspmConfig, &memcfg); } https://review.coreboot.org/c/coreboot/+/58428/comment/748fd5fc_d4bcda54 PS66, Line 66: memid != 3 && memid != 5 && memid != 7 Idea: put this into a helper function: static bool is_dual_channel(const unsigned int memid) { return memid != 3 && memid != 5 && memid != 7; } https://review.coreboot.org/c/coreboot/+/58428/comment/f7e62c80_47f7a6b7 PS66, Line 72: ; Semicolon not needed -- To view, visit https://review.coreboot.org/c/coreboot/+/58428 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Idbaa907dc38dc521961806132f21b7a90324ec9c Gerrit-Change-Number: 58428 Gerrit-PatchSet: 66 Gerrit-Owner: Sean Rhodes <admin(a)starlabs.systems> Gerrit-Reviewer: Andy Pont <andy.pont(a)sdcsystems.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Sean Rhodes <admin(a)starlabs.systems> Gerrit-Attention: Andy Pont <andy.pont(a)sdcsystems.com> Gerrit-Comment-Date: Sat, 18 Dec 2021 12:09:28 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in coreboot[master]: mb/acer/g43t-am3: Add documentation
by Angel Pons (Code Review) 18 Dec '21

18 Dec '21

Attention is currently required from: Felix Singer, Paul Menzel, Michael Büchler. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56838 ) Change subject: mb/acer/g43t-am3: Add documentation ...................................................................... Patch Set 3: Code-Review+1 (1 comment) File Documentation/mainboard/acer/g43t-am3.md: https://review.coreboot.org/c/coreboot/+/56838/comment/77cb8b7d_a82c4745 PS2, Line 108: flashrom -p internal -r backup.rom > I'm pretty sure I haven't, but I found the board in a dumpster so it might very well be that somebody before me did. The relevant fields in the Flash Descriptor are all 0xff.. suspicious?! > > I'll rewrite that section to warn about this.. > > Actually I think flashrom always gave a note that `The Flash Descriptor Override Strap-Pin is set`. Should I leave it like this? Hmmm, this explains why the IFD restrictions are ignored. I'm sure it's controlled with the red jumper labelled "ME DISABLE", between the ICH10 southbridge and the SATA ports: https://imgur.com/N98fJsR.png Would be good to test if changing this jumper results in the IFD restrictions being enforced (and the flashrom note disappearing), and documenting the jumper if so. -- To view, visit https://review.coreboot.org/c/coreboot/+/56838 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I0e296b3efbff0260f32badc699f1062f9885fa53 Gerrit-Change-Number: 56838 Gerrit-PatchSet: 3 Gerrit-Owner: Michael Büchler <michael.buechler(a)posteo.net> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Michael Büchler <michael.buechler(a)posteo.net> Gerrit-Comment-Date: Sat, 18 Dec 2021 11:37:38 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Comment-In-Reply-To: Michael Büchler <michael.buechler(a)posteo.net> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/amd/stoneyridge/fch: add GNVS-related TODOs
by Kyösti Mälkki (Code Review) 18 Dec '21

18 Dec '21

Attention is currently required from: Marshall Dawson, Felix Held. Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/60196 ) Change subject: soc/amd/stoneyridge/fch: add GNVS-related TODOs ...................................................................... Patch Set 2: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/60196 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I3691b05606b9430cb60923780a6131993a9887d4 Gerrit-Change-Number: 60196 Gerrit-PatchSet: 2 Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Comment-Date: Sat, 18 Dec 2021 08:40:13 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in coreboot[master]: crossgcc: Update acpica from 20210331 to 20211217
by HAOUAS Elyes (Code Review) 18 Dec '21

18 Dec '21

Attention is currently required from: HAOUAS Elyes. Hello build bot (Jenkins), Paul Menzel, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/59174 to look at the new patch set (#10). Change subject: crossgcc: Update acpica from 20210331 to 20211217 ...................................................................... crossgcc: Update acpica from 20210331 to 20211217 Changes: Version 20211217: https://acpica.org/node/197 Version 20210930: https://acpica.org/node/196 Version 20210730: https://acpica.org/node/195 Version 20210604: https://acpica.org/node/193 Change-Id: I3a03b74e95f910b50aa2f7ce502b1e9ad5b6df37 Signed-off-by: Elyes HAOUAS <ehaouas(a)noos.fr> --- M util/crossgcc/buildgcc R util/crossgcc/patches/acpica-unix2-20211217_iasl.patch D util/crossgcc/sum/acpica-unix2-20210331.tar.gz.cksum A util/crossgcc/sum/acpica-unix2-20211217.tar.gz.cksum 4 files changed, 2 insertions(+), 2 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/74/59174/10 -- To view, visit https://review.coreboot.org/c/coreboot/+/59174 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I3a03b74e95f910b50aa2f7ce502b1e9ad5b6df37 Gerrit-Change-Number: 59174 Gerrit-PatchSet: 10 Gerrit-Owner: HAOUAS Elyes <ehaouas(a)noos.fr> Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: HAOUAS Elyes <ehaouas(a)noos.fr> Gerrit-MessageType: newpatchset

1 0

Change in coreboot[master]: mb/google/guybrush: Disable GPIO export for Goodix Touchscreen
by Felix Held (Code Review) 18 Dec '21

18 Dec '21

Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/60173 ) Change subject: mb/google/guybrush: Disable GPIO export for Goodix Touchscreen ...................................................................... mb/google/guybrush: Disable GPIO export for Goodix Touchscreen We want ACPI to own the GPIOs. This will stop the GPIOs from being exposed to the OS driver. BUG=b:209705576, b:210694108 TEST=Dump ACPI table and verify GPIO are no longer in _CRS. Signed-off-by: Raul E Rangel <rrangel(a)chromium.org> Change-Id: I8d2af41e1d04b98f0e3e19a95d7b91d08ecdf17b Reviewed-on: https://review.coreboot.org/c/coreboot/+/60173 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Rob Barnes <robbarnes(a)google.com> --- M src/mainboard/google/guybrush/variants/guybrush/overridetree.cb 1 file changed, 1 insertion(+), 0 deletions(-) Approvals: build bot (Jenkins): Verified Rob Barnes: Looks good to me, approved diff --git a/src/mainboard/google/guybrush/variants/guybrush/overridetree.cb b/src/mainboard/google/guybrush/variants/guybrush/overridetree.cb index 01876c8..934fdbf 100644 --- a/src/mainboard/google/guybrush/variants/guybrush/overridetree.cb +++ b/src/mainboard/google/guybrush/variants/guybrush/overridetree.cb @@ -103,6 +103,7 @@ register "generic.stop_delay_ms" = "170" register "generic.stop_off_delay_ms" = "1" register "generic.has_power_resource" = "1" + register "generic.disable_gpio_export_in_crs" = "1" register "hid_desc_reg_offset" = "0x01" device i2c 5d on end end -- To view, visit https://review.coreboot.org/c/coreboot/+/60173 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I8d2af41e1d04b98f0e3e19a95d7b91d08ecdf17b Gerrit-Change-Number: 60173 Gerrit-PatchSet: 2 Gerrit-Owner: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Rob Barnes <robbarnes(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-MessageType: merged

1 0

Change in coreboot[master]: soc/amd/common/block/acpimmio/print_reset_status: add missing status bit
by Felix Held (Code Review) 18 Dec '21

18 Dec '21

Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/60184 ) Change subject: soc/amd/common/block/acpimmio/print_reset_status: add missing status bit ...................................................................... soc/amd/common/block/acpimmio/print_reset_status: add missing status bit Both the Picasso PPR #55570 Rev 3.18 and the Cezanne PPR #56569 Rev 3.03 define bit 9 of the PM_RST_STATUS register as internal Thermal Trip reset status bit. Signed-off-by: Felix Held <felix-coreboot(a)felixheld.de> Change-Id: Ida8b13fe62b16c18fc9924520b83220e73eca624 Reviewed-on: https://review.coreboot.org/c/coreboot/+/60184 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Raul Rangel <rrangel(a)chromium.org> Reviewed-by: Jason Glenesk <jason.glenesk(a)gmail.com> Reviewed-by: Paul Menzel <paulepanter(a)mailbox.org> --- M src/soc/amd/common/block/acpimmio/print_reset_status.c 1 file changed, 1 insertion(+), 0 deletions(-) Approvals: build bot (Jenkins): Verified Paul Menzel: Looks good to me, but someone else must approve Raul Rangel: Looks good to me, approved Jason Glenesk: Looks good to me, but someone else must approve diff --git a/src/soc/amd/common/block/acpimmio/print_reset_status.c b/src/soc/amd/common/block/acpimmio/print_reset_status.c index 3825753..309401e 100644 --- a/src/soc/amd/common/block/acpimmio/print_reset_status.c +++ b/src/soc/amd/common/block/acpimmio/print_reset_status.c @@ -33,6 +33,7 @@ [3] = "ThermalTripFromTemp", [4] = "RemotePowerDownFromASF", [5] = "ShutDownFan0", + [9] = "InternalThermalTrip", [16] = "UserRst", [17] = "SoftPciRst", [18] = "DoInit", -- To view, visit https://review.coreboot.org/c/coreboot/+/60184 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ida8b13fe62b16c18fc9924520b83220e73eca624 Gerrit-Change-Number: 60184 Gerrit-PatchSet: 3 Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

Change in coreboot[master]: mb/acer/g43t-am3: Add documentation
by Michael Büchler (Code Review) 18 Dec '21

18 Dec '21

Attention is currently required from: Felix Singer, Paul Menzel, Angel Pons. Michael Büchler has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56838 ) Change subject: mb/acer/g43t-am3: Add documentation ...................................................................... Patch Set 3: (3 comments) File Documentation/mainboard/acer/g43t-am3.md: https://review.coreboot.org/c/coreboot/+/56838/comment/cc003468_d29eef73 PS2, Line 105: `-c MX25L1605D/MX25L1608D/MX25L1673E` and `-c MX25L1605` should work. > Hmmm, the block erasers defined in flashrom for these chips are incompatible: opcode 0x20 erases 64 […] Oh interesting, so with `MX25L1605`, flashrom wrongly assumes that 64 KiB get erased when it issues a 0x20 command. Thanks for checking this. https://review.coreboot.org/c/coreboot/+/56838/comment/e69e30e5_9d72f636 PS2, Line 108: flashrom -p internal -r backup.rom > Huh, I would've expected the ME region to be unreadable internally. […] I'm pretty sure I haven't, but I found the board in a dumpster so it might very well be that somebody before me did. The relevant fields in the Flash Descriptor are all 0xff.. suspicious?! I'll rewrite that section to warn about this.. Actually I think flashrom always gave a note that `The Flash Descriptor Override Strap-Pin is set`. Should I leave it like this? https://review.coreboot.org/c/coreboot/+/56838/comment/b404939b_a332e01e PS2, Line 120: There : seems to be a diode that prevents you from powering the whole board with : your external programmer. > I typically avoid using 2nd person (you, your) when writing documentation. […] agreed -- To view, visit https://review.coreboot.org/c/coreboot/+/56838 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I0e296b3efbff0260f32badc699f1062f9885fa53 Gerrit-Change-Number: 56838 Gerrit-PatchSet: 3 Gerrit-Owner: Michael Büchler <michael.buechler(a)posteo.net> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Sat, 18 Dec 2021 01:39:31 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: comment

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit December 2021