coreboot-gerrit March 2022

coreboot-gerrit@coreboot.org

1 participants
6700 discussions

Change in coreboot[master]: util/amdfwtool: use ISH support for Sabrina SoC
by Felix Held (Code Review) 31 Mar '22

31 Mar '22

Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/63186 ) Change subject: util/amdfwtool: use ISH support for Sabrina SoC ...................................................................... util/amdfwtool: use ISH support for Sabrina SoC The PSP in the Sabrina SoC uses the image slot header to find the second level PSP directory table, so it needs the ISH to be generated. Signed-off-by: Felix Held <felix-coreboot(a)felixheld.de> Change-Id: I9e6308854147c9f6f72d722215c833ee86ee4f94 Reviewed-on: https://review.coreboot.org/c/coreboot/+/63186 Reviewed-by: Raul Rangel <rrangel(a)chromium.org> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M util/amdfwtool/amdfwtool.c 1 file changed, 11 insertions(+), 0 deletions(-) Approvals: build bot (Jenkins): Verified Raul Rangel: Looks good to me, approved diff --git a/util/amdfwtool/amdfwtool.c b/util/amdfwtool/amdfwtool.c index 4b76228..80595ddb 100644 --- a/util/amdfwtool/amdfwtool.c +++ b/util/amdfwtool/amdfwtool.c @@ -1458,6 +1458,14 @@ } +static bool needs_ish(enum platform platform_type) +{ + if (platform_type == PLATFORM_SABRINA) + return true; + else + return false; +} + int main(int argc, char **argv) { int c; @@ -1689,6 +1697,9 @@ } } + if (needs_ish(soc_id)) + cb_config.need_ish = true; + if (cb_config.need_ish) cb_config.recovery_ab = true; -- To view, visit https://review.coreboot.org/c/coreboot/+/63186 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I9e6308854147c9f6f72d722215c833ee86ee4f94 Gerrit-Change-Number: 63186 Gerrit-PatchSet: 3 Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> 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]: soc/amd/sabrina/include/gpio: add I3C3 IOMUX definitions
by Karthik Ramasubramanian (Code Review) 31 Mar '22

31 Mar '22

Attention is currently required from: Jason Glenesk, Raul Rangel, Marshall Dawson, Fred Reitberger, Felix Held. Karthik Ramasubramanian has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/63232 ) Change subject: soc/amd/sabrina/include/gpio: add I3C3 IOMUX definitions ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/63232 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I9688f1816aa840c64441495ed451997a474b306f Gerrit-Change-Number: 63232 Gerrit-PatchSet: 1 Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org> Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org> Gerrit-Attention: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Attention: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Comment-Date: Wed, 30 Mar 2022 23:42:15 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in coreboot[master]: util/amdfwtool: add Sabrina SoC type
by Felix Held (Code Review) 31 Mar '22

31 Mar '22

Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/63185 ) Change subject: util/amdfwtool: add Sabrina SoC type ...................................................................... util/amdfwtool: add Sabrina SoC type Add PLATFORM_SABRINA to the enum of supported platforms and integrate it into the existing code. Signed-off-by: Zheng Bao <fishbaozi(a)gmail.com> Signed-off-by: Felix Held <felix-coreboot(a)felixheld.de> Change-Id: Ibe52b44395619f697686bd900a522562abbe7646 Reviewed-on: https://review.coreboot.org/c/coreboot/+/63185 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Raul Rangel <rrangel(a)chromium.org> --- M util/amdfwtool/amdfwtool.c 1 file changed, 6 insertions(+), 0 deletions(-) Approvals: build bot (Jenkins): Verified Raul Rangel: Looks good to me, approved diff --git a/util/amdfwtool/amdfwtool.c b/util/amdfwtool/amdfwtool.c index 7c977a2..4b76228 100644 --- a/util/amdfwtool/amdfwtool.c +++ b/util/amdfwtool/amdfwtool.c @@ -558,6 +558,7 @@ PLATFORM_CEZANNE, PLATFORM_MENDOCINO, PLATFORM_LUCIENNE, + PLATFORM_SABRINA, }; static uint32_t get_psp_id(enum platform soc_id) @@ -576,6 +577,7 @@ psp_id = 0xBC0C0140; break; case PLATFORM_MENDOCINO: + case PLATFORM_SABRINA: psp_id = 0xBC0D0900; break; case PLATFORM_STONEYRIDGE: @@ -1406,6 +1408,7 @@ case PLATFORM_LUCIENNE: case PLATFORM_CEZANNE: case PLATFORM_MENDOCINO: + case PLATFORM_SABRINA: amd_romsig->efs_gen.gen = EFS_SECOND_GEN; amd_romsig->spi_readmode_f17_mod_30_3f = efs_spi_readmode; amd_romsig->spi_fastspeed_f17_mod_30_3f = efs_spi_speed; @@ -1448,6 +1451,8 @@ return PLATFORM_RENOIR; else if (!strcasecmp(soc_name, "Lucienne")) return PLATFORM_LUCIENNE; + else if (!strcasecmp(soc_name, "Sabrina")) + return PLATFORM_SABRINA; else return PLATFORM_UNKNOWN; @@ -1898,6 +1903,7 @@ amd_romsig->bios3_entry = BUFF_TO_RUN(ctx, biosdir); break; case PLATFORM_MENDOCINO: + case PLATFORM_SABRINA: break; case PLATFORM_STONEYRIDGE: case PLATFORM_RAVEN: 1 is the latest approved patch-set. No files were changed between the latest approved patch-set and the submitted one. -- To view, visit https://review.coreboot.org/c/coreboot/+/63185 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ibe52b44395619f697686bd900a522562abbe7646 Gerrit-Change-Number: 63185 Gerrit-PatchSet: 3 Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: Zheng Bao Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

Change in coreboot[master]: util/amdfwtool: select A/B recovery when ISH is used
by Felix Held (Code Review) 31 Mar '22

31 Mar '22

Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/63184 ) Change subject: util/amdfwtool: select A/B recovery when ISH is used ...................................................................... util/amdfwtool: select A/B recovery when ISH is used In newer AMD SoCs, the image slot header is used in the AMD A/B recovery scheme, so set recovery_ab to true when need_ish is true. Also move the block of code before the process_config call, since that call will already use the recovery_ab field of the cb_config struct. Signed-off-by: Felix Held <felix-coreboot(a)felixheld.de> Change-Id: I65903765514f215bf5cc9b949d0b95aff781eb34 Reviewed-on: https://review.coreboot.org/c/coreboot/+/63184 Reviewed-by: Raul Rangel <rrangel(a)chromium.org> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M util/amdfwtool/amdfwtool.c 1 file changed, 6 insertions(+), 4 deletions(-) Approvals: build bot (Jenkins): Verified Raul Rangel: Looks good to me, approved diff --git a/util/amdfwtool/amdfwtool.c b/util/amdfwtool/amdfwtool.c index 9e7f920..7c977a2 100644 --- a/util/amdfwtool/amdfwtool.c +++ b/util/amdfwtool/amdfwtool.c @@ -1684,6 +1684,12 @@ } } + if (cb_config.need_ish) + cb_config.recovery_ab = true; + + if (cb_config.recovery_ab) + cb_config.multi_level = true; + if (config) { config_handle = fopen(config, "r"); if (config_handle == NULL) { @@ -1724,10 +1730,6 @@ retval = 1; } - if (cb_config.recovery_ab) { - cb_config.multi_level = true; - } - if (retval) { usage(); return retval; -- To view, visit https://review.coreboot.org/c/coreboot/+/63184 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I65903765514f215bf5cc9b949d0b95aff781eb34 Gerrit-Change-Number: 63184 Gerrit-PatchSet: 3 Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> 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]: soc/amd/common/block/i2c/i23c_pad_ctrl: only configure mode and voltage
by Felix Held (Code Review) 31 Mar '22

31 Mar '22

Attention is currently required from: Jason Glenesk, Raul Rangel, Marshall Dawson, Fred Reitberger, Karthik Ramasubramanian. Hello Karthik Ramasubramanian, I'd like you to do a code review. Please visit https://review.coreboot.org/c/coreboot/+/63234 to review the following change. Change subject: soc/amd/common/block/i2c/i23c_pad_ctrl: only configure mode and voltage ...................................................................... soc/amd/common/block/i2c/i23c_pad_ctrl: only configure mode and voltage The fch_i23c_pad_init implementation was written without looking at any reference code and turned out to not work properly on hardware. Before this function writes to the MISC_I23C_PAD_CTRL registers, the value read back is 0x3000003c which results in the I2C bus communication to work while the 0x300003fc the code writes to the register breaks the I2C communication. Removing the code that sets bits 6..9 fixes the I2C bus communication. Signed-off-by: Felix Held <felix-coreboot(a)felixheld.de> Tested-by: Karthikeyan Ramasubramanian <kramasub(a)google.com> Change-Id: Ie6758b3d13c59b20ce810225fca8a365713b7a2b --- M src/soc/amd/common/block/i2c/i23c_pad_ctrl.c 1 file changed, 0 insertions(+), 7 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/34/63234/1 diff --git a/src/soc/amd/common/block/i2c/i23c_pad_ctrl.c b/src/soc/amd/common/block/i2c/i23c_pad_ctrl.c index 2439054..bff6223 100644 --- a/src/soc/amd/common/block/i2c/i23c_pad_ctrl.c +++ b/src/soc/amd/common/block/i2c/i23c_pad_ctrl.c @@ -44,12 +44,5 @@ break; } - pad_ctrl &= ~I23C_PAD_CTRL_FALLSLEW_SEL_MASK; - pad_ctrl |= speed == I2C_SPEED_STANDARD ? - I23C_PAD_CTRL_FALLSLEW_SEL_STD : I23C_PAD_CTRL_FALLSLEW_SEL_LOW; - - pad_ctrl &= ~I23C_PAD_CTRL_SLEW_N_MASK; - pad_ctrl |= I23C_PAD_CTRL_SLEW_N_FAST; - misc_write32(MISC_I23C_PAD_CTRL(bus), pad_ctrl); } -- To view, visit https://review.coreboot.org/c/coreboot/+/63234 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ie6758b3d13c59b20ce810225fca8a365713b7a2b Gerrit-Change-Number: 63234 Gerrit-PatchSet: 1 Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org> Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org> Gerrit-Attention: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Attention: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-MessageType: newchange

1 0

Change in coreboot[master]: soc/amd/sabrina/i2c: handle all I2C pads as I23C pad type
by Felix Held (Code Review) 31 Mar '22

31 Mar '22

Attention is currently required from: Jason Glenesk, Raul Rangel, Marshall Dawson, Fred Reitberger. Felix Held has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/63233 ) Change subject: soc/amd/sabrina/i2c: handle all I2C pads as I23C pad type ...................................................................... soc/amd/sabrina/i2c: handle all I2C pads as I23C pad type Contradicting the PPR #57243 version 1.56, the I2C3 pad control register in the MISC ACPIMMIO region is the same new I23C pad type as the corresponding registers for I2C0..2 and not the older I2C pad control register type used on Picasso and Cezanne. All I2C pads being of the new I23C type is in line with the GPIOMUX settings for the pins used by I2C0..3 that can alternatively connect the pins to an I3C controller. Signed-off-by: Felix Held <felix-coreboot(a)felixheld.de> Change-Id: I51b0ddf8ba2ccfee823e3d4d26a77b11825b1029 --- M src/soc/amd/sabrina/Kconfig M src/soc/amd/sabrina/i2c.c 2 files changed, 1 insertion(+), 7 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/33/63233/1 diff --git a/src/soc/amd/sabrina/Kconfig b/src/soc/amd/sabrina/Kconfig index 3df7a4b..486ec22 100644 --- a/src/soc/amd/sabrina/Kconfig +++ b/src/soc/amd/sabrina/Kconfig @@ -53,7 +53,6 @@ select SOC_AMD_COMMON_BLOCK_GRAPHICS # TODO: Check if this is still correct select SOC_AMD_COMMON_BLOCK_HAS_ESPI # TODO: Check if this is still correct select SOC_AMD_COMMON_BLOCK_I2C - select SOC_AMD_COMMON_BLOCK_I2C_PAD_CTRL select SOC_AMD_COMMON_BLOCK_I23C_PAD_CTRL select SOC_AMD_COMMON_BLOCK_IOMMU select SOC_AMD_COMMON_BLOCK_LPC # TODO: Check if this is still correct diff --git a/src/soc/amd/sabrina/i2c.c b/src/soc/amd/sabrina/i2c.c index 1008f56..e805eff 100644 --- a/src/soc/amd/sabrina/i2c.c +++ b/src/soc/amd/sabrina/i2c.c @@ -39,12 +39,7 @@ if (bus >= ARRAY_SIZE(config->i2c_pad)) return; - /* The I/O pads of I2C0..2 are the new I23C pads and the I/O pads of I2C3 still are the - same I2C pads as in Picasso and Cezanne. */ - if (bus <= 2) - fch_i23c_pad_init(bus, cfg->speed, &config->i2c_pad[bus]); - else - fch_i2c_pad_init(bus, cfg->speed, &config->i2c_pad[bus]); + fch_i23c_pad_init(bus, cfg->speed, &config->i2c_pad[bus]); } const struct soc_i2c_ctrlr_info *soc_get_i2c_ctrlr_info(size_t *num_ctrlrs) -- To view, visit https://review.coreboot.org/c/coreboot/+/63233 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I51b0ddf8ba2ccfee823e3d4d26a77b11825b1029 Gerrit-Change-Number: 63233 Gerrit-PatchSet: 1 Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Fred Reitberger <reitbergerfred(a)gmail.com> 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-Attention: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org> Gerrit-Attention: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Attention: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-MessageType: newchange

1 0

Change in coreboot[master]: soc/amd/sabrina/include/gpio: add I3C3 IOMUX definitions
by Felix Held (Code Review) 31 Mar '22

31 Mar '22

Felix Held has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/63232 ) Change subject: soc/amd/sabrina/include/gpio: add I3C3 IOMUX definitions ...................................................................... soc/amd/sabrina/include/gpio: add I3C3 IOMUX definitions According to PPR #57243 version 1.56, the IOMUX setting 2 of the pins 19 and 20 is the I3C3 controller and not the I2C3 controller. Signed-off-by: Felix Held <felix-coreboot(a)felixheld.de> Change-Id: I9688f1816aa840c64441495ed451997a474b306f --- M src/soc/amd/sabrina/include/soc/gpio.h 1 file changed, 2 insertions(+), 2 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/32/63232/1 diff --git a/src/soc/amd/sabrina/include/soc/gpio.h b/src/soc/amd/sabrina/include/soc/gpio.h index fbb19cd..1cd72cd 100644 --- a/src/soc/amd/sabrina/include/soc/gpio.h +++ b/src/soc/amd/sabrina/include/soc/gpio.h @@ -148,11 +148,11 @@ #define GPIO_18_IOMUX_GPIOxx 1 #define GPIO_19_IOMUX_SMBUS1_SCL 0 #define GPIO_19_IOMUX_I2C3_SCL 1 -/* GPIO 19 IOMUX == 2 is also I2C3_SCL */ +#define GPIO_19_IOMUX_I3C3_SCL 2 #define GPIO_19_IOMUX_GPIOxx 3 #define GPIO_20_IOMUX_SMBUS1_SDA 0 #define GPIO_20_IOMUX_I2C3_SDA 1 -/* GPIO 19 IOMUX == 2 is also I2C3_SDA */ +#define GPIO_20_IOMUX_I3C3_SDA 2 #define GPIO_20_IOMUX_GPIOxx 3 #define GPIO_21_IOMUX_ESPI_RESET_L 0 #define GPIO_21_IOMUX_KBRST_L 1 -- To view, visit https://review.coreboot.org/c/coreboot/+/63232 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I9688f1816aa840c64441495ed451997a474b306f Gerrit-Change-Number: 63232 Gerrit-PatchSet: 1 Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-MessageType: newchange

1 0

Change in coreboot[master]: soc/intel/common: implement ioc driver
by Wonkyu Kim (Code Review) 31 Mar '22

31 Mar '22

Attention is currently required from: Jamie Ryu, Subrata Banik, Ethan Tsao, Ravishankar Sarawadi, Paul Menzel, Nick Vaccaro, Tim Wawrzynczak. Wonkyu Kim has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/63198 ) Change subject: soc/intel/common: implement ioc driver ...................................................................... Patch Set 3: (1 comment) File src/soc/intel/common/block/ioc/Kconfig: https://review.coreboot.org/c/coreboot/+/63198/comment/063dead7_e4e2004c PS2, Line 4: Intel Processor common IO Cache(IOC) > Please add a space before (. Ack -- To view, visit https://review.coreboot.org/c/coreboot/+/63198 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I768027c2ca78310c03845f70f17df19dc8cd0982 Gerrit-Change-Number: 63198 Gerrit-PatchSet: 3 Gerrit-Owner: Wonkyu Kim <wonkyu.kim(a)intel.com> Gerrit-Reviewer: Ethan Tsao <ethan.tsao(a)intel.com> Gerrit-Reviewer: Jamie Ryu <jamie.m.ryu(a)intel.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Reviewer: Ravishankar Sarawadi <ravishankar.sarawadi(a)intel.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Jamie Ryu <jamie.m.ryu(a)intel.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Ethan Tsao <ethan.tsao(a)intel.com> Gerrit-Attention: Ravishankar Sarawadi <ravishankar.sarawadi(a)intel.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)google.com> Gerrit-Comment-Date: Wed, 30 Mar 2022 23:38:17 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: tpm: Accept Google Ti50 TPM DID:VID
by Julius Werner (Code Review) 31 Mar '22

31 Mar '22

Attention is currently required from: Jes Klinke, Christian Walter, Jett Rink. Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/63158 ) Change subject: tpm: Accept Google Ti50 TPM DID:VID ...................................................................... Patch Set 6: (6 comments) File src/drivers/crb/tis.c: https://review.coreboot.org/c/coreboot/+/63158/comment/e8246a15_e4d8e55d PS6, Line 20: {0x6666, 0x50a4, "TI50"}, nit: I don't think this really needs to be here, and Cr50 should've never been here either. "crb" is Intel's built-in TPM solution. File src/drivers/i2c/tpm/cr50.c: https://review.coreboot.org/c/coreboot/+/63158/comment/44eb79ec_5dbc0fa7 PS6, Line 478: gsc It's a bit odd to change only this and none of the other instances of "cr50" in here. I would say if we want to rename things, we should do it everywhere consistently. https://review.coreboot.org/c/coreboot/+/63158/comment/2c3773c7_b39a3c27 PS6, Line 509: cr50_set_board_cfg(); /* No-op for Ti50. */ Does this mean we send the command and Ti50 just ignores it? That sounds like a waste of boot time. If the AP knows that this is only needed for Cr50, it should just not send it for Ti50. File src/drivers/spi/tpm/tpm.c: https://review.coreboot.org/c/coreboot/+/63158/comment/b0cdf4d7_55301a67 PS6, Line 422: H1 based Is it? File src/drivers/tpm/cr50.h: https://review.coreboot.org/c/coreboot/+/63158/comment/4e8efaeb_c17d355a PS6, Line 11: GSC_TI50 = 2, I'm wondering if we really need to handle this differentiation at runtime? Long-term (beyond the current bring-up experimentation) we're never planning to support both on the same boot target, right? I think if you just made a MAINBOARD_HAS_TI50_I2C_TPM next to the current MAINBOARD_HAS_CR50_I2C_TPM and hooked the difference off that, you could skip a bunch of the complexity here. File src/drivers/tpm/cr50.c: https://review.coreboot.org/c/coreboot/+/63158/comment/0a2b16fa_ac91a54e PS6, Line 249: INFO ERR? -- To view, visit https://review.coreboot.org/c/coreboot/+/63158 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I1e1f8eb9b94fc2d5689656335dc1135b47880986 Gerrit-Change-Number: 63158 Gerrit-PatchSet: 6 Gerrit-Owner: Jes Klinke <jbk(a)chromium.org> Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Jett Rink <jettrink(a)chromium.org> Gerrit-CC: Jett Rink <jettrink(a)google.com> Gerrit-Attention: Jes Klinke <jbk(a)chromium.org> Gerrit-Attention: Christian Walter <christian.walter(a)9elements.com> Gerrit-Attention: Jett Rink <jettrink(a)google.com> Gerrit-Comment-Date: Wed, 30 Mar 2022 23:12:06 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: mb/google/brya/var/agah: Replace amp max98309 with max98360
by Tony Huang (Code Review) 31 Mar '22

31 Mar '22

Attention is currently required from: Wisley Chen. Tony Huang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/63196 ) Change subject: mb/google/brya/var/agah: Replace amp max98309 with max98360 ...................................................................... Patch Set 2: (1 comment) Commit Message: https://review.coreboot.org/c/coreboot/+/63196/comment/f36b6a62_93859b67 PS1, Line 9: Based on the latest schematic, replace amp max98390 with max98360. > suggestion: […] Hi Tim. Thanks for the suggestion and it's done. -- To view, visit https://review.coreboot.org/c/coreboot/+/63196 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I7ea36d276f7ffeae1510483027092e2bc59690fc Gerrit-Change-Number: 63196 Gerrit-PatchSet: 2 Gerrit-Owner: Tony Huang <tony-huang(a)quanta.corp-partner.google.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: Wisley Chen <wisley.chen(a)quanta.corp-partner.google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Wisley Chen <wisley.chen(a)quanta.corp-partner.google.com> Gerrit-Comment-Date: Wed, 30 Mar 2022 23:04:51 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-MessageType: comment

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit March 2022