Attention is currently required from: Ravi kumar.
Sajida Bhanu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/50586 )
Change subject: HACK sc7280: QSIP SPI NOR addressing mode HACK
......................................................................
Patch Set 4:
(2 comments)
File src/drivers/spi/spi_flash.c:
https://review.coreboot.org/c/coreboot/+/50586/comment/f65f7f2e_88aa2a60
PS4, Line 493: if (reg8 & 0x20){
> space required before the open brace '{'
sure
https://review.coreboot.org/c/coreboot/+/50586/comment/f9fe8af4_9627c04c
PS4, Line 497: while (1);
> trailing statements should be on next line
sure
--
To view, visit https://review.coreboot.org/c/coreboot/+/50586
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ied5b647d0fcc8e3effff3bb7c8680ed5a0c1f3d4
Gerrit-Change-Number: 50586
Gerrit-PatchSet: 4
Gerrit-Owner: Ravi kumar <rbokka(a)codeaurora.org>
Gerrit-Reviewer: Sajida Bhanu <sbhanu(a)codeaurora.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Attention: Ravi kumar <rbokka(a)codeaurora.org>
Gerrit-Comment-Date: Tue, 02 Mar 2021 06:59:35 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-MessageType: comment
Leah Rowe has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/51144 )
Change subject: util/chromeos: Use the same inventory each time and verify checksums
......................................................................
Patch Set 5:
(1 comment)
Patchset:
PS5:
> who is going to continually update the config file URL so that new boards are supported? […]
You are quite right. I will re-write the change so that it still checks sha1sums of files per the inventory, but I will re-write it to always use the latest inventory file.
--
To view, visit https://review.coreboot.org/c/coreboot/+/51144
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: If0166518d252a36d8f9a9c17d375fbdd9e3116f6
Gerrit-Change-Number: 51144
Gerrit-PatchSet: 5
Gerrit-Owner: Leah Rowe <leahleahrowerowe(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-CC: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Comment-Date: Tue, 02 Mar 2021 06:55:57 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-MessageType: comment
Attention is currently required from: Tim Wawrzynczak, Amanda Hwang, Ivy Jian, Patrick Rudolph, EricR Lai.
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/51167 )
Change subject: util: Add new memory part for brya boards
......................................................................
Patch Set 1:
(2 comments)
File util/spd_tools/lp4x/global_lp4x_mem_parts.json.txt:
https://review.coreboot.org/c/coreboot/+/51167/comment/651a9ceb_ae316889
PS1, Line 238: 4
Isn't this 2?
https://review.coreboot.org/c/coreboot/+/51167/comment/e11bbcca_ab9ee1f9
PS1, Line 239: 1
Isn't this 2? There are two ZQ pins ZQ0 and ZQ1.
--
To view, visit https://review.coreboot.org/c/coreboot/+/51167
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ic06e9d672a2d3db2b4ea12d15b462843c90db8f6
Gerrit-Change-Number: 51167
Gerrit-PatchSet: 1
Gerrit-Owner: Amanda Hwang <amanda_hwang(a)compal.corp-partner.google.com>
Gerrit-Reviewer: EricR Lai <ericr_lai(a)compal.corp-partner.google.com>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Ivy Jian <ivy_jian(a)compal.corp-partner.google.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Attention: Amanda Hwang <amanda_hwang(a)compal.corp-partner.google.com>
Gerrit-Attention: Ivy Jian <ivy_jian(a)compal.corp-partner.google.com>
Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Attention: EricR Lai <ericr_lai(a)compal.corp-partner.google.com>
Gerrit-Comment-Date: Tue, 02 Mar 2021 06:51:32 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Mary Ruthven, Joel Kitching, Furquan Shaikh, Vadim Bendebury, Christian Walter, Julius Werner.
Karthik Ramasubramanian has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/51164 )
Change subject: security/tpm/tss/vendor/cr50: Introduce vendor sub-command to reset EC
......................................................................
Patch Set 1:
(1 comment)
File src/security/tpm/tss/vendor/cr50/cr50.h:
https://review.coreboot.org/c/coreboot/+/51164/comment/92f07111_3c825b4b
PS1, Line 104: Issue a halt() after triggering EC reset because cr50 leaves 50 ms after receiving
: * the command to reset the EC.
> Should the function perform a halt() too instead of expecting the caller to do it?
I don't have a strong opinion. For error scenario, this function returns to the caller with the error code. I felt it will be consistent to return to the caller even on the success scenario.
--
To view, visit https://review.coreboot.org/c/coreboot/+/51164
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I46063678511d27fea5eabbd12fc3af0b1df68143
Gerrit-Change-Number: 51164
Gerrit-PatchSet: 1
Gerrit-Owner: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Joel Kitching <kitching(a)google.com>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: Mary Ruthven <mruthven(a)google.com>
Gerrit-Reviewer: Vadim Bendebury <vbendeb(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Mary Ruthven <mruthven(a)google.com>
Gerrit-Attention: Joel Kitching <kitching(a)google.com>
Gerrit-Attention: Furquan Shaikh <furquan(a)google.com>
Gerrit-Attention: Vadim Bendebury <vbendeb(a)google.com>
Gerrit-Attention: Christian Walter <christian.walter(a)9elements.com>
Gerrit-Attention: Julius Werner <jwerner(a)chromium.org>
Gerrit-Comment-Date: Tue, 02 Mar 2021 05:45:31 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Furquan Shaikh <furquan(a)google.com>
Gerrit-MessageType: comment
Attention is currently required from: Angel Pons, Subrata Banik, Michael Niewöhner, Patrick Rudolph.
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/51148 )
Change subject: soc/intel/cannonlake: Move `gpi_clear_int_cfg()` call
......................................................................
Patch Set 1: Code-Review+2
(2 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/51148/comment/5912c4a3_f9813aec
PS1, Line 9: To allow unifying bootblock.c in follow-ups, move a function call.
Change from `bootblock_soc_init()` to `bootblock_pch_init()` looks okay to me since it is moved after `report_platform_info()` instead of before.
File src/soc/intel/cannonlake/bootblock/pch.c:
https://review.coreboot.org/c/coreboot/+/51148/comment/95798b4c_bd3b181c
PS1, Line 139: gpi_clear_int_cfg
Not for this change:
This is something that is required for every platform. It has been added to cannonlake/ and apollolake/ based on the problems that were observed on certain mainboard configurations for these platforms. But, it is really required for all Intel platforms. Also, I don't think there is anything that requires this to be done early on in bootblock. I think what we should do is -- add a boot state step just before jumping to payload or resuming to OS to clearing the interrupt status/enable registers in soc/common/block/gpio.
--
To view, visit https://review.coreboot.org/c/coreboot/+/51148
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I0f40ee7fd47f7f9f582f314dfcd1b4b93b1db791
Gerrit-Change-Number: 51148
Gerrit-PatchSet: 1
Gerrit-Owner: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-Attention: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Comment-Date: Tue, 02 Mar 2021 05:45:10 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Leah Rowe has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/51144 )
Change subject: util/chromeos: Use the same inventory each time and verify checksums
......................................................................
Abandoned
I will re-write this to always use the latest inventory file from google, but still check sha1sums of individual recovery images from that inventory, when downloading recovery images. the issue with the current pull request is that it doesn't use the latest inventory; this is fine for my purposes (retroboot) but for coreboot, the latest one should always be used. Perhaps google has a page showing the sha1sum or something for the current inventory file?
--
To view, visit https://review.coreboot.org/c/coreboot/+/51144
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: If0166518d252a36d8f9a9c17d375fbdd9e3116f6
Gerrit-Change-Number: 51144
Gerrit-PatchSet: 5
Gerrit-Owner: Leah Rowe <leahleahrowerowe(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-CC: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-MessageType: abandon
Attention is currently required from: Martin Roth, Jamie Ryu, Rizwan Qureshi, Sridhar Siricilla, Krishna P Bhat D, Patrick Rudolph, Karthik Ramasubramanian.
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/50472 )
Change subject: elog: Support logging CSE Lite info in elog
......................................................................
Patch Set 6:
(2 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/50472/comment/38ab46b1_ad8ad1ee
PS6, Line 25: 2 | 2021-01-21 08:46:18 | CSE Info | Lite | 13.50.0.1269 | 13.50.0.1269
: | YES | 0x5 | 0x3 | RO
: 3 | 2021-01-21 08:46:21 | CSE Info | Lite | 13.50.0.1269 | 13.50.0.1269
: | YES | 0x5 | 0x0 | RW
These two entries can be a bit confusing. Should we think about adding these events in ramstage after cse_fw_sync happens so that if the platform reboots, we only capture this once i.e. after the sync?
https://review.coreboot.org/c/coreboot/+/50472/comment/84776f2e_e6b22772
PS6, Line 29: 4 | 2021-01-21 08:46:25 | System boot | 3098
Humm.. this is what I was referring to. "System boot" should be the first entry for a fresh boot and all events that happen after that. It is helpful in building the timeline.
--
To view, visit https://review.coreboot.org/c/coreboot/+/50472
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ib183941d1fa0a9971e30b4092ea3d23daaa29334
Gerrit-Change-Number: 50472
Gerrit-PatchSet: 6
Gerrit-Owner: Krishna P Bhat D <krishna.p.bhat.d(a)intel.com>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Jamie Ryu <jamie.m.ryu(a)intel.com>
Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Reviewer: Martin Roth <martinroth(a)google.com>
Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Rizwan Qureshi <rizwan.qureshi(a)intel.com>
Gerrit-Reviewer: Sridhar Siricilla <sridhar.siricilla(a)intel.com>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Attention: Martin Roth <martinroth(a)google.com>
Gerrit-Attention: Jamie Ryu <jamie.m.ryu(a)intel.com>
Gerrit-Attention: Rizwan Qureshi <rizwan.qureshi(a)intel.com>
Gerrit-Attention: Sridhar Siricilla <sridhar.siricilla(a)intel.com>
Gerrit-Attention: Krishna P Bhat D <krishna.p.bhat.d(a)intel.com>
Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Comment-Date: Tue, 02 Mar 2021 05:33:55 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Tim Wawrzynczak, EricR Lai.
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/51166 )
Change subject: drivers/generic/max98357a: Use depends HAVE_ACPI_TABLES
......................................................................
Patch Set 1: Code-Review+2
--
To view, visit https://review.coreboot.org/c/coreboot/+/51166
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ie6ebfde49f0f3c205e174c5113feb75444dedba8
Gerrit-Change-Number: 51166
Gerrit-PatchSet: 1
Gerrit-Owner: EricR Lai <ericr_lai(a)compal.corp-partner.google.com>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Attention: EricR Lai <ericr_lai(a)compal.corp-partner.google.com>
Gerrit-Comment-Date: Tue, 02 Mar 2021 05:29:16 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment