coreboot-gerrit June 2021

coreboot-gerrit@coreboot.org

1 participants
7127 discussions

Change in coreboot[master]: psp_verstage: initialize i2c in soc_init
by Furquan Shaikh (Code Review) 03 Jun '21

03 Jun '21

Attention is currently required from: Jason Glenesk, Raul Rangel, Marshall Dawson, Kangheui Won, Felix Held. Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/55136 ) Change subject: psp_verstage: initialize i2c in soc_init ...................................................................... Patch Set 2: (1 comment) File src/soc/amd/picasso/psp_verstage/chipset.c: https://review.coreboot.org/c/coreboot/+/55136/comment/e5123b4d_8ca0c039 PS2, Line 31: espi_setup(); Actually, zork is also doing the exact same work i.e. ``` » enable_aoac_devices(); » setup_i2c(); ``` in verstage_mainboard_init(). Do you think it makes sense to simply add this to psp_verstage/chipset.c? -- To view, visit https://review.coreboot.org/c/coreboot/+/55136 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I5f7b73be67a692ea7de31ae53bd111d0e4b6998c Gerrit-Change-Number: 55136 Gerrit-PatchSet: 2 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: Furquan Shaikh <furquan(a)google.com> Gerrit-CC: Martin Roth <martinroth(a)google.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.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: Kangheui Won <khwon(a)chromium.org> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Comment-Date: Thu, 03 Jun 2021 03:51:56 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: psp_verstage: initialize i2c in soc_init
by Kangheui Won (Code Review) 03 Jun '21

03 Jun '21

Attention is currently required from: Jason Glenesk, Raul Rangel, Furquan Shaikh, Marshall Dawson, Felix Held. Kangheui Won has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/55136 ) Change subject: psp_verstage: initialize i2c in soc_init ...................................................................... Patch Set 2: (1 comment) File src/mainboard/google/guybrush/verstage.c: https://review.coreboot.org/c/coreboot/+/55136/comment/2b1928f4_e085efc1 PS1, Line 28: if (CONFIG(VBOOT_STARTS_BEFORE_BOOTBLOCK)) { : enable_aoac_devices(); : printk(BIOS_DEBUG, "Setting up i2c\n"); : i2c_soc_early_init(); : printk(BIOS_DEBUG, "i2c setup\n"); : } > i2c_soc_early_init() takes care of initializing only those I2C buses which are marked for early_init […] >psp_verstage is specific to Chrome OS anyways Ah yes I missed this. Thanks for pointing it out. -- To view, visit https://review.coreboot.org/c/coreboot/+/55136 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I5f7b73be67a692ea7de31ae53bd111d0e4b6998c Gerrit-Change-Number: 55136 Gerrit-PatchSet: 2 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: Furquan Shaikh <furquan(a)google.com> Gerrit-CC: Martin Roth <martinroth(a)google.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org> Gerrit-Attention: Furquan Shaikh <furquan(a)google.com> Gerrit-Attention: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Comment-Date: Thu, 03 Jun 2021 03:49:18 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Furquan Shaikh <furquan(a)google.com> Comment-In-Reply-To: Kangheui Won <khwon(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: cezanne/psp_verstage: add reset/timer svc
by Kangheui Won (Code Review) 03 Jun '21

03 Jun '21

Attention is currently required from: Jason Glenesk, Raul Rangel, Marshall Dawson, Felix Held. Hello build bot (Jenkins), Jason Glenesk, Raul Rangel, Marshall Dawson, Felix Held, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/55137 to look at the new patch set (#2). Change subject: cezanne/psp_verstage: add reset/timer svc ...................................................................... cezanne/psp_verstage: add reset/timer svc The new cezanne PSP release added support for these svcs. So add those functionality back to cezanne psp_verstage. BUG=b:187906425 Signed-off-by: Kangheui Won <khwon(a)chromium.org> Change-Id: Id542f5ed0762f582ea966466d67ed938ecb9c1f0 --- M src/soc/amd/cezanne/psp_verstage/chipset.c M src/soc/amd/cezanne/psp_verstage/svc.c M src/soc/amd/common/psp_verstage/Makefile.inc M src/vendorcode/amd/fsp/cezanne/include/bl_uapp/bl_syscall_public.h 4 files changed, 55 insertions(+), 21 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/37/55137/2 -- To view, visit https://review.coreboot.org/c/coreboot/+/55137 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Id542f5ed0762f582ea966466d67ed938ecb9c1f0 Gerrit-Change-Number: 55137 Gerrit-PatchSet: 2 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: Paul Menzel <paulepanter(a)mailbox.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: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-MessageType: newpatchset

1 0

Change in coreboot[master]: psp_verstage: initialize i2c in soc_init
by Kangheui Won (Code Review) 03 Jun '21

03 Jun '21

Attention is currently required from: Raul Rangel, Kangheui Won, Felix Held. Hello build bot (Jenkins), Raul Rangel, Felix Held, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/55136 to look at the new patch set (#2). Change subject: psp_verstage: initialize i2c in soc_init ...................................................................... psp_verstage: initialize i2c in soc_init GSC is connected with AP via i2c bus so we need to enable i2c in psp_verstage. Also split out verstage_soc_init into chipset.c since it makes more sense to have separate versions for different generations. Signed-off-by: Kangheui Won <khwon(a)chromium.org> Change-Id: I5f7b73be67a692ea7de31ae53bd111d0e4b6998c --- M src/mainboard/google/guybrush/verstage.c M src/soc/amd/cezanne/psp_verstage/chipset.c M src/soc/amd/common/psp_verstage/fch.c M src/soc/amd/picasso/psp_verstage/chipset.c 4 files changed, 21 insertions(+), 10 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/36/55136/2 -- To view, visit https://review.coreboot.org/c/coreboot/+/55136 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I5f7b73be67a692ea7de31ae53bd111d0e4b6998c Gerrit-Change-Number: 55136 Gerrit-PatchSet: 2 Gerrit-Owner: Kangheui Won <khwon(a)chromium.org> 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-CC: Furquan Shaikh <furquan(a)google.com> Gerrit-CC: Martin Roth <martinroth(a)google.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org> Gerrit-Attention: Kangheui Won <khwon(a)chromium.org> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-MessageType: newpatchset

1 0

Change in coreboot[master]: mb/google/brya/brya0: Fix irq and CS lines for FPMCU
by EricR Lai (Code Review) 03 Jun '21

03 Jun '21

Attention is currently required from: Tim Wawrzynczak. EricR Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/55147 ) Change subject: mb/google/brya/brya0: Fix irq and CS lines for FPMCU ...................................................................... Patch Set 2: (1 comment) File src/mainboard/google/brya/variants/brya0/overridetree.cb: https://review.coreboot.org/c/coreboot/+/55147/comment/1208bd44_ae71aafd PS2, Line 333: device spi 0 on end > so, not gspi1 need to change device spi1.. […] Ack -- To view, visit https://review.coreboot.org/c/coreboot/+/55147 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I9ef6c99f011969fc444e0c12b806529cb82bba3d Gerrit-Change-Number: 55147 Gerrit-PatchSet: 2 Gerrit-Owner: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: EricR Lai <ericr_lai(a)compal.corp-partner.google.com> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Comment-Date: Thu, 03 Jun 2021 03:44:37 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: EricR Lai <ericr_lai(a)compal.corp-partner.google.com> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: mb/google/brya/brya0: Fix irq and CS lines for FPMCU
by EricR Lai (Code Review) 03 Jun '21

03 Jun '21

Attention is currently required from: Tim Wawrzynczak. EricR Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/55147 ) Change subject: mb/google/brya/brya0: Fix irq and CS lines for FPMCU ...................................................................... Patch Set 2: (1 comment) File src/mainboard/google/brya/variants/brya0/overridetree.cb: https://review.coreboot.org/c/coreboot/+/55147/comment/9d70f270_7eeb3b7e PS2, Line 333: device spi 0 on end so, not gspi1 need to change device spi1.. if we have tpm in gspi0, still set spi0 here? -- To view, visit https://review.coreboot.org/c/coreboot/+/55147 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I9ef6c99f011969fc444e0c12b806529cb82bba3d Gerrit-Change-Number: 55147 Gerrit-PatchSet: 2 Gerrit-Owner: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: EricR Lai <ericr_lai(a)compal.corp-partner.google.com> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Comment-Date: Thu, 03 Jun 2021 03:35:30 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: mb/google/brya/brya0: Fix irq and CS lines for FPMCU
by EricR Lai (Code Review) 03 Jun '21

03 Jun '21

Attention is currently required from: Tim Wawrzynczak. EricR Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/55147 ) Change subject: mb/google/brya/brya0: Fix irq and CS lines for FPMCU ...................................................................... Patch Set 2: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/55147 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I9ef6c99f011969fc444e0c12b806529cb82bba3d Gerrit-Change-Number: 55147 Gerrit-PatchSet: 2 Gerrit-Owner: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: EricR Lai <ericr_lai(a)compal.corp-partner.google.com> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Comment-Date: Thu, 03 Jun 2021 03:34:08 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in coreboot[master]: mb/google/guybrush/var/guybrush: Update GPIO configuration
by Eric Peers (Code Review) 03 Jun '21

03 Jun '21

Attention is currently required from: Raul Rangel, Furquan Shaikh, Martin Roth, Karthik Ramasubramanian. Eric Peers has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/55148 ) Change subject: mb/google/guybrush/var/guybrush: Update GPIO configuration ...................................................................... Patch Set 1: (3 comments) Commit Message: https://review.coreboot.org/c/coreboot/+/55148/comment/2bf8a2db_dfdb0460 PS1, Line 14: TP183 > Looks like this was configured as NF (SPI_CS3_L)? it was unused. Ought to comment as such. https://b.corp.google.com/issues/188542649#comment3 https://review.coreboot.org/c/coreboot/+/55148/comment/ff16546d_ddb32823 PS1, Line 20: TEST=Build Guybrush mainboard. probably need a boot board id #1 as well and confirm Speaker and reset are working. I realize we can't test id2 yet, but we can backtest id1. File src/mainboard/google/guybrush/variants/guybrush/gpio.c: https://review.coreboot.org/c/coreboot/+/55148/comment/15f9bee5_6ff3169d PS1, Line 14: HIGH > This was set to LOW because of the incorrect voltage rating. […] 1.8V target as per b/188542497 -- To view, visit https://review.coreboot.org/c/coreboot/+/55148 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I31523b3e03d2b59577f33eae548747834cfc98aa Gerrit-Change-Number: 55148 Gerrit-PatchSet: 1 Gerrit-Owner: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Eric Peers <epeers(a)google.com> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> 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: Furquan Shaikh <furquan(a)google.com> Gerrit-Attention: Martin Roth <martinroth(a)google.com> Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Comment-Date: Thu, 03 Jun 2021 03:15:36 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Furquan Shaikh <furquan(a)google.com> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: mb/google/guybrush: initialize i2c in psp_verstage
by Furquan Shaikh (Code Review) 03 Jun '21

03 Jun '21

Attention is currently required from: Raul Rangel, Kangheui Won, Felix Held. Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/55136 ) Change subject: mb/google/guybrush: initialize i2c in psp_verstage ...................................................................... Patch Set 1: (1 comment) File src/mainboard/google/guybrush/verstage.c: https://review.coreboot.org/c/coreboot/+/55136/comment/abfa5ec1_eceb92a2 PS1, Line 28: if (CONFIG(VBOOT_STARTS_BEFORE_BOOTBLOCK)) { : enable_aoac_devices(); : printk(BIOS_DEBUG, "Setting up i2c\n"); : i2c_soc_early_init(); : printk(BIOS_DEBUG, "i2c setup\n"); : } > I thought i2c is only used for the communication with GSC so the code is specific to guybrush. […] i2c_soc_early_init() takes care of initializing only those I2C buses which are marked for early_init. Also, psp_verstage is specific to Chrome OS anyways. I don't think it hurts to initialize the AOAC power for the aoac_devs as part of SoC code. It is anyways required for all Chrome OS mainboards. Also, these calls will be eventually duplicated across all Chrome OS mainboards which seems unnecessary. There is no mainboard specific element to it. -- To view, visit https://review.coreboot.org/c/coreboot/+/55136 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I5f7b73be67a692ea7de31ae53bd111d0e4b6998c Gerrit-Change-Number: 55136 Gerrit-PatchSet: 1 Gerrit-Owner: Kangheui Won <khwon(a)chromium.org> 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-CC: Furquan Shaikh <furquan(a)google.com> Gerrit-CC: Martin Roth <martinroth(a)google.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org> Gerrit-Attention: Kangheui Won <khwon(a)chromium.org> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Comment-Date: Thu, 03 Jun 2021 03:13:06 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Furquan Shaikh <furquan(a)google.com> Comment-In-Reply-To: Kangheui Won <khwon(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: mb/google/guybrush: Override I2C3 pad configuration
by Furquan Shaikh (Code Review) 03 Jun '21

03 Jun '21

Attention is currently required from: Raul Rangel, Martin Roth, Karthik Ramasubramanian. Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/55149 ) Change subject: mb/google/guybrush: Override I2C3 pad configuration ...................................................................... Patch Set 1: (2 comments) File src/mainboard/google/guybrush/Makefile.inc: https://review.coreboot.org/c/coreboot/+/55149/comment/8908e942_25bc10bc PS1, Line 5: This will have to be done in verstage.c too for PSP on verstage. File src/mainboard/google/guybrush/i2c.c: https://review.coreboot.org/c/coreboot/+/55149/comment/f8690157_e51e13ff PS1, Line 8: mainboard_i2c_override What do you think about moving this to a devicetree config? That will eliminate the need for the callback function and also not require mainboard code to manipulate SoC register bits. i.e. drive SoC configuration using controls in device tree. -- To view, visit https://review.coreboot.org/c/coreboot/+/55149 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I595a64736fdac0274abffb68c5e521302275b845 Gerrit-Change-Number: 55149 Gerrit-PatchSet: 1 Gerrit-Owner: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> 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: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Comment-Date: Thu, 03 Jun 2021 02:49:48 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit June 2021