coreboot-gerrit May 2021

coreboot-gerrit@coreboot.org

1 participants
6048 discussions

Change in coreboot[master]: drivers/sn65dsi86: Switch EDID reading to use "indirect mode"
by Julius Werner (Code Review) 07 May '21

07 May '21

Attention is currently required from: Vinod Polimera, Douglas Anderson, Xuxin Xiong. Hello Vinod Polimera, build bot (Jenkins), Douglas Anderson, Xuxin Xiong, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/52959 to look at the new patch set (#4). Change subject: drivers/sn65dsi86: Switch EDID reading to use "indirect mode" ...................................................................... drivers/sn65dsi86: Switch EDID reading to use "indirect mode" The SN65DSI86 eDP bridge supports two ways to read the EDID: for now we've been using "direct mode", which works by basically making the bridge I2C device listen to another chip address besides its own and proxy all requests received there directly to the eDP AUX channel. The great part about that mode is that it is super easy and hassle-free to use. The not so great part about it is that it doesn't work: for EDID extensions, the last byte (which happens to contain the checksum) is somehow always read as zero. We presume this is a hardware bug in the bridge part. The other, much more annoying way is "indirect mode", where each byte transmitted over the AUX channel has to be manually set up in the I2C registers of the bridge, just like we're already doing with DPCD transactions. Thankfully, we can reuse most of the DPCD code for this so it's not a lot of extra code. It's a bit slower but not as much as you'd expect (26ms instead of 18ms on my board), and the difference is not very relevant compared to common total times for display init. Also, some of the (previously unused) enum definitions for the AUX_CMD mode field of the bridge had just been plain wrong for some reason, and needed to be fixed to make this work. Signed-off-by: Julius Werner <jwerner(a)chromium.org> Change-Id: I65f80193380d3c3841f9f5c26897ed672f45e15a --- M src/drivers/ti/sn65dsi86bridge/sn65dsi86bridge.c 1 file changed, 100 insertions(+), 75 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/59/52959/4 -- To view, visit https://review.coreboot.org/c/coreboot/+/52959 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I65f80193380d3c3841f9f5c26897ed672f45e15a Gerrit-Change-Number: 52959 Gerrit-PatchSet: 4 Gerrit-Owner: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Douglas Anderson <dianders(a)chromium.org> Gerrit-Reviewer: Vinod Polimera <vpolimer(a)qualcomm.corp-partner.google.com> Gerrit-Reviewer: Xuxin Xiong <xuxinxiong(a)huaqin.corp-partner.google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Ravi Kumar Bokka <c_rbokka(a)qualcomm.corp-partner.google.com> Gerrit-CC: mturney mturney <mturney(a)codeaurora.org> Gerrit-Attention: Vinod Polimera <vpolimer(a)qualcomm.corp-partner.google.com> Gerrit-Attention: Douglas Anderson <dianders(a)chromium.org> Gerrit-Attention: Xuxin Xiong <xuxinxiong(a)huaqin.corp-partner.google.com> Gerrit-MessageType: newpatchset

1 0

Change in coreboot[master]: drivers/sn65dsi86: Switch EDID reading to use "indirect mode"
by Julius Werner (Code Review) 07 May '21

07 May '21

Attention is currently required from: Vinod Polimera, Douglas Anderson, Xuxin Xiong. Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/52959 ) Change subject: drivers/sn65dsi86: Switch EDID reading to use "indirect mode" ...................................................................... Patch Set 2: (1 comment) File src/drivers/ti/sn65dsi86bridge/sn65dsi86bridge.c: https://review.coreboot.org/c/coreboot/+/52959/comment/23ac7337_bf656adc PS2, Line 201: AUX_CMD_SEND > From looking at the kernel code (and the samples in the ti manual IIRC), I believe you can't set thi […] Sorry, I must have been tired or something... you're totally right of course. I'm not sure if setting the command before the send bit really makes a difference but I can do it just in case, too. We were also not waiting for acknowledgement when writing with the old code... probably a good idea to add that, too. -- To view, visit https://review.coreboot.org/c/coreboot/+/52959 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I65f80193380d3c3841f9f5c26897ed672f45e15a Gerrit-Change-Number: 52959 Gerrit-PatchSet: 2 Gerrit-Owner: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Douglas Anderson <dianders(a)chromium.org> Gerrit-Reviewer: Vinod Polimera <vpolimer(a)qualcomm.corp-partner.google.com> Gerrit-Reviewer: Xuxin Xiong <xuxinxiong(a)huaqin.corp-partner.google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Ravi Kumar Bokka <c_rbokka(a)qualcomm.corp-partner.google.com> Gerrit-CC: mturney mturney <mturney(a)codeaurora.org> Gerrit-Attention: Vinod Polimera <vpolimer(a)qualcomm.corp-partner.google.com> Gerrit-Attention: Douglas Anderson <dianders(a)chromium.org> Gerrit-Attention: Xuxin Xiong <xuxinxiong(a)huaqin.corp-partner.google.com> Gerrit-Comment-Date: Fri, 07 May 2021 00:42:45 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Douglas Anderson <dianders(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: drivers/sn65dsi86: Switch EDID reading to use "indirect mode"
by Julius Werner (Code Review) 07 May '21

07 May '21

Attention is currently required from: Vinod Polimera, Julius Werner, Xuxin Xiong. Hello Vinod Polimera, build bot (Jenkins), Douglas Anderson, Xuxin Xiong, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/52959 to look at the new patch set (#3). Change subject: drivers/sn65dsi86: Switch EDID reading to use "indirect mode" ...................................................................... drivers/sn65dsi86: Switch EDID reading to use "indirect mode" The SN65DSI86 eDP bridge supports two ways to read the EDID: for now we've been using "direct mode", which works by basically making the bridge I2C device listen to another chip address besides its own and proxy all requests received there directly to the eDP AUX channel. The great part about that mode is that it is super easy and hassle-free to use. The not so great part about it is that it doesn't work: for EDID extensions, the last byte (which happens to contain the checksum) is somehow always read as zero. We presume this is a hardware bug in the bridge part. The other, much more annoying way is "indirect mode", where each byte transmitted over the AUX channel has to be manually set up in the I2C registers of the bridge, just like we're already doing with DPCD transactions. Thankfully, we can reuse most of the DPCD code for this so it's not a lot of extra code. It's a bit slower but not as much as you'd expect (26ms instead of 18ms on my board), and the difference is not very relevant compared to common total times for display init. Also, some of the (previously unused) enum definitions for the AUX_CMD mode field of the bridge had just been plain wrong for some reason, and needed to be fixed to make this work. Signed-off-by: Julius Werner <jwerner(a)chromium.org> Change-Id: I65f80193380d3c3841f9f5c26897ed672f45e15a --- M src/drivers/ti/sn65dsi86bridge/sn65dsi86bridge.c 1 file changed, 102 insertions(+), 75 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/59/52959/3 -- To view, visit https://review.coreboot.org/c/coreboot/+/52959 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I65f80193380d3c3841f9f5c26897ed672f45e15a Gerrit-Change-Number: 52959 Gerrit-PatchSet: 3 Gerrit-Owner: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Douglas Anderson <dianders(a)chromium.org> Gerrit-Reviewer: Vinod Polimera <vpolimer(a)qualcomm.corp-partner.google.com> Gerrit-Reviewer: Xuxin Xiong <xuxinxiong(a)huaqin.corp-partner.google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Ravi Kumar Bokka <c_rbokka(a)qualcomm.corp-partner.google.com> Gerrit-CC: mturney mturney <mturney(a)codeaurora.org> Gerrit-Attention: Vinod Polimera <vpolimer(a)qualcomm.corp-partner.google.com> Gerrit-Attention: Julius Werner <jwerner(a)chromium.org> Gerrit-Attention: Xuxin Xiong <xuxinxiong(a)huaqin.corp-partner.google.com> Gerrit-MessageType: newpatchset

1 0

Change in coreboot[master]: soc/amd/cezanne: Force resets to be cold
by Marshall Dawson (Code Review) 07 May '21

07 May '21

Attention is currently required from: Jason Glenesk, Raul Rangel, Martin Roth. Marshall Dawson has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/52982 ) Change subject: soc/amd/cezanne: Force resets to be cold ...................................................................... Patch Set 1: (1 comment) Commit Message: https://review.coreboot.org/c/coreboot/+/52982/comment/bf32e4b6_162da80f PS1, Line 10: AGESA has cleared it in : FSP-S. > Setting the bit in the OS and rebooting, it already comes back set. […] That's pretty funny. I thought I was seeing intermittent different behavior, but now you're convincing me I wasn't crazy. -- To view, visit https://review.coreboot.org/c/coreboot/+/52982 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I7d4ca5665335b20100a5c802d12d79c0d0597ad9 Gerrit-Change-Number: 52982 Gerrit-PatchSet: 1 Gerrit-Owner: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Martin Roth <martinroth(a)google.com> Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org> Gerrit-Attention: Martin Roth <martinroth(a)google.com> Gerrit-Comment-Date: Thu, 06 May 2021 23:51:18 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Martin Roth <martinroth(a)google.com> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: mb/google/guybrush: Enable GFX HDA device
by Felix Held (Code Review) 07 May '21

07 May '21

Attention is currently required from: Raul Rangel, Martin Roth, Mathew King, Karthik Ramasubramanian. Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/52974 ) Change subject: mb/google/guybrush: Enable GFX HDA device ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/52974 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I5e35440e8e70ee125d37c7ac30c9219ec69c7c6e Gerrit-Change-Number: 52974 Gerrit-PatchSet: 1 Gerrit-Owner: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Mathew King <mathewk(a)chromium.org> Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org> Gerrit-Attention: Martin Roth <martinroth(a)google.com> Gerrit-Attention: Mathew King <mathewk(a)chromium.org> Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Comment-Date: Thu, 06 May 2021 23:41:11 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in coreboot[master]: mb/google/mancomb: Implement tis_plat_irq_status
by Felix Held (Code Review) 07 May '21

07 May '21

Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/52950 ) Change subject: mb/google/mancomb: Implement tis_plat_irq_status ...................................................................... mb/google/mancomb: Implement tis_plat_irq_status This patch was implemented on Guybrush at CB:52352 BUG=b:185397933 TEST=Build Signed-off-by: Martin Roth <martinroth(a)chromium.org> Change-Id: I141a504a827f37724fab0aaed7498fd543e471d6 Reviewed-on: https://review.coreboot.org/c/coreboot/+/52950 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Raul Rangel <rrangel(a)chromium.org> Reviewed-by: Marshall Dawson <marshalldawson3rd(a)gmail.com> --- M src/mainboard/google/mancomb/variants/baseboard/Makefile.inc A src/mainboard/google/mancomb/variants/baseboard/tpm_tis.c 2 files changed, 13 insertions(+), 0 deletions(-) Approvals: build bot (Jenkins): Verified Marshall Dawson: Looks good to me, approved Raul Rangel: Looks good to me, approved diff --git a/src/mainboard/google/mancomb/variants/baseboard/Makefile.inc b/src/mainboard/google/mancomb/variants/baseboard/Makefile.inc index 3b86b93..908a885 100644 --- a/src/mainboard/google/mancomb/variants/baseboard/Makefile.inc +++ b/src/mainboard/google/mancomb/variants/baseboard/Makefile.inc @@ -1,7 +1,11 @@ bootblock-y += gpio.c +romstage-y += tpm_tis.c + ramstage-y += gpio.c +ramstage-y += tpm_tis.c verstage-y += gpio.c +verstage-y += tpm_tis.c smm-y += gpio.c diff --git a/src/mainboard/google/mancomb/variants/baseboard/tpm_tis.c b/src/mainboard/google/mancomb/variants/baseboard/tpm_tis.c new file mode 100644 index 0000000..4550728 --- /dev/null +++ b/src/mainboard/google/mancomb/variants/baseboard/tpm_tis.c @@ -0,0 +1,9 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later */ + +#include <security/tpm/tis.h> +#include <soc/gpio.h> + +int tis_plat_irq_status(void) +{ + return gpio_interrupt_status(GPIO_3); +} -- To view, visit https://review.coreboot.org/c/coreboot/+/52950 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I141a504a827f37724fab0aaed7498fd543e471d6 Gerrit-Change-Number: 52950 Gerrit-PatchSet: 3 Gerrit-Owner: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Ivy Jian <ivy_jian(a)compal.corp-partner.google.com> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Reviewer: chris wang <Chris.Wang(a)amd.com> Gerrit-MessageType: merged

1 0

Change in coreboot[master]: mb/google/mancomb: Update Kconfig with needed options
by Felix Held (Code Review) 07 May '21

07 May '21

Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/52949 ) Change subject: mb/google/mancomb: Update Kconfig with needed options ...................................................................... mb/google/mancomb: Update Kconfig with needed options DISABLE_KEYBOARD_RESET_PIN - This pin goes to a test point and is not used for the reset. DRIVERS_UART_ACPI - Add the UART ACPI code FW_CONFIG - Mancomb uses the firmware config interface PSP_DISABLE_POSTCODES - The PSP is not yet initializing eSPI correctly to send post codes to the EC, so disable them for now. BUG=None Test=Build Signed-off-by: Martin Roth <martinroth(a)chromium.org> Change-Id: I39efcc8d1e0fb1e7ac0b0541a49db0ac0ee56481 Reviewed-on: https://review.coreboot.org/c/coreboot/+/52949 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Raul Rangel <rrangel(a)chromium.org> Reviewed-by: Marshall Dawson <marshalldawson3rd(a)gmail.com> --- M src/mainboard/google/mancomb/Kconfig 1 file changed, 4 insertions(+), 0 deletions(-) Approvals: build bot (Jenkins): Verified Marshall Dawson: Looks good to me, approved Raul Rangel: Looks good to me, approved diff --git a/src/mainboard/google/mancomb/Kconfig b/src/mainboard/google/mancomb/Kconfig index 4ed29cd..552c022 100644 --- a/src/mainboard/google/mancomb/Kconfig +++ b/src/mainboard/google/mancomb/Kconfig @@ -9,20 +9,24 @@ def_bool y select AMD_SOC_CONSOLE_UART select BOARD_ROMSIZE_KB_16384 + select DISABLE_KEYBOARD_RESET_PIN select DISABLE_SPI_FLASH_ROM_SHARING select DRIVERS_I2C_GENERIC select DRIVERS_I2C_HID + select DRIVERS_UART_ACPI select EC_GOOGLE_CHROMEEC select EC_GOOGLE_CHROMEEC_BOARDID select EC_GOOGLE_CHROMEEC_ESPI select EC_GOOGLE_CHROMEEC_SKUID select ELOG select ELOG_GSMI + select FW_CONFIG select HAVE_ACPI_RESUME select HAVE_EM100_SUPPORT select MAINBOARD_HAS_CHROMEOS select MAINBOARD_HAS_I2C_TPM_CR50 select MAINBOARD_HAS_TPM2 + select PSP_DISABLE_POSTCODES select SOC_AMD_CEZANNE select SOC_AMD_COMMON_BLOCK_USE_ESPI -- To view, visit https://review.coreboot.org/c/coreboot/+/52949 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I39efcc8d1e0fb1e7ac0b0541a49db0ac0ee56481 Gerrit-Change-Number: 52949 Gerrit-PatchSet: 2 Gerrit-Owner: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Ivy Jian <ivy_jian(a)compal.corp-partner.google.com> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Reviewer: chris wang <Chris.Wang(a)amd.com> Gerrit-MessageType: merged

1 0

Change in coreboot[master]: cezanne/psp_verstage: clean up duplicated target
by Kangheui Won (Code Review) 07 May '21

07 May '21

Attention is currently required from: Jason Glenesk, Raul Rangel, Martin Roth, Marshall Dawson, Felix Held. Kangheui Won has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/52962 ) Change subject: cezanne/psp_verstage: clean up duplicated target ...................................................................... Patch Set 3: (1 comment) File src/soc/amd/cezanne/psp_verstage/Makefile.inc: https://review.coreboot.org/c/coreboot/+/52962/comment/9ca6bcc6_6010bb53 PS1, Line 15: > Extra line Done -- To view, visit https://review.coreboot.org/c/coreboot/+/52962 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ica4b09282d1c4cfc555c18ba50951458b8580826 Gerrit-Change-Number: 52962 Gerrit-PatchSet: 3 Gerrit-Owner: Kangheui Won <khwon(a)chromium.org> 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: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Martin Roth <martinroth(a)google.com> Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org> Gerrit-Attention: Martin Roth <martinroth(a)google.com> Gerrit-Attention: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Comment-Date: Thu, 06 May 2021 23:38:20 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Martin Roth <martinroth(a)google.com> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: cezanne/psp_verstage: populate a/b firmware
by Kangheui Won (Code Review) 07 May '21

07 May '21

Attention is currently required from: Jason Glenesk, Raul Rangel, Martin Roth, Marshall Dawson, Arthur Heymans, Felix Held. Kangheui Won has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/52961 ) Change subject: cezanne/psp_verstage: populate a/b firmware ...................................................................... Patch Set 3: (1 comment) File src/mainboard/google/guybrush/Kconfig: https://review.coreboot.org/c/coreboot/+/52961/comment/039ca947_0c8d0edb PS1, Line 74: config CEZANNE_FW_A_POSITION : hex : default 0xFF012040 : depends on VBOOT_SLOTS_RW_AB : help : Location of the AMD firmware in the RW_A region. This is the : start of the RW-A region + 64 bytes for the cbfs header. : : config CEZANNE_FW_B_POSITION : hex : default 0xFF312040 : depends on VBOOT_SLOTS_RW_AB : help : Location of the AMD firmware in the RW_B region. This is the : start of the RW-A region + 64 bytes for the cbfs header. > Seems like a good plan. Done -- To view, visit https://review.coreboot.org/c/coreboot/+/52961 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Idb31afa7a513f01593b2af75515a170dfca8d360 Gerrit-Change-Number: 52961 Gerrit-PatchSet: 3 Gerrit-Owner: Kangheui Won <khwon(a)chromium.org> 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: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-CC: Martin Roth <martinroth(a)google.com> Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org> Gerrit-Attention: Martin Roth <martinroth(a)google.com> Gerrit-Attention: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Comment-Date: Thu, 06 May 2021 23:38:01 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Raul Rangel <rrangel(a)chromium.org> Comment-In-Reply-To: Martin Roth <martinroth(a)google.com> Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: mb/google/mancomb: Fix EC SCI configuration
by Felix Held (Code Review) 07 May '21

07 May '21

Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/52948 ) Change subject: mb/google/mancomb: Fix EC SCI configuration ...................................................................... mb/google/mancomb: Fix EC SCI configuration This change fixes two problems: 1) We had the enum values for .direction and .level swapped. The naming is very confusing... 2) ESPI_SYS is not a good event to use for EC SCI. It is a level/low event that is only cleared by reading the eSPI status register 0x9C. Cezanne has added a new event source that directly exposes the SCI bit. This is the correct event source to use for EC SCI. This same patch was added for Guybrush at CB:52673 BUG=b:186045622, b:181139095 TEST=Build Signed-off-by: Martin Roth <martinroth(a)chromium.org> Change-Id: Iac86d2ef5bdd21fbb0a0d4e235efe4fe621023b2 Reviewed-on: https://review.coreboot.org/c/coreboot/+/52948 Reviewed-by: Felix Held <felix-coreboot(a)felixheld.de> Reviewed-by: Raul Rangel <rrangel(a)chromium.org> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M src/mainboard/google/mancomb/ec.c 1 file changed, 3 insertions(+), 3 deletions(-) Approvals: build bot (Jenkins): Verified Felix Held: Looks good to me, approved Raul Rangel: Looks good to me, approved diff --git a/src/mainboard/google/mancomb/ec.c b/src/mainboard/google/mancomb/ec.c index 5a0bc1b..67fdda1 100644 --- a/src/mainboard/google/mancomb/ec.c +++ b/src/mainboard/google/mancomb/ec.c @@ -9,10 +9,10 @@ static const struct sci_source espi_sci_sources[] = { { - .scimap = SMITYPE_ESPI_SYS, + .scimap = SMITYPE_ESPI_SCI_B, .gpe = EC_SCI_GPI, - .direction = SMI_SCI_LVL, - .level = SMI_SCI_LVL_HIGH + .direction = SMI_SCI_LVL_HIGH, /* enum smi_sci_lvl */ + .level = SMI_SCI_EDG, /* enum smi_sci_dir */ } }; -- To view, visit https://review.coreboot.org/c/coreboot/+/52948 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Iac86d2ef5bdd21fbb0a0d4e235efe4fe621023b2 Gerrit-Change-Number: 52948 Gerrit-PatchSet: 2 Gerrit-Owner: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Ivy Jian <ivy_jian(a)compal.corp-partner.google.com> Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Reviewer: chris wang <Chris.Wang(a)amd.com> Gerrit-MessageType: merged

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit May 2021